On Thu, 10 Mar 2022, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > Let's start supporting static DRRS by trying to match the refresh > rate the user has requested, assuming the panel supports suitable > timings. > > For now we stick to just our current two timings: > - fixed_mode: the panel's preferred mode > - downclock_mode: the lowest refresh rate mode we found > Some panels may support more timings than that, but we'll > have to convert our fixed_mode/downclock_mode pointers > into a full list before we can handle that.
So this won't fix panels with, say, a 120 Hz mode that isn't the panel's preferred mode? > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 11 +++++++++++ > drivers/gpu/drm/i915/display/intel_drrs.c | 8 +++++--- > drivers/gpu/drm/i915/display/intel_panel.c | 20 ++++++++++++++++++-- > 3 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index af659320c02e..9bd958377a54 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4599,6 +4599,17 @@ static int intel_dp_get_modes(struct drm_connector > *connector) > num_modes++; > } > } > + if (intel_dp_is_edp(intel_attached_dp(intel_connector)) && > + intel_connector->panel.downclock_mode) { I thought you wanted abstract access to connector->panel.downclock_mode in the previous patch. > + struct drm_display_mode *mode; > + > + mode = drm_mode_duplicate(connector->dev, > + > intel_connector->panel.downclock_mode); > + if (mode) { > + drm_mode_probed_add(connector, mode); > + num_modes++; > + } > + } > > if (num_modes) > return num_modes; > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c > b/drivers/gpu/drm/i915/display/intel_drrs.c > index 5b2eb55c1340..dc1733c9abab 100644 > --- a/drivers/gpu/drm/i915/display/intel_drrs.c > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c > @@ -383,7 +383,7 @@ intel_drrs_init(struct intel_connector *connector, > return NULL; > } > > - if (dev_priv->vbt.drrs_type != DRRS_TYPE_SEAMLESS) { > + if (dev_priv->vbt.drrs_type == DRRS_TYPE_NONE) { > drm_dbg_kms(&dev_priv->drm, > "[CONNECTOR:%d:%s] DRRS not supported according to > VBT\n", > connector->base.base.id, connector->base.name); > @@ -399,8 +399,10 @@ intel_drrs_init(struct intel_connector *connector, > } > > drm_dbg_kms(&dev_priv->drm, > - "[CONNECTOR:%d:%s] seamless DRRS supported\n", > - connector->base.base.id, connector->base.name); > + "[CONNECTOR:%d:%s] %s DRRS supported\n", > + connector->base.base.id, connector->base.name, > + dev_priv->vbt.drrs_type == DRRS_TYPE_SEAMLESS ? > + "seamless" : "static"); > > return downclock_mode; > } > diff --git a/drivers/gpu/drm/i915/display/intel_panel.c > b/drivers/gpu/drm/i915/display/intel_panel.c > index 127ad9643360..6ddbb69dcfdc 100644 > --- a/drivers/gpu/drm/i915/display/intel_panel.c > +++ b/drivers/gpu/drm/i915/display/intel_panel.c > @@ -49,14 +49,30 @@ const struct drm_display_mode * > intel_panel_fixed_mode(struct intel_connector *connector, > const struct drm_display_mode *mode) > { > - return connector->panel.fixed_mode; > + const struct drm_display_mode *fixed_mode = connector->panel.fixed_mode; > + const struct drm_display_mode *downclock_mode = > connector->panel.downclock_mode; > + > + /* pick the one that is closer in terms of vrefresh */ > + /* FIXME make this a a list of modes so we can have more than two */ Indeed feels a bit hackish... This being the last and arguably fairly simple part of the series, I do wonder about the pros and cons of merging this as an interim solution. IIUC if the user wanted to do static/manual drrs while seamless is supported, this does not disable seamless when the user chooses preferred vrefresh and a downclock vrefresh is available. So you can choose lower vrefresh and stay there, but you can't choose higher vrefresh and stay there. > + if (fixed_mode && downclock_mode && > + abs(drm_mode_vrefresh(downclock_mode) - drm_mode_vrefresh(mode)) < > + abs(drm_mode_vrefresh(fixed_mode) - drm_mode_vrefresh(mode))) > + return downclock_mode; > + else > + return fixed_mode; > } > > const struct drm_display_mode * > intel_panel_downclock_mode(struct intel_connector *connector, > const struct drm_display_mode *fixed_mode) > { > - return connector->panel.downclock_mode; > + const struct drm_display_mode *downclock_mode = > connector->panel.downclock_mode; > + > + if (downclock_mode && > + drm_mode_vrefresh(downclock_mode) < drm_mode_vrefresh(fixed_mode)) With this use, I think the fixed_mode name here is a bit misleading. The downclock mode will always have a vrefresh lower than the fixed mode. But you're not always passing the fixed mode, right? > + return downclock_mode; > + else > + return NULL; > } > > int intel_panel_compute_config(struct intel_connector *connector, -- Jani Nikula, Intel Open Source Graphics Center
