Re: [PATCH v6] drm/msm/dpu: allow sharing SSPP between planes

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

On 2/26/2025 6:31 PM, Dmitry Baryshkov wrote:
> Since SmartDMA planes provide two rectangles, it is possible to use them
> to drive two different DRM planes, first plane getting the rect_0,
> another one using rect_1 of the same SSPP. The sharing algorithm is
> pretty simple, it requires that each of the planes can be driven by the
> single rectangle and only consecutive planes are considered.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> This patch has been deferred from v4 of virtual wide patchset to
> simplify the merging path. Now as the wide planes have been merged, pick
> up the patch that allows sharing of the SSPPs between two planes.
> ---
> Changes in v6:
> - Fixed typo (consecutive) in the commit message (Abhinav)
> - Renamed prev_plane_state to prev_adjancent_plane_state (Abhinav)
> - Renamed prev_pstate to prev_adjancent_pstate as a followup to the
>    previous change
> - Link to v5: https://lore.kernel.org/r/20241215-dpu-share-sspp-v5-1-665d266183f9@linaro.org
> 
> Changes in v5:
> - Rebased on top of the current msm-next-lumag
> - Renamed dpu_plane_try_multirect() to dpu_plane_try_multirect_shared()
>    (Abhinav)
> - Link to v4: https://lore.kernel.org/dri-devel/20240314000216.392549-11-dmitry.baryshkov@linaro.org/
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 156 +++++++++++++++++++++++++-----
>   1 file changed, 130 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index af3e541f60c303eb5212524e877129359b5ca98c..11c4d5dfd9703b27b7ffde34e9b4f92ec956c3c0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -887,10 +887,9 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
>   	return 0;
>   }
>   

<snip>

>   
> +static int dpu_plane_try_multirect_shared(struct dpu_plane_state *pstate,
> +					  struct dpu_plane_state *prev_adjancent_pstate,

pls fix typo

prev_adjancent_pstate ---> prev_adjacent_pstate in this entire function

With that fixed,


Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> +					  const struct msm_format *fmt,
> +					  uint32_t max_linewidth)
> +{
> +	struct dpu_sw_pipe *pipe = &pstate->pipe;
> +	struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> +	struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> +	struct dpu_sw_pipe *prev_pipe = &prev_adjancent_pstate->pipe;
> +	struct dpu_sw_pipe_cfg *prev_pipe_cfg = &prev_adjancent_pstate->pipe_cfg;
> +	const struct msm_format *prev_fmt = msm_framebuffer_format(prev_adjancent_pstate->base.fb);
> +	u16 max_tile_height = 1;
> +
> +	if (prev_adjancent_pstate->r_pipe.sspp != NULL ||
> +	    prev_pipe->multirect_mode != DPU_SSPP_MULTIRECT_NONE)
> +		return false;
> +
> +	if (!dpu_plane_is_multirect_capable(pipe->sspp, pipe_cfg, fmt) ||
> +	    !dpu_plane_is_multirect_capable(prev_pipe->sspp, prev_pipe_cfg, prev_fmt))
> +		return false;
> +
> +	if (MSM_FORMAT_IS_UBWC(fmt))
> +		max_tile_height = max(max_tile_height, fmt->tile_height);
> +
> +	if (MSM_FORMAT_IS_UBWC(prev_fmt))
> +		max_tile_height = max(max_tile_height, prev_fmt->tile_height);
> +
> +	r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +	r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +
> +	r_pipe->sspp = NULL;
> +
> +	if (dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth) &&
> +	    dpu_plane_is_parallel_capable(prev_pipe_cfg, prev_fmt, max_linewidth) &&
> +	    (pipe_cfg->dst_rect.x1 >= prev_pipe_cfg->dst_rect.x2 ||
> +	     prev_pipe_cfg->dst_rect.x1 >= pipe_cfg->dst_rect.x2)) {
> +		pipe->sspp = prev_pipe->sspp;
> +
> +		pipe->multirect_index = DPU_SSPP_RECT_1;
> +		pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
> +
> +		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
> +		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_PARALLEL;
> +
> +		return true;
> +	}
> +
> +	if (pipe_cfg->dst_rect.y1 >= prev_pipe_cfg->dst_rect.y2 + 2 * max_tile_height ||
> +	    prev_pipe_cfg->dst_rect.y1 >= pipe_cfg->dst_rect.y2 + 2 * max_tile_height) {
> +		pipe->sspp = prev_pipe->sspp;
> +
> +		pipe->multirect_index = DPU_SSPP_RECT_1;
> +		pipe->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
> +
> +		prev_pipe->multirect_index = DPU_SSPP_RECT_0;
> +		prev_pipe->multirect_mode = DPU_SSPP_MULTIRECT_TIME_MX;
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>   static int dpu_plane_atomic_check(struct drm_plane *plane,
>   				  struct drm_atomic_state *state)
>   {
> @@ -1098,13 +1177,14 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
>   static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   					      struct dpu_global_state *global_state,
>   					      struct drm_atomic_state *state,
> -					      struct drm_plane_state *plane_state)
> +					      struct drm_plane_state *plane_state,
> +					      struct drm_plane_state *prev_adjancent_plane_state)
>   {
>   	const struct drm_crtc_state *crtc_state = NULL;
>   	struct drm_plane *plane = plane_state->plane;
>   	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>   	struct dpu_rm_sspp_requirements reqs;
> -	struct dpu_plane_state *pstate;
> +	struct dpu_plane_state *pstate, *prev_adjancent_pstate;
>   	struct dpu_sw_pipe *pipe;
>   	struct dpu_sw_pipe *r_pipe;
>   	struct dpu_sw_pipe_cfg *pipe_cfg;
> @@ -1116,6 +1196,8 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   							   plane_state->crtc);
>   
>   	pstate = to_dpu_plane_state(plane_state);
> +	prev_adjancent_pstate = prev_adjancent_plane_state ?
> +		to_dpu_plane_state(prev_adjancent_plane_state) : NULL;
>   	pipe = &pstate->pipe;
>   	r_pipe = &pstate->r_pipe;
>   	pipe_cfg = &pstate->pipe_cfg;
> @@ -1134,24 +1216,42 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   
>   	reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
>   
> -	pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> -	if (!pipe->sspp)
> -		return -ENODEV;
> +	if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
> +		if (!prev_adjancent_pstate ||
> +		    !dpu_plane_try_multirect_shared(pstate, prev_adjancent_pstate, fmt,
> +						    dpu_kms->catalog->caps->max_linewidth)) {
> +			pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> +			if (!pipe->sspp)
> +				return -ENODEV;
>   
> -	if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> -					      pipe->sspp,
> -					      msm_framebuffer_format(plane_state->fb),
> -					      dpu_kms->catalog->caps->max_linewidth)) {
> -		/* multirect is not possible, use two SSPP blocks */
> -		r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> -		if (!r_pipe->sspp)
> +			r_pipe->sspp = NULL;
> +
> +			pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +			pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +
> +			r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +			r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +		}
> +	} else {
> +		pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> +		if (!pipe->sspp)
>   			return -ENODEV;
>   
> -		pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> -		pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +		if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> +						      pipe->sspp,
> +						      msm_framebuffer_format(plane_state->fb),
> +						      dpu_kms->catalog->caps->max_linewidth)) {
> +			/* multirect is not possible, use two SSPP blocks */
> +			r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> +			if (!r_pipe->sspp)
> +				return -ENODEV;
>   
> -		r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> -		r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +			pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +			pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +
> +			r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> +			r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> +		}
>   	}
>   
>   	return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
> @@ -1164,6 +1264,7 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   			       unsigned int num_planes)
>   {
>   	unsigned int i;
> +	struct drm_plane_state *prev_adjancent_plane_state = NULL;
>   
>   	for (i = 0; i < num_planes; i++) {
>   		struct drm_plane_state *plane_state = states[i];
> @@ -1173,9 +1274,12 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   			continue;
>   
>   		int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> -							 state, plane_state);
> +							     state, plane_state,
> +							     prev_adjancent_plane_state);
>   		if (ret)
> -			return ret;
> +			break;
> +
> +		prev_adjancent_plane_state = plane_state;
>   	}
>   
>   	return 0;
> 
> ---
> base-commit: 89839e69f6154feecd79bd01171375225b0296e9
> change-id: 20241215-dpu-share-sspp-75a566eec185
> 
> Best regards,