On Tue, Nov 10, 2020 at 12:47:46PM +0200, Jani Nikula wrote:
> On Thu, 22 Oct 2020, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > ---
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
