diff mbox series

[07/19] drm/i915/vrr: Prepare for fixed refresh rate timings

Message ID 20250214121130.1808451-8-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

Nautiyal, Ankit K Feb. 14, 2025, 12:11 p.m. UTC 
Currently we always compute the timings as if vrr is enabled.
With this approach the state checker becomes complicated when we
introduce fixed refresh rate mode with vrr timing generator.

To avoid the complications, instead of always computing vrr timings, we
compute vrr timings based on uapi.vrr_enable knob.
So when the knob is disabled we always compute vmin=flipline=vmax.

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

Comments

Ville Syrjälä Feb. 17, 2025, 6:12 p.m. UTC | #1 
On Fri, Feb 14, 2025 at 05:41:17PM +0530, Ankit Nautiyal wrote:
> Currently we always compute the timings as if vrr is enabled.
> With this approach the state checker becomes complicated when we
> introduce fixed refresh rate mode with vrr timing generator.
> 
> To avoid the complications, instead of always computing vrr timings, we
> compute vrr timings based on uapi.vrr_enable knob.
> So when the knob is disabled we always compute vmin=flipline=vmax.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 54 ++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 3bcf2a026ad3..a4ed102a2119 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -263,6 +263,35 @@ int intel_vrr_fixed_rr_vtotal(const struct intel_crtc_state *crtc_state)
>  			intel_vrr_real_vblank_delay(crtc_state);
>  }
>  
> +static
> +int intel_vrr_fixed_rr_vmax(const struct intel_crtc_state *crtc_state)
> +{
> +	return intel_vrr_fixed_rr_vtotal(crtc_state);
> +}
> +
> +static
> +int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	return intel_vrr_fixed_rr_vtotal(crtc_state) -
> +		intel_vrr_flipline_offset(display);
> +}
> +
> +static
> +int intel_vrr_fixed_rr_flipline(const struct intel_crtc_state *crtc_state)
> +{
> +	return intel_vrr_fixed_rr_vtotal(crtc_state);
> +}
> +
> +static
> +void intel_vrr_prepare_fixed_timings(struct intel_crtc_state *crtc_state)
> +{
> +	crtc_state->vrr.vmax = intel_vrr_fixed_rr_vmax(crtc_state);
> +	crtc_state->vrr.vmin = intel_vrr_fixed_rr_vmin(crtc_state);
> +	crtc_state->vrr.flipline = intel_vrr_fixed_rr_flipline(crtc_state);

Same comment as to the previous patch: vblank delay is not a thing
at this point, so this needs to just use the actual timings without
any adjustments.

The rest of the patch looks fine.

> +}
> +
>  static
>  int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state)
>  {
> @@ -343,6 +372,8 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  		intel_vrr_compute_vrr_timings(crtc_state);
>  	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>  		intel_vrr_compute_cmrr_timings(crtc_state);
> +	else
> +		intel_vrr_prepare_fixed_timings(crtc_state);
>  
>  	if (HAS_AS_SDP(display)) {
>  		crtc_state->vrr.vsync_start =
> @@ -514,6 +545,13 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>  	if (!crtc_state->vrr.enable)
>  		return;
>  
> +	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
> +		       crtc_state->vrr.vmin - 1);
> +	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
> +		       crtc_state->vrr.vmax - 1);
> +	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
> +		       crtc_state->vrr.flipline - 1);
> +
>  	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
>  		       TRANS_PUSH_EN);
>  
> @@ -527,6 +565,20 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> +static
> +void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +
> +	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
> +		       intel_vrr_fixed_rr_vmin(crtc_state) - 1);
> +	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
> +		       intel_vrr_fixed_rr_vmax(crtc_state) - 1);
> +	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
> +		       intel_vrr_fixed_rr_flipline(crtc_state) - 1);
> +}
> +
>  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(old_crtc_state);
> @@ -541,6 +593,8 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>  				TRANS_VRR_STATUS(display, cpu_transcoder),
>  				VRR_STATUS_VRR_EN_LIVE, 1000);
>  	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
> +
> +	intel_vrr_set_fixed_rr_timings(old_crtc_state);
>  }
>  
>  static
> -- 
> 2.45.2
Nautiyal, Ankit K Feb. 19, 2025, 12:40 p.m. UTC | #2 
On 2/17/2025 11:42 PM, Ville Syrjälä wrote:
> On Fri, Feb 14, 2025 at 05:41:17PM +0530, Ankit Nautiyal wrote:
>> Currently we always compute the timings as if vrr is enabled.
>> With this approach the state checker becomes complicated when we
>> introduce fixed refresh rate mode with vrr timing generator.
>>
>> To avoid the complications, instead of always computing vrr timings, we
>> compute vrr timings based on uapi.vrr_enable knob.
>> So when the knob is disabled we always compute vmin=flipline=vmax.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 54 ++++++++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 3bcf2a026ad3..a4ed102a2119 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -263,6 +263,35 @@ int intel_vrr_fixed_rr_vtotal(const struct intel_crtc_state *crtc_state)
>>   			intel_vrr_real_vblank_delay(crtc_state);
>>   }
>>   
>> +static
>> +int intel_vrr_fixed_rr_vmax(const struct intel_crtc_state *crtc_state)
>> +{
>> +	return intel_vrr_fixed_rr_vtotal(crtc_state);
>> +}
>> +
>> +static
>> +int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_display *display = to_intel_display(crtc_state);
>> +
>> +	return intel_vrr_fixed_rr_vtotal(crtc_state) -
>> +		intel_vrr_flipline_offset(display);
>> +}
>> +
>> +static
>> +int intel_vrr_fixed_rr_flipline(const struct intel_crtc_state *crtc_state)
>> +{
>> +	return intel_vrr_fixed_rr_vtotal(crtc_state);
>> +}
>> +
>> +static
>> +void intel_vrr_prepare_fixed_timings(struct intel_crtc_state *crtc_state)
>> +{
>> +	crtc_state->vrr.vmax = intel_vrr_fixed_rr_vmax(crtc_state);
>> +	crtc_state->vrr.vmin = intel_vrr_fixed_rr_vmin(crtc_state);
>> +	crtc_state->vrr.flipline = intel_vrr_fixed_rr_flipline(crtc_state);
> Same comment as to the previous patch: vblank delay is not a thing
> at this point, so this needs to just use the actual timings without
> any adjustments.

Will just make:

         crtc_state->vrr.vmax = intel_vrr_vmin_flipline(crtc_state);
         crtc_state->vrr.flipline = intel_vrr_vmin_flipline(crtc_state);


Regards,

Ankit

>
> The rest of the patch looks fine.
>
>> +}
>> +
>>   static
>>   int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state)
>>   {
>> @@ -343,6 +372,8 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>>   		intel_vrr_compute_vrr_timings(crtc_state);
>>   	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>>   		intel_vrr_compute_cmrr_timings(crtc_state);
>> +	else
>> +		intel_vrr_prepare_fixed_timings(crtc_state);
>>   
>>   	if (HAS_AS_SDP(display)) {
>>   		crtc_state->vrr.vsync_start =
>> @@ -514,6 +545,13 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>>   	if (!crtc_state->vrr.enable)
>>   		return;
>>   
>> +	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
>> +		       crtc_state->vrr.vmin - 1);
>> +	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
>> +		       crtc_state->vrr.vmax - 1);
>> +	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
>> +		       crtc_state->vrr.flipline - 1);
>> +
>>   	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
>>   		       TRANS_PUSH_EN);
>>   
>> @@ -527,6 +565,20 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>>   	}
>>   }
>>   
>> +static
>> +void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_display *display = to_intel_display(crtc_state);
>> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>> +
>> +	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
>> +		       intel_vrr_fixed_rr_vmin(crtc_state) - 1);
>> +	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
>> +		       intel_vrr_fixed_rr_vmax(crtc_state) - 1);
>> +	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
>> +		       intel_vrr_fixed_rr_flipline(crtc_state) - 1);
>> +}
>> +
>>   void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>>   {
>>   	struct intel_display *display = to_intel_display(old_crtc_state);
>> @@ -541,6 +593,8 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
>>   				TRANS_VRR_STATUS(display, cpu_transcoder),
>>   				VRR_STATUS_VRR_EN_LIVE, 1000);
>>   	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
>> +
>> +	intel_vrr_set_fixed_rr_timings(old_crtc_state);
>>   }
>>   
>>   static
>> -- 
>> 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 3bcf2a026ad3..a4ed102a2119 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -263,6 +263,35 @@  int intel_vrr_fixed_rr_vtotal(const struct intel_crtc_state *crtc_state)
 			intel_vrr_real_vblank_delay(crtc_state);
 }
 
+static
+int intel_vrr_fixed_rr_vmax(const struct intel_crtc_state *crtc_state)
+{
+	return intel_vrr_fixed_rr_vtotal(crtc_state);
+}
+
+static
+int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+
+	return intel_vrr_fixed_rr_vtotal(crtc_state) -
+		intel_vrr_flipline_offset(display);
+}
+
+static
+int intel_vrr_fixed_rr_flipline(const struct intel_crtc_state *crtc_state)
+{
+	return intel_vrr_fixed_rr_vtotal(crtc_state);
+}
+
+static
+void intel_vrr_prepare_fixed_timings(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->vrr.vmax = intel_vrr_fixed_rr_vmax(crtc_state);
+	crtc_state->vrr.vmin = intel_vrr_fixed_rr_vmin(crtc_state);
+	crtc_state->vrr.flipline = intel_vrr_fixed_rr_flipline(crtc_state);
+}
+
 static
 int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state)
 {
@@ -343,6 +372,8 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 		intel_vrr_compute_vrr_timings(crtc_state);
 	else if (is_cmrr_frac_required(crtc_state) && is_edp)
 		intel_vrr_compute_cmrr_timings(crtc_state);
+	else
+		intel_vrr_prepare_fixed_timings(crtc_state);
 
 	if (HAS_AS_SDP(display)) {
 		crtc_state->vrr.vsync_start =
@@ -514,6 +545,13 @@  void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
 	if (!crtc_state->vrr.enable)
 		return;
 
+	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
+		       crtc_state->vrr.vmin - 1);
+	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
+		       crtc_state->vrr.vmax - 1);
+	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
+		       crtc_state->vrr.flipline - 1);
+
 	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
 		       TRANS_PUSH_EN);
 
@@ -527,6 +565,20 @@  void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
 	}
 }
 
+static
+void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_vmin(crtc_state) - 1);
+	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_vmax(crtc_state) - 1);
+	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_flipline(crtc_state) - 1);
+}
+
 void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 {
 	struct intel_display *display = to_intel_display(old_crtc_state);
@@ -541,6 +593,8 @@  void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
 				TRANS_VRR_STATUS(display, cpu_transcoder),
 				VRR_STATUS_VRR_EN_LIVE, 1000);
 	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
+
+	intel_vrr_set_fixed_rr_timings(old_crtc_state);
 }
 
 static