* [PATCH v2 2/2] drm/vc4: Add support for X/Y reflection
2018-12-05 16:45 [PATCH v2 1/2] drm/vc4: Fix negative X/Y positioning on SAND planes Boris Brezillon
@ 2018-12-05 16:45 ` Boris Brezillon
2018-12-06 19:00 ` Eric Anholt
2018-12-06 18:59 ` [PATCH v2 1/2] drm/vc4: Fix negative X/Y positioning on SAND planes Eric Anholt
1 sibling, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2018-12-05 16:45 UTC (permalink / raw)
To: Eric Anholt; +Cc: Boris Brezillon, dri-devel
Add support for X/Y reflection when the plane is using linear or T-tiled
formats. X/Y reflection hasn't been tested on SAND formats, so we reject
them until proper testing/debugging has been done.
Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Eric, I dropped your Reviewed-by as this version also contains support
for rotation/reflection on SAND formats.
Changes in v2:
- Use drm_rotation_simplify() to support ROTATE_180
- Support rotation/reflection on SAND formats
---
drivers/gpu/drm/vc4/vc4_plane.c | 59 ++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 3132f5e1d16a..335aaf815290 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -492,8 +492,9 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
bool mix_plane_alpha;
bool covers_screen;
u32 scl0, scl1, pitch0;
- u32 tiling;
+ u32 tiling, src_y;
u32 hvs_format = format->hvs;
+ unsigned int rotation;
int ret, i;
if (vc4_state->dlist_initialized)
@@ -520,6 +521,16 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
h_subsample = drm_format_horz_chroma_subsampling(format->drm);
v_subsample = drm_format_vert_chroma_subsampling(format->drm);
+ rotation = drm_rotation_simplify(state->rotation,
+ DRM_MODE_ROTATE_0 |
+ DRM_MODE_REFLECT_X |
+ DRM_MODE_REFLECT_Y);
+
+ /* We must point to the last line when Y reflection is enabled. */
+ src_y = vc4_state->src_y;
+ if (rotation & DRM_MODE_REFLECT_Y)
+ src_y += vc4_state->src_h[0] - 1;
+
switch (base_format_mod) {
case DRM_FORMAT_MOD_LINEAR:
tiling = SCALER_CTL0_TILING_LINEAR;
@@ -529,9 +540,10 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
* out.
*/
for (i = 0; i < num_planes; i++) {
- vc4_state->offsets[i] += vc4_state->src_y /
+ vc4_state->offsets[i] += src_y /
(i ? v_subsample : 1) *
fb->pitches[i];
+
vc4_state->offsets[i] += vc4_state->src_x /
(i ? h_subsample : 1) *
fb->format->cpp[i];
@@ -557,22 +569,38 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
u32 tiles_w = fb->pitches[0] >> (tile_size_shift - tile_h_shift);
u32 tiles_l = vc4_state->src_x >> tile_w_shift;
u32 tiles_r = tiles_w - tiles_l;
- u32 tiles_t = vc4_state->src_y >> tile_h_shift;
+ u32 tiles_t = src_y >> tile_h_shift;
/* Intra-tile offsets, which modify the base address (the
* SCALER_PITCH0_TILE_Y_OFFSET tells HVS how to walk from that
* base address).
*/
- u32 tile_y = (vc4_state->src_y >> 4) & 1;
- u32 subtile_y = (vc4_state->src_y >> 2) & 3;
- u32 utile_y = vc4_state->src_y & 3;
+ u32 tile_y = (src_y >> 4) & 1;
+ u32 subtile_y = (src_y >> 2) & 3;
+ u32 utile_y = src_y & 3;
u32 x_off = vc4_state->src_x & tile_w_mask;
- u32 y_off = vc4_state->src_y & tile_h_mask;
+ u32 y_off = src_y & tile_h_mask;
+
+ /* When Y reflection is requested we must set the
+ * SCALER_PITCH0_TILE_LINE_DIR flag to tell HVS that all lines
+ * after the initial one should be fetched in descending order,
+ * which makes sense since we start from the last line and go
+ * backward.
+ * Don't know why we need y_off = max_y_off - y_off, but it's
+ * definitely required (I guess it's also related to the "going
+ * backward" situation).
+ */
+ if (rotation & DRM_MODE_REFLECT_Y) {
+ y_off = tile_h_mask - y_off;
+ pitch0 = SCALER_PITCH0_TILE_LINE_DIR;
+ } else {
+ pitch0 = 0;
+ }
tiling = SCALER_CTL0_TILING_256B_OR_T;
- pitch0 = (VC4_SET_FIELD(x_off, SCALER_PITCH0_SINK_PIX) |
- VC4_SET_FIELD(y_off, SCALER_PITCH0_TILE_Y_OFFSET) |
- VC4_SET_FIELD(tiles_l, SCALER_PITCH0_TILE_WIDTH_L) |
- VC4_SET_FIELD(tiles_r, SCALER_PITCH0_TILE_WIDTH_R));
+ pitch0 |= (VC4_SET_FIELD(x_off, SCALER_PITCH0_SINK_PIX) |
+ VC4_SET_FIELD(y_off, SCALER_PITCH0_TILE_Y_OFFSET) |
+ VC4_SET_FIELD(tiles_l, SCALER_PITCH0_TILE_WIDTH_L) |
+ VC4_SET_FIELD(tiles_r, SCALER_PITCH0_TILE_WIDTH_R));
vc4_state->offsets[0] += tiles_t * (tiles_w << tile_size_shift);
vc4_state->offsets[0] += subtile_y << 8;
vc4_state->offsets[0] += utile_y << 4;
@@ -643,7 +671,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
*/
for (i = 0; i < num_planes; i++) {
vc4_state->offsets[i] += param * tile_w * tile;
- vc4_state->offsets[i] += (vc4_state->src_y /
+ vc4_state->offsets[i] += (src_y /
(i ? v_subsample : 1)) *
tile_w;
}
@@ -669,6 +697,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
/* Control word */
vc4_dlist_write(vc4_state,
SCALER_CTL0_VALID |
+ (rotation & DRM_MODE_REFLECT_X ? SCALER_CTL0_HFLIP : 0) |
+ (rotation & DRM_MODE_REFLECT_Y ? SCALER_CTL0_VFLIP : 0) |
VC4_SET_FIELD(SCALER_CTL0_RGBA_EXPAND_ROUND, SCALER_CTL0_RGBA_EXPAND) |
(format->pixel_order << SCALER_CTL0_ORDER_SHIFT) |
(hvs_format << SCALER_CTL0_PIXEL_FORMAT_SHIFT) |
@@ -1150,6 +1180,11 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
drm_plane_helper_add(plane, &vc4_plane_helper_funcs);
drm_plane_create_alpha_property(plane);
+ drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
+ DRM_MODE_ROTATE_0 |
+ DRM_MODE_ROTATE_180 |
+ DRM_MODE_REFLECT_X |
+ DRM_MODE_REFLECT_Y);
return plane;
}
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] drm/vc4: Fix negative X/Y positioning on SAND planes
2018-12-05 16:45 [PATCH v2 1/2] drm/vc4: Fix negative X/Y positioning on SAND planes Boris Brezillon
2018-12-05 16:45 ` [PATCH v2 2/2] drm/vc4: Add support for X/Y reflection Boris Brezillon
@ 2018-12-06 18:59 ` Eric Anholt
2018-12-06 19:29 ` Boris Brezillon
1 sibling, 1 reply; 6+ messages in thread
From: Eric Anholt @ 2018-12-06 18:59 UTC (permalink / raw)
Cc: Boris Brezillon, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 3561 bytes --]
Boris Brezillon <boris.brezillon@bootlin.com> writes:
> Commit 3e407417b192 ("drm/vc4: Fix X/Y positioning of planes using
> T_TILES modifier") fixed the problem with T_TILES format, but left
> things in a non-working state for SAND formats. Address that now.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> Hi Eric,
>
> So, I finally spent time debugging my SANDXXX pattern generator and
> could validate that negative X/Y positioning does not work (which I
> was expecting :-)). The fix turns out to be simpler than I thought
> (much simpler than on T-tiles), and we now have negative X/Y
> positioning working on all kind of formats.
>
> Regards,
>
> Boris
>
> Changes in v2:
> - New patch
> ---
> drivers/gpu/drm/vc4/vc4_plane.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 75db62cbe468..3132f5e1d16a 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -595,6 +595,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
> case DRM_FORMAT_MOD_BROADCOM_SAND128:
> case DRM_FORMAT_MOD_BROADCOM_SAND256: {
> uint32_t param = fourcc_mod_broadcom_param(fb->modifier);
> + u32 tile_w, tile, x_off, pix_per_tile;
>
> /* Column-based NV12 or RGBA.
> */
> @@ -614,12 +615,15 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
> switch (base_format_mod) {
> case DRM_FORMAT_MOD_BROADCOM_SAND64:
> tiling = SCALER_CTL0_TILING_64B;
> + tile_w = 64;
> break;
> case DRM_FORMAT_MOD_BROADCOM_SAND128:
> tiling = SCALER_CTL0_TILING_128B;
> + tile_w = 128;
> break;
> case DRM_FORMAT_MOD_BROADCOM_SAND256:
> tiling = SCALER_CTL0_TILING_256B_OR_T;
> + tile_w = 256;
> break;
> default:
> break;
> @@ -630,6 +634,28 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
> return -EINVAL;
> }
>
> + pix_per_tile = tile_w / fb->format->cpp[0];
> + tile = vc4_state->src_x / pix_per_tile;
> + x_off = vc4_state->src_x % pix_per_tile;
> +
> + /* Adjust the base pointer to the first pixel to be scanned
> + * out.
> + */
> + for (i = 0; i < num_planes; i++) {
> + vc4_state->offsets[i] += param * tile_w * tile;
> + vc4_state->offsets[i] += (vc4_state->src_y /
> + (i ? v_subsample : 1)) *
> + tile_w;
> + }
> +
> + /*
> + * SCALER_PITCH0_SINK_PIX does not seem to work for SAND
> + * formats. Specify a negative START_X instead, even if it's
> + * less efficient.
> + */
> + if (x_off)
> + vc4_state->crtc_x = -x_off;
Wait. If we were supposed to start at a nonzero x position within the
FB, then we instead put the image off the left hand side of the screen?
That seems wrong.
Did you test if we can just vc4_state->offsets[i] += x_off * cpp?
> +
> pitch0 = VC4_SET_FIELD(param, SCALER_TILE_HEIGHT);
> break;
> }
> @@ -655,7 +681,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
> vc4_state->pos0_offset = vc4_state->dlist_count;
> vc4_dlist_write(vc4_state,
> VC4_SET_FIELD(state->alpha >> 8, SCALER_POS0_FIXED_ALPHA) |
> - VC4_SET_FIELD(vc4_state->crtc_x, SCALER_POS0_START_X) |
> + VC4_SET_FIELD(vc4_state->crtc_x & SCALER_POS0_START_X_MASK,
> + SCALER_POS0_START_X) |
> VC4_SET_FIELD(vc4_state->crtc_y, SCALER_POS0_START_Y));
>
> /* Position Word 1: Scaled Image Dimensions. */
> --
> 2.17.1
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread