[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABymUCPkfYHhtxEqN7KgBPHEvYh=NRotutJ=eiLm=ZUB85t-Qw@mail.gmail.com>
Date: Thu, 15 Jan 2026 17:57:01 +0800
From: Jun Nie <jun.nie@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Abhinav Kumar <abhinav.kumar@...ux.dev>, Dmitry Baryshkov <lumag@...nel.org>,
Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Clark <robin.clark@....qualcomm.com>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v16 09/10] drm/msm/dpu: support plane splitting in
quad-pipe case
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2026年1月15日周四 17:39写道:
>
> On Thu, Jan 15, 2026 at 05:34:28PM +0800, Jun Nie wrote:
> > Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2026年1月15日周四 00:12写道:
> > >
> > > On Wed, Jan 14, 2026 at 10:48:17PM +0800, Jun Nie wrote:
> > > > Jun Nie <jun.nie@...aro.org> 于2025年9月18日周四 21:30写道:
> > > > >
> > > > > The content of every half of screen is sent out via one interface in
> > > > > dual-DSI case. The content for every interface is blended by a LM
> > > > > pair in quad-pipe case, thus a LM pair should not blend any content
> > > > > that cross the half of screen in this case. Clip plane into pipes per
> > > > > left and right half screen ROI if topology is quad pipe case.
> > > > >
> > > > > The clipped rectangle on every half of screen is futher handled by two
> > > > > pipes if its width exceeds a limit for a single pipe.
> > > > >
> > > > > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> > > > > Reviewed-by: Jessica Zhang <jessica.zhang@....qualcomm.com>
> > > > > ---
> > > > > 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 | 137 +++++++++++++++++++++---------
> > > > > 3 files changed, 110 insertions(+), 40 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > index d825eb8e40ae8bd456ede6269951339e3053d0d3..e925d93b38feac0594d735fdc2c5b9fd5ae83e6a 100644
> > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > @@ -1604,6 +1604,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 5ae58352cbee1251a0140879f04fc7c304cae674..89a5feb6308bcac537562c3dc4e61c16c92e460c 100644
> > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > > @@ -824,8 +824,12 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
> > > > > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> > > > > struct dpu_sw_pipe_cfg *pipe_cfg;
> > > > > struct dpu_sw_pipe_cfg *r_pipe_cfg;
> > > > > + struct dpu_sw_pipe_cfg init_pipe_cfg;
> > > > > struct drm_rect fb_rect = { 0 };
> > > > > + const struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> > > > > uint32_t max_linewidth;
> > > > > + u32 num_lm;
> > > > > + int stage_id, num_stages;
> > > > >
> > > > > min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO);
> > > > > max_scale = MAX_DOWNSCALE_RATIO << 16;
> > > > > @@ -848,13 +852,10 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
> > > > > return -EINVAL;
> > > > > }
> > > > >
> > > > > - /* move the assignment here, to ease handling to another pairs later */
> > > > > - pipe_cfg = &pstate->pipe_cfg[0];
> > > > > - r_pipe_cfg = &pstate->pipe_cfg[1];
> > > > > - /* state->src is 16.16, src_rect is not */
> > > > > - drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
> > > > > + num_lm = dpu_crtc_get_num_lm(crtc_state);
> > > > >
> > > > > - pipe_cfg->dst_rect = new_plane_state->dst;
> > > > > + /* state->src is 16.16, src_rect is not */
> > > > > + drm_rect_fp_to_int(&init_pipe_cfg.src_rect, &new_plane_state->src);
> > > > >
> > > > > fb_rect.x2 = new_plane_state->fb->width;
> > > > > fb_rect.y2 = new_plane_state->fb->height;
> > > > > @@ -879,35 +880,94 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
> > > > >
> > > > > max_linewidth = pdpu->catalog->caps->max_linewidth;
> > > > >
> > > > > - drm_rect_rotate(&pipe_cfg->src_rect,
> > > > > + drm_rect_rotate(&init_pipe_cfg.src_rect,
> > > > > new_plane_state->fb->width, new_plane_state->fb->height,
> > > > > new_plane_state->rotation);
> > > > >
> > > > > - if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
> > > > > - _dpu_plane_calc_clk(&crtc_state->adjusted_mode, pipe_cfg) > max_mdp_clk_rate) {
> > > > > - if (drm_rect_width(&pipe_cfg->src_rect) > 2 * max_linewidth) {
> > > > > - DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
> > > > > - DRM_RECT_ARG(&pipe_cfg->src_rect), max_linewidth);
> > > > > - return -E2BIG;
> > > > > + /*
> > > > > + * We have 1 mixer pair cfg for 1:1:1 and 2:2:1 topology, 2 mixer pair
> > > > > + * configs for left and right half screen in case of 4:4:2 topology.
> > > > > + * But we may have 2 rect to split wide plane that exceeds limit with 1
> > > > > + * config for 2:2:1. So need to handle both wide plane splitting, and
> > > > > + * two halves of screen splitting for quad-pipe case. Check dest
> > > > > + * rectangle left/right clipping first, then check wide rectangle
> > > > > + * splitting in every half next.
> > > > > + */
> > > > > + num_stages = (num_lm + 1) / 2;
> > > >
> > > > Hi Dmitry,
> > > > Because the plane is checked before crtc is checked in the drm framework. While
> > > > the topology is decided in crtc check. Thus num_lm is 0 when this function is
> > > > called for the first time. As a result, the below iteration is not run
> > > > at all and leads
> > > > to iommu warning.
> > >
> > > How does it lead to IOMMU warnings?
> >
> > Because the pipe is not configured with width/height etc when the iteration is
> > skipped. I have not found the root cause so far. But per the null IOMMU iova
> > value, suppose it is due to DMA buffer not being prepared when DMA is started.
>
> I'd think, that corresponding SRC regs are either garbage or zero programmed.
You are right in that. Sorry for my words is not accurate. I mean the
DMA buffer is not
feed to DMA engine correctly.
>
> >
> > >
> > > > Do you suggest to change drm framework with adding extra crtc check before
> > > > plane check, or you prefer the below line here?
> > > >
> > > > num_stages = max(1, (num_lm + 1) / 2);
> > >
> > > DRM framework provides enough hooks to be able to influence the order or
> > > operations without changing the framework. But, I'd like to point out
> > > that for the virtual plane case we already perform plane operations
> > > from dpu_crtc_atomic_check(). You can employ the same approach.
> >
> > Thanks for the suggestion! I see dpu_assign_plane_resources() is called
> > from crtc side, which avoids the plane splitting before topology decision.
> > To use this method, it looks like we are enabling the virtual plane by default.
> > Because the virtual plane differs from the traditional method only with the
> > plane splitting and resource preparation. Can we just enable the virtual
> > plane by default in this situation?
>
> In which situation? It is a global switch. And we need to be able to
> work with it turned off, until corresponding code is dropped.
I mean the situation that the plane SSPP allocation and related resource
preparation shall be deferred until crtc calling the plane API. In this way,
the traditional plane management is almost identical with the virtual
plane method. Or could you point out what shall differ for the two methods
after we deferred the preparation? Thanks!
Jun
>
> >
> > Jun
> >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry
Powered by blists - more mailing lists