Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The display plane atomic update ignores the plane state's source coordinates and dimensions, causing corrupted output for cropped or panned framebuffers. - [High] The CRTC atomic flush immediately sends the vblank event instead of arming it, breaking page-flip synchronization. -- commit c951a1d1c3d9caa9dc42e8ab8693ac3a8121b925 Author: Thomas Zimmermann <[email protected]> Subject: drm/hibmc: Fix list of formats on the primary plane This commit removes unsupported formats from the primary plane, leaving only XRGB8888 and RGB565. It addresses issues where planes with alpha channels or BGR order were not properly supported, and fixes distorted output for RGB888. > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > index 7c0b88c774b5e..2e6e189bec1a0 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > @@ -135,10 +135,8 @@ static void hibmc_plane_atomic_update(struct drm_plane > *plane, > } [Severity: High] This isn't a bug introduced by this patch, but does hibmc_plane_atomic_update() ignore the plane state's source coordinates? It programs the hardware's visible scanout width using the full fb->width instead of the active source width (new_state->src_w >> 16): writel(gpu_addr, priv->mmio + HIBMC_CRT_FB_ADDRESS); reg = drm_format_info_min_pitch(fb->format, 0, fb->width); line_l = fb->pitches[0]; writel(HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_WIDTH, reg) | HIBMC_FIELD(HIBMC_CRT_FB_WIDTH_OFFS, line_l), priv->mmio + HIBMC_CRT_FB_WIDTH); Since the shadow plane damage copy maintains the original framebuffer's layout in VRAM, scanning out from offset 0 with fb->width means the hardware will always display the top-left of the framebuffer. If userspace provides a framebuffer larger than the CRTC (e.g., for panning) or requests cropping, will the hardware fetch the wrong number of pixels per scanline and display the wrong region? > > static const u32 channel_formats1[] = { > - DRM_FORMAT_RGB565, DRM_FORMAT_BGR565, DRM_FORMAT_RGB888, > - DRM_FORMAT_BGR888, DRM_FORMAT_XRGB8888, DRM_FORMAT_XBGR8888, > - DRM_FORMAT_RGBA8888, DRM_FORMAT_BGRA8888, DRM_FORMAT_ARGB8888, > - DRM_FORMAT_ABGR8888 > + DRM_FORMAT_XRGB8888, > + DRM_FORMAT_RGB565, > }; [Severity: High] This is a pre-existing issue, but does hibmc_crtc_atomic_flush() immediately send the vblank event instead of arming it? if (crtc->state->event) drm_crtc_send_vblank_event(crtc, crtc->state->event); crtc->state->event = NULL; Because the driver has working vblank interrupts (as shown by hibmc_crtc_enable_vblank()), shouldn't it defer the event until the actual vertical blanking interval with drm_crtc_arm_vblank_event()? Immediately completing the event violates the atomic KMS API contract, causing userspace compositors to render the next frame prematurely, which results in screen tearing and breaks presentation timing guarantees. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
