| Message ID | 20230818190501.241062-1-navaremanasi@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] drm/i915/display: Dual refresh rate fastset fixes with VRR fastset | expand |
Hi Jani, I have added the Fixes tag as well as the gitlab issue that this patch fixes as per your feedback on prev rev (Rev3). Could you please review this version and provide your feedback so we can get this upstream fix merged. Thanks for your time and consideration. Regards Manasi On Fri, Aug 18, 2023 at 12:05 PM Manasi Navare <navaremanasi@chromium.org> wrote: > > Dual refresh rate (DRR) fastset seamlessly lets refresh rate > throttle without needing a full modeset. > However with the recent VRR fastset patches that got merged this > logic was broken. This is broken because now with VRR fastset > VRR parameters are calculated by default at the nominal refresh rate say 120Hz. > Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock > changes and this throws a mismatch in VRR parameters and fastset logic > for DRR gets thrown off and full modeset is indicated. > > This patch fixes this by ignoring the pipe mismatch for VRR parameters > in the case of DRR and when VRR is not enabled. With this fix, DRR > will still throttle seamlessly as long as VRR is not enabled. > > This will still need a full modeset for both DRR and VRR operating together > during the refresh rate throttle and then enabling VRR since now VRR > parameters need to be recomputed with new crtc clock and written to HW. > > This DRR + VRR fastset in conjunction needs more work in the driver and > will be fixed in later patches. > > v3: > Compute new VRR params whenever there is fastset and its non DRRS. > This will ensure it computes while switching to a fixed RR (Mitul) > > v2: > Check for pipe config mismatch in crtc clock whenever VRR is enabled > > Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 > Cc: Drew Davenport <ddavenport@chromium.org> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > Signed-off-by: Manasi Navare <navaremanasi@chromium.org> > --- > drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 763ab569d8f3..2cf3b22adaf7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > PIPE_CONF_CHECK_I(pipe_bpp); > > - if (!fastset || !pipe_config->seamless_m_n) { > + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { > PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); > PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); > } > @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > if (!fastset) > PIPE_CONF_CHECK_BOOL(vrr.enable); > - 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); > + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { > + 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); > + } > > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > -- > 2.42.0.rc1.204.g551eb34607-goog >
On Fri, 18 Aug 2023, Manasi Navare <navaremanasi@chromium.org> wrote: > Dual refresh rate (DRR) fastset seamlessly lets refresh rate > throttle without needing a full modeset. > However with the recent VRR fastset patches that got merged this > logic was broken. This is broken because now with VRR fastset > VRR parameters are calculated by default at the nominal refresh rate say 120Hz. > Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock > changes and this throws a mismatch in VRR parameters and fastset logic > for DRR gets thrown off and full modeset is indicated. > > This patch fixes this by ignoring the pipe mismatch for VRR parameters > in the case of DRR and when VRR is not enabled. With this fix, DRR > will still throttle seamlessly as long as VRR is not enabled. > > This will still need a full modeset for both DRR and VRR operating together > during the refresh rate throttle and then enabling VRR since now VRR > parameters need to be recomputed with new crtc clock and written to HW. > > This DRR + VRR fastset in conjunction needs more work in the driver and > will be fixed in later patches. > > v3: > Compute new VRR params whenever there is fastset and its non DRRS. > This will ensure it computes while switching to a fixed RR (Mitul) > > v2: > Check for pipe config mismatch in crtc clock whenever VRR is enabled > > Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") How could this have broken fastsets, when this made it possible to do vrr enable/disable fastsets to begin with? I was hoping to get a regressing commit to make this easier to reason. > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 > Cc: Drew Davenport <ddavenport@chromium.org> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > Signed-off-by: Manasi Navare <navaremanasi@chromium.org> > --- > drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 763ab569d8f3..2cf3b22adaf7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > PIPE_CONF_CHECK_I(pipe_bpp); > > - if (!fastset || !pipe_config->seamless_m_n) { > + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { > PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); > PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); > } > @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > if (!fastset) > PIPE_CONF_CHECK_BOOL(vrr.enable); > - 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); > + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { I just don't get the conditions here and above. For example, why wouldn't we check the parameters e.g. on full modeset that disables vrr? I think we'll need a matrix of the features, which of them can be combined together, which are mutually exclusive, and which are expected to be fastsets. BR, Jani. > + 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); > + } > > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I
On Thu, Aug 24, 2023 at 02:27:49PM +0300, Jani Nikula wrote: > On Fri, 18 Aug 2023, Manasi Navare <navaremanasi@chromium.org> wrote: > > Dual refresh rate (DRR) fastset seamlessly lets refresh rate > > throttle without needing a full modeset. > > However with the recent VRR fastset patches that got merged this > > logic was broken. This is broken because now with VRR fastset > > VRR parameters are calculated by default at the nominal refresh rate say 120Hz. > > Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock > > changes and this throws a mismatch in VRR parameters and fastset logic > > for DRR gets thrown off and full modeset is indicated. > > > > This patch fixes this by ignoring the pipe mismatch for VRR parameters > > in the case of DRR and when VRR is not enabled. With this fix, DRR > > will still throttle seamlessly as long as VRR is not enabled. > > > > This will still need a full modeset for both DRR and VRR operating together > > during the refresh rate throttle and then enabling VRR since now VRR > > parameters need to be recomputed with new crtc clock and written to HW. > > > > This DRR + VRR fastset in conjunction needs more work in the driver and > > will be fixed in later patches. > > > > v3: > > Compute new VRR params whenever there is fastset and its non DRRS. > > This will ensure it computes while switching to a fixed RR (Mitul) > > > > v2: > > Check for pipe config mismatch in crtc clock whenever VRR is enabled > > > > Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") > > How could this have broken fastsets, when this made it possible to do > vrr enable/disable fastsets to begin with? I was hoping to get a > regressing commit to make this easier to reason. > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 > > Cc: Drew Davenport <ddavenport@chromium.org> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > Signed-off-by: Manasi Navare <navaremanasi@chromium.org> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 763ab569d8f3..2cf3b22adaf7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > > PIPE_CONF_CHECK_I(pipe_bpp); > > > > - if (!fastset || !pipe_config->seamless_m_n) { > > + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { > > PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); > > PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); > > } > > @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > > if (!fastset) > > PIPE_CONF_CHECK_BOOL(vrr.enable); > > - 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); > > + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { > > I just don't get the conditions here and above. For example, why > wouldn't we check the parameters e.g. on full modeset that disables vrr? > > I think we'll need a matrix of the features, which of them can be > combined together, which are mutually exclusive, and which are expected > to be fastsets. I wouldn't expect a panel to support both DRRS and VRR. Pick one and stick to it. I haven't thought through all the implications of changing all the VRR parameters live, so fastsets doing that are currently not possible. I have a branch for LRR (which is essentially manual VRR) but that was writtent before the VRR fastset support, so needs a full redesign now. Until then I don't think we can do anything.
Hi Ville and Jani, Thanks for your review and feedback. To rightly explain the use case here: Case 1: We are at 120 Hz refresh rate so by default VRR registers are programmed for 120Hz case. And say the VRR range is 30-120Hz we get the full range here. Case 2: When we switch to downclock mode of 60Hz and if a game is being played we want the VRR parameters to be updated in a fastset fashion to now reflect a new range of 30-60Hz. These use cases would need both VRR and DRR to work together and seamlessly. I tried updating VRR parameters in update_crtc() hook to do this and that works, but that will be a different patch series. But until then, this patch tries to make sure atleast DRR works correctly and seamlessly switches between the refresh rates when VRR is not enabled. This would be are first step to atleast fix the fastset of DRR that was broken when VRR fastset was enabled since thats when VRR parameters started to get computed always by default. @Ville Syrjälä @Jani Nikula : Based on your feedback, may be I can use the vrr_enabling and vrr_disabling() and only during these conditions is when it can check the pipe mismatch for the VRR parameters and in all other cases when it is fastset + seamless_m_n we can ignore the VRR pipe mismatches? Let me know if this looks like a good approach or what would be your suggestion on excluding the pipe mismatches for VRR params in case of DRRS and fastset mode? Regards Manasi On Thu, Aug 24, 2023 at 4:41 AM Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Thu, Aug 24, 2023 at 02:27:49PM +0300, Jani Nikula wrote: > > On Fri, 18 Aug 2023, Manasi Navare <navaremanasi@chromium.org> wrote: > > > Dual refresh rate (DRR) fastset seamlessly lets refresh rate > > > throttle without needing a full modeset. > > > However with the recent VRR fastset patches that got merged this > > > logic was broken. This is broken because now with VRR fastset > > > VRR parameters are calculated by default at the nominal refresh rate say 120Hz. > > > Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock > > > changes and this throws a mismatch in VRR parameters and fastset logic > > > for DRR gets thrown off and full modeset is indicated. > > > > > > This patch fixes this by ignoring the pipe mismatch for VRR parameters > > > in the case of DRR and when VRR is not enabled. With this fix, DRR > > > will still throttle seamlessly as long as VRR is not enabled. > > > > > > This will still need a full modeset for both DRR and VRR operating together > > > during the refresh rate throttle and then enabling VRR since now VRR > > > parameters need to be recomputed with new crtc clock and written to HW. > > > > > > This DRR + VRR fastset in conjunction needs more work in the driver and > > > will be fixed in later patches. > > > > > > v3: > > > Compute new VRR params whenever there is fastset and its non DRRS. > > > This will ensure it computes while switching to a fixed RR (Mitul) > > > > > > v2: > > > Check for pipe config mismatch in crtc clock whenever VRR is enabled > > > > > > Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") > > > > How could this have broken fastsets, when this made it possible to do > > vrr enable/disable fastsets to begin with? I was hoping to get a > > regressing commit to make this easier to reason. > > > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 > > > Cc: Drew Davenport <ddavenport@chromium.org> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Sean Paul <seanpaul@chromium.org> > > > Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > > Signed-off-by: Manasi Navare <navaremanasi@chromium.org> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index 763ab569d8f3..2cf3b22adaf7 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > > > PIPE_CONF_CHECK_I(pipe_bpp); > > > > > > - if (!fastset || !pipe_config->seamless_m_n) { > > > + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { > > > PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); > > > PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); > > > } > > > @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > > > > if (!fastset) > > > PIPE_CONF_CHECK_BOOL(vrr.enable); > > > - 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); > > > + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { > > > > I just don't get the conditions here and above. For example, why > > wouldn't we check the parameters e.g. on full modeset that disables vrr? > > > > I think we'll need a matrix of the features, which of them can be > > combined together, which are mutually exclusive, and which are expected > > to be fastsets. > > I wouldn't expect a panel to support both DRRS and VRR. Pick one and > stick to it. > > I haven't thought through all the implications of changing all the VRR > parameters live, so fastsets doing that are currently not possible. I > have a branch for LRR (which is essentially manual VRR) but that was > writtent before the VRR fastset support, so needs a full redesign now. > Until then I don't think we can do anything. > > -- > Ville Syrjälä > Intel
Hi @Jani Nikula , Thanks for your feedback. Please find my comments below: On Thu, Aug 24, 2023 at 4:27 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 18 Aug 2023, Manasi Navare <navaremanasi@chromium.org> wrote: > > Dual refresh rate (DRR) fastset seamlessly lets refresh rate > > throttle without needing a full modeset. > > However with the recent VRR fastset patches that got merged this > > logic was broken. This is broken because now with VRR fastset > > VRR parameters are calculated by default at the nominal refresh rate say 120Hz. > > Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock > > changes and this throws a mismatch in VRR parameters and fastset logic > > for DRR gets thrown off and full modeset is indicated. > > > > This patch fixes this by ignoring the pipe mismatch for VRR parameters > > in the case of DRR and when VRR is not enabled. With this fix, DRR > > will still throttle seamlessly as long as VRR is not enabled. > > > > This will still need a full modeset for both DRR and VRR operating together > > during the refresh rate throttle and then enabling VRR since now VRR > > parameters need to be recomputed with new crtc clock and written to HW. > > > > This DRR + VRR fastset in conjunction needs more work in the driver and > > will be fixed in later patches. > > > > v3: > > Compute new VRR params whenever there is fastset and its non DRRS. > > This will ensure it computes while switching to a fixed RR (Mitul) > > > > v2: > > Check for pipe config mismatch in crtc clock whenever VRR is enabled > > > > Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") > > How could this have broken fastsets, when this made it possible to do > vrr enable/disable fastsets to begin with? I was hoping to get a > regressing commit to make this easier to reason. Actually this patch enabled VRR fastsets and because of that it actually broke the logic of DRR/DRRS fastsets. To pinpoint the exact regression patch is the one that started computing VRR parameters always irrespective of VRR enable because that started causing pipe mismatches in the working DRR fastset case. That patch would be : "drm/i915/vrr: Relocate VRR enable/disable". I will add this exact regression patch there as per your feedback. Regards Manasi > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 > > Cc: Drew Davenport <ddavenport@chromium.org> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > > Signed-off-by: Manasi Navare <navaremanasi@chromium.org> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 763ab569d8f3..2cf3b22adaf7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) > > PIPE_CONF_CHECK_I(pipe_bpp); > > > > - if (!fastset || !pipe_config->seamless_m_n) { > > + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { > > PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); > > PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); > > } > > @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > > if (!fastset) > > PIPE_CONF_CHECK_BOOL(vrr.enable); > > - 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); > > + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { > > I just don't get the conditions here and above. For example, why > wouldn't we check the parameters e.g. on full modeset that disables vrr? > > I think we'll need a matrix of the features, which of them can be > combined together, which are mutually exclusive, and which are expected > to be fastsets. > > BR, > Jani. > > > > + 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); > > + } > > > > #undef PIPE_CONF_CHECK_X > > #undef PIPE_CONF_CHECK_I > > -- > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 763ab569d8f3..2cf3b22adaf7 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -5352,7 +5352,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, if (IS_G4X(dev_priv) || DISPLAY_VER(dev_priv) >= 5) PIPE_CONF_CHECK_I(pipe_bpp); - if (!fastset || !pipe_config->seamless_m_n) { + if (!fastset || !pipe_config->seamless_m_n || pipe_config->vrr.enable) { PIPE_CONF_CHECK_I(hw.pipe_mode.crtc_clock); PIPE_CONF_CHECK_I(hw.adjusted_mode.crtc_clock); } @@ -5387,11 +5387,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, if (!fastset) PIPE_CONF_CHECK_BOOL(vrr.enable); - 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); + if ((fastset && !pipe_config->seamless_m_n) || pipe_config->vrr.enable) { + 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); + } #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I
Dual refresh rate (DRR) fastset seamlessly lets refresh rate throttle without needing a full modeset. However with the recent VRR fastset patches that got merged this logic was broken. This is broken because now with VRR fastset VRR parameters are calculated by default at the nominal refresh rate say 120Hz. Now when DRR throttle happens to switch refresh rate to 60Hz, crtc clock changes and this throws a mismatch in VRR parameters and fastset logic for DRR gets thrown off and full modeset is indicated. This patch fixes this by ignoring the pipe mismatch for VRR parameters in the case of DRR and when VRR is not enabled. With this fix, DRR will still throttle seamlessly as long as VRR is not enabled. This will still need a full modeset for both DRR and VRR operating together during the refresh rate throttle and then enabling VRR since now VRR parameters need to be recomputed with new crtc clock and written to HW. This DRR + VRR fastset in conjunction needs more work in the driver and will be fixed in later patches. v3: Compute new VRR params whenever there is fastset and its non DRRS. This will ensure it computes while switching to a fixed RR (Mitul) v2: Check for pipe config mismatch in crtc clock whenever VRR is enabled Fixes: 1af1d18825d3 ("drm/i915/vrr: Allow VRR to be toggled during fastsets") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9154 Cc: Drew Davenport <ddavenport@chromium.org> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> Signed-off-by: Manasi Navare <navaremanasi@chromium.org> --- drivers/gpu/drm/i915/display/intel_display.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)