[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAO9ioeUS5Oq1Ka9uh1idourTNsfp5bg4TcVWSMV_rkByy+e9Yg@mail.gmail.com>
Date: Fri, 30 May 2025 18:54:29 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
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>, Simona Vetter <simona@...ll.ch>,
Jessica Zhang <quic_jesszhan@...cinc.com>,
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 v10 10/12] drm/msm/dpu: support SSPP assignment for
quad-pipe case
On Fri, 30 May 2025 at 17:59, Jun Nie <jun.nie@...aro.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年5月29日周四 02:22写道:
> >
> > On Mon, May 26, 2025 at 05:28:28PM +0800, Jun Nie wrote:
> > > Currently, SSPPs are assigned to a maximum of two pipes. However,
> > > quad-pipe usage scenarios require four pipes and involve configuring
> > > two stages. In quad-pipe case, the first two pipes share a set of
> > > mixer configurations and enable multi-rect mode when certain
> > > conditions are met. The same applies to the subsequent two pipes.
> > >
> > > Assign SSPPs to the pipes in each stage using a unified method and
> > > to loop the stages accordingly.
> > >
> > > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 +++
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 +
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 126 ++++++++++++++++++------------
> > > 3 files changed, 88 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 85f585206218f4578e18b00452762dbada060e9c..47ab43dfec76acc058fb275d1928603e8e8e7fc6 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -1562,6 +1562,17 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * dpu_crtc_get_num_lm - Get mixer number in this CRTC pipeline
> > > + * @state: Pointer to drm crtc state object
> > > + */
> > > +unsigned int dpu_crtc_get_num_lm(const struct drm_crtc_state *state)
> > > +{
> > > + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
> > > +
> > > + return cstate->num_mixers;
> > > +}
> > > +
> > > #ifdef CONFIG_DEBUG_FS
> > > static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
> > > {
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > index 94392b9b924546f96e738ae20920cf9afd568e6b..6eaba5696e8e6bd1246a9895c4c8714ca6589b10 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > @@ -267,4 +267,6 @@ static inline enum dpu_crtc_client_type dpu_crtc_get_client_type(
> > >
> > > void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event);
> > >
> > > +unsigned int dpu_crtc_get_num_lm(const struct drm_crtc_state *state);
> > > +
> > > #endif /* _DPU_CRTC_H_ */
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > index 0bb153a71353ca9eaca138ebbee4cd699414771d..f721dc504bbbe3a49986239adee113bfb6790f70 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > @@ -961,6 +961,33 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
> > > dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
> > > }
> > >
> > > +static bool dpu_plane_check_single_pipe(struct dpu_plane_state *pstate,
> > > + struct dpu_sw_pipe **single_pipe,
> > > + struct dpu_sw_pipe_cfg **single_pipe_cfg,
> > > + bool config_pipe)
> > > +{
> > > + int i, valid_pipe = 0;
> > > + struct dpu_sw_pipe *pipe;
> > > +
> > > + for (i = 0; i < PIPES_PER_PLANE; i++) {
> > > + if (drm_rect_width(&pstate->pipe_cfg[i].src_rect) != 0) {
> > > + valid_pipe++;
> > > + if (valid_pipe > 1)
> > > + return false;
> > > + *single_pipe = &pstate->pipe[i];
> > > + *single_pipe_cfg = &pstate->pipe_cfg[i];
> > > + } else {
> > > + if (!config_pipe)
> > > + continue;
> > > + pipe = &pstate->pipe[i];
> > > + pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > + pipe->sspp = NULL;
> >
> > If this function is 'check', then why does it change something in the
> > pipe configuration?
>
> I see modification is made in other check functions, like
> dpu_plane_atomic_check_nosspp(). So
> the name is referenced. Do you think dpu_plane_get_single_pipe() is OK here?
Because it follows the semantics of drm_foo_atomic_check_bar(), the
_atomic_check_ being a key part.
--
With best wishes
Dmitry
Powered by blists - more mailing lists