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? > + 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/[email protected]?part=3
