On Wed, 12 Jun 2024 at 04:48, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
>
>
>
> 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.
My source was the original 'virtual' / 'multirect' implementation in
the SDE driver.
>
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > ---
> > 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?
No. The stage can span multiple planes now, see one of the chunks below.
>
> 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);
> > }
> >
--
With best wishes
Dmitry
