Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect comparison between the enum value `pixel_blend_mode` and the bitmask `blend_modes` causes a valid default mode to be silently overwritten. --
--- Patch [2]: [PATCH v2 2/3] drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state() --- commit 43504d2c3aaff4f127ccc271e9439f3f380cd55b Author: Lyude Paul <[email protected]> drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state() This commit introduces nv50_wndw_default_state() to enforce a valid pixel_blend_mode for planes supporting alpha channels during driver startup. > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > index 2635458d52acc..0f331f47bded8 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c [ ... ] > @@ -848,6 +848,28 @@ static const u64 nv50_cursor_format_modifiers[] = { > DRM_FORMAT_MOD_INVALID, > }; > > +/* > + * Setup defaults for the atomic wndw state > + */ > +void > +nv50_wndw_default_state(struct nv50_wndw *wndw) > +{ > + struct nv50_wndw_atom *armw = nv50_wndw_atom(wndw->plane.state); > + const unsigned int blend_modes = wndw->func->blend_modes; > + > + drm_modeset_lock_assert_held(&wndw->plane.mutex); > + > + /* Ensure the plane's atomic state didn't default to a pixel_blend_mode > we don't support */ > + if (blend_modes && (!(armw->state.pixel_blend_mode & blend_modes))) { [Severity: High] Does this incorrectly perform a bitwise AND between an enum value and a bitmask? Since DRM_MODE_BLEND_PREMULTI is defined as 0, when pixel_blend_mode defaults to PREMULTI, armw->state.pixel_blend_mode & blend_modes will evaluate to 0. This causes the negation !(0) to evaluate to true, unintentionally entering this block and overwriting the valid default mode with DRM_MODE_BLEND_COVERAGE. Should this comparison be using BIT(armw->state.pixel_blend_mode) & blend_modes to properly check if the enum value is supported by the bitmask? > + if (blend_modes & BIT(DRM_MODE_BLEND_COVERAGE)) > + armw->state.pixel_blend_mode = DRM_MODE_BLEND_COVERAGE; > + else if (blend_modes & BIT(DRM_MODE_BLEND_PREMULTI)) > + armw->state.pixel_blend_mode = DRM_MODE_BLEND_PREMULTI; > + else if (blend_modes & BIT(DRM_MODE_BLEND_PIXEL_NONE)) > + armw->state.pixel_blend_mode = > DRM_MODE_BLEND_PIXEL_NONE; > + } > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
