Re: [PATCH v2] drm: Allow determining if current task is output poll worker

Lyude Paul
2018-02-14T19:07:56Z

I think your idea of having the extra kerneldoc as a seperate patch to make
this easier to backport should work fine :). Thanks for the good work!

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Wed, 2018-02-14 at 08:41 +0100, Lukas Wunner wrote:
> Introduce a helper to determine if the current task is an output poll
> worker.
>
> This allows us to fix a long-standing deadlock in several DRM drivers
> wherein the ->runtime_suspend callback waits for the output poll worker
> to finish and the worker in turn calls a ->detect callback which waits
> for runtime suspend to finish. The ->detect callback is invoked from
> multiple call sites and waiting for runtime suspend to finish is the
> correct thing to do except if it's executing in the context of the
> worker.
>
> v2: Expand kerneldoc to specifically mention deadlock between
> output poll worker and autosuspend worker as use case. (Lyude)
>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++++++
> include/drm/drm_crtc_helper.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 6dc2dde5b672..7a6b2dc08913 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -654,6 +654,26 @@ static void output_poll_execute(struct work_struct
> *work)
> schedule_delayed_work(delayed_work,
> DRM_OUTPUT_POLL_PERIOD);
> }
>
> +/**
> + * drm_kms_helper_is_poll_worker - is %current task an output poll worker?
> + *
> + * Determine if %current task is an output poll worker. This can be used
> + * to select distinct code paths for output polling versus other contexts.
> + *
> + * One use case is to avoid a deadlock between the output poll worker and
> + * the autosuspend worker wherein the latter waits for polling to finish
> + * upon calling drm_kms_helper_poll_disable(), while the former waits for
> + * runtime suspend to finish upon calling pm_runtime_get_sync() in a
> + * connector ->detect hook.
> + */
> +bool drm_kms_helper_is_poll_worker(void)
> +{
> + struct work_struct *work = current_work();
> +
> + return work && work->func == output_poll_execute;
> +}
> +EXPORT_SYMBOL(drm_kms_helper_is_poll_worker);
> +
> /**
> * drm_kms_helper_poll_disable - disable output polling
> * @dev: drm_device
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 76e237bd989b..6914633037a5 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -77,5 +77,6 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev);
>
> void drm_kms_helper_poll_disable(struct drm_device *dev);
> void drm_kms_helper_poll_enable(struct drm_device *dev);
> +bool drm_kms_helper_is_poll_worker(void);
>
> #endif
--
Cheers,
Lyude Paul
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org