On Thu, Mar 22, 2018 at 01:05:24AM +0000, Pandiyan, Dhinakaran wrote:
> On Wed, 2018-03-21 at 21:48 +0200, Ville Syrjälä wrote:
> > On Tue, Mar 20, 2018 at 03:41:51PM -0700, Dhinakaran Pandiyan wrote:
> > > Timestamps are useful for IGT tests that trigger PSR exit and/or wait for
> > > PSR entry.
> > >
> > > Cc: Ville Syrjälä <[email protected]>
> > > Cc: Rodrigo Vivi <[email protected]>
> > > Cc: Daniel Vetter <[email protected]>
> > > Signed-off-by: Dhinakaran Pandiyan <[email protected]>
> > > ---
> > > drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++++++++
> > > drivers/gpu/drm/i915/i915_drv.h | 3 +++
> > > drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++++++++++--
> > > 3 files changed, 34 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 669f3d56054a..d28dc4d8388e 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2686,6 +2686,18 @@ static int i915_edp_psr_status(struct seq_file *m,
> > > void *data)
> > > }
> > > mutex_unlock(&dev_priv->psr.lock);
> > >
> > > + if (READ_ONCE(dev_priv->psr.debug)) {
> > > + unsigned int seq;
> > > +
> > > + do {
> > > + seq = read_seqbegin(&dev_priv->psr.debug_lock);
> > > + 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);
> > > + } while (read_seqretry(&dev_priv->psr.debug_lock, seq));
> >
> > What does the seqlock buy us?
>
> Reading debugfs wouldn't block the update inside the irq handler,
> compared to using a spinlock. We need to serialize the read and update
> parts, don't we? Otherwise tests might end up reading partial updates.
Hmm. ktime_t is 64 bits so I guess on 32bit systems it could be a slight
issue. Not sure we should care that much about PSR debug on 32bit though.
It's a rather unlikely configuration, and at least we don't do that in
the ci.
>
> >
> > > + }
> > > +
> > > intel_runtime_pm_put(dev_priv);
> > > return 0;
> > > }
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 136fa2267a66..b8170882e1ab 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -609,6 +609,9 @@ struct i915_psr {
> > > bool alpm;
> > > bool has_hw_tracking;
> > > bool debug;
> > > + ktime_t last_entry_attempt;
> > > + ktime_t last_exit;
> > > + seqlock_t debug_lock;
> > >
> > > void (*enable_source)(struct intel_dp *,
> > > const struct intel_crtc_state *);
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 64ecea80438d..a83d95b1b587 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -125,28 +125,44 @@ void intel_psr_irq_handler(struct drm_i915_private
> > > *dev_priv, u32 psr_iir)
> > > {
> > > u32 transcoders = BIT(TRANSCODER_EDP);
> > > enum transcoder cpu_transcoder;
> > > + ktime_t time_ns = ktime_get();
> > > + unsigned long flags = 0;
> > >
> > > if (INTEL_GEN(dev_priv) >= 8)
> > > transcoders |= BIT(TRANSCODER_A) |
> > > BIT(TRANSCODER_B) |
> > > BIT(TRANSCODER_C);
> > >
> > > + write_seqlock_irqsave(&dev_priv->psr.debug_lock, flags);
> > > for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> > > + bool handled = false;
> > > +
> > > + /* PSR supported only on one transcoder currently */
> > > + WARN_ON_ONCE(handled);
> > > +
> > > /* FIXME: Exit PSR when this happens. */
> > > - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > > + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> > > + handled = true;
> > > DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
> > > transcoder_name(cpu_transcoder));
> > >
> > > + }
> > > +
> > > if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > > - DRM_DEBUG_KMS("[transcoder %s] PSR entry in 2
> > > vblanks\n",
> > > + handled = true;
> > > + dev_priv->psr.last_entry_attempt = time_ns;
> > > + DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2
> > > vblanks\n",
> > > transcoder_name(cpu_transcoder));
> > > }
> > >
> > > if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> > > + handled = true;
> > > + dev_priv->psr.last_exit = time_ns;
> > > DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > > transcoder_name(cpu_transcoder));
> > > }
> > > }
> > > + write_sequnlock_irqrestore(&dev_priv->psr.debug_lock, flags);
> > > }
> > >
> > > static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> > > @@ -1160,6 +1176,7 @@ void intel_psr_init(struct drm_i915_private
> > > *dev_priv)
> > >
> > > INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
> > > mutex_init(&dev_priv->psr.lock);
> > > + seqlock_init(&dev_priv->psr.debug_lock);
> > >
> > > if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > > dev_priv->psr.enable_source = vlv_psr_enable_source;
> > > --
> > > 2.14.1
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
