On Thu, 10 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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ä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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
