> -----Original Message-----
> From: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> Sent: Thursday, August 8, 2024 8:57 PM
> To: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
> Cc: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; intel-xe@xxxxxxxxxxxxxxxxxxxxx;
> platform-driver-x86@xxxxxxxxxxxxxxx; ilpo.jarvinen@xxxxxxxxxxxxxxx; Brost,
> Matthew <matthew.brost@xxxxxxxxx>; andriy.shevchenko@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT
> base adjust
>
> On Thu, 2024-08-08 at 17:01 -0400, Rodrigo Vivi wrote:
> > On Thu, Aug 08, 2024 at 12:49:58PM -0700, David E. Box wrote:
> > > Hi Mike
> > >
> > > On Thu, 2024-07-25 at 08:23 -0400, Michael J. Ruhl wrote:
> > > > DVSEC offsets are based on the endpoint BAR. If an endpoint is
> > > > not available allow the offset information to be adjusted by the
> > > > parent driver.
> > >
> > > I know I wrote the original version of these patches but I no longer
> > > like this solution. The s32 is too small for a 64 bit address and
> > > calculating the offset just to add it back in the PMT driver is
> > > unnecessary.
> >
> > yeap, 64bit sounds better indeed.
> >
> > > Instead, I sent you
> > > replacement patches for 5 and 6 that allow passing the telemetry
> > > region address directly to the PMT driver.
> >
> > Was these replacements sent straight to PMT list or to Mike so he can
> > adjust the series?
> >
> > I'm wondering if we should merge this through our drm-xe-next or
> > through PMT channels. Thoughts?
> >
> > In any case, ack from my side to get the xe patches merged together
> > through PMT.
> >
> > But if someone prefer to get this merged through drm-xe-next, then we
> > need to act fast and get this ready with the final patches and acked
> > by you PMT maintainers, in the next 2 weeks because our window under
> > drm closes much earlier.
> >
> > Around 6.11-rc5 is when we close the drm window towards 6.12 and we
> > are almost within 6.11-rc3.
> >
> > Thoughts?
>
> For me Patches 1-4 are good to go for BMG support. Patches 5 and 6 add DG2
> support but need some work. They should wait.
David, Ilpo,
The DG2 patches are a nice to have.
Please take patch 1 - 4.
I will revisit 5 and 6 (with David's suggested changes) in the future.
Thanks!
Mike
> David
>
> >
> > Thanks,
> > Rodrigo.
> >
> > >
> > > David
> > >
> > > >
> > > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
> > > > ---
> > > > drivers/platform/x86/intel/pmt/class.h | 1 +
> > > > drivers/platform/x86/intel/pmt/telemetry.c | 9 +++++++++
> > > > drivers/platform/x86/intel/vsec.c | 1 +
> > > > include/linux/intel_vsec.h | 3 +++
> > > > 4 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/platform/x86/intel/pmt/class.h
> > > > b/drivers/platform/x86/intel/pmt/class.h
> > > > index a267ac964423..984cd40ee814 100644
> > > > --- a/drivers/platform/x86/intel/pmt/class.h
> > > > +++ b/drivers/platform/x86/intel/pmt/class.h
> > > > @@ -46,6 +46,7 @@ struct intel_pmt_entry {
> > > > void __iomem *base;
> > > > struct pmt_callbacks *cb;
> > > > unsigned long base_addr;
> > > > + s32 base_adjust;
> > > > size_t size;
> > > > u32 guid;
> > > > int devid;
> > > > diff --git a/drivers/platform/x86/intel/pmt/telemetry.c
> > > > b/drivers/platform/x86/intel/pmt/telemetry.c
> > > > index c9feac859e57..88e4f1315097 100644
> > > > --- a/drivers/platform/x86/intel/pmt/telemetry.c
> > > > +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> > > > @@ -78,6 +78,13 @@ static int pmt_telem_header_decode(struct
> > > > intel_pmt_entry *entry,
> > > > header->access_type = TELEM_ACCESS(readl(disc_table));
> > > > header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> > > > header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> > > > + if (entry->base_adjust) {
> > > > + u32 new_base = header->base_offset + entry->base_adjust;
> > > > +
> > > > + dev_dbg(dev, "Adjusting base offset from 0x%x to 0x%x\n",
> > > > + header->base_offset, new_base);
> > > > + header->base_offset = new_base;
> > > > + }
> > > >
> > > > /* Size is measured in DWORDS, but accessor returns bytes */
> > > > header->size = TELEM_SIZE(readl(disc_table)); @@ -302,6 +309,8
> > > > @@ static int pmt_telem_probe(struct auxiliary_device *auxdev,
> > > > const struct auxilia
> > > > for (i = 0; i < intel_vsec_dev->num_resources; i++) {
> > > > struct intel_pmt_entry *entry = &priv->entry[priv-
> > > > > num_entries];
> > > >
> > > > + entry->base_adjust = intel_vsec_dev->base_adjust;
> > > > +
> > > > mutex_lock(&ep_lock);
> > > > ret = intel_pmt_dev_create(entry, &pmt_telem_ns,
> > > > intel_vsec_dev, i);
> > > > mutex_unlock(&ep_lock);
> > > > diff --git a/drivers/platform/x86/intel/vsec.c
> > > > b/drivers/platform/x86/intel/vsec.c
> > > > index 7b5cc9993974..be079d62a7bc 100644
> > > > --- a/drivers/platform/x86/intel/vsec.c
> > > > +++ b/drivers/platform/x86/intel/vsec.c
> > > > @@ -212,6 +212,7 @@ static int intel_vsec_add_dev(struct pci_dev
> > > > *pdev, struct intel_vsec_header *he
> > > > intel_vsec_dev->num_resources = header->num_entries;
> > > > intel_vsec_dev->quirks = info->quirks;
> > > > intel_vsec_dev->base_addr = info->base_addr;
> > > > + intel_vsec_dev->base_adjust = info->base_adjust;
> > > > intel_vsec_dev->priv_data = info->priv_data;
> > > >
> > > > if (header->id == VSEC_ID_SDSI)
> > > > diff --git a/include/linux/intel_vsec.h
> > > > b/include/linux/intel_vsec.h index 11ee185566c3..75d17fa10d05
> > > > 100644
> > > > --- a/include/linux/intel_vsec.h
> > > > +++ b/include/linux/intel_vsec.h
> > > > @@ -88,6 +88,7 @@ struct pmt_callbacks {
> > > > * @caps: bitmask of PMT capabilities for the given headers
> > > > * @quirks: bitmask of VSEC device quirks
> > > > * @base_addr: allow a base address to be specified (rather than
> > > > derived)
> > > > + * @base_adjust: allow adjustment to base offset information
> > > > */
> > > > struct intel_vsec_platform_info {
> > > > struct device *parent;
> > > > @@ -96,6 +97,7 @@ struct intel_vsec_platform_info {
> > > > unsigned long caps;
> > > > unsigned long quirks;
> > > > u64 base_addr;
> > > > + s32 base_adjust;
> > > > };
> > > >
> > > > /**
> > > > @@ -121,6 +123,7 @@ struct intel_vsec_device {
> > > > size_t priv_data_size;
> > > > unsigned long quirks;
> > > > u64 base_addr;
> > > > + s32 base_adjust;
> > > > };
> > > >
> > > > int intel_vsec_add_aux(struct pci_dev *pdev, struct device
> > > > *parent,
> > >
