| Message ID | 20241219-sm8650-v6-13-hmd-deckard-mdss-quad-upstream-32-v3-0-92c7c0a228e3@linaro.org |
|---|---|
| Headers | show |
| Series | (no cover subject) | expand |
On Thu, Dec 19, 2024 at 03:49:19PM +0800, Jun Nie wrote: > If DSC is enabled, the only case is with 2 DSC engines so far. More > usage case will be added, Can't parse this, excuse me. The patch LGTM. > such as 4 DSC in 4:4:2 topoplogy. > So get real number of DSCs to decide whether DSC merge is needed. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) >
On Thu, Dec 19, 2024 at 03:49:23PM +0800, Jun Nie wrote: > Add the case to reserve multiple pairs mixers for high resolution. You are not adding anything. > Current code only supports one pair of mixer usage case. To support > quad-pipe usage case, two pairs of mixers are needed. > > Current code resets number of mixer on failure of pair's peer test and > retry on another pair. If two pairs are needed, the failure on the test > of 2nd pair results clearing to the 1st pair. This patch only clear the > bit for the 2nd pair allocation before retry on another pair. > > Signed-off-by: Jun Nie <jun.nie@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 cde3c5616f9bc..a8b01b78c02c7 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > @@ -316,7 +316,11 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm, > if (!rm->mixer_blks[i]) > continue; > > - lm_count = 0; > + /* > + * Clear the last bit to drop the previous primary mixer if > + * fail to find its peer. if the driver failed > + */ > + lm_count &= 0xfe; > lm_idx[lm_count] = i; > > if (!_dpu_rm_check_lm_and_get_connected_blks(rm, global_state, > > -- > 2.34.1 >
On Thu, Dec 19, 2024 at 03:49:27PM +0800, Jun Nie wrote: > Split PIPES_PER_STAGE definition per plane and mixer pair. Because > there are more than 2 pipes in quad pipe case, while 2 pipes at most > per mixer pair. A stage struct serve a mixer pair, so pipes per > plane is split out as PIPES_PER_PLANE. I can barely understand this. The patch itself LGTM. > > Signed-off-by: Jun Nie <jun.nie@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 | 18 +++++++++--------- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 ++-- > 4 files changed, 14 insertions(+), 13 deletions(-) >
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. > + } > } > > 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 >
To: Rob Clark <robdclark@gmail.com> To: Abhinav Kumar <quic_abhinavk@quicinc.com> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> To: Sean Paul <sean@poorly.run> To: Marijn Suijten <marijn.suijten@somainline.org> To: David Airlie <airlied@gmail.com> To: Simona Vetter <simona@ffwll.ch> Cc: linux-arm-msm@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: freedreno@lists.freedesktop.org Cc: linux-kernel@vger.kernel.org drm/msm/dpu: Support quad pipe with dual-DSI 2 or more SSPPs and dual-DSI interface are need for super wide DSI panel. And 4 DSC are preferred for power optimal in this case. This patch set extend 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. This patch set depends on virtual plane patch set v7: https://lore.kernel.org/all/20241130-dpu-virtual-wide-v7-0-991053fcf63c@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 (15): drm/msm/dpu: Do not fix number of DSC drm/msm/dpu: configure DSC per number in use 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: switch RM to use crtc_id rather than enc_id for allocation drm/msm/dpu: bind correct pingpong for quad pipe drm/msm/dpu: handle pipes as array drm/msm/dpu: split PIPES_PER_STAGE definition per plane and mixer drm/msm/dpu: Add pipe as trace argument drm/msm/dpu: blend pipes per mixer pairs config drm/msm/dpu: support plane splitting in quad-pipe case drm/msm/dpu: Support quad-pipe in SSPP checking drm/msm/dpu: support SSPP assignment for quad-pipe case drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI case drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 77 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 74 +++-- 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_hw_sspp.h | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 403 ++++++++++++++--------- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 219 ++++++------ drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 32 +- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 +- 13 files changed, 511 insertions(+), 348 deletions(-) --- base-commit: a9b9ea7b45d661fff0f3fd2937703a536f528cd2 change-id: 20241219-sm8650-v6-13-hmd-deckard-mdss-quad-upstream-32-2bdbc22f5131 Best regards,