[<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