| Message ID | 20250318-drm-vc4-kunit-failures-v1-2-779864d9ab37@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: tests: Fix locking failures | expand |
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 > 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"? Best Regards, - Maíra > + * > + * 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_del_output(struct kunit *test, > struct drm_atomic_state *state, > enum vc4_encoder_type type) > { > struct drm_device *drm = state->dev; >
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
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. + * + * 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_del_output(struct kunit *test, struct drm_atomic_state *state, enum vc4_encoder_type type) { struct drm_device *drm = state->dev;
vc4_mock_atomic_add_output() and vc4_mock_atomic_del_output() public but 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(+)