| Message ID | 1454461578-8649-2-git-send-email-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016-02-02 17:06, 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. As discussed with Thierry on IRC, using regmap cache often does not really make sense in the context of display controllers. While trying to use the suspend/resume, I realized that the current implementation in the DCU driver has bugs and does not work (e.g. an explicit READREG is missing, but even with that in place, restoring all registers and enable the controller then seem not to work reliable). Using the new suspend/resume helpers seem to work better... Therefor I NACK my own patch here and vote for removing the regcache entirely (and use the new DRM supsend/resume helpers to implement suspend/resume)... -- Stefan > > Signed-off-by: Stefan Agner <stefan@agner.ch> > Cc: Mark Brown <broonie@kernel.org> > --- > Changes since v1: > - Do not move drm_dev_alloc > > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 18 +++++++++++------- > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 6 ++++-- > 2 files changed, 15 insertions(+), 9 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..9c21aad 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,13 @@ 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; > + > + 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,11 +347,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 {
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..9c21aad 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,13 @@ 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; + + 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,11 +347,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 {
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> --- Changes since v1: - Do not move drm_dev_alloc drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 18 +++++++++++------- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 6 ++++-- 2 files changed, 15 insertions(+), 9 deletions(-)