[PATCH 16/20] drm/i915/display: Use fixed rr timings in intel_set_transcoder_timings_lrr()
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Thu Feb 27 10:28:58 UTC 2025
On 2/26/2025 8:30 PM, Ville Syrjälä wrote:
> On Mon, Feb 24, 2025 at 11:47:13AM +0530, Ankit Nautiyal wrote:
>> To have Guardband/Pipeline_full reconfigured seamlessly, move the
>> guardband and pipeline_full checks out from the pure !fastset block in
>> intel_pipe_config_compare().
>> Update the intel_set_transcoder_timings_lrr() function to use
>> fixed refresh rate timings for platforms which always use
>> VRR timing generator.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_display.c | 43 ++++++++++++++++----
>> drivers/gpu/drm/i915/display/intel_vrr.c | 13 +++++-
>> drivers/gpu/drm/i915/display/intel_vrr.h | 2 +
>> 3 files changed, 48 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 752ed44d8892..d3c12bc3645c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -2854,6 +2854,9 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
>> intel_de_write(dev_priv, TRANS_VTOTAL(dev_priv, cpu_transcoder),
>> VACTIVE(crtc_vdisplay - 1) |
>> VTOTAL(crtc_vtotal - 1));
>> +
>> + intel_vrr_set_fixed_rr_timings(crtc_state);
>> + intel_vrr_update_guardband(crtc_state, false);
>> }
>>
>> static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
>> @@ -5127,17 +5130,36 @@ pipe_config_cx0pll_mismatch(struct drm_printer *p, bool fastset,
>> intel_cx0pll_dump_hw_state(display, b);
>> }
>>
>> -static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_state)
>> +static
>> +bool allow_vrr_guardband_fastset(const struct intel_crtc_state *old_crtc_state,
>> + const struct intel_crtc_state *new_crtc_state)
>> +{
>> + struct intel_display *display = to_intel_display(old_crtc_state);
>> +
>> + if (!HAS_VRR(display))
>> + return false;
>> +
>> + if (DISPLAY_VER(display) >= 13)
>> + return new_crtc_state->vrr.guardband != old_crtc_state->vrr.guardband;
>> +
>> + return new_crtc_state->vrr.pipeline_full != old_crtc_state->vrr.pipeline_full;
>> +}
> None of this stuff should be necessary. allow_vblank_delay_fastset()
> only says whether we're allowed the fastset or not. It isn't supposed
> to check if it's necessary or not.
>
>> +
>> +static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_state,
>> + const struct intel_crtc_state *new_crtc_state)
>> {
>> struct intel_display *display = to_intel_display(old_crtc_state);
>>
>> /*
>> - * Allow fastboot to fix up vblank delay (handled via LRR
>> - * codepaths), a bit dodgy as the registers aren't
>> + * Allow fastboot to fix up vblank delay, vrr guardband/pipeline_full
>> + * (handled via LRR codepaths), a bit dodgy as the registers aren't
>> * double buffered but seems to be working more or less...
>> */
>> - return HAS_LRR(display) && old_crtc_state->inherited &&
>> - !intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);
>> + if (!HAS_LRR(display) || intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI))
>> + return false;
>> +
>> + return old_crtc_state->inherited ||
>> + allow_vrr_guardband_fastset(old_crtc_state, new_crtc_state);
>> }
>>
>> bool
>> @@ -5273,7 +5295,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>> PIPE_CONF_CHECK_I(name.crtc_hsync_start); \
>> PIPE_CONF_CHECK_I(name.crtc_hsync_end); \
>> PIPE_CONF_CHECK_I(name.crtc_vdisplay); \
>> - if (!fastset || !allow_vblank_delay_fastset(current_config)) \
>> + if (!fastset || !allow_vblank_delay_fastset(current_config, pipe_config)) \
>> PIPE_CONF_CHECK_I(name.crtc_vblank_start); \
>> PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
>> PIPE_CONF_CHECK_I(name.crtc_vsync_end); \
>> @@ -5569,8 +5591,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);
>> @@ -5578,6 +5598,11 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>> PIPE_CONF_CHECK_BOOL(cmrr.enable);
>> }
>>
>> + if (!fastset || !allow_vblank_delay_fastset(current_config, pipe_config)) {
>> + PIPE_CONF_CHECK_I(vrr.guardband);
>> + PIPE_CONF_CHECK_I(vrr.pipeline_full);
>> + }
> I'd like to see one patch that just moves these completely out
> of the !fastset block, and a second patch that adds the fastboot
> exception. That way if we do see any problems from the live
> guardband reprogramming we can just revert it.
I have one doubt.
If we remove guardband/pipeline_full from the !fastset block, the
VRR-seamless-switch case will start failing.
This is because the test tries a virtual LRR mode, resulting in a change
in the VRR guardband.
Since its not in fastset block, we will need a full modeset, but the
test expects a seamless switch.
Regards,
Ankit
>
> Hmm, except you haven't changed the vrr_compute_config() yet at
> this point in the series, so changing this would break both fastset
> and state checker, I think. So we might have to move this patch to be
> later in the series.
>
>> +
>> #undef PIPE_CONF_CHECK_X
>> #undef PIPE_CONF_CHECK_I
>> #undef PIPE_CONF_CHECK_LLI
>> @@ -5875,7 +5900,7 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta
>> drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] fastset requirement not met, forcing full modeset\n",
>> crtc->base.base.id, crtc->base.name);
>> } else {
>> - if (allow_vblank_delay_fastset(old_crtc_state))
>> + if (allow_vblank_delay_fastset(old_crtc_state, new_crtc_state))
>> new_crtc_state->update_lrr = true;
>> new_crtc_state->uapi.mode_changed = false;
>> }
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
>> index 11b7edbdb116..e77f5b483b09 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
>> @@ -284,7 +284,6 @@ 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_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
>> {
>> struct intel_display *display = to_intel_display(crtc_state);
>> @@ -677,3 +676,15 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>> if (crtc_state->vrr.enable)
>> crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
>> }
>> +
>> +void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, bool needs_modeset)
>> +{
>> + struct intel_display *display = to_intel_display(crtc_state);
>> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>> + u32 vrr_ctl = trans_vrr_ctl(crtc_state);
>> +
>> + if (intel_vrr_always_use_vrr_tg(display) && !needs_modeset)
>> + vrr_ctl |= VRR_CTL_VRR_ENABLE;
>> +
>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), vrr_ctl);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
>> index 514822577e8a..e4e9cadcdf9e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
>> @@ -35,5 +35,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state);
>> int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
>> int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
>> int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state);
>> +void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, bool needs_modeset);
>> +void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state);
>>
>> #endif /* __INTEL_VRR_H__ */
>> --
>> 2.45.2
More information about the Intel-gfx
mailing list