[PATCH 2/2] drm/xe/gsc: add support for GSC proxy interrupt
Teres Alexis, Alan Previn
alan.previn.teres.alexis at intel.com
Tue Jan 9 19:17:53 UTC 2024
On Mon, 2024-01-08 at 11:48 -0800, Daniele Ceraolo Spurio wrote:
> On 1/8/2024 11:32 AM, Teres Alexis, Alan Previn wrote:
> > On Mon, 2023-12-11 at 17:05 -0800, Daniele Ceraolo Spurio wrote:
alan:snip
> > > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > index 5f5a72e9d0d8..9f5f2150034a 100644
> > > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > > @@ -450,6 +450,7 @@
> > > #define INTR_ENGINE_CLASS(x) REG_FIELD_GET(GENMASK(18, 16), x)
> > > #define INTR_ENGINE_INTR(x) REG_FIELD_GET(GENMASK(15, 0), x)
> > > #define OTHER_GUC_INSTANCE 0
> > > +#define OTHER_HECI2_INSTANCE 3
> > alan: should we stick with OTHER_GSC_HECI2_INSTANCE to match the naming in our hw specs?
>
> AFAICS the HW specs actually have "Security Engine HECI2"; I abbreviated
> it to just HECI2 since the mask register was like that as well (see
> below comment)
alan: hmm... i think there is a hw spec page that just lists the various engine instances
- hope i'm not mistaken - will sync with u offline.
alan:snip
> > > #define RENDER_COPY_INTR_ENABLE XE_REG(0x190030)
> > > @@ -462,6 +463,7 @@
> > > #define VCS0_VCS1_INTR_MASK XE_REG(0x1900a8)
> > > #define VCS2_VCS3_INTR_MASK XE_REG(0x1900ac)
> > > #define VECS0_VECS1_INTR_MASK XE_REG(0x1900d0)
> > > +#define HECI2_RSVD_INTR_MASK XE_REG(0x1900e4)
> > alan: similarly, i believe it more maintainable to follow hw spec: CRYPTO_RSVD_INTR_ENABLE
>
> This is HECI2_RSVD_INTR_MASK in the MTL specs.
alan: okay thanks - I see that.
> > > int xe_gsc_proxy_start(struct xe_gsc *gsc)
> > > {
> > > int err;
> > >
> > > + /* enable the proxy interrupt in the GSC shim layer */
> > > + gsc_proxy_irq_toggle(gsc, true);
> > alan: nit: just a question -> does hw/fw require us to enable the gsc heci2 interrupts if
> > we are not yet processing event-based requests but rather doing just the proactive-startup-proxy
> > sequence? (i.e. maybe only enable IRQs after the proxy-request handler call below? but i
> > guess it doesnt matter either way)
>
> Not right now, but there was talk a while back to move from a loop to a
> fully irq-driven flow (i.e. the GSC would generate a new interrupt for
> each message instead of chaining them as it does now). If we ever go
> that way, that would require us to have irq enabled before the first
> proxy is sent, which is why I went with that ordering here.
alan: sounds good - sounds like something we dont want to invest in unless we really
have a use-case for it considering the gsc-proxy-interaction is basically a fixed fsm.
> > > + /** @work_actions: mask of actions to be performed in the work */
> > alan: nit: did you mean "in the worker"?
>
> we usually call the functions just _work(), not _worker(), so I matched here
alan: sounds good - its a nit anyway.
More information about the Intel-xe
mailing list