Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
On Thu, 2018-03-22 at 21:08 +, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-03-20 22:41:51)
> > 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);
>
> You are inside a hardirq handler. irqsave/irqrestore are not required.
>
> Even a seqlock here looks overkill, but whatever. (I don't buy that you
> need that precise level of coordination for a slow debugfs interface.)
>
Looks like I'll make two reviewers happy without the seqlock, will
remove it in the next version :)
> > for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > transcoders) {
> > + bool handled = false;
> > +
> > + /* PSR supported only on one transcoder currently */
> > + WARN_ON_ONCE(handled);
>
> Now this is just silly. At least get the check right.
Argh, I should have caught it. Thanks.
> -Chris
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
Quoting Dhinakaran Pandiyan (2018-03-20 22:41:51)
> 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);
You are inside a hardirq handler. irqsave/irqrestore are not required.
Even a seqlock here looks overkill, but whatever. (I don't buy that you
need that precise level of coordination for a slow debugfs interface.)
> for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders)
> {
> + bool handled = false;
> +
> + /* PSR supported only on one transcoder currently */
> + WARN_ON_ONCE(handled);
Now this is just silly. At least get the check right.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
On Thu, 2018-03-22 at 11:21 +0200, Ville Syrjälä wrote:
> On Thu, Mar 22, 2018 at 01:05:24AM +, 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ä
> > > > Cc: Rodrigo Vivi
> > > > Cc: Daniel Vetter
> > > > Signed-off-by: Dhinakaran Pandiyan
> > > > ---
> > > > 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.
>
Another thing is, if two IIR bits are set at the same time. EXIT and
PRE_ENTRY probably won't be though. How about if an earlier interrupt
was not handled? Won't multiple IIR bits be set? Implementing the lock
is slightly future proof and doesn't slow down irq handling.
> >
> > >
> > > > + }
> > > > +
> > > > 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
Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
On Thu, Mar 22, 2018 at 01:05:24AM +, 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ä
> > > Cc: Rodrigo Vivi
> > > Cc: Daniel Vetter
> > > Signed-off-by: Dhinakaran Pandiyan
> > > ---
> > > 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_pri
Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
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ä
> > Cc: Rodrigo Vivi
> > Cc: Daniel Vetter
> > Signed-off-by: Dhinakaran Pandiyan
> > ---
> > 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.
>
> > + }
> > +
> > 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_
Re: [Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
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ä
> Cc: Rodrigo Vivi
> Cc: Daniel Vetter
> Signed-off-by: Dhinakaran Pandiyan
> ---
> 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?
> + }
> +
> 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.
Timestamps are useful for IGT tests that trigger PSR exit and/or wait for
PSR entry.
Cc: Ville Syrjälä
Cc: Rodrigo Vivi
Cc: Daniel Vetter
Signed-off-by: Dhinakaran Pandiyan
---
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));
+ }
+
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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
