[PATCH 1/2] drm/fsl-dcu: fix register initialization

[PATCH 1/2] drm/fsl-dcu: fix register initialization

[Stefan Agner]

The layer enumeration start with 0 (0-15 for LS1021a and 0-63 for
Vybrid) whereas the register enumeration start from 1 (1-10 for
LS1021a and 1-9 for Vybrid). The loop started off from 0 for both
iterations and initialized the number of layers inclusive, which
is one layer too many.

All extensively written registers seem to be unassigned, it seems
that the write to those registers did not do any harm in practice.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
This two patches apply cleanly on top of my earlier DCU fixes                    
patchset:                                                                        
https://lkml.org/lkml/2015/11/18/953

 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index db69725..3c92889 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -169,8 +169,8 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 		reg_num = LS1021A_LAYER_REG_NUM;
 	else
 		reg_num = VF610_LAYER_REG_NUM;
-	for (i = 0; i <= fsl_dev->soc->total_layer; i++) {
-		for (j = 0; j < reg_num; j++)
+	for (i = 0; i < fsl_dev->soc->total_layer; i++) {
+		for (j = 1; j <= reg_num; j++)
 			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
 	}
 	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/fsl-dcu: use flat regmap cache

[Stefan Agner]

Using flat regmap cache instead of RB-tree to avoid the following
lockdep warning on driver load:
[    0.697285] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755 lockdep_trace_alloc+0x15c/0x160()
[    0.697449] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))

The RB-tree regmap cache needs to allocate new space on first
writes. However, allocations in an atomic context (e.g. when a
spinlock is held) are not allowed. The function regmap_write
calls map->lock, which acquires a spinlock in the fast_io case.
Since the FSL DCU driver uses MMIO, the regmap bus of type
regmap_mmio is being used which has fast_io set to true.

The MMIO space of the DCU driver is reasonable condense, hence
using the much faster flat regmap cache is anyway the better
choice.

Signed-off-by: Stefan Agner <stefan@agner.ch>
Cc: Mark Brown <broonie@kernel.org>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 20 ++++++++++++--------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  6 ++++--
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index e01c813..971c520 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -36,11 +36,11 @@ static bool fsl_dcu_drm_is_volatile_reg(struct device *dev, unsigned int reg)
 	return false;
 }
 
-static const struct regmap_config fsl_dcu_regmap_config = {
+static struct regmap_config fsl_dcu_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
 	.val_bits = 32,
-	.cache_type = REGCACHE_RBTREE,
+	.cache_type = REGCACHE_FLAT,
 
 	.volatile_reg = fsl_dcu_drm_is_volatile_reg,
 };
@@ -260,12 +260,14 @@ static const struct fsl_dcu_soc_data fsl_dcu_ls1021a_data = {
 	.name = "ls1021a",
 	.total_layer = 16,
 	.max_layer = 4,
+	.max_register = LS1021A_DCU_MAX_REGISTER,
 };
 
 static const struct fsl_dcu_soc_data fsl_dcu_vf610_data = {
 	.name = "vf610",
 	.total_layer = 64,
 	.max_layer = 6,
+	.max_register = VF610_DCU_MAX_REGISTER,
 };
 
 static const struct of_device_id fsl_dcu_of_match[] = {
@@ -331,6 +333,14 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	id = of_match_node(fsl_dcu_of_match, pdev->dev.of_node);
+	if (!id)
+		return -ENODEV;
+
+	drm = drm_dev_alloc(driver, dev);
+	fsl_dev->soc = id->data;
+
+	fsl_dcu_regmap_config.max_register = fsl_dev->soc->max_register;
 	fsl_dev->regmap = devm_regmap_init_mmio(dev, base,
 			&fsl_dcu_regmap_config);
 	if (IS_ERR(fsl_dev->regmap)) {
@@ -338,12 +348,6 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 		return PTR_ERR(fsl_dev->regmap);
 	}
 
-	id = of_match_node(fsl_dcu_of_match, pdev->dev.of_node);
-	if (!id)
-		return -ENODEV;
-	fsl_dev->soc = id->data;
-
-	drm = drm_dev_alloc(driver, dev);
 	if (!drm)
 		return -ENOMEM;
 
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index 2a724f3..7c296a0 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -114,8 +114,6 @@
 #define DCU_UPDATE_MODE_MODE		BIT(31)
 #define DCU_UPDATE_MODE_READREG		BIT(30)
 
-#define DCU_DCFB_MAX			0x300
-
 #define DCU_CTRLDESCLN(layer, reg)	(0x200 + (reg - 1) * 4 + (layer) * 0x40)
 
 #define DCU_LAYER_HEIGHT(x)		((x) << 16)
@@ -155,6 +153,9 @@
 #define DCU_LAYER_POST_SKIP(x)		((x) << 16)
 #define DCU_LAYER_PRE_SKIP(x)		(x)
 
+#define VF610_DCU_MAX_REGISTER		0x11fc
+#define LS1021A_DCU_MAX_REGISTER	0x5fc
+
 #define FSL_DCU_RGB565			4
 #define FSL_DCU_RGB888			5
 #define FSL_DCU_ARGB8888		6
@@ -175,6 +176,7 @@ struct fsl_dcu_soc_data {
 	unsigned int total_layer;
 	/*max layer number DCU supported*/
 	unsigned int max_layer;
+	unsigned int max_register;
 };
 
 struct fsl_dcu_drm_device {
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/fsl-dcu: use flat regmap cache

[Stefan Agner]

On 2016-01-20 18:50, Stefan Agner wrote:
> Using flat regmap cache instead of RB-tree to avoid the following
> lockdep warning on driver load:
> [    0.697285] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755
> lockdep_trace_alloc+0x15c/0x160()
> [    0.697449] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
> 
> The RB-tree regmap cache needs to allocate new space on first
> writes. However, allocations in an atomic context (e.g. when a
> spinlock is held) are not allowed. The function regmap_write
> calls map->lock, which acquires a spinlock in the fast_io case.
> Since the FSL DCU driver uses MMIO, the regmap bus of type
> regmap_mmio is being used which has fast_io set to true.
> 
> The MMIO space of the DCU driver is reasonable condense, hence
> using the much faster flat regmap cache is anyway the better
> choice.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Cc: Mark Brown <broonie@kernel.org>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 20 ++++++++++++--------
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  6 ++++--
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index e01c813..971c520 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -36,11 +36,11 @@ static bool fsl_dcu_drm_is_volatile_reg(struct
> device *dev, unsigned int reg)
>  	return false;
>  }
>  
> -static const struct regmap_config fsl_dcu_regmap_config = {
> +static struct regmap_config fsl_dcu_regmap_config = {
>  	.reg_bits = 32,
>  	.reg_stride = 4,
>  	.val_bits = 32,
> -	.cache_type = REGCACHE_RBTREE,
> +	.cache_type = REGCACHE_FLAT,
>  
>  	.volatile_reg = fsl_dcu_drm_is_volatile_reg,
>  };
> @@ -260,12 +260,14 @@ static const struct fsl_dcu_soc_data
> fsl_dcu_ls1021a_data = {
>  	.name = "ls1021a",
>  	.total_layer = 16,
>  	.max_layer = 4,
> +	.max_register = LS1021A_DCU_MAX_REGISTER,
>  };
>  
>  static const struct fsl_dcu_soc_data fsl_dcu_vf610_data = {
>  	.name = "vf610",
>  	.total_layer = 64,
>  	.max_layer = 6,
> +	.max_register = VF610_DCU_MAX_REGISTER,
>  };
>  
>  static const struct of_device_id fsl_dcu_of_match[] = {
> @@ -331,6 +333,14 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	id = of_match_node(fsl_dcu_of_match, pdev->dev.of_node);
> +	if (!id)
> +		return -ENODEV;
> +
> +	drm = drm_dev_alloc(driver, dev);
> +	fsl_dev->soc = id->data;
> +
> +	fsl_dcu_regmap_config.max_register = fsl_dev->soc->max_register;
>  	fsl_dev->regmap = devm_regmap_init_mmio(dev, base,
>  			&fsl_dcu_regmap_config);
>  	if (IS_ERR(fsl_dev->regmap)) {
> @@ -338,12 +348,6 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>  		return PTR_ERR(fsl_dev->regmap);
>  	}
>  
> -	id = of_match_node(fsl_dcu_of_match, pdev->dev.of_node);
> -	if (!id)
> -		return -ENODEV;
> -	fsl_dev->soc = id->data;
> -
> -	drm = drm_dev_alloc(driver, dev);
>  	if (!drm)
>  		return -ENOMEM;


The drm_dev_alloc call should be close to its error handling, and moving
is anyway not necessary...

Will fix that in v2

--
Stefan

>  
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> index 2a724f3..7c296a0 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -114,8 +114,6 @@
>  #define DCU_UPDATE_MODE_MODE		BIT(31)
>  #define DCU_UPDATE_MODE_READREG		BIT(30)
>  
> -#define DCU_DCFB_MAX			0x300
> -
>  #define DCU_CTRLDESCLN(layer, reg)	(0x200 + (reg - 1) * 4 + (layer) * 0x40)
>  
>  #define DCU_LAYER_HEIGHT(x)		((x) << 16)
> @@ -155,6 +153,9 @@
>  #define DCU_LAYER_POST_SKIP(x)		((x) << 16)
>  #define DCU_LAYER_PRE_SKIP(x)		(x)
>  
> +#define VF610_DCU_MAX_REGISTER		0x11fc
> +#define LS1021A_DCU_MAX_REGISTER	0x5fc
> +
>  #define FSL_DCU_RGB565			4
>  #define FSL_DCU_RGB888			5
>  #define FSL_DCU_ARGB8888		6
> @@ -175,6 +176,7 @@ struct fsl_dcu_soc_data {
>  	unsigned int total_layer;
>  	/*max layer number DCU supported*/
>  	unsigned int max_layer;
> +	unsigned int max_register;
>  };
>  
>  struct fsl_dcu_drm_device {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel