Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
enabled. We prefer to use 4 pipes for dual DSI case for it is power optimal
for DSC.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
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 | 3 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
7 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index bad75af4e50ab..3c51c199f3e05 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;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index d1bb3f84fe440..ce41fb364f3db 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 96d06db3e4be5..6e54ddeaffacd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -54,7 +54,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 IDLE_SHORT_TIMEOUT 1
@@ -664,15 +664,19 @@ static struct msm_display_topology dpu_encoder_get_topology(
/* Datapath topology selection
*
- * Dual display
+ * Dual display without DSC
* 2 LM, 2 INTF ( Split display using 2 interfaces)
*
+ * Dual display with DSC
+ * 4 LM, 2 INTF ( Split display using 2 interfaces)
+ *
* Single display
* 1 LM, 1 INTF
* 2 LM, 1 INTF (stream merge to support high resolution interfaces)
*
* Add dspps to the reservation requirements if ctm is requested
*/
+
if (intf_count == 2)
topology.num_lm = 2;
else if (!dpu_kms->catalog->caps->has_3d_merge)
@@ -691,10 +695,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
* 2 DSC encoders, 2 layer mixers and 1 interface
* this is power optimal and can drive up to (including) 4k
* screens
+ * But for dual display case, we prefer 4 layer mixers. Because
+ * the resolution is always high in the case and 4 DSCs are more
+ * power optimal.
*/
- topology.num_dsc = 2;
- topology.num_lm = 2;
- topology.num_intf = 1;
+
+ if (intf_count == 2) {
+ topology.num_dsc = 4;
+ topology.num_lm = 4;
+ topology.num_intf = 2;
+ } else {
+ topology.num_dsc = 2;
+ topology.num_lm = 2;
+ topology.num_intf = 1;
+ }
}
return topology;
@@ -2195,8 +2209,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 63f09857025c2..d378a990cc0fb 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,8 @@ 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 == CRTC_DUAL_MIXERS ||
+ dpu_cstate->num_mixers == CRTC_QUAD_MIXERS) &&
!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 3ab79092a7f25..d9cc84b081b1f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -25,6 +25,7 @@
#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 27ef0771da5d2..1fe21087a141a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -33,8 +33,8 @@
#endif
#define STAGES_PER_PLANE 2
-#define PIPES_PER_PLANE 2
#define PIPES_PER_STAGE 2
+#define PIPES_PER_PLANE (STAGES_PER_PLANE * STAGES_PER_PLANE)
#ifndef DPU_MAX_DE_CURVES
#define DPU_MAX_DE_CURVES 3
#endif
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 57ccb73c45683..b5c1ad2a75594 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
trace_dpu_plane_disable(DRMID(plane), false,
pstate->pipe[i].multirect_mode);
- if (pipe->sspp && i == 1) {
+ if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
pipe->multirect_index = DPU_SSPP_RECT_SOLO;
pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
--
2.34.1On Thu, Dec 19, 2024 at 03:49:33PM +0800, Jun Nie wrote:
> Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
> enabled. We prefer to use 4 pipes for dual DSI case for it is power optimal
> for DSC.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> 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 | 3 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
> 7 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index bad75af4e50ab..3c51c199f3e05 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;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index d1bb3f84fe440..ce41fb364f3db 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 96d06db3e4be5..6e54ddeaffacd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -54,7 +54,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 IDLE_SHORT_TIMEOUT 1
>
> @@ -664,15 +664,19 @@ static struct msm_display_topology dpu_encoder_get_topology(
>
> /* Datapath topology selection
> *
> - * Dual display
> + * Dual display without DSC
> * 2 LM, 2 INTF ( Split display using 2 interfaces)
> *
> + * Dual display with DSC
> + * 4 LM, 2 INTF ( Split display using 2 interfaces)
> + *
> * Single display
> * 1 LM, 1 INTF
> * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> *
> * Add dspps to the reservation requirements if ctm is requested
> */
> +
> if (intf_count == 2)
> topology.num_lm = 2;
> else if (!dpu_kms->catalog->caps->has_3d_merge)
> @@ -691,10 +695,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> * 2 DSC encoders, 2 layer mixers and 1 interface
> * this is power optimal and can drive up to (including) 4k
> * screens
> + * But for dual display case, we prefer 4 layer mixers. Because
> + * the resolution is always high in the case and 4 DSCs are more
> + * power optimal.
> */
> - topology.num_dsc = 2;
> - topology.num_lm = 2;
> - topology.num_intf = 1;
> +
> + if (intf_count == 2) {
> + topology.num_dsc = 4;
> + topology.num_lm = 4;
> + topology.num_intf = 2;
> + } else {
> + topology.num_dsc = 2;
> + topology.num_lm = 2;
> + topology.num_intf = 1;
Why is it only enabled for the DSC case? Also I'd like to point out
platforms like sm7150 or msm8998 which have only 2 DSC blocks. The
condition here needs more work to work with those platforms too.
> + }
> }
>
> return topology;
> @@ -2195,8 +2209,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 63f09857025c2..d378a990cc0fb 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,8 @@ 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 == CRTC_DUAL_MIXERS ||
> + dpu_cstate->num_mixers == CRTC_QUAD_MIXERS) &&
Misaligned. Also isn't it enough to check that 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 3ab79092a7f25..d9cc84b081b1f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -25,6 +25,7 @@
> #define DPU_MAX_IMG_HEIGHT 0x3fff
>
> #define CRTC_DUAL_MIXERS 2
Do we still need this define?
> +#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 27ef0771da5d2..1fe21087a141a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -33,8 +33,8 @@
> #endif
>
> #define STAGES_PER_PLANE 2
> -#define PIPES_PER_PLANE 2
> #define PIPES_PER_STAGE 2
> +#define PIPES_PER_PLANE (STAGES_PER_PLANE * STAGES_PER_PLANE)
This is incorrect.
> #ifndef DPU_MAX_DE_CURVES
> #define DPU_MAX_DE_CURVES 3
> #endif
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 57ccb73c45683..b5c1ad2a75594 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> trace_dpu_plane_disable(DRMID(plane), false,
> pstate->pipe[i].multirect_mode);
>
> - if (pipe->sspp && i == 1) {
> + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
Separate change, please. Also I'm not sure how does that work with the
shared SSPP case that I pointed to in one of the previous replies.
> pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年12月20日周五 07:46写道：
>
> On Thu, Dec 19, 2024 at 03:49:33PM +0800, Jun Nie wrote:
> > Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
> > enabled. We prefer to use 4 pipes for dual DSI case for it is power optimal
> > for DSC.
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> > 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 | 3 ++-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
> > 7 files changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index bad75af4e50ab..3c51c199f3e05 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;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > index d1bb3f84fe440..ce41fb364f3db 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 96d06db3e4be5..6e54ddeaffacd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -54,7 +54,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 IDLE_SHORT_TIMEOUT 1
> >
> > @@ -664,15 +664,19 @@ static struct msm_display_topology dpu_encoder_get_topology(
> >
> > /* Datapath topology selection
> > *
> > - * Dual display
> > + * Dual display without DSC
> > * 2 LM, 2 INTF ( Split display using 2 interfaces)
> > *
> > + * Dual display with DSC
> > + * 4 LM, 2 INTF ( Split display using 2 interfaces)
> > + *
> > * Single display
> > * 1 LM, 1 INTF
> > * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> > *
> > * Add dspps to the reservation requirements if ctm is requested
> > */
> > +
> > if (intf_count == 2)
> > topology.num_lm = 2;
> > else if (!dpu_kms->catalog->caps->has_3d_merge)
> > @@ -691,10 +695,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> > * 2 DSC encoders, 2 layer mixers and 1 interface
> > * this is power optimal and can drive up to (including) 4k
> > * screens
> > + * But for dual display case, we prefer 4 layer mixers. Because
> > + * the resolution is always high in the case and 4 DSCs are more
> > + * power optimal.
> > */
> > - topology.num_dsc = 2;
> > - topology.num_lm = 2;
> > - topology.num_intf = 1;
> > +
> > + if (intf_count == 2) {
> > + topology.num_dsc = 4;
> > + topology.num_lm = 4;
> > + topology.num_intf = 2;
> > + } else {
> > + topology.num_dsc = 2;
> > + topology.num_lm = 2;
> > + topology.num_intf = 1;
>
> Why is it only enabled for the DSC case? Also I'd like to point out
> platforms like sm7150 or msm8998 which have only 2 DSC blocks. The
> condition here needs more work to work with those platforms too.
Because the DSC + quad-pipe is assumed with wide LCD case that dual
pipe can not handle due to width limitation. If DSC is not involved, the
resolution is not too big to involve DSC together with 2 interfaces. Do you
think there is need to support quad-pipe without DSC?
Of course, it is a valid case to use 2 DSC with 2 interfaces. Below logic shall
be better.
if (intf_count == 2) {
topology.num_dsc = dpu_kms->catalog->dsc_count >= 4 ? 4 : 2;
topology.num_lm = topology.num_dsc;
topology.num_intf = 2;
} else {
topology.num_dsc = 2;
topology.num_lm = 2;
topology.num_intf = 1;
}
>
> > + }
> > }
> >
> > return topology;
> > @@ -2195,8 +2209,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 63f09857025c2..d378a990cc0fb 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,8 @@ 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 == CRTC_DUAL_MIXERS ||
> > + dpu_cstate->num_mixers == CRTC_QUAD_MIXERS) &&
>
> Misaligned. Also isn't it enough to check that num_mixers != 1?
Yeah, num_mixers != 1 should work.
>
> > !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 3ab79092a7f25..d9cc84b081b1f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > @@ -25,6 +25,7 @@
> > #define DPU_MAX_IMG_HEIGHT 0x3fff
> >
> > #define CRTC_DUAL_MIXERS 2
>
> Do we still need this define?
>
> > +#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 27ef0771da5d2..1fe21087a141a 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > @@ -33,8 +33,8 @@
> > #endif
> >
> > #define STAGES_PER_PLANE 2
> > -#define PIPES_PER_PLANE 2
> > #define PIPES_PER_STAGE 2
> > +#define PIPES_PER_PLANE (STAGES_PER_PLANE * STAGES_PER_PLANE)
>
> This is incorrect.
>
> > #ifndef DPU_MAX_DE_CURVES
> > #define DPU_MAX_DE_CURVES 3
> > #endif
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 57ccb73c45683..b5c1ad2a75594 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> > trace_dpu_plane_disable(DRMID(plane), false,
> > pstate->pipe[i].multirect_mode);
> >
> > - if (pipe->sspp && i == 1) {
> > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
>
> Separate change, please. Also I'm not sure how does that work with the
> shared SSPP case that I pointed to in one of the previous replies.
Maybe we can add a peer member in the pipe to reference each other, then we have
chance to use multirect across all pipes in all planes.
>
> > pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> >
> >
> > --
> > 2.34.1
> >
>
> --
> With best wishes
> Dmitry
On Fri, Jan 03, 2025 at 11:49:07PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年12月20日周五 07:46写道：
> >
> > On Thu, Dec 19, 2024 at 03:49:33PM +0800, Jun Nie wrote:
> > > Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
> > > enabled. We prefer to use 4 pipes for dual DSI case for it is power optimal
> > > for DSC.
> > >
> > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> > > 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 | 3 ++-
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
> > > 7 files changed, 30 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index bad75af4e50ab..3c51c199f3e05 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;
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > index d1bb3f84fe440..ce41fb364f3db 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 96d06db3e4be5..6e54ddeaffacd 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -54,7 +54,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 IDLE_SHORT_TIMEOUT 1
> > >
> > > @@ -664,15 +664,19 @@ static struct msm_display_topology dpu_encoder_get_topology(
> > >
> > > /* Datapath topology selection
> > > *
> > > - * Dual display
> > > + * Dual display without DSC
> > > * 2 LM, 2 INTF ( Split display using 2 interfaces)
> > > *
> > > + * Dual display with DSC
> > > + * 4 LM, 2 INTF ( Split display using 2 interfaces)
> > > + *
> > > * Single display
> > > * 1 LM, 1 INTF
> > > * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> > > *
> > > * Add dspps to the reservation requirements if ctm is requested
> > > */
> > > +
> > > if (intf_count == 2)
> > > topology.num_lm = 2;
> > > else if (!dpu_kms->catalog->caps->has_3d_merge)
> > > @@ -691,10 +695,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> > > * 2 DSC encoders, 2 layer mixers and 1 interface
> > > * this is power optimal and can drive up to (including) 4k
> > > * screens
> > > + * But for dual display case, we prefer 4 layer mixers. Because
> > > + * the resolution is always high in the case and 4 DSCs are more
> > > + * power optimal.
> > > */
> > > - topology.num_dsc = 2;
> > > - topology.num_lm = 2;
> > > - topology.num_intf = 1;
> > > +
> > > + if (intf_count == 2) {
> > > + topology.num_dsc = 4;
> > > + topology.num_lm = 4;
> > > + topology.num_intf = 2;
> > > + } else {
> > > + topology.num_dsc = 2;
> > > + topology.num_lm = 2;
> > > + topology.num_intf = 1;
> >
> > Why is it only enabled for the DSC case? Also I'd like to point out
> > platforms like sm7150 or msm8998 which have only 2 DSC blocks. The
> > condition here needs more work to work with those platforms too.
>
> Because the DSC + quad-pipe is assumed with wide LCD case that dual
> pipe can not handle due to width limitation. If DSC is not involved, the
> resolution is not too big to involve DSC together with 2 interfaces. Do you
> think there is need to support quad-pipe without DSC?
Yes, of course. The same logic: ultra wide resolutions. If two LMs are
not enough for the panel / monitor.
>
> Of course, it is a valid case to use 2 DSC with 2 interfaces. Below logic shall
> be better.
>
> if (intf_count == 2) {
> topology.num_dsc = dpu_kms->catalog->dsc_count >= 4 ? 4 : 2;
> topology.num_lm = topology.num_dsc;
> topology.num_intf = 2;
> } else {
> topology.num_dsc = 2;
> topology.num_lm = 2;
> topology.num_intf = 1;
> }
It all starts to look like a collection of use-case quirks. But indeed,
it should work.
>
> >
> > > + }
> > > }
> > >
> > > return topology;
> > > @@ -2195,8 +2209,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 63f09857025c2..d378a990cc0fb 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,8 @@ 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 == CRTC_DUAL_MIXERS ||
> > > + dpu_cstate->num_mixers == CRTC_QUAD_MIXERS) &&
> >
> > Misaligned. Also isn't it enough to check that num_mixers != 1?
>
> Yeah, num_mixers != 1 should work.
> >
> > > !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 3ab79092a7f25..d9cc84b081b1f 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > @@ -25,6 +25,7 @@
> > > #define DPU_MAX_IMG_HEIGHT 0x3fff
> > >
> > > #define CRTC_DUAL_MIXERS 2
> >
> > Do we still need this define?
> >
> > > +#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 27ef0771da5d2..1fe21087a141a 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > > @@ -33,8 +33,8 @@
> > > #endif
> > >
> > > #define STAGES_PER_PLANE 2
> > > -#define PIPES_PER_PLANE 2
> > > #define PIPES_PER_STAGE 2
> > > +#define PIPES_PER_PLANE (STAGES_PER_PLANE * STAGES_PER_PLANE)
> >
> > This is incorrect.
> >
> > > #ifndef DPU_MAX_DE_CURVES
> > > #define DPU_MAX_DE_CURVES 3
> > > #endif
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > index 57ccb73c45683..b5c1ad2a75594 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > @@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> > > trace_dpu_plane_disable(DRMID(plane), false,
> > > pstate->pipe[i].multirect_mode);
> > >
> > > - if (pipe->sspp && i == 1) {
> > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
> >
> > Separate change, please. Also I'm not sure how does that work with the
> > shared SSPP case that I pointed to in one of the previous replies.
>
> Maybe we can add a peer member in the pipe to reference each other, then we have
> chance to use multirect across all pipes in all planes.
I'd rather not. We have pairs of pipes. I'd rather see the code stay the
same way: processing one pair at the same time.
>
>
> >
> > > pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > >
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年1月4日周六 01:51写道：
>
> On Fri, Jan 03, 2025 at 11:49:07PM +0800, Jun Nie wrote:
> > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年12月20日周五 07:46写道：
> > >
> > > On Thu, Dec 19, 2024 at 03:49:33PM +0800, Jun Nie wrote:
> > > > Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
> > > > enabled. We prefer to use 4 pipes for dual DSI case for it is power optimal
> > > > for DSC.
> > > >
> > > > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > > > ---
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> > > > 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 | 3 ++-
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 1 +
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 2 +-
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +-
> > > > 7 files changed, 30 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > index bad75af4e50ab..3c51c199f3e05 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;
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > > index d1bb3f84fe440..ce41fb364f3db 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 96d06db3e4be5..6e54ddeaffacd 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > > @@ -54,7 +54,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 IDLE_SHORT_TIMEOUT 1
> > > >
> > > > @@ -664,15 +664,19 @@ static struct msm_display_topology dpu_encoder_get_topology(
> > > >
> > > > /* Datapath topology selection
> > > > *
> > > > - * Dual display
> > > > + * Dual display without DSC
> > > > * 2 LM, 2 INTF ( Split display using 2 interfaces)
> > > > *
> > > > + * Dual display with DSC
> > > > + * 4 LM, 2 INTF ( Split display using 2 interfaces)
> > > > + *
> > > > * Single display
> > > > * 1 LM, 1 INTF
> > > > * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> > > > *
> > > > * Add dspps to the reservation requirements if ctm is requested
> > > > */
> > > > +
> > > > if (intf_count == 2)
> > > > topology.num_lm = 2;
> > > > else if (!dpu_kms->catalog->caps->has_3d_merge)
> > > > @@ -691,10 +695,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> > > > * 2 DSC encoders, 2 layer mixers and 1 interface
> > > > * this is power optimal and can drive up to (including) 4k
> > > > * screens
> > > > + * But for dual display case, we prefer 4 layer mixers. Because
> > > > + * the resolution is always high in the case and 4 DSCs are more
> > > > + * power optimal.
> > > > */
> > > > - topology.num_dsc = 2;
> > > > - topology.num_lm = 2;
> > > > - topology.num_intf = 1;
> > > > +
> > > > + if (intf_count == 2) {
> > > > + topology.num_dsc = 4;
> > > > + topology.num_lm = 4;
> > > > + topology.num_intf = 2;
> > > > + } else {
> > > > + topology.num_dsc = 2;
> > > > + topology.num_lm = 2;
> > > > + topology.num_intf = 1;
> > >
> > > Why is it only enabled for the DSC case? Also I'd like to point out
> > > platforms like sm7150 or msm8998 which have only 2 DSC blocks. The
> > > condition here needs more work to work with those platforms too.
> >
> > Because the DSC + quad-pipe is assumed with wide LCD case that dual
> > pipe can not handle due to width limitation. If DSC is not involved, the
> > resolution is not too big to involve DSC together with 2 interfaces. Do you
> > think there is need to support quad-pipe without DSC?
>
> Yes, of course. The same logic: ultra wide resolutions. If two LMs are
> not enough for the panel / monitor.
>
> >
> > Of course, it is a valid case to use 2 DSC with 2 interfaces. Below logic shall
> > be better.
> >
> > if (intf_count == 2) {
> > topology.num_dsc = dpu_kms->catalog->dsc_count >= 4 ? 4 : 2;
> > topology.num_lm = topology.num_dsc;
> > topology.num_intf = 2;
> > } else {
> > topology.num_dsc = 2;
> > topology.num_lm = 2;
> > topology.num_intf = 1;
> > }
>
> It all starts to look like a collection of use-case quirks. But indeed,
> it should work.
>
> >
> > >
> > > > + }
> > > > }
> > > >
> > > > return topology;
> > > > @@ -2195,8 +2209,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 63f09857025c2..d378a990cc0fb 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,8 @@ 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 == CRTC_DUAL_MIXERS ||
> > > > + dpu_cstate->num_mixers == CRTC_QUAD_MIXERS) &&
> > >
> > > Misaligned. Also isn't it enough to check that num_mixers != 1?
> >
> > Yeah, num_mixers != 1 should work.
> > >
> > > > !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 3ab79092a7f25..d9cc84b081b1f 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > > @@ -25,6 +25,7 @@
> > > > #define DPU_MAX_IMG_HEIGHT 0x3fff
> > > >
> > > > #define CRTC_DUAL_MIXERS 2
> > >
> > > Do we still need this define?
> > >
> > > > +#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 27ef0771da5d2..1fe21087a141a 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> > > > @@ -33,8 +33,8 @@
> > > > #endif
> > > >
> > > > #define STAGES_PER_PLANE 2
> > > > -#define PIPES_PER_PLANE 2
> > > > #define PIPES_PER_STAGE 2
> > > > +#define PIPES_PER_PLANE (STAGES_PER_PLANE * STAGES_PER_PLANE)
> > >
> > > This is incorrect.
> > >
> > > > #ifndef DPU_MAX_DE_CURVES
> > > > #define DPU_MAX_DE_CURVES 3
> > > > #endif
> > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > index 57ccb73c45683..b5c1ad2a75594 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > @@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> > > > trace_dpu_plane_disable(DRMID(plane), false,
> > > > pstate->pipe[i].multirect_mode);
> > > >
> > > > - if (pipe->sspp && i == 1) {
> > > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
> > >
> > > Separate change, please. Also I'm not sure how does that work with the
> > > shared SSPP case that I pointed to in one of the previous replies.
> >
> > Maybe we can add a peer member in the pipe to reference each other, then we have
> > chance to use multirect across all pipes in all planes.
>
> I'd rather not. We have pairs of pipes. I'd rather see the code stay the
> same way: processing one pair at the same time.
I mean only use the peer only when the SSPP multi-rect pips cross
planes. This shall not change
too much to current SSPP management.
>
> >
> >
> > >
> > > > pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > > pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > >
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry
On Mon, Jan 06, 2025 at 04:21:43PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2025年1月4日周六 01:51写道：
> >
> > On Fri, Jan 03, 2025 at 11:49:07PM +0800, Jun Nie wrote:
> > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年12月20日周五 07:46写道：
> > > >
> > > > On Thu, Dec 19, 2024 at 03:49:33PM +0800, Jun Nie wrote:
> > > >
> > > > > #ifndef DPU_MAX_DE_CURVES
> > > > > #define DPU_MAX_DE_CURVES 3
> > > > > #endif
> > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > > index 57ccb73c45683..b5c1ad2a75594 100644
> > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > > @@ -1474,7 +1474,7 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> > > > > trace_dpu_plane_disable(DRMID(plane), false,
> > > > > pstate->pipe[i].multirect_mode);
> > > > >
> > > > > - if (pipe->sspp && i == 1) {
> > > > > + if (pipe->sspp && pipe->multirect_index == DPU_SSPP_RECT_1) {
> > > >
> > > > Separate change, please. Also I'm not sure how does that work with the
> > > > shared SSPP case that I pointed to in one of the previous replies.
> > >
> > > Maybe we can add a peer member in the pipe to reference each other, then we have
> > > chance to use multirect across all pipes in all planes.
> >
> > I'd rather not. We have pairs of pipes. I'd rather see the code stay the
> > same way: processing one pair at the same time.
>
> I mean only use the peer only when the SSPP multi-rect pips cross
> planes. This shall not change
> too much to current SSPP management.
Still no. Please please don't add extra 'peer' member. There should be
no need to have one.
> >
> > >
> > >
> > > >
> > > > > pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > > > pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > > >
--
With best wishes
Dmitry
© 2016 - 2025 Red Hat, Inc.