Hi Marek
On Mon, 7 Oct 2024 at 12:59, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>
> Hi Dave,
>
> On 21.06.2024 17:20, Dave Stevenson wrote:
> > From: Maxime Ripard <mripard@xxxxxxxxxx>
> >
> > With the introduction of the BCM2712 support, we will get yet another
> > generation of display engine to support.
> >
> > The binary check of whether it's VC5 or not thus doesn't work anymore,
> > especially since some parts of the driver will have changed with BCM2711,
> > and some others with BCM2712.
> >
> > Let's introduce an enum to store the generation the driver is running
> > on, which should provide more flexibility.
> >
> > Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
>
> This patch landed recently in linux-next as commit 24c5ed3ddf27
> ("drm/vc4: Introduce generation number enum"). In my tests I found that
> it introduces the following warning on Raspberry Pi3B+ board:
>
> ================================================
> WARNING: lock held when returning to user space!
> 6.11.0-rc5+ #15405 Tainted: G C
> ------------------------------------------------
> (udev-worker)/161 is leaving the kernel with locks still held!
> 1 lock held by (udev-worker)/161:
> #0: ffff80008338bff8 (drm_unplug_srcu){.?.?}-{0:0}, at:
> drm_dev_enter+0x0/0xdc
>
>
> I suspect that the error path is somewhere broken and the conversion
> triggered that path.
I must remember to test with lockdep debugging enabled.
> A wild guess (noticed by grepping for 'drm_dev_enter') is that the
> following chunk is broken:
>
> > diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> > index 933177cb8d66..7380a02a69a2 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> > @@ -224,7 +224,7 @@ static void vc4_hvs_lut_load(struct vc4_hvs *hvs,
> > if (!drm_dev_enter(drm, &idx))
> > return;
> >
> > - if (hvs->vc4->is_vc5)
> > + if (hvs->vc4->gen == VC4_GEN_4)
> > return;
> >
> > /* The LUT memory is laid out with each HVS channel in order,
>
> as changing the above check to 'if (hvs->vc4->gen > VC4_GEN_4)' fixes this issue (tested also on top of linux-next). I think that one has to review the checks again as well as the error paths in the driver.
It's actually "drm/vc4: hvs: Don't write gamma luts on 2711" that
introduced the problem - we need a drm_dev_exit before returning. The
conversion is actually wrong though, so that needs fixing too.
The same is true in vc4_hvs_atomic_flush that we have a return after
having called drm_dev_enter, and no drm_dev_exit.
It looks like I've got a job in the morning - thanks for the report.
Dave
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
