Hi,
On Fri, Apr 25, 2025 at 01:27:03PM +0300, Cristian Ciocaltea wrote:
> Provide a wrapper over drm_kunit_helper_enable_crtc_connector() to
> automatically handle EDEADLK.
>
> This is going to help improve the error handling in a bunch of test
> cases without open coding the restart of the atomic sequence.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocal...@collabora.com>
> ---
> drivers/gpu/drm/tests/drm_kunit_helpers.c | 39
> +++++++++++++++++++++++++++++++
> include/drm/drm_kunit_helpers.h | 7 ++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/tests/drm_kunit_helpers.c
> b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> index
> 5f7257840d8ef0aeabe5f00802f5037ed652ae66..4e1174c50b1f2b6358eb740cd73c6d86e53d0df9
> 100644
> --- a/drivers/gpu/drm/tests/drm_kunit_helpers.c
> +++ b/drivers/gpu/drm/tests/drm_kunit_helpers.c
> @@ -332,6 +332,45 @@ int drm_kunit_helper_enable_crtc_connector(struct kunit
> *test,
> }
> EXPORT_SYMBOL_GPL(drm_kunit_helper_enable_crtc_connector);
>
> +/**
> + * drm_kunit_helper_try_enable_crtc_connector - (Re)tries to enable a CRTC
> -> Connector output
> + * @test: The test context object
> + * @drm: The device to alloc the plane for
> + * @crtc: The CRTC to enable
> + * @connector: The Connector to enable
> + * @mode: The display mode to configure the CRTC with
> + * @ctx: Locking context
> + *
> + * This function is a wrapper over @drm_kunit_helper_enable_crtc_connector
> + * to automatically handle EDEADLK and (re)try to enable the route from
> + * @crtc to @connector, with the given @mode.
> + *
> + * Returns:
> + *
> + * A pointer to the new CRTC, or an ERR_PTR() otherwise.
> + */
> +int drm_kunit_helper_try_enable_crtc_connector(struct kunit *test,
> + struct drm_device *drm,
> + struct drm_crtc *crtc,
> + struct drm_connector *connector,
> + const struct drm_display_mode
> *mode,
> + struct drm_modeset_acquire_ctx
> *ctx)
> +{
> + int ret;
> +
> +retry_enable:
> + ret = drm_kunit_helper_enable_crtc_connector(test, drm, crtc, connector,
> + mode, ctx);
> + if (ret == -EDEADLK) {
> + ret = drm_modeset_backoff(ctx);
> + if (!ret)
> + goto retry_enable;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_kunit_helper_try_enable_crtc_connector);I'm not sure it's a good idea. This function might affect the locking context of the caller without even reporting it. Generally speaking, I'd really prefer to have explicit locking, even if it means slightly more boilerplate. Maxime
signature.asc
Description: PGP signature
