Re: [PATCH] drm/vc4: Fix pitch setup for T-format scanout.

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Wed, 27 Sep 2017 12:32:09 -0700
Eric Anholt <eric@anholt.net> wrote:

> The documentation said to use src_w here, and I didn't consider that
> we actually needed to be using pitch somewhere in our setup.  Fixes
> scanout on my DSI panel when X11 does initial setup with 1920x1080
> HDMI and 800x480 DSI both at 0,0 of the same framebuffer.
> 
> Signed-off-by: Eric Anholt <eric@anholt.net>
> Fixes: 98830d91da08 ("drm/vc4: Add T-format scanout support.")
> ---
>  drivers/gpu/drm/vc4/vc4_plane.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 2968b3ebb895..4ad0b9fcae99 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -547,14 +547,24 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
>  		tiling = SCALER_CTL0_TILING_LINEAR;
>  		pitch0 = VC4_SET_FIELD(fb->pitches[0], SCALER_SRC_PITCH);
>  		break;
> -	case DRM_FORMAT_MOD_BROADCOM_VC4_T_TILED:
> +
> +	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."
> +		 */
> +		u32 tile_size_shift = 12;
> +		u32 tile_h_shift = 5;

Maybe you should explain where those _shift values come from in the
above comment (tile size is 4K hence tile_size_shift = 12 and a
tile is a 32x32 pixels element, hence tile_h_shift = 5).

Other than that,

Reviewed-by: Boris Brezillon <boris.brezillon@free-electrons.com>

> +		u32 tiles_w = fb->pitches[0] >> (tile_size_shift - tile_h_shift);
> +
>  		tiling = SCALER_CTL0_TILING_256B_OR_T;
>  
> -		pitch0 = (VC4_SET_FIELD(0, SCALER_PITCH0_TILE_Y_OFFSET),
> -			  VC4_SET_FIELD(0, SCALER_PITCH0_TILE_WIDTH_L),
> -			  VC4_SET_FIELD((vc4_state->src_w[0] + 31) >> 5,
> -					SCALER_PITCH0_TILE_WIDTH_R));
> +		pitch0 = (VC4_SET_FIELD(0, SCALER_PITCH0_TILE_Y_OFFSET) |
> +			  VC4_SET_FIELD(0, SCALER_PITCH0_TILE_WIDTH_L) |
> +			  VC4_SET_FIELD(tiles_w, SCALER_PITCH0_TILE_WIDTH_R));
>  		break;
> +	}
> +
>  	default:
>  		DRM_DEBUG_KMS("Unsupported FB tiling flag 0x%16llx",
>  			      (long long)fb->modifier);