[Intel-gfx] [PATCH v10 09/17] drm/i915/pxp: Implement PXP irq handler

[Teres Alexis, Alan Previn]

On Mon, 2021-09-20 at 12:04 -0400, Rodrigo Vivi wrote:
> On Fri, Sep 17, 2021 at 09:20:00PM -0700, Alan Previn wrote:
> > From: "Huang, Sean Z" <sean.z.huang at intel.com>
> > 
> > The HW will generate a teardown interrupt when session termination is
> > required, which requires i915 to submit a terminating batch. Once the HW
> > is done with the termination it will generate another interrupt, at
> > which point it is safe to re-create the session.
> > 
> > Since the termination and re-creation flow is something we want to
> > trigger from the driver as well, use a common work function that can be
> > called both from the irq handler and from the driver set-up flows, which
> > has the addded benefit of allowing us to skip any extra locks because
> > the work itself serializes the operations.
> > 
> > v2: use struct completion instead of bool (Chris)
> > v3: drop locks, clean up functions and improve comments (Chris),
> >     move to common work function.
> > v4: improve comments, simplify wait logic (Rodrigo)
> > v5: unconditionally set interrupts, rename state_attacked var (Rodrigo)
> > 
> > Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
> > Signed-off-by: Huang, Sean Z <sean.z.huang at intel.com>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> > +#include <linux/workqueue.h>
> > +#include "intel_pxp.h"
> > +#include "intel_pxp_irq.h"
> > +#include "intel_pxp_session.h"
> > +#include "gt/intel_gt_irq.h"
> > +#include "gt/intel_gt_types.h"
> > +#include "i915_irq.h"
> > +#include "i915_reg.h"
> > +
> > +/**
> > + * intel_pxp_irq_handler - Handles PXP interrupts.
> > + * @pxp: pointer to pxp struct
> > + * @iir: interrupt vector
> > + */
> > +void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir)
> > +{
> > +	struct intel_gt *gt = pxp_to_gt(pxp);
> 
> this compiles, but I don't see how this can work.
> 
> shouldn't we use the container_of here directly instead of trying
> to use something that is not properly defined?
> 
Its now a function that's abstracted in .c file and prototype defined in the intel_pxp.h. Unless i
misunderstood something, i thought this was one of the options discussed in last rev - i can change it
again to use the container_of directly in all the instances if you like. side note: inline was still
defined but only for the CONFIG_PXP==off case that doesn't require the additional header inclusions.

> > +