| Message ID | 20231009-topic-sm8550-graphics-sspp-split-clk-v1-0-806c0dee4e43@linaro.org |
|---|---|
| Headers | show |
| Series | drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split | expand |
On 09/10/2023 19:36, Neil Armstrong wrote: > Add an helper to setup the force clock control as it will > be used in multiple HW files. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 23 +---------------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 ++++ > 3 files changed, 26 insertions(+), 22 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 09/10/2023 19:36, Neil Armstrong wrote: > Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified > on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the > qos & limit params then call the clk_force_ctrl() op accordingly. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +-- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +++--- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 37 +++++++++++++++------- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 12 ++++--- > 4 files changed, 40 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > index 78037a697633..e4dfe0be7207 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit( > struct dpu_vbif_set_ot_params ot_params; > > memset(&ot_params, 0, sizeof(ot_params)); > + ot_params.wb = hw_wb; > ot_params.xin_id = hw_wb->caps->xin_id; > ot_params.num = hw_wb->idx - WB_0; > ot_params.width = phys_enc->cached_mode.hdisplay; > @@ -52,7 +53,6 @@ static void dpu_encoder_phys_wb_set_ot_limit( > ot_params.is_wfd = true; > ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode); > ot_params.vbif_idx = hw_wb->caps->vbif_idx; > - ot_params.clk_ctrl = hw_wb->caps->clk_ctrl; > ot_params.rd = false; > > dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params); > @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap( > hw_wb = phys_enc->hw_wb; > > memset(&qos_params, 0, sizeof(qos_params)); > + qos_params.wb = hw_wb; > qos_params.vbif_idx = hw_wb->caps->vbif_idx; > qos_params.xin_id = hw_wb->caps->xin_id; > - qos_params.clk_ctrl = hw_wb->caps->clk_ctrl; > qos_params.num = hw_wb->idx - WB_0; > qos_params.is_rt = false; > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index c2aaaded07ed..b0b662068377 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); > > memset(&ot_params, 0, sizeof(ot_params)); > + ot_params.sspp = pipe->sspp; > ot_params.xin_id = pipe->sspp->cap->xin_id; > ot_params.num = pipe->sspp->idx - SSPP_NONE; > ot_params.width = drm_rect_width(&pipe_cfg->src_rect); > @@ -357,7 +358,6 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, > ot_params.is_wfd = !pdpu->is_rt_pipe; > ot_params.frame_rate = frame_rate; > ot_params.vbif_idx = VBIF_RT; > - ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl; > ot_params.rd = true; > > dpu_vbif_set_ot_limit(dpu_kms, &ot_params); > @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane, > > memset(&qos_params, 0, sizeof(qos_params)); > qos_params.vbif_idx = VBIF_RT; > - qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl; > + qos_params.sspp = pipe->sspp; > qos_params.xin_id = pipe->sspp->cap->xin_id; > qos_params.num = pipe->sspp->idx - SSPP_VIG0; > qos_params.is_rt = pdpu->is_rt_pipe; > > - DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n", > + DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n", > qos_params.num, > qos_params.vbif_idx, > - qos_params.xin_id, qos_params.is_rt, > - qos_params.clk_ctrl); > + qos_params.xin_id, qos_params.is_rt); > > dpu_vbif_set_qos_remap(dpu_kms, &qos_params); > } > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > index 2ae5cba1848b..a79559084a91 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c > @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif, > return ot_lim; > } > > -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp, > - unsigned int clk_ctrl, > +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp, > + struct dpu_hw_wb *wb, > + struct dpu_hw_mdp *mdp, > bool enable) > { > - return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable); > + if (sspp && sspp->cap->clk_ctrl_reg) > + return sspp->ops.setup_clk_force_ctrl(sspp, enable); > + else if (wb && wb->caps->clk_ctrl_reg) > + return wb->ops.setup_clk_force_ctrl(wb, enable); > + else This is what I wanted to avoid. If we move the caller function to the sspp / WB, we will not need this kind of wrapper. > + return mdp->ops.setup_clk_force_ctrl(mdp, > + sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl, > + enable); > } > > /** > @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, > return; > } > > - if (!mdp->ops.setup_clk_force_ctrl || > - !vbif->ops.set_limit_conf || > - !vbif->ops.set_halt_ctrl) > + if ((!params->sspp && !params->wb) || > + (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) || > + (params->wb && !params->wb->ops.setup_clk_force_ctrl) || > + !mdp->ops.setup_clk_force_ctrl) > + return; > + > + if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl) > return; > > /* set write_gather_en for all write clients */ > @@ -207,7 +219,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, > trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim, > params->vbif_idx); > > - forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true); > + forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true); I'd suggest removing the setup_clk_force_ctrl from dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which will enable the clock, call dpu_vbif then disable the clock. In my opinion this is simpler than the condition in the previous chunk. > > vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim); > > @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, > vbif->ops.set_halt_ctrl(vbif, params->xin_id, false); > > if (forced_on) > - dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false); > + dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false); > } > > void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, > @@ -245,7 +257,10 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, > return; > } > > - if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) { > + if ((!params->sspp && !params->wb) || > + (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) || > + (params->wb && !params->wb->ops.setup_clk_force_ctrl) || > + !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) { > DRM_DEBUG_ATOMIC("qos remap not supported\n"); > return; > } > @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, > return; > } > > - forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true); > + forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true); > > for (i = 0; i < qos_tbl->npriority_lvl; i++) { > DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n", > @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, > } > > if (forced_on) > - dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false); > + dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false); > } > > void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms) > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > index ab490177d886..a4fe76e390d9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h > @@ -7,7 +7,12 @@ > > #include "dpu_kms.h" > > +struct dpu_hw_sspp; > +struct dpu_hw_wb; > + > struct dpu_vbif_set_ot_params { > + struct dpu_hw_sspp *sspp; > + struct dpu_hw_wb *wb; > u32 xin_id; > u32 num; > u32 width; > @@ -16,28 +21,27 @@ struct dpu_vbif_set_ot_params { > bool rd; > bool is_wfd; > u32 vbif_idx; > - u32 clk_ctrl; > }; > > struct dpu_vbif_set_memtype_params { > u32 xin_id; > u32 vbif_idx; > - u32 clk_ctrl; > bool is_cacheable; > }; > > /** > * struct dpu_vbif_set_qos_params - QoS remapper parameter > + * @sspp: backing SSPP > * @vbif_idx: vbif identifier > * @xin_id: client interface identifier > - * @clk_ctrl: clock control identifier of the xin > * @num: pipe identifier (debug only) > * @is_rt: true if pipe is used in real-time use case > */ > struct dpu_vbif_set_qos_params { > + struct dpu_hw_sspp *sspp; > + struct dpu_hw_wb *wb; > u32 vbif_idx; > u32 xin_id; > - u32 clk_ctrl; > u32 num; > bool is_rt; > }; >
On 09/10/2023 19:07, Dmitry Baryshkov wrote: > On 09/10/2023 19:36, Neil Armstrong wrote: >> Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified >> on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the >> qos & limit params then call the clk_force_ctrl() op accordingly. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +++--- >> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 37 +++++++++++++++------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 12 ++++--- >> 4 files changed, 40 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >> index 78037a697633..e4dfe0be7207 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c >> @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit( >> struct dpu_vbif_set_ot_params ot_params; >> memset(&ot_params, 0, sizeof(ot_params)); >> + ot_params.wb = hw_wb; >> ot_params.xin_id = hw_wb->caps->xin_id; >> ot_params.num = hw_wb->idx - WB_0; >> ot_params.width = phys_enc->cached_mode.hdisplay; >> @@ -52,7 +53,6 @@ static void dpu_encoder_phys_wb_set_ot_limit( >> ot_params.is_wfd = true; >> ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode); >> ot_params.vbif_idx = hw_wb->caps->vbif_idx; >> - ot_params.clk_ctrl = hw_wb->caps->clk_ctrl; >> ot_params.rd = false; >> dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params); >> @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap( >> hw_wb = phys_enc->hw_wb; >> memset(&qos_params, 0, sizeof(qos_params)); >> + qos_params.wb = hw_wb; >> qos_params.vbif_idx = hw_wb->caps->vbif_idx; >> qos_params.xin_id = hw_wb->caps->xin_id; >> - qos_params.clk_ctrl = hw_wb->caps->clk_ctrl; >> qos_params.num = hw_wb->idx - WB_0; >> qos_params.is_rt = false; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> index c2aaaded07ed..b0b662068377 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c >> @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, >> struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane); >> memset(&ot_params, 0, sizeof(ot_params)); >> + ot_params.sspp = pipe->sspp; >> ot_params.xin_id = pipe->sspp->cap->xin_id; >> ot_params.num = pipe->sspp->idx - SSPP_NONE; >> ot_params.width = drm_rect_width(&pipe_cfg->src_rect); >> @@ -357,7 +358,6 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane, >> ot_params.is_wfd = !pdpu->is_rt_pipe; >> ot_params.frame_rate = frame_rate; >> ot_params.vbif_idx = VBIF_RT; >> - ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl; >> ot_params.rd = true; >> dpu_vbif_set_ot_limit(dpu_kms, &ot_params); >> @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane, >> memset(&qos_params, 0, sizeof(qos_params)); >> qos_params.vbif_idx = VBIF_RT; >> - qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl; >> + qos_params.sspp = pipe->sspp; >> qos_params.xin_id = pipe->sspp->cap->xin_id; >> qos_params.num = pipe->sspp->idx - SSPP_VIG0; >> qos_params.is_rt = pdpu->is_rt_pipe; >> - DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n", >> + DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n", >> qos_params.num, >> qos_params.vbif_idx, >> - qos_params.xin_id, qos_params.is_rt, >> - qos_params.clk_ctrl); >> + qos_params.xin_id, qos_params.is_rt); >> dpu_vbif_set_qos_remap(dpu_kms, &qos_params); >> } >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c >> index 2ae5cba1848b..a79559084a91 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c >> @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif, >> return ot_lim; >> } >> -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp, >> - unsigned int clk_ctrl, >> +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp, >> + struct dpu_hw_wb *wb, >> + struct dpu_hw_mdp *mdp, >> bool enable) >> { >> - return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable); >> + if (sspp && sspp->cap->clk_ctrl_reg) >> + return sspp->ops.setup_clk_force_ctrl(sspp, enable); >> + else if (wb && wb->caps->clk_ctrl_reg) >> + return wb->ops.setup_clk_force_ctrl(wb, enable); >> + else > > This is what I wanted to avoid. > > If we move the caller function to the sspp / WB, we will not need this kind of wrapper. I tried it, but it requires passing the mdp pointer to the setup_clk_force_ctrl op, which is IMHO not super clean... or if you have a way to get dpu_hw_mdp from within hw_sspp/hw_wb it would help. > >> + return mdp->ops.setup_clk_force_ctrl(mdp, >> + sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl, >> + enable); >> } >> /** >> @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, >> return; >> } >> - if (!mdp->ops.setup_clk_force_ctrl || >> - !vbif->ops.set_limit_conf || >> - !vbif->ops.set_halt_ctrl) >> + if ((!params->sspp && !params->wb) || >> + (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) || >> + (params->wb && !params->wb->ops.setup_clk_force_ctrl) || >> + !mdp->ops.setup_clk_force_ctrl) >> + return; >> + >> + if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl) >> return; >> /* set write_gather_en for all write clients */ >> @@ -207,7 +219,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, >> trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim, >> params->vbif_idx); >> - forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true); >> + forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true); > > I'd suggest removing the setup_clk_force_ctrl from dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which will enable the clock, call dpu_vbif then disable the clock. > > In my opinion this is simpler than the condition in the previous chunk. Indeed this is a nice option, but the hw_mdp pointer requirement into hw_sspp/hw_wb still puzzles me. > >> vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim); >> @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms, >> vbif->ops.set_halt_ctrl(vbif, params->xin_id, false); >> if (forced_on) >> - dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false); >> + dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false); >> } >> void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, >> @@ -245,7 +257,10 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, >> return; >> } >> - if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) { >> + if ((!params->sspp && !params->wb) || >> + (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) || >> + (params->wb && !params->wb->ops.setup_clk_force_ctrl) || >> + !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) { >> DRM_DEBUG_ATOMIC("qos remap not supported\n"); >> return; >> } >> @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, >> return; >> } >> - forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true); >> + forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true); >> for (i = 0; i < qos_tbl->npriority_lvl; i++) { >> DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n", >> @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms, >> } >> if (forced_on) >> - dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false); >> + dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false); >> } >> void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms) >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h >> index ab490177d886..a4fe76e390d9 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h >> @@ -7,7 +7,12 @@ >> #include "dpu_kms.h" >> +struct dpu_hw_sspp; >> +struct dpu_hw_wb; >> + >> struct dpu_vbif_set_ot_params { >> + struct dpu_hw_sspp *sspp; >> + struct dpu_hw_wb *wb; >> u32 xin_id; >> u32 num; >> u32 width; >> @@ -16,28 +21,27 @@ struct dpu_vbif_set_ot_params { >> bool rd; >> bool is_wfd; >> u32 vbif_idx; >> - u32 clk_ctrl; >> }; >> struct dpu_vbif_set_memtype_params { >> u32 xin_id; >> u32 vbif_idx; >> - u32 clk_ctrl; >> bool is_cacheable; >> }; >> /** >> * struct dpu_vbif_set_qos_params - QoS remapper parameter >> + * @sspp: backing SSPP >> * @vbif_idx: vbif identifier >> * @xin_id: client interface identifier >> - * @clk_ctrl: clock control identifier of the xin >> * @num: pipe identifier (debug only) >> * @is_rt: true if pipe is used in real-time use case >> */ >> struct dpu_vbif_set_qos_params { >> + struct dpu_hw_sspp *sspp; >> + struct dpu_hw_wb *wb; >> u32 vbif_idx; >> u32 xin_id; >> - u32 clk_ctrl; >> u32 num; >> bool is_rt; >> }; >> > Thanks, Neil
On 2023-10-09 18:36:11, Neil Armstrong wrote: > Starting with the SM8550 platform, the SSPP & WB Clock Controls are > no more in the MDP TOP registers, but in the SSPP & WB register space. > > Add the corresponding SSPP & WB ops and use them from the vbif QoS > and OT limit setup functions. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > Neil Armstrong (5): > drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper > drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb > drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper > drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control > drm/msm: dpu1: sm8550: move split clock controls to sspp entries Fyi we're all using drm/msm/dpu: now :) - Marijn > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++----------- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +-- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 +++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 9 +++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 23 +------------ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 ++++++++++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 9 +++++ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 4 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 38 +++++++++++++++++----- > drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 12 ++++--- > 13 files changed, 120 insertions(+), 61 deletions(-) > --- > base-commit: 9119cf579b4432b36be9d33a92f4331922067d92 > change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa > > Best regards, > -- > Neil Armstrong <neil.armstrong@linaro.org> >
On 10/10/2023 10:10, Marijn Suijten wrote: > On 2023-10-09 18:36:11, Neil Armstrong wrote: >> Starting with the SM8550 platform, the SSPP & WB Clock Controls are >> no more in the MDP TOP registers, but in the SSPP & WB register space. >> >> Add the corresponding SSPP & WB ops and use them from the vbif QoS >> and OT limit setup functions. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> Neil Armstrong (5): >> drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper >> drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb >> drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper >> drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control >> drm/msm: dpu1: sm8550: move split clock controls to sspp entries > > Fyi we're all using drm/msm/dpu: now :) Ack, thx, will change for v2 > > - Marijn > >> >> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++----------- >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 +++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 9 +++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 23 +------------ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 ++++++++++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 9 +++++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 4 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 38 +++++++++++++++++----- >> drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 12 ++++--- >> 13 files changed, 120 insertions(+), 61 deletions(-) >> --- >> base-commit: 9119cf579b4432b36be9d33a92f4331922067d92 >> change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa >> >> Best regards, >> -- >> Neil Armstrong <neil.armstrong@linaro.org> >>
Starting with the SM8550 platform, the SSPP & WB Clock Controls are no more in the MDP TOP registers, but in the SSPP & WB register space. Add the corresponding SSPP & WB ops and use them from the vbif QoS and OT limit setup functions. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- Neil Armstrong (5): drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control drm/msm: dpu1: sm8550: move split clock controls to sspp entries .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++----------- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 4 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 9 +++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 9 +++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 23 +------------ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 ++++++++++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 4 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 9 +++++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 4 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 38 +++++++++++++++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h | 12 ++++--- 13 files changed, 120 insertions(+), 61 deletions(-) --- base-commit: 9119cf579b4432b36be9d33a92f4331922067d92 change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa Best regards,