| Message ID | 20240710192249.3915396-3-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: > From: "David E. Box" <david.e.box@linux.intel.com> > > Some PMT providers require device specific actions before their telemetry > can be read. Provide assignable PMT read callbacks to allow providers to > perform those actions. > > Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > --- > drivers/platform/x86/intel/vsec.c | 1 + > include/linux/intel_vsec.h | 14 ++++++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c > index 2b46807f868b..7b5cc9993974 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->priv_data = info->priv_data; > > if (header->id == VSEC_ID_SDSI) > intel_vsec_dev->ida = &intel_vsec_sdsi_ida; > diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h > index 1a287541a2f9..4569a55e8645 100644 > --- a/include/linux/intel_vsec.h > +++ b/include/linux/intel_vsec.h > @@ -67,6 +67,19 @@ enum intel_vsec_quirks { > VSEC_QUIRK_EARLY_HW = BIT(4), > }; > > +/** > + * struct pmt_callbacks - Callback infrastructure for PMT devices > + * ->read_telem() when specified, called by client driver to access PMT data (instead > + * of direct copy). > + * @args: pci device info pointer PCI > + * @guid: ID of data to acccss > + * @data: buffer for the data to be copied > + * @count: size of buffer > + */ > +struct pmt_callbacks { > + int (*read_telem)(void *args, u32 guid, u64 *data, u32 count); Can you explain why args is void * and not struct pci_dev *? I was already unsure earlier why void * is needed but now you even explicitly documented it to be "pci device info pointer". > +}; > + > /** > * struct intel_vsec_platform_info - Platform specific data > * @parent: parent device in the auxbus chain > @@ -78,6 +91,7 @@ enum intel_vsec_quirks { > struct intel_vsec_platform_info { > struct device *parent; > struct intel_vsec_header **headers; > + void *priv_data; > unsigned long caps; > unsigned long quirks; > u64 base_addr; >
>-----Original Message----- >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> >Sent: Thursday, July 11, 2024 7:17 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 2/6] platform/x86/intel/vsec: Add PMT read callbacks > >On Wed, 10 Jul 2024, Michael J. Ruhl wrote: > >> From: "David E. Box" <david.e.box@linux.intel.com> >> >> Some PMT providers require device specific actions before their telemetry >> can be read. Provide assignable PMT read callbacks to allow providers to >> perform those actions. >> >> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com> >> Signed-off-by: David E. Box <david.e.box@linux.intel.com> >> --- >> drivers/platform/x86/intel/vsec.c | 1 + >> include/linux/intel_vsec.h | 14 ++++++++++++++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/drivers/platform/x86/intel/vsec.c >b/drivers/platform/x86/intel/vsec.c >> index 2b46807f868b..7b5cc9993974 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->priv_data = info->priv_data; >> >> if (header->id == VSEC_ID_SDSI) >> intel_vsec_dev->ida = &intel_vsec_sdsi_ida; >> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h >> index 1a287541a2f9..4569a55e8645 100644 >> --- a/include/linux/intel_vsec.h >> +++ b/include/linux/intel_vsec.h >> @@ -67,6 +67,19 @@ enum intel_vsec_quirks { >> VSEC_QUIRK_EARLY_HW = BIT(4), >> }; >> >> +/** >> + * struct pmt_callbacks - Callback infrastructure for PMT devices >> + * ->read_telem() when specified, called by client driver to access PMT data >(instead >> + * of direct copy). >> + * @args: pci device info pointer > >PCI > >> + * @guid: ID of data to acccss >> + * @data: buffer for the data to be copied >> + * @count: size of buffer >> + */ >> +struct pmt_callbacks { >> + int (*read_telem)(void *args, u32 guid, u64 *data, u32 count); > >Can you explain why args is void * and not struct pci_dev *? I was already >unsure earlier why void * is needed but now you even explicitly documented >it to be "pci device info pointer". This was an artifact of a previous attempt. I was going to use an opaque pointer to pass the device info, but David pointed out that the PCI device was available and sufficient. I have updated to use struct pce_dev. Thanks! M >> +}; >> + >> /** >> * struct intel_vsec_platform_info - Platform specific data >> * @parent: parent device in the auxbus chain >> @@ -78,6 +91,7 @@ enum intel_vsec_quirks { >> struct intel_vsec_platform_info { >> struct device *parent; >> struct intel_vsec_header **headers; >> + void *priv_data; >> unsigned long caps; >> unsigned long quirks; >> u64 base_addr; >> > >-- > i.
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c index 2b46807f868b..7b5cc9993974 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->priv_data = info->priv_data; if (header->id == VSEC_ID_SDSI) intel_vsec_dev->ida = &intel_vsec_sdsi_ida; diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h index 1a287541a2f9..4569a55e8645 100644 --- a/include/linux/intel_vsec.h +++ b/include/linux/intel_vsec.h @@ -67,6 +67,19 @@ enum intel_vsec_quirks { VSEC_QUIRK_EARLY_HW = BIT(4), }; +/** + * struct pmt_callbacks - Callback infrastructure for PMT devices + * ->read_telem() when specified, called by client driver to access PMT data (instead + * of direct copy). + * @args: pci device info pointer + * @guid: ID of data to acccss + * @data: buffer for the data to be copied + * @count: size of buffer + */ +struct pmt_callbacks { + int (*read_telem)(void *args, u32 guid, u64 *data, u32 count); +}; + /** * struct intel_vsec_platform_info - Platform specific data * @parent: parent device in the auxbus chain @@ -78,6 +91,7 @@ enum intel_vsec_quirks { struct intel_vsec_platform_info { struct device *parent; struct intel_vsec_header **headers; + void *priv_data; unsigned long caps; unsigned long quirks; u64 base_addr;