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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
