On 2024-06-13 20:05:11, Dmitry Baryshkov wrote: > Rename dpu_hw_setup_vsync_source functions to make the names match the > implementation: on DPU 5.x the TOP only contains timer setup, while 3.x > and 4.x used MDP_VSYNC_SEL register to select TE source. Yeah that was never really clear anymore after I split this function in two in commit a2ff096803b3 ("drm/msm/dpu: Disable MDP vsync source selection on DPU 5.0.0 and above"). > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c > index 05e48cf4ec1d..6e2ac50b94a4 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c > @@ -107,8 +107,8 @@ static void dpu_hw_get_danger_status(struct dpu_hw_mdp *mdp, > status->sspp[SSPP_CURSOR1] = (value >> 26) & 0x3; > } > > -static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp, > - struct dpu_vsync_source_cfg *cfg) > +static void dpu_hw_setup_wd_timer(struct dpu_hw_mdp *mdp, > + struct dpu_vsync_source_cfg *cfg) > { > struct dpu_hw_blk_reg_map *c; > u32 reg, wd_load_value, wd_ctl, wd_ctl2; > @@ -163,8 +163,8 @@ static void dpu_hw_setup_vsync_source(struct dpu_hw_mdp *mdp, > } > } > > -static void dpu_hw_setup_vsync_source_and_vsync_sel(struct dpu_hw_mdp *mdp, > - struct dpu_vsync_source_cfg *cfg) > +static void dpu_hw_setup_vsync_sel(struct dpu_hw_mdp *mdp, Maybe keep setup_wd_timer_and_vsync_sel()? OTOH it doesn't always set wd_timer, only when vsync_source calls for it. > + struct dpu_vsync_source_cfg *cfg) > { > struct dpu_hw_blk_reg_map *c; > u32 reg, i; > @@ -187,7 +187,7 @@ static void dpu_hw_setup_vsync_source_and_vsync_sel(struct dpu_hw_mdp *mdp, > } > DPU_REG_WRITE(c, MDP_VSYNC_SEL, reg); > > - dpu_hw_setup_vsync_source(mdp, cfg); > + dpu_hw_setup_wd_timer(mdp, cfg); > } > > static void dpu_hw_get_safe_status(struct dpu_hw_mdp *mdp, > @@ -239,9 +239,9 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops, > ops->get_danger_status = dpu_hw_get_danger_status; > > if (cap & BIT(DPU_MDP_VSYNC_SEL)) > - ops->setup_vsync_source = dpu_hw_setup_vsync_source_and_vsync_sel; > + ops->setup_vsync_source = dpu_hw_setup_vsync_sel; > else > - ops->setup_vsync_source = dpu_hw_setup_vsync_source; > + ops->setup_vsync_source = dpu_hw_setup_wd_timer; Should the callback also be renamed - and the docs improved? Seems the vsync_sel register is used to selsect a vsync_source (plus some other bits like the pingpong block). - Marijn > > ops->get_safe_status = dpu_hw_get_safe_status; > > > -- > 2.39.2 >