Re: [PATCH 32/89] drm/i915/skl: Adjust the display engine interrupts

Rodrigo Vivi
2014-09-17T18:41:55Z

[-- Attachment #1.1: Type: text/plain, Size: 6534 bytes --]

Oh cool here are the actual fixes on de_pipe int bits!

I agree with Daniel that a separated function would be better, but what is
here is right anyway so
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

On Thu, Sep 4, 2014 at 6:19 AM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Sep 04, 2014 at 12:26:58PM +0100, Damien Lespiau wrote:
> > To accomodate the extra planes, the bit definitions were shuffled around
> > a bit.
> >
> > v2: Rebase on top of the for_each_pipe() change adding dev_priv as first
> > argument.
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 37
> ++++++++++++++++++++++++++++---------
> > drivers/gpu/drm/i915/i915_reg.h | 13 +++++++++++++
> > 2 files changed, 41 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> > index c62a2e4..734b73e 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2584,7 +2584,7 @@ static irqreturn_t gen8_irq_handler(int irq, void
> *arg)
> > }
> >
> > for_each_pipe(dev_priv, pipe) {
> > - uint32_t pipe_iir;
> > + uint32_t pipe_iir, flip_done = 0, fault_errors = 0;
> >
> > if (!(master_ctl & GEN8_DE_PIPE_IRQ(pipe)))
> > continue;
> > @@ -2593,10 +2593,16 @@ static irqreturn_t gen8_irq_handler(int irq,
> void *arg)
> > if (pipe_iir) {
> > ret = IRQ_HANDLED;
> > I915_WRITE(GEN8_DE_PIPE_IIR(pipe), pipe_iir);
> > +
> > if (pipe_iir & GEN8_PIPE_VBLANK)
> > intel_pipe_handle_vblank(dev, pipe);
> >
> > - if (pipe_iir & GEN8_PIPE_PRIMARY_FLIP_DONE) {
> > + if (IS_GEN9(dev))
> > + flip_done = pipe_iir &
> GEN9_PIPE_PLANE1_FLIP_DONE;
> > + else
> > + flip_done = pipe_iir &
> GEN8_PIPE_PRIMARY_FLIP_DONE;
> > +
> > + if (flip_done) {
> > intel_prepare_page_flip(dev, pipe);
> > intel_finish_page_flip_plane(dev, pipe);
> > }
> > @@ -2611,11 +2617,16 @@ static irqreturn_t gen8_irq_handler(int irq,
> void *arg)
> > pipe_name(pipe));
> > }
> >
> > - if (pipe_iir & GEN8_DE_PIPE_IRQ_FAULT_ERRORS) {
> > +
> > + if (IS_GEN9(dev))
> > + fault_errors = pipe_iir &
> GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
> > + else
> > + fault_errors = pipe_iir &
> GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> > +
> > + if (fault_errors)
> > DRM_ERROR("Fault errors on pipe %c\n:
> 0x%08x",
> > pipe_name(pipe),
> > pipe_iir &
> GEN8_DE_PIPE_IRQ_FAULT_ERRORS);
> > - }
> > } else
> > DRM_ERROR("The master control interrupt lied (DE
> PIPE)!\n");
>
> gen8+ irq handling functions would _really_ benefit from a bit of function
> extraction to prevent them from falling off the right edge of my screen
> all the time ...
>
> Just in case you have no idea what to do on a rainy day ;-)
> -Daniel
>
> > }
> > @@ -3845,12 +3856,20 @@ static void gen8_gt_irq_postinstall(struct
> drm_i915_private *dev_priv)
> >
> > static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> > {
> > - uint32_t de_pipe_masked = GEN8_PIPE_PRIMARY_FLIP_DONE |
> > - GEN8_PIPE_CDCLK_CRC_DONE |
> > - GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> > - uint32_t de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > - GEN8_PIPE_FIFO_UNDERRUN;
> > + uint32_t de_pipe_masked = GEN8_PIPE_CDCLK_CRC_DONE;
> > + uint32_t de_pipe_enables;
> > int pipe;
> > +
> > + if (IS_GEN9(dev_priv))
> > + de_pipe_masked |= GEN9_PIPE_PLANE1_FLIP_DONE |
> > + GEN9_DE_PIPE_IRQ_FAULT_ERRORS;
> > + else
> > + de_pipe_masked |= GEN8_PIPE_PRIMARY_FLIP_DONE |
> > + GEN8_DE_PIPE_IRQ_FAULT_ERRORS;
> > +
> > + de_pipe_enables = de_pipe_masked | GEN8_PIPE_VBLANK |
> > + GEN8_PIPE_FIFO_UNDERRUN;
> > +
> > dev_priv->de_irq_mask[PIPE_A] = ~de_pipe_masked;
> > dev_priv->de_irq_mask[PIPE_B] = ~de_pipe_masked;
> > dev_priv->de_irq_mask[PIPE_C] = ~de_pipe_masked;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index acd0a7b..5928a75 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4840,10 +4840,23 @@ enum punit_power_well {
> > #define GEN8_PIPE_SCAN_LINE_EVENT (1 << 2)
> > #define GEN8_PIPE_VSYNC (1 << 1)
> > #define GEN8_PIPE_VBLANK (1 << 0)
> > +#define GEN9_PIPE_CURSOR_FAULT (1 << 11)
> > +#define GEN9_PIPE_PLANE3_FAULT (1 << 9)
> > +#define GEN9_PIPE_PLANE2_FAULT (1 << 8)
> > +#define GEN9_PIPE_PLANE1_FAULT (1 << 7)
> > +#define GEN9_PIPE_PLANE3_FLIP_DONE (1 << 5)
> > +#define GEN9_PIPE_PLANE2_FLIP_DONE (1 << 4)
> > +#define GEN9_PIPE_PLANE1_FLIP_DONE (1 << 3)
> > +#define GEN9_PIPE_PLANE_FLIP_DONE(p) (1 << (3 + p))
> > #define GEN8_DE_PIPE_IRQ_FAULT_ERRORS \
> > (GEN8_PIPE_CURSOR_FAULT | \
> > GEN8_PIPE_SPRITE_FAULT | \
> > GEN8_PIPE_PRIMARY_FAULT)
> > +#define GEN9_DE_PIPE_IRQ_FAULT_ERRORS \
> > + (GEN9_PIPE_CURSOR_FAULT | \
> > + GEN9_PIPE_PLANE3_FAULT | \
> > + GEN9_PIPE_PLANE2_FAULT | \
> > + GEN9_PIPE_PLANE1_FAULT)
> >
> > #define GEN8_DE_PORT_ISR 0x44440
> > #define GEN8_DE_PORT_IMR 0x44444
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 9233 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org