diff mbox series

[17/21] drm/i915/display: Move vrr.guardband/pipeline_full out of !fastset block

Message ID 20250306131100.3989503-18-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 6, 2025, 1:10 p.m. UTC
The vrr.guardband/pipeline_full depend on the vrr.vmin. Since we have
set vrr.vmin to adjusted_mode->crtc_vtotal, this shouldn't change on the
fly. With this we can move vrr.guardband/pipeline_full out from !fastset
block.

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

Comments

Ville Syrjala March 7, 2025, 2:26 p.m. UTC | #1
On Thu, Mar 06, 2025 at 06:40:56PM +0530, Ankit Nautiyal wrote:
> The vrr.guardband/pipeline_full depend on the vrr.vmin. Since we have
> set vrr.vmin to adjusted_mode->crtc_vtotal, this shouldn't change on the
> fly. With this we can move vrr.guardband/pipeline_full out from !fastset
> block.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 322a05648f58..a642496e366c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5393,8 +5393,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  		PIPE_CONF_CHECK_I(vrr.vmin);
>  		PIPE_CONF_CHECK_I(vrr.vmax);
>  		PIPE_CONF_CHECK_I(vrr.flipline);
> -		PIPE_CONF_CHECK_I(vrr.pipeline_full);
> -		PIPE_CONF_CHECK_I(vrr.guardband);
>  		PIPE_CONF_CHECK_I(vrr.vsync_start);
>  		PIPE_CONF_CHECK_I(vrr.vsync_end);
>  		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> @@ -5402,6 +5400,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  		PIPE_CONF_CHECK_BOOL(cmrr.enable);
>  	}
>  
> +	PIPE_CONF_CHECK_I(vrr.pipeline_full);
> +	PIPE_CONF_CHECK_I(vrr.guardband);


Assuming we can't reprogram the guardband safely live 
I think this would have to become 

if (!fastset || always_use_vrr_tg()) {
        ...
}

which avoids breaking the LRR fastset on older platforms.

Whether we can still risk the fastboot exception for the new
platforms I don't know. I guess leave it out for now and
ponder it further later.

In the future I guess one option to resurrecting the fastsets
would be to somehow use a more fixed size gurdband instead of
the full vblank length, but that would need a lot of thought
and work, so definitely not something we can just do right
now.

> +
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_LLI
> -- 
> 2.45.2
Ankit Nautiyal March 9, 2025, 4:35 p.m. UTC | #2
On 3/7/2025 7:56 PM, Ville Syrjälä wrote:
> On Thu, Mar 06, 2025 at 06:40:56PM +0530, Ankit Nautiyal wrote:
>> The vrr.guardband/pipeline_full depend on the vrr.vmin. Since we have
>> set vrr.vmin to adjusted_mode->crtc_vtotal, this shouldn't change on the
>> fly. With this we can move vrr.guardband/pipeline_full out from !fastset
>> block.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 322a05648f58..a642496e366c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -5393,8 +5393,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>>   		PIPE_CONF_CHECK_I(vrr.vmin);
>>   		PIPE_CONF_CHECK_I(vrr.vmax);
>>   		PIPE_CONF_CHECK_I(vrr.flipline);
>> -		PIPE_CONF_CHECK_I(vrr.pipeline_full);
>> -		PIPE_CONF_CHECK_I(vrr.guardband);
>>   		PIPE_CONF_CHECK_I(vrr.vsync_start);
>>   		PIPE_CONF_CHECK_I(vrr.vsync_end);
>>   		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
>> @@ -5402,6 +5400,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>>   		PIPE_CONF_CHECK_BOOL(cmrr.enable);
>>   	}
>>   
>> +	PIPE_CONF_CHECK_I(vrr.pipeline_full);
>> +	PIPE_CONF_CHECK_I(vrr.guardband);
>
> Assuming we can't reprogram the guardband safely live
> I think this would have to become
>
> if (!fastset || always_use_vrr_tg()) {
>          ...
> }
>
> which avoids breaking the LRR fastset on older platforms.
>
> Whether we can still risk the fastboot exception for the new
> platforms I don't know. I guess leave it out for now and
> ponder it further later.
>
> In the future I guess one option to resurrecting the fastsets
> would be to somehow use a more fixed size gurdband instead of
> the full vblank length, but that would need a lot of thought
> and work, so definitely not something we can just do right
> now.


Yes I agree.

Will have the guardband under the new check.

Do we need to let pipeline_full as it is in !fastset since 
always_use_vrr_tg() is anyways false for older platforms? Or we keep it 
along with guardband?


Regards,

Ankit


>
>> +
>>   #undef PIPE_CONF_CHECK_X
>>   #undef PIPE_CONF_CHECK_I
>>   #undef PIPE_CONF_CHECK_LLI
>> -- 
>> 2.45.2
Ville Syrjala March 10, 2025, 3:52 p.m. UTC | #3
On Sun, Mar 09, 2025 at 10:05:48PM +0530, Nautiyal, Ankit K wrote:
> 
> On 3/7/2025 7:56 PM, Ville Syrjälä wrote:
> > On Thu, Mar 06, 2025 at 06:40:56PM +0530, Ankit Nautiyal wrote:
> >> The vrr.guardband/pipeline_full depend on the vrr.vmin. Since we have
> >> set vrr.vmin to adjusted_mode->crtc_vtotal, this shouldn't change on the
> >> fly. With this we can move vrr.guardband/pipeline_full out from !fastset
> >> block.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 322a05648f58..a642496e366c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -5393,8 +5393,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> >>   		PIPE_CONF_CHECK_I(vrr.vmin);
> >>   		PIPE_CONF_CHECK_I(vrr.vmax);
> >>   		PIPE_CONF_CHECK_I(vrr.flipline);
> >> -		PIPE_CONF_CHECK_I(vrr.pipeline_full);
> >> -		PIPE_CONF_CHECK_I(vrr.guardband);
> >>   		PIPE_CONF_CHECK_I(vrr.vsync_start);
> >>   		PIPE_CONF_CHECK_I(vrr.vsync_end);
> >>   		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> >> @@ -5402,6 +5400,9 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
> >>   		PIPE_CONF_CHECK_BOOL(cmrr.enable);
> >>   	}
> >>   
> >> +	PIPE_CONF_CHECK_I(vrr.pipeline_full);
> >> +	PIPE_CONF_CHECK_I(vrr.guardband);
> >
> > Assuming we can't reprogram the guardband safely live
> > I think this would have to become
> >
> > if (!fastset || always_use_vrr_tg()) {
> >          ...
> > }
> >
> > which avoids breaking the LRR fastset on older platforms.
> >
> > Whether we can still risk the fastboot exception for the new
> > platforms I don't know. I guess leave it out for now and
> > ponder it further later.
> >
> > In the future I guess one option to resurrecting the fastsets
> > would be to somehow use a more fixed size gurdband instead of
> > the full vblank length, but that would need a lot of thought
> > and work, so definitely not something we can just do right
> > now.
> 
> 
> Yes I agree.
> 
> Will have the guardband under the new check.
> 
> Do we need to let pipeline_full as it is in !fastset since 
> always_use_vrr_tg() is anyways false for older platforms? Or we keep it 
> along with guardband?

I think we want to keep the two together. That way we can also
easily test the always_use_vrr_tg() codepaths on older platforms.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 322a05648f58..a642496e366c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5393,8 +5393,6 @@  intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_I(vrr.vmin);
 		PIPE_CONF_CHECK_I(vrr.vmax);
 		PIPE_CONF_CHECK_I(vrr.flipline);
-		PIPE_CONF_CHECK_I(vrr.pipeline_full);
-		PIPE_CONF_CHECK_I(vrr.guardband);
 		PIPE_CONF_CHECK_I(vrr.vsync_start);
 		PIPE_CONF_CHECK_I(vrr.vsync_end);
 		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
@@ -5402,6 +5400,9 @@  intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_BOOL(cmrr.enable);
 	}
 
+	PIPE_CONF_CHECK_I(vrr.pipeline_full);
+	PIPE_CONF_CHECK_I(vrr.guardband);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_LLI