Re: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a conditional check that contradicts its commit message and results in an incomplete fix for a DRM core WARN_ON. While claiming to 'unconditionally create blend_mode prop for wndws', the code limits this to planes where `func->blend_modes` is non-zero. Older primary planes (e.g., `base507c`) expose alpha formats but lack `.blend_modes` initialization, meaning they will still trigger the warning.
--

--- Patch [3]: [PATCH v2 3/3] drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws ---
commit d890a1a203754ff1359f8cdacfa01e033bb7b8f3
Author: Lyude Paul <lyude@redhat.com>
drm/nouveau/kms/nv50-: Unconditionally create blend_mode prop for wndws

This patch attempts to resolve a DRM core validation warning by ensuring the
blend_mode property is created for all window types that expose color formats
with an alpha channel, even if they lack a blend_set function.

> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 0f331f47bded8..84b9a16bdb8ab 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c

[ ... ]

> @@ -944,6 +937,17 @@ nv50_wndw_new_(const struct nv50_wndw_func *func, struct drm_device *dev,
>  			return ret;
>  	}
>  
> +	/*
> +	 * DRM requires that we have a blend mode property for any type of plane that exposes color
> +	 * formats with an alpha channel. So do this, even if we don't actually have control for the
> +	 * blend property hooked up with blend_set.
> +	 */
> +	if (func->blend_modes) {

[Severity: Medium]
Does this conditional check contradict the commit message? The commit states
the blend_mode property is created unconditionally for all types of wndws.

However, for older primary planes like base507c, the nv50_wndw_func definition
does not initialize .blend_modes:

static const struct nv50_wndw_func
base507c = {
	.acquire = base507c_acquire,
	.release = base507c_release,
	.sema_set = base507c_sema_set,
	...

Since func->blend_modes evaluates to 0 here, the property creation is skipped.

Because base507c_format exposes formats with an alpha channel (such as
DRM_FORMAT_ARGB8888), won't skipping the property creation here leave the issue
unfixed and cause the DRM core validation WARN_ON to still trigger?

> +		ret = drm_plane_create_blend_mode_property(&wndw->plane, func->blend_modes);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }

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