| Message ID | 20240725122346.4063913-6-michael.j.ruhl@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Support PMT features in Xe | expand |
>-----Original Message----- >From: Ruhl, Michael J <michael.j.ruhl@intel.com> >Sent: Thursday, July 25, 2024 8:24 AM >To: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; >david.e.box@linux.intel.com; ilpo.jarvinen@linux.intel.com; Brost, Matthew ><matthew.brost@intel.com>; andriy.shevchenko@linux.intel.com >Cc: Ruhl, Michael J <michael.j.ruhl@intel.com> >Subject: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base >adjust > >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. Hello, Any further comments or questions WRT this patch? Thanks, M >Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> >--- > 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, >-- >2.44.0
On Tue, 30 Jul 2024, Ruhl, Michael J wrote: > >-----Original Message----- > >From: Ruhl, Michael J <michael.j.ruhl@intel.com> > >Sent: Thursday, July 25, 2024 8:24 AM > >To: intel-xe@lists.freedesktop.org; platform-driver-x86@vger.kernel.org; > >david.e.box@linux.intel.com; ilpo.jarvinen@linux.intel.com; Brost, Matthew > ><matthew.brost@intel.com>; andriy.shevchenko@linux.intel.com > >Cc: Ruhl, Michael J <michael.j.ruhl@intel.com> > >Subject: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base > >adjust > > > >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. > > Hello, > > Any further comments or questions WRT this patch? Hi, Please don't send take notice of my patch/series asap emails. Especially this close to the original sending, at minimum wait at least 2 weeks. But in case of platform-drivers-x86, that's hardly necessary even then as we don't forget patches, they're tracked in patchwork which is kept up to date. You can find you if your patch is still in the queue from the patchwork, if it is, we'll get to it eventually. A maintainer (or some reviewer, if we're lucky :-)) will get to your patch/series eventually. People just have many things to do and have to prioritize their time. We're barely past the merge window so there's plenty of time in this cycle and this tends to be the busiest time of the cycle.
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. Instead, I sent you replacement patches for 5 and 6 that allow passing the telemetry region address directly to the PMT driver. David > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > --- > 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,
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? Thanks, Rodrigo. > > David > > > > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > > --- > > 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, >
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 > > Thanks, > Rodrigo. > > > > > David > > > > > > > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > > > --- > > > 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, > >
> -----Original Message----- > From: David E. Box <david.e.box@linux.intel.com> > Sent: Thursday, August 8, 2024 8:57 PM > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Cc: Ruhl, Michael J <michael.j.ruhl@intel.com>; intel-xe@lists.freedesktop.org; > platform-driver-x86@vger.kernel.org; ilpo.jarvinen@linux.intel.com; Brost, > Matthew <matthew.brost@intel.com>; andriy.shevchenko@linux.intel.com > 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@intel.com> > > > > --- > > > > 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, > > >
On Fri, 9 Aug 2024, Ruhl, Michael J wrote: > > -----Original Message----- > > From: David E. Box <david.e.box@linux.intel.com> > > Sent: Thursday, August 8, 2024 8:57 PM > > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > Cc: Ruhl, Michael J <michael.j.ruhl@intel.com>; intel-xe@lists.freedesktop.org; > > platform-driver-x86@vger.kernel.org; ilpo.jarvinen@linux.intel.com; Brost, > > Matthew <matthew.brost@intel.com>; andriy.shevchenko@linux.intel.com > > 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. Hans is the one handling pdx86 for-next patches in this cycle (we as the pdx86 maintainers alternate it on every other kernel release). Please remember to add him into receipient list when you send the next version with my comments on 4th patch addressed (always include all relevant maintainers when sending patches).
On Mon, Aug 12, 2024 at 12:22:36PM +0300, Ilpo Järvinen wrote: > On Fri, 9 Aug 2024, Ruhl, Michael J wrote: > > > > -----Original Message----- > > > From: David E. Box <david.e.box@linux.intel.com> > > > Sent: Thursday, August 8, 2024 8:57 PM > > > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > Cc: Ruhl, Michael J <michael.j.ruhl@intel.com>; intel-xe@lists.freedesktop.org; > > > platform-driver-x86@vger.kernel.org; ilpo.jarvinen@linux.intel.com; Brost, > > > Matthew <matthew.brost@intel.com>; andriy.shevchenko@linux.intel.com > > > 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. > > Hans is the one handling pdx86 for-next patches in this cycle (we as the > pdx86 maintainers alternate it on every other kernel release). Please > remember to add him into receipient list when you send the next version > with my comments on 4th patch addressed (always include all relevant > maintainers when sending patches). Hans, Ilmo, any chance we could get these PMT along with the Xe ones into drm-xe-next -> drm-next? Well, if you believe the risk of conflicts later on your side is bigger, then let's forgot and you have my ack to add the Xe patches along with your PMT patches on your tree. But if there's a possibility of getting these through our tree, I would appreciate. Thanks, Rodrigo. > > -- > i.
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,
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. Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com> --- 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(+)