On Mon, 2026-07-20 at 21:15 +0000, [email protected] wrote: > 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 <[email protected]> > 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?
this is kind of fascinating, it's referencing an issue that was on the last version of this patch series but is most certainly fixed on this patch series. Weird. > > > + ret = drm_plane_create_blend_mode_property(&wndw- > > >plane, func->blend_modes); > > + if (ret) > > + return ret; > > + } > > + > > return 0; > > }
