Re: [PATCH 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-09-11 13:15, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> src/dst rects are checked in both atomic_check and atomic_update, with
> the more comprehensive check occurring in atomic_update, which is
> backwards. So consolodate the checks in atomic_check.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 56 +++++++----------------
>  1 file changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 0f428f66b951..853902ac4fb9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1079,13 +1079,26 @@ static bool dpu_plane_validate_src(struct 
> drm_rect
> *src,
>  static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
>  		struct drm_plane_state *state)
>  {
> -	int ret = 0;
> +	int ret = 0, min_scale;
>  	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	const struct drm_crtc_state *crtc_state = NULL;
>  	const struct dpu_format *fmt;
>  	struct drm_rect src, dst, fb_rect = { 0 };
> -	uint32_t max_upscale = 1, max_downscale = 1;
>  	uint32_t min_src_size, max_linewidth;
> -	int hscale = 1, vscale = 1;
> +
> +	if (state->crtc)
> +		crtc_state = state->crtc->state;
Isn't the crtc_state retrieved here the old crtc state? Since the plane 
check is called before swap, the CRTC state
for this commit should be retrieved from global atomic state 
state->state using drm_atomic_get_existing_crtc_state.

Jeykumar S.
> +
> +	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, true);
> +	if (ret) {
> +		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n",
> ret);
> +		return ret;
> +	}
> +	if (!state->visible)
> +		return 0;
> 
>  	src.x1 = state->src_x >> 16;
>  	src.y1 = state->src_y >> 16;
> @@ -1099,25 +1112,6 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
> 
>  	max_linewidth = pdpu->pipe_sblk->common->maxlinewidth;
> 
> -	if (pdpu->features & DPU_SSPP_SCALER) {
> -		max_downscale = pdpu->pipe_sblk->maxdwnscale;
> -		max_upscale = pdpu->pipe_sblk->maxupscale;
> -	}
> -	if (drm_rect_width(&src) < drm_rect_width(&dst))
> -		hscale = drm_rect_calc_hscale(&src, &dst, 1, max_upscale);
> -	else
> -		hscale = drm_rect_calc_hscale(&dst, &src, 1,
> max_downscale);
> -	if (drm_rect_height(&src) < drm_rect_height(&dst))
> -		vscale = drm_rect_calc_vscale(&src, &dst, 1, max_upscale);
> -	else
> -		vscale = drm_rect_calc_vscale(&dst, &src, 1,
> max_downscale);
> -
> -	DPU_DEBUG_PLANE(pdpu, "check %d -> %d\n",
> -		dpu_plane_enabled(plane->state),
> dpu_plane_enabled(state));
> -
> -	if (!dpu_plane_enabled(state))
> -		goto exit;
> -
>  	fmt = to_dpu_format(msm_framebuffer_format(state->fb));
> 
>  	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
> @@ -1156,16 +1150,8 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
>  		DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
> line:%u\n",
>  				DRM_RECT_ARG(&src), max_linewidth);
>  		ret = -E2BIG;
> -
> -	/* check scaler capability */
> -	} else if (hscale < 0 || vscale < 0) {
> -		DPU_ERROR_PLANE(pdpu, "invalid scaling requested src="
> -				DRM_RECT_FMT " dst=" DRM_RECT_FMT "\n",
> -				DRM_RECT_ARG(&src), DRM_RECT_ARG(&dst));
> -		ret = -E2BIG;
>  	}
> 
> -exit:
>  	return ret;
>  }
> 
> @@ -1237,7 +1223,6 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	const struct dpu_format *fmt;
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> -	int ret, min_scale;
> 
>  	if (!plane) {
>  		DPU_ERROR("invalid plane\n");
> @@ -1276,15 +1261,6 @@ 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);
> 
> -	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;
> -	}
> -
>  	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),

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno