Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Initial pixel_blend_mode property is set to an unsupported value (PREMULTI), causing userspace compositor crashes. - [High] Fast-path atomic commits silently drop alpha and pixel_blend_mode property updates. --
--- Patch [1]: [PATCH] drm/imx: dcss: Silence blend mode not set boot warning --- commit b8d70d6b989c5f19b4322dbd26927d62d307bf28 Author: Laurentiu Palcu <[email protected]> drm/imx: dcss: Silence blend mode not set boot warning This addresses a boot warning about missing blend mode when a pixel format with alpha is exposed. It adds the alpha and blending properties to the primary plane. > diff --git a/drivers/gpu/drm/imx/dcss/dcss-crtc.c > b/drivers/gpu/drm/imx/dcss/dcss-crtc.c > index ab41759a9f52d..8358219c94c0d 100644 > --- a/drivers/gpu/drm/imx/dcss/dcss-crtc.c > +++ b/drivers/gpu/drm/imx/dcss/dcss-crtc.c [ ... ] > @@ -202,6 +203,11 @@ int dcss_crtc_init(struct dcss_crtc *crtc, struct > drm_device *drm) > return ret; > } > > + drm_plane_create_alpha_property(&crtc->plane[0]->base); > + drm_plane_create_blend_mode_property(&crtc->plane[0]->base, > + BIT(DRM_MODE_BLEND_PIXEL_NONE) | > + BIT(DRM_MODE_BLEND_COVERAGE)); [Severity: High] Does exposing only PIXEL_NONE and COVERAGE cause issues with the default plane reset? Looking at the driver, it uses the default drm_atomic_helper_plane_reset hook: drivers/gpu/drm/imx/dcss/dcss-plane.c: struct drm_plane_funcs dcss_plane_funcs = { ... .reset = drm_atomic_helper_plane_reset, ... }; This core helper unconditionally initializes state->pixel_blend_mode to DRM_MODE_BLEND_PREMULTI. If the property is created here with only PIXEL_NONE and COVERAGE supported, will userspace compositors (like Weston) receive an invalid enum value during startup and crash when querying properties? > + > crtc->irq = platform_get_irq_byname(pdev, "vblank"); > if (crtc->irq < 0) > return crtc->irq; [ ... ] > diff --git a/drivers/gpu/drm/imx/dcss/dcss-plane.c > b/drivers/gpu/drm/imx/dcss/dcss-plane.c > index 1746dc5d4b642..06413d0612349 100644 > --- a/drivers/gpu/drm/imx/dcss/dcss-plane.c > +++ b/drivers/gpu/drm/imx/dcss/dcss-plane.c [ ... ] > @@ -336,7 +344,8 @@ static void dcss_plane_atomic_update(struct drm_plane > *plane, > dcss_dtg_plane_pos_set(dcss->dtg, dcss_plane->ch_num, > dst.x1, dst.y1, dst_w, dst_h); > dcss_dtg_plane_alpha_set(dcss->dtg, dcss_plane->ch_num, > - fb->format, new_state->alpha >> 8); > + fb->format, new_state->alpha >> 8, > + new_state->pixel_blend_mode); [Severity: High] Does the fast-path logic earlier in this function silently drop alpha and pixel_blend_mode updates? Earlier in dcss_plane_atomic_update(), there is an early return based on dcss_plane_needs_setup(): if (old_state->fb && !drm_atomic_crtc_needs_modeset(crtc_state) && !dcss_plane_needs_setup(new_state, old_state)) { dcss_plane_atomic_set_base(dcss_plane); return; } Since dcss_plane_needs_setup() only checks geometry, formatting, rotation, and scaling: return state->crtc_x != old_state->crtc_x || ... state->scaling_filter != old_state->scaling_filter; Will an atomic commit that only modifies the alpha or pixel_blend_mode properties cause dcss_plane_needs_setup() to return false? This would result in returning early and skipping the dcss_dtg_plane_alpha_set() hardware update entirely. > > if (!dcss_plane->ch_num && (new_state->alpha >> 8) == 0) > enable = false; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
