[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CY5PR11MB6366B470B6B6A72F32F2EB18EDF0A@CY5PR11MB6366.namprd11.prod.outlook.com>
Date: Wed, 13 Sep 2023 12:48:48 +0000
From: "Usyskin, Alexander" <alexander.usyskin@...el.com>
To: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@...el.com>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
"De Marchi, Lucas" <lucas.demarchi@...el.com>,
"Vivi, Rodrigo" <rodrigo.vivi@...el.com>
CC: "Winkler, Tomas" <tomas.winkler@...el.com>,
"Lubart, Vitaly" <vitaly.lubart@...el.com>,
"intel-xe@...ts.freedesktop.org" <intel-xe@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/4] drm/xe/gsc: add gsc device support
> > +struct xe_gsc {
>
> Please use a different name for this instead of just xe_gsc. In Xe we're
> likely never going to fully use the GSC via HECI, only the GSCFI/CSC
> part. In MTL and newer we also have the actual GSC part being split off
> and placed inside GT (behind the GSCCS), so if we call this just xe_gsc
> as well it'll be confusing. I suggest calling this something like
> xe_heci_gsc, xe_heci_interface or something like that. I had actually
> suggested this for i915 as well
> (https://patchwork.freedesktop.org/patch/509653/) but Tomas was
> concerned it might make backporting fixes difficult, so I dropped it;
> this is not a concern for Xe right now.
>
> Note that this means renaming all the exposed functions as well.
>
> Daniele
>
Sure, will rename it xe_heci_gsc and move to xe_device.
> > diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> > index 1dee3e832eb5..1ae4cc9f1522 100644
> > --- a/drivers/gpu/drm/xe/xe_irq.c
> > +++ b/drivers/gpu/drm/xe/xe_irq.c
> > @@ -128,6 +128,7 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
> > struct xe_device *xe = gt_to_xe(gt);
> > u32 ccs_mask, bcs_mask;
> > u32 irqs, dmask, smask;
> > + u32 gsc_mask = GSC_IRQ_INTF(1);
> >
> > if (xe_device_guc_submission_enabled(xe)) {
> > irqs = GT_RENDER_USER_INTERRUPT |
> > @@ -180,6 +181,9 @@ void xe_irq_enable_hwe(struct xe_gt *gt)
> > if (xe_hw_engine_mask_per_class(gt,
> XE_ENGINE_CLASS_OTHER)) {
> > xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
> irqs);
> > xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
> ~irqs);
> > + } else if (HAS_HECI_GSCFI(xe)) {
> > + xe_mmio_write32(gt, GUNIT_GSC_INTR_ENABLE,
> gsc_mask);
> > + xe_mmio_write32(gt, GUNIT_GSC_INTR_MASK,
> ~gsc_mask);
Danielle, how this enablement should look when gsc_heci is not gt related?
I'm somewhat confused here.
--
Thanks,
Sasha
Powered by blists - more mailing lists