lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a49164d9-50ec-4362-954d-5667725d1a94@quicinc.com>
Date: Sun, 23 Feb 2025 19:53:25 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Rob Clark
	<robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        Marijn Suijten
	<marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona
 Vetter <simona@...ll.ch>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
        <freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5] drm/msm/dpu: allow sharing SSPP between planes



On 12/15/2024 5:10 AM, 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 consequetive planes are considered.
> 

consequetive - > consecutive (this was highlighted in v4 too)

> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
> This patch has been deferred from v5 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 since v4:
> - 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 | 153 +++++++++++++++++++++++++-----
>   1 file changed, 128 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 098abc2c0003cde90ce6219c97ee18fa055a92a5..2fd21d27df4741a8d942f9c9bae989f319cfb8d8 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;
>   }
>   
> -static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
> -						   struct dpu_sw_pipe_cfg *pipe_cfg,
> -						   const struct msm_format *fmt,
> -						   uint32_t max_linewidth)
> +static int dpu_plane_is_multirect_capable(struct dpu_hw_sspp *sspp,
> +					  struct dpu_sw_pipe_cfg *pipe_cfg,
> +					  const struct msm_format *fmt)
>   {
>   	if (drm_rect_width(&pipe_cfg->src_rect) != drm_rect_width(&pipe_cfg->dst_rect) ||
>   	    drm_rect_height(&pipe_cfg->src_rect) != drm_rect_height(&pipe_cfg->dst_rect))
> @@ -902,10 +901,6 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
>   	if (MSM_FORMAT_IS_YUV(fmt))
>   		return false;
>   
> -	if (MSM_FORMAT_IS_UBWC(fmt) &&
> -	    drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2)
> -		return false;
> -
>   	if (!test_bit(DPU_SSPP_SMART_DMA_V1, &sspp->cap->features) &&
>   	    !test_bit(DPU_SSPP_SMART_DMA_V2, &sspp->cap->features))
>   		return false;
> @@ -913,6 +908,27 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
>   	return true;
>   }
>   
> +static int dpu_plane_is_parallel_capable(struct dpu_sw_pipe_cfg *pipe_cfg,
> +					 const struct msm_format *fmt,
> +					 uint32_t max_linewidth)
> +{
> +	if (MSM_FORMAT_IS_UBWC(fmt) &&
> +	    drm_rect_width(&pipe_cfg->src_rect) > max_linewidth / 2)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
> +						   struct dpu_sw_pipe_cfg *pipe_cfg,
> +						   const struct msm_format *fmt,
> +						   uint32_t max_linewidth)
> +{
> +	return dpu_plane_is_multirect_capable(sspp, pipe_cfg, fmt) &&
> +		dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
> +}
> +
> +
>   static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
>   				       struct drm_atomic_state *state,
>   				       const struct drm_crtc_state *crtc_state)
> @@ -1001,6 +1017,69 @@ static bool dpu_plane_try_multirect_parallel(struct dpu_sw_pipe *pipe, struct dp
>   	return true;
>   }
>   
> +static int dpu_plane_try_multirect_shared(struct dpu_plane_state *pstate,
> +					  struct dpu_plane_state *prev_pstate,
> +					  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_pstate->pipe;
> +	struct dpu_sw_pipe_cfg *prev_pipe_cfg = &prev_pstate->pipe_cfg;
> +	const struct msm_format *prev_fmt = msm_framebuffer_format(prev_pstate->base.fb);
> +	u16 max_tile_height = 1;
> +
> +	if (prev_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_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_pstate;
>   	struct dpu_sw_pipe *pipe;
>   	struct dpu_sw_pipe *r_pipe;
>   	struct dpu_sw_pipe_cfg *pipe_cfg;
> @@ -1116,6 +1196,7 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>   							   plane_state->crtc);
>   
>   	pstate = to_dpu_plane_state(plane_state);
> +	prev_pstate = prev_plane_state ? to_dpu_plane_state(prev_plane_state) : NULL;
>   	pipe = &pstate->pipe;
>   	r_pipe = &pstate->r_pipe;
>   	pipe_cfg = &pstate->pipe_cfg;
> @@ -1134,24 +1215,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_pstate ||
> +		    !dpu_plane_try_multirect_shared(pstate, prev_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);
> @@ -1165,6 +1264,7 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   {
>   	unsigned int i;
>   	int ret;
> +	struct drm_plane_state *prev_plane_state = NULL;
>   
>   	for (i = 0; i < num_planes; i++) {
>   		struct drm_plane_state *plane_state = states[i];
> @@ -1174,9 +1274,12 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
>   			continue;
>   
>   		ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> -							 state, plane_state);
> +							 state, plane_state,
> +							 prev_plane_state);
>   		if (ret)
>   			break;
> +
> +		prev_plane_state = plane_state;
>   	}

We agreed to rename this to prev_adjacent_plane_state in v4. This has 
not been addressed.

With these two changes done,

Reviewed-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>   
>   	return ret;
> 
> ---
> base-commit: d82c9281189d2b27642ede2760db495379503b86
> change-id: 20241215-dpu-share-sspp-75a566eec185
> 
> Best regards,


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ