From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 07/13] drm/i915: Clean up DRRS refresh rate enum
Date: Thu, 10 Mar 2022 11:43:54 +0200 [thread overview]
Message-ID: <87mthyxhut.fsf@intel.com> (raw)
In-Reply-To: <20220310004802.16310-8-ville.syrjala@linux.intel.com>
On Thu, 10 Mar 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the DRRS refresh rate enum less magical.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I might've gone for removing the enum altogether, and tracking the
actual refresh rates, but this is an improvement on what we have now,
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> --- .../drm/i915/display/intel_display_debugfs.c | 18 ++------
> drivers/gpu/drm/i915/display/intel_drrs.c | 44 +++++++++----------
> drivers/gpu/drm/i915/i915_drv.h | 14 ++---- 3 files changed, 28
> insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 28414472110e..798bf233a60f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -1149,7 +1149,6 @@ static void drrs_status_per_crtc(struct seq_file *m,
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct i915_drrs *drrs = &dev_priv->drrs;
> - int vrefresh = 0;
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
>
> @@ -1191,21 +1190,12 @@ static void drrs_status_per_crtc(struct seq_file *m,
> drrs->busy_frontbuffer_bits);
>
> seq_puts(m, "\n\t\t");
> - if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
> - seq_puts(m, "DRRS_State: DRRS_HIGH_RR\n");
> - vrefresh = drm_mode_vrefresh(panel->fixed_mode);
> - } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
> - seq_puts(m, "DRRS_State: DRRS_LOW_RR\n");
> - vrefresh = drm_mode_vrefresh(panel->downclock_mode);
> - } else {
> - seq_printf(m, "DRRS_State: Unknown(%d)\n",
> - drrs->refresh_rate_type);
> - mutex_unlock(&drrs->mutex);
> - return;
> - }
> - seq_printf(m, "\t\tVrefresh: %d", vrefresh);
>
> + seq_printf(m, "DRRS refresh rate: %s\n",
> + drrs->refresh_rate == DRRS_REFRESH_RATE_LOW ?
> + "low" : "high");
> seq_puts(m, "\n\t\t");
> +
> mutex_unlock(&drrs->mutex);
> } else {
> /* DRRS not supported. Print the VBT parameter*/
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
> index 7c4a3ecee93a..3979ceaaf651 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> @@ -103,7 +103,7 @@ intel_drrs_compute_config(struct intel_dp *intel_dp,
>
> static void
> intel_drrs_set_refresh_rate_pipeconf(const struct intel_crtc_state *crtc_state,
> - enum drrs_refresh_rate_type refresh_type)
> + enum drrs_refresh_rate refresh_rate)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -117,7 +117,7 @@ intel_drrs_set_refresh_rate_pipeconf(const struct intel_crtc_state *crtc_state,
>
> val = intel_de_read(dev_priv, PIPECONF(cpu_transcoder));
>
> - if (refresh_type == DRRS_LOW_RR)
> + if (refresh_rate == DRRS_REFRESH_RATE_LOW)
> val |= bit;
> else
> val &= ~bit;
> @@ -127,22 +127,21 @@ intel_drrs_set_refresh_rate_pipeconf(const struct intel_crtc_state *crtc_state,
>
> static void
> intel_drrs_set_refresh_rate_m_n(const struct intel_crtc_state *crtc_state,
> - enum drrs_refresh_rate_type refresh_type)
> + enum drrs_refresh_rate refresh_rate)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>
> intel_cpu_transcoder_set_m1_n1(crtc, crtc_state->cpu_transcoder,
> - refresh_type == DRRS_LOW_RR ?
> + refresh_rate == DRRS_REFRESH_RATE_LOW ?
> &crtc_state->dp_m2_n2 : &crtc_state->dp_m_n);
> }
>
> static void intel_drrs_set_state(struct drm_i915_private *dev_priv,
> const struct intel_crtc_state *crtc_state,
> - enum drrs_refresh_rate_type refresh_type)
> + enum drrs_refresh_rate refresh_rate)
> {
> struct intel_dp *intel_dp = dev_priv->drrs.dp;
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> - struct drm_display_mode *mode;
>
> if (!intel_dp) {
> drm_dbg_kms(&dev_priv->drm, "DRRS not supported.\n");
> @@ -160,7 +159,7 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv,
> return;
> }
>
> - if (refresh_type == dev_priv->drrs.refresh_rate_type)
> + if (refresh_rate == dev_priv->drrs.refresh_rate)
> return;
>
> if (!crtc_state->hw.active) {
> @@ -170,18 +169,14 @@ static void intel_drrs_set_state(struct drm_i915_private *dev_priv,
> }
>
> if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv))
> - intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_type);
> + intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_rate);
> else if (DISPLAY_VER(dev_priv) > 6)
> - intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_type);
> + intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_rate);
>
> - dev_priv->drrs.refresh_rate_type = refresh_type;
> + dev_priv->drrs.refresh_rate = refresh_rate;
>
> - if (refresh_type == DRRS_LOW_RR)
> - mode = intel_dp->attached_connector->panel.downclock_mode;
> - else
> - mode = intel_dp->attached_connector->panel.fixed_mode;
> - drm_dbg_kms(&dev_priv->drm, "eDP Refresh Rate set to : %dHz\n",
> - drm_mode_vrefresh(mode));
> + drm_dbg_kms(&dev_priv->drm, "eDP Refresh Rate set to : %s\n",
> + refresh_rate == DRRS_REFRESH_RATE_LOW ? "low" : "high");
> }
>
> static void
> @@ -229,7 +224,7 @@ intel_drrs_disable_locked(struct intel_dp *intel_dp,
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>
> - intel_drrs_set_state(dev_priv, crtc_state, DRRS_HIGH_RR);
> + intel_drrs_set_state(dev_priv, crtc_state, DRRS_REFRESH_RATE_HIGH);
> dev_priv->drrs.dp = NULL;
> }
>
> @@ -297,7 +292,6 @@ static void intel_drrs_downclock_work(struct work_struct *work)
> struct drm_i915_private *dev_priv =
> container_of(work, typeof(*dev_priv), drrs.work.work);
> struct intel_dp *intel_dp;
> - struct drm_crtc *crtc;
>
> mutex_lock(&dev_priv->drrs.mutex);
>
> @@ -311,11 +305,13 @@ static void intel_drrs_downclock_work(struct work_struct *work)
> * recheck.
> */
>
> - if (dev_priv->drrs.busy_frontbuffer_bits)
> - goto unlock;
> + if (!dev_priv->drrs.busy_frontbuffer_bits) {
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
>
> - crtc = dp_to_dig_port(intel_dp)->base.base.crtc;
> - intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config, DRRS_LOW_RR);
> + intel_drrs_set_state(dev_priv, crtc->config,
> + DRRS_REFRESH_RATE_LOW);
> + }
>
> unlock:
> mutex_unlock(&dev_priv->drrs.mutex);
> @@ -354,7 +350,7 @@ static void intel_drrs_frontbuffer_update(struct drm_i915_private *dev_priv,
> /* flush/invalidate means busy screen hence upclock */
> if (frontbuffer_bits)
> intel_drrs_set_state(dev_priv, to_intel_crtc(crtc)->config,
> - DRRS_HIGH_RR);
> + DRRS_REFRESH_RATE_HIGH);
>
> /*
> * flush also means no more activity hence schedule downclock, if all
> @@ -466,7 +462,7 @@ intel_drrs_init(struct intel_connector *connector,
>
> dev_priv->drrs.type = dev_priv->vbt.drrs_type;
>
> - dev_priv->drrs.refresh_rate_type = DRRS_HIGH_RR;
> + dev_priv->drrs.refresh_rate = DRRS_REFRESH_RATE_HIGH;
> drm_dbg_kms(&dev_priv->drm,
> "[CONNECTOR:%d:%s] seamless DRRS supported\n",
> connector->base.base.id, connector->base.name);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0fc5d7e447b9..7d622d1afe93 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -196,15 +196,9 @@ struct drm_i915_display_funcs {
>
> #define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
>
> -/*
> - * HIGH_RR is the highest eDP panel refresh rate read from EDID
> - * LOW_RR is the lowest eDP panel refresh rate found from EDID
> - * parsing for same resolution.
> - */
> -enum drrs_refresh_rate_type {
> - DRRS_HIGH_RR,
> - DRRS_LOW_RR,
> - DRRS_MAX_RR, /* RR count */
> +enum drrs_refresh_rate {
> + DRRS_REFRESH_RATE_HIGH,
> + DRRS_REFRESH_RATE_LOW,
> };
>
> enum drrs_type {
> @@ -218,7 +212,7 @@ struct i915_drrs {
> struct delayed_work work;
> struct intel_dp *dp;
> unsigned busy_frontbuffer_bits;
> - enum drrs_refresh_rate_type refresh_rate_type;
> + enum drrs_refresh_rate refresh_rate;
> enum drrs_type type;
> };
--
Jani Nikula, Intel Open Source Graphics Center
next prev parent reply other threads:[~2022-03-10 9:44 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-10 0:47 [Intel-gfx] [PATCH 00/13] drm/i915: DRRS fixes/cleanups and start of static DRRS Ville Syrjala
2022-03-10 0:47 ` [Intel-gfx] [PATCH 01/13] drm/i915: Fix up some DRRS type checks Ville Syrjala
2022-03-10 9:25 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 02/13] drm/i915: Constify intel_drrs_init() args Ville Syrjala
2022-03-10 9:25 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 03/13] drm/i915: Pimp DRRS debugs Ville Syrjala
2022-03-10 9:27 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 04/13] drm/i915: Read DRRS MSA timing delay from VBT Ville Syrjala
2022-03-10 9:32 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 05/13] drm/i915: Program MSA timing delay on ilk/snb/ivb Ville Syrjala
2022-03-10 9:37 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 06/13] drm/i915: Polish drrs type enum Ville Syrjala
2022-03-10 9:38 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 07/13] drm/i915: Clean up DRRS refresh rate enum Ville Syrjala
2022-03-10 9:43 ` Jani Nikula [this message]
2022-03-10 0:47 ` [Intel-gfx] [PATCH 08/13] drm/i915: Rename PIPECONF refresh select bits Ville Syrjala
2022-03-10 9:44 ` Jani Nikula
2022-03-10 0:47 ` [Intel-gfx] [PATCH 09/13] drm/i915: Stash DRRS state under intel_crtc Ville Syrjala
2022-03-10 10:53 ` Jani Nikula
2022-03-10 11:12 ` Ville Syrjälä
2022-03-10 17:45 ` Souza, Jose
2022-03-10 18:29 ` Ville Syrjälä
2022-03-10 0:47 ` [Intel-gfx] [PATCH 10/13] drm/i915: Move DRRS enable/disable higher up Ville Syrjala
2022-03-10 9:54 ` Jani Nikula
2022-03-10 0:48 ` [Intel-gfx] [PATCH 11/13] drm/i915: Enable eDP DRRS on ilk/snb port A Ville Syrjala
2022-03-10 9:59 ` Jani Nikula
2022-03-10 0:48 ` [Intel-gfx] [PATCH 12/13] drm/i915: Introduce intel_panel_{fixed, downclock}_mode() Ville Syrjala
2022-03-10 10:09 ` Jani Nikula
2022-03-10 0:48 ` [Intel-gfx] [PATCH 13/13] drm/i915: Implement static DRRS Ville Syrjala
2022-03-10 10:30 ` Jani Nikula
2022-03-10 11:01 ` Ville Syrjälä
2022-03-10 11:26 ` Jani Nikula
2022-03-10 1:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: DRRS fixes/cleanups and start of " Patchwork
2022-03-10 1:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-03-10 2:06 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-03-10 9:14 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mthyxhut.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.