Re: [PATCH 2/4] drm/vc4: tests: Document output handling functions

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2733 bytes --]

On Sun, Mar 23, 2025 at 03:34:50PM -0300, Maíra Canal wrote:
> Hi Maxime,
> 
> On 18/03/25 11:17, Maxime Ripard wrote:
> > vc4_mock_atomic_add_output() and vc4_mock_atomic_del_output() public but
> 
> Nit: s/public/are public

Good catch, thanks!

> > aren't documented. Let's provide the documentation.
> > 
> > In particular, special care should be taken to deal with EDEADLK.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >   drivers/gpu/drm/vc4/tests/vc4_mock_output.c | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > index f0ddc223c1f839e8a14f37fdcbb72e7b2c836aa1..577d9a9563696791632aec614c381a214886bf27 100644
> > --- a/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > +++ b/drivers/gpu/drm/vc4/tests/vc4_mock_output.c
> > @@ -59,10 +59,23 @@ struct vc4_dummy_output *vc4_dummy_output(struct kunit *test,
> >   static const struct drm_display_mode default_mode = {
> >   	DRM_SIMPLE_MODE(640, 480, 64, 48)
> >   };
> > +/**
> > + * vc4_mock_atomic_add_output() - Enables an output in a state
> > + * @test: The test context object
> > + * @state: Atomic state to enable the output in.
> > + * @type: Type of the output encoder
> > + *
> > + * Adds an output CRTC and connector to a state, and enables them.
> > + *
> > + * Returns:
> > + * 0 on success, a negative error code on failure. If the error is
> > + * EDEADLK, the entire atomic sequence must be restarted. All other
> > + * errors are fatal.
> > + */
> >   int vc4_mock_atomic_add_output(struct kunit *test,
> >   			       struct drm_atomic_state *state,
> >   			       enum vc4_encoder_type type)
> >   {
> >   	struct drm_device *drm = state->dev;
> > @@ -103,10 +116,23 @@ int vc4_mock_atomic_add_output(struct kunit *test,
> >   	crtc_state->active = true;
> >   	return 0;
> >   }
> > +/**
> > + * vc4_mock_atomic_del_output() - Disables an output in a state
> > + * @test: The test context object
> > + * @state: Atomic state to disable the output in.
> > + * @type: Type of the output encoder
> > + *
> > + * Adds an output CRTC and connector to a state, and disables them.
> 
> I'm not sure if "Adds" properly expresses what the function does. How
> about "Finds an output CRTC and connector in a state, and disables
> them"?

Yeah, I see what you mean, but it doesn't really work either. The state
is empty before, we do try to find them to add a new state for them in
the global one, and that new state will disable them.

So we can't find them in that state prior to adding them, which is what
this function does.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]