Re: [PATCH 12/25] drm/msm/dpu: inline _dpu_plane_set_scanout

[Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>]

On 2/9/2022 9:25 AM, Dmitry Baryshkov wrote:
> In preparation to reworking dpu_plane_sspp_atomic_update() inline the
> _dpu_plane_set_scanout() function.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 41 ++++++++++-------------
>   1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index d029ce806039..3ce7dcc285e2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -490,28 +490,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane)
>   	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>   }
>   
> -static void _dpu_plane_set_scanout(struct drm_plane *plane,
> -		struct dpu_plane_state *pstate,
> -		struct dpu_hw_pipe_cfg *pipe_cfg,
> -		struct drm_framebuffer *fb)
> -{
> -	struct dpu_plane *pdpu = to_dpu_plane(plane);
> -	struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> -	struct msm_gem_address_space *aspace = kms->base.aspace;
> -	int ret;
> -
> -	ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg->layout);
> -	if (ret == -EAGAIN)
> -		DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
> -	else if (ret)
> -		DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
> -	else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
> -		trace_dpu_plane_set_scanout(&pstate->pipe,
> -					    &pipe_cfg->layout);
> -		pstate->pipe.sspp->ops.setup_sourceaddress(&pstate->pipe, pipe_cfg);
> -	}
> -}
> -
>   static void _dpu_plane_setup_scaler3(struct dpu_hw_pipe *pipe_hw,
>   		uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
>   		struct dpu_hw_scaler3_cfg *scale_cfg,
> @@ -1074,10 +1052,27 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
>   	const struct dpu_format *fmt =
>   		to_dpu_format(msm_framebuffer_format(fb));
>   	struct dpu_hw_pipe_cfg pipe_cfg;
> +	struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> +	struct msm_gem_address_space *aspace = kms->base.aspace;
> +	bool update_src_addr = true;
> +	int ret;
>   
>   	memset(&pipe_cfg, 0, sizeof(struct dpu_hw_pipe_cfg));
>   
> -	_dpu_plane_set_scanout(plane, pstate, &pipe_cfg, fb);
> +	ret = dpu_format_populate_layout(aspace, fb, &pipe_cfg.layout);
> +	if (ret == -EAGAIN) {
> +		DPU_DEBUG_PLANE(pdpu, "not updating same src addrs\n");
> +		update_src_addr = false;
> +	} else if (ret) {
> +		DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
> +		update_src_addr = false;
> +	}
> +
Do we need update_src_addr?

It seems we can just do

if (!ret &&  pipe->sspp->ops.setup_sourceaddress) {
	.....
	.....
}

> +	if (update_src_addr &&
> +	    pipe->sspp->ops.setup_sourceaddress) {
> +		trace_dpu_plane_set_scanout(pipe, &pipe_cfg.layout);
> +		pipe->sspp->ops.setup_sourceaddress(pipe, &pipe_cfg);
> +	}
>   
>   	pstate->pending = true;
>