* [PATCH] drm/fsl-dcu: use flat regmap cache
@ 2016-06-03 22:43 Stefan Agner
2016-06-03 23:00 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Agner @ 2016-06-03 22:43 UTC (permalink / raw)
To: airlied, dri-devel
Cc: alison.wang, jianwei.wang.chn, meng.yi, alexander.stein,
Stefan Agner, Mark Brown, stable
Using flat regmap cache instead of RB-tree to avoid the following
lockdep warning on driver load:
WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755 lockdep_trace_alloc+0x15c/0x160()
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.
Use flat regmap cache and specify max register to be large
enouth to cover all registers available in LS1021a and Vybrids
register space.
Signed-off-by: Stefan Agner <stefan@agner.ch>
Cc: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
While regmap cache is used for suspend/resume only (which is
broken in its current state) Mark noted that using the RB regmap
cache can also cause issues during initialization of the driver.
This patch migrates to flat regmap cache (which we can also use
to fix the issue in stable kernels), and yet another patchset
moves to the atomic suspend/resume helpers (which will not go
into stable...)
Dave, I saw that you just sent out the pull for rc2, will send
a pull request for rc3 early next week...
--
Stefan
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
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 0ec1ad9..dc723f7 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -42,9 +42,10 @@ static const 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,
+ .max_register = 0x11fc,
};
static int fsl_dcu_drm_irq_init(struct drm_device *dev)
--
2.8.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fsl-dcu: use flat regmap cache
2016-06-03 22:43 [PATCH] drm/fsl-dcu: use flat regmap cache Stefan Agner
@ 2016-06-03 23:00 ` Daniel Vetter
2016-06-04 1:51 ` Stefan Agner
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2016-06-03 23:00 UTC (permalink / raw)
To: Stefan Agner
Cc: airlied, dri-devel, jianwei.wang.chn, meng.yi, alison.wang,
Mark Brown, stable, alexander.stein
On Fri, Jun 03, 2016 at 03:43:19PM -0700, Stefan Agner wrote:
> Using flat regmap cache instead of RB-tree to avoid the following
> lockdep warning on driver load:
> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755 lockdep_trace_alloc+0x15c/0x160()
> 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.
>
> Use flat regmap cache and specify max register to be large
> enouth to cover all registers available in LS1021a and Vybrids
> register space.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
> While regmap cache is used for suspend/resume only (which is
> broken in its current state) Mark noted that using the RB regmap
> cache can also cause issues during initialization of the driver.
> This patch migrates to flat regmap cache (which we can also use
> to fix the issue in stable kernels), and yet another patchset
> moves to the atomic suspend/resume helpers (which will not go
> into stable...)
While talking suspend/resume, I highly recommend
drm_atomic_helper_suspend and drm_atomic_helper_resume. In my experience
dumb safe/restore of registers for restoring modeset state leads to tears.
-Daniel
>
> Dave, I saw that you just sent out the pull for rc2, will send
> a pull request for rc3 early next week...
>
> --
> Stefan
>
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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 0ec1ad9..dc723f7 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -42,9 +42,10 @@ static const 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,
> + .max_register = 0x11fc,
> };
>
> static int fsl_dcu_drm_irq_init(struct drm_device *dev)
> --
> 2.8.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fsl-dcu: use flat regmap cache
2016-06-03 23:00 ` Daniel Vetter
@ 2016-06-04 1:51 ` Stefan Agner
2016-06-04 10:12 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Agner @ 2016-06-04 1:51 UTC (permalink / raw)
To: Daniel Vetter
Cc: airlied, dri-devel, jianwei.wang.chn, meng.yi, alison.wang,
Mark Brown, stable, alexander.stein, Daniel Vetter
On 2016-06-03 16:00, Daniel Vetter wrote:
> On Fri, Jun 03, 2016 at 03:43:19PM -0700, Stefan Agner wrote:
>> Using flat regmap cache instead of RB-tree to avoid the following
>> lockdep warning on driver load:
>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755 lockdep_trace_alloc+0x15c/0x160()
>> 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.
>>
>> Use flat regmap cache and specify max register to be large
>> enouth to cover all registers available in LS1021a and Vybrids
>> register space.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: stable@vger.kernel.org
>> ---
>> While regmap cache is used for suspend/resume only (which is
>> broken in its current state) Mark noted that using the RB regmap
>> cache can also cause issues during initialization of the driver.
>> This patch migrates to flat regmap cache (which we can also use
>> to fix the issue in stable kernels), and yet another patchset
>> moves to the atomic suspend/resume helpers (which will not go
>> into stable...)
>
> While talking suspend/resume, I highly recommend
> drm_atomic_helper_suspend and drm_atomic_helper_resume. In my experience
> dumb safe/restore of registers for restoring modeset state leads to tears.
> -Daniel
Since you commented to the patchset implementing exactly that I assume
you have seen it.
I still would like to have this patch applied to fix the regmap cache
warning also for older kernels.
--
Stefan
>
>>
>> Dave, I saw that you just sent out the pull for rc2, will send
>> a pull request for rc3 early next week...
>>
>> --
>> Stefan
>>
>> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> 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 0ec1ad9..dc723f7 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -42,9 +42,10 @@ static const 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,
>> + .max_register = 0x11fc,
>> };
>>
>> static int fsl_dcu_drm_irq_init(struct drm_device *dev)
>> --
>> 2.8.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fsl-dcu: use flat regmap cache
2016-06-04 1:51 ` Stefan Agner
@ 2016-06-04 10:12 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2016-06-04 10:12 UTC (permalink / raw)
To: Stefan Agner
Cc: Dave Airlie, dri-devel, Jianwei Wang, Meng Yi, Huan Wang,
Mark Brown, stable, alexander.stein
On Sat, Jun 4, 2016 at 3:51 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-06-03 16:00, Daniel Vetter wrote:
>> On Fri, Jun 03, 2016 at 03:43:19PM -0700, Stefan Agner wrote:
>>> Using flat regmap cache instead of RB-tree to avoid the following
>>> lockdep warning on driver load:
>>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2755 lockdep_trace_alloc+0x15c/0x160()
>>> 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.
>>>
>>> Use flat regmap cache and specify max register to be large
>>> enouth to cover all registers available in LS1021a and Vybrids
>>> register space.
>>>
>>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>>> Cc: Mark Brown <broonie@kernel.org>
>>> Cc: stable@vger.kernel.org
>>> ---
>>> While regmap cache is used for suspend/resume only (which is
>>> broken in its current state) Mark noted that using the RB regmap
>>> cache can also cause issues during initialization of the driver.
>>> This patch migrates to flat regmap cache (which we can also use
>>> to fix the issue in stable kernels), and yet another patchset
>>> moves to the atomic suspend/resume helpers (which will not go
>>> into stable...)
>>
>> While talking suspend/resume, I highly recommend
>> drm_atomic_helper_suspend and drm_atomic_helper_resume. In my experience
>> dumb safe/restore of registers for restoring modeset state leads to tears.
>> -Daniel
>
> Since you commented to the patchset implementing exactly that I assume
> you have seen it.
Yup, that was pretty awesome coincidence ;-)
> I still would like to have this patch applied to fix the regmap cache
> warning also for older kernels.
Sure, this was just a quick comment on your remark that there's still
trouble left with suspend/resume.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-04 10:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-03 22:43 [PATCH] drm/fsl-dcu: use flat regmap cache Stefan Agner
2016-06-03 23:00 ` Daniel Vetter
2016-06-04 1:51 ` Stefan Agner
2016-06-04 10:12 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox