Re: [PATCH v3] drm/msm: dpu: Allow planes to extend past active display

[Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>]

On 2018-08-29 10:49, Sean Paul wrote:
> From: Sean Paul <seanpaul@xxxxxxxxxxxx>
>
> The atomic_check is a bit too aggressive with respect to planes which
> leave the active area. This caused a bunch of log spew when the cursor
> got to the edge of the screen and stopped it from going all the way.
>
> This patch removes the conservative bounds checks from atomic and clips
> the dst rect such that we properly display planes which go off the
> screen.
>
> Changes in v2:
> - Apply the clip to src as well (taking into account scaling)
> Changes in v3:
> - Use drm_atomic_helper_check_plane_state() to clip src/dst
>
> Cc: Sravanthi Kollukuduru <skolluku@xxxxxxxxxxxxxx>
> Cc: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c   |  3 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c  | 34 +++++++++++++---------
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c |  1 -
>  drivers/gpu/drm/msm/msm_drv.h              |  2 ++
>  4 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 07c2d15b45f2..f0a5e776ba32 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1551,8 +1551,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc
> *crtc,
>  		cnt++;
>
>  		dst = drm_plane_state_dest(pstate);
> -		if (!drm_rect_intersect(&clip, &dst) ||
> -		    !drm_rect_equals(&clip, &dst)) {
> +		if (!drm_rect_intersect(&clip, &dst)) {
>  			DPU_ERROR("invalid vertical/horizontal destination\n");
>  			DPU_ERROR("display: " DRM_RECT_FMT " plane: "
>  				  DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index efdf9b200dd9..46de0de466ff 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1254,7 +1254,7 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	const struct dpu_format *fmt;
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> -	struct drm_rect src, dst;
> +	int ret, min_scale;
>
>  	if (!plane) {
>  		DPU_ERROR("invalid plane\n");
> @@ -1293,21 +1293,29 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>  	_dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
>
> -	src.x1 = state->src_x >> 16;
> -	src.y1 = state->src_y >> 16;
> -	src.x2 = src.x1 + (state->src_w >> 16);
> -	src.y2 = src.y1 + (state->src_h >> 16);
> +	min_scale = FRAC_16_16(1, pdpu->pipe_sblk->maxdwnscale);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc->state, 
> min_scale,
> +					  pdpu->pipe_sblk->maxupscale << 16,
> +					  true, false);
> +	if (ret) {
> +		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
> +		return ret;
> +	}
>
> -	dst = drm_plane_state_dest(state);
> +	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " 
> DRM_RECT_FMT
> +			", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src),
> +			crtc->base.id, DRM_RECT_ARG(&state->dst),
> +			(char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
>
> -	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FMT "->crtc%u " DRM_RECT_FMT
> -			", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_ARG(&src),
> -			crtc->base.id, DRM_RECT_ARG(&dst),
> -			(char *)&fmt->base.pixel_format,
> -			DPU_FORMAT_IS_UBWC(fmt));
> +	pdpu->pipe_cfg.src_rect = state->src;
> +
> +	/* state->src is 16.16, src_rect is not */
> +	pdpu->pipe_cfg.src_rect.x1 >>= 16;
> +	pdpu->pipe_cfg.src_rect.x2 >>= 16;
> +	pdpu->pipe_cfg.src_rect.y1 >>= 16;
> +	pdpu->pipe_cfg.src_rect.y2 >>= 16;
>
> -	pdpu->pipe_cfg.src_rect = src;
> -	pdpu->pipe_cfg.dst_rect = dst;
> +	pdpu->pipe_cfg.dst_rect = state->dst;
>
>  	_dpu_plane_setup_scaler(pdpu, pstate, fmt, false);
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> index 7d306c5acd09..273cbbe27c2e 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
> @@ -259,7 +259,6 @@ static void mdp5_plane_cleanup_fb(struct drm_plane
> *plane,
>  	msm_framebuffer_cleanup(fb, kms->aspace);
>  }
>
> -#define FRAC_16_16(mult, div)    (((mult) << 16) / (div))
>  static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state
> *crtc_state,
>  					      struct drm_plane_state *state)
>  {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h 
> b/drivers/gpu/drm/msm/msm_drv.h
> index 8e510d5c758a..9d11f321f5a9 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -62,6 +62,8 @@ struct msm_gem_vma;
>  #define MAX_BRIDGES    8
>  #define MAX_CONNECTORS 8
>
> +#define FRAC_16_16(mult, div)    (((mult) << 16) / (div))
> +
>  struct msm_file_private {
>  	rwlock_t queuelock;
>  	struct list_head submitqueues;

--
Jeykumar S