Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] DMA buffer underflow caused by mixing clipped and unclipped plane coordinates Pre-existing issues: - [High] Hardware clipping broken because `drm_fb_dma_get_gem_addr()` ignores clipped source coordinates -- commit ecc3d1ed4b5c80402d5086efdcdc7ff6a120827c Author: Chen-Yu Tsai <[email protected]> drm/sun4i: Align VI buffer addresses for subsampled formats This commit adjusts the DMA address returned by drm_fb_dma_get_gem_addr for subsampled formats by calculating an intra-block offset and subtracting it. This ensures the address points to the start of a subsampled block for the display hardware. > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c > @@ -207,6 +207,20 @@ static void sun8i_vi_layer_update_buffer(struct > sun8i_layer *layer, > /* Get the start of the displayed memory */ > dma_addr = drm_fb_dma_get_gem_addr(fb, state, i); [Severity: High] This is a pre-existing issue, but does using drm_fb_dma_get_gem_addr break hardware clipping since it ignores clipped source coordinates? The drm_fb_dma_get_gem_addr function calculates the base address using the unclipped coordinates (state->src_x and state->src_y). If the hardware relies on the DMA base address to handle source X/Y offsets, providing the unclipped address would cause it to fetch pixels starting from the unclipped origin. This could shift the image and reveal regions that should be hidden when a plane is placed partially off-screen on the left or top edge. > > + /* > + * The mixer can handle odd offsets into sub-sampled YUV > + * planes, but needs the address of the first pixel in each > + * sub-sampled block. Adjust the luma buffer address backwards. > + */ > + if (i == 0) { > + u32 x_diff, y_diff; > + > + x_diff = (state->src.x1 >> 16) & (format->hsub - 1); > + y_diff = (state->src.y1 >> 16) & (format->vsub - 1); > + dma_addr -= y_diff * fb->pitches[i]; > + dma_addr -= x_diff * format->cpp[i]; [Severity: High] Can this subtraction lead to a DMA buffer underflow if the plane is clipped? If userspace positions a sub-sampled YUV plane partially off-screen to the left or top, the DRM core clips the source rectangle. This makes state->src.x1 or state->src.y1 positive, while the original state->src_x and state->src_y remain 0. Because drm_fb_dma_get_gem_addr calculates the base dma_addr using the unclipped coordinates, it will start at offset 0. The patch then calculates x_diff and y_diff using the clipped state->src.x1 and state->src.y1. If the plane is clipped by 1 pixel, y_diff might be 1. Subtracting y_diff * pitch from the unclipped base dma_addr (which is at offset 0) would shift the pointer into unallocated memory before the start of the buffer, which could trigger IOMMU faults or memory corruption. > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
