Re: [PATCH v21] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case

<[email protected]> 于2026年7月27日周一 17:55写道：

>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Unused pipe configurations are not cleared when transitioning from quad-pipe to fewer pipes, leading to underflows, stale pointer dereferences, and hardware corruption.
> - [High] Missing VSYNC source configuration for the second physical encoder in quad-pipe configurations.
>
> Pre-existing issues:
> - [High] `dpu_crtc_get_lm_crc` and `dpu_crtc_get_encoder_crc` leak uninitialized kernel stack memory to userspace.
> --
>
> --- Patch [1]: [PATCH v21] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case ---
> commit f34d7a5a3191bc6943da26710d69eaffa718ab5d
> Author: Jun Nie <[email protected]>
> Subject: drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case
>
> This commit expands the pipe array size to 4 to support quad-pipe topologies.
> It requests 4 mixers and 4 DSCs for high-resolution scenarios where dual
> interfaces are enabled for virtual planes. Additional pipes enable parallel
> data processing to stay within width constraints and MDP clock rates.
>
> Link: https://lore.kernel.org/r/20250918-v6-16-rc2-quad-pipe-upstream-4-v16-10-ff6232e3472f@linaro.org
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 42d0a529b4d51..1c6d65ca561fd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -200,7 +200,7 @@ static int dpu_crtc_get_lm_crc(struct drm_crtc *crtc,
> >               struct dpu_crtc_state *crtc_state)
> >  {
> >       struct dpu_crtc_mixer *m;
> > -     u32 crcs[CRTC_DUAL_MIXERS];
> > +     u32 crcs[CRTC_QUAD_MIXERS];
>
> [Severity: High]
> This is a pre-existing issue, but does expanding this array to 4 increase the
> amount of uninitialized kernel stack memory leaked to userspace?
>
> When populating the array in dpu_crtc_get_lm_crc(), if a layer mixer is
> unassigned or lacks the collect_misr operation, the loop skips initialization
> for that element. Also, if num_mixers dynamically decreases after CRC
> generation is enabled, drm_crtc_add_crc_entry() still copies values_cnt
> elements, which can read past the initialized elements.
>
> Can this same uninitialized stack memory leak occur in
> dpu_crtc_get_encoder_crc() with crcs[INTF_MAX]?

Increasing the array size here does not mean leaking stack to user space. The
crc data is copied to user space with necessary length that's from the
calculation
of  dpu_encoder_get_crc_values_cnt().
>
> [ ... ]
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 778e231d49677..90d44c4777fd0 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -55,7 +55,7 @@
> >  #define MAX_PHYS_ENCODERS_PER_VIRTUAL \
> >       (MAX_H_TILES_PER_DISPLAY * NUM_PHYS_ENCODER_TYPES)
> >
> > -#define MAX_CHANNELS_PER_ENC 2
> > +#define MAX_CHANNELS_PER_ENC 4
>
> [Severity: High]
> Does this quad-pipe configuration leave the second interface without a valid
> VSYNC source?
>
> In a quad-pipe dual-interface configuration, the mapping between physical
> encoders and pingpong blocks uses a stride of 2 (the first uses hw_pp[0] and
> the second uses hw_pp[2]).
>
> However, _dpu_encoder_update_vsync_source() appears to assign VSYNC sources
> using sequential indices:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:_dpu_encoder_update_vsync_source() {
>     ...
>     for (i = 0; i < dpu_enc->num_phys_encs; i++)
>         vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx;
>     ...
> }
>
> Will this configure hw_pp[0] and hw_pp[1] instead of the intended hw_pp[0]
> and hw_pp[2], potentially causing TE timeouts and visual tearing?

_dpu_encoder_update_vsync_source is used only for cmd mode DSI panel. While only
video mode panel is supported for dual panel currently. And the VSYNC
source should
be aligned in timing, due to the 2 panel are modeled as single logic
panel in CRTC level.
So there is only 1 VSYNC source in high level DRM framework.
>
> [ ... ]
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > index 0e65bf5ddc4a6..fd1f3e7982062 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > @@ -34,7 +34,7 @@
> >  #define DPU_MAX_PLANES                       4
> >  #endif
> >
> > -#define STAGES_PER_PLANE             1
> > +#define STAGES_PER_PLANE             2
>
> [Severity: High]
> Can increasing STAGES_PER_PLANE to 2 lead to a stale pointer dereference or
> hardware corruption when transitioning to fewer pipes?
>
> When transitioning from a quad-pipe topology (4 mixers, 2 stages) to a single
> or dual-pipe topology (1 stage), the duplicated pipe_cfg array retains stale
> rectangle configurations because dpu_plane_split() only clears up to the new
> num_stages:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c:dpu_plane_split() {
>     ...
>     for (stage_id = 0; stage_id < num_stages; stage_id++) {
>     ...
> }
>
> Later, dpu_plane_virtual_assign_resources() iterates up to STAGES_PER_PLANE
> and allocates an SSPP for the stale configuration.
>
> During the atomic commit, if cstate->num_mixers shrinks to 1, the calculation
> in _dpu_crtc_blend_setup_mixer() underflows:
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:_dpu_crtc_blend_setup_mixer() {
>     ...
>     lms_in_pair = min(cstate->num_mixers - (stage * PIPES_PER_STAGE),
>                       PIPES_PER_STAGE);
>     ...
> }
>
> This causes min(4294967295U, 2U) to evaluate to 2.
>
> Will _dpu_crtc_blend_setup_pipe() then attempt to program lms_in_pair = 2
> mixers using a pointer starting at mixer[2]? Since num_mixers == 1, could
> accessing mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(...) cause a
> NULL pointer dereference?

No. It will not happen, because pipe and pipe_cfg is allocated and zeroed for
every plane preparation in dpu_crtc_assign_planes. If num_mixers is reset, the
pipe_cfg is reconfigured, sspp will not be allocated for unused pipe_cfg. Thus
below check in _dpu_crtc_blend_setup_mixer() will avoid accessing to unused
 mixer[lm_idx]

if (!pstate->pipe[pipe_idx].sspp)
   continue;

+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -883,6 +883,8 @@ static int dpu_plane_split(struct drm_plane *plane,
        u32 num_lm;
        int stage_id, num_stages;

+       memset(&pstate->pipe[0], PIPES_PER_PLANE * sizeof(struct dpu_sw_pipe));
+       memset(&pstate->pipe_cfg[0], PIPES_PER_PLANE * sizeof(struct
dpu_sw_pipe_cfg));
        max_linewidth = pdpu->catalog->caps->max_linewidth;

        /* In non-virtual plane case, one mixer pair is always needed. */
@@ -936,11 +938,8 @@ static int dpu_plane_split(struct drm_plane *plane,
                 */
                if (!drm_rect_clip_scaled(&pipe_cfg->src_rect,
                                          &pipe_cfg->dst_rect,
-                                         &mixer_rect)) {
-                       memset(pipe_cfg, 0, 2 * sizeof(struct dpu_sw_pipe_cfg));
-
+                                         &mixer_rect))
                        continue;
-               }

                pipe_cfg->dst_rect.x1 -= mixer_rect.x1;
                pipe_cfg->dst_rect.x2 -= mixer_rect.x1;
@@ -967,8 +966,6 @@ static int dpu_plane_split(struct drm_plane *plane,
                                        DRM_RECT_FMT " and " DRM_RECT_FMT "\n",
                                        DRM_RECT_ARG(&pipe_cfg->src_rect),
                                        DRM_RECT_ARG(&r_pipe_cfg->src_rect));
-               } else {
-                       memset(r_pipe_cfg, 0, sizeof(struct dpu_sw_pipe_cfg));
                }


>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1