Boris Brezillon <boris.brezillon@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
