[Intel-gfx] [PATCH 5/5] drm/i915/psr: Timestamps for PSR entry and exit interrupts.

Pandiyan, Dhinakaran dhinakaran.pandiyan at intel.com
Thu Mar 22 23:55:05 UTC 2018




On Thu, 2018-03-22 at 21:08 +0000, 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


More information about the Intel-gfx mailing list