[PATCH v4 12/23] drm/tests: helpers: Add a (re)try helper variant to enable CRTC connector

[Maxime Ripard]

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.ciocaltea at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 273 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250516/b6804552/attachment.sig>