Re: [PATCH v2 1/6] drm/nouveau/kms/nv50-: Move DPCD backlight disable into its own function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The encoder atomic disable callback invokes sleeping functions via the backlight disable helper.
--

--- Patch [1]: [PATCH v2 1/6] drm/nouveau/kms/nv50-: Move DPCD backlight disable into its own function ---
commit a6ddaa482a749f2e2d18fe83acb2593aeea89961
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/kms/nv50-: Move DPCD backlight disable into its own function

This commit extracts the DPCD backlight disable logic from the
nv50_sor_atomic_disable() callback into its own separate helper function.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -1561,24 +1561,39 @@ nv50_sor_update(struct nouveau_encoder *nv_encoder, u8 head,

[ ... ]

> +#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
> +static inline void
> +nv50_sor_atomic_disable_backlight(struct nouveau_drm *drm,
> +				  struct nouveau_encoder *nv_encoder,
> +				  struct drm_atomic_commit *state)
> +{

[ ... ]

> +	ret = drm_edp_backlight_disable(&nv_connector->aux, &backlight->edp_info);
> +	if (ret < 0)
> +		NV_ERROR(drm, "Failed to disable backlight on [CONNECTOR:%d:%s]: %d\n",
> +			 nv_connector->base.base.id, nv_connector->base.name, ret);
> +}
> +#endif
> +
>  static void
>  nv50_sor_atomic_disable(struct drm_encoder *encoder, struct drm_atomic_commit *state)
>  {

[ ... ]

> +#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
> +	nv50_sor_atomic_disable_backlight(nouveau_drm(state->dev), nv_encoder, state);
>  #endif

[Severity: High]
This is a pre-existing issue, but extracting this logic into a helper
highlights a potential problem with the calling context.

Does this call chain execute a sleeping operation while in an atomic context?

The encoder atomic_disable callback runs in an atomic context where sleeping
is forbidden. However, nv50_sor_atomic_disable() invokes
nv50_sor_atomic_disable_backlight(), which then calls
drm_edp_backlight_disable().

The drm_edp_backlight_disable() function performs DP AUX transactions that
acquire aux->hw_mutex and wait for hardware responses, which are sleeping
operations.

Could this lead to scheduling while atomic warnings or potential deadlocks
during display mode settings, monitor sleep, or teardown on hardware using
DPCD backlights?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819155934.154125-1-lyude@redhat.com?part=1