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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJppd36417E2vprowz9UJZ5C+WgPPacbY91PFoa2fjSvkXg@mail.gmail.com>
Date: Tue, 3 Sep 2024 12:11:58 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Jun Nie <jun.nie@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>, 
	Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, linux-arm-msm@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 18/21] drm/msm/dpu: blend pipes by left and right

On Tue, 3 Sept 2024 at 10:53, Jun Nie <jun.nie@...aro.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@...aro.org> 于2024年8月29日周四 19:51写道:
> >
> > On Thu, 29 Aug 2024 at 13:21, Jun Nie <jun.nie@...aro.org> wrote:
> > >
> > > Blend pipes by left and right. The first 2 pipes are for
> > > left half screen and the later 2 pipes are for right in quad
> > > pipe case.
> > >
> > > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 13 +++++++++++--
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 +++++++---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c  | 19 +++++++++++++++++--
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h  |  4 +++-
> > >  4 files changed, 38 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 3b3cd17976082..8fd56f8f2851f 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -574,8 +574,17 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
> > >                         mixer[i].mixer_op_mode,
> > >                         ctl->idx - CTL_0);
> > >
> > > -               ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx,
> > > -                       &stage_cfg);
> > > +               /*
> > > +                * call dpu_hw_ctl_setup_blendstage() to blend layers per stage cfg.
> > > +                * There is 4 mixers at most. The first 2 are for the left half, and
> > > +                * the later 2 are for the right half.
> > > +                */
> > > +               if (cstate->num_mixers == 4 && i >= 2)
> > > +                       ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx,
> > > +                               &stage_cfg, true);
> > > +               else
> > > +                       ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx,
> > > +                               &stage_cfg, false);
> > >         }
> > >  }
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index 76793201b984e..5d927f23e35b2 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -2049,9 +2049,13 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
> > >                 if (phys_enc->hw_ctl->ops.update_pending_flush_mixer)
> > >                         phys_enc->hw_ctl->ops.update_pending_flush_mixer(ctl, hw_mixer[i]->idx);
> > >
> > > -               /* clear all blendstages */
> > > -               if (phys_enc->hw_ctl->ops.setup_blendstage)
> > > -                       phys_enc->hw_ctl->ops.setup_blendstage(ctl, hw_mixer[i]->idx, NULL);
> > > +               /* clear all blendstages in both left and right */
> > > +               if (phys_enc->hw_ctl->ops.setup_blendstage) {
> > > +                       phys_enc->hw_ctl->ops.setup_blendstage(ctl,
> > > +                               hw_mixer[i]->idx, NULL, false);
> > > +                       phys_enc->hw_ctl->ops.setup_blendstage(ctl,
> > > +                               hw_mixer[i]->idx, NULL, true);
> > > +               }
> > >         }
> > >  }
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > > index 602dfad127c2a..2072d18520326 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > > @@ -478,12 +478,13 @@ static const struct ctl_blend_config ctl_blend_config[][2] = {
> > >  };
> > >
> > >  static void dpu_hw_ctl_setup_blendstage(struct dpu_hw_ctl *ctx,
> > > -       enum dpu_lm lm, struct dpu_hw_stage_cfg *stage_cfg)
> > > +       enum dpu_lm lm, struct dpu_hw_stage_cfg *stage_cfg, bool right)
> > >  {
> > >         struct dpu_hw_blk_reg_map *c = &ctx->hw;
> > >         u32 mix, ext, mix_ext;
> > >         u32 mixercfg[5] = { 0 };
> > >         int i, j;
> > > +       int pipe_start, pipe_end;
> > >         int stages;
> > >         int pipes_per_stage;
> > >
> > > @@ -502,13 +503,27 @@ static void dpu_hw_ctl_setup_blendstage(struct dpu_hw_ctl *ctx,
> > >         if (!stage_cfg)
> > >                 goto exit;
> > >
> > > +       /*
> > > +        * For quad pipe case, blend pipes in right side separately. Otherwise,
> > > +        * all content is on the left half by defaut (no splitting case).
> > > +        */
> > > +       if (!right) {
> >
> > I think the approach to set PIPES_PER_STAGE to 4 is incorrect. It
> > complicates the code too much. Instead please use two separate
> > instances, each one representing a single LM pair and corresponding
> > set of SW pipes. Yes, you'd have to iterate over them manually.
> > However I think it's also going to make code simpler.
>
> OK. I can explore this method.
> >
> > > +               pipe_start = 0;
> > > +               pipe_end = pipes_per_stage == PIPES_PER_STAGE ? 2 : 1;
> >
> > pipe_end = pipes_per_stage
> >
> > > +       } else {
> > > +               pipe_start = 2;
> > > +               pipe_end = PIPES_PER_STAGE;
> >
> > So, the right part always has 2 pipes? What if the
> > DPU_MIXER_SOURCESPLIT isn't supported?
>
> Yeah, the case is missed. It should be like this for the right half:
> pipe_start = pipes_per_stage == PIPES_PER_STAGE ? 2 : 1;
> pipe_end = pipes_per_stage == PIPES_PER_STAGE ? PIPES_PER_STAGE : 2;

Well, we can be better than that and use
pipe_start = left ? 0 : 2
pipe_end = pipe_start + pipes_per_stage;

>
>
> >
> >
> > > +       }
> > > +
> > > +       DRM_DEBUG_ATOMIC("blend lm %d on the %s side\n", lm - LM_0,
> > > +                        right ? "right" : "left");
> > >         for (i = 0; i <= stages; i++) {
> > >                 /* overflow to ext register if 'i + 1 > 7' */
> > >                 mix = (i + 1) & 0x7;
> > >                 ext = i >= 7;
> > >                 mix_ext = (i + 1) & 0xf;
> > >
> > > -               for (j = 0 ; j < pipes_per_stage; j++) {
> > > +               for (j = pipe_start; j < pipe_end; j++) {
> > >                         enum dpu_sspp_multirect_index rect_index =
> > >                                 stage_cfg->multirect_index[i][j];
> > >                         enum dpu_sspp pipe = stage_cfg->stage[i][j];
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > > index 557ec9a924f81..2dac7885fc5e7 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > > @@ -25,6 +25,8 @@ struct dpu_hw_ctl;
> > >  /**
> > >   * struct dpu_hw_stage_cfg - blending stage cfg
> > >   * @stage : SSPP_ID at each stage
> > > + *          The first 2 in PIPES_PER_STAGE(4) are for the first SSPP.
> > > + *          The 3rd/4th in PIPES_PER_STAGE(4) are for the 2nd SSPP.
> > >   * @multirect_index: index of the rectangle of SSPP.
> > >   */
> > >  struct dpu_hw_stage_cfg {
> > > @@ -243,7 +245,7 @@ struct dpu_hw_ctl_ops {
> > >          * @cfg       : blend stage configuration
> > >          */
> > >         void (*setup_blendstage)(struct dpu_hw_ctl *ctx,
> > > -               enum dpu_lm lm, struct dpu_hw_stage_cfg *cfg);
> > > +               enum dpu_lm lm, struct dpu_hw_stage_cfg *cfg, bool right);
> > >
> > >         void (*set_active_pipes)(struct dpu_hw_ctl *ctx,
> > >                 unsigned long *fetch_active);
> > >
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ