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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox