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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <n2ojf77winz6b4kchmt6bnppomb6cpg4okrwnh6iibsemou4as@t5lhg3m24bjm>
Date: Fri, 16 May 2025 15:15:56 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Dave Stevenson <dave.stevenson@...pberrypi.com>, 
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, Dmitry Baryshkov <lumag@...nel.org>, kernel@...labora.com, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 12/23] drm/tests: helpers: Add a (re)try helper
 variant to enable CRTC connector

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@...labora.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

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)