[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250328-radiant-azure-falcon-aafcf3@houat>
Date: Fri, 28 Mar 2025 16:32:59 +0100
From: Maxime Ripard <mripard@...nel.org>
To: Maíra Canal <mcanal@...lia.com>
Cc: Dave Stevenson <dave.stevenson@...pberrypi.com>,
Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] drm/vc4: tests: Stop allocating the state in test
init
On Sun, Mar 23, 2025 at 03:48:17PM -0300, Maíra Canal wrote:
> Hi Maxime,
>
> On 18/03/25 11:17, Maxime Ripard wrote:
> > The vc4-pv-muxing-combinations and vc5-pv-muxing-combinations test
> > suites use a common test init function which, in part, allocates the
> > drm atomic state the test will use.
> >
> > That allocation relies on drm_kunit_helper_atomic_state_alloc(), and
> > thus requires a struct drm_modeset_acquire_ctx. This context will then
> > be stored in the allocated state->acquire_ctx field.
> >
> > However, the context is local to the test init function, and is cleared
> > as soon as drm_kunit_helper_atomic_state_alloc() is done. We thus end up
> > with an dangling pointer to a cleared context in state->acquire_ctx for
> > our test to consumes.
> >
> > We should really allocate the context and the state in the test
> > functions, so we can also control when we're done with it.
> >
> > Fixes: 30188df0c387 ("drm/tests: Drop drm_kunit_helper_acquire_ctx_alloc()")
> > Signed-off-by: Maxime Ripard <mripard@...nel.org>
> > ---
> > drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 41 +++++++++++++++++---------
> > 1 file changed, 27 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
> > index 992e8f5c5c6ea8d92338a8fe739fa1115ff85338..52c04ef33206bf4f9e21e3c8b7cea932824a67fa 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
> > @@ -18,11 +18,10 @@
> > #include "vc4_mock.h"
> > struct pv_muxing_priv {
> > struct vc4_dev *vc4;
> > - struct drm_atomic_state *state;
>
> Can't we add `struct drm_modeset_acquire_ctx` here? Then, we can be sure
> that the context exists during the entire test case.
>
> Also, we can add `drm_modeset_drop_locks()` and
> `drm_modeset_acquire_fini()` to a exit function in the kunit suite.
That's what we were doing before, but the kunit functions and exit
functions are run in a separate thread by design, which then cause
lockdep to complain.
It's not great, but we can't really change either :/
Maxime
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists