Re: [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void

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

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> All of the checks in dpu_plane_sspp_atomic_update() are impossible, so
> remove them and make the function void. This removes the need to error
> check in dpu_plane_atomic_update(). Additionally, remove impossible 
> checks
> in dpu_plane_atomic_update().
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 +++++------------------
>  1 file changed, 14 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index ce80c51b10aa..2cfa5583463e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1192,45 +1192,17 @@ void dpu_plane_set_error(struct drm_plane 
> *plane,
> bool error)
>  	pdpu->is_error = error;
>  }
> 
> -static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> -				struct drm_plane_state *old_state)
> +static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> +					 struct drm_plane_state
> *old_state)
>  {
> -	uint32_t nplanes, src_flags;
> -	struct dpu_plane *pdpu;
> -	struct drm_plane_state *state;
> -	struct dpu_plane_state *pstate;
> -	struct dpu_plane_state *old_pstate;
> -	const struct dpu_format *fmt;
> -	struct drm_crtc *crtc;
> -	struct drm_framebuffer *fb;
> -
> -	if (!plane) {
> -		DPU_ERROR("invalid plane\n");
> -		return -EINVAL;
> -	} else if (!plane->state) {
> -		DPU_ERROR("invalid plane state\n");
> -		return -EINVAL;
> -	} else if (!old_state) {
> -		DPU_ERROR("invalid old state\n");
> -		return -EINVAL;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -	state = plane->state;
> -
> -	pstate = to_dpu_plane_state(state);
> -
> -	old_pstate = to_dpu_plane_state(old_state);
> -
> -	crtc = state->crtc;
> -	fb = state->fb;
> -	if (!crtc || !fb) {
> -		DPU_ERROR_PLANE(pdpu, "invalid crtc %d or fb %d\n",
> -				crtc != 0, fb != 0);
> -		return -EINVAL;
> -	}
> -	fmt = to_dpu_format(msm_framebuffer_format(fb));
> -	nplanes = fmt->num_planes;
> +	uint32_t src_flags;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> +	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_framebuffer *fb = state->fb;
> +	const struct dpu_format *fmt =
> +		to_dpu_format(msm_framebuffer_format(fb));
> 
>  	memset(&(pdpu->pipe_cfg), 0, sizeof(struct dpu_hw_pipe_cfg));
> 
> @@ -1261,7 +1233,7 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	/* override for color fill */
>  	if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
>  		/* skip remaining processing on color fill */
> -		return 0;
> +		return;
>  	}
> 
>  	if (pdpu->pipe_hw->ops.setup_rects) {
> @@ -1332,7 +1304,6 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	}
> 
>  	_dpu_plane_set_qos_remap(plane);
> -	return 0;
>  }
> 
>  static void _dpu_plane_atomic_disable(struct drm_plane *plane,
> @@ -1371,31 +1342,17 @@ static void _dpu_plane_atomic_disable(struct
> drm_plane *plane,
>  static void dpu_plane_atomic_update(struct drm_plane *plane,
>  				struct drm_plane_state *old_state)
>  {
> -	struct dpu_plane *pdpu;
> -	struct drm_plane_state *state;
> -
> -	if (!plane) {
> -		DPU_ERROR("invalid plane\n");
> -		return;
> -	} else if (!plane->state) {
> -		DPU_ERROR("invalid plane state\n");
> -		return;
> -	}
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> 
> -	pdpu = to_dpu_plane(plane);
>  	pdpu->is_error = false;
> -	state = plane->state;
> 
>  	DPU_DEBUG_PLANE(pdpu, "\n");
> 
>  	if (!state->visible) {
>  		_dpu_plane_atomic_disable(plane, old_state);
>  	} else {
> -		int ret;
> -
> -		ret = dpu_plane_sspp_atomic_update(plane, old_state);
> -		/* atomic_check should have ensured that this doesn't fail
> */
> -		WARN_ON(ret < 0);
> +		dpu_plane_sspp_atomic_update(plane, old_state);
>  	}
>  }

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