[PATCH] drm/msm/dpu: Set vsync source irrespective of mdp top support

[PATCH] drm/msm/dpu: Set vsync source irrespective of mdp top support

[Teguh Sobirin]

Move the loop over phys_encs outside the
hw_mdptop->ops.setup_vsync_source block.
This way, vsync_sel() is called for each interface.

This change ensures TE vsync selection works
even if setup_vsync_source is not implemented.

Signed-off-by: Teguh Sobirin <teguh@sobir.in>
---
 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 258edaa18fc0..f36c5c7924a3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -784,24 +784,20 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
 		return;
 	}
 
+	/* Set vsync source irrespective of mdp top support */
+	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;
-		vsync_cfg.frame_rate = drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode);
-
-		vsync_cfg.vsync_source = disp_info->vsync_source;
-
-		hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg);
-
-		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

Re: [PATCH] drm/msm/dpu: Set vsync source irrespective of mdp top support

[Dmitry Baryshkov]

On Tue, Oct 28, 2025 at 08:36:35PM +0800, Teguh Sobirin wrote:
> Move the loop over phys_encs outside the
> hw_mdptop->ops.setup_vsync_source block.
> This way, vsync_sel() is called for each interface.
> 
> This change ensures TE vsync selection works
> even if setup_vsync_source is not implemented.

Please see Documentation/processs/submitting-patches.rst. Don't describe
the change and pleasw wrap the commit message on 72-75 chars boundary.

I can suggest something like this:

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
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.

The patch itself looks good to me.

> 
> Signed-off-by: Teguh Sobirin <teguh@sobir.in>
> ---
>  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 258edaa18fc0..f36c5c7924a3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -784,24 +784,20 @@ static void _dpu_encoder_update_vsync_source(struct dpu_encoder_virt *dpu_enc,
>  		return;
>  	}
>  
> +	/* Set vsync source irrespective of mdp top support */
> +	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;
> -		vsync_cfg.frame_rate = drm_mode_vrefresh(&dpu_enc->base.crtc->state->adjusted_mode);
> -
> -		vsync_cfg.vsync_source = disp_info->vsync_source;
> -
> -		hw_mdptop->ops.setup_vsync_source(hw_mdptop, &vsync_cfg);
> -
> -		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
> 

-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm/dpu: Set vsync source irrespective of mdp top support

[Dmitry Baryshkov]

On Tue, Oct 28, 2025 at 07:02:28PM +0200, Dmitry Baryshkov wrote:
> On Tue, Oct 28, 2025 at 08:36:35PM +0800, Teguh Sobirin wrote:
> > Move the loop over phys_encs outside the
> > hw_mdptop->ops.setup_vsync_source block.
> > This way, vsync_sel() is called for each interface.
> > 
> > This change ensures TE vsync selection works
> > even if setup_vsync_source is not implemented.
> 
> Please see Documentation/processs/submitting-patches.rst. Don't describe
> the change and pleasw wrap the commit message on 72-75 chars boundary.
> 
> I can suggest something like this:
> 
> 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
> 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.
> 
> The patch itself looks good to me.


Teguh, any updates on the patch?

-- 
With best wishes
Dmitry

Re: [PATCH] drm/msm/dpu: Set vsync source irrespective of mdp top support

[Teguh Sobirin]

> 
> On 14 Nov 2025, at 12.10, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> 
> On Tue, Oct 28, 2025 at 07:02:28PM +0200, Dmitry Baryshkov wrote:
>>> On Tue, Oct 28, 2025 at 08:36:35PM +0800, Teguh Sobirin wrote:
>>> Move the loop over phys_encs outside the
>>> hw_mdptop->ops.setup_vsync_source block.
>>> This way, vsync_sel() is called for each interface.
>>> 
>>> This change ensures TE vsync selection works
>>> even if setup_vsync_source is not implemented.
>> 
>> Please see Documentation/processs/submitting-patches.rst. Don't describe
>> the change and pleasw wrap the commit message on 72-75 chars boundary.
>> 
>> I can suggest something like this:
>> 
>> 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
>> 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.
>> 
>> The patch itself looks good to me.
> 
> 
> Teguh, any updates on the patch?
> 
> --
> With best wishes
> Dmitry

I will send a new one with corrected commit message

Regards,
Teguh.