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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux