From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Souza, Jose" Subject: Re: [PATCH v2 2/6] drm/i915: Refactor PSR status debugfs Date: Thu, 3 Jan 2019 16:29:14 +0000 Message-ID: <3f8ad52f0286eb0d639c7669130eb13bb945af6b.camel@intel.com> References: <20190103142107.18304-1-jose.souza@intel.com> <20190103142107.18304-2-jose.souza@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1668382112==" Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTPS id 81C516E563 for ; Thu, 3 Jan 2019 16:29:16 +0000 (UTC) In-Reply-To: <20190103142107.18304-2-jose.souza@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "intel-gfx@lists.freedesktop.org" Cc: "Pandiyan, Dhinakaran" , "Vivi, Rodrigo" List-Id: intel-gfx@lists.freedesktop.org --===============1668382112== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-mTv4MibcaWd+FSYumjTw" --=-mTv4MibcaWd+FSYumjTw Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 193823048f96..13005d5306ba 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2529,7 +2529,8 @@ DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status); static void psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) { - u32 val, psr_status; + u32 val, status_val; + const char *status =3D "unknown"; =20 if (dev_priv->psr.psr2_enabled) { static const char * const live_status[] =3D { @@ -2545,14 +2546,11 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) "BUF_ON", "TG_ON" }; - psr_status =3D I915_READ(EDP_PSR2_STATUS); - val =3D (psr_status & EDP_PSR2_STATUS_STATE_MASK) >> - EDP_PSR2_STATUS_STATE_SHIFT; - if (val < ARRAY_SIZE(live_status)) { - seq_printf(m, "Source PSR status: 0x%x [%s]\n", - psr_status, live_status[val]); - return; - } + val =3D I915_READ(EDP_PSR2_STATUS); + status_val =3D (val & EDP_PSR2_STATUS_STATE_MASK) >> + EDP_PSR2_STATUS_STATE_SHIFT; + if (status_val < ARRAY_SIZE(live_status)) + status =3D live_status[status_val]; } else { static const char * const live_status[] =3D { "IDLE", @@ -2564,74 +2562,78 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) "SRDOFFACK", "SRDENT_ON", }; - psr_status =3D I915_READ(EDP_PSR_STATUS); - val =3D (psr_status & EDP_PSR_STATUS_STATE_MASK) >> - EDP_PSR_STATUS_STATE_SHIFT; - if (val < ARRAY_SIZE(live_status)) { - seq_printf(m, "Source PSR status: 0x%x [%s]\n", - psr_status, live_status[val]); - return; - } + val =3D I915_READ(EDP_PSR_STATUS); + status_val =3D (val & EDP_PSR_STATUS_STATE_MASK) >> + EDP_PSR_STATUS_STATE_SHIFT; + if (status_val < ARRAY_SIZE(live_status)) + status =3D live_status[status_val]; } =20 - seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "unknown"); + seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val); } =20 static int i915_edp_psr_status(struct seq_file *m, void *data) { struct drm_i915_private *dev_priv =3D node_to_i915(m->private); - u32 psrperf =3D 0; - bool enabled =3D false; - bool sink_support; + struct i915_psr *psr =3D &dev_priv->psr; + const char *status; + bool enabled; + u32 val; =20 if (!HAS_PSR(dev_priv)) return -ENODEV; =20 - sink_support =3D dev_priv->psr.sink_support; - seq_printf(m, "Sink_Support: %s\n", yesno(sink_support)); - if (!sink_support) + seq_printf(m, "Sink support: %s", yesno(psr->sink_support)); + if (psr->dp) + seq_printf(m, " [0x%02x]", psr->dp->psr_dpcd[0]); + seq_puts(m, "\n"); + + if (!psr->sink_support) return 0; =20 intel_runtime_pm_get(dev_priv); + mutex_lock(&psr->lock); =20 - mutex_lock(&dev_priv->psr.lock); - seq_printf(m, "PSR mode: %s\n", - dev_priv->psr.psr2_enabled ? "PSR2" : "PSR1"); - seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled)); - seq_printf(m, "Busy frontbuffer bits: 0x%03x\n", - dev_priv->psr.busy_frontbuffer_bits); - - if (dev_priv->psr.psr2_enabled) - enabled =3D I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE; + if (psr->enabled) + status =3D psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled"; else - enabled =3D I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE; + status =3D "disabled"; + seq_printf(m, "PSR mode: %s\n", status); =20 - seq_printf(m, "Main link in standby mode: %s\n", - yesno(dev_priv->psr.link_standby)); + if (!psr->enabled) + goto unlock; =20 - seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled)); + if (psr->psr2_enabled) { + val =3D I915_READ(EDP_PSR2_CTL); + enabled =3D val & EDP_PSR2_ENABLE; + } else { + val =3D I915_READ(EDP_PSR_CTL); + enabled =3D val & EDP_PSR_ENABLE; + } + seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", + enableddisabled(enabled), val); + psr_source_status(dev_priv, m); + seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", + psr->busy_frontbuffer_bits); =20 /* * SKL+ Perf counter is reset to 0 everytime DC state is entered */ if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { - psrperf =3D I915_READ(EDP_PSR_PERF_CNT) & - EDP_PSR_PERF_CNT_MASK; - - seq_printf(m, "Performance_Counter: %u\n", psrperf); + val =3D I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK; + seq_printf(m, "Performance counter: %u\n", val); } =20 - psr_source_status(dev_priv, m); - mutex_unlock(&dev_priv->psr.lock); - - if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) { + if (psr->debug & I915_PSR_DEBUG_IRQ) { seq_printf(m, "Last attempted entry at: %lld\n", - dev_priv->psr.last_entry_attempt); - seq_printf(m, "Last exit at: %lld\n", - dev_priv->psr.last_exit); + psr->last_entry_attempt); + seq_printf(m, "Last exit at: %lld\n", psr->last_exit); } =20 +unlock: + mutex_unlock(&psr->lock); intel_runtime_pm_put(dev_priv); + return 0; } =20 On Thu, 2019-01-03 at 06:21 -0800, Jos=C3=A9 Roberto de Souza wrote: > The old debugfs fields was not following a naming partern and it was > a bit confusing. >=20 > So it went from: > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status > Sink_Support: yes > PSR mode: PSR1 > Enabled: yes > Busy frontbuffer bits: 0x000 > Main link in standby mode: no > HW Enabled & Active bit: yes > Source PSR status: 0x24050006 [SRDONACK] >=20 > To: > ~$ sudo more /sys/kernel/debug/dri/0/i915_edp_psr_status > Sink support: yes [0x03] > PSR mode: PSR1 enabled > Source PSR ctl: enabled [0x81f00e26] > Source PSR status: IDLE [0x04010006] > Busy frontbuffer bits: 0x00000000 >=20 > The 'Main link in standby mode' was removed as it is not useful but > if needed by someone the information is still in the register value > of 'Source PSR ctl' inside of the brackets, PSR mode and Enabled was > squashed into PSR mode, some renames and reorders and we have this > cleaner version. This will also make easy to parse debugfs for IGT > tests. >=20 > v2: Printing sink PSR version with only 2 hex digits as it is a byte >=20 > Cc: Rodrigo Vivi > Cc: Dhinakaran Pandiyan > Suggested-by: Dhinakaran Pandiyan > Signed-off-by: Jos=C3=A9 Roberto de Souza > --- > drivers/gpu/drm/i915/i915_debugfs.c | 95 ++++++++++++++------------- > -- > 1 file changed, 47 insertions(+), 48 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 193823048f96..1a31921598e7 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2529,7 +2529,8 @@ DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status); > static void > psr_source_status(struct drm_i915_private *dev_priv, struct seq_file > *m) > { > - u32 val, psr_status; > + u32 val, status_val; > + const char *status =3D "unknown"; > =20 > if (dev_priv->psr.psr2_enabled) { > static const char * const live_status[] =3D { > @@ -2545,14 +2546,11 @@ psr_source_status(struct drm_i915_private > *dev_priv, struct seq_file *m) > "BUF_ON", > "TG_ON" > }; > - psr_status =3D I915_READ(EDP_PSR2_STATUS); > - val =3D (psr_status & EDP_PSR2_STATUS_STATE_MASK) >> > - EDP_PSR2_STATUS_STATE_SHIFT; > - if (val < ARRAY_SIZE(live_status)) { > - seq_printf(m, "Source PSR status: 0x%x [%s]\n", > - psr_status, live_status[val]); > - return; > - } > + val =3D I915_READ(EDP_PSR2_STATUS); > + status_val =3D (val & EDP_PSR2_STATUS_STATE_MASK) >> > + EDP_PSR2_STATUS_STATE_SHIFT; > + if (status_val < ARRAY_SIZE(live_status)) > + status =3D live_status[status_val]; > } else { > static const char * const live_status[] =3D { > "IDLE", > @@ -2564,74 +2562,75 @@ psr_source_status(struct drm_i915_private > *dev_priv, struct seq_file *m) > "SRDOFFACK", > "SRDENT_ON", > }; > - psr_status =3D I915_READ(EDP_PSR_STATUS); > - val =3D (psr_status & EDP_PSR_STATUS_STATE_MASK) >> > - EDP_PSR_STATUS_STATE_SHIFT; > - if (val < ARRAY_SIZE(live_status)) { > - seq_printf(m, "Source PSR status: 0x%x [%s]\n", > - psr_status, live_status[val]); > - return; > - } > + val =3D I915_READ(EDP_PSR_STATUS); > + status_val =3D (val & EDP_PSR_STATUS_STATE_MASK) >> > + EDP_PSR_STATUS_STATE_SHIFT; > + if (status_val < ARRAY_SIZE(live_status)) > + status =3D live_status[status_val]; > } > =20 > - seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, > "unknown"); > + seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val); > } > =20 > static int i915_edp_psr_status(struct seq_file *m, void *data) > { > struct drm_i915_private *dev_priv =3D node_to_i915(m->private); > - u32 psrperf =3D 0; > - bool enabled =3D false; > - bool sink_support; > + struct i915_psr *psr =3D &dev_priv->psr; > + const char *status; > + bool enabled; > + u32 val; > =20 > if (!HAS_PSR(dev_priv)) > return -ENODEV; > =20 > - sink_support =3D dev_priv->psr.sink_support; > - seq_printf(m, "Sink_Support: %s\n", yesno(sink_support)); > - if (!sink_support) > + seq_printf(m, "Sink support: %s", yesno(psr->sink_support)); > + seq_printf(m, " [0x%02x]\n", psr->dp->psr_dpcd[0]); > + if (!psr->sink_support) > return 0; > =20 > intel_runtime_pm_get(dev_priv); > + mutex_lock(&psr->lock); > =20 > - mutex_lock(&dev_priv->psr.lock); > - seq_printf(m, "PSR mode: %s\n", > - dev_priv->psr.psr2_enabled ? "PSR2" : "PSR1"); > - seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled)); > - seq_printf(m, "Busy frontbuffer bits: 0x%03x\n", > - dev_priv->psr.busy_frontbuffer_bits); > - > - if (dev_priv->psr.psr2_enabled) > - enabled =3D I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE; > + if (psr->enabled) > + status =3D psr->psr2_enabled ? "PSR2 enabled" : "PSR1 > enabled"; > else > - enabled =3D I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE; > + status =3D "disabled"; > + seq_printf(m, "PSR mode: %s\n", status); > =20 > - seq_printf(m, "Main link in standby mode: %s\n", > - yesno(dev_priv->psr.link_standby)); > + if (!psr->enabled) > + goto unlock; > =20 > - seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled)); > + if (psr->psr2_enabled) { > + val =3D I915_READ(EDP_PSR2_CTL); > + enabled =3D val & EDP_PSR2_ENABLE; > + } else { > + val =3D I915_READ(EDP_PSR_CTL); > + enabled =3D val & EDP_PSR_ENABLE; > + } > + seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", > + enableddisabled(enabled), val); > + psr_source_status(dev_priv, m); > + seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", > + psr->busy_frontbuffer_bits); > =20 > /* > * SKL+ Perf counter is reset to 0 everytime DC state is > entered > */ > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > - psrperf =3D I915_READ(EDP_PSR_PERF_CNT) & > - EDP_PSR_PERF_CNT_MASK; > - > - seq_printf(m, "Performance_Counter: %u\n", psrperf); > + val =3D I915_READ(EDP_PSR_PERF_CNT) & > EDP_PSR_PERF_CNT_MASK; > + seq_printf(m, "Performance counter: %u\n", val); > } > =20 > - psr_source_status(dev_priv, m); > - mutex_unlock(&dev_priv->psr.lock); > - > - if (READ_ONCE(dev_priv->psr.debug) & I915_PSR_DEBUG_IRQ) { > + if (psr->debug & I915_PSR_DEBUG_IRQ) { > seq_printf(m, "Last attempted entry at: %lld\n", > - dev_priv->psr.last_entry_attempt); > - seq_printf(m, "Last exit at: %lld\n", > - dev_priv->psr.last_exit); > + psr->last_entry_attempt); > + seq_printf(m, "Last exit at: %lld\n", psr->last_exit); > } > =20 > +unlock: > + mutex_unlock(&psr->lock); > intel_runtime_pm_put(dev_priv); > + > return 0; > } > =20 --=-mTv4MibcaWd+FSYumjTw Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEVNG051EijGa0MiaQVenbO/mOWkkFAlwuOFgACgkQVenbO/mO WkmXKwf/awIE9V34ca21p0CE1M/EPp8jwLbzhEA9ZeaUA3IzM7n7BmPjIXa52K2a /UM1kSwW8hTdj+PMtb87TE+J7v8epxiVTGZ38nlPr4UKIAkzXcu14Q9SBrRsDNSL gJRDLfk5nS49RfJIV39FrOhK4zmee6QC0iLMBGJpVXxk36woBp6rtiBmYKhRZpNr V/ZRL5fkccE9csS1SLmXt59LEY3VFmMhE44H1YMHCOy8Iyitp0NM8Ib5I1FkXxLH iVlNb9dotjFL43VkdAWof2+zXZuQjQv8YBfI+eOp5Gtx3eUhr0GZuSoSZn0Rcxs7 mKLNZoijZLgFktoI56DBQmvOlgmgTQ== =X3Wh -----END PGP SIGNATURE----- --=-mTv4MibcaWd+FSYumjTw-- --===============1668382112== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============1668382112==--