| Message ID | 20240710192249.3915396-7-michael.j.ruhl@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Support PMT features in Xe | expand |
On Wed, 10 Jul 2024, Michael J. Ruhl wrote: > DG2 needs to adjust the discovery offset WRT the GT BAR not the WRT = w.r.t. ? > P2SB bar so add the base_adjust value to allow for the difference > to be used. Could you please try to write out the problem in clearer terms. Currently the reasonale given in patch 5 doesn't even match the one given here or it is written in so unclear terms I cannot make the connection. > Update xe_vsec.c to include DG2 header information. > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > --- > drivers/gpu/drm/xe/xe_vsec.c | 81 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) > > diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c > index 98999d467db1..531ddd32a1a6 100644 > --- a/drivers/gpu/drm/xe/xe_vsec.c > +++ b/drivers/gpu/drm/xe/xe_vsec.c > @@ -19,6 +19,16 @@ > > #define SOC_BASE 0x280000 > > +/* from drivers/platform/x86/intel/pmt/telemetry.c */ > +#define TELEM_BASE_OFFSET 0x8 > + > +#define DG2_PMT_BASE 0xE8000 > +#define DG2_DISCOVERY_START 0x6000 > +#define DG2_TELEM_START 0x4000 > + > +#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_DISCOVERY_START) > +#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_TELEM_START) > + > #define BMG_PMT_BASE 0xDB000 > #define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE) > > @@ -32,6 +42,20 @@ > #define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08) > #define SG_REMAP_BITS GENMASK(31, 24) > > +static struct intel_vsec_header dg2_telemetry = { > + .length = 0x10, > + .id = VSEC_ID_TELEMETRY, > + .num_entries = 1, > + .entry_size = 3, > + .tbir = GFX_BAR, > + .offset = DG2_DISCOVERY_OFFSET, > +}; > + > +static struct intel_vsec_header *dg2_capabilities[] = { > + &dg2_telemetry, > + NULL > +}; > + > static struct intel_vsec_header bmg_telemetry = { > .length = 0x10, > .id = VSEC_ID_TELEMETRY, > @@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] = { > > enum xe_vsec { > XE_VSEC_UNKNOWN = 0, > + XE_VSEC_DG2, > XE_VSEC_BMG, > }; > > static struct intel_vsec_platform_info xe_vsec_info[] = { > + [XE_VSEC_DG2] = { > + .caps = VSEC_CAP_TELEMETRY, > + .headers = dg2_capabilities, > + .quirks = VSEC_QUIRK_EARLY_HW, > + }, > [XE_VSEC_BMG] = { > .caps = VSEC_CAP_TELEMETRY, > .headers = bmg_capabilities, > @@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = { > }; > > static const int vsec_platforms[] = { > + [XE_DG2] = XE_VSEC_DG2, > [XE_BATTLEMAGE] = XE_VSEC_BMG, > }; > > @@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct xe_device *xe) > return vsec_platforms[xe->info.platform]; > } > > +/* > + * Access the DG2 PMT MMIO discovery table > + * > + * The intel_vsec driver does not typically access the discovery table. > + * Instead, it creates a memory resource for the table and passes it > + * to the PMT telemetry driver. Each discovery table contains 3 items, > + * - GUID > + * - Telemetry size > + * - Telemetry offset (offset from P2SB BAR, not GT) > + * > + * For DG2 we know what the telemetry offset is, but we still need to > + * use the discovery table to pass the GUID and the size. So figure > + * out the difference between the P2SB offset and the GT offset and > + * save this so that the telemetry driver can use it to adjust the > + * value. > + */ > +static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev, > + struct intel_vsec_platform_info *info) > +{ > + void __iomem *base; > + u32 telem_offset; > + u64 addr; > + > + /* compile check to verify that quirk has P2SB quirk added */ I don't know what "quirk" the first instance refers to (I think I probably know what the P2SB quirk is). What is the purpose/meaning of this comment? Is it some leftover for code that no longer exists as it talks about "compile check"??
>-----Original Message----- >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> >Sent: Thursday, July 11, 2024 7:45 AM >To: Ruhl, Michael J <michael.j.ruhl@intel.com> >Cc: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; >david.e.box@linux.intel.com; Brost, Matthew <matthew.brost@intel.com> >Subject: Re: [PATCH v6 6/6] drm/xe/vsec: Add support for DG2 > >On Wed, 10 Jul 2024, Michael J. Ruhl wrote: > >> DG2 needs to adjust the discovery offset WRT the GT BAR not the > >WRT = w.r.t. ? > >> P2SB bar so add the base_adjust value to allow for the difference >> to be used. > >Could you please try to write out the problem in clearer terms. Currently >the reasonale given in patch 5 doesn't even match the one given here or >it is written in so unclear terms I cannot make the connection. Yes. I will right this up better. >> Update xe_vsec.c to include DG2 header information. >> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> >> --- >> drivers/gpu/drm/xe/xe_vsec.c | 81 >++++++++++++++++++++++++++++++++++++ >> 1 file changed, 81 insertions(+) >> >> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c >> index 98999d467db1..531ddd32a1a6 100644 >> --- a/drivers/gpu/drm/xe/xe_vsec.c >> +++ b/drivers/gpu/drm/xe/xe_vsec.c >> @@ -19,6 +19,16 @@ >> >> #define SOC_BASE 0x280000 >> >> +/* from drivers/platform/x86/intel/pmt/telemetry.c */ >> +#define TELEM_BASE_OFFSET 0x8 >> + >> +#define DG2_PMT_BASE 0xE8000 >> +#define DG2_DISCOVERY_START 0x6000 >> +#define DG2_TELEM_START 0x4000 >> + >> +#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE + >DG2_DISCOVERY_START) >> +#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE + >DG2_TELEM_START) >> + >> #define BMG_PMT_BASE 0xDB000 >> #define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE) >> >> @@ -32,6 +42,20 @@ >> #define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08) >> #define SG_REMAP_BITS GENMASK(31, 24) >> >> +static struct intel_vsec_header dg2_telemetry = { >> + .length = 0x10, >> + .id = VSEC_ID_TELEMETRY, >> + .num_entries = 1, >> + .entry_size = 3, >> + .tbir = GFX_BAR, >> + .offset = DG2_DISCOVERY_OFFSET, >> +}; >> + >> +static struct intel_vsec_header *dg2_capabilities[] = { >> + &dg2_telemetry, >> + NULL >> +}; >> + >> static struct intel_vsec_header bmg_telemetry = { >> .length = 0x10, >> .id = VSEC_ID_TELEMETRY, >> @@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] = >{ >> >> enum xe_vsec { >> XE_VSEC_UNKNOWN = 0, >> + XE_VSEC_DG2, >> XE_VSEC_BMG, >> }; >> >> static struct intel_vsec_platform_info xe_vsec_info[] = { >> + [XE_VSEC_DG2] = { >> + .caps = VSEC_CAP_TELEMETRY, >> + .headers = dg2_capabilities, >> + .quirks = VSEC_QUIRK_EARLY_HW, >> + }, >> [XE_VSEC_BMG] = { >> .caps = VSEC_CAP_TELEMETRY, >> .headers = bmg_capabilities, >> @@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = { >> }; >> >> static const int vsec_platforms[] = { >> + [XE_DG2] = XE_VSEC_DG2, >> [XE_BATTLEMAGE] = XE_VSEC_BMG, >> }; >> >> @@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct >xe_device *xe) >> return vsec_platforms[xe->info.platform]; >> } >> >> +/* >> + * Access the DG2 PMT MMIO discovery table >> + * >> + * The intel_vsec driver does not typically access the discovery table. >> + * Instead, it creates a memory resource for the table and passes it >> + * to the PMT telemetry driver. Each discovery table contains 3 items, >> + * - GUID >> + * - Telemetry size >> + * - Telemetry offset (offset from P2SB BAR, not GT) >> + * >> + * For DG2 we know what the telemetry offset is, but we still need to >> + * use the discovery table to pass the GUID and the size. So figure >> + * out the difference between the P2SB offset and the GT offset and >> + * save this so that the telemetry driver can use it to adjust the >> + * value. >> + */ >> +static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev, >> + struct intel_vsec_platform_info *info) >> +{ >> + void __iomem *base; >> + u32 telem_offset; >> + u64 addr; >> + >> + /* compile check to verify that quirk has P2SB quirk added */ > >I don't know what "quirk" the first instance refers to (I think I >probably know what the P2SB quirk is). > >What is the purpose/meaning of this comment? Is it some leftover for >code that no longer exists as it talks about "compile check"?? Artifact from my first attempt. I will remove this and update the comment below (another quirk reference). Thanks! M >-- > i. > >> + >> + addr = pci_resource_start(pdev, GFX_BAR) + info->headers[0]->offset; >> + base = ioremap_wc(addr, 16); >> + if (!base) >> + return -ENOMEM; >> + >> + telem_offset = readl(base + TELEM_BASE_OFFSET); >> + >> + /* Use the base_addr + P2SB quirk to pass this info */ >> + if (telem_offset < DG2_TELEM_OFFSET) >> + info->base_adjust = -(DG2_TELEM_OFFSET - telem_offset); >> + else >> + info->base_adjust = -(telem_offset - DG2_TELEM_OFFSET); >> + >> + iounmap(base); >> + >> + return 0; >> +} >> + >> /** >> * xe_vsec_init - Initialize resources and add intel_vsec auxiliary >> * interface >> @@ -196,6 +270,7 @@ void xe_vsec_init(struct xe_device *xe) >> struct device *dev = xe->drm.dev; >> struct pci_dev *pdev = to_pci_dev(dev); >> enum xe_vsec platform; >> + u32 ret; >> >> platform = get_platform_info(xe); >> if (platform == XE_VSEC_UNKNOWN) >> @@ -206,6 +281,12 @@ void xe_vsec_init(struct xe_device *xe) >> return; >> >> switch (platform) { >> + case XE_VSEC_DG2: >> + ret = dg2_adjust_offset(pdev, dev, info); >> + if (ret) >> + return; >> + break; >> + >> case XE_VSEC_BMG: >> info->priv_data = &xe_pmt_cb; >> break; >>
diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c index 98999d467db1..531ddd32a1a6 100644 --- a/drivers/gpu/drm/xe/xe_vsec.c +++ b/drivers/gpu/drm/xe/xe_vsec.c @@ -19,6 +19,16 @@ #define SOC_BASE 0x280000 +/* from drivers/platform/x86/intel/pmt/telemetry.c */ +#define TELEM_BASE_OFFSET 0x8 + +#define DG2_PMT_BASE 0xE8000 +#define DG2_DISCOVERY_START 0x6000 +#define DG2_TELEM_START 0x4000 + +#define DG2_DISCOVERY_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_DISCOVERY_START) +#define DG2_TELEM_OFFSET (SOC_BASE + DG2_PMT_BASE + DG2_TELEM_START) + #define BMG_PMT_BASE 0xDB000 #define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE) @@ -32,6 +42,20 @@ #define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08) #define SG_REMAP_BITS GENMASK(31, 24) +static struct intel_vsec_header dg2_telemetry = { + .length = 0x10, + .id = VSEC_ID_TELEMETRY, + .num_entries = 1, + .entry_size = 3, + .tbir = GFX_BAR, + .offset = DG2_DISCOVERY_OFFSET, +}; + +static struct intel_vsec_header *dg2_capabilities[] = { + &dg2_telemetry, + NULL +}; + static struct intel_vsec_header bmg_telemetry = { .length = 0x10, .id = VSEC_ID_TELEMETRY, @@ -48,10 +72,16 @@ static struct intel_vsec_header *bmg_capabilities[] = { enum xe_vsec { XE_VSEC_UNKNOWN = 0, + XE_VSEC_DG2, XE_VSEC_BMG, }; static struct intel_vsec_platform_info xe_vsec_info[] = { + [XE_VSEC_DG2] = { + .caps = VSEC_CAP_TELEMETRY, + .headers = dg2_capabilities, + .quirks = VSEC_QUIRK_EARLY_HW, + }, [XE_VSEC_BMG] = { .caps = VSEC_CAP_TELEMETRY, .headers = bmg_capabilities, @@ -174,6 +204,7 @@ struct pmt_callbacks xe_pmt_cb = { }; static const int vsec_platforms[] = { + [XE_DG2] = XE_VSEC_DG2, [XE_BATTLEMAGE] = XE_VSEC_BMG, }; @@ -185,6 +216,49 @@ static enum xe_vsec get_platform_info(struct xe_device *xe) return vsec_platforms[xe->info.platform]; } +/* + * Access the DG2 PMT MMIO discovery table + * + * The intel_vsec driver does not typically access the discovery table. + * Instead, it creates a memory resource for the table and passes it + * to the PMT telemetry driver. Each discovery table contains 3 items, + * - GUID + * - Telemetry size + * - Telemetry offset (offset from P2SB BAR, not GT) + * + * For DG2 we know what the telemetry offset is, but we still need to + * use the discovery table to pass the GUID and the size. So figure + * out the difference between the P2SB offset and the GT offset and + * save this so that the telemetry driver can use it to adjust the + * value. + */ +static int dg2_adjust_offset(struct pci_dev *pdev, struct device *dev, + struct intel_vsec_platform_info *info) +{ + void __iomem *base; + u32 telem_offset; + u64 addr; + + /* compile check to verify that quirk has P2SB quirk added */ + + addr = pci_resource_start(pdev, GFX_BAR) + info->headers[0]->offset; + base = ioremap_wc(addr, 16); + if (!base) + return -ENOMEM; + + telem_offset = readl(base + TELEM_BASE_OFFSET); + + /* Use the base_addr + P2SB quirk to pass this info */ + if (telem_offset < DG2_TELEM_OFFSET) + info->base_adjust = -(DG2_TELEM_OFFSET - telem_offset); + else + info->base_adjust = -(telem_offset - DG2_TELEM_OFFSET); + + iounmap(base); + + return 0; +} + /** * xe_vsec_init - Initialize resources and add intel_vsec auxiliary * interface @@ -196,6 +270,7 @@ void xe_vsec_init(struct xe_device *xe) struct device *dev = xe->drm.dev; struct pci_dev *pdev = to_pci_dev(dev); enum xe_vsec platform; + u32 ret; platform = get_platform_info(xe); if (platform == XE_VSEC_UNKNOWN) @@ -206,6 +281,12 @@ void xe_vsec_init(struct xe_device *xe) return; switch (platform) { + case XE_VSEC_DG2: + ret = dg2_adjust_offset(pdev, dev, info); + if (ret) + return; + break; + case XE_VSEC_BMG: info->priv_data = &xe_pmt_cb; break;
DG2 needs to adjust the discovery offset WRT the GT BAR not the P2SB bar so add the base_adjust value to allow for the difference to be used. Update xe_vsec.c to include DG2 header information. Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> --- drivers/gpu/drm/xe/xe_vsec.c | 81 ++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)