On 2025-11-21 14:02:08, Teguh Sobirin wrote: > Since DPU 5.x the vsync source TE setup is split between MDP TOP and > INTF blocks. Currently all code to setup vsync_source is only exectued
typo: executed. > if MDP TOP implements the setup_vsync_source() callback. However on > DPU >= 8.x this callback is not implemented, making DPU driver skip all > vsync setup. Move the INTF part out of this condition, letting DPU > driver to setup TE vsync selection on all new DPU devices. > > Signed-off-by: Teguh Sobirin <[email protected]> > --- > Changes in v2: > - Corrected commit message suggested by Dmitry Baryshkov. > - Link to v1: > https://lore.kernel.org/linux-arm-msm/tyupr06mb6099cbbe5090db12a2c187e3dd...@tyupr06mb6099.apcprd06.prod.outlook.com/ > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++++++++------------ > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index d1cfe81a3373..f468d054f5bd 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -774,24 +774,20 @@ static void _dpu_encoder_update_vsync_source(struct > dpu_encoder_virt *dpu_enc, > return; > } > > + /* Set vsync source irrespective of mdp top support */ Unnecessary comment, it's clear from the code flow that vsync_cfg.vsync_source is passed down into both functions. Perhaps we should pass disp_info->vsync_source directly into hw_intf->ops.vsync_sel() and only initialize vsync_cfg when hw_mdptop has the function, to make this clear. > + vsync_cfg.vsync_source = disp_info->vsync_source; > + > if (hw_mdptop->ops.setup_vsync_source) { > for (i = 0; i < dpu_enc->num_phys_encs; i++) > vsync_cfg.ppnumber[i] = dpu_enc->hw_pp[i]->idx; > + } > > - vsync_cfg.pp_count = dpu_enc->num_phys_encs; This change is not mentioned in the commit description. While true that pp_count is only used by dpu_hw_setup_vsync_sel(), that is still a valid function to be called on DPU < 5; it denotes the amount of ppnumber[i] array entries are used above. > - vsync_cfg.frame_rate = > drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode); Same, also not mentioned in the commit description. frame_rate is used by dpu_hw_setup_wd_timer() on DPU < 8. > - > - vsync_cfg.vsync_source = disp_info->vsync_source; > - > - hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg); But all of the above comments don't matter if the call to setup_vsync_source() is removed entirely - it didn't move anywhere else. This is not described in the commit message. - Marijn > - > - for (i = 0; i < dpu_enc->num_phys_encs; i++) { > - phys_enc = dpu_enc->phys_encs[i]; > + for (i = 0; i < dpu_enc->num_phys_encs; i++) { > + phys_enc = dpu_enc->phys_encs[i]; > > - if (phys_enc->has_intf_te && > phys_enc->hw_intf->ops.vsync_sel) > - > phys_enc->hw_intf->ops.vsync_sel(phys_enc->hw_intf, > - vsync_cfg.vsync_source); > - } > + if (phys_enc->has_intf_te && phys_enc->hw_intf->ops.vsync_sel) > + phys_enc->hw_intf->ops.vsync_sel(phys_enc->hw_intf, > + vsync_cfg.vsync_source); > } > } > > -- > 2.34.1 >
