* [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI
@ 2025-05-06 15:47 Jun Nie
2025-05-06 15:47 ` [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane Jun Nie
` (13 more replies)
0 siblings, 14 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
2 or more SSPPs and dual-DSI interface are need for super wide panel.
And 4 DSC are preferred for power optimal in this case due to width
limitation of SSPP and MDP clock rate constrain. This patch set
extends number of pipes to 4 and revise related mixer blending logic
to support quad pipe. All these changes depends on the virtual plane
feature to split a super wide drm plane horizontally into 2 or more sub
clip. Thus DMA of multiple SSPPs can share the effort of fetching the
whole drm plane.
The first pipe pair co-work with the first mixer pair to cover the left
half of screen and 2nd pair of pipes and mixers are for the right half
of screen. If a plane is only for the right half of screen, only one
or two of pipes in the 2nd pipe pair are valid, and no SSPP or mixer is
assinged for invalid pipe.
For those panel that does not require quad-pipe, only 1 or 2 pipes in
the 1st pipe pair will be used. There is no concept of right half of
screen.
For legacy non virtual plane mode, the first 1 or 2 pipes are used for
the single SSPP and its multi-rect mode.
Changes in v9:
- Rebase to latest mainline and drop 3 patches as mainline already cover
the logic.
"Do not fix number of DSC"
"configure DSC per number in use"
"switch RM to use crtc_id rather than enc_id for allocation"
- Add a patch to check crtc before checking plane in drm framework.
- Add a patch to use dedicated WB number in an encoder to avoid regression.
- Revise the condition to decide quad-pipe topology.
- Link to v8: https://lore.kernel.org/r/20250303-sm8650-v6-14-hmd-deckard-mdss-quad-upstream-oldbootwrapper-36-prep-v8-0-eb5df105c807@linaro.org
Changes in v8:
- Fix looping pipes of a plane in _dpu_plane_color_fill()
- Improve pipe assignment with deleting pipes loop in stage.
- Define PIPES_PER_PLANE properly when it appears fisrt.
- rename lms_in_pair to lms_in_stage to avoid confusion.
- Add review tags.
- Link to v7: https://lore.kernel.org/r/20250226-sm8650-v6-14-hmd-deckard-mdss-quad-upstream-oldbootwrapper-36-prep-v7-0-8d5f5f426eb2@linaro.org
Changes in v7:
- Improve pipe assignment to avoid point to invalid memory.
- Define STAGES_PER_PLANE as 2 only when quad-pipe is introduced.
- Polish LM number when blending pipes with min() and pull up to caller func.
- Add review tags.
- Link to v6: https://lore.kernel.org/r/20250217-sm8650-v6-14-hmd-deckard-mdss-quad-upstream-oldbootwrapper-36-prep-v6-0-c11402574367@linaro.org
Changes in v6:
- Replace LM number with PP number to calculate PP number per encoder.
- Rebase to Linux v6.14-rc2.
- Add review tags.
- Link to v5: https://lore.kernel.org/r/20250118-sm8650-v6-13-hmd-deckard-mdss-quad-upstream-33-v5-0-9701a16340da@linaro.org
Changes in v5:
- Iterate SSPP flushing within the required mixer pair, instead of all
active mixers or specific mixer.
- Limit qaud-pipe usage case to SoC with 4 or more DSC engines and 2
interfaces case.
- Remove valid flag and use width for pipe validation.
- Polish commit messages and code comments.
- Link to v4: https://lore.kernel.org/r/20250116-sm8650-v6-13-hmd-deckard-mdss-quad-upstream-33-v4-0-74749c6eba33@linaro.org
Changes in v4:
- Restrict SSPP flushing to the required mixer, instead of all active mixers.
- Polish commit messages and code comments.
- Rebase to latest msm/drm-next branch.
- Move pipe checking patch to the top of patch set.
- Link to v3: https://lore.kernel.org/dri-devel/20241219-sm8650-v6-13-hmd-deckard-mdss-quad-upstream-32-v3-0-92c7c0a228e3@linaro.org
Changes in v3:
- Split change in trace into a separate patch.
- Rebase to latest msm-next branch.
- Reorder patch sequence to make sure valid flag is set in earlier patch
- Rectify rewrite patch to move logic change into other patch
- Polish commit messages and code comments.
- Link to v2: https://lore.kernel.org/dri-devel/20241009-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-21-v2-0-76d4f5d413bf@linaro.org
Changes in v2:
- Revise the patch sequence with changing to 2 pipes topology first. Then
prepare for quad-pipe setup, then enable quad-pipe at last.
- Split DSI patches into other patch set.
- Link to v1: https://lore.kernel.org/all/20240829-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-8-v1-0-bdb05b4b5a2e@linaro.org
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
Jun Nie (14):
drm/atomic-helper: Add crtc check before checking plane
drm/msm/dpu: check every pipe per capability
drm/msm/dpu: polish log for resource allocation
drm/msm/dpu: decide right side per last bit
drm/msm/dpu: fix mixer number counter on allocation
drm/msm/dpu: bind correct pingpong for quad pipe
drm/msm/dpu: Add pipe as trace argument
drm/msm/dpu: handle pipes as array
drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer
drm/msm/dpu: Use dedicated WB number definition
drm/msm/dpu: blend pipes per mixer pairs config
drm/msm/dpu: support SSPP assignment for quad-pipe case
drm/msm/dpu: support plane splitting in quad-pipe case
drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case
drivers/gpu/drm/drm_atomic_helper.c | 19 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 115 ++++---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 8 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 43 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 408 ++++++++++++++---------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 29 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +-
11 files changed, 403 insertions(+), 247 deletions(-)
---
base-commit: 227403a8562686275197009528ff9f1b440cb20f
change-id: 20250506-quad-pipe-upstream-a640ddc6b1b6
Best regards,
--
Jun Nie <jun.nie@linaro.org>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 10:47 ` Dmitry Baryshkov
2025-05-06 15:47 ` [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability Jun Nie
` (12 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Some display controller support flexible CRTC and DMA, such as the display
controllers in snapdragon SoCs. CRTC can be implemented with several mixers
in parallel, and plane fetching can be implemented with several DMA under
umberala of a virtual drm plane.
The mixer number is decided per panel resolution and clock rate constrain
first, which happens in CRTC side. Then plane is split per mixer number
and configure DMA accordingly.
To support such forthcoming usage case, CRTC checking shall happen before
checking plane. Add the checking in the drm_atomic_helper_check_modeset().
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5302ab3248985d3e0a47e40fd3deb7ad0d9f775b..5bca4c9683838c38574c8cb7c0bc9d57960314fe 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -816,6 +816,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
return ret;
}
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+ const struct drm_crtc_helper_funcs *funcs;
+
+ funcs = crtc->helper_private;
+
+ if (!funcs || !funcs->atomic_check)
+ continue;
+
+ ret = funcs->atomic_check(crtc, state);
+ if (ret) {
+ drm_dbg_atomic(crtc->dev,
+ "[CRTC:%d:%s] atomic driver check failed\n",
+ crtc->base.id, crtc->name);
+ return ret;
+ }
+ }
+
+
+
ret = mode_valid(state);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
2025-05-06 15:47 ` [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 18:38 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 03/14] drm/msm/dpu: polish log for resource allocation Jun Nie
` (11 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
The capability stored in sblk and pipe_hw_caps is checked only for
SSPP of the first pipe in the pair with current implementation. That
of the 2nd pipe, r_pipe, is not checked and may violate hardware
capability. Move requirement check to dpu_plane_atomic_check_pipe()
for the check of every pipe.
Fixes: ("dbbf57dfd04e6 drm/msm/dpu: split dpu_plane_atomic_check()")
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 ++++++++++++++++---------------
1 file changed, 36 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index af3e541f60c303eb5212524e877129359b5ca98c..aeb90c287245d6aaa18b9f280d1e628ee6ed74f5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -729,12 +729,40 @@ static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
struct dpu_sw_pipe *pipe,
struct dpu_sw_pipe_cfg *pipe_cfg,
- const struct msm_format *fmt,
- const struct drm_display_mode *mode)
+ const struct drm_display_mode *mode,
+ struct drm_plane_state *new_plane_state)
{
uint32_t min_src_size;
struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
int ret;
+ const struct msm_format *fmt;
+ uint32_t supported_rotations;
+ const struct dpu_sspp_cfg *pipe_hw_caps;
+ const struct dpu_sspp_sub_blks *sblk;
+
+ pipe_hw_caps = pipe->sspp->cap;
+ sblk = pipe->sspp->cap->sblk;
+
+ /*
+ * We already have verified scaling against platform limitations.
+ * Now check if the SSPP supports scaling at all.
+ */
+ if (!sblk->scaler_blk.len &&
+ ((drm_rect_width(&new_plane_state->src) >> 16 !=
+ drm_rect_width(&new_plane_state->dst)) ||
+ (drm_rect_height(&new_plane_state->src) >> 16 !=
+ drm_rect_height(&new_plane_state->dst))))
+ return -ERANGE;
+
+ fmt = msm_framebuffer_format(new_plane_state->fb);
+
+ supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
+
+ if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
+ supported_rotations |= DRM_MODE_ROTATE_90;
+
+ pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
+ supported_rotations);
min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -923,47 +951,20 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
struct dpu_sw_pipe *pipe = &pstate->pipe;
struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
- const struct msm_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
- uint32_t supported_rotations;
- const struct dpu_sspp_cfg *pipe_hw_caps;
- const struct dpu_sspp_sub_blks *sblk;
int ret = 0;
- pipe_hw_caps = pipe->sspp->cap;
- sblk = pipe->sspp->cap->sblk;
-
- /*
- * We already have verified scaling against platform limitations.
- * Now check if the SSPP supports scaling at all.
- */
- if (!sblk->scaler_blk.len &&
- ((drm_rect_width(&new_plane_state->src) >> 16 !=
- drm_rect_width(&new_plane_state->dst)) ||
- (drm_rect_height(&new_plane_state->src) >> 16 !=
- drm_rect_height(&new_plane_state->dst))))
- return -ERANGE;
-
- fmt = msm_framebuffer_format(new_plane_state->fb);
-
- supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
-
- if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
- supported_rotations |= DRM_MODE_ROTATE_90;
-
- pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
- supported_rotations);
- r_pipe_cfg->rotation = pipe_cfg->rotation;
-
- ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt,
- &crtc_state->adjusted_mode);
+ ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
+ &crtc_state->adjusted_mode,
+ new_plane_state);
if (ret)
return ret;
if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
- ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt,
- &crtc_state->adjusted_mode);
+ ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
+ &crtc_state->adjusted_mode,
+ new_plane_state);
if (ret)
return ret;
}
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 03/14] drm/msm/dpu: polish log for resource allocation
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
2025-05-06 15:47 ` [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane Jun Nie
2025-05-06 15:47 ` [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 04/14] drm/msm/dpu: decide right side per last bit Jun Nie
` (10 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
It is more likely that resource allocation may fail in complex usage
case, such as quad-pipe case, than existing usage cases.
A resource type ID is printed on failure in the current implementation,
but the raw ID number is not explicit enough to help easily understand
which resource caused the failure, so add a table to match the type ID
to an human readable resource name and use it in the error print.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 2e296f79cba1437470eeb30900a650f6f4e334b6..2d8ff4b524715c658188fe56bc337e3ffa831c0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -865,6 +865,21 @@ void dpu_rm_release_all_sspp(struct dpu_global_state *global_state,
ARRAY_SIZE(global_state->sspp_to_crtc_id), crtc_id);
}
+static char *dpu_hw_blk_type_name[] = {
+ [DPU_HW_BLK_TOP] = "TOP",
+ [DPU_HW_BLK_SSPP] = "SSPP",
+ [DPU_HW_BLK_LM] = "LM",
+ [DPU_HW_BLK_CTL] = "CTL",
+ [DPU_HW_BLK_PINGPONG] = "pingpong",
+ [DPU_HW_BLK_INTF] = "INTF",
+ [DPU_HW_BLK_WB] = "WB",
+ [DPU_HW_BLK_DSPP] = "DSPP",
+ [DPU_HW_BLK_MERGE_3D] = "merge_3d",
+ [DPU_HW_BLK_DSC] = "DSC",
+ [DPU_HW_BLK_CDM] = "CDM",
+ [DPU_HW_BLK_MAX] = "unknown",
+};
+
/**
* dpu_rm_get_assigned_resources - Get hw resources of the given type that are
* assigned to this encoder
@@ -946,13 +961,13 @@ int dpu_rm_get_assigned_resources(struct dpu_rm *rm,
}
if (num_blks == blks_size) {
- DPU_ERROR("More than %d resources assigned to crtc %d\n",
- blks_size, crtc_id);
+ DPU_ERROR("More than %d %s assigned to crtc %d\n",
+ blks_size, dpu_hw_blk_type_name[type], crtc_id);
break;
}
if (!hw_blks[i]) {
- DPU_ERROR("Allocated resource %d unavailable to assign to crtc %d\n",
- type, crtc_id);
+ DPU_ERROR("%s unavailable to assign to crtc %d\n",
+ dpu_hw_blk_type_name[type], crtc_id);
break;
}
blks[num_blks++] = hw_blks[i];
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 04/14] drm/msm/dpu: decide right side per last bit
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (2 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 03/14] drm/msm/dpu: polish log for resource allocation Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 05/14] drm/msm/dpu: fix mixer number counter on allocation Jun Nie
` (9 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Currently, only one pair of mixers is supported, so a non-zero counter
value is sufficient to identify the correct mixer within that pair.
However, future implementations may involve multiple mixer pairs. With
the current implementation, all mixers within the second pair would be
incorrectly selected as right mixer. To correctly select the mixer
within a pair, test the least significant bit of the counter. If the
least significant bit is not set, select the mixer as left one;
otherwise, select the mixer as right one for all pairs.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0714936d883523e5c53bfb42f932234db76c58db..b0ee89ed7053f940d01bb55a8ea8de8ec5bc3462 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -369,11 +369,10 @@ static void _dpu_crtc_setup_blend_cfg(struct dpu_crtc_mixer *mixer,
static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
{
struct dpu_crtc_state *crtc_state;
- int lm_idx, lm_horiz_position;
+ int lm_idx;
crtc_state = to_dpu_crtc_state(crtc->state);
- lm_horiz_position = 0;
for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) {
const struct drm_rect *lm_roi = &crtc_state->lm_bounds[lm_idx];
struct dpu_hw_mixer *hw_lm = crtc_state->mixers[lm_idx].hw_lm;
@@ -384,7 +383,7 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
cfg.out_width = drm_rect_width(lm_roi);
cfg.out_height = drm_rect_height(lm_roi);
- cfg.right_mixer = lm_horiz_position++;
+ cfg.right_mixer = lm_idx & 0x1;
cfg.flags = 0;
hw_lm->ops.setup_mixer_out(hw_lm, &cfg);
}
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 05/14] drm/msm/dpu: fix mixer number counter on allocation
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (3 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 04/14] drm/msm/dpu: decide right side per last bit Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 06/14] drm/msm/dpu: bind correct pingpong for quad pipe Jun Nie
` (8 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Current code only supports usage cases with one pair of mixers at
most. To support quad-pipe usage case, two pairs of mixers need to
be reserved. The lm_count for all pairs is cleared if a peer
allocation fails in current implementation. Reset the current lm_count
to an even number instead of completely clearing it. This prevents all
pairs from being cleared in cases where multiple LM pairs are needed.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 2d8ff4b524715c658188fe56bc337e3ffa831c0a..bc7639a46386c6b9457edf3afdf6f747a632651f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -374,7 +374,11 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm,
if (!rm->mixer_blks[i])
continue;
- lm_count = 0;
+ /*
+ * Reset lm_count to an even index. This will drop the previous
+ * primary mixer if failed to find its peer.
+ */
+ lm_count &= ~1;
lm_idx[lm_count] = i;
if (!_dpu_rm_check_lm_and_get_connected_blks(rm, global_state,
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 06/14] drm/msm/dpu: bind correct pingpong for quad pipe
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (4 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 05/14] drm/msm/dpu: fix mixer number counter on allocation Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 07/14] drm/msm/dpu: Add pipe as trace argument Jun Nie
` (7 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
There are 2 interfaces and 4 pingpong in quad pipe. Map the 2nd
interface to 3rd PP instead of the 2nd PP.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3edfcd7d49c1efb2b7ed2b9f0cc38a4b3073acf6..3e4ff345bd6d9c0e2a08274874fac40690409cf0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1157,7 +1157,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_cwb[MAX_CHANNELS_PER_ENC];
- int num_ctl, num_pp, num_dsc;
+ int num_ctl, num_pp, num_dsc, num_pp_per_intf;
int num_cwb = 0;
bool is_cwb_encoder;
unsigned int dsc_mask = 0;
@@ -1236,10 +1236,16 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
}
+ /*
+ * There may be 4 PP and 2 INTF for quad pipe case, so INTF is not
+ * mapped to PP 1:1. Let's calculate the stride with pipe/INTF
+ */
+ num_pp_per_intf = num_pp / dpu_enc->num_phys_encs;
+
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
- phys->hw_pp = dpu_enc->hw_pp[i];
+ phys->hw_pp = dpu_enc->hw_pp[num_pp_per_intf * i];
if (!phys->hw_pp) {
DPU_ERROR_ENC(dpu_enc,
"no pp block assigned at idx: %d\n", i);
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 07/14] drm/msm/dpu: Add pipe as trace argument
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (5 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 06/14] drm/msm/dpu: bind correct pingpong for quad pipe Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 08/14] drm/msm/dpu: handle pipes as array Jun Nie
` (6 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Add pipe as trace argument in trace_dpu_crtc_setup_mixer() to ease
converting pipe into pipe array later.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b0ee89ed7053f940d01bb55a8ea8de8ec5bc3462..095497d80e8a4168ed6b7b0a1b0f535bb9099d7d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -411,7 +411,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
state, to_dpu_plane_state(state), stage_idx,
- format->pixel_format,
+ format->pixel_format, pipe,
modifier);
DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d multirect_idx %d\n",
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 5307cbc2007c5044c5b897c53b44a8e356f1ad0f..cb24ad2a6d8d386bbc97b173854c410220725a0d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -651,9 +651,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
TP_PROTO(uint32_t crtc_id, uint32_t plane_id,
struct drm_plane_state *state, struct dpu_plane_state *pstate,
uint32_t stage_idx, uint32_t pixel_format,
- uint64_t modifier),
+ struct dpu_sw_pipe *pipe, uint64_t modifier),
TP_ARGS(crtc_id, plane_id, state, pstate, stage_idx,
- pixel_format, modifier),
+ pixel_format, pipe, modifier),
TP_STRUCT__entry(
__field( uint32_t, crtc_id )
__field( uint32_t, plane_id )
@@ -676,9 +676,9 @@ TRACE_EVENT(dpu_crtc_setup_mixer,
__entry->dst_rect = drm_plane_state_dest(state);
__entry->stage_idx = stage_idx;
__entry->stage = pstate->stage;
- __entry->sspp = pstate->pipe.sspp->idx;
- __entry->multirect_idx = pstate->pipe.multirect_index;
- __entry->multirect_mode = pstate->pipe.multirect_mode;
+ __entry->sspp = pipe->sspp->idx;
+ __entry->multirect_idx = pipe->multirect_index;
+ __entry->multirect_mode = pipe->multirect_mode;
__entry->pixel_format = pixel_format;
__entry->modifier = modifier;
),
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 08/14] drm/msm/dpu: handle pipes as array
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (6 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 07/14] drm/msm/dpu: Add pipe as trace argument Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 18:00 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer Jun Nie
` (5 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
There are 2 pipes in a drm plane at most currently, while 4 pipes are
required for quad-pipe case. Generalize the handling to pipe pair and
ease handling to another pipe pair later. Store pipes in array with
removing dedicated r_pipe.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 +++----
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 169 +++++++++++++++++-------------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +--
3 files changed, 113 insertions(+), 103 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 095497d80e8a4168ed6b7b0a1b0f535bb9099d7d..6338603bf8be9fcc4324b098d5d69d20235cdbae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
const struct msm_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
- uint32_t lm_idx;
+ uint32_t lm_idx, i;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
@@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
- set_bit(pstate->pipe.sspp->idx, fetch_active);
- _dpu_crtc_blend_setup_pipe(crtc, plane,
- mixer, cstate->num_mixers,
- pstate->stage,
- format, fb ? fb->modifier : 0,
- &pstate->pipe, 0, stage_cfg);
-
- if (pstate->r_pipe.sspp) {
- set_bit(pstate->r_pipe.sspp->idx, fetch_active);
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ set_bit(pstate->pipe[i].sspp->idx, fetch_active);
_dpu_crtc_blend_setup_pipe(crtc, plane,
mixer, cstate->num_mixers,
pstate->stage,
format, fb ? fb->modifier : 0,
- &pstate->r_pipe, 1, stage_cfg);
+ &pstate->pipe[i], i, stage_cfg);
}
/* blend config update */
@@ -1634,15 +1629,15 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
state->crtc_x, state->crtc_y, state->crtc_w,
state->crtc_h);
- seq_printf(s, "\tsspp[0]:%s\n",
- pstate->pipe.sspp->cap->name);
- seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
- pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
- if (pstate->r_pipe.sspp) {
- seq_printf(s, "\tsspp[1]:%s\n",
- pstate->r_pipe.sspp->cap->name);
- seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
- pstate->r_pipe.multirect_mode, pstate->r_pipe.multirect_index);
+
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ seq_printf(s, "\tsspp[%d]:%s\n",
+ i, pstate->pipe[i].sspp->cap->name);
+ seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n",
+ i, pstate->pipe[i].multirect_mode,
+ pstate->pipe[i].multirect_index);
}
seq_puts(s, "\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index aeb90c287245d6aaa18b9f280d1e628ee6ed74f5..4cb81a6a692be51d342d9a6f322b632e5fd07b2c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
struct msm_drm_private *priv = plane->dev->dev_private;
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24);
+ int i;
DPU_DEBUG_PLANE(pdpu, "\n");
@@ -632,12 +633,13 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
return;
/* update sspp */
- _dpu_plane_color_fill_pipe(pstate, &pstate->pipe, &pstate->pipe_cfg.dst_rect,
- fill_color, fmt);
-
- if (pstate->r_pipe.sspp)
- _dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe, &pstate->r_pipe_cfg.dst_rect,
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i],
+ &pstate->pipe_cfg[i].dst_rect,
fill_color, fmt);
+ }
}
static int dpu_plane_prepare_fb(struct drm_plane *plane,
@@ -827,8 +829,8 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
- struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
- struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+ struct dpu_sw_pipe_cfg *pipe_cfg;
+ struct dpu_sw_pipe_cfg *r_pipe_cfg;
struct drm_rect fb_rect = { 0 };
uint32_t max_linewidth;
@@ -853,6 +855,9 @@ 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);
@@ -949,10 +954,10 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
drm_atomic_get_new_plane_state(state, plane);
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
- struct dpu_sw_pipe *pipe = &pstate->pipe;
- struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
- struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
- struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+ struct dpu_sw_pipe *pipe = &pstate->pipe[0];
+ struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
+ struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
+ struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
int ret = 0;
ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
@@ -1011,10 +1016,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
- struct dpu_sw_pipe *pipe = &pstate->pipe;
- struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
- struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
- struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+ struct dpu_sw_pipe *pipe = &pstate->pipe[0];
+ struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
+ struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
+ struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
const struct drm_crtc_state *crtc_state = NULL;
uint32_t max_linewidth = dpu_kms->catalog->caps->max_linewidth;
@@ -1058,7 +1063,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
drm_atomic_get_old_plane_state(state, plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state);
struct drm_crtc_state *crtc_state;
- int ret;
+ int ret, i;
if (plane_state->crtc)
crtc_state = drm_atomic_get_new_crtc_state(state,
@@ -1073,8 +1078,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
* resources are freed by dpu_crtc_assign_plane_resources(),
* but clean them here.
*/
- pstate->pipe.sspp = NULL;
- pstate->r_pipe.sspp = NULL;
+ for (i = 0; i < PIPES_PER_STAGE; i++)
+ pstate->pipe[i].sspp = NULL;
return 0;
}
@@ -1111,19 +1116,21 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
struct dpu_sw_pipe_cfg *pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg;
const struct msm_format *fmt;
+ int i;
if (plane_state->crtc)
crtc_state = drm_atomic_get_new_crtc_state(state,
plane_state->crtc);
pstate = to_dpu_plane_state(plane_state);
- pipe = &pstate->pipe;
- r_pipe = &pstate->r_pipe;
- pipe_cfg = &pstate->pipe_cfg;
- r_pipe_cfg = &pstate->r_pipe_cfg;
- pipe->sspp = NULL;
- r_pipe->sspp = NULL;
+ pipe = &pstate->pipe[0];
+ r_pipe = &pstate->pipe[1];
+ pipe_cfg = &pstate->pipe_cfg[0];
+ r_pipe_cfg = &pstate->pipe_cfg[1];
+
+ for (i = 0; i < PIPES_PER_STAGE; i++)
+ pstate->pipe[i].sspp = NULL;
if (!plane_state->fb)
return -EINVAL;
@@ -1212,6 +1219,7 @@ void dpu_plane_flush(struct drm_plane *plane)
{
struct dpu_plane *pdpu;
struct dpu_plane_state *pstate;
+ int i;
if (!plane || !plane->state) {
DPU_ERROR("invalid plane\n");
@@ -1232,8 +1240,8 @@ void dpu_plane_flush(struct drm_plane *plane)
/* force 100% alpha */
_dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
else {
- dpu_plane_flush_csc(pdpu, &pstate->pipe);
- dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
+ for (i = 0; i < PIPES_PER_STAGE; i++)
+ dpu_plane_flush_csc(pdpu, &pstate->pipe[i]);
}
/* flag h/w flush complete */
@@ -1334,15 +1342,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- struct dpu_sw_pipe *pipe = &pstate->pipe;
- struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
struct drm_crtc *crtc = state->crtc;
struct drm_framebuffer *fb = state->fb;
bool is_rt_pipe;
const struct msm_format *fmt =
msm_framebuffer_format(fb);
- struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
- struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+ int i;
pstate->pending = true;
@@ -1357,12 +1362,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
crtc->base.id, DRM_RECT_ARG(&state->dst),
&fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt));
- dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
- drm_mode_vrefresh(&crtc->mode),
- &pstate->layout);
-
- if (r_pipe->sspp) {
- dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
+ /* move the assignment here, to ease handling to another pairs later */
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i],
+ &pstate->pipe_cfg[i], fmt,
drm_mode_vrefresh(&crtc->mode),
&pstate->layout);
}
@@ -1370,15 +1375,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
if (pstate->needs_qos_remap)
pstate->needs_qos_remap = false;
- pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt,
- &crtc->mode, pipe_cfg);
-
- pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
-
- if (r_pipe->sspp) {
- pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, r_pipe_cfg);
+ pstate->plane_fetch_bw = 0;
+ pstate->plane_clk = 0;
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt,
+ &crtc->mode, &pstate->pipe_cfg[i]);
- pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
+ pstate->plane_clk = max(pstate->plane_clk,
+ _dpu_plane_calc_clk(&crtc->mode,
+ &pstate->pipe_cfg[i]));
}
}
@@ -1386,17 +1393,31 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
{
struct drm_plane_state *state = plane->state;
struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
+ struct dpu_sw_pipe *pipe;
+ int i;
- trace_dpu_plane_disable(DRMID(plane), false,
- pstate->pipe.multirect_mode);
+ for (i = 0; i < PIPES_PER_STAGE; i += 1) {
+ pipe = &pstate->pipe[i];
+ if (!pipe->sspp)
+ continue;
- if (r_pipe->sspp) {
- r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ trace_dpu_plane_disable(DRMID(plane), false,
+ pstate->pipe[i].multirect_mode);
+
+ if (!pipe->sspp)
+ continue;
- if (r_pipe->sspp->ops.setup_multirect)
- r_pipe->sspp->ops.setup_multirect(r_pipe);
+ if (i % PIPES_PER_STAGE == 0)
+ continue;
+
+ /*
+ * clear multirect for the right pipe so that the SSPP
+ * can be further reused in the solo mode
+ */
+ pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ if (pipe->sspp->ops.setup_multirect)
+ pipe->sspp->ops.setup_multirect(pipe);
}
pstate->pending = true;
@@ -1491,31 +1512,26 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
const struct drm_plane_state *state)
{
const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
- const struct dpu_sw_pipe *pipe = &pstate->pipe;
- const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
- const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
- const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
+ const struct dpu_sw_pipe *pipe;
+ const struct dpu_sw_pipe_cfg *pipe_cfg;
+ int i;
drm_printf(p, "\tstage=%d\n", pstate->stage);
- if (pipe->sspp) {
- drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
- drm_printf(p, "\tmultirect_mode[0]=%s\n",
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ pipe = &pstate->pipe[i];
+ if (!pipe->sspp)
+ continue;
+ pipe_cfg = &pstate->pipe_cfg[i];
+ drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name);
+ drm_printf(p, "\tmultirect_mode[%d]=%s\n", i,
dpu_get_multirect_mode(pipe->multirect_mode));
- drm_printf(p, "\tmultirect_index[0]=%s\n",
+ drm_printf(p, "\tmultirect_index[%d]=%s\n", i,
dpu_get_multirect_index(pipe->multirect_index));
- drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->src_rect));
- drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->dst_rect));
- }
-
- if (r_pipe->sspp) {
- drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
- drm_printf(p, "\tmultirect_mode[1]=%s\n",
- dpu_get_multirect_mode(r_pipe->multirect_mode));
- drm_printf(p, "\tmultirect_index[1]=%s\n",
- dpu_get_multirect_index(r_pipe->multirect_index));
- drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->src_rect));
- drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
+ drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i,
+ DRM_RECT_ARG(&pipe_cfg->src_rect));
+ drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i,
+ DRM_RECT_ARG(&pipe_cfg->dst_rect));
}
}
@@ -1553,14 +1569,17 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable)
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+ int i;
if (!pdpu->is_rt_pipe)
return;
pm_runtime_get_sync(&dpu_kms->pdev->dev);
- _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable);
- if (pstate->r_pipe.sspp)
- _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable);
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ if (!pstate->pipe[i].sspp)
+ continue;
+ _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable);
+ }
pm_runtime_put_sync(&dpu_kms->pdev->dev);
}
#endif
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index acd5725175cdde4fcf7a9f71bb446251c5a14d22..052fd046e8463855b16b30389c2efc67c0c15281 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -18,10 +18,8 @@
* struct dpu_plane_state: Define dpu extension of drm plane state object
* @base: base drm plane state object
* @aspace: pointer to address space for input/output buffers
- * @pipe: software pipe description
- * @r_pipe: software pipe description of the second pipe
- * @pipe_cfg: software pipe configuration
- * @r_pipe_cfg: software pipe configuration for the second pipe
+ * @pipe: software pipe description array
+ * @pipe_cfg: software pipe configuration array
* @stage: assigned by crtc blender
* @needs_qos_remap: qos remap settings need to be updated
* @multirect_index: index of the rectangle of SSPP
@@ -35,10 +33,8 @@
struct dpu_plane_state {
struct drm_plane_state base;
struct msm_gem_address_space *aspace;
- struct dpu_sw_pipe pipe;
- struct dpu_sw_pipe r_pipe;
- struct dpu_sw_pipe_cfg pipe_cfg;
- struct dpu_sw_pipe_cfg r_pipe_cfg;
+ struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
+ struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
enum dpu_stage stage;
bool needs_qos_remap;
bool pending;
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (7 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 08/14] drm/msm/dpu: handle pipes as array Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 20:42 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition Jun Nie
` (4 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
The stage contains configuration for a mixer pair. Currently the plane
supports just one stage and 2 pipes. Quad-pipe support will require
handling 2 stages and 4 pipes at the same time. In preparation for that
add a separate define, PIPES_PER_PLANE, to denote number of pipes that
can be used by the plane.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 +++++++-------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 ++--
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 6338603bf8be9fcc4324b098d5d69d20235cdbae..d710b4eec7ad946a4cf74d6ac5f4db90e8dcf1fd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -463,7 +463,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
- for (i = 0; i < PIPES_PER_STAGE; i++) {
+ for (i = 0; i < PIPES_PER_PLANE; i++) {
if (!pstate->pipe[i].sspp)
continue;
set_bit(pstate->pipe[i].sspp->idx, fetch_active);
@@ -1272,7 +1272,7 @@ static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct drm_crtc_state
return ret;
}
-#define MAX_CHANNELS_PER_CRTC 2
+#define MAX_CHANNELS_PER_CRTC PIPES_PER_PLANE
#define MAX_HDISPLAY_SPLIT 1080
static struct msm_display_topology dpu_crtc_get_topology(
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 8d820cd1b5545d247515763039b341184e814e32..b0ed41108a32158c0bc3be2e25fc229b218fd6c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -34,6 +34,7 @@
#define DPU_MAX_PLANES 4
#endif
+#define PIPES_PER_PLANE 2
#define PIPES_PER_STAGE 2
#ifndef DPU_MAX_DE_CURVES
#define DPU_MAX_DE_CURVES 3
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 4cb81a6a692be51d342d9a6f322b632e5fd07b2c..ea7e3fdf52f726737941ad33218a843dca17280b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1078,7 +1078,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
* resources are freed by dpu_crtc_assign_plane_resources(),
* but clean them here.
*/
- for (i = 0; i < PIPES_PER_STAGE; i++)
+ for (i = 0; i < PIPES_PER_PLANE; i++)
pstate->pipe[i].sspp = NULL;
return 0;
@@ -1129,7 +1129,7 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
pipe_cfg = &pstate->pipe_cfg[0];
r_pipe_cfg = &pstate->pipe_cfg[1];
- for (i = 0; i < PIPES_PER_STAGE; i++)
+ for (i = 0; i < PIPES_PER_PLANE; i++)
pstate->pipe[i].sspp = NULL;
if (!plane_state->fb)
@@ -1240,7 +1240,7 @@ void dpu_plane_flush(struct drm_plane *plane)
/* force 100% alpha */
_dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
else {
- for (i = 0; i < PIPES_PER_STAGE; i++)
+ for (i = 0; i < PIPES_PER_PLANE; i++)
dpu_plane_flush_csc(pdpu, &pstate->pipe[i]);
}
@@ -1363,7 +1363,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
&fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt));
/* move the assignment here, to ease handling to another pairs later */
- for (i = 0; i < PIPES_PER_STAGE; i++) {
+ for (i = 0; i < PIPES_PER_PLANE; i++) {
if (!pstate->pipe[i].sspp)
continue;
dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i],
@@ -1377,7 +1377,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
pstate->plane_fetch_bw = 0;
pstate->plane_clk = 0;
- for (i = 0; i < PIPES_PER_STAGE; i++) {
+ for (i = 0; i < PIPES_PER_PLANE; i++) {
if (!pstate->pipe[i].sspp)
continue;
pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt,
@@ -1396,7 +1396,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
struct dpu_sw_pipe *pipe;
int i;
- for (i = 0; i < PIPES_PER_STAGE; i += 1) {
+ for (i = 0; i < PIPES_PER_PLANE; i += 1) {
pipe = &pstate->pipe[i];
if (!pipe->sspp)
continue;
@@ -1518,7 +1518,7 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
drm_printf(p, "\tstage=%d\n", pstate->stage);
- for (i = 0; i < PIPES_PER_STAGE; i++) {
+ for (i = 0; i < PIPES_PER_PLANE; i++) {
pipe = &pstate->pipe[i];
if (!pipe->sspp)
continue;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 052fd046e8463855b16b30389c2efc67c0c15281..18ff5ec2603ed63ce45f530ced3407d3b70c737b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -33,8 +33,8 @@
struct dpu_plane_state {
struct drm_plane_state base;
struct msm_gem_address_space *aspace;
- struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
- struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
+ struct dpu_sw_pipe pipe[PIPES_PER_PLANE];
+ struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_PLANE];
enum dpu_stage stage;
bool needs_qos_remap;
bool pending;
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (8 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 13:10 ` Dmitry Baryshkov
2025-05-08 18:03 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config Jun Nie
` (3 subsequent siblings)
13 siblings, 2 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Currently MAX_CHANNELS_PER_ENC is defined as 2, because 2 channels are
supported at most in one encoder. The case of 4 channels per encoder is
to be added. To avoid breaking current WB usage case, use dedicated WB
definition before 4 WB usage case is supported in future.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3e4ff345bd6d9c0e2a08274874fac40690409cf0..a125a477404fdafee6495f6d3778160f14a01a96 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -56,6 +56,7 @@
(MAX_H_TILES_PER_DISPLAY * NUM_PHYS_ENCODER_TYPES)
#define MAX_CHANNELS_PER_ENC 2
+#define MAX_CWB_PER_ENC 2
#define IDLE_SHORT_TIMEOUT 1
@@ -182,7 +183,7 @@ struct dpu_encoder_virt {
struct dpu_encoder_phys *cur_master;
struct dpu_encoder_phys *cur_slave;
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
- struct dpu_hw_cwb *hw_cwb[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_cwb *hw_cwb[MAX_CWB_PER_ENC];
struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
unsigned int dsc_mask;
@@ -2372,7 +2373,7 @@ void dpu_encoder_helper_phys_setup_cwb(struct dpu_encoder_phys *phys_enc,
*/
cwb_cfg.input = INPUT_MODE_LM_OUT;
- for (int i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ for (int i = 0; i < MAX_CWB_PER_ENC; i++) {
hw_cwb = dpu_enc->hw_cwb[i];
if (!hw_cwb)
continue;
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (9 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 20:53 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case Jun Nie
` (2 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
Currently, only 2 pipes are used at most for a plane. A stage structure
describes the configuration for a mixer pair. So only one stage is needed
for current usage cases. The quad-pipe case will be added in future and 2
stages are used in the case. So extend the stage to an array with array
size STAGES_PER_PLANE and blend pipes per mixer pair with configuration in
the stage structure.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 45 +++++++++++++++++++----------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +-
2 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d710b4eec7ad946a4cf74d6ac5f4db90e8dcf1fd..f35cb1f7a7d2c2c63b4228bc47b85bb57cddbe6b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -392,7 +392,7 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
struct drm_plane *plane,
struct dpu_crtc_mixer *mixer,
- u32 num_mixers,
+ u32 lms_in_pair,
enum dpu_stage stage,
const struct msm_format *format,
uint64_t modifier,
@@ -426,7 +426,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
stage_cfg->multirect_index[stage][stage_idx] = pipe->multirect_index;
/* blend config update */
- for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+ for (lm_idx = 0; lm_idx < lms_in_pair; lm_idx++)
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, sspp_idx);
}
@@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
const struct msm_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
- uint32_t lm_idx, i;
+ uint32_t lm_idx, stage, i, pipe_idx, head_pipe_in_stage, lms_in_pair;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
@@ -463,15 +463,24 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
- for (i = 0; i < PIPES_PER_PLANE; i++) {
- if (!pstate->pipe[i].sspp)
- continue;
- set_bit(pstate->pipe[i].sspp->idx, fetch_active);
- _dpu_crtc_blend_setup_pipe(crtc, plane,
- mixer, cstate->num_mixers,
- pstate->stage,
- format, fb ? fb->modifier : 0,
- &pstate->pipe[i], i, stage_cfg);
+ /* loop pipe per mixer pair with config in stage structure */
+ for (stage = 0; stage < STAGES_PER_PLANE; stage++) {
+ head_pipe_in_stage = stage * PIPES_PER_STAGE;
+ for (i = 0; i < PIPES_PER_STAGE; i++) {
+ pipe_idx = i + head_pipe_in_stage;
+ if (!pstate->pipe[pipe_idx].sspp)
+ continue;
+ lms_in_pair = min(cstate->num_mixers - (stage * PIPES_PER_STAGE),
+ PIPES_PER_STAGE);
+ set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active);
+ _dpu_crtc_blend_setup_pipe(crtc, plane,
+ &mixer[head_pipe_in_stage],
+ lms_in_pair,
+ pstate->stage,
+ format, fb ? fb->modifier : 0,
+ &pstate->pipe[pipe_idx], i,
+ &stage_cfg[stage]);
+ }
}
/* blend config update */
@@ -503,7 +512,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
struct dpu_crtc_mixer *mixer = cstate->mixers;
struct dpu_hw_ctl *ctl;
struct dpu_hw_mixer *lm;
- struct dpu_hw_stage_cfg stage_cfg;
+ struct dpu_hw_stage_cfg stage_cfg[STAGES_PER_PLANE];
int i;
DRM_DEBUG_ATOMIC("%s\n", dpu_crtc->name);
@@ -516,9 +525,9 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
}
/* initialize stage cfg */
- memset(&stage_cfg, 0, sizeof(struct dpu_hw_stage_cfg));
+ memset(&stage_cfg, 0, sizeof(stage_cfg));
- _dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer, &stage_cfg);
+ _dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer, stage_cfg);
for (i = 0; i < cstate->num_mixers; i++) {
ctl = mixer[i].lm_ctl;
@@ -535,8 +544,12 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
mixer[i].mixer_op_mode,
ctl->idx - CTL_0);
+ /*
+ * call dpu_hw_ctl_setup_blendstage() to blend layers per stage cfg.
+ * stage data is shared between PIPES_PER_STAGE pipes.
+ */
ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx,
- &stage_cfg);
+ &stage_cfg[i / PIPES_PER_STAGE]);
}
}
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 b0ed41108a32158c0bc3be2e25fc229b218fd6c5..7c74221380b2c05225c9f82ed6d33765042aec78 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -34,8 +34,9 @@
#define DPU_MAX_PLANES 4
#endif
-#define PIPES_PER_PLANE 2
+#define STAGES_PER_PLANE 1
#define PIPES_PER_STAGE 2
+#define PIPES_PER_PLANE (PIPES_PER_STAGE * STAGES_PER_PLANE)
#ifndef DPU_MAX_DE_CURVES
#define DPU_MAX_DE_CURVES 3
#endif
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (10 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 18:51 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 13/14] drm/msm/dpu: support plane splitting in " Jun Nie
2025-05-06 15:47 ` [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case Jun Nie
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
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@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 68 +++++++++++++++++++------------
1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ea7e3fdf52f726737941ad33218a843dca17280b..de3f52d743e1d1f11ae8721a316b9872d4139069 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1112,11 +1112,10 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
struct dpu_rm_sspp_requirements reqs;
struct dpu_plane_state *pstate;
struct dpu_sw_pipe *pipe;
- struct dpu_sw_pipe *r_pipe;
struct dpu_sw_pipe_cfg *pipe_cfg;
- struct dpu_sw_pipe_cfg *r_pipe_cfg;
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
const struct msm_format *fmt;
- int i;
+ int i, num_lm, stage_id, num_stages;
if (plane_state->crtc)
crtc_state = drm_atomic_get_new_crtc_state(state,
@@ -1124,11 +1123,6 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
pstate = to_dpu_plane_state(plane_state);
- pipe = &pstate->pipe[0];
- r_pipe = &pstate->pipe[1];
- pipe_cfg = &pstate->pipe_cfg[0];
- r_pipe_cfg = &pstate->pipe_cfg[1];
-
for (i = 0; i < PIPES_PER_PLANE; i++)
pstate->pipe[i].sspp = NULL;
@@ -1142,24 +1136,46 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
- pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
- if (!pipe->sspp)
- return -ENODEV;
-
- if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
- pipe->sspp,
- msm_framebuffer_format(plane_state->fb),
- dpu_kms->catalog->caps->max_linewidth)) {
- /* multirect is not possible, use two SSPP blocks */
- r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
- if (!r_pipe->sspp)
- return -ENODEV;
-
- pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
- r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ num_lm = dpu_crtc_get_num_lm(crtc_state);
+ num_stages = (num_lm + 1) / 2;
+ for (stage_id = 0; stage_id < num_stages; stage_id++) {
+ for (i = stage_id * PIPES_PER_STAGE; i < (stage_id + 1) * PIPES_PER_STAGE; i++) {
+ struct dpu_sw_pipe *r_pipe;
+ struct dpu_sw_pipe_cfg *r_pipe_cfg;
+
+ pipe = &pstate->pipe[i];
+ pipe_cfg = &pstate->pipe_cfg[i];
+
+ if (drm_rect_width(&pipe_cfg->src_rect) == 0)
+ break;
+
+ pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
+ if (!pipe->sspp)
+ return -ENODEV;
+
+ r_pipe = &pstate->pipe[i + 1];
+ r_pipe_cfg = &pstate->pipe_cfg[i + 1];
+
+ /*
+ * If current pipe is the first pipe in pipe pair, check
+ * multi-rect opportunity for the 2nd pipe in the pair.
+ * SSPP multi-rect mode cross mixer pairs is not supported.
+ */
+ if ((i % PIPES_PER_STAGE == 0) &&
+ drm_rect_width(&r_pipe_cfg->src_rect) != 0 &&
+ dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
+ pipe->sspp,
+ msm_framebuffer_format(plane_state->fb),
+ dpu_kms->catalog->caps->max_linewidth)) {
+ i++;
+ } else {
+ /* multirect is not possible, use two SSPP blocks */
+ pipe->multirect_index = DPU_SSPP_RECT_SOLO;
+ pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+ DPU_DEBUG_PLANE(pdpu, "allocating sspp_%d for pipe %d.\n",
+ pipe->sspp->idx - SSPP_NONE, i);
+ }
+ }
}
return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 13/14] drm/msm/dpu: support plane splitting in quad-pipe case
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (11 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 21:24 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case Jun Nie
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
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@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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 | 134 +++++++++++++++++++++---------
3 files changed, 107 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 f35cb1f7a7d2c2c63b4228bc47b85bb57cddbe6b..a56d68de219910a827830293e8ff24f4cdee74e4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1560,6 +1560,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 de3f52d743e1d1f11ae8721a316b9872d4139069..cf2b4d5cb0ccc144c2cf8fd227c862b0b6e7725f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -831,8 +831,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;
@@ -855,13 +859,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;
@@ -886,35 +887,91 @@ 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;
+ /* iterate mixer configs for this plane, to separate left/right with the id */
+ for (stage_id = 0; stage_id < num_stages; stage_id++) {
+ struct drm_rect mixer_rect = {stage_id * mode->hdisplay / num_stages, 0,
+ (stage_id + 1) * mode->hdisplay / num_stages,
+ mode->vdisplay};
+ int cfg_idx = stage_id * PIPES_PER_STAGE;
+
+ pipe_cfg = &pstate->pipe_cfg[cfg_idx];
+ r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1];
+
+ drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
+ pipe_cfg->dst_rect = new_plane_state->dst;
+
+ DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT
+ " vs clip window " DRM_RECT_FMT "\n",
+ DRM_RECT_ARG(&pipe_cfg->src_rect),
+ DRM_RECT_ARG(&mixer_rect));
+
+ /*
+ * If this plane does not fall into mixer rect, check next
+ * mixer rect.
+ */
+ 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));
+
+ continue;
}
- *r_pipe_cfg = *pipe_cfg;
- pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
- pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
- r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
- r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
- } else {
- memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg));
- }
+ pipe_cfg->dst_rect.x1 -= mixer_rect.x1;
+ pipe_cfg->dst_rect.x2 -= mixer_rect.x1;
+
+ DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n",
+ DRM_RECT_ARG(&pipe_cfg->src_rect), DRM_RECT_ARG(&pipe_cfg->dst_rect));
- drm_rect_rotate_inv(&pipe_cfg->src_rect,
- new_plane_state->fb->width, new_plane_state->fb->height,
- new_plane_state->rotation);
- if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
- drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
- new_plane_state->fb->width, new_plane_state->fb->height,
+ /* Split wide rect into 2 rect */
+ if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
+ _dpu_plane_calc_clk(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;
+ }
+
+ memcpy(r_pipe_cfg, pipe_cfg, sizeof(struct dpu_sw_pipe_cfg));
+ pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
+ pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
+ r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
+ r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
+ DPU_DEBUG_PLANE(pdpu, "Split wide plane into:"
+ 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));
+ }
+
+ drm_rect_rotate_inv(&pipe_cfg->src_rect,
+ new_plane_state->fb->width,
+ new_plane_state->fb->height,
new_plane_state->rotation);
+ if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
+ drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
+ new_plane_state->fb->width,
+ new_plane_state->fb->height,
+ new_plane_state->rotation);
+ }
+
pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
return 0;
@@ -954,20 +1011,17 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
drm_atomic_get_new_plane_state(state, plane);
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
- struct dpu_sw_pipe *pipe = &pstate->pipe[0];
- struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
- struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
- struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
- int ret = 0;
-
- ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
- &crtc_state->adjusted_mode,
- new_plane_state);
- if (ret)
- return ret;
+ struct dpu_sw_pipe *pipe;
+ struct dpu_sw_pipe_cfg *pipe_cfg;
+ int ret = 0, i;
- if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
- ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
+ for (i = 0; i < PIPES_PER_PLANE; i++) {
+ pipe = &pstate->pipe[i];
+ pipe_cfg = &pstate->pipe_cfg[i];
+ if (!pipe->sspp)
+ continue;
+ DPU_DEBUG_PLANE(pdpu, "pipe %d is in use, validate it\n", i);
+ ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
&crtc_state->adjusted_mode,
new_plane_state);
if (ret)
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
` (12 preceding siblings ...)
2025-05-06 15:47 ` [PATCH v9 13/14] drm/msm/dpu: support plane splitting in " Jun Nie
@ 2025-05-06 15:47 ` Jun Nie
2025-05-08 21:30 ` Jessica Zhang
13 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-06 15:47 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Dmitry Baryshkov,
Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Jun Nie
To support high-resolution cases that exceed the width limitation of
a pair of SSPPs, or scenarios that surpass the maximum MDP clock rate,
additional pipes are necessary to enable parallel data processing
within the SSPP width constraints and MDP clock rate.
Request 4 mixers and 4 DSCs for high-resolution cases where both DSC
and dual interfaces are enabled. More use cases can be incorporated
later if quad-pipe capabilities are required.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 27 +++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 28 ++++++++----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
6 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index a56d68de219910a827830293e8ff24f4cdee74e4..f22ca13b35e778fed3e6e02c581ef689a5544cfb 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];
int rc = 0;
int i;
@@ -1296,6 +1296,7 @@ static struct msm_display_topology dpu_crtc_get_topology(
struct drm_display_mode *mode = &crtc_state->adjusted_mode;
struct msm_display_topology topology = {0};
struct drm_encoder *drm_enc;
+ u32 num_rt_intf;
drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc_state->encoder_mask)
dpu_encoder_update_topology(drm_enc, &topology, crtc_state->state,
@@ -1309,11 +1310,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
* Dual display
* 2 LM, 2 INTF ( Split display using 2 interfaces)
*
+ * If DSC is enabled, try to use 4:4:2 topology if there is enough
+ * resource. Otherwise, use 2:2:2 topology.
+ *
* Single display
* 1 LM, 1 INTF
* 2 LM, 1 INTF (stream merge to support high resolution interfaces)
*
- * If DSC is enabled, use 2 LMs for 2:2:1 topology
+ * If DSC is enabled, use 2:2:1 topology
*
* Add dspps to the reservation requirements if ctm is requested
*
@@ -1325,14 +1329,23 @@ static struct msm_display_topology dpu_crtc_get_topology(
* (mode->hdisplay > MAX_HDISPLAY_SPLIT) check.
*/
- if (topology.num_intf == 2 && !topology.cwb_enabled)
- topology.num_lm = 2;
- else if (topology.num_dsc == 2)
+ num_rt_intf = topology.num_intf;
+ if (topology.cwb_enabled)
+ num_rt_intf--;
+
+ if (topology.num_dsc) {
+ if (dpu_kms->catalog->dsc_count >= num_rt_intf * 2)
+ topology.num_dsc = num_rt_intf * 2;
+ else
+ topology.num_dsc = num_rt_intf;
+ topology.num_lm = topology.num_dsc;
+ } else if (num_rt_intf == 2) {
topology.num_lm = 2;
- else if (dpu_kms->catalog->caps->has_3d_merge)
+ } else if (dpu_kms->catalog->caps->has_3d_merge) {
topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
- else
+ } else {
topology.num_lm = 1;
+ }
if (crtc_state->ctm)
topology.num_dspp = topology.num_lm;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 6eaba5696e8e6bd1246a9895c4c8714ca6589b10..455073c7025b0bcb970d8817f197d9bcacc6dca5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -210,7 +210,7 @@ struct dpu_crtc_state {
bool bw_control;
bool bw_split_vote;
- struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
+ struct drm_rect lm_bounds[CRTC_QUAD_MIXERS];
uint64_t input_fence_timeout_ns;
@@ -218,10 +218,10 @@ struct dpu_crtc_state {
/* HW Resources reserved for the crtc */
u32 num_mixers;
- struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
+ struct dpu_crtc_mixer mixers[CRTC_QUAD_MIXERS];
u32 num_ctls;
- struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
+ struct dpu_hw_ctl *hw_ctls[CRTC_QUAD_MIXERS];
enum dpu_crtc_crc_source crc_source;
int crc_frame_skip_count;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index a125a477404fdafee6495f6d3778160f14a01a96..bbb0873a891879722f2b02767fc48b7406de70f8 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
#define MAX_CWB_PER_ENC 2
#define IDLE_SHORT_TIMEOUT 1
@@ -675,22 +675,12 @@ void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
dsc = dpu_encoder_get_dsc_config(drm_enc);
- /* We only support 2 DSC mode (with 2 LM and 1 INTF) */
- if (dsc) {
- /*
- * Use 2 DSC encoders, 2 layer mixers and 1 or 2 interfaces
- * when Display Stream Compression (DSC) is enabled,
- * and when enough DSC blocks are available.
- * This is power-optimal and can drive up to (including) 4k
- * screens.
- */
- WARN(topology->num_intf > 2,
- "DSC topology cannot support more than 2 interfaces\n");
- if (topology->num_intf >= 2 || dpu_kms->catalog->dsc_count >= 2)
- topology->num_dsc = 2;
- else
- topology->num_dsc = 1;
- }
+ /*
+ * Set DSC number as 1 to mark the enabled status, will be adjusted
+ * in dpu_crtc_get_topology()
+ */
+ if (dsc)
+ topology->num_dsc = 1;
connector = drm_atomic_get_new_connector_for_encoder(state, drm_enc);
if (!connector)
@@ -2176,8 +2166,8 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
struct dpu_hw_mixer_cfg mixer;
int i, num_lm;
struct dpu_global_state *global_state;
- struct dpu_hw_blk *hw_lm[2];
- struct dpu_hw_mixer *hw_mixer[2];
+ struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_mixer *hw_mixer[MAX_CHANNELS_PER_ENC];
struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
memset(&mixer, 0, sizeof(mixer));
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 61b22d9494546885db609efa156222792af73d2a..09395d7910ac87c035b65cf476350bf6c9619612 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -302,7 +302,7 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
/* Use merge_3d unless DSC MERGE topology is used */
if (phys_enc->split_role == ENC_ROLE_SOLO &&
- dpu_cstate->num_mixers == CRTC_DUAL_MIXERS &&
+ (dpu_cstate->num_mixers != 1) &&
!dpu_encoder_use_dsc_merge(phys_enc->parent))
return BLEND_3D_H_ROW_INT;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 4cea19e1a20380c56ae014f2d33a6884a72e0ca0..77a7a5375d545483edb316e8428df12212191362 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -24,7 +24,7 @@
#define DPU_MAX_IMG_WIDTH 0x3fff
#define DPU_MAX_IMG_HEIGHT 0x3fff
-#define CRTC_DUAL_MIXERS 2
+#define CRTC_QUAD_MIXERS 4
#define MAX_XIN_COUNT 16
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 7c74221380b2c05225c9f82ed6d33765042aec78..94f117e36af83602e5254ebcf5f28de36fd79886 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
#define PIPES_PER_STAGE 2
#define PIPES_PER_PLANE (PIPES_PER_STAGE * STAGES_PER_PLANE)
#ifndef DPU_MAX_DE_CURVES
--
2.34.1
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane
2025-05-06 15:47 ` [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane Jun Nie
@ 2025-05-08 10:47 ` Dmitry Baryshkov
2025-05-09 3:08 ` Jun Nie
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Baryshkov @ 2025-05-08 10:47 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, linux-arm-msm, dri-devel,
freedreno, linux-kernel
On Tue, May 06, 2025 at 11:47:31PM +0800, Jun Nie wrote:
> Some display controller support flexible CRTC and DMA, such as the display
> controllers in snapdragon SoCs. CRTC can be implemented with several mixers
> in parallel, and plane fetching can be implemented with several DMA under
> umberala of a virtual drm plane.
>
> The mixer number is decided per panel resolution and clock rate constrain
> first, which happens in CRTC side. Then plane is split per mixer number
> and configure DMA accordingly.
Here you are describing a behaviour of one particular driver as a reason
to change the framework.
>
> To support such forthcoming usage case, CRTC checking shall happen before
> checking plane. Add the checking in the drm_atomic_helper_check_modeset().
So, now drivers will get two calls to atomic_check(), one coming in
circumstances which were not expected by the drivers before. Are you
sure that this won't break anything?
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 5302ab3248985d3e0a47e40fd3deb7ad0d9f775b..5bca4c9683838c38574c8cb7c0bc9d57960314fe 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -816,6 +816,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> return ret;
> }
>
> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> + const struct drm_crtc_helper_funcs *funcs;
> +
> + funcs = crtc->helper_private;
> +
> + if (!funcs || !funcs->atomic_check)
> + continue;
> +
> + ret = funcs->atomic_check(crtc, state);
> + if (ret) {
> + drm_dbg_atomic(crtc->dev,
> + "[CRTC:%d:%s] atomic driver check failed\n",
> + crtc->base.id, crtc->name);
> + return ret;
> + }
> + }
> +
> +
> +
Too many empty lines. But the main quesiton is: why are you calling it
before mode_valid()? According to your description a better place would
be in drm_atomic_helper_check_planes().
> ret = mode_valid(state);
> if (ret)
> return ret;
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition
2025-05-06 15:47 ` [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition Jun Nie
@ 2025-05-08 13:10 ` Dmitry Baryshkov
2025-05-08 18:03 ` Jessica Zhang
1 sibling, 0 replies; 30+ messages in thread
From: Dmitry Baryshkov @ 2025-05-08 13:10 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, linux-arm-msm, dri-devel,
freedreno, linux-kernel
On Tue, May 06, 2025 at 11:47:40PM +0800, Jun Nie wrote:
> Currently MAX_CHANNELS_PER_ENC is defined as 2, because 2 channels are
> supported at most in one encoder. The case of 4 channels per encoder is
> to be added. To avoid breaking current WB usage case, use dedicated WB
> definition before 4 WB usage case is supported in future.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 08/14] drm/msm/dpu: handle pipes as array
2025-05-06 15:47 ` [PATCH v9 08/14] drm/msm/dpu: handle pipes as array Jun Nie
@ 2025-05-08 18:00 ` Jessica Zhang
0 siblings, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 18:00 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> There are 2 pipes in a drm plane at most currently, while 4 pipes are
> required for quad-pipe case. Generalize the handling to pipe pair and
> ease handling to another pipe pair later. Store pipes in array with
> removing dedicated r_pipe.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Hi Jun,
I think my R-b from v8 was dropped. Since I don't see any changes from
v8 here:
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Thanks,
Jessica Zhang
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 +++----
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 169 +++++++++++++++++-------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +--
> 3 files changed, 113 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 095497d80e8a4168ed6b7b0a1b0f535bb9099d7d..6338603bf8be9fcc4324b098d5d69d20235cdbae 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> const struct msm_format *format;
> struct dpu_hw_ctl *ctl = mixer->lm_ctl;
>
> - uint32_t lm_idx;
> + uint32_t lm_idx, i;
> bool bg_alpha_enable = false;
> DECLARE_BITMAP(fetch_active, SSPP_MAX);
>
> @@ -463,20 +463,15 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> bg_alpha_enable = true;
>
> - set_bit(pstate->pipe.sspp->idx, fetch_active);
> - _dpu_crtc_blend_setup_pipe(crtc, plane,
> - mixer, cstate->num_mixers,
> - pstate->stage,
> - format, fb ? fb->modifier : 0,
> - &pstate->pipe, 0, stage_cfg);
> -
> - if (pstate->r_pipe.sspp) {
> - set_bit(pstate->r_pipe.sspp->idx, fetch_active);
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + set_bit(pstate->pipe[i].sspp->idx, fetch_active);
> _dpu_crtc_blend_setup_pipe(crtc, plane,
> mixer, cstate->num_mixers,
> pstate->stage,
> format, fb ? fb->modifier : 0,
> - &pstate->r_pipe, 1, stage_cfg);
> + &pstate->pipe[i], i, stage_cfg);
> }
>
> /* blend config update */
> @@ -1634,15 +1629,15 @@ static int _dpu_debugfs_status_show(struct seq_file *s, void *data)
> seq_printf(s, "\tdst x:%4d dst_y:%4d dst_w:%4d dst_h:%4d\n",
> state->crtc_x, state->crtc_y, state->crtc_w,
> state->crtc_h);
> - seq_printf(s, "\tsspp[0]:%s\n",
> - pstate->pipe.sspp->cap->name);
> - seq_printf(s, "\tmultirect[0]: mode: %d index: %d\n",
> - pstate->pipe.multirect_mode, pstate->pipe.multirect_index);
> - if (pstate->r_pipe.sspp) {
> - seq_printf(s, "\tsspp[1]:%s\n",
> - pstate->r_pipe.sspp->cap->name);
> - seq_printf(s, "\tmultirect[1]: mode: %d index: %d\n",
> - pstate->r_pipe.multirect_mode, pstate->r_pipe.multirect_index);
> +
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + seq_printf(s, "\tsspp[%d]:%s\n",
> + i, pstate->pipe[i].sspp->cap->name);
> + seq_printf(s, "\tmultirect[%d]: mode: %d index: %d\n",
> + i, pstate->pipe[i].multirect_mode,
> + pstate->pipe[i].multirect_index);
> }
>
> seq_puts(s, "\n");
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index aeb90c287245d6aaa18b9f280d1e628ee6ed74f5..4cb81a6a692be51d342d9a6f322b632e5fd07b2c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -619,6 +619,7 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
> struct msm_drm_private *priv = plane->dev->dev_private;
> struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
> u32 fill_color = (color & 0xFFFFFF) | ((alpha & 0xFF) << 24);
> + int i;
>
> DPU_DEBUG_PLANE(pdpu, "\n");
>
> @@ -632,12 +633,13 @@ static void _dpu_plane_color_fill(struct dpu_plane *pdpu,
> return;
>
> /* update sspp */
> - _dpu_plane_color_fill_pipe(pstate, &pstate->pipe, &pstate->pipe_cfg.dst_rect,
> - fill_color, fmt);
> -
> - if (pstate->r_pipe.sspp)
> - _dpu_plane_color_fill_pipe(pstate, &pstate->r_pipe, &pstate->r_pipe_cfg.dst_rect,
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + _dpu_plane_color_fill_pipe(pstate, &pstate->pipe[i],
> + &pstate->pipe_cfg[i].dst_rect,
> fill_color, fmt);
> + }
> }
>
> static int dpu_plane_prepare_fb(struct drm_plane *plane,
> @@ -827,8 +829,8 @@ static int dpu_plane_atomic_check_nosspp(struct drm_plane *plane,
> struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> + struct dpu_sw_pipe_cfg *pipe_cfg;
> + struct dpu_sw_pipe_cfg *r_pipe_cfg;
> struct drm_rect fb_rect = { 0 };
> uint32_t max_linewidth;
>
> @@ -853,6 +855,9 @@ 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);
>
> @@ -949,10 +954,10 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> drm_atomic_get_new_plane_state(state, plane);
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> - struct dpu_sw_pipe *pipe = &pstate->pipe;
> - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> + struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> int ret = 0;
>
> ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> @@ -1011,10 +1016,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> - struct dpu_sw_pipe *pipe = &pstate->pipe;
> - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> + struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> const struct drm_crtc_state *crtc_state = NULL;
> uint32_t max_linewidth = dpu_kms->catalog->caps->max_linewidth;
>
> @@ -1058,7 +1063,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> drm_atomic_get_old_plane_state(state, plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state);
> struct drm_crtc_state *crtc_state;
> - int ret;
> + int ret, i;
>
> if (plane_state->crtc)
> crtc_state = drm_atomic_get_new_crtc_state(state,
> @@ -1073,8 +1078,8 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> * resources are freed by dpu_crtc_assign_plane_resources(),
> * but clean them here.
> */
> - pstate->pipe.sspp = NULL;
> - pstate->r_pipe.sspp = NULL;
> + for (i = 0; i < PIPES_PER_STAGE; i++)
> + pstate->pipe[i].sspp = NULL;
>
> return 0;
> }
> @@ -1111,19 +1116,21 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> struct dpu_sw_pipe_cfg *pipe_cfg;
> struct dpu_sw_pipe_cfg *r_pipe_cfg;
> const struct msm_format *fmt;
> + int i;
>
> if (plane_state->crtc)
> crtc_state = drm_atomic_get_new_crtc_state(state,
> plane_state->crtc);
>
> pstate = to_dpu_plane_state(plane_state);
> - pipe = &pstate->pipe;
> - r_pipe = &pstate->r_pipe;
> - pipe_cfg = &pstate->pipe_cfg;
> - r_pipe_cfg = &pstate->r_pipe_cfg;
>
> - pipe->sspp = NULL;
> - r_pipe->sspp = NULL;
> + pipe = &pstate->pipe[0];
> + r_pipe = &pstate->pipe[1];
> + pipe_cfg = &pstate->pipe_cfg[0];
> + r_pipe_cfg = &pstate->pipe_cfg[1];
> +
> + for (i = 0; i < PIPES_PER_STAGE; i++)
> + pstate->pipe[i].sspp = NULL;
>
> if (!plane_state->fb)
> return -EINVAL;
> @@ -1212,6 +1219,7 @@ void dpu_plane_flush(struct drm_plane *plane)
> {
> struct dpu_plane *pdpu;
> struct dpu_plane_state *pstate;
> + int i;
>
> if (!plane || !plane->state) {
> DPU_ERROR("invalid plane\n");
> @@ -1232,8 +1240,8 @@ void dpu_plane_flush(struct drm_plane *plane)
> /* force 100% alpha */
> _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> else {
> - dpu_plane_flush_csc(pdpu, &pstate->pipe);
> - dpu_plane_flush_csc(pdpu, &pstate->r_pipe);
> + for (i = 0; i < PIPES_PER_STAGE; i++)
> + dpu_plane_flush_csc(pdpu, &pstate->pipe[i]);
> }
>
> /* flag h/w flush complete */
> @@ -1334,15 +1342,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct drm_plane_state *state = plane->state;
> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> - struct dpu_sw_pipe *pipe = &pstate->pipe;
> - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> struct drm_crtc *crtc = state->crtc;
> struct drm_framebuffer *fb = state->fb;
> bool is_rt_pipe;
> const struct msm_format *fmt =
> msm_framebuffer_format(fb);
> - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> + int i;
>
> pstate->pending = true;
>
> @@ -1357,12 +1362,12 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> crtc->base.id, DRM_RECT_ARG(&state->dst),
> &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt));
>
> - dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
> - drm_mode_vrefresh(&crtc->mode),
> - &pstate->layout);
> -
> - if (r_pipe->sspp) {
> - dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
> + /* move the assignment here, to ease handling to another pairs later */
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i],
> + &pstate->pipe_cfg[i], fmt,
> drm_mode_vrefresh(&crtc->mode),
> &pstate->layout);
> }
> @@ -1370,15 +1375,17 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> if (pstate->needs_qos_remap)
> pstate->needs_qos_remap = false;
>
> - pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt,
> - &crtc->mode, pipe_cfg);
> -
> - pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
> -
> - if (r_pipe->sspp) {
> - pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, r_pipe_cfg);
> + pstate->plane_fetch_bw = 0;
> + pstate->plane_clk = 0;
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt,
> + &crtc->mode, &pstate->pipe_cfg[i]);
>
> - pstate->plane_clk = max(pstate->plane_clk, _dpu_plane_calc_clk(&crtc->mode, r_pipe_cfg));
> + pstate->plane_clk = max(pstate->plane_clk,
> + _dpu_plane_calc_clk(&crtc->mode,
> + &pstate->pipe_cfg[i]));
> }
> }
>
> @@ -1386,17 +1393,31 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> {
> struct drm_plane_state *state = plane->state;
> struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> - struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> + struct dpu_sw_pipe *pipe;
> + int i;
>
> - trace_dpu_plane_disable(DRMID(plane), false,
> - pstate->pipe.multirect_mode);
> + for (i = 0; i < PIPES_PER_STAGE; i += 1) {
> + pipe = &pstate->pipe[i];
> + if (!pipe->sspp)
> + continue;
>
> - if (r_pipe->sspp) {
> - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> + trace_dpu_plane_disable(DRMID(plane), false,
> + pstate->pipe[i].multirect_mode);
> +
> + if (!pipe->sspp)
> + continue;
>
> - if (r_pipe->sspp->ops.setup_multirect)
> - r_pipe->sspp->ops.setup_multirect(r_pipe);
> + if (i % PIPES_PER_STAGE == 0)
> + continue;
> +
> + /*
> + * clear multirect for the right pipe so that the SSPP
> + * can be further reused in the solo mode
> + */
> + pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> + if (pipe->sspp->ops.setup_multirect)
> + pipe->sspp->ops.setup_multirect(pipe);
> }
>
> pstate->pending = true;
> @@ -1491,31 +1512,26 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
> const struct drm_plane_state *state)
> {
> const struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> - const struct dpu_sw_pipe *pipe = &pstate->pipe;
> - const struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> - const struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> - const struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> + const struct dpu_sw_pipe *pipe;
> + const struct dpu_sw_pipe_cfg *pipe_cfg;
> + int i;
>
> drm_printf(p, "\tstage=%d\n", pstate->stage);
>
> - if (pipe->sspp) {
> - drm_printf(p, "\tsspp[0]=%s\n", pipe->sspp->cap->name);
> - drm_printf(p, "\tmultirect_mode[0]=%s\n",
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + pipe = &pstate->pipe[i];
> + if (!pipe->sspp)
> + continue;
> + pipe_cfg = &pstate->pipe_cfg[i];
> + drm_printf(p, "\tsspp[%d]=%s\n", i, pipe->sspp->cap->name);
> + drm_printf(p, "\tmultirect_mode[%d]=%s\n", i,
> dpu_get_multirect_mode(pipe->multirect_mode));
> - drm_printf(p, "\tmultirect_index[0]=%s\n",
> + drm_printf(p, "\tmultirect_index[%d]=%s\n", i,
> dpu_get_multirect_index(pipe->multirect_index));
> - drm_printf(p, "\tsrc[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->src_rect));
> - drm_printf(p, "\tdst[0]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&pipe_cfg->dst_rect));
> - }
> -
> - if (r_pipe->sspp) {
> - drm_printf(p, "\tsspp[1]=%s\n", r_pipe->sspp->cap->name);
> - drm_printf(p, "\tmultirect_mode[1]=%s\n",
> - dpu_get_multirect_mode(r_pipe->multirect_mode));
> - drm_printf(p, "\tmultirect_index[1]=%s\n",
> - dpu_get_multirect_index(r_pipe->multirect_index));
> - drm_printf(p, "\tsrc[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->src_rect));
> - drm_printf(p, "\tdst[1]=" DRM_RECT_FMT "\n", DRM_RECT_ARG(&r_pipe_cfg->dst_rect));
> + drm_printf(p, "\tsrc[%d]=" DRM_RECT_FMT "\n", i,
> + DRM_RECT_ARG(&pipe_cfg->src_rect));
> + drm_printf(p, "\tdst[%d]=" DRM_RECT_FMT "\n", i,
> + DRM_RECT_ARG(&pipe_cfg->dst_rect));
> }
> }
>
> @@ -1553,14 +1569,17 @@ void dpu_plane_danger_signal_ctrl(struct drm_plane *plane, bool enable)
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(plane->state);
> struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> + int i;
>
> if (!pdpu->is_rt_pipe)
> return;
>
> pm_runtime_get_sync(&dpu_kms->pdev->dev);
> - _dpu_plane_set_qos_ctrl(plane, &pstate->pipe, enable);
> - if (pstate->r_pipe.sspp)
> - _dpu_plane_set_qos_ctrl(plane, &pstate->r_pipe, enable);
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + if (!pstate->pipe[i].sspp)
> + continue;
> + _dpu_plane_set_qos_ctrl(plane, &pstate->pipe[i], enable);
> + }
> pm_runtime_put_sync(&dpu_kms->pdev->dev);
> }
> #endif
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index acd5725175cdde4fcf7a9f71bb446251c5a14d22..052fd046e8463855b16b30389c2efc67c0c15281 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -18,10 +18,8 @@
> * struct dpu_plane_state: Define dpu extension of drm plane state object
> * @base: base drm plane state object
> * @aspace: pointer to address space for input/output buffers
> - * @pipe: software pipe description
> - * @r_pipe: software pipe description of the second pipe
> - * @pipe_cfg: software pipe configuration
> - * @r_pipe_cfg: software pipe configuration for the second pipe
> + * @pipe: software pipe description array
> + * @pipe_cfg: software pipe configuration array
> * @stage: assigned by crtc blender
> * @needs_qos_remap: qos remap settings need to be updated
> * @multirect_index: index of the rectangle of SSPP
> @@ -35,10 +33,8 @@
> struct dpu_plane_state {
> struct drm_plane_state base;
> struct msm_gem_address_space *aspace;
> - struct dpu_sw_pipe pipe;
> - struct dpu_sw_pipe r_pipe;
> - struct dpu_sw_pipe_cfg pipe_cfg;
> - struct dpu_sw_pipe_cfg r_pipe_cfg;
> + struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
> + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
> enum dpu_stage stage;
> bool needs_qos_remap;
> bool pending;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition
2025-05-06 15:47 ` [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition Jun Nie
2025-05-08 13:10 ` Dmitry Baryshkov
@ 2025-05-08 18:03 ` Jessica Zhang
1 sibling, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 18:03 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> Currently MAX_CHANNELS_PER_ENC is defined as 2, because 2 channels are
> supported at most in one encoder. The case of 4 channels per encoder is
> to be added. To avoid breaking current WB usage case, use dedicated WB
> definition before 4 WB usage case is supported in future.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 3e4ff345bd6d9c0e2a08274874fac40690409cf0..a125a477404fdafee6495f6d3778160f14a01a96 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -56,6 +56,7 @@
> (MAX_H_TILES_PER_DISPLAY * NUM_PHYS_ENCODER_TYPES)
>
> #define MAX_CHANNELS_PER_ENC 2
> +#define MAX_CWB_PER_ENC 2
>
> #define IDLE_SHORT_TIMEOUT 1
>
> @@ -182,7 +183,7 @@ struct dpu_encoder_virt {
> struct dpu_encoder_phys *cur_master;
> struct dpu_encoder_phys *cur_slave;
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> - struct dpu_hw_cwb *hw_cwb[MAX_CHANNELS_PER_ENC];
> + struct dpu_hw_cwb *hw_cwb[MAX_CWB_PER_ENC];
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>
> unsigned int dsc_mask;
> @@ -2372,7 +2373,7 @@ void dpu_encoder_helper_phys_setup_cwb(struct dpu_encoder_phys *phys_enc,
> */
> cwb_cfg.input = INPUT_MODE_LM_OUT;
>
> - for (int i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> + for (int i = 0; i < MAX_CWB_PER_ENC; i++) {
> hw_cwb = dpu_enc->hw_cwb[i];
> if (!hw_cwb)
> continue;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability
2025-05-06 15:47 ` [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability Jun Nie
@ 2025-05-08 18:38 ` Jessica Zhang
0 siblings, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 18:38 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> The capability stored in sblk and pipe_hw_caps is checked only for
> SSPP of the first pipe in the pair with current implementation. That
> of the 2nd pipe, r_pipe, is not checked and may violate hardware
> capability. Move requirement check to dpu_plane_atomic_check_pipe()
> for the check of every pipe.
>
> Fixes: ("dbbf57dfd04e6 drm/msm/dpu: split dpu_plane_atomic_check()")
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Hi Jun,
Just wanted to note that this has already been merged in msm-next [1] so
I think you can drop this in the next revision.
But, FWIW
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Thanks,
Jessica Zhang
[1]
https://gitlab.freedesktop.org/drm/msm/-/commit/bcaa391e177c06a58c5f3cd18484a317d40239aa
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 ++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index af3e541f60c303eb5212524e877129359b5ca98c..aeb90c287245d6aaa18b9f280d1e628ee6ed74f5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -729,12 +729,40 @@ static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
> static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
> struct dpu_sw_pipe *pipe,
> struct dpu_sw_pipe_cfg *pipe_cfg,
> - const struct msm_format *fmt,
> - const struct drm_display_mode *mode)
> + const struct drm_display_mode *mode,
> + struct drm_plane_state *new_plane_state)
> {
> uint32_t min_src_size;
> struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> int ret;
> + const struct msm_format *fmt;
> + uint32_t supported_rotations;
> + const struct dpu_sspp_cfg *pipe_hw_caps;
> + const struct dpu_sspp_sub_blks *sblk;
> +
> + pipe_hw_caps = pipe->sspp->cap;
> + sblk = pipe->sspp->cap->sblk;
> +
> + /*
> + * We already have verified scaling against platform limitations.
> + * Now check if the SSPP supports scaling at all.
> + */
> + if (!sblk->scaler_blk.len &&
> + ((drm_rect_width(&new_plane_state->src) >> 16 !=
> + drm_rect_width(&new_plane_state->dst)) ||
> + (drm_rect_height(&new_plane_state->src) >> 16 !=
> + drm_rect_height(&new_plane_state->dst))))
> + return -ERANGE;
> +
> + fmt = msm_framebuffer_format(new_plane_state->fb);
> +
> + supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> +
> + if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> + supported_rotations |= DRM_MODE_ROTATE_90;
> +
> + pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> + supported_rotations);
>
> min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
>
> @@ -923,47 +951,20 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> struct dpu_sw_pipe *pipe = &pstate->pipe;
> struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> - const struct msm_format *fmt;
> struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> - uint32_t supported_rotations;
> - const struct dpu_sspp_cfg *pipe_hw_caps;
> - const struct dpu_sspp_sub_blks *sblk;
> int ret = 0;
>
> - pipe_hw_caps = pipe->sspp->cap;
> - sblk = pipe->sspp->cap->sblk;
> -
> - /*
> - * We already have verified scaling against platform limitations.
> - * Now check if the SSPP supports scaling at all.
> - */
> - if (!sblk->scaler_blk.len &&
> - ((drm_rect_width(&new_plane_state->src) >> 16 !=
> - drm_rect_width(&new_plane_state->dst)) ||
> - (drm_rect_height(&new_plane_state->src) >> 16 !=
> - drm_rect_height(&new_plane_state->dst))))
> - return -ERANGE;
> -
> - fmt = msm_framebuffer_format(new_plane_state->fb);
> -
> - supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> -
> - if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> - supported_rotations |= DRM_MODE_ROTATE_90;
> -
> - pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> - supported_rotations);
> - r_pipe_cfg->rotation = pipe_cfg->rotation;
> -
> - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt,
> - &crtc_state->adjusted_mode);
> + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> + &crtc_state->adjusted_mode,
> + new_plane_state);
> if (ret)
> return ret;
>
> if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
> - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt,
> - &crtc_state->adjusted_mode);
> + ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
> + &crtc_state->adjusted_mode,
> + new_plane_state);
> if (ret)
> return ret;
> }
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case
2025-05-06 15:47 ` [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case Jun Nie
@ 2025-05-08 18:51 ` Jessica Zhang
0 siblings, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 18:51 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, 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@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 68 +++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index ea7e3fdf52f726737941ad33218a843dca17280b..de3f52d743e1d1f11ae8721a316b9872d4139069 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1112,11 +1112,10 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> struct dpu_rm_sspp_requirements reqs;
> struct dpu_plane_state *pstate;
> struct dpu_sw_pipe *pipe;
> - struct dpu_sw_pipe *r_pipe;
> struct dpu_sw_pipe_cfg *pipe_cfg;
> - struct dpu_sw_pipe_cfg *r_pipe_cfg;
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> const struct msm_format *fmt;
> - int i;
> + int i, num_lm, stage_id, num_stages;
>
> if (plane_state->crtc)
> crtc_state = drm_atomic_get_new_crtc_state(state,
> @@ -1124,11 +1123,6 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>
> pstate = to_dpu_plane_state(plane_state);
>
> - pipe = &pstate->pipe[0];
> - r_pipe = &pstate->pipe[1];
> - pipe_cfg = &pstate->pipe_cfg[0];
> - r_pipe_cfg = &pstate->pipe_cfg[1];
> -
> for (i = 0; i < PIPES_PER_PLANE; i++)
> pstate->pipe[i].sspp = NULL;
>
> @@ -1142,24 +1136,46 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>
> reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
>
> - pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> - if (!pipe->sspp)
> - return -ENODEV;
> -
> - if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> - pipe->sspp,
> - msm_framebuffer_format(plane_state->fb),
> - dpu_kms->catalog->caps->max_linewidth)) {
> - /* multirect is not possible, use two SSPP blocks */
> - r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> - if (!r_pipe->sspp)
> - return -ENODEV;
> -
> - pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> - pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> -
> - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> + num_lm = dpu_crtc_get_num_lm(crtc_state);
Hi Jun,
I think there might be some patch ordering issue here since
`dpu_crtc_get_num_lm()` is only declared/defined in the next patch. This
will break compilation upon partial application of the series.
Also, can you rebase this on top of the latest msm-next? The changes
here will conflict with commit 3ed12a36 ("drm/msm/dpu: allow sharing
SSPP between planes").
Thanks,
Jessica Zhang
> + num_stages = (num_lm + 1) / 2;
> + for (stage_id = 0; stage_id < num_stages; stage_id++) {
> + for (i = stage_id * PIPES_PER_STAGE; i < (stage_id + 1) * PIPES_PER_STAGE; i++) {
> + struct dpu_sw_pipe *r_pipe;
> + struct dpu_sw_pipe_cfg *r_pipe_cfg;
> +
> + pipe = &pstate->pipe[i];
> + pipe_cfg = &pstate->pipe_cfg[i];
> +
> + if (drm_rect_width(&pipe_cfg->src_rect) == 0)
> + break;
> +
> + pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm, global_state, crtc, &reqs);
> + if (!pipe->sspp)
> + return -ENODEV;
> +
> + r_pipe = &pstate->pipe[i + 1];
> + r_pipe_cfg = &pstate->pipe_cfg[i + 1];
> +
> + /*
> + * If current pipe is the first pipe in pipe pair, check
> + * multi-rect opportunity for the 2nd pipe in the pair.
> + * SSPP multi-rect mode cross mixer pairs is not supported.
> + */
> + if ((i % PIPES_PER_STAGE == 0) &&
> + drm_rect_width(&r_pipe_cfg->src_rect) != 0 &&
> + dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> + pipe->sspp,
> + msm_framebuffer_format(plane_state->fb),
> + dpu_kms->catalog->caps->max_linewidth)) {
> + i++;
> + } else {
> + /* multirect is not possible, use two SSPP blocks */
> + pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> + pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> + DPU_DEBUG_PLANE(pdpu, "allocating sspp_%d for pipe %d.\n",
> + pipe->sspp->idx - SSPP_NONE, i);
> + }
> + }
> }
>
> return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer
2025-05-06 15:47 ` [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer Jun Nie
@ 2025-05-08 20:42 ` Jessica Zhang
2025-05-08 20:55 ` Jessica Zhang
0 siblings, 1 reply; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 20:42 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> The stage contains configuration for a mixer pair. Currently the plane
> supports just one stage and 2 pipes. Quad-pipe support will require
> handling 2 stages and 4 pipes at the same time. In preparation for that
> add a separate define, PIPES_PER_PLANE, to denote number of pipes that
> can be used by the plane.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Hi Jun,
I think the comment from v7 about propogating the PIPES_PER_PLANE change
to _dpu_plane_color_fill() got dropped in this version [1].
Also, any reason PIPES_PER_STAGE was kept for
dpu_plane_danger_signal_ctrl()?
Thanks,
Jessica Zhang
[1]
https://patchwork.freedesktop.org/patch/640534/?series=139762&rev=6#comment_1171802
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 +++++++-------
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 ++--
> 4 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 6338603bf8be9fcc4324b098d5d69d20235cdbae..d710b4eec7ad946a4cf74d6ac5f4db90e8dcf1fd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -463,7 +463,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> bg_alpha_enable = true;
>
> - for (i = 0; i < PIPES_PER_STAGE; i++) {
> + for (i = 0; i < PIPES_PER_PLANE; i++) {
> if (!pstate->pipe[i].sspp)
> continue;
> set_bit(pstate->pipe[i].sspp->idx, fetch_active);
> @@ -1272,7 +1272,7 @@ static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct drm_crtc_state
> return ret;
> }
>
> -#define MAX_CHANNELS_PER_CRTC 2
> +#define MAX_CHANNELS_PER_CRTC PIPES_PER_PLANE
> #define MAX_HDISPLAY_SPLIT 1080
>
> static struct msm_display_topology dpu_crtc_get_topology(
> 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 8d820cd1b5545d247515763039b341184e814e32..b0ed41108a32158c0bc3be2e25fc229b218fd6c5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -34,6 +34,7 @@
> #define DPU_MAX_PLANES 4
> #endif
>
> +#define PIPES_PER_PLANE 2
> #define PIPES_PER_STAGE 2
> #ifndef DPU_MAX_DE_CURVES
> #define DPU_MAX_DE_CURVES 3
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 4cb81a6a692be51d342d9a6f322b632e5fd07b2c..ea7e3fdf52f726737941ad33218a843dca17280b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1078,7 +1078,7 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> * resources are freed by dpu_crtc_assign_plane_resources(),
> * but clean them here.
> */
> - for (i = 0; i < PIPES_PER_STAGE; i++)
> + for (i = 0; i < PIPES_PER_PLANE; i++)
> pstate->pipe[i].sspp = NULL;
>
> return 0;
> @@ -1129,7 +1129,7 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> pipe_cfg = &pstate->pipe_cfg[0];
> r_pipe_cfg = &pstate->pipe_cfg[1];
>
> - for (i = 0; i < PIPES_PER_STAGE; i++)
> + for (i = 0; i < PIPES_PER_PLANE; i++)
> pstate->pipe[i].sspp = NULL;
>
> if (!plane_state->fb)
> @@ -1240,7 +1240,7 @@ void dpu_plane_flush(struct drm_plane *plane)
> /* force 100% alpha */
> _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
> else {
> - for (i = 0; i < PIPES_PER_STAGE; i++)
> + for (i = 0; i < PIPES_PER_PLANE; i++)
> dpu_plane_flush_csc(pdpu, &pstate->pipe[i]);
> }
>
> @@ -1363,7 +1363,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt));
>
> /* move the assignment here, to ease handling to another pairs later */
> - for (i = 0; i < PIPES_PER_STAGE; i++) {
> + for (i = 0; i < PIPES_PER_PLANE; i++) {
> if (!pstate->pipe[i].sspp)
> continue;
> dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i],
> @@ -1377,7 +1377,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
>
> pstate->plane_fetch_bw = 0;
> pstate->plane_clk = 0;
> - for (i = 0; i < PIPES_PER_STAGE; i++) {
> + for (i = 0; i < PIPES_PER_PLANE; i++) {
> if (!pstate->pipe[i].sspp)
> continue;
> pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog, fmt,
> @@ -1396,7 +1396,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> struct dpu_sw_pipe *pipe;
> int i;
>
> - for (i = 0; i < PIPES_PER_STAGE; i += 1) {
> + for (i = 0; i < PIPES_PER_PLANE; i += 1) {
> pipe = &pstate->pipe[i];
> if (!pipe->sspp)
> continue;
> @@ -1518,7 +1518,7 @@ static void dpu_plane_atomic_print_state(struct drm_printer *p,
>
> drm_printf(p, "\tstage=%d\n", pstate->stage);
>
> - for (i = 0; i < PIPES_PER_STAGE; i++) {
> + for (i = 0; i < PIPES_PER_PLANE; i++) {
> pipe = &pstate->pipe[i];
> if (!pipe->sspp)
> continue;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> index 052fd046e8463855b16b30389c2efc67c0c15281..18ff5ec2603ed63ce45f530ced3407d3b70c737b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
> @@ -33,8 +33,8 @@
> struct dpu_plane_state {
> struct drm_plane_state base;
> struct msm_gem_address_space *aspace;
> - struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
> - struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
> + struct dpu_sw_pipe pipe[PIPES_PER_PLANE];
> + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_PLANE];
> enum dpu_stage stage;
> bool needs_qos_remap;
> bool pending;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config
2025-05-06 15:47 ` [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config Jun Nie
@ 2025-05-08 20:53 ` Jessica Zhang
0 siblings, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 20:53 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> Currently, only 2 pipes are used at most for a plane. A stage structure
> describes the configuration for a mixer pair. So only one stage is needed
> for current usage cases. The quad-pipe case will be added in future and 2
> stages are used in the case. So extend the stage to an array with array
> size STAGES_PER_PLANE and blend pipes per mixer pair with configuration in
> the stage structure.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 45 +++++++++++++++++++----------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +-
> 2 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index d710b4eec7ad946a4cf74d6ac5f4db90e8dcf1fd..f35cb1f7a7d2c2c63b4228bc47b85bb57cddbe6b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -392,7 +392,7 @@ static void _dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)
> static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
> struct drm_plane *plane,
> struct dpu_crtc_mixer *mixer,
> - u32 num_mixers,
> + u32 lms_in_pair,
> enum dpu_stage stage,
> const struct msm_format *format,
> uint64_t modifier,
> @@ -426,7 +426,7 @@ static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
> stage_cfg->multirect_index[stage][stage_idx] = pipe->multirect_index;
>
> /* blend config update */
> - for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
> + for (lm_idx = 0; lm_idx < lms_in_pair; lm_idx++)
> mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl, sspp_idx);
> }
>
> @@ -442,7 +442,7 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> const struct msm_format *format;
> struct dpu_hw_ctl *ctl = mixer->lm_ctl;
>
> - uint32_t lm_idx, i;
> + uint32_t lm_idx, stage, i, pipe_idx, head_pipe_in_stage, lms_in_pair;
> bool bg_alpha_enable = false;
> DECLARE_BITMAP(fetch_active, SSPP_MAX);
>
> @@ -463,15 +463,24 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> bg_alpha_enable = true;
>
> - for (i = 0; i < PIPES_PER_PLANE; i++) {
> - if (!pstate->pipe[i].sspp)
> - continue;
> - set_bit(pstate->pipe[i].sspp->idx, fetch_active);
> - _dpu_crtc_blend_setup_pipe(crtc, plane,
> - mixer, cstate->num_mixers,
> - pstate->stage,
> - format, fb ? fb->modifier : 0,
> - &pstate->pipe[i], i, stage_cfg);
> + /* loop pipe per mixer pair with config in stage structure */
> + for (stage = 0; stage < STAGES_PER_PLANE; stage++) {
> + head_pipe_in_stage = stage * PIPES_PER_STAGE;
> + for (i = 0; i < PIPES_PER_STAGE; i++) {
> + pipe_idx = i + head_pipe_in_stage;
> + if (!pstate->pipe[pipe_idx].sspp)
> + continue;
> + lms_in_pair = min(cstate->num_mixers - (stage * PIPES_PER_STAGE),
> + PIPES_PER_STAGE);
> + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active);
> + _dpu_crtc_blend_setup_pipe(crtc, plane,
> + &mixer[head_pipe_in_stage],
> + lms_in_pair,
> + pstate->stage,
> + format, fb ? fb->modifier : 0,
> + &pstate->pipe[pipe_idx], i,
> + &stage_cfg[stage]);
> + }
> }
>
> /* blend config update */
> @@ -503,7 +512,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
> struct dpu_crtc_mixer *mixer = cstate->mixers;
> struct dpu_hw_ctl *ctl;
> struct dpu_hw_mixer *lm;
> - struct dpu_hw_stage_cfg stage_cfg;
> + struct dpu_hw_stage_cfg stage_cfg[STAGES_PER_PLANE];
> int i;
>
> DRM_DEBUG_ATOMIC("%s\n", dpu_crtc->name);
> @@ -516,9 +525,9 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
> }
>
> /* initialize stage cfg */
> - memset(&stage_cfg, 0, sizeof(struct dpu_hw_stage_cfg));
> + memset(&stage_cfg, 0, sizeof(stage_cfg));
>
> - _dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer, &stage_cfg);
> + _dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer, stage_cfg);
>
> for (i = 0; i < cstate->num_mixers; i++) {
> ctl = mixer[i].lm_ctl;
> @@ -535,8 +544,12 @@ static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
> mixer[i].mixer_op_mode,
> ctl->idx - CTL_0);
>
> + /*
> + * call dpu_hw_ctl_setup_blendstage() to blend layers per stage cfg.
> + * stage data is shared between PIPES_PER_STAGE pipes.
> + */
> ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx,
> - &stage_cfg);
> + &stage_cfg[i / PIPES_PER_STAGE]);
> }
> }
>
> 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 b0ed41108a32158c0bc3be2e25fc229b218fd6c5..7c74221380b2c05225c9f82ed6d33765042aec78 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -34,8 +34,9 @@
> #define DPU_MAX_PLANES 4
> #endif
>
> -#define PIPES_PER_PLANE 2
> +#define STAGES_PER_PLANE 1
> #define PIPES_PER_STAGE 2
> +#define PIPES_PER_PLANE (PIPES_PER_STAGE * STAGES_PER_PLANE)
> #ifndef DPU_MAX_DE_CURVES
> #define DPU_MAX_DE_CURVES 3
> #endif
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer
2025-05-08 20:42 ` Jessica Zhang
@ 2025-05-08 20:55 ` Jessica Zhang
2025-05-23 10:03 ` Jun Nie
0 siblings, 1 reply; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 20:55 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/8/2025 1:42 PM, Jessica Zhang wrote:
>
>
> On 5/6/2025 8:47 AM, Jun Nie wrote:
>> The stage contains configuration for a mixer pair. Currently the plane
>> supports just one stage and 2 pipes. Quad-pipe support will require
>> handling 2 stages and 4 pipes at the same time. In preparation for that
>> add a separate define, PIPES_PER_PLANE, to denote number of pipes that
>> can be used by the plane.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Hi Jun,
>
> I think the comment from v7 about propogating the PIPES_PER_PLANE change
> to _dpu_plane_color_fill() got dropped in this version [1].
s/propogating/propagating/
>
> Also, any reason PIPES_PER_STAGE was kept for
> dpu_plane_danger_signal_ctrl()?
I have the same question for _dpu_debugfs_status_show(). It seems to me
both helpers should be using PIPES_PER_PLANE.
Thanks,
Jessica Zhang
>
> Thanks,
>
> Jessica Zhang
>
> [1] https://patchwork.freedesktop.org/patch/640534/?
> series=139762&rev=6#comment_1171802
>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++--
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 +++++++-------
>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 ++--
>> 4 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/
>> drm/msm/disp/dpu1/dpu_crtc.c
>> index
>> 6338603bf8be9fcc4324b098d5d69d20235cdbae..d710b4eec7ad946a4cf74d6ac5f4db90e8dcf1fd 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -463,7 +463,7 @@ static void _dpu_crtc_blend_setup_mixer(struct
>> drm_crtc *crtc,
>> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
>> bg_alpha_enable = true;
>> - for (i = 0; i < PIPES_PER_STAGE; i++) {
>> + for (i = 0; i < PIPES_PER_PLANE; i++) {
>> if (!pstate->pipe[i].sspp)
>> continue;
>> set_bit(pstate->pipe[i].sspp->idx, fetch_active);
>> @@ -1272,7 +1272,7 @@ static int dpu_crtc_reassign_planes(struct
>> drm_crtc *crtc, struct drm_crtc_state
>> return ret;
>> }
>> -#define MAX_CHANNELS_PER_CRTC 2
>> +#define MAX_CHANNELS_PER_CRTC PIPES_PER_PLANE
>> #define MAX_HDISPLAY_SPLIT 1080
>> static struct msm_display_topology dpu_crtc_get_topology(
>> 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
>> 8d820cd1b5545d247515763039b341184e814e32..b0ed41108a32158c0bc3be2e25fc229b218fd6c5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
>> @@ -34,6 +34,7 @@
>> #define DPU_MAX_PLANES 4
>> #endif
>> +#define PIPES_PER_PLANE 2
>> #define PIPES_PER_STAGE 2
>> #ifndef DPU_MAX_DE_CURVES
>> #define DPU_MAX_DE_CURVES 3
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/
>> drm/msm/disp/dpu1/dpu_plane.c
>> index
>> 4cb81a6a692be51d342d9a6f322b632e5fd07b2c..ea7e3fdf52f726737941ad33218a843dca17280b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -1078,7 +1078,7 @@ static int dpu_plane_virtual_atomic_check(struct
>> drm_plane *plane,
>> * resources are freed by dpu_crtc_assign_plane_resources(),
>> * but clean them here.
>> */
>> - for (i = 0; i < PIPES_PER_STAGE; i++)
>> + for (i = 0; i < PIPES_PER_PLANE; i++)
>> pstate->pipe[i].sspp = NULL;
>> return 0;
>> @@ -1129,7 +1129,7 @@ static int
>> dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
>> pipe_cfg = &pstate->pipe_cfg[0];
>> r_pipe_cfg = &pstate->pipe_cfg[1];
>> - for (i = 0; i < PIPES_PER_STAGE; i++)
>> + for (i = 0; i < PIPES_PER_PLANE; i++)
>> pstate->pipe[i].sspp = NULL;
>> if (!plane_state->fb)
>> @@ -1240,7 +1240,7 @@ void dpu_plane_flush(struct drm_plane *plane)
>> /* force 100% alpha */
>> _dpu_plane_color_fill(pdpu, pdpu->color_fill, 0xFF);
>> else {
>> - for (i = 0; i < PIPES_PER_STAGE; i++)
>> + for (i = 0; i < PIPES_PER_PLANE; i++)
>> dpu_plane_flush_csc(pdpu, &pstate->pipe[i]);
>> }
>> @@ -1363,7 +1363,7 @@ static void dpu_plane_sspp_atomic_update(struct
>> drm_plane *plane,
>> &fmt->pixel_format, MSM_FORMAT_IS_UBWC(fmt));
>> /* move the assignment here, to ease handling to another pairs
>> later */
>> - for (i = 0; i < PIPES_PER_STAGE; i++) {
>> + for (i = 0; i < PIPES_PER_PLANE; i++) {
>> if (!pstate->pipe[i].sspp)
>> continue;
>> dpu_plane_sspp_update_pipe(plane, &pstate->pipe[i],
>> @@ -1377,7 +1377,7 @@ static void dpu_plane_sspp_atomic_update(struct
>> drm_plane *plane,
>> pstate->plane_fetch_bw = 0;
>> pstate->plane_clk = 0;
>> - for (i = 0; i < PIPES_PER_STAGE; i++) {
>> + for (i = 0; i < PIPES_PER_PLANE; i++) {
>> if (!pstate->pipe[i].sspp)
>> continue;
>> pstate->plane_fetch_bw += _dpu_plane_calc_bw(pdpu->catalog,
>> fmt,
>> @@ -1396,7 +1396,7 @@ static void _dpu_plane_atomic_disable(struct
>> drm_plane *plane)
>> struct dpu_sw_pipe *pipe;
>> int i;
>> - for (i = 0; i < PIPES_PER_STAGE; i += 1) {
>> + for (i = 0; i < PIPES_PER_PLANE; i += 1) {
>> pipe = &pstate->pipe[i];
>> if (!pipe->sspp)
>> continue;
>> @@ -1518,7 +1518,7 @@ static void dpu_plane_atomic_print_state(struct
>> drm_printer *p,
>> drm_printf(p, "\tstage=%d\n", pstate->stage);
>> - for (i = 0; i < PIPES_PER_STAGE; i++) {
>> + for (i = 0; i < PIPES_PER_PLANE; i++) {
>> pipe = &pstate->pipe[i];
>> if (!pipe->sspp)
>> continue;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/
>> drm/msm/disp/dpu1/dpu_plane.h
>> index
>> 052fd046e8463855b16b30389c2efc67c0c15281..18ff5ec2603ed63ce45f530ced3407d3b70c737b 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
>> @@ -33,8 +33,8 @@
>> struct dpu_plane_state {
>> struct drm_plane_state base;
>> struct msm_gem_address_space *aspace;
>> - struct dpu_sw_pipe pipe[PIPES_PER_STAGE];
>> - struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_STAGE];
>> + struct dpu_sw_pipe pipe[PIPES_PER_PLANE];
>> + struct dpu_sw_pipe_cfg pipe_cfg[PIPES_PER_PLANE];
>> enum dpu_stage stage;
>> bool needs_qos_remap;
>> bool pending;
>>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 13/14] drm/msm/dpu: support plane splitting in quad-pipe case
2025-05-06 15:47 ` [PATCH v9 13/14] drm/msm/dpu: support plane splitting in " Jun Nie
@ 2025-05-08 21:24 ` Jessica Zhang
2025-05-23 10:21 ` Jun Nie
0 siblings, 1 reply; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 21:24 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> 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@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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 | 134 +++++++++++++++++++++---------
> 3 files changed, 107 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 f35cb1f7a7d2c2c63b4228bc47b85bb57cddbe6b..a56d68de219910a827830293e8ff24f4cdee74e4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1560,6 +1560,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 de3f52d743e1d1f11ae8721a316b9872d4139069..cf2b4d5cb0ccc144c2cf8fd227c862b0b6e7725f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -831,8 +831,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;
> @@ -855,13 +859,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;
> @@ -886,35 +887,91 @@ 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;
> + /* iterate mixer configs for this plane, to separate left/right with the id */
> + for (stage_id = 0; stage_id < num_stages; stage_id++) {
> + struct drm_rect mixer_rect = {stage_id * mode->hdisplay / num_stages, 0,
> + (stage_id + 1) * mode->hdisplay / num_stages,
> + mode->vdisplay};
Hi Jun,
Nit: can you specify the fields when initializing the drm_rect struct here?
e.g.:
mixer_rect = {
.x1 = ...,
.y1 = ...
...
}
> + int cfg_idx = stage_id * PIPES_PER_STAGE;
> +
> + pipe_cfg = &pstate->pipe_cfg[cfg_idx];
> + r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1];
> +
> + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
> + pipe_cfg->dst_rect = new_plane_state->dst;
> +
> + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT
> + " vs clip window " DRM_RECT_FMT "\n",
> + DRM_RECT_ARG(&pipe_cfg->src_rect),
> + DRM_RECT_ARG(&mixer_rect));
> +
> + /*
> + * If this plane does not fall into mixer rect, check next
> + * mixer rect.
> + */
> + 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));
> +
> + continue;
> }
>
> - *r_pipe_cfg = *pipe_cfg;
> - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
> - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
> - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
> - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> - } else {
> - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg));
> - }
> + pipe_cfg->dst_rect.x1 -= mixer_rect.x1;
> + pipe_cfg->dst_rect.x2 -= mixer_rect.x1;
> +
> + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n",
> + DRM_RECT_ARG(&pipe_cfg->src_rect), DRM_RECT_ARG(&pipe_cfg->dst_rect));
>
> - drm_rect_rotate_inv(&pipe_cfg->src_rect,
> - new_plane_state->fb->width, new_plane_state->fb->height,
> - new_plane_state->rotation);
> - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
> - drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
> - new_plane_state->fb->width, new_plane_state->fb->height,
> + /* Split wide rect into 2 rect */
> + if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
> + _dpu_plane_calc_clk(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;
> + }
> +
> + memcpy(r_pipe_cfg, pipe_cfg, sizeof(struct dpu_sw_pipe_cfg));
> + pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
> + pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
> + r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
> + r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> + DPU_DEBUG_PLANE(pdpu, "Split wide plane into:"
> + 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));
> + }
> +
> + drm_rect_rotate_inv(&pipe_cfg->src_rect,
> + new_plane_state->fb->width,
> + new_plane_state->fb->height,
> new_plane_state->rotation);
>
> + if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
> + drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
> + new_plane_state->fb->width,
> + new_plane_state->fb->height,
> + new_plane_state->rotation);
> + }
> +
> pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
>
> return 0;
> @@ -954,20 +1011,17 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> drm_atomic_get_new_plane_state(state, plane);
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> - struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> - struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> - int ret = 0;
> -
> - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> - &crtc_state->adjusted_mode,
> - new_plane_state);
> - if (ret)
> - return ret;
> + struct dpu_sw_pipe *pipe;
> + struct dpu_sw_pipe_cfg *pipe_cfg;
> + int ret = 0, i;
>
> - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
Any reason why this check was dropped?
Thanks,
Jessica Zhang
> - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
> + for (i = 0; i < PIPES_PER_PLANE; i++) {
> + pipe = &pstate->pipe[i];
> + pipe_cfg = &pstate->pipe_cfg[i];
> + if (!pipe->sspp)
> + continue;
> + DPU_DEBUG_PLANE(pdpu, "pipe %d is in use, validate it\n", i);
> + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> &crtc_state->adjusted_mode,
> new_plane_state);
> if (ret)
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case
2025-05-06 15:47 ` [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case Jun Nie
@ 2025-05-08 21:30 ` Jessica Zhang
0 siblings, 0 replies; 30+ messages in thread
From: Jessica Zhang @ 2025-05-08 21:30 UTC (permalink / raw)
To: Jun Nie, Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 5/6/2025 8:47 AM, Jun Nie wrote:
> To support high-resolution cases that exceed the width limitation of
> a pair of SSPPs, or scenarios that surpass the maximum MDP clock rate,
> additional pipes are necessary to enable parallel data processing
> within the SSPP width constraints and MDP clock rate.
>
> Request 4 mixers and 4 DSCs for high-resolution cases where both DSC
> and dual interfaces are enabled. More use cases can be incorporated
> later if quad-pipe capabilities are required.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 27 +++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 ++---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 28 ++++++++----------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
> 6 files changed, 35 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index a56d68de219910a827830293e8ff24f4cdee74e4..f22ca13b35e778fed3e6e02c581ef689a5544cfb 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];
>
> int rc = 0;
> int i;
> @@ -1296,6 +1296,7 @@ static struct msm_display_topology dpu_crtc_get_topology(
> struct drm_display_mode *mode = &crtc_state->adjusted_mode;
> struct msm_display_topology topology = {0};
> struct drm_encoder *drm_enc;
> + u32 num_rt_intf;
>
> drm_for_each_encoder_mask(drm_enc, crtc->dev, crtc_state->encoder_mask)
> dpu_encoder_update_topology(drm_enc, &topology, crtc_state->state,
> @@ -1309,11 +1310,14 @@ static struct msm_display_topology dpu_crtc_get_topology(
> * Dual display
> * 2 LM, 2 INTF ( Split display using 2 interfaces)
> *
> + * If DSC is enabled, try to use 4:4:2 topology if there is enough
> + * resource. Otherwise, use 2:2:2 topology.
> + *
> * Single display
> * 1 LM, 1 INTF
> * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> *
> - * If DSC is enabled, use 2 LMs for 2:2:1 topology
> + * If DSC is enabled, use 2:2:1 topology
> *
> * Add dspps to the reservation requirements if ctm is requested
> *
> @@ -1325,14 +1329,23 @@ static struct msm_display_topology dpu_crtc_get_topology(
> * (mode->hdisplay > MAX_HDISPLAY_SPLIT) check.
> */
>
> - if (topology.num_intf == 2 && !topology.cwb_enabled)
> - topology.num_lm = 2;
> - else if (topology.num_dsc == 2)
> + num_rt_intf = topology.num_intf;
> + if (topology.cwb_enabled)
> + num_rt_intf--;
> +
> + if (topology.num_dsc) {
> + if (dpu_kms->catalog->dsc_count >= num_rt_intf * 2)
> + topology.num_dsc = num_rt_intf * 2;
> + else
> + topology.num_dsc = num_rt_intf;
> + topology.num_lm = topology.num_dsc;
> + } else if (num_rt_intf == 2) {
> topology.num_lm = 2;
> - else if (dpu_kms->catalog->caps->has_3d_merge)
> + } else if (dpu_kms->catalog->caps->has_3d_merge) {
> topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
> - else
> + } else {
> topology.num_lm = 1;
> + }
>
> if (crtc_state->ctm)
> topology.num_dspp = topology.num_lm;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index 6eaba5696e8e6bd1246a9895c4c8714ca6589b10..455073c7025b0bcb970d8817f197d9bcacc6dca5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -210,7 +210,7 @@ struct dpu_crtc_state {
>
> bool bw_control;
> bool bw_split_vote;
> - struct drm_rect lm_bounds[CRTC_DUAL_MIXERS];
> + struct drm_rect lm_bounds[CRTC_QUAD_MIXERS];
>
> uint64_t input_fence_timeout_ns;
>
> @@ -218,10 +218,10 @@ struct dpu_crtc_state {
>
> /* HW Resources reserved for the crtc */
> u32 num_mixers;
> - struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
> + struct dpu_crtc_mixer mixers[CRTC_QUAD_MIXERS];
>
> u32 num_ctls;
> - struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
> + struct dpu_hw_ctl *hw_ctls[CRTC_QUAD_MIXERS];
>
> enum dpu_crtc_crc_source crc_source;
> int crc_frame_skip_count;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index a125a477404fdafee6495f6d3778160f14a01a96..bbb0873a891879722f2b02767fc48b7406de70f8 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
> #define MAX_CWB_PER_ENC 2
>
> #define IDLE_SHORT_TIMEOUT 1
> @@ -675,22 +675,12 @@ void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
>
> dsc = dpu_encoder_get_dsc_config(drm_enc);
>
> - /* We only support 2 DSC mode (with 2 LM and 1 INTF) */
> - if (dsc) {
> - /*
> - * Use 2 DSC encoders, 2 layer mixers and 1 or 2 interfaces
> - * when Display Stream Compression (DSC) is enabled,
> - * and when enough DSC blocks are available.
> - * This is power-optimal and can drive up to (including) 4k
> - * screens.
> - */
> - WARN(topology->num_intf > 2,
> - "DSC topology cannot support more than 2 interfaces\n");
> - if (topology->num_intf >= 2 || dpu_kms->catalog->dsc_count >= 2)
> - topology->num_dsc = 2;
> - else
> - topology->num_dsc = 1;
> - }
> + /*
> + * Set DSC number as 1 to mark the enabled status, will be adjusted
> + * in dpu_crtc_get_topology()
> + */
> + if (dsc)
> + topology->num_dsc = 1;
>
> connector = drm_atomic_get_new_connector_for_encoder(state, drm_enc);
> if (!connector)
> @@ -2176,8 +2166,8 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
> struct dpu_hw_mixer_cfg mixer;
> int i, num_lm;
> struct dpu_global_state *global_state;
> - struct dpu_hw_blk *hw_lm[2];
> - struct dpu_hw_mixer *hw_mixer[2];
> + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> + struct dpu_hw_mixer *hw_mixer[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
>
> memset(&mixer, 0, sizeof(mixer));
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index 61b22d9494546885db609efa156222792af73d2a..09395d7910ac87c035b65cf476350bf6c9619612 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -302,7 +302,7 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
>
> /* Use merge_3d unless DSC MERGE topology is used */
> if (phys_enc->split_role == ENC_ROLE_SOLO &&
> - dpu_cstate->num_mixers == CRTC_DUAL_MIXERS &&
> + (dpu_cstate->num_mixers != 1) &&
> !dpu_encoder_use_dsc_merge(phys_enc->parent))
> return BLEND_3D_H_ROW_INT;
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 4cea19e1a20380c56ae014f2d33a6884a72e0ca0..77a7a5375d545483edb316e8428df12212191362 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -24,7 +24,7 @@
> #define DPU_MAX_IMG_WIDTH 0x3fff
> #define DPU_MAX_IMG_HEIGHT 0x3fff
>
> -#define CRTC_DUAL_MIXERS 2
> +#define CRTC_QUAD_MIXERS 4
>
> #define MAX_XIN_COUNT 16
>
> 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 7c74221380b2c05225c9f82ed6d33765042aec78..94f117e36af83602e5254ebcf5f28de36fd79886 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
> #define PIPES_PER_STAGE 2
> #define PIPES_PER_PLANE (PIPES_PER_STAGE * STAGES_PER_PLANE)
> #ifndef DPU_MAX_DE_CURVES
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane
2025-05-08 10:47 ` Dmitry Baryshkov
@ 2025-05-09 3:08 ` Jun Nie
2025-05-09 15:18 ` Dmitry Baryshkov
0 siblings, 1 reply; 30+ messages in thread
From: Jun Nie @ 2025-05-09 3:08 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, linux-arm-msm, dri-devel,
freedreno, linux-kernel
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 于2025年5月8日周四 18:47写道:
>
> On Tue, May 06, 2025 at 11:47:31PM +0800, Jun Nie wrote:
> > Some display controller support flexible CRTC and DMA, such as the display
> > controllers in snapdragon SoCs. CRTC can be implemented with several mixers
> > in parallel, and plane fetching can be implemented with several DMA under
> > umberala of a virtual drm plane.
> >
> > The mixer number is decided per panel resolution and clock rate constrain
> > first, which happens in CRTC side. Then plane is split per mixer number
> > and configure DMA accordingly.
>
> Here you are describing a behaviour of one particular driver as a reason
> to change the framework.
Yeah, the specific driver requires a change in framework. Maybe the
comment is not
proper?
>
> >
> > To support such forthcoming usage case, CRTC checking shall happen before
> > checking plane. Add the checking in the drm_atomic_helper_check_modeset().
>
> So, now drivers will get two calls to atomic_check(), one coming in
> circumstances which were not expected by the drivers before. Are you
> sure that this won't break anything?
Yes, it is a concern. Is there any way to limit the change in
framework to specific
driver with a flag, such as DRM_FLAG_CHECK_CRTC_BEFORE_PLANE?
>
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > index 5302ab3248985d3e0a47e40fd3deb7ad0d9f775b..5bca4c9683838c38574c8cb7c0bc9d57960314fe 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -816,6 +816,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> > return ret;
> > }
> >
> > + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> > + const struct drm_crtc_helper_funcs *funcs;
> > +
> > + funcs = crtc->helper_private;
> > +
> > + if (!funcs || !funcs->atomic_check)
> > + continue;
> > +
> > + ret = funcs->atomic_check(crtc, state);
> > + if (ret) {
> > + drm_dbg_atomic(crtc->dev,
> > + "[CRTC:%d:%s] atomic driver check failed\n",
> > + crtc->base.id, crtc->name);
> > + return ret;
> > + }
> > + }
> > +
> > +
> > +
>
> Too many empty lines. But the main quesiton is: why are you calling it
> before mode_valid()? According to your description a better place would
> be in drm_atomic_helper_check_planes().
>
Agree, that's the proper function. Will remove the empty line in next version.
> > ret = mode_valid(state);
> > if (ret)
> > return ret;
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane
2025-05-09 3:08 ` Jun Nie
@ 2025-05-09 15:18 ` Dmitry Baryshkov
0 siblings, 0 replies; 30+ messages in thread
From: Dmitry Baryshkov @ 2025-05-09 15:18 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Jessica Zhang, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, linux-arm-msm, dri-devel,
freedreno, linux-kernel
On 09/05/2025 06:08, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 于2025年5月8日周四 18:47写道:
>>
>> On Tue, May 06, 2025 at 11:47:31PM +0800, Jun Nie wrote:
>>> Some display controller support flexible CRTC and DMA, such as the display
>>> controllers in snapdragon SoCs. CRTC can be implemented with several mixers
>>> in parallel, and plane fetching can be implemented with several DMA under
>>> umberala of a virtual drm plane.
>>>
>>> The mixer number is decided per panel resolution and clock rate constrain
>>> first, which happens in CRTC side. Then plane is split per mixer number
>>> and configure DMA accordingly.
>>
>> Here you are describing a behaviour of one particular driver as a reason
>> to change the framework.
>
> Yeah, the specific driver requires a change in framework. Maybe the
> comment is not
> proper?
Yes. Explain how does that benefit the framework / other drivers.
Otherwise the answer would be as simple as 'replace
drm_atomic_helper_check_planes() in your driver'.
>>
>>>
>>> To support such forthcoming usage case, CRTC checking shall happen before
>>> checking plane. Add the checking in the drm_atomic_helper_check_modeset().
>>
>> So, now drivers will get two calls to atomic_check(), one coming in
>> circumstances which were not expected by the drivers before. Are you
>> sure that this won't break anything?
>
> Yes, it is a concern. Is there any way to limit the change in
> framework to specific
> driver with a flag, such as DRM_FLAG_CHECK_CRTC_BEFORE_PLANE?
Definitely not with a flag. You can try adding a new helper callback,
but I don't know how DRM core maintainers would react to it.
>>
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>> drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>> index 5302ab3248985d3e0a47e40fd3deb7ad0d9f775b..5bca4c9683838c38574c8cb7c0bc9d57960314fe 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -816,6 +816,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>> return ret;
>>> }
>>>
>>> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>>> + const struct drm_crtc_helper_funcs *funcs;
>>> +
>>> + funcs = crtc->helper_private;
>>> +
>>> + if (!funcs || !funcs->atomic_check)
>>> + continue;
>>> +
>>> + ret = funcs->atomic_check(crtc, state);
>>> + if (ret) {
>>> + drm_dbg_atomic(crtc->dev,
>>> + "[CRTC:%d:%s] atomic driver check failed\n",
>>> + crtc->base.id, crtc->name);
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> +
>>> +
>>
>> Too many empty lines. But the main quesiton is: why are you calling it
>> before mode_valid()? According to your description a better place would
>> be in drm_atomic_helper_check_planes().
>>
> Agree, that's the proper function. Will remove the empty line in next version.
>
>>> ret = mode_valid(state);
>>> if (ret)
>>> return ret;
>>>
>>> --
>>> 2.34.1
>>>
>>
>> --
>> With best wishes
>> Dmitry
>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer
2025-05-08 20:55 ` Jessica Zhang
@ 2025-05-23 10:03 ` Jun Nie
0 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-23 10:03 UTC (permalink / raw)
To: Jessica Zhang
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov, linux-arm-msm, dri-devel,
freedreno, linux-kernel
Jessica Zhang <quic_jesszhan@quicinc.com> 于2025年5月9日周五 04:55写道:
>
>
>
> On 5/8/2025 1:42 PM, Jessica Zhang wrote:
> >
> >
> > On 5/6/2025 8:47 AM, Jun Nie wrote:
> >> The stage contains configuration for a mixer pair. Currently the plane
> >> supports just one stage and 2 pipes. Quad-pipe support will require
> >> handling 2 stages and 4 pipes at the same time. In preparation for that
> >> add a separate define, PIPES_PER_PLANE, to denote number of pipes that
> >> can be used by the plane.
> >>
> >> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Hi Jun,
> >
> > I think the comment from v7 about propogating the PIPES_PER_PLANE change
> > to _dpu_plane_color_fill() got dropped in this version [1].
>
> s/propogating/propagating/
>
> >
> > Also, any reason PIPES_PER_STAGE was kept for
> > dpu_plane_danger_signal_ctrl()?
>
> I have the same question for _dpu_debugfs_status_show(). It seems to me
> both helpers should be using PIPES_PER_PLANE.
>
> Thanks,
>
> Jessica Zhang
>
Thanks for pointing this out! They both should be changed to PIPES_PER_PLANE.
Will fix it in next version.
Jun
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v9 13/14] drm/msm/dpu: support plane splitting in quad-pipe case
2025-05-08 21:24 ` Jessica Zhang
@ 2025-05-23 10:21 ` Jun Nie
0 siblings, 0 replies; 30+ messages in thread
From: Jun Nie @ 2025-05-23 10:21 UTC (permalink / raw)
To: Jessica Zhang
Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Dmitry Baryshkov, linux-arm-msm, dri-devel,
freedreno, linux-kernel
Jessica Zhang <quic_jesszhan@quicinc.com> 于2025年5月9日周五 05:24写道:
>
>
>
> On 5/6/2025 8:47 AM, Jun Nie wrote:
> > 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@linaro.org>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.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 | 134 +++++++++++++++++++++---------
> > 3 files changed, 107 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 f35cb1f7a7d2c2c63b4228bc47b85bb57cddbe6b..a56d68de219910a827830293e8ff24f4cdee74e4 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -1560,6 +1560,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 de3f52d743e1d1f11ae8721a316b9872d4139069..cf2b4d5cb0ccc144c2cf8fd227c862b0b6e7725f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -831,8 +831,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;
> > @@ -855,13 +859,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;
> > @@ -886,35 +887,91 @@ 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;
> > + /* iterate mixer configs for this plane, to separate left/right with the id */
> > + for (stage_id = 0; stage_id < num_stages; stage_id++) {
> > + struct drm_rect mixer_rect = {stage_id * mode->hdisplay / num_stages, 0,
> > + (stage_id + 1) * mode->hdisplay / num_stages,
> > + mode->vdisplay};
>
> Hi Jun,
>
> Nit: can you specify the fields when initializing the drm_rect struct here?
>
> e.g.:
>
> mixer_rect = {
> .x1 = ...,
> .y1 = ...
> ...
> }
>
Will do.
> > + int cfg_idx = stage_id * PIPES_PER_STAGE;
> > +
> > + pipe_cfg = &pstate->pipe_cfg[cfg_idx];
> > + r_pipe_cfg = &pstate->pipe_cfg[cfg_idx + 1];
> > +
> > + drm_rect_fp_to_int(&pipe_cfg->src_rect, &new_plane_state->src);
> > + pipe_cfg->dst_rect = new_plane_state->dst;
> > +
> > + DPU_DEBUG_PLANE(pdpu, "checking src " DRM_RECT_FMT
> > + " vs clip window " DRM_RECT_FMT "\n",
> > + DRM_RECT_ARG(&pipe_cfg->src_rect),
> > + DRM_RECT_ARG(&mixer_rect));
> > +
> > + /*
> > + * If this plane does not fall into mixer rect, check next
> > + * mixer rect.
> > + */
> > + 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));
> > +
> > + continue;
> > }
> >
> > - *r_pipe_cfg = *pipe_cfg;
> > - pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
> > - pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
> > - r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
> > - r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> > - } else {
> > - memset(r_pipe_cfg, 0, sizeof(*r_pipe_cfg));
> > - }
> > + pipe_cfg->dst_rect.x1 -= mixer_rect.x1;
> > + pipe_cfg->dst_rect.x2 -= mixer_rect.x1;
> > +
> > + DPU_DEBUG_PLANE(pdpu, "Got clip src:" DRM_RECT_FMT " dst: " DRM_RECT_FMT "\n",
> > + DRM_RECT_ARG(&pipe_cfg->src_rect), DRM_RECT_ARG(&pipe_cfg->dst_rect));
> >
> > - drm_rect_rotate_inv(&pipe_cfg->src_rect,
> > - new_plane_state->fb->width, new_plane_state->fb->height,
> > - new_plane_state->rotation);
> > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
> > - drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
> > - new_plane_state->fb->width, new_plane_state->fb->height,
> > + /* Split wide rect into 2 rect */
> > + if ((drm_rect_width(&pipe_cfg->src_rect) > max_linewidth) ||
> > + _dpu_plane_calc_clk(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;
> > + }
> > +
> > + memcpy(r_pipe_cfg, pipe_cfg, sizeof(struct dpu_sw_pipe_cfg));
> > + pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 + pipe_cfg->src_rect.x2) >> 1;
> > + pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 + pipe_cfg->dst_rect.x2) >> 1;
> > + r_pipe_cfg->src_rect.x1 = pipe_cfg->src_rect.x2;
> > + r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
> > + DPU_DEBUG_PLANE(pdpu, "Split wide plane into:"
> > + 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));
> > + }
> > +
> > + drm_rect_rotate_inv(&pipe_cfg->src_rect,
> > + new_plane_state->fb->width,
> > + new_plane_state->fb->height,
> > new_plane_state->rotation);
> >
> > + if (drm_rect_width(&r_pipe_cfg->src_rect) != 0)
> > + drm_rect_rotate_inv(&r_pipe_cfg->src_rect,
> > + new_plane_state->fb->width,
> > + new_plane_state->fb->height,
> > + new_plane_state->rotation);
> > + }
> > +
> > pstate->needs_qos_remap = drm_atomic_crtc_needs_modeset(crtc_state);
> >
> > return 0;
> > @@ -954,20 +1011,17 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> > drm_atomic_get_new_plane_state(state, plane);
> > struct dpu_plane *pdpu = to_dpu_plane(plane);
> > struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> > - struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> > - struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> > - int ret = 0;
> > -
> > - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> > - &crtc_state->adjusted_mode,
> > - new_plane_state);
> > - if (ret)
> > - return ret;
> > + struct dpu_sw_pipe *pipe;
> > + struct dpu_sw_pipe_cfg *pipe_cfg;
> > + int ret = 0, i;
> >
> > - if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
>
> Any reason why this check was dropped?
>
> Thanks,
>
> Jessica Zhang
Because we have a pipe array now and will loop on it, not just the 2nd pipe.
It is converted to below 2 lines, as sspp is assigned only for valid pipe.
+ if (!pipe->sspp)
+ continue;
>
> > - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
> > + for (i = 0; i < PIPES_PER_PLANE; i++) {
> > + pipe = &pstate->pipe[i];
> > + pipe_cfg = &pstate->pipe_cfg[i];
> > + if (!pipe->sspp)
> > + continue;
> > + DPU_DEBUG_PLANE(pdpu, "pipe %d is in use, validate it\n", i);
> > + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> > &crtc_state->adjusted_mode,
> > new_plane_state);
> > if (ret)
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2025-05-23 10:21 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-06 15:47 [PATCH v9 00/14] drm/msm/dpu: Support quad pipe with dual-DSI Jun Nie
2025-05-06 15:47 ` [PATCH v9 01/14] drm/atomic-helper: Add crtc check before checking plane Jun Nie
2025-05-08 10:47 ` Dmitry Baryshkov
2025-05-09 3:08 ` Jun Nie
2025-05-09 15:18 ` Dmitry Baryshkov
2025-05-06 15:47 ` [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability Jun Nie
2025-05-08 18:38 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 03/14] drm/msm/dpu: polish log for resource allocation Jun Nie
2025-05-06 15:47 ` [PATCH v9 04/14] drm/msm/dpu: decide right side per last bit Jun Nie
2025-05-06 15:47 ` [PATCH v9 05/14] drm/msm/dpu: fix mixer number counter on allocation Jun Nie
2025-05-06 15:47 ` [PATCH v9 06/14] drm/msm/dpu: bind correct pingpong for quad pipe Jun Nie
2025-05-06 15:47 ` [PATCH v9 07/14] drm/msm/dpu: Add pipe as trace argument Jun Nie
2025-05-06 15:47 ` [PATCH v9 08/14] drm/msm/dpu: handle pipes as array Jun Nie
2025-05-08 18:00 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 09/14] drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer Jun Nie
2025-05-08 20:42 ` Jessica Zhang
2025-05-08 20:55 ` Jessica Zhang
2025-05-23 10:03 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 10/14] drm/msm/dpu: Use dedicated WB number definition Jun Nie
2025-05-08 13:10 ` Dmitry Baryshkov
2025-05-08 18:03 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 11/14] drm/msm/dpu: blend pipes per mixer pairs config Jun Nie
2025-05-08 20:53 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 12/14] drm/msm/dpu: support SSPP assignment for quad-pipe case Jun Nie
2025-05-08 18:51 ` Jessica Zhang
2025-05-06 15:47 ` [PATCH v9 13/14] drm/msm/dpu: support plane splitting in " Jun Nie
2025-05-08 21:24 ` Jessica Zhang
2025-05-23 10:21 ` Jun Nie
2025-05-06 15:47 ` [PATCH v9 14/14] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case Jun Nie
2025-05-08 21:30 ` Jessica Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®