diff mbox series

[16/22] drm/i915/vrr: Use fixed timings for platforms that support VRR

Message ID 20250304081948.3177034-17-ankit.k.nautiyal@intel.com (mailing list archive)
State New
Headers show
Series Use VRR timing generator for fixed refresh rate modes | expand

Commit Message

Ankit Nautiyal March 4, 2025, 8:19 a.m. UTC 
For fixed refresh rate use fixed timings for all platforms that support
VRR. For this add checks to avoid computing and reading VRR for
platforms that do not support VRR.

v2: Avoid touching check for VRR_CTL_FLIP_LINE_EN. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Ville Syrjälä March 4, 2025, 6:53 p.m. UTC | #1 
On Tue, Mar 04, 2025 at 01:49:42PM +0530, Ankit Nautiyal wrote:
> For fixed refresh rate use fixed timings for all platforms that support
> VRR. For this add checks to avoid computing and reading VRR for
> platforms that do not support VRR.
> 
> v2: Avoid touching check for VRR_CTL_FLIP_LINE_EN. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 97040ab9ed86..11f23affd13a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -347,6 +347,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	int vmin, vmax;
>  
> +	if (!HAS_VRR(display))
> +		return;
> +
>  	/*
>  	 * FIXME all joined pipes share the same transcoder.
>  	 * Need to account for that during VRR toggle/push/etc.
> @@ -370,8 +373,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  		vmax = vmin;
>  	}
>  
> -	if (vmin >= vmax)
> -		return;
> +	vmin = intel_vrr_compute_vmin(crtc_state);

Didn't we already do that?

>  
>  	crtc_state->vrr.vmin = vmin;
>  	crtc_state->vrr.vmax = vmax;
> @@ -385,7 +387,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	 */
>  	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
>  
> -	if (crtc_state->uapi.vrr_enabled)
> +	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
>  		intel_vrr_compute_vrr_timings(crtc_state);
>  	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>  		intel_vrr_compute_cmrr_timings(crtc_state);
> @@ -659,6 +661,9 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 trans_vrr_ctl, trans_vrr_vsync;
>  
> +	if (!HAS_VRR(display))
> +		return;
> +

The caller already checks that, so should not be needed here.

>  	trans_vrr_ctl = intel_de_read(display,
>  				      TRANS_VRR_CTL(display, cpu_transcoder));
>  
> -- 
> 2.45.2
Ankit Nautiyal March 5, 2025, 8:46 a.m. UTC | #2 
On 3/5/2025 12:23 AM, Ville Syrjälä wrote:
> On Tue, Mar 04, 2025 at 01:49:42PM +0530, Ankit Nautiyal wrote:
>> For fixed refresh rate use fixed timings for all platforms that support
>> VRR. For this add checks to avoid computing and reading VRR for
>> platforms that do not support VRR.
>>
>> v2: Avoid touching check for VRR_CTL_FLIP_LINE_EN. (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 97040ab9ed86..11f23affd13a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -347,6 +347,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>>   	int vmin, vmax;
>>   
>> +	if (!HAS_VRR(display))
>> +		return;
>> +
>>   	/*
>>   	 * FIXME all joined pipes share the same transcoder.
>>   	 * Need to account for that during VRR toggle/push/etc.
>> @@ -370,8 +373,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   		vmax = vmin;
>>   	}
>>   
>> -	if (vmin >= vmax)
>> -		return;
>> +	vmin = intel_vrr_compute_vmin(crtc_state);
> Didn't we already do that?


(facepalm) Yes, indeed, this is already set in an earlier patch. It 
should be removed. Thanks for catching this.


>
>>   
>>   	crtc_state->vrr.vmin = vmin;
>>   	crtc_state->vrr.vmax = vmax;
>> @@ -385,7 +387,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   	 */
>>   	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
>>   
>> -	if (crtc_state->uapi.vrr_enabled)
>> +	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
>>   		intel_vrr_compute_vrr_timings(crtc_state);
>>   	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>>   		intel_vrr_compute_cmrr_timings(crtc_state);
>> @@ -659,6 +661,9 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>   	u32 trans_vrr_ctl, trans_vrr_vsync;
>>   
>> +	if (!HAS_VRR(display))
>> +		return;
>> +
> The caller already checks that, so should not be needed here.

Will remove this check from here.


Regards,

Ankit

>
>>   	trans_vrr_ctl = intel_de_read(display,
>>   				      TRANS_VRR_CTL(display, cpu_transcoder));
>>   
>> -- 
>> 2.45.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 97040ab9ed86..11f23affd13a 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -347,6 +347,9 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	int vmin, vmax;
 
+	if (!HAS_VRR(display))
+		return;
+
 	/*
 	 * FIXME all joined pipes share the same transcoder.
 	 * Need to account for that during VRR toggle/push/etc.
@@ -370,8 +373,7 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 		vmax = vmin;
 	}
 
-	if (vmin >= vmax)
-		return;
+	vmin = intel_vrr_compute_vmin(crtc_state);
 
 	crtc_state->vrr.vmin = vmin;
 	crtc_state->vrr.vmax = vmax;
@@ -385,7 +387,7 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 	 */
 	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
 
-	if (crtc_state->uapi.vrr_enabled)
+	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
 		intel_vrr_compute_vrr_timings(crtc_state);
 	else if (is_cmrr_frac_required(crtc_state) && is_edp)
 		intel_vrr_compute_cmrr_timings(crtc_state);
@@ -659,6 +661,9 @@  void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 trans_vrr_ctl, trans_vrr_vsync;
 
+	if (!HAS_VRR(display))
+		return;
+
 	trans_vrr_ctl = intel_de_read(display,
 				      TRANS_VRR_CTL(display, cpu_transcoder));