Repository navigation
drm/vc4: Fix plane scaling edge cases that corrupt other planes - #7702
Open
popcornmix wants to merge 18 commits into
Open
popcornmix wants to merge 18 commits into
popcornmix wants to merge 18 commits into
Conversation
A 1 pixel source is scaled with TPZ, which cannot upscale and instead repeats the last pixel out to the destination size. H-TPZ runs before the vertical scaler, so the lines written to the LBM are crtc_w wide. vc4_lbm_channel_size() sized the LBM from min(src_w, crtc_w), so a 1x1 source upscaled to a large rectangle got a single LBM line and overran into the LBM of whichever plane was allocated after it, corrupting both. The vc4/vc5 path already uses crtc_w for H-TPZ. Fixes: raspberrypi#7701 Signed-off-by: Dom Cobley <popcornmix@gmail.com>
Collaborator
Author
|
Also tested on Pi 5 (C1) with a 6.18.57 kernel: before the fix, the neighbouring plane had 460,160 corrupt pixels; after, none. |
Collaborator
Author
|
We have some additional YUV bugs, which I'm just narrowing in on. |
A source of 1 pixel or less is scaled with TPZ, which can't upscale and instead repeats the last pixel fetched. That goes wrong in two cases: - YUV: a 2 pixel wide NV12 source has a 1 pixel chroma channel, so chroma uses TPZ while luma uses PPF, and a 1 pixel source puts luma on TPZ. Either way the plane is rendered the wrong colour, and the chroma LBM, sized from the source width, is overrun into the next plane. Seen on BCM2835, BCM2711 and BCM2712. - A fractional source position: the mode_set functions fetch an extra pixel at each partial end, so the pixel repeated across the plane is the one beyond the source rectangle. Use PPF for a TPZ upscale in those cases, as larger sources already do. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
The TPZ scale field is 21 bits of 16.16, so a horizontal downscale of more than 32x doesn't fit. VC4_SET_FIELD() warned and masked it, and the plane was scanned out the wrong colour. Unlike the vertical direction, there is no line skipping to bring the ratio down, so reject it in atomic check. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4_get_scaling_mode() returned VC4_SCALING_NONE whenever the integer part of the source size matched the destination, and ignored the source position. With a fractional source position or size, the mode_set functions fetch an extra pixel at each partial end, and without scaling those were all output, so the plane was drawn up to 2 pixels wider and taller than its CRTC rectangle. Only report no scaling for an exact match, and use PPF when the source starts part way into a pixel. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
The mode_set functions fetch an extra pixel at each partial end of the source, but the LBM was sized from the integer source width. A plane with a fractional source position or size that is vertically scaled overran its LBM into the next plane's, corrupting both. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4_lbm_channel_size() divided the width by the horizontal subsampling, rounding down. A 1 pixel wide YUV source got no LBM at all, and odd widths, such as 33 pixels upscaled, got a line too short, either of which corrupts the next plane's LBM. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
__vc6_lbm_size() only took the chroma channel into account when it was horizontally subsampled, so YUV444 got an LBM for luma only: none at all at 1:1, where only chroma is scaled. The width is also programmed doubled for YUV444, and the hardware uses an LBM for that doubled width. Size both channels for all YUV formats, and double the width for YUV444. Without this a YUV444 plane corrupts the plane after it in the LBM even when unscaled. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4_plane_mode_set() programs double the width for YUV444 so that all of the chroma is fetched, but when the plane wasn't horizontally scaled the luma was output at that doubled width too, drawing the plane twice as wide as its CRTC rectangle. Enable the luma scaler at 1:1 for YUV444, as vc6_plane_mode_set() already does. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
On BCM2712 a YUV source less than one chroma pixel wide that is scaled corrupts both its own output and that of the plane after it in the LBM, however large an LBM it is given. Reject it in atomic check. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
When skipping lines for a large vertical downscale, the luma pitch was multiplied by vdownsample but the chroma pitch of planar formats was divided by it, so chroma was read from the wrong lines. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
A vertical TPZ downscale of more than 3x skips source lines, through the pitch for linear buffers or a 3 bit line skip field for SAND, but the skip wasn't limited to what those hold: - A large linear pitch times the skip overflowed the 16 bit (BCM2835, BCM2711) or 17 bit (BCM2712) pitch field, so the wrong lines were read. On BCM2712 VC4_SET_FIELD() warned about it. - More than 8 overflowed the SAND line skip field. - T-tiled buffers have no line pitch to multiply. - Chroma lines are skipped too, so for YUV a skip that turned the chroma into an upscale left TPZ unable to scale it, and the plane was rendered the wrong colour and shifted. Factor the calculation out of the two mode_set functions, cap the skip to the field and the chroma ratio, and reject a vertical TPZ scale that then overflows the scale field. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc6_plane_mode_set() only handles linear, SAND128 and SAND256 buffers, and rejects anything else, but the planes advertised T-tiled and SAND64 too. A client using them got a framebuffer that every commit rejected. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4_plane_mode_set() programs double the source width for YUV444, which for a 4096 pixel wide source overflows the 13 bit POS2 width field on BCM2711 (12 bit on BCM2835). VC4_SET_FIELD() warned and the width was truncated. Reject such planes in atomic check. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
popcornmix
changed the title
popcornmix
marked this pull request as draft
October 21, 2026 20:07
The mode_set functions added a pixel to the fetch for each end of the source rectangle that wasn't pixel aligned. When both ends are partial and their fractions add up to less than a pixel, that is one more than the source covers, so the pixel beyond the right or bottom edge was fetched and filtered into the edge of the plane. With a 2x2 source at (16.5, 16.5) scaled to 400x400 the right and bottom edges picked up the colour of the pixels outside the source. Fetch the pixels the source rectangle actually covers, and size the LBM from the same count. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
On BCM2835 the chroma of a planar YUV plane is placed about 3 source pixels to the left when its horizontal PPF downscale is exactly 1.5, for example NV12 300 pixels wide scaled to 100, where luma uses TPZ and chroma sits on the 2/3 PPF limit. 1.495 and TPZ at 1.51 are both correct, as is the same scale on BCM2711 and BCM2712, and vertically. Use TPZ for chroma at that limit on BCM2835. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
With DRM_MODE_REFLECT_Y the scan starts at the last line of the source and works up, but that line was found from the integer source height, before the extra line fetched for a partial pixel at the top or bottom was added. A vertically reflected plane with a fractional source started one line too high, so its edge showed the line above the source rectangle and lost the last line. Work out the fetched width and height before they are used. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4_plane_async_set_fb() pointed the plane at the start of the new framebuffer, adjusted only for a Y reflection, so the plane's source crop was dropped: a primary plane scanning out from (64,48) of its framebuffer showed the new one from (0,0) after a legacy async page flip. Record the offset the mode_set functions computed for the source position, reflection and tiling, and apply it to the new framebuffer. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
A YUV source less than one chroma sample wide or tall is rendered the wrong colour on BCM2835, BCM2711 and BCM2712 (a 4:2:0 source 1 line tall picks up the next chroma line, and 1 pixel wide is wrong throughout). BCM2712 already rejected the 1 pixel wide case, which also corrupts the next plane's LBM. Reject both directions on all generations. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
Collaborator
Author
|
vc4 plane format/scaling test What it does: It scans out a test plane (A) through the writeback connector and checks every pixel of the result. A has four coloured quadrants inside a guard colour, and is scaled to its destination rectangle. A second scaled plane (B) is placed so that its line buffer follows A's. Each case reports:
Expected colours come from the same format at 1:1, so the YUV matrix doesn't matter. suite.py runs every supported format (RGB, YUV planar and semi-planar, 10/12/16-bit, SAND128/256, P030) through about 950 cases each. They cover 1×1 and tiny sources, the TPZ/PPF switch-over, subsampled formats where luma and chroma use different scalers, very large up- and downscales, and odd or fractional source offsets. It can also run X/Y reflection and seeded random cases. How to run (on the Pi console with no desktop or Kodi running, since it needs DRM master):
To run a single case: asynctest.c checks that a legacy async page flip keeps a cropped and reflected plane's source offset (sudo ./asynctest 0..3). |
popcornmix
marked this pull request as ready for review
October 22, 2026 13:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Started as a fix for #7701 (a 1x1 source scaled up corrupts other planes), then widened after a writeback-based test of every plane format across ~950 scaling cases each (tiny sources, around the TPZ/PPF switch, subsampled formats where luma and chroma scale differently, large up/downscales, odd and fractional source positions), with a second plane placed so that its LBM follows the plane under test.
Fixes, by symptom:
VC4_SET_FIELD()warnings, garbage output): horizontal TPZ downscales over 32x, and large vertical line skips, are now capped or rejected; YUV444 over 4095 pixels wide is rejected on BCM2711.REFLECT_Ya fractional source started one line too high. Both now render exactly.Results, cases with corruption / wrong interior / drawn outside the rectangle:
Final passes on the complete series, adding X/Y reflection (35,574 cases on Pi 5 and Pi 4, 7,260 on Pi 3) and seeded random size/offset/fraction/reflection combinations (98,000 on Pi 5, 49,000 on Pi 4, 3,000 on Pi 3): no other-plane corruption, nothing drawn outside the CRTC rectangle, no wrong interiors beyond a few levels of rounding on Pi 3, no kernel warnings. The last two commits (async page flip, small YUV rejection) came after those passes and were checked directly: an async page flip of a cropped, reflected primary plane now scans out the right crop (writeback capture, Pi 5 and Pi 3).
No kernel warnings with the fixes (the unmodified kernel warns on every overflowing case). The full runs were on a 7.3 kernel. This 6.18 branch was built and given quick plain and reflected runs plus the async test on Pi 5 (D0), clean.
Still open: YUV444 is scanned out as 3-plane 4:2:2 with the width doubled, so the luma fetch runs past the source and the last source pixel at the right edge blends with whatever follows it (up to ~36 levels on large upscales). There is no 4:4:4 3-plane mode to avoid that.