Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Partial updates in the sharp-memory driver corrupt the display because framebuffer lines are unconditionally paired with panel line addresses starting at 1. This problem wasn't introduced by this patch, but exists in the current codebase. -- commit d169c425f8ab54f8c291ce0979baadd0ddecc949 Author: Jonathan Frazin <[email protected]> drm/mipi-dbi: honour the plane source offset when flushing This commit updates mipi_dbi_fb_dirty() to account for the plane's source offset. It subtracts src_x and src_y from the damage rectangle when programming the column and page addresses, ensuring the correct panel window is addressed for framebuffers larger than the panel. > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c > index 25cf04d029f77..38db45a9dbc35 100644 > --- a/drivers/gpu/drm/drm_mipi_dbi.c > +++ b/drivers/gpu/drm/drm_mipi_dbi.c [ ... ] > @@ -298,8 +299,13 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, > struct drm_framebuffer *fb, > tr = src->vaddr; /* TODO: Use mapping abstraction properly */ > } > > - mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1, > - rect->y2 - 1); > + /* > + * @rect is in framebuffer coordinates and has been clipped to the plane > + * src rectangle by the damage iterator. The panel is addressed relative > + * to the src origin, so subtract it here. > + */ > + mipi_dbi_set_window_address(dbidev, rect->x1 - src_x, rect->x2 - 1 - > src_x, > + rect->y1 - src_y, rect->y2 - 1 - src_y); [Severity: High] This isn't a bug introduced by this patch, but looking at how partial updates are handled here, I noticed a pre-existing issue in the sharp-memory driver where partial updates corrupt the display because framebuffer lines are unconditionally paired with panel line addresses starting at 1. Looking at drivers/gpu/drm/tiny/sharp-memory.c in sharp_memory_set_tx_buffer_addresses(): for (u32 line = 0; line < clip.y2; ++line) buffer[line * pitch] = line + 1; The line addresses unconditionally start at 1 regardless of clip.y1, preparing the transmit buffer with panel addresses starting at the top. In drivers/gpu/drm/drm_format_helper.c:drm_fb_xrgb8888_to_mono(): src32 = memcpy(src32, vaddr, len_src32); drm_fb_xrgb8888_to_gray8_line(gray8, src32, linepixels); drm_fb_gray8_to_mono_line(mono, gray8, linepixels); Data is extracted from the correct y-offset in the source framebuffer but written sequentially to the start of the destination buffer. Then in drivers/gpu/drm/tiny/sharp-memory.c:sharp_memory_update_display(): sharp_memory_set_tx_buffer_addresses(&tx_buffer[1], clip, pitch); sharp_memory_set_tx_buffer_data(&tx_buffer[2], fb, vmap, clip, pitch, fmtcnv_state); ret = sharp_memory_spi_write(smd->spi, tx_buffer, tx_buffer_size); The offset data is placed at the start of the transmit buffer and the entire buffer size is written to the SPI bus, pushing misaligned display data. When a userspace compositor submits an atomic commit with damage clips where clip.y1 > 0 (a partial update not starting at the top of the screen), the damaged region from the framebuffer is extracted but written to the beginning of the SPI transmit buffer. This causes the updated region to be drawn at the top of the display rather than its correct coordinates, and unmodified regions below it are updated with stale or out-of-sync memory. Since damage clips can be triggered by any unprivileged application indirectly through standard compositor operations, can this cause severe display corruption in normal userspace usage? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
