drm/mipi-dbi: display a cropped region of an oversized framebuffer - #7589
drm/mipi-dbi: display a cropped region of an oversized framebuffer#7589HorseyofCoursey wants to merge 4 commits into
Conversation
6by9
left a comment
6by9
left a comment
There was a problem hiding this comment.
Tested on my hx8357 (PiTFT 3.5" resistive) and works as expected.
Altering to kmstest -v -p 0,0-320x420 480x640-RG16 -v <x>,<y>-320x480 is still rejected as that would be scaling the plane rather than cropping, so that's all good.
For me with this display (and quite probably your ili9341) it always goes through the copy path as swap_bytes is set, needing to swap the two bytes in RGB565.
That's a limitation of the SPI controller in Pi0-4. It looks like Pi5 should support 16 bit transfers (and possibly 32bit) and hence not need the swap flag.
Once we have this series tidied up, we can look at sending it upstream.
|
@6by9 Thanks for testing on the hx8357d. Good point on swap bytesm, makes sense the zero-copy path never fires on Pi 0–4, so the stride-match optimization would only ever help Pi 5. |
|
Ideally all the tiny drm drivers that use mipi-dbi-spi would benefit from the increase in max_width/max_height, but no rush. That can be done when sending upstream. |
|
Makes sense, and upstream it collapses to a single change in drm_mipi_dbi.c since mainline still has the limits in the core, so all the mipi-dbi drivers get it for free there. Only 7.2.y needs the per-driver version. Happy to do the full 7.2.y sweep separately if you want it before the upstream submission, otherwise I'll fold it into that. Once this lands in 7.2.y, do you want to send it to dri-devel yourself, or would you rather I do it with you CC'd? Happy either way; if I send it I'll rebase onto drm-misc-next and collapse 2/3 into the single core change first. |
5c0bf85 to
71ba7f2
Compare
71ba7f2 to
c54b7f5
Compare
mipi_dbi_fb_dirty() takes the damage rectangle from drm_atomic_helper_damage_merged(), which is expressed in framebuffer coordinates and already clipped to the plane's source rectangle. It then passed that rectangle straight to mipi_dbi_set_window_address(), which is correct only while the source rectangle starts at (0,0) - i.e. while the framebuffer is exactly panel-sized. If a driver allows a framebuffer larger than the panel and the plane selects a sub-region with a non-zero src_x/src_y, the controller was still addressed in framebuffer coordinates, so the wrong part of the panel was written and an out-of-range window could be programmed. Pass the integer plane source origin down to mipi_dbi_fb_dirty() and subtract it when programming the column/page address. The copy into the transfer buffer still uses the framebuffer-coordinate rectangle, so it keeps reading the correct pixels from an oversized source. With a panel-sized framebuffer src_x/src_y are zero and behaviour is unchanged. Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com>
The driver set mode_config.max_width/max_height equal to the panel
dimensions, so KMS rejected any framebuffer that was not exactly
panel-sized:
ili9341 spi0.0: bad framebuffer width 480, should be >= 240 && <= 240
Raise the maximums so userspace can allocate a larger framebuffer and
choose the displayed region through the plane's source rectangle - a
crop / pan with no scaling. The minimums, the fixed display mode and
the connector are unchanged, and drm_mipi_dbi now translates the source
offset when addressing the controller.
The transfer buffer is sized from the display mode, and the plane check
(drm_mipi_dbi_plane_helper_atomic_check) forbids scaling and
repositioning, so the flushed rectangle stays bounded by the panel size
regardless of the framebuffer dimensions.
Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com>
Same change as the preceding ili9341 patch: mode_config.max_width/height were pinned to the panel dimensions, rejecting any framebuffer that was not exactly panel-sized. Raise them so a sub-region of a larger framebuffer can be displayed via the plane source rectangle, now that drm_mipi_dbi translates the source offset. The fixed mode, the minimums and the connector are unchanged; the plane check forbids scaling and repositioning and tx_buf is sized from the mode, so the flushed rectangle stays bounded by the panel. Compile-tested only; the functional testing was done on ili9341. Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com>
ca5c948 to
edcf2f2
Compare
|
@6by9 Rebased onto current rpi-7.2.y (the branch was force-pushed since this was opened, which is what blew up the diff). Back to the 3 commits, no code changes from the approved version, range-diff is clean and the three objects still build. Should I keep this PR to the ili9341 + hx8357d max_width bumps as-is, get it merged, then I'll do the wider mipi-dbi-spi sweep + collapse 2/3 into the single drm_mipi_dbi.c change as part of the dri-devel submission (rebased on drm-misc-next, you CC'd). |
Extend the ili9341/hx8357d change to every other drm/tiny SPI driver that flushes through the shared DRM_MIPI_DBI_PLANE_HELPER_FUNCS, so a client can scan out a cropped sub-region of a larger framebuffer on any of them: ili9486, panel-mipi-dbi, mi0283qt, ili9163 panel-mipi-dbi is the generic driver used for ST7789 and other MIPI DBI controllers without a dedicated driver, so this covers most SPI TFT breakouts in practice. All four use drm_mipi_dbi_plane_helper_atomic_update() unchanged, which now translates the plane source offset (see "drm/mipi-dbi: honour the plane source offset when flushing"). The fixed mode, minimums, connector and mode-sized transfer buffer are untouched, and the plane check still forbids scaling and repositioning, so the flushed rectangle stays bounded by the panel. ili9225 is deliberately excluded: it has its own atomic_update / ili9225_fb_dirty() that addresses the panel from the damage rectangle without the source offset, so raising its limits would let a mispositioned buffer through. Only ili9341 has been tested on hardware; the other four are compile- tested only. Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YCFunyWDqtsV58BGexkwUE
|
Update: also raises the limit on the remaining shared-flush mipi-dbi drivers (ili9486, panel-mipi-dbi, mi0283qt, ili9163) per @6by9 suggestion. ili9225 excluded (custom flush path, no source-offset support). Only ili9341 is hardware-tested; the rest are compile-tested. |
2 participants
Currently a drm/tiny MIPI-DBI panel can only scan out a framebuffer that
is exactly panel-sized, always from the origin. This series lets a client
allocate a larger framebuffer and choose the displayed region via the
plane source rectangle — a crop / pan with no scaling.
Pass the integer plane src origin into mipi_dbi_fb_dirty() and
subtract it in mipi_dbi_set_window_address(). The buffer copy still
uses framebuffer coordinates so it reads the right pixels from an
oversized source. src=(0,0) behaviour is unchanged.
2/3. drm/tiny/{ili9341,hx8357d}: raise mode_config.max_width/height,
which were pinned to the panel size and rejected any non-panel-sized
framebuffer. Fixed mode / minimums / connector unchanged; the plane
check forbids scaling and repositioning and tx_buf is mode-sized, so
the flushed rectangle stays bounded by the panel.
Change 1 is in the shared core; changes 2/3 are the per-driver limit
bump (the max_* values moved into the tiny drivers in 7.2). Entirely
within the DBI path — no shared shadow-plane / damage-helper changes.
Tested (Pi Zero 2 W, generic 2.4" 240x320 ILI9341, landscape):
width"), displayed from origin.
kmstest -f 480x640-RG16 -v <x>,<y>-320x240:the displayed region tracks the source rectangle.
performance regression.
hx8357d: compile-tested only.
Follow-up, not in this series: mipi_dbi_fb_dirty() can skip the tx_buf
repack when the framebuffer stride matches the panel width (rows are
contiguous). Independent of this feature; will send separately.
Thanks to @6by9 for the guidance throughout! (: