Re: [Intel-gfx] [PATCH 3/4] drm/i915/vrr: Reorder transcoder vs. vrr enable/disable

2022-12-05 Thread Navare, Manasi
On Fri, Dec 02, 2022 at 03:44:11PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> On mtl it looks like disabling VRR after the transcoder has
> been disabled can cause the pipe/transcoder to get stuck
> when re-enabled in non-vrr mode. Reversing the order seems to
> help.
> 
> Bspec is extremely confused about the VRR enable/disable sequence
> anyway, and this now more closely matches the non-modeset VRR
> sequence, whereas the full modeset sequence still claims that
> the original order is fine. But since we eventually want to toggle
> VRR without a full modeset anyway this seems like the better order
> to follow.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Manasi Navare 

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 5f9a2410fc4c..69595cbb2766 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2726,10 +2726,10 @@ static void intel_ddi_post_disable(struct 
> intel_atomic_state *state,
>   if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) {
>   intel_crtc_vblank_off(old_crtc_state);
>  
> - intel_disable_transcoder(old_crtc_state);
> -
>   intel_vrr_disable(old_crtc_state);
>  
> + intel_disable_transcoder(old_crtc_state);
> +
>   intel_ddi_disable_transcoder_func(old_crtc_state);
>  
>   intel_dsc_disable(old_crtc_state);
> @@ -2946,13 +2946,13 @@ static void intel_enable_ddi(struct 
> intel_atomic_state *state,
>   if (!intel_crtc_is_bigjoiner_slave(crtc_state))
>   intel_ddi_enable_transcoder_func(encoder, crtc_state);
>  
> - intel_vrr_enable(encoder, crtc_state);
> -
>   /* Enable/Disable DP2.0 SDP split config before transcoder */
>   intel_audio_sdp_split_update(encoder, crtc_state);
>  
>   intel_enable_transcoder(crtc_state);
>  
> + intel_vrr_enable(encoder, crtc_state);
> +
>   intel_crtc_vblank_on(crtc_state);
>  
>   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -- 
> 2.37.4
> 


[Intel-gfx] [PATCH 3/4] drm/i915/vrr: Reorder transcoder vs. vrr enable/disable

2022-12-02 Thread Ville Syrjala
From: Ville Syrjälä 

On mtl it looks like disabling VRR after the transcoder has
been disabled can cause the pipe/transcoder to get stuck
when re-enabled in non-vrr mode. Reversing the order seems to
help.

Bspec is extremely confused about the VRR enable/disable sequence
anyway, and this now more closely matches the non-modeset VRR
sequence, whereas the full modeset sequence still claims that
the original order is fine. But since we eventually want to toggle
VRR without a full modeset anyway this seems like the better order
to follow.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 5f9a2410fc4c..69595cbb2766 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2726,10 +2726,10 @@ static void intel_ddi_post_disable(struct 
intel_atomic_state *state,
if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) {
intel_crtc_vblank_off(old_crtc_state);
 
-   intel_disable_transcoder(old_crtc_state);
-
intel_vrr_disable(old_crtc_state);
 
+   intel_disable_transcoder(old_crtc_state);
+
intel_ddi_disable_transcoder_func(old_crtc_state);
 
intel_dsc_disable(old_crtc_state);
@@ -2946,13 +2946,13 @@ static void intel_enable_ddi(struct intel_atomic_state 
*state,
if (!intel_crtc_is_bigjoiner_slave(crtc_state))
intel_ddi_enable_transcoder_func(encoder, crtc_state);
 
-   intel_vrr_enable(encoder, crtc_state);
-
/* Enable/Disable DP2.0 SDP split config before transcoder */
intel_audio_sdp_split_update(encoder, crtc_state);
 
intel_enable_transcoder(crtc_state);
 
+   intel_vrr_enable(encoder, crtc_state);
+
intel_crtc_vblank_on(crtc_state);
 
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-- 
2.37.4