| Message ID | 20241009-sm8650-v6-11-hmd-pocf-mdss-quad-upstream-21-v2-9-76d4f5d413bf@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm/dpu: Support quad pipe with dual-DSI | expand |
On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > Blend pipes by set of mixer pair config. The first 2 pipes are for left > half screen with the first set of mixer pair config. And the later 2 pipes > are for right in quad pipe case. > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > + uint32_t lm_idx, lm_pair, i, pipe_idx; > bool bg_alpha_enable = false; > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > + if (!pstate->pipe[pipe_idx].sspp) > + continue; > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > + _dpu_crtc_blend_setup_pipe(crtc, plane, > + mixer, cstate->num_mixers, > + pstate->stage, > + format, fb ? fb->modifier : 0, > + &pstate->pipe[pipe_idx], i, > + &stage_cfg[lm_pair]); > + } > } > > /* blend config update */ > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; After seeing this code, can we define STAGES_PER_PLANE (and also keep PLANES_PER_STAGE defined to 2)? > int i; > > DRM_DEBUG_ATOMIC("%s\n", dpu_crtc->name); > @@ -516,9 +521,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[0]); > > for (i = 0; i < cstate->num_mixers; i++) { > ctl = mixer[i].lm_ctl; > @@ -535,8 +540,13 @@ 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. > + * There is 4 mixers at most. The first 2 are for the left half, and > + * the later 2 are for the right half. > + */ > ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx, > - &stage_cfg); > + &stage_cfg[i / 2]); > } > } > > 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 3277debe11689..d8f5cffa60ea6 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 PIPES_PER_PLANE 2 > #define PIPES_PER_LM_PAIR 2 > +#define LM_PAIRS_PER_PLANE 2 > #ifndef DPU_MAX_DE_CURVES > #define DPU_MAX_DE_CURVES 3 > #endif > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > half screen with the first set of mixer pair config. And the later 2 pipes > > are for right in quad pipe case. > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > bool bg_alpha_enable = false; > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > + if (!pstate->pipe[pipe_idx].sspp) > > + continue; > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > + mixer, cstate->num_mixers, > > + pstate->stage, > > + format, fb ? fb->modifier : 0, > > + &pstate->pipe[pipe_idx], i, > > + &stage_cfg[lm_pair]); > > + } > > } > > > > /* blend config update */ > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > After seeing this code, can we define STAGES_PER_PLANE (and > also keep PLANES_PER_STAGE defined to 2)? > Could you elaborate it? Stages describe how many layers to be blended. Plane is a DRM concept that describe a buffer to be display in specific display driver. Plane is already mapped to SSPP/multi-rect in DPU driver in blending stage level. So I am confused here. - Jun
On Fri, 11 Oct 2024 at 09:40, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > > half screen with the first set of mixer pair config. And the later 2 pipes > > > are for right in quad pipe case. > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > > bool bg_alpha_enable = false; > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > > + if (!pstate->pipe[pipe_idx].sspp) > > > + continue; > > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > > + mixer, cstate->num_mixers, > > > + pstate->stage, > > > + format, fb ? fb->modifier : 0, > > > + &pstate->pipe[pipe_idx], i, > > > + &stage_cfg[lm_pair]); > > > + } > > > } > > > > > > /* blend config update */ > > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > > > After seeing this code, can we define STAGES_PER_PLANE (and > > also keep PLANES_PER_STAGE defined to 2)? > > > Could you elaborate it? Stages describe how many layers to be blended. > Plane is a DRM concept that describe a buffer to be display in specific > display driver. Plane is already mapped to SSPP/multi-rect in DPU driver > in blending stage level. So I am confused here. We have dpu_hw_stage_cfg, you are adding a second instance of it. So we now have two stages per plane. > > - Jun
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:03写道： > > On Fri, 11 Oct 2024 at 09:40, Jun Nie <jun.nie@linaro.org> wrote: > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > > > > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > > > half screen with the first set of mixer pair config. And the later 2 pipes > > > > are for right in quad pipe case. > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > --- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > > > bool bg_alpha_enable = false; > > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > > > + if (!pstate->pipe[pipe_idx].sspp) > > > > + continue; > > > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > > > + mixer, cstate->num_mixers, > > > > + pstate->stage, > > > > + format, fb ? fb->modifier : 0, > > > > + &pstate->pipe[pipe_idx], i, > > > > + &stage_cfg[lm_pair]); > > > > + } > > > > } > > > > > > > > /* blend config update */ > > > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > > > > > After seeing this code, can we define STAGES_PER_PLANE (and > > > also keep PLANES_PER_STAGE defined to 2)? > > > > > Could you elaborate it? Stages describe how many layers to be blended. > > Plane is a DRM concept that describe a buffer to be display in specific > > display driver. Plane is already mapped to SSPP/multi-rect in DPU driver > > in blending stage level. So I am confused here. > > We have dpu_hw_stage_cfg, you are adding a second instance of it. So > we now have two stages per plane. So you suggest to replace LM_PAIRS_PER_PLANE with STAGES_PER_PLANE, right? I assume a stage is coupled with a LM pair. But for PLANES_PER_STAGE, I am still confused. A stage or a LM pair can involve many SSPP layers. How it related to planes? Plane is a concepts from higher level. > -- > With best wishes > Dmitry
On Fri, 11 Oct 2024 at 10:11, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:03写道： > > > > On Fri, 11 Oct 2024 at 09:40, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > > > > > > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > > > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > > > > half screen with the first set of mixer pair config. And the later 2 pipes > > > > > are for right in quad pipe case. > > > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > > --- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > > > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > > > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > > > > bool bg_alpha_enable = false; > > > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > > > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > > > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > > > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > > > > + if (!pstate->pipe[pipe_idx].sspp) > > > > > + continue; > > > > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > > > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > > > > + mixer, cstate->num_mixers, > > > > > + pstate->stage, > > > > > + format, fb ? fb->modifier : 0, > > > > > + &pstate->pipe[pipe_idx], i, > > > > > + &stage_cfg[lm_pair]); > > > > > + } > > > > > } > > > > > > > > > > /* blend config update */ > > > > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > > > > > > > After seeing this code, can we define STAGES_PER_PLANE (and > > > > also keep PLANES_PER_STAGE defined to 2)? > > > > > > > Could you elaborate it? Stages describe how many layers to be blended. > > > Plane is a DRM concept that describe a buffer to be display in specific > > > display driver. Plane is already mapped to SSPP/multi-rect in DPU driver > > > in blending stage level. So I am confused here. > > > > We have dpu_hw_stage_cfg, you are adding a second instance of it. So > > we now have two stages per plane. > > So you suggest to replace LM_PAIRS_PER_PLANE with STAGES_PER_PLANE, > right? I assume a stage is coupled with a LM pair. > > But for PLANES_PER_STAGE, I am still confused. A stage or a LM pair can > involve many SSPP layers. How it related to planes? Plane is a concepts from > higher level. PIPES_PER_STAGE, excuse me.
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:13写道： > > On Fri, 11 Oct 2024 at 10:11, Jun Nie <jun.nie@linaro.org> wrote: > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:03写道： > > > > > > On Fri, 11 Oct 2024 at 09:40, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > > > > > > > > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > > > > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > > > > > half screen with the first set of mixer pair config. And the later 2 pipes > > > > > > are for right in quad pipe case. > > > > > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > > > --- > > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > > > > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > > > > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > > > > > bool bg_alpha_enable = false; > > > > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > > > > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > > > > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > > > > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > > > > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > > > > > + if (!pstate->pipe[pipe_idx].sspp) > > > > > > + continue; > > > > > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > > > > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > > > > > + mixer, cstate->num_mixers, > > > > > > + pstate->stage, > > > > > > + format, fb ? fb->modifier : 0, > > > > > > + &pstate->pipe[pipe_idx], i, > > > > > > + &stage_cfg[lm_pair]); > > > > > > + } > > > > > > } > > > > > > > > > > > > /* blend config update */ > > > > > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > > > > > > > > > After seeing this code, can we define STAGES_PER_PLANE (and > > > > > also keep PLANES_PER_STAGE defined to 2)? > > > > > > > > > Could you elaborate it? Stages describe how many layers to be blended. > > > > Plane is a DRM concept that describe a buffer to be display in specific > > > > display driver. Plane is already mapped to SSPP/multi-rect in DPU driver > > > > in blending stage level. So I am confused here. > > > > > > We have dpu_hw_stage_cfg, you are adding a second instance of it. So > > > we now have two stages per plane. > > > > So you suggest to replace LM_PAIRS_PER_PLANE with STAGES_PER_PLANE, > > right? I assume a stage is coupled with a LM pair. > > > > But for PLANES_PER_STAGE, I am still confused. A stage or a LM pair can > > involve many SSPP layers. How it related to planes? Plane is a concepts from > > higher level. > > PIPES_PER_STAGE, excuse me. Do you mean to keep PIPES_PER_STAGE and do not introduce PIPES_PER_LM_PAIR, or use both? Looks like they are equal in hardware nature. A stage structure serves a mixer pair with 2 pipes. We can use PIPES_PER_LM_PAIR and add comment to indicate it, thus avoid defining too many macro. > > -- > With best wishes > Dmitry
On Tue, 15 Oct 2024 at 11:27, Jun Nie <jun.nie@linaro.org> wrote: > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:13写道： > > > > On Fri, 11 Oct 2024 at 10:11, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月11日周五 15:03写道： > > > > > > > > On Fri, 11 Oct 2024 at 09:40, Jun Nie <jun.nie@linaro.org> wrote: > > > > > > > > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年10月10日周四 21:15写道： > > > > > > > > > > > > On Wed, Oct 09, 2024 at 04:50:22PM GMT, Jun Nie wrote: > > > > > > > Blend pipes by set of mixer pair config. The first 2 pipes are for left > > > > > > > half screen with the first set of mixer pair config. And the later 2 pipes > > > > > > > are for right in quad pipe case. > > > > > > > > > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > > > > > --- > > > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- > > > > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + > > > > > > > 2 files changed, 25 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 43d9817cd858f..66f745399a602 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, i; > > > > > > > + uint32_t lm_idx, lm_pair, i, pipe_idx; > > > > > > > bool bg_alpha_enable = false; > > > > > > > DECLARE_BITMAP(fetch_active, SSPP_MAX); > > > > > > > > > > > > > > @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ > > > > > > > + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { > > > > > > > + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { > > > > > > > + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; > > > > > > > + if (!pstate->pipe[pipe_idx].sspp) > > > > > > > + continue; > > > > > > > + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); > > > > > > > + _dpu_crtc_blend_setup_pipe(crtc, plane, > > > > > > > + mixer, cstate->num_mixers, > > > > > > > + pstate->stage, > > > > > > > + format, fb ? fb->modifier : 0, > > > > > > > + &pstate->pipe[pipe_idx], i, > > > > > > > + &stage_cfg[lm_pair]); > > > > > > > + } > > > > > > > } > > > > > > > > > > > > > > /* blend config update */ > > > > > > > @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; > > > > > > > > > > > > After seeing this code, can we define STAGES_PER_PLANE (and > > > > > > also keep PLANES_PER_STAGE defined to 2)? > > > > > > > > > > > Could you elaborate it? Stages describe how many layers to be blended. > > > > > Plane is a DRM concept that describe a buffer to be display in specific > > > > > display driver. Plane is already mapped to SSPP/multi-rect in DPU driver > > > > > in blending stage level. So I am confused here. > > > > > > > > We have dpu_hw_stage_cfg, you are adding a second instance of it. So > > > > we now have two stages per plane. > > > > > > So you suggest to replace LM_PAIRS_PER_PLANE with STAGES_PER_PLANE, > > > right? I assume a stage is coupled with a LM pair. > > > > > > But for PLANES_PER_STAGE, I am still confused. A stage or a LM pair can > > > involve many SSPP layers. How it related to planes? Plane is a concepts from > > > higher level. > > > > PIPES_PER_STAGE, excuse me. > > Do you mean to keep PIPES_PER_STAGE and do not introduce PIPES_PER_LM_PAIR, > or use both? Looks like they are equal in hardware nature. A stage > structure serves > a mixer pair with 2 pipes. We can use PIPES_PER_LM_PAIR and add comment to > indicate it, thus avoid defining too many macro. Yes, don't introduce PIPES_PER_LM_PAIR and just add STAGES_PER_PLANE. > > > > > -- > > With best wishes > > Dmitry
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 43d9817cd858f..66f745399a602 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, i; + uint32_t lm_idx, lm_pair, i, pipe_idx; bool bg_alpha_enable = false; DECLARE_BITMAP(fetch_active, SSPP_MAX); @@ -463,15 +463,20 @@ 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_LM_PAIR; 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 */ + for (lm_pair = 0; lm_pair < PIPES_PER_PLANE / 2; lm_pair++) { + for (i = 0; i < PIPES_PER_LM_PAIR; i++) { + pipe_idx = i + lm_pair * PIPES_PER_LM_PAIR; + if (!pstate->pipe[pipe_idx].sspp) + continue; + set_bit(pstate->pipe[pipe_idx].sspp->idx, fetch_active); + _dpu_crtc_blend_setup_pipe(crtc, plane, + mixer, cstate->num_mixers, + pstate->stage, + format, fb ? fb->modifier : 0, + &pstate->pipe[pipe_idx], i, + &stage_cfg[lm_pair]); + } } /* blend config update */ @@ -503,7 +508,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[LM_PAIRS_PER_PLANE]; int i; DRM_DEBUG_ATOMIC("%s\n", dpu_crtc->name); @@ -516,9 +521,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[0]); for (i = 0; i < cstate->num_mixers; i++) { ctl = mixer[i].lm_ctl; @@ -535,8 +540,13 @@ 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. + * There is 4 mixers at most. The first 2 are for the left half, and + * the later 2 are for the right half. + */ ctl->ops.setup_blendstage(ctl, mixer[i].hw_lm->idx, - &stage_cfg); + &stage_cfg[i / 2]); } } 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 3277debe11689..d8f5cffa60ea6 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 PIPES_PER_PLANE 2 #define PIPES_PER_LM_PAIR 2 +#define LM_PAIRS_PER_PLANE 2 #ifndef DPU_MAX_DE_CURVES #define DPU_MAX_DE_CURVES 3 #endif
Blend pipes by set of mixer pair config. The first 2 pipes are for left half screen with the first set of mixer pair config. And the later 2 pipes are for right in quad pipe case. Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 38 ++++++++++++++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 1 + 2 files changed, 25 insertions(+), 14 deletions(-)