Boris Brezillon boris.brezillon@bootlin.com writes:
From: Eric Anholt eric@anholt.net
X/Y positioning of T-format buffers is quite tricky and the current implementation was failing to position a plane using this format correctly when the X, Y or both X and Y offsets were negative.
Wait, were things working for you with even postivie X/Y offsets on T? Because it wasn't for me, and I think the tile_h_mask change is important for making positive work.
Signed-off-by: Eric Anholt eric@anholt.net Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com
Hi Eric,
I kept the SoB and authorship since you're the original author, but I also significantly reworked the code, so I'd be more confident if you could have a close look at this code.
I think you should definitely grab authorship on this one. You did the work to make it actually work.
Also, I intentionally did not add a Fixes and Cc-stable to this commit because it depends on a rework we've done in vc4_plane_setup_clipping_and_scaling() which cannot be easily backported. What I could do though is add a patch that rejects all negative crtc_{x,y}.
Agreed. Given that we're trying to fix a bug that nobody else has reported to me yet, I think we can skip dealing with this for stable.
drivers/gpu/drm/vc4/vc4_plane.c | 51 +++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 2b8ba1c412be..ade47c3f65d1 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -539,22 +539,59 @@ static int vc4_plane_mode_set(struct drm_plane *plane, (i ? h_subsample : 1) * fb->format->cpp[i]; }
break;
case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED: {
/* For T-tiled, the FB pitch is "how many bytes from* one row to the next, such that pitch * tile_h ==* tile_size * tiles_per_row."*/
/* Whole-tile offsets, mostly for setting the pitch. */u32 tile_w_shift = fb->format->cpp[0] == 2 ? 6 : 5;u32 tile_w_mask = (1 << tile_w_shift) - 1;/* The height mask on 32-bit-per-pixel tiles is 63, i.e. 2* times the height (in pixels) of a 4k tile. I just assumed* this is also true for other RGB formats, but maybe it's not.*/u32 tile_h_mask = (2 << tile_h_shift) - 1;
Only 2 and 4-byte formats are supported for T format, and tiles are 32 pixels high for both of those.
Other than that,
Reviewed-by: Eric Anholt eric@anholt.net
I have a comment on patch 3 I'd like to sort out, but other than that I'm pleased with this whole series. Thanks for persevering on it!