| Message ID | 20201022222709.29386-6-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VRR/Adaptive Sync enabling in i915 | expand |
On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > This forces a complete modeset if vrr drm crtc state goes > from enabled to disabled and vice versa. > This patch also computes vrr state variables from the mode timings > and based on the vrr property set by userspace as well as hardware's > vrr capability. > > v2: > *Rebase > v3: > * Vmin = max (vtotal, vmin) (Manasi) > v4: > * set crtc_state->vrr.enable = 0 for disable request > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 7 +++- > drivers/gpu/drm/i915/display/intel_dp.c | 1 + > drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_vrr.h | 2 ++ > 4 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index f41b6f8b5618..f70cc3b2a1a4 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > PIPE_CONF_CHECK_I(mst_master_transcoder); > > + PIPE_CONF_CHECK_BOOL(vrr.enable); > + PIPE_CONF_CHECK_I(vrr.vtotalmin); > + PIPE_CONF_CHECK_I(vrr.vtotalmax); > + > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > #undef PIPE_CONF_CHECK_BOOL > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, > > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > - if (new_crtc_state->inherited != old_crtc_state->inherited) > + if (new_crtc_state->inherited != old_crtc_state->inherited || > + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) Somehow this feels like a really specific check to add considering the abstraction level of the function in general. > new_crtc_state->uapi.mode_changed = true; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 3794b8f35edc..3185c4ca523d 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, > if (!HAS_DDI(dev_priv)) > intel_dp_set_clock(encoder, pipe_config); > > + intel_vrr_compute_config(intel_dp, pipe_config); > intel_psr_compute_config(intel_dp, pipe_config); > intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > constant_n); > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 0c8a91fabb64..56114f535f94 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector) > info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10; > } > > +void > +intel_vrr_compute_config(struct intel_dp *intel_dp, > + struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > + const struct drm_display_info *info = &connector->display_info; > + > + if (!intel_is_vrr_capable(connector)) > + return; > + > + if (!crtc_state->uapi.vrr_enabled) { > + drm_dbg(&dev_priv->drm, > + "VRR disable requested by Userspace\n"); drm_dbg_kms, though is this useful information? Quite a bit of log spam I'd think. > + crtc_state->vrr.enable = false; > + return; > + } > + > + crtc_state->vrr.enable = true; > + crtc_state->vrr.vtotalmin = > + max_t(u16, adjusted_mode->crtc_vtotal, > + DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000, > + adjusted_mode->crtc_htotal * > + info->monitor_range.max_vfreq)); > + crtc_state->vrr.vtotalmax = > + max_t(u16, adjusted_mode->crtc_vtotal, > + DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, > + adjusted_mode->crtc_htotal * > + info->monitor_range.min_vfreq)); > + > + drm_dbg(&dev_priv->drm, drm_dbg_kms > + "VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n", > + yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin, > + crtc_state->vrr.vtotalmax); > +} > + > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 755746c7525c..1e6fe8fe92ec 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -15,5 +15,7 @@ struct intel_encoder; > struct intel_dp; > > bool intel_is_vrr_capable(struct drm_connector *connector); > +void intel_vrr_compute_config(struct intel_dp *intel_dp, > + struct intel_crtc_state *crtc_state); > > #endif /* __INTEL_VRR_H__ */
On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote: > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > > This forces a complete modeset if vrr drm crtc state goes > > from enabled to disabled and vice versa. > > This patch also computes vrr state variables from the mode timings > > and based on the vrr property set by userspace as well as hardware's > > vrr capability. > > > > v2: > > *Rebase > > v3: > > * Vmin = max (vtotal, vmin) (Manasi) > > v4: > > * set crtc_state->vrr.enable = 0 for disable request > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++- > > drivers/gpu/drm/i915/display/intel_dp.c | 1 + > > drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_vrr.h | 2 ++ > > 4 files changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index f41b6f8b5618..f70cc3b2a1a4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > > PIPE_CONF_CHECK_I(mst_master_transcoder); > > > > + PIPE_CONF_CHECK_BOOL(vrr.enable); > > + PIPE_CONF_CHECK_I(vrr.vtotalmin); > > + PIPE_CONF_CHECK_I(vrr.vtotalmax); > > + > > #undef PIPE_CONF_CHECK_X > > #undef PIPE_CONF_CHECK_I > > #undef PIPE_CONF_CHECK_BOOL > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, > > > > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > > new_crtc_state, i) { > > - if (new_crtc_state->inherited != old_crtc_state->inherited) > > + if (new_crtc_state->inherited != old_crtc_state->inherited || > > + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) > > Somehow this feels like a really specific check to add considering the > abstraction level of the function in general. Should I then create a separate function to force a full modeset by setting mode changed if vrr_enabled changed? And call that from intel_atomic_check() ? > > > new_crtc_state->uapi.mode_changed = true; > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 3794b8f35edc..3185c4ca523d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > if (!HAS_DDI(dev_priv)) > > intel_dp_set_clock(encoder, pipe_config); > > > > + intel_vrr_compute_config(intel_dp, pipe_config); > > intel_psr_compute_config(intel_dp, pipe_config); > > intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > > constant_n); > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > > index 0c8a91fabb64..56114f535f94 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector) > > info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10; > > } > > > > +void > > +intel_vrr_compute_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state *crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > + struct drm_connector *connector = &intel_connector->base; > > + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > > + const struct drm_display_info *info = &connector->display_info; > > + > > + if (!intel_is_vrr_capable(connector)) > > + return; > > + > > + if (!crtc_state->uapi.vrr_enabled) { > > + drm_dbg(&dev_priv->drm, > > + "VRR disable requested by Userspace\n"); > > drm_dbg_kms, though is this useful information? Quite a bit of log spam > I'd think. Yea this one can probably remove Manasi > > > + crtc_state->vrr.enable = false; > > + return; > > + } > > + > > + crtc_state->vrr.enable = true; > > + crtc_state->vrr.vtotalmin = > > + max_t(u16, adjusted_mode->crtc_vtotal, > > + DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000, > > + adjusted_mode->crtc_htotal * > > + info->monitor_range.max_vfreq)); > > + crtc_state->vrr.vtotalmax = > > + max_t(u16, adjusted_mode->crtc_vtotal, > > + DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, > > + adjusted_mode->crtc_htotal * > > + info->monitor_range.min_vfreq)); > > + > > + drm_dbg(&dev_priv->drm, > > drm_dbg_kms > > > + "VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n", > > + yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin, > > + crtc_state->vrr.vtotalmax); > > +} > > + > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > > index 755746c7525c..1e6fe8fe92ec 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -15,5 +15,7 @@ struct intel_encoder; > > struct intel_dp; > > > > bool intel_is_vrr_capable(struct drm_connector *connector); > > +void intel_vrr_compute_config(struct intel_dp *intel_dp, > > + struct intel_crtc_state *crtc_state); > > > > #endif /* __INTEL_VRR_H__ */ > > -- > Jani Nikula, Intel Open Source Graphics Center
On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote: > On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote: > > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > This forces a complete modeset if vrr drm crtc state goes > > > from enabled to disabled and vice versa. > > > This patch also computes vrr state variables from the mode timings > > > and based on the vrr property set by userspace as well as hardware's > > > vrr capability. > > > > > > v2: > > > *Rebase > > > v3: > > > * Vmin = max (vtotal, vmin) (Manasi) > > > v4: > > > * set crtc_state->vrr.enable = 0 for disable request > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 7 +++- > > > drivers/gpu/drm/i915/display/intel_dp.c | 1 + > > > drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++ > > > drivers/gpu/drm/i915/display/intel_vrr.h | 2 ++ > > > 4 files changed, 47 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index f41b6f8b5618..f70cc3b2a1a4 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, > > > > > > PIPE_CONF_CHECK_I(mst_master_transcoder); > > > > > > + PIPE_CONF_CHECK_BOOL(vrr.enable); > > > + PIPE_CONF_CHECK_I(vrr.vtotalmin); > > > + PIPE_CONF_CHECK_I(vrr.vtotalmax); > > > + > > > #undef PIPE_CONF_CHECK_X > > > #undef PIPE_CONF_CHECK_I > > > #undef PIPE_CONF_CHECK_BOOL > > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, > > > > > > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > > > new_crtc_state, i) { > > > - if (new_crtc_state->inherited != old_crtc_state->inherited) > > > + if (new_crtc_state->inherited != old_crtc_state->inherited || > > > + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) > > > > Somehow this feels like a really specific check to add considering the > > abstraction level of the function in general. Actually while discussing with @Ville on IRC, he had proposed just adding it here since we already have this loop existing that loops through the old and new crtc states and we need to set the mode_changed = true right up at the top. But if you think its more intuitive to create a separate function for this I could do that Ville, Jani N any thoughts? Manasi > > Should I then create a separate function to force a full modeset by setting mode changed > if vrr_enabled changed? > And call that from intel_atomic_check() ? > > > > > > new_crtc_state->uapi.mode_changed = true; > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 3794b8f35edc..3185c4ca523d 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, > > > if (!HAS_DDI(dev_priv)) > > > intel_dp_set_clock(encoder, pipe_config); > > > > > > + intel_vrr_compute_config(intel_dp, pipe_config); > > > intel_psr_compute_config(intel_dp, pipe_config); > > > intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, > > > constant_n); > > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > > > index 0c8a91fabb64..56114f535f94 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > > @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector) > > > info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10; > > > } > > > > > > +void > > > +intel_vrr_compute_config(struct intel_dp *intel_dp, > > > + struct intel_crtc_state *crtc_state) > > > +{ > > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > > + struct drm_connector *connector = &intel_connector->base; > > > + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; > > > + const struct drm_display_info *info = &connector->display_info; > > > + > > > + if (!intel_is_vrr_capable(connector)) > > > + return; > > > + > > > + if (!crtc_state->uapi.vrr_enabled) { > > > + drm_dbg(&dev_priv->drm, > > > + "VRR disable requested by Userspace\n"); > > > > drm_dbg_kms, though is this useful information? Quite a bit of log spam > > I'd think. > > Yea this one can probably remove > > Manasi > > > > > > > + crtc_state->vrr.enable = false; > > > + return; > > > + } > > > + > > > + crtc_state->vrr.enable = true; > > > + crtc_state->vrr.vtotalmin = > > > + max_t(u16, adjusted_mode->crtc_vtotal, > > > + DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000, > > > + adjusted_mode->crtc_htotal * > > > + info->monitor_range.max_vfreq)); > > > + crtc_state->vrr.vtotalmax = > > > + max_t(u16, adjusted_mode->crtc_vtotal, > > > + DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, > > > + adjusted_mode->crtc_htotal * > > > + info->monitor_range.min_vfreq)); > > > + > > > + drm_dbg(&dev_priv->drm, > > > > drm_dbg_kms > > > > > + "VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n", > > > + yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin, > > > + crtc_state->vrr.vtotalmax); > > > +} > > > + > > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > > > index 755746c7525c..1e6fe8fe92ec 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > > @@ -15,5 +15,7 @@ struct intel_encoder; > > > struct intel_dp; > > > > > > bool intel_is_vrr_capable(struct drm_connector *connector); > > > +void intel_vrr_compute_config(struct intel_dp *intel_dp, > > > + struct intel_crtc_state *crtc_state); > > > > > > #endif /* __INTEL_VRR_H__ */ > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote: > On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote: >> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote: >> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: >> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, >> > > >> > > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, >> > > new_crtc_state, i) { >> > > - if (new_crtc_state->inherited != old_crtc_state->inherited) >> > > + if (new_crtc_state->inherited != old_crtc_state->inherited || >> > > + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) >> > >> > Somehow this feels like a really specific check to add considering the >> > abstraction level of the function in general. > > Actually while discussing with @Ville on IRC, he had proposed just adding it here > since we already have this loop existing that loops through the old and new crtc states > and we need to set the mode_changed = true right up at the top. > But if you think its more intuitive to create a separate function for this I could do that > > Ville, Jani N any thoughts? It's just a gut feeling. Kind of uneasy feeling that in the future people look at that, and see you have this check there, and then add more, and more. Anyway, whatever Ville says works for me as well. ;) BR, Jani.
On Thu, Dec 03, 2020 at 06:39:43PM +0200, Jani Nikula wrote: > On Wed, 02 Dec 2020, "Navare, Manasi" <manasi.d.navare@intel.com> wrote: > > On Tue, Dec 01, 2020 at 02:52:59PM -0800, Navare, Manasi wrote: > >> On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote: > >> > On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@intel.com> wrote: > >> > > @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, > >> > > > >> > > for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, > >> > > new_crtc_state, i) { > >> > > - if (new_crtc_state->inherited != old_crtc_state->inherited) > >> > > + if (new_crtc_state->inherited != old_crtc_state->inherited || > >> > > + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) > >> > > >> > Somehow this feels like a really specific check to add considering the > >> > abstraction level of the function in general. > > > > Actually while discussing with @Ville on IRC, he had proposed just adding it here > > since we already have this loop existing that loops through the old and new crtc states > > and we need to set the mode_changed = true right up at the top. > > But if you think its more intuitive to create a separate function for this I could do that > > > > Ville, Jani N any thoughts? > > It's just a gut feeling. Kind of uneasy feeling that in the future > people look at that, and see you have this check there, and then add > more, and more. > > Anyway, whatever Ville says works for me as well. ;) Yea I actually also agree reg this so let me just move this to a separate function where itw ill loop through crtc states and force mode changed for this condition. If Ville thinks otherwise we can remove it since I havent got any review comments from Ville yet. Manasi > > BR, > Jani. > > > -- > 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 f41b6f8b5618..f70cc3b2a1a4 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -14213,6 +14213,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config, PIPE_CONF_CHECK_I(mst_master_transcoder); + PIPE_CONF_CHECK_BOOL(vrr.enable); + PIPE_CONF_CHECK_I(vrr.vtotalmin); + PIPE_CONF_CHECK_I(vrr.vtotalmax); + #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I #undef PIPE_CONF_CHECK_BOOL @@ -15202,7 +15206,8 @@ static int intel_atomic_check(struct drm_device *dev, for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { - if (new_crtc_state->inherited != old_crtc_state->inherited) + if (new_crtc_state->inherited != old_crtc_state->inherited || + new_crtc_state->uapi.vrr_enabled != old_crtc_state->uapi.vrr_enabled) new_crtc_state->uapi.mode_changed = true; } diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 3794b8f35edc..3185c4ca523d 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2752,6 +2752,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, if (!HAS_DDI(dev_priv)) intel_dp_set_clock(encoder, pipe_config); + intel_vrr_compute_config(intel_dp, pipe_config); intel_psr_compute_config(intel_dp, pipe_config); intel_dp_drrs_compute_config(intel_dp, pipe_config, output_bpp, constant_n); diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 0c8a91fabb64..56114f535f94 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -26,3 +26,41 @@ bool intel_is_vrr_capable(struct drm_connector *connector) info->monitor_range.max_vfreq - info->monitor_range.min_vfreq > 10; } +void +intel_vrr_compute_config(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_connector *connector = &intel_connector->base; + struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode; + const struct drm_display_info *info = &connector->display_info; + + if (!intel_is_vrr_capable(connector)) + return; + + if (!crtc_state->uapi.vrr_enabled) { + drm_dbg(&dev_priv->drm, + "VRR disable requested by Userspace\n"); + crtc_state->vrr.enable = false; + return; + } + + crtc_state->vrr.enable = true; + crtc_state->vrr.vtotalmin = + max_t(u16, adjusted_mode->crtc_vtotal, + DIV_ROUND_CLOSEST(adjusted_mode->crtc_clock * 1000, + adjusted_mode->crtc_htotal * + info->monitor_range.max_vfreq)); + crtc_state->vrr.vtotalmax = + max_t(u16, adjusted_mode->crtc_vtotal, + DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000, + adjusted_mode->crtc_htotal * + info->monitor_range.min_vfreq)); + + drm_dbg(&dev_priv->drm, + "VRR Config: Enable = %s Vtotal Min = %d Vtotal Max = %d\n", + yesno(crtc_state->vrr.enable), crtc_state->vrr.vtotalmin, + crtc_state->vrr.vtotalmax); +} + diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index 755746c7525c..1e6fe8fe92ec 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -15,5 +15,7 @@ struct intel_encoder; struct intel_dp; bool intel_is_vrr_capable(struct drm_connector *connector); +void intel_vrr_compute_config(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state); #endif /* __INTEL_VRR_H__ */
This forces a complete modeset if vrr drm crtc state goes from enabled to disabled and vice versa. This patch also computes vrr state variables from the mode timings and based on the vrr property set by userspace as well as hardware's vrr capability. v2: *Rebase v3: * Vmin = max (vtotal, vmin) (Manasi) v4: * set crtc_state->vrr.enable = 0 for disable request Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 7 +++- drivers/gpu/drm/i915/display/intel_dp.c | 1 + drivers/gpu/drm/i915/display/intel_vrr.c | 38 ++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_vrr.h | 2 ++ 4 files changed, 47 insertions(+), 1 deletion(-)