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

[Chris Wilson <chris@chris-wilson.co.uk>]

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