diff mbox series

[20/35] drm/i915/vrr: Avoid sending PUSH when VRR TG is used with Fixed refresh rate

Message ID 20250124150020.2271747-21-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 Jan. 24, 2025, 3 p.m. UTC 
As per the spec, the PUSH enable must be set if not configuring for a
fixed refresh rate (i.e Vmin == Flipline == Vmax is not true).

v2: Use helper intel_vrr_use_push(). (Ville)
v3: directly use the condition, instead of checking for VRR mode.

Bspec: 68925
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> (v1)
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Ville Syrjala Jan. 24, 2025, 9:12 p.m. UTC | #1 
On Fri, Jan 24, 2025 at 08:30:05PM +0530, Ankit Nautiyal wrote:
> As per the spec, the PUSH enable must be set if not configuring for a
> fixed refresh rate (i.e Vmin == Flipline == Vmax is not true).
> 
> v2: Use helper intel_vrr_use_push(). (Ville)
> v3: directly use the condition, instead of checking for VRR mode.
> 
> Bspec: 68925
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> (v1)
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 65bbe40881d6..34e44cd52353 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -437,13 +437,18 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>  			       VRR_VSYNC_START(crtc_state->vrr.vsync_start));
>  }
>  
> +static bool intel_vrr_use_push(const struct intel_crtc_state *crtc_state)
> +{
> +	return crtc_state->vrr.flipline != crtc_state->vrr.vmax;
> +}

> +
>  void intel_vrr_send_push(struct intel_dsb *dsb,
>  			 const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> -	if (!intel_vrrtg_is_enabled(crtc_state))
> +	if (!intel_vrr_use_push(crtc_state))
>  		return;
>  
>  	if (dsb)
> @@ -462,7 +467,7 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> -	if (!intel_vrrtg_is_enabled(crtc_state))
> +	if (!intel_vrr_use_push(crtc_state))
>  		return false;
>  
>  	return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND;
> @@ -476,8 +481,9 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>  	if (!intel_vrrtg_is_enabled(crtc_state))
>  		return;
>  
> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> -		       TRANS_PUSH_EN);
> +	if (intel_vrr_use_push(crtc_state))
> +		intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
> +			       TRANS_PUSH_EN);

What I saw on TGL and ADL is that if you don't enable PUSH then nothing
works. So I think we just want to have this enabled whenever the VRR
timing generator is enabled.

>  
>  	if (crtc_state->vrr.mode == INTEL_VRRTG_MODE_CMRR) {
>  		intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
> -- 
> 2.45.2
Nautiyal, Ankit K Jan. 30, 2025, 10:55 a.m. UTC | #2 
On 1/25/2025 2:42 AM, Ville Syrjälä wrote:
> On Fri, Jan 24, 2025 at 08:30:05PM +0530, Ankit Nautiyal wrote:
>> As per the spec, the PUSH enable must be set if not configuring for a
>> fixed refresh rate (i.e Vmin == Flipline == Vmax is not true).
>>
>> v2: Use helper intel_vrr_use_push(). (Ville)
>> v3: directly use the condition, instead of checking for VRR mode.
>>
>> Bspec: 68925
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> (v1)
>> ---
>>   drivers/gpu/drm/i915/display/intel_vrr.c | 14 ++++++++++----
>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 65bbe40881d6..34e44cd52353 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -437,13 +437,18 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>>   			       VRR_VSYNC_START(crtc_state->vrr.vsync_start));
>>   }
>>   
>> +static bool intel_vrr_use_push(const struct intel_crtc_state *crtc_state)
>> +{
>> +	return crtc_state->vrr.flipline != crtc_state->vrr.vmax;
>> +}
>> +
>>   void intel_vrr_send_push(struct intel_dsb *dsb,
>>   			 const struct intel_crtc_state *crtc_state)
>>   {
>>   	struct intel_display *display = to_intel_display(crtc_state);
>>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>   
>> -	if (!intel_vrrtg_is_enabled(crtc_state))
>> +	if (!intel_vrr_use_push(crtc_state))
>>   		return;
>>   
>>   	if (dsb)
>> @@ -462,7 +467,7 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
>>   	struct intel_display *display = to_intel_display(crtc_state);
>>   	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>   
>> -	if (!intel_vrrtg_is_enabled(crtc_state))
>> +	if (!intel_vrr_use_push(crtc_state))
>>   		return false;
>>   
>>   	return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND;
>> @@ -476,8 +481,9 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
>>   	if (!intel_vrrtg_is_enabled(crtc_state))
>>   		return;
>>   
>> -	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
>> -		       TRANS_PUSH_EN);
>> +	if (intel_vrr_use_push(crtc_state))
>> +		intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
>> +			       TRANS_PUSH_EN);
> What I saw on TGL and ADL is that if you don't enable PUSH then nothing
> works. So I think we just want to have this enabled whenever the VRR
> timing generator is enabled.


Then perhaps I'll drop this patch, or should we have a specific check 
for TGL/ADL (though we are not enabling fixed refresh rate on these as yet).

Regards,

Ankit

>>   
>>   	if (crtc_state->vrr.mode == INTEL_VRRTG_MODE_CMRR) {
>>   		intel_de_write(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 65bbe40881d6..34e44cd52353 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -437,13 +437,18 @@  void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
 			       VRR_VSYNC_START(crtc_state->vrr.vsync_start));
 }
 
+static bool intel_vrr_use_push(const struct intel_crtc_state *crtc_state)
+{
+	return crtc_state->vrr.flipline != crtc_state->vrr.vmax;
+}
+
 void intel_vrr_send_push(struct intel_dsb *dsb,
 			 const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
-	if (!intel_vrrtg_is_enabled(crtc_state))
+	if (!intel_vrr_use_push(crtc_state))
 		return;
 
 	if (dsb)
@@ -462,7 +467,7 @@  bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state)
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
-	if (!intel_vrrtg_is_enabled(crtc_state))
+	if (!intel_vrr_use_push(crtc_state))
 		return false;
 
 	return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND;
@@ -476,8 +481,9 @@  void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
 	if (!intel_vrrtg_is_enabled(crtc_state))
 		return;
 
-	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
-		       TRANS_PUSH_EN);
+	if (intel_vrr_use_push(crtc_state))
+		intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
+			       TRANS_PUSH_EN);
 
 	if (crtc_state->vrr.mode == INTEL_VRRTG_MODE_CMRR) {
 		intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),