Re: [PATCH v4 12/13] drm/msm/dpu: allow sharing of blending stages

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 3/13/2024 5:02 PM, Dmitry Baryshkov wrote:
> It is possible to slightly bend the limitations of the HW blender. If
> two rectangles are contiguous (like two rectangles of a single plane)
> they can be blended using a single LM blending stage, allowing one to
> blend more planes via a single LM.
> 

Can you pls let me know the source of this optimization (assuming its 
present downstream) ?

Otherwise I will have to lookup some more docs to confirm this.

It certainly makes sense, that if the same layer is being split across 
two SSPP's we can certainly use the same blend stage. But want to make 
sure this is already in place somewhere and not something which was 
tried and just worked.


> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  9 ++++--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 37 ++++++++++++++++++-----
>   2 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 794c5643584f..fbbd7f635d04 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -445,6 +445,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>   
>   	uint32_t lm_idx;
>   	bool bg_alpha_enable = false;
> +	unsigned int stage_indices[DPU_STAGE_MAX] = {};
>   	DECLARE_BITMAP(fetch_active, SSPP_MAX);
>   
>   	memset(fetch_active, 0, sizeof(fetch_active));
> @@ -469,7 +470,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>   					   mixer, cstate->num_mixers,
>   					   pstate->stage,
>   					   format, fb ? fb->modifier : 0,
> -					   &pstate->pipe, 0, stage_cfg);
> +					   &pstate->pipe,
> +					   stage_indices[pstate->stage]++,
> +					   stage_cfg);
>   
>   		if (pstate->r_pipe.sspp) {
>   			set_bit(pstate->r_pipe.sspp->idx, fetch_active);
> @@ -477,7 +480,9 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
>   						   mixer, cstate->num_mixers,
>   						   pstate->stage,
>   						   format, fb ? fb->modifier : 0,
> -						   &pstate->r_pipe, 1, stage_cfg);
> +						   &pstate->r_pipe,
> +						   stage_indices[pstate->stage]++,
> +						   stage_cfg);
>   		}

Is this part of the change related to this patch? We moved from 
hard-coding 0 and 1 for the stage_idx to stage_indices[pstate->stage] 
will still result in the same values of 0 and 1 right?

The sharing will be achieved with the change below of doing
pstate->stage = prev_pstate->stage.

Rest of the change LGTM.


>   
>   		/* blend config update */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 2e1c544efc4a..43dfe13eb298 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -827,13 +827,6 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
>   	if (!new_plane_state->visible)
>   		return 0;
>   
> -	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
> -	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
> -		DPU_ERROR("> %d plane stages assigned\n",
> -			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
> -		return -EINVAL;
> -	}
> -
>   	/* state->src is 16.16, src_rect is not */
>   	drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
>   
> @@ -971,6 +964,18 @@ static int dpu_plane_try_multirect(struct dpu_plane_state *pstate,
>   		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
>   		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
>   
> +		if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1 &&
> +		    pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2 &&
> +		    pipe_cfg->dst_rect.x1 == prev_pipe_cfg->dst_rect.x2) {
> +			pstate->stage = prev_pstate->stage;
> +		} else if (pipe_cfg->dst_rect.y1 == prev_pipe_cfg->dst_rect.y1 &&
> +			   pipe_cfg->dst_rect.y2 == prev_pipe_cfg->dst_rect.y2 &&
> +			   pipe_cfg->dst_rect.x2 == prev_pipe_cfg->dst_rect.x1) {
> +			pstate->stage = prev_pstate->stage;
> +			pipe->multirect_index = DPU_SSPP_RECT_0;
> +			prev_pipe->multirect_index = DPU_SSPP_RECT_1;
> +		}
> +
>   		return true;
>   	}
>   
> @@ -1080,6 +1085,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
>   	if (!new_plane_state->visible)
>   		return 0;
>   
> +	pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
> +	if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
> +		DPU_ERROR("> %d plane stages assigned\n",
> +			  pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
> +		return -EINVAL;
> +	}
> +
>   	pipe->multirect_index = DPU_SSPP_RECT_SOLO;
>   	pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>   	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> @@ -1221,6 +1233,11 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   
>   	max_linewidth = dpu_kms->catalog->caps->max_linewidth;
>   
> +	if (prev_pstate)
> +		pstate->stage = prev_pstate->stage + 1;
> +	else
> +		pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
> +
>   	if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
>   		if (!prev_pstate ||
>   		    !dpu_plane_try_multirect(pstate, prev_pstate, fmt, max_linewidth)) {
> @@ -1267,6 +1284,12 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   		}
>   	}
>   
> +	if (pstate->stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
> +		DPU_ERROR("> %d plane stages assigned\n",
> +			  dpu_kms->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
> +		return -EINVAL;
> +	}
> +
>   	return dpu_plane_atomic_check_pipes(plane, state, crtc_state);
>   }
>