* [PATCH v9 0/6] Support PMT features in Xe
@ 2024-07-25 12:23 Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl
DG2 and Battlemage have the Intel Platform Monitoring Technology (PMT)
feature available, but not in the "standard" (pci endpoint) way.
Add support to the vsec and Xe drivers to allow access to the PMT space
for the DG2 and BMG devices.
The intel_vsec_register() function allows drivers to provide telemetry
header information (usually found at probe time), to allow the PMT
driver to probe the telemetry features.
Battlemage has a shared memory area (selected by index), so a callback
function is required to access the appropriate PMT data.
V2:
Re-worked DG2 support patches using a base_adjust rather than a
quirk.
Updated GUID decode, for correct decode.
v3:
Fixed a documentation issue for the pmt struct.
v4:
Fixed a documentation issue in the xe_vsec.c module
v5:
Addressed review comments for patch 4 (Xe driver)
Add r/b for the first three patches
v6:
Added kernel doc to moved data structure
Added required include files
Correct usage for FIELD_PREP()/FIELD_GET()
Whitespace clean up
Removed unnecessary type cast
v7:
Commit message updates
v8:
Added some r/b (patch 2 and 3).
Updated kernel doc patch 2 (priv_data) patch 5 (base_adjust)
v9:
Add r/b for the Xe driver patches
David E. Box (3):
platform/x86/intel/vsec.h: Move to include/linux
platform/x86/intel/vsec: Add PMT read callbacks
platform/x86/intel/pmt: Use PMT callbacks
Michael J. Ruhl (3):
drm/xe/vsec: Support BMG devices
platform/x86/intel/pmt: Add support for PMT base adjust
drm/xe/vsec: Add support for DG2
MAINTAINERS | 3 +-
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 5 +
drivers/gpu/drm/xe/xe_device_types.h | 6 +
drivers/gpu/drm/xe/xe_vsec.c | 300 ++++++++++++++++++
drivers/gpu/drm/xe/xe_vsec.h | 13 +
drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
drivers/platform/x86/intel/pmt/class.c | 28 +-
drivers/platform/x86/intel/pmt/class.h | 11 +-
drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
drivers/platform/x86/intel/pmt/telemetry.c | 21 +-
drivers/platform/x86/intel/sdsi.c | 3 +-
drivers/platform/x86/intel/tpmi.c | 3 +-
drivers/platform/x86/intel/vsec.c | 9 +-
.../vsec.h => include/linux/intel_vsec.h | 50 ++-
15 files changed, 428 insertions(+), 29 deletions(-)
create mode 100644 drivers/gpu/drm/xe/xe_vsec.c
create mode 100644 drivers/gpu/drm/xe/xe_vsec.h
rename drivers/platform/x86/intel/vsec.h => include/linux/intel_vsec.h (61%)
--
2.44.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v9 1/6] platform/x86/intel/vsec.h: Move to include/linux
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl
From: "David E. Box" <david.e.box@linux.intel.com>
Some drivers outside of PDX86 need access to the vsec header. Move it to
include/linux to make it easier to include.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
MAINTAINERS | 3 +-
drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
drivers/platform/x86/intel/pmt/class.c | 2 +-
drivers/platform/x86/intel/pmt/class.h | 2 +-
drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
drivers/platform/x86/intel/pmt/telemetry.c | 2 +-
drivers/platform/x86/intel/sdsi.c | 3 +-
drivers/platform/x86/intel/tpmi.c | 3 +-
drivers/platform/x86/intel/vsec.c | 7 ++--
.../vsec.h => include/linux/intel_vsec.h | 32 +++++++++++++++++--
10 files changed, 41 insertions(+), 17 deletions(-)
rename drivers/platform/x86/intel/vsec.h => include/linux/intel_vsec.h (71%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 89034b72a020..c7de5b7e0983 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11425,7 +11425,8 @@ F: drivers/platform/x86/intel/uncore-frequency/
INTEL VENDOR SPECIFIC EXTENDED CAPABILITIES DRIVER
M: David E. Box <david.e.box@linux.intel.com>
S: Supported
-F: drivers/platform/x86/intel/vsec.*
+F: drivers/platform/x86/intel/vsec.c
+F: include/linux/intel_vsec.h
INTEL VIRTUAL BUTTON DRIVER
M: AceLan Kao <acelan.kao@canonical.com>
diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
index 1bde86c54eb9..baddaaec25ee 100644
--- a/drivers/platform/x86/intel/pmc/core_ssram.c
+++ b/drivers/platform/x86/intel/pmc/core_ssram.c
@@ -9,11 +9,11 @@
*/
#include <linux/cleanup.h>
+#include <linux/intel_vsec.h>
#include <linux/pci.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include "core.h"
-#include "../vsec.h"
#include "../pmt/telemetry.h"
#define SSRAM_HDR_SIZE 0x100
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 4b53940a64e2..d7939b28e937 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -9,12 +9,12 @@
*/
#include <linux/kernel.h>
+#include <linux/intel_vsec.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/module.h>
#include <linux/mm.h>
#include <linux/pci.h>
-#include "../vsec.h"
#include "class.h"
#define PMT_XA_START 1
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index d23c63b73ab7..d6f9ccaf28c8 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -2,13 +2,13 @@
#ifndef _INTEL_PMT_CLASS_H
#define _INTEL_PMT_CLASS_H
+#include <linux/intel_vsec.h>
#include <linux/xarray.h>
#include <linux/types.h>
#include <linux/bits.h>
#include <linux/err.h>
#include <linux/io.h>
-#include "../vsec.h"
#include "telemetry.h"
/* PMT access types */
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index 4014c02cafdb..9079d5dffc03 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -9,6 +9,7 @@
*/
#include <linux/auxiliary_bus.h>
+#include <linux/intel_vsec.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -16,7 +17,6 @@
#include <linux/uaccess.h>
#include <linux/overflow.h>
-#include "../vsec.h"
#include "class.h"
/* Crashlog discovery header types */
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 09258564dfc4..3478f891ea0b 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -9,6 +9,7 @@
*/
#include <linux/auxiliary_bus.h>
+#include <linux/intel_vsec.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -16,7 +17,6 @@
#include <linux/uaccess.h>
#include <linux/overflow.h>
-#include "../vsec.h"
#include "class.h"
#define TELEM_SIZE_OFFSET 0x0
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 277e4f4b20ac..9d137621f0e6 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -12,6 +12,7 @@
#include <linux/bits.h>
#include <linux/bitfield.h>
#include <linux/device.h>
+#include <linux/intel_vsec.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -22,8 +23,6 @@
#include <linux/types.h>
#include <linux/uaccess.h>
-#include "vsec.h"
-
#define ACCESS_TYPE_BARID 2
#define ACCESS_TYPE_LOCAL 3
diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 6c0cbccd80bb..b9fa1cbfdcf7 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -51,6 +51,7 @@
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/intel_tpmi.h>
+#include <linux/intel_vsec.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/module.h>
@@ -59,8 +60,6 @@
#include <linux/sizes.h>
#include <linux/string_helpers.h>
-#include "vsec.h"
-
/**
* struct intel_tpmi_pfs_entry - TPMI PM Feature Structure (PFS) entry
* @tpmi_id: TPMI feature identifier (what the feature is and its data format).
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 0fdfaf3a4f5c..2b46807f868b 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -17,14 +17,13 @@
#include <linux/bits.h>
#include <linux/cleanup.h>
#include <linux/delay.h>
-#include <linux/kernel.h>
#include <linux/idr.h>
+#include <linux/intel_vsec.h>
+#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/types.h>
-#include "vsec.h"
-
#define PMT_XA_START 0
#define PMT_XA_MAX INT_MAX
#define PMT_XA_LIMIT XA_LIMIT(PMT_XA_START, PMT_XA_MAX)
@@ -341,7 +340,7 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
void intel_vsec_register(struct pci_dev *pdev,
struct intel_vsec_platform_info *info)
{
- if (!pdev || !info)
+ if (!pdev || !info || !info->headers)
return;
intel_vsec_walk_header(pdev, info);
diff --git a/drivers/platform/x86/intel/vsec.h b/include/linux/intel_vsec.h
similarity index 71%
rename from drivers/platform/x86/intel/vsec.h
rename to include/linux/intel_vsec.h
index e23e76129691..6495e37c9079 100644
--- a/drivers/platform/x86/intel/vsec.h
+++ b/include/linux/intel_vsec.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _VSEC_H
-#define _VSEC_H
+#ifndef _INTEL_VSEC_H
+#define _INTEL_VSEC_H
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
@@ -67,7 +67,14 @@ enum intel_vsec_quirks {
VSEC_QUIRK_EARLY_HW = BIT(4),
};
-/* Platform specific data */
+/**
+ * struct intel_vsec_platform_info - Platform specific data
+ * @parent: parent device in the auxbus chain
+ * @headers: list of headers to define the PMT client devices to create
+ * @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)
+ */
struct intel_vsec_platform_info {
struct device *parent;
struct intel_vsec_header **headers;
@@ -76,6 +83,18 @@ struct intel_vsec_platform_info {
u64 base_addr;
};
+/**
+ * struct intel_sec_device - Auxbus specific device information
+ * @auxdev: auxbus device struct for auxbus access
+ * @pcidev: pci device associated with the device
+ * @resource: any resources shared by the parent
+ * @ida: id reference
+ * @num_resources: number of resources
+ * @id: xarray id
+ * @priv_data: any private data needed
+ * @quirks: specified quirks
+ * @base_addr: base address of entries (if specified)
+ */
struct intel_vsec_device {
struct auxiliary_device auxdev;
struct pci_dev *pcidev;
@@ -103,6 +122,13 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device
return container_of(auxdev, struct intel_vsec_device, auxdev);
}
+#if IS_ENABLED(CONFIG_INTEL_VSEC)
void intel_vsec_register(struct pci_dev *pdev,
struct intel_vsec_platform_info *info);
+#else
+static inline void intel_vsec_register(struct pci_dev *pdev,
+ struct intel_vsec_platform_info *info)
+{
+}
+#endif
#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v9 2/6] platform/x86/intel/vsec: Add PMT read callbacks
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 3/6] platform/x86/intel/pmt: Use PMT callbacks Michael J. Ruhl
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl
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: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
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 | 15 +++++++++++++++
2 files changed, 16 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 6495e37c9079..11ee185566c3 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -67,10 +67,24 @@ 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).
+ * @pdev: PCI device reference for the callback's use
+ * @guid: ID of data to acccss
+ * @data: buffer for the data to be copied
+ * @count: size of buffer
+ */
+struct pmt_callbacks {
+ int (*read_telem)(struct pci_dev *pdev, u32 guid, u64 *data, u32 count);
+};
+
/**
* struct intel_vsec_platform_info - Platform specific data
* @parent: parent device in the auxbus chain
* @headers: list of headers to define the PMT client devices to create
+ * @priv_data: private data, usable by parent devices, currently a callback
* @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)
@@ -78,6 +92,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;
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v9 3/6] platform/x86/intel/pmt: Use PMT callbacks
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl
From: "David E. Box" <david.e.box@linux.intel.com>
PMT providers may require device specific actions before their telemetry
may be read. If the read_telem() is assigned, call it instead of
memcpy_fromio() and return. Since this needs to be done in multiple
locations, add pmt_telem_read_mmio() as a wrapper function to perform this
and any other needed checks.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
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/pmt/class.c | 26 +++++++++++++++++-----
drivers/platform/x86/intel/pmt/class.h | 8 +++++--
drivers/platform/x86/intel/pmt/telemetry.c | 10 +++++----
3 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index d7939b28e937..c04bb7f97a4d 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -58,6 +58,22 @@ pmt_memcpy64_fromio(void *to, const u64 __iomem *from, size_t count)
return count;
}
+int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf,
+ void __iomem *addr, u32 count)
+{
+ if (cb && cb->read_telem)
+ return cb->read_telem(pdev, guid, buf, count);
+
+ if (guid == GUID_SPR_PUNIT)
+ /* PUNIT on SPR only supports aligned 64-bit read */
+ return pmt_memcpy64_fromio(buf, addr, count);
+
+ memcpy_fromio(buf, addr, count);
+
+ return count;
+}
+EXPORT_SYMBOL_NS_GPL(pmt_telem_read_mmio, INTEL_PMT);
+
/*
* sysfs
*/
@@ -79,11 +95,8 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
if (count > entry->size - off)
count = entry->size - off;
- if (entry->guid == GUID_SPR_PUNIT)
- /* PUNIT on SPR only supports aligned 64-bit read */
- count = pmt_memcpy64_fromio(buf, entry->base + off, count);
- else
- memcpy_fromio(buf, entry->base + off, count);
+ count = pmt_telem_read_mmio(entry->ep->pcidev, entry->cb, entry->header.guid, buf,
+ entry->base + off, count);
return count;
}
@@ -239,6 +252,7 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
entry->guid = header->guid;
entry->size = header->size;
+ entry->cb = ivdev->priv_data;
return 0;
}
@@ -300,7 +314,7 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
goto fail_ioremap;
if (ns->pmt_add_endpoint) {
- ret = ns->pmt_add_endpoint(entry, ivdev->pcidev);
+ ret = ns->pmt_add_endpoint(ivdev, entry);
if (ret)
goto fail_add_endpoint;
}
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index d6f9ccaf28c8..a267ac964423 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -24,6 +24,7 @@ struct pci_dev;
struct telem_endpoint {
struct pci_dev *pcidev;
struct telem_header header;
+ struct pmt_callbacks *cb;
void __iomem *base;
bool present;
struct kref kref;
@@ -43,6 +44,7 @@ struct intel_pmt_entry {
struct kobject *kobj;
void __iomem *disc_table;
void __iomem *base;
+ struct pmt_callbacks *cb;
unsigned long base_addr;
size_t size;
u32 guid;
@@ -55,10 +57,12 @@ struct intel_pmt_namespace {
const struct attribute_group *attr_grp;
int (*pmt_header_decode)(struct intel_pmt_entry *entry,
struct device *dev);
- int (*pmt_add_endpoint)(struct intel_pmt_entry *entry,
- struct pci_dev *pdev);
+ int (*pmt_add_endpoint)(struct intel_vsec_device *ivdev,
+ struct intel_pmt_entry *entry);
};
+int pmt_telem_read_mmio(struct pci_dev *pdev, struct pmt_callbacks *cb, u32 guid, void *buf,
+ void __iomem *addr, u32 count);
bool intel_pmt_is_early_client_hw(struct device *dev);
int intel_pmt_dev_create(struct intel_pmt_entry *entry,
struct intel_pmt_namespace *ns,
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index 3478f891ea0b..c9feac859e57 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -93,8 +93,8 @@ static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
return 0;
}
-static int pmt_telem_add_endpoint(struct intel_pmt_entry *entry,
- struct pci_dev *pdev)
+static int pmt_telem_add_endpoint(struct intel_vsec_device *ivdev,
+ struct intel_pmt_entry *entry)
{
struct telem_endpoint *ep;
@@ -104,13 +104,14 @@ static int pmt_telem_add_endpoint(struct intel_pmt_entry *entry,
return -ENOMEM;
ep = entry->ep;
- ep->pcidev = pdev;
+ ep->pcidev = ivdev->pcidev;
ep->header.access_type = entry->header.access_type;
ep->header.guid = entry->header.guid;
ep->header.base_offset = entry->header.base_offset;
ep->header.size = entry->header.size;
ep->base = entry->base;
ep->present = true;
+ ep->cb = ivdev->priv_data;
kref_init(&ep->kref);
@@ -218,7 +219,8 @@ int pmt_telem_read(struct telem_endpoint *ep, u32 id, u64 *data, u32 count)
if (offset + NUM_BYTES_QWORD(count) > size)
return -EINVAL;
- memcpy_fromio(data, ep->base + offset, NUM_BYTES_QWORD(count));
+ pmt_telem_read_mmio(ep->pcidev, ep->cb, ep->header.guid, data, ep->base + offset,
+ NUM_BYTES_QWORD(count));
return ep->present ? 0 : -EPIPE;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v9 4/6] drm/xe/vsec: Support BMG devices
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
` (2 preceding siblings ...)
2024-07-25 12:23 ` [PATCH v9 3/6] platform/x86/intel/pmt: Use PMT callbacks Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-08-07 19:23 ` David E. Box
2024-08-12 9:01 ` Ilpo Järvinen
2024-07-25 12:23 ` [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
` (2 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl, Rodrigo Vivi
Utilize the PMT callback API to add support for the BMG
devices.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
drivers/gpu/drm/xe/Makefile | 1 +
drivers/gpu/drm/xe/xe_device.c | 5 +
drivers/gpu/drm/xe/xe_device_types.h | 6 +
drivers/gpu/drm/xe/xe_vsec.c | 222 +++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vsec.h | 13 ++
5 files changed, 247 insertions(+)
create mode 100644 drivers/gpu/drm/xe/xe_vsec.c
create mode 100644 drivers/gpu/drm/xe/xe_vsec.h
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 1ff9602a52f6..a3c044b46fed 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -112,6 +112,7 @@ xe-y += xe_bb.o \
xe_vm.o \
xe_vram.o \
xe_vram_freq.o \
+ xe_vsec.o \
xe_wait_user_fence.o \
xe_wa.o \
xe_wopcm.o
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 1aba6f9eaa19..0bdfbe849e64 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -53,6 +53,7 @@
#include "xe_ttm_sys_mgr.h"
#include "xe_vm.h"
#include "xe_vram.h"
+#include "xe_vsec.h"
#include "xe_wait_user_fence.h"
#include "xe_wa.h"
@@ -370,6 +371,8 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
goto err;
}
+ drmm_mutex_init(&xe->drm, &xe->pmt.lock);
+
err = xe_display_create(xe);
if (WARN_ON(err))
goto err;
@@ -745,6 +748,8 @@ int xe_device_probe(struct xe_device *xe)
for_each_gt(gt, xe, id)
xe_gt_sanitize_freq(gt);
+ xe_vsec_init(xe);
+
return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
err_fini_display:
diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index 5b7292a9a66d..2509d7428f2d 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -458,6 +458,12 @@ struct xe_device {
struct mutex lock;
} d3cold;
+ /** @pmt: Support the PMT driver callback interface */
+ struct {
+ /** @pmt.lock: protect access for telemetry data */
+ struct mutex lock;
+ } pmt;
+
/**
* @pm_callback_task: Track the active task that is running in either
* the runtime_suspend or runtime_resume callbacks.
diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
new file mode 100644
index 000000000000..2c967aaa4072
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_vsec.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2022 - 2024 Intel Corporation
+ */
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/intel_vsec.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pci.h>
+
+#include "xe_device.h"
+#include "xe_device_types.h"
+#include "xe_drv.h"
+#include "xe_mmio.h"
+#include "xe_platform_types.h"
+#include "xe_pm.h"
+#include "xe_vsec.h"
+
+#define SOC_BASE 0x280000
+
+#define BMG_PMT_BASE 0xDB000
+#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
+
+#define BMG_TELEMETRY_BASE 0xE0000
+#define BMG_TELEMETRY_OFFSET (SOC_BASE + BMG_TELEMETRY_BASE)
+
+#define BMG_DEVICE_ID 0xE2F8
+
+#define GFX_BAR 0
+
+#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
+#define SG_REMAP_BITS GENMASK(31, 24)
+
+static struct intel_vsec_header bmg_telemetry = {
+ .length = 0x10,
+ .id = VSEC_ID_TELEMETRY,
+ .num_entries = 2,
+ .entry_size = 4,
+ .tbir = GFX_BAR,
+ .offset = BMG_DISCOVERY_OFFSET,
+};
+
+static struct intel_vsec_header *bmg_capabilities[] = {
+ &bmg_telemetry,
+ NULL
+};
+
+enum xe_vsec {
+ XE_VSEC_UNKNOWN = 0,
+ XE_VSEC_BMG,
+};
+
+static struct intel_vsec_platform_info xe_vsec_info[] = {
+ [XE_VSEC_BMG] = {
+ .caps = VSEC_CAP_TELEMETRY,
+ .headers = bmg_capabilities,
+ },
+ { }
+};
+
+/*
+ * The GUID will have the following bits to decode:
+ *
+ * X(4bits) - {Telemetry space iteration number (0,1,..)}
+ * X(4bits) - Segment (SEGMENT_INDEPENDENT-0, Client-1, Server-2)
+ * X(4bits) - SOC_SKU
+ * XXXX(16bits)– Device ID – changes for each down bin SKU’s
+ * X(2bits) - Capability Type (Crashlog-0, Telemetry Aggregator-1, Watcher-2)
+ * X(2bits) - Record-ID (0-PUNIT, 1-OOBMSM_0, 2-OOBMSM_1)
+ */
+#define GUID_TELEM_ITERATION GENMASK(3, 0)
+#define GUID_SEGMENT GENMASK(7, 4)
+#define GUID_SOC_SKU GENMASK(11, 8)
+#define GUID_DEVICE_ID GENMASK(27, 12)
+#define GUID_CAP_TYPE GENMASK(29, 28)
+#define GUID_RECORD_ID GENMASK(31, 30)
+
+#define PUNIT_TELEMETRY_OFFSET 0x0200
+#define PUNIT_WATCHER_OFFSET 0x14A0
+#define OOBMSM_0_WATCHER_OFFSET 0x18D8
+#define OOBMSM_1_TELEMETRY_OFFSET 0x1000
+
+enum record_id {
+ PUNIT,
+ OOBMSM_0,
+ OOBMSM_1
+};
+
+enum capability {
+ CRASHLOG,
+ TELEMETRY,
+ WATCHER
+};
+
+static int guid_decode(u32 guid, int *index, u32 *offset)
+{
+ u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
+ u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
+ u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
+
+ if (device_id != BMG_DEVICE_ID)
+ return -ENODEV;
+
+ if (record_id > OOBMSM_1 || cap_type > WATCHER)
+ return -EINVAL;
+
+ *offset = 0;
+
+ if (cap_type == CRASHLOG) {
+ *index = record_id == PUNIT ? 2 : 4;
+ return 0;
+ }
+
+ switch (record_id) {
+ case PUNIT:
+ *index = 0;
+ if (cap_type == TELEMETRY)
+ *offset = PUNIT_TELEMETRY_OFFSET;
+ else
+ *offset = PUNIT_WATCHER_OFFSET;
+ break;
+
+ case OOBMSM_0:
+ *index = 1;
+ if (cap_type == WATCHER)
+ *offset = OOBMSM_0_WATCHER_OFFSET;
+ break;
+
+ case OOBMSM_1:
+ *index = 1;
+ if (cap_type == TELEMETRY)
+ *offset = OOBMSM_1_TELEMETRY_OFFSET;
+ break;
+ }
+
+ return 0;
+}
+
+static int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, u32 count)
+{
+ struct xe_device *xe = pdev_to_xe_device(pdev);
+ void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
+ u32 mem_region;
+ u32 offset;
+ int ret;
+
+ ret = guid_decode(guid, &mem_region, &offset);
+ if (ret)
+ return ret;
+
+ telem_addr += offset;
+
+ mutex_lock(&xe->pmt.lock);
+
+ /* indicate that we are not at an appropriate power level */
+ ret = -ENODATA;
+ if (xe_pm_runtime_get_if_active(xe) > 0) {
+ /* set SoC re-mapper index register based on GUID memory region */
+ xe_mmio_rmw32(xe->tiles[0].primary_gt, SG_REMAP_INDEX1, SG_REMAP_BITS,
+ FIELD_PREP(SG_REMAP_BITS, mem_region));
+
+ memcpy_fromio(data, telem_addr, count);
+ ret = count;
+ xe_pm_runtime_put(xe);
+ }
+ mutex_unlock(&xe->pmt.lock);
+
+ return ret;
+}
+
+struct pmt_callbacks xe_pmt_cb = {
+ .read_telem = xe_pmt_telem_read,
+};
+
+static const int vsec_platforms[] = {
+ [XE_BATTLEMAGE] = XE_VSEC_BMG,
+};
+
+static enum xe_vsec get_platform_info(struct xe_device *xe)
+{
+ if (xe->info.platform > XE_BATTLEMAGE)
+ return XE_VSEC_UNKNOWN;
+
+ return vsec_platforms[xe->info.platform];
+}
+
+/**
+ * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
+ * interface
+ * @xe: valid xe instance
+ */
+void xe_vsec_init(struct xe_device *xe)
+{
+ struct intel_vsec_platform_info *info;
+ struct device *dev = xe->drm.dev;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ enum xe_vsec platform;
+
+ platform = get_platform_info(xe);
+ if (platform == XE_VSEC_UNKNOWN)
+ return;
+
+ info = &xe_vsec_info[platform];
+ if (!info->headers)
+ return;
+
+ switch (platform) {
+ case XE_VSEC_BMG:
+ info->priv_data = &xe_pmt_cb;
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * Register a VSEC. Cleanup is handled using device managed
+ * resources.
+ */
+ intel_vsec_register(pdev, info);
+}
+MODULE_IMPORT_NS(INTEL_VSEC);
diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h
new file mode 100644
index 000000000000..3fd29a21cad6
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_vsec.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright © 2022 - 2024 Intel Corporation
+ */
+
+#ifndef _XE_VSEC_H_
+#define _XE_VSEC_H_
+
+struct xe_device;
+
+void xe_vsec_init(struct xe_device *xe);
+
+#endif
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
` (3 preceding siblings ...)
2024-07-25 12:23 ` [PATCH v9 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-07-30 12:29 ` Ruhl, Michael J
2024-08-08 19:49 ` David E. Box
2024-07-25 12:23 ` [PATCH v9 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
2024-08-12 14:23 ` [PATCH v9 0/6] Support PMT features in Xe Hans de Goede
6 siblings, 2 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl
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(+)
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v9 6/6] drm/xe/vsec: Add support for DG2
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
` (4 preceding siblings ...)
2024-07-25 12:23 ` [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
@ 2024-07-25 12:23 ` Michael J. Ruhl
2024-08-12 14:23 ` [PATCH v9 0/6] Support PMT features in Xe Hans de Goede
6 siblings, 0 replies; 20+ messages in thread
From: Michael J. Ruhl @ 2024-07-25 12:23 UTC (permalink / raw)
To: intel-xe, platform-driver-x86, david.e.box, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: michael.j.ruhl, Rodrigo Vivi
PMT (DVSEC) offset information is based on the PCI BAR
for the telemetry (PCI) endpoint (also known as the P2SB).
However the DG2 endpoint is not completely functional,
and is disabled.
In order to allow access to the DG2 PMT features it is
necessary to inform the VSEC driver of the correct offset
via the base_adjust value.
This odjustment is the difference between the telemetry
offset (read from the PMT register) and the fixed offset in
the Xe SOC space. Calculate the offset, and pass it to the
VSEC driver on header registration.
Update xe_vsec.c to include DG2 header information.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
drivers/gpu/drm/xe/xe_vsec.c | 78 ++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
index 2c967aaa4072..5f96d4cdc841 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,46 @@ 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;
+
+ 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);
+
+ 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 +267,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 +278,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;
--
2.44.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* RE: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-25 12:23 ` [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
@ 2024-07-30 12:29 ` Ruhl, Michael J
2024-07-30 13:08 ` Ilpo Järvinen
2024-08-08 19:49 ` David E. Box
1 sibling, 1 reply; 20+ messages in thread
From: Ruhl, Michael J @ 2024-07-30 12:29 UTC (permalink / raw)
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,
andriy.shevchenko@linux.intel.com
>-----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
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-30 12:29 ` Ruhl, Michael J
@ 2024-07-30 13:08 ` Ilpo Järvinen
0 siblings, 0 replies; 20+ messages in thread
From: Ilpo Järvinen @ 2024-07-30 13:08 UTC (permalink / raw)
To: Ruhl, Michael J
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew, andriy.shevchenko@linux.intel.com
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.
--
i.
> >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
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 4/6] drm/xe/vsec: Support BMG devices
2024-07-25 12:23 ` [PATCH v9 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
@ 2024-08-07 19:23 ` David E. Box
2024-08-12 9:01 ` Ilpo Järvinen
1 sibling, 0 replies; 20+ messages in thread
From: David E. Box @ 2024-08-07 19:23 UTC (permalink / raw)
To: Michael J. Ruhl, intel-xe, platform-driver-x86, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
Cc: Rodrigo Vivi
Hi Mike,
Reviewed-by: David E. Box <david.e.box@linux.intel.com>
On Thu, 2024-07-25 at 08:23 -0400, Michael J. Ruhl wrote:
> Utilize the PMT callback API to add support for the BMG
> devices.
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 5 +
> drivers/gpu/drm/xe/xe_device_types.h | 6 +
> drivers/gpu/drm/xe/xe_vsec.c | 222 +++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_vsec.h | 13 ++
> 5 files changed, 247 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/xe_vsec.c
> create mode 100644 drivers/gpu/drm/xe/xe_vsec.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 1ff9602a52f6..a3c044b46fed 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -112,6 +112,7 @@ xe-y += xe_bb.o \
> xe_vm.o \
> xe_vram.o \
> xe_vram_freq.o \
> + xe_vsec.o \
> xe_wait_user_fence.o \
> xe_wa.o \
> xe_wopcm.o
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 1aba6f9eaa19..0bdfbe849e64 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -53,6 +53,7 @@
> #include "xe_ttm_sys_mgr.h"
> #include "xe_vm.h"
> #include "xe_vram.h"
> +#include "xe_vsec.h"
> #include "xe_wait_user_fence.h"
> #include "xe_wa.h"
>
> @@ -370,6 +371,8 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
> goto err;
> }
>
> + drmm_mutex_init(&xe->drm, &xe->pmt.lock);
> +
> err = xe_display_create(xe);
> if (WARN_ON(err))
> goto err;
> @@ -745,6 +748,8 @@ int xe_device_probe(struct xe_device *xe)
> for_each_gt(gt, xe, id)
> xe_gt_sanitize_freq(gt);
>
> + xe_vsec_init(xe);
> +
> return devm_add_action_or_reset(xe->drm.dev, xe_device_sanitize, xe);
>
> err_fini_display:
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> b/drivers/gpu/drm/xe/xe_device_types.h
> index 5b7292a9a66d..2509d7428f2d 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -458,6 +458,12 @@ struct xe_device {
> struct mutex lock;
> } d3cold;
>
> + /** @pmt: Support the PMT driver callback interface */
> + struct {
> + /** @pmt.lock: protect access for telemetry data */
> + struct mutex lock;
> + } pmt;
> +
> /**
> * @pm_callback_task: Track the active task that is running in either
> * the runtime_suspend or runtime_resume callbacks.
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> new file mode 100644
> index 000000000000..2c967aaa4072
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2022 - 2024 Intel Corporation
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/intel_vsec.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +
> +#include "xe_device.h"
> +#include "xe_device_types.h"
> +#include "xe_drv.h"
> +#include "xe_mmio.h"
> +#include "xe_platform_types.h"
> +#include "xe_pm.h"
> +#include "xe_vsec.h"
> +
> +#define SOC_BASE 0x280000
> +
> +#define BMG_PMT_BASE 0xDB000
> +#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
> +
> +#define BMG_TELEMETRY_BASE 0xE0000
> +#define BMG_TELEMETRY_OFFSET (SOC_BASE + BMG_TELEMETRY_BASE)
> +
> +#define BMG_DEVICE_ID 0xE2F8
> +
> +#define GFX_BAR 0
> +
> +#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
> +#define SG_REMAP_BITS GENMASK(31, 24)
> +
> +static struct intel_vsec_header bmg_telemetry = {
> + .length = 0x10,
> + .id = VSEC_ID_TELEMETRY,
> + .num_entries = 2,
> + .entry_size = 4,
> + .tbir = GFX_BAR,
> + .offset = BMG_DISCOVERY_OFFSET,
> +};
> +
> +static struct intel_vsec_header *bmg_capabilities[] = {
> + &bmg_telemetry,
> + NULL
> +};
> +
> +enum xe_vsec {
> + XE_VSEC_UNKNOWN = 0,
> + XE_VSEC_BMG,
> +};
> +
> +static struct intel_vsec_platform_info xe_vsec_info[] = {
> + [XE_VSEC_BMG] = {
> + .caps = VSEC_CAP_TELEMETRY,
> + .headers = bmg_capabilities,
> + },
> + { }
> +};
> +
> +/*
> + * The GUID will have the following bits to decode:
> + *
> + * X(4bits) - {Telemetry space iteration number (0,1,..)}
> + * X(4bits) - Segment (SEGMENT_INDEPENDENT-0, Client-1, Server-2)
> + * X(4bits) - SOC_SKU
> + * XXXX(16bits)– Device ID – changes for each down bin SKU’s
> + * X(2bits) - Capability Type (Crashlog-0, Telemetry Aggregator-1, Watcher-2)
> + * X(2bits) - Record-ID (0-PUNIT, 1-OOBMSM_0, 2-OOBMSM_1)
> + */
> +#define GUID_TELEM_ITERATION GENMASK(3, 0)
> +#define GUID_SEGMENT GENMASK(7, 4)
> +#define GUID_SOC_SKU GENMASK(11, 8)
> +#define GUID_DEVICE_ID GENMASK(27, 12)
> +#define GUID_CAP_TYPE GENMASK(29, 28)
> +#define GUID_RECORD_ID GENMASK(31, 30)
> +
> +#define PUNIT_TELEMETRY_OFFSET 0x0200
> +#define PUNIT_WATCHER_OFFSET 0x14A0
> +#define OOBMSM_0_WATCHER_OFFSET 0x18D8
> +#define OOBMSM_1_TELEMETRY_OFFSET 0x1000
> +
> +enum record_id {
> + PUNIT,
> + OOBMSM_0,
> + OOBMSM_1
> +};
> +
> +enum capability {
> + CRASHLOG,
> + TELEMETRY,
> + WATCHER
> +};
> +
> +static int guid_decode(u32 guid, int *index, u32 *offset)
> +{
> + u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
> + u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> + u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
> +
> + if (device_id != BMG_DEVICE_ID)
> + return -ENODEV;
> +
> + if (record_id > OOBMSM_1 || cap_type > WATCHER)
> + return -EINVAL;
> +
> + *offset = 0;
> +
> + if (cap_type == CRASHLOG) {
> + *index = record_id == PUNIT ? 2 : 4;
> + return 0;
> + }
> +
> + switch (record_id) {
> + case PUNIT:
> + *index = 0;
> + if (cap_type == TELEMETRY)
> + *offset = PUNIT_TELEMETRY_OFFSET;
> + else
> + *offset = PUNIT_WATCHER_OFFSET;
> + break;
> +
> + case OOBMSM_0:
> + *index = 1;
> + if (cap_type == WATCHER)
> + *offset = OOBMSM_0_WATCHER_OFFSET;
> + break;
> +
> + case OOBMSM_1:
> + *index = 1;
> + if (cap_type == TELEMETRY)
> + *offset = OOBMSM_1_TELEMETRY_OFFSET;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, u32
> count)
> +{
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> + void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
> + u32 mem_region;
> + u32 offset;
> + int ret;
> +
> + ret = guid_decode(guid, &mem_region, &offset);
> + if (ret)
> + return ret;
> +
> + telem_addr += offset;
> +
> + mutex_lock(&xe->pmt.lock);
> +
> + /* indicate that we are not at an appropriate power level */
> + ret = -ENODATA;
> + if (xe_pm_runtime_get_if_active(xe) > 0) {
> + /* set SoC re-mapper index register based on GUID memory
> region */
> + xe_mmio_rmw32(xe->tiles[0].primary_gt, SG_REMAP_INDEX1,
> SG_REMAP_BITS,
> + FIELD_PREP(SG_REMAP_BITS, mem_region));
> +
> + memcpy_fromio(data, telem_addr, count);
> + ret = count;
> + xe_pm_runtime_put(xe);
> + }
> + mutex_unlock(&xe->pmt.lock);
> +
> + return ret;
> +}
> +
> +struct pmt_callbacks xe_pmt_cb = {
> + .read_telem = xe_pmt_telem_read,
> +};
> +
> +static const int vsec_platforms[] = {
> + [XE_BATTLEMAGE] = XE_VSEC_BMG,
> +};
> +
> +static enum xe_vsec get_platform_info(struct xe_device *xe)
> +{
> + if (xe->info.platform > XE_BATTLEMAGE)
> + return XE_VSEC_UNKNOWN;
> +
> + return vsec_platforms[xe->info.platform];
> +}
> +
> +/**
> + * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
> + * interface
> + * @xe: valid xe instance
> + */
> +void xe_vsec_init(struct xe_device *xe)
> +{
> + struct intel_vsec_platform_info *info;
> + struct device *dev = xe->drm.dev;
> + struct pci_dev *pdev = to_pci_dev(dev);
> + enum xe_vsec platform;
> +
> + platform = get_platform_info(xe);
> + if (platform == XE_VSEC_UNKNOWN)
> + return;
> +
> + info = &xe_vsec_info[platform];
> + if (!info->headers)
> + return;
> +
> + switch (platform) {
> + case XE_VSEC_BMG:
> + info->priv_data = &xe_pmt_cb;
> + break;
> + default:
> + break;
> + }
> +
> + /*
> + * Register a VSEC. Cleanup is handled using device managed
> + * resources.
> + */
> + intel_vsec_register(pdev, info);
> +}
> +MODULE_IMPORT_NS(INTEL_VSEC);
> diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h
> new file mode 100644
> index 000000000000..3fd29a21cad6
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_vsec.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright © 2022 - 2024 Intel Corporation
> + */
> +
> +#ifndef _XE_VSEC_H_
> +#define _XE_VSEC_H_
> +
> +struct xe_device;
> +
> +void xe_vsec_init(struct xe_device *xe);
> +
> +#endif
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-07-25 12:23 ` [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
2024-07-30 12:29 ` Ruhl, Michael J
@ 2024-08-08 19:49 ` David E. Box
2024-08-08 21:01 ` Rodrigo Vivi
1 sibling, 1 reply; 20+ messages in thread
From: David E. Box @ 2024-08-08 19:49 UTC (permalink / raw)
To: Michael J. Ruhl, intel-xe, platform-driver-x86, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
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,
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-08-08 19:49 ` David E. Box
@ 2024-08-08 21:01 ` Rodrigo Vivi
2024-08-09 0:57 ` David E. Box
0 siblings, 1 reply; 20+ messages in thread
From: Rodrigo Vivi @ 2024-08-08 21:01 UTC (permalink / raw)
To: David E. Box
Cc: Michael J. Ruhl, intel-xe, platform-driver-x86, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
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,
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-08-08 21:01 ` Rodrigo Vivi
@ 2024-08-09 0:57 ` David E. Box
2024-08-09 18:21 ` Ruhl, Michael J
0 siblings, 1 reply; 20+ messages in thread
From: David E. Box @ 2024-08-09 0:57 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: Michael J. Ruhl, intel-xe, platform-driver-x86, ilpo.jarvinen,
matthew.brost, andriy.shevchenko
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,
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-08-09 0:57 ` David E. Box
@ 2024-08-09 18:21 ` Ruhl, Michael J
2024-08-12 9:22 ` Ilpo Järvinen
0 siblings, 1 reply; 20+ messages in thread
From: Ruhl, Michael J @ 2024-08-09 18:21 UTC (permalink / raw)
To: david.e.box@linux.intel.com, Vivi, Rodrigo
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
ilpo.jarvinen@linux.intel.com, Brost, Matthew,
andriy.shevchenko@linux.intel.com
> -----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,
> > >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 4/6] drm/xe/vsec: Support BMG devices
2024-07-25 12:23 ` [PATCH v9 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
2024-08-07 19:23 ` David E. Box
@ 2024-08-12 9:01 ` Ilpo Järvinen
2024-08-12 17:14 ` Ruhl, Michael J
1 sibling, 1 reply; 20+ messages in thread
From: Ilpo Järvinen @ 2024-08-12 9:01 UTC (permalink / raw)
To: Michael J. Ruhl
Cc: intel-xe, platform-driver-x86, david.e.box, matthew.brost,
Andy Shevchenko, Rodrigo Vivi
[-- Attachment #1: Type: text/plain, Size: 5801 bytes --]
On Thu, 25 Jul 2024, Michael J. Ruhl wrote:
> Utilize the PMT callback API to add support for the BMG
> devices.
The shortlog and commit message are a bit terse on details what this
change is about, it's all hidden into the acronyms :-).
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> new file mode 100644
> index 000000000000..2c967aaa4072
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2022 - 2024 Intel Corporation
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/intel_vsec.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +
> +#include "xe_device.h"
> +#include "xe_device_types.h"
> +#include "xe_drv.h"
> +#include "xe_mmio.h"
> +#include "xe_platform_types.h"
> +#include "xe_pm.h"
> +#include "xe_vsec.h"
> +
> +#define SOC_BASE 0x280000
> +
> +#define BMG_PMT_BASE 0xDB000
> +#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
> +
> +#define BMG_TELEMETRY_BASE 0xE0000
> +#define BMG_TELEMETRY_OFFSET (SOC_BASE + BMG_TELEMETRY_BASE)
> +
> +#define BMG_DEVICE_ID 0xE2F8
> +
> +#define GFX_BAR 0
> +
> +#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
> +#define SG_REMAP_BITS GENMASK(31, 24)
> +
> +static struct intel_vsec_header bmg_telemetry = {
> + .length = 0x10,
> + .id = VSEC_ID_TELEMETRY,
> + .num_entries = 2,
> + .entry_size = 4,
> + .tbir = GFX_BAR,
> + .offset = BMG_DISCOVERY_OFFSET,
> +};
> +
> +static struct intel_vsec_header *bmg_capabilities[] = {
> + &bmg_telemetry,
> + NULL
> +};
> +
> +enum xe_vsec {
> + XE_VSEC_UNKNOWN = 0,
> + XE_VSEC_BMG,
> +};
> +
> +static struct intel_vsec_platform_info xe_vsec_info[] = {
> + [XE_VSEC_BMG] = {
> + .caps = VSEC_CAP_TELEMETRY,
> + .headers = bmg_capabilities,
> + },
> + { }
> +};
> +
> +/*
> + * The GUID will have the following bits to decode:
> + *
> + * X(4bits) - {Telemetry space iteration number (0,1,..)}
> + * X(4bits) - Segment (SEGMENT_INDEPENDENT-0, Client-1, Server-2)
> + * X(4bits) - SOC_SKU
> + * XXXX(16bits)– Device ID – changes for each down bin SKU’s
> + * X(2bits) - Capability Type (Crashlog-0, Telemetry Aggregator-1, Watcher-2)
> + * X(2bits) - Record-ID (0-PUNIT, 1-OOBMSM_0, 2-OOBMSM_1)
> + */
> +#define GUID_TELEM_ITERATION GENMASK(3, 0)
> +#define GUID_SEGMENT GENMASK(7, 4)
> +#define GUID_SOC_SKU GENMASK(11, 8)
> +#define GUID_DEVICE_ID GENMASK(27, 12)
> +#define GUID_CAP_TYPE GENMASK(29, 28)
> +#define GUID_RECORD_ID GENMASK(31, 30)
> +
> +#define PUNIT_TELEMETRY_OFFSET 0x0200
> +#define PUNIT_WATCHER_OFFSET 0x14A0
> +#define OOBMSM_0_WATCHER_OFFSET 0x18D8
> +#define OOBMSM_1_TELEMETRY_OFFSET 0x1000
> +
> +enum record_id {
> + PUNIT,
> + OOBMSM_0,
> + OOBMSM_1
> +};
> +
> +enum capability {
> + CRASHLOG,
> + TELEMETRY,
> + WATCHER
> +};
> +
> +static int guid_decode(u32 guid, int *index, u32 *offset)
> +{
> + u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
> + u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> + u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
> +
> + if (device_id != BMG_DEVICE_ID)
> + return -ENODEV;
> +
> + if (record_id > OOBMSM_1 || cap_type > WATCHER)
> + return -EINVAL;
> +
> + *offset = 0;
> +
> + if (cap_type == CRASHLOG) {
> + *index = record_id == PUNIT ? 2 : 4;
> + return 0;
> + }
> +
> + switch (record_id) {
> + case PUNIT:
> + *index = 0;
> + if (cap_type == TELEMETRY)
> + *offset = PUNIT_TELEMETRY_OFFSET;
> + else
> + *offset = PUNIT_WATCHER_OFFSET;
> + break;
> +
> + case OOBMSM_0:
> + *index = 1;
> + if (cap_type == WATCHER)
> + *offset = OOBMSM_0_WATCHER_OFFSET;
> + break;
> +
> + case OOBMSM_1:
> + *index = 1;
> + if (cap_type == TELEMETRY)
> + *offset = OOBMSM_1_TELEMETRY_OFFSET;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64 *data, u32 count)
> +{
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> + void __iomem *telem_addr = xe->mmio.regs + BMG_TELEMETRY_OFFSET;
> + u32 mem_region;
> + u32 offset;
> + int ret;
> +
> + ret = guid_decode(guid, &mem_region, &offset);
> + if (ret)
> + return ret;
> +
> + telem_addr += offset;
> +
> + mutex_lock(&xe->pmt.lock);
> +
> + /* indicate that we are not at an appropriate power level */
> + ret = -ENODATA;
> + if (xe_pm_runtime_get_if_active(xe) > 0) {
xe_pm_runtime_get_if_active() returns bool so > 0 looks odd. In fact,
active > 0 compare is already done by that called function so perhaps you
mixed up what kind of value is returned by xe_pm_runtime_get_if_active().
Also, I'd restructure this logic with guard & use of reverse logic.
guard(mutex)(&xe->pmt.lock);
/* indicate that we are not at an appropriate power level */
if (!xe_pm_runtime_get_if_active(xe))
return -ENODATA;
... the rest of the code de-indented by one level (minus the
mutex_unlock() which is no longer needed because guard() is used).
With those fixed, I think this one is ready to go so after fixing, please
add:
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> + /* set SoC re-mapper index register based on GUID memory region */
> + xe_mmio_rmw32(xe->tiles[0].primary_gt, SG_REMAP_INDEX1, SG_REMAP_BITS,
> + FIELD_PREP(SG_REMAP_BITS, mem_region));
> +
> + memcpy_fromio(data, telem_addr, count);
> + ret = count;
> + xe_pm_runtime_put(xe);
> + }
> + mutex_unlock(&xe->pmt.lock);
> +
> + return ret;
> +}
--
i.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-08-09 18:21 ` Ruhl, Michael J
@ 2024-08-12 9:22 ` Ilpo Järvinen
2024-08-13 16:59 ` Rodrigo Vivi
0 siblings, 1 reply; 20+ messages in thread
From: Ilpo Järvinen @ 2024-08-12 9:22 UTC (permalink / raw)
To: Ruhl, Michael J, Hans de Goede
Cc: david.e.box@linux.intel.com, Vivi, Rodrigo,
intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, Brost, Matthew,
andriy.shevchenko@linux.intel.com
[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]
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).
--
i.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 0/6] Support PMT features in Xe
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
` (5 preceding siblings ...)
2024-07-25 12:23 ` [PATCH v9 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
@ 2024-08-12 14:23 ` Hans de Goede
2024-08-12 18:13 ` Ruhl, Michael J
6 siblings, 1 reply; 20+ messages in thread
From: Hans de Goede @ 2024-08-12 14:23 UTC (permalink / raw)
To: Michael J. Ruhl, intel-xe, platform-driver-x86, david.e.box,
ilpo.jarvinen, matthew.brost, andriy.shevchenko
Hi,
On 7/25/24 2:23 PM, Michael J. Ruhl wrote:
> DG2 and Battlemage have the Intel Platform Monitoring Technology (PMT)
> feature available, but not in the "standard" (pci endpoint) way.
>
> Add support to the vsec and Xe drivers to allow access to the PMT space
> for the DG2 and BMG devices.
>
> The intel_vsec_register() function allows drivers to provide telemetry
> header information (usually found at probe time), to allow the PMT
> driver to probe the telemetry features.
>
> Battlemage has a shared memory area (selected by index), so a callback
> function is required to access the appropriate PMT data.
Thank you for your patch-series, I've applied patches 1-3 to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
For patches 4 - 6 please address the review remarks and post
a v10 based on top of my review-hans branch.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
>
> V2:
> Re-worked DG2 support patches using a base_adjust rather than a
> quirk.
> Updated GUID decode, for correct decode.
> v3:
> Fixed a documentation issue for the pmt struct.
> v4:
> Fixed a documentation issue in the xe_vsec.c module
> v5:
> Addressed review comments for patch 4 (Xe driver)
> Add r/b for the first three patches
> v6:
> Added kernel doc to moved data structure
> Added required include files
> Correct usage for FIELD_PREP()/FIELD_GET()
> Whitespace clean up
> Removed unnecessary type cast
> v7:
> Commit message updates
> v8:
> Added some r/b (patch 2 and 3).
> Updated kernel doc patch 2 (priv_data) patch 5 (base_adjust)
> v9:
> Add r/b for the Xe driver patches
>
> David E. Box (3):
> platform/x86/intel/vsec.h: Move to include/linux
> platform/x86/intel/vsec: Add PMT read callbacks
> platform/x86/intel/pmt: Use PMT callbacks
>
> Michael J. Ruhl (3):
> drm/xe/vsec: Support BMG devices
> platform/x86/intel/pmt: Add support for PMT base adjust
> drm/xe/vsec: Add support for DG2
>
> MAINTAINERS | 3 +-
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device.c | 5 +
> drivers/gpu/drm/xe/xe_device_types.h | 6 +
> drivers/gpu/drm/xe/xe_vsec.c | 300 ++++++++++++++++++
> drivers/gpu/drm/xe/xe_vsec.h | 13 +
> drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
> drivers/platform/x86/intel/pmt/class.c | 28 +-
> drivers/platform/x86/intel/pmt/class.h | 11 +-
> drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
> drivers/platform/x86/intel/pmt/telemetry.c | 21 +-
> drivers/platform/x86/intel/sdsi.c | 3 +-
> drivers/platform/x86/intel/tpmi.c | 3 +-
> drivers/platform/x86/intel/vsec.c | 9 +-
> .../vsec.h => include/linux/intel_vsec.h | 50 ++-
> 15 files changed, 428 insertions(+), 29 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/xe_vsec.c
> create mode 100644 drivers/gpu/drm/xe/xe_vsec.h
> rename drivers/platform/x86/intel/vsec.h => include/linux/intel_vsec.h (61%)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 4/6] drm/xe/vsec: Support BMG devices
2024-08-12 9:01 ` Ilpo Järvinen
@ 2024-08-12 17:14 ` Ruhl, Michael J
0 siblings, 0 replies; 20+ messages in thread
From: Ruhl, Michael J @ 2024-08-12 17:14 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
Brost, Matthew, Andy Shevchenko, Vivi, Rodrigo
> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Monday, August 12, 2024 5:01 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>;
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Vivi, Rodrigo
> <rodrigo.vivi@intel.com>
> Subject: Re: [PATCH v9 4/6] drm/xe/vsec: Support BMG devices
>
> On Thu, 25 Jul 2024, Michael J. Ruhl wrote:
>
> > Utilize the PMT callback API to add support for the BMG devices.
>
> The shortlog and commit message are a bit terse on details what this change is
> about, it's all hidden into the acronyms :-).
>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> > ---
>
> > diff --git a/drivers/gpu/drm/xe/xe_vsec.c
> > b/drivers/gpu/drm/xe/xe_vsec.c new file mode 100644 index
> > 000000000000..2c967aaa4072
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_vsec.c
> > @@ -0,0 +1,222 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright © 2022 - 2024 Intel Corporation */ #include
> > +<linux/bitfield.h> #include <linux/bits.h> #include
> > +<linux/intel_vsec.h> #include <linux/module.h> #include
> > +<linux/mutex.h> #include <linux/pci.h>
> > +
> > +#include "xe_device.h"
> > +#include "xe_device_types.h"
> > +#include "xe_drv.h"
> > +#include "xe_mmio.h"
> > +#include "xe_platform_types.h"
> > +#include "xe_pm.h"
> > +#include "xe_vsec.h"
> > +
> > +#define SOC_BASE 0x280000
> > +
> > +#define BMG_PMT_BASE 0xDB000
> > +#define BMG_DISCOVERY_OFFSET (SOC_BASE + BMG_PMT_BASE)
> > +
> > +#define BMG_TELEMETRY_BASE 0xE0000
> > +#define BMG_TELEMETRY_OFFSET (SOC_BASE + BMG_TELEMETRY_BASE)
> > +
> > +#define BMG_DEVICE_ID 0xE2F8
> > +
> > +#define GFX_BAR 0
> > +
> > +#define SG_REMAP_INDEX1 XE_REG(SOC_BASE + 0x08)
> > +#define SG_REMAP_BITS GENMASK(31, 24)
> > +
> > +static struct intel_vsec_header bmg_telemetry = {
> > + .length = 0x10,
> > + .id = VSEC_ID_TELEMETRY,
> > + .num_entries = 2,
> > + .entry_size = 4,
> > + .tbir = GFX_BAR,
> > + .offset = BMG_DISCOVERY_OFFSET,
> > +};
> > +
> > +static struct intel_vsec_header *bmg_capabilities[] = {
> > + &bmg_telemetry,
> > + NULL
> > +};
> > +
> > +enum xe_vsec {
> > + XE_VSEC_UNKNOWN = 0,
> > + XE_VSEC_BMG,
> > +};
> > +
> > +static struct intel_vsec_platform_info xe_vsec_info[] = {
> > + [XE_VSEC_BMG] = {
> > + .caps = VSEC_CAP_TELEMETRY,
> > + .headers = bmg_capabilities,
> > + },
> > + { }
> > +};
> > +
> > +/*
> > + * The GUID will have the following bits to decode:
> > + *
> > + * X(4bits) - {Telemetry space iteration number (0,1,..)}
> > + * X(4bits) - Segment (SEGMENT_INDEPENDENT-0, Client-1, Server-2)
> > + * X(4bits) - SOC_SKU
> > + * XXXX(16bits)– Device ID – changes for each down bin SKU’s
> > + * X(2bits) - Capability Type (Crashlog-0, Telemetry Aggregator-1,
> > +Watcher-2)
> > + * X(2bits) - Record-ID (0-PUNIT, 1-OOBMSM_0, 2-OOBMSM_1) */
> > +#define GUID_TELEM_ITERATION GENMASK(3, 0)
> > +#define GUID_SEGMENT GENMASK(7, 4)
> > +#define GUID_SOC_SKU GENMASK(11, 8)
> > +#define GUID_DEVICE_ID GENMASK(27, 12)
> > +#define GUID_CAP_TYPE GENMASK(29, 28)
> > +#define GUID_RECORD_ID GENMASK(31, 30)
> > +
> > +#define PUNIT_TELEMETRY_OFFSET 0x0200
> > +#define PUNIT_WATCHER_OFFSET 0x14A0
> > +#define OOBMSM_0_WATCHER_OFFSET 0x18D8
> > +#define OOBMSM_1_TELEMETRY_OFFSET 0x1000
> > +
> > +enum record_id {
> > + PUNIT,
> > + OOBMSM_0,
> > + OOBMSM_1
> > +};
> > +
> > +enum capability {
> > + CRASHLOG,
> > + TELEMETRY,
> > + WATCHER
> > +};
> > +
> > +static int guid_decode(u32 guid, int *index, u32 *offset) {
> > + u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
> > + u32 cap_type = FIELD_GET(GUID_CAP_TYPE, guid);
> > + u32 device_id = FIELD_GET(GUID_DEVICE_ID, guid);
> > +
> > + if (device_id != BMG_DEVICE_ID)
> > + return -ENODEV;
> > +
> > + if (record_id > OOBMSM_1 || cap_type > WATCHER)
> > + return -EINVAL;
> > +
> > + *offset = 0;
> > +
> > + if (cap_type == CRASHLOG) {
> > + *index = record_id == PUNIT ? 2 : 4;
> > + return 0;
> > + }
> > +
> > + switch (record_id) {
> > + case PUNIT:
> > + *index = 0;
> > + if (cap_type == TELEMETRY)
> > + *offset = PUNIT_TELEMETRY_OFFSET;
> > + else
> > + *offset = PUNIT_WATCHER_OFFSET;
> > + break;
> > +
> > + case OOBMSM_0:
> > + *index = 1;
> > + if (cap_type == WATCHER)
> > + *offset = OOBMSM_0_WATCHER_OFFSET;
> > + break;
> > +
> > + case OOBMSM_1:
> > + *index = 1;
> > + if (cap_type == TELEMETRY)
> > + *offset = OOBMSM_1_TELEMETRY_OFFSET;
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int xe_pmt_telem_read(struct pci_dev *pdev, u32 guid, u64
> > +*data, u32 count) {
> > + struct xe_device *xe = pdev_to_xe_device(pdev);
> > + void __iomem *telem_addr = xe->mmio.regs +
> BMG_TELEMETRY_OFFSET;
> > + u32 mem_region;
> > + u32 offset;
> > + int ret;
> > +
> > + ret = guid_decode(guid, &mem_region, &offset);
> > + if (ret)
> > + return ret;
> > +
> > + telem_addr += offset;
> > +
> > + mutex_lock(&xe->pmt.lock);
> > +
> > + /* indicate that we are not at an appropriate power level */
> > + ret = -ENODATA;
> > + if (xe_pm_runtime_get_if_active(xe) > 0) {
>
> xe_pm_runtime_get_if_active() returns bool so > 0 looks odd. In fact, active >
> 0 compare is already done by that called function so perhaps you mixed up
> what kind of value is returned by xe_pm_runtime_get_if_active().
Hi Ilpo,
Yup, the underlying pm_runtime_xxx call can return an error...and I missed the bool
conversion. I will update.
> Also, I'd restructure this logic with guard & use of reverse logic.
>
> guard(mutex)(&xe->pmt.lock);
>
> /* indicate that we are not at an appropriate power level */
> if (!xe_pm_runtime_get_if_active(xe))
> return -ENODATA;
>
> ... the rest of the code de-indented by one level (minus the
> mutex_unlock() which is no longer needed because guard() is used).
>
> With those fixed, I think this one is ready to go so after fixing, please
> add:
>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
The guard is new to me. I will get this updated.
Thank you!
M
>
> > + /* set SoC re-mapper index register based on GUID memory
> region */
> > + xe_mmio_rmw32(xe->tiles[0].primary_gt,
> SG_REMAP_INDEX1, SG_REMAP_BITS,
> > + FIELD_PREP(SG_REMAP_BITS, mem_region));
> > +
> > + memcpy_fromio(data, telem_addr, count);
> > + ret = count;
> > + xe_pm_runtime_put(xe);
> > + }
> > + mutex_unlock(&xe->pmt.lock);
> > +
> > + return ret;
> > +}
>
> --
> i.
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH v9 0/6] Support PMT features in Xe
2024-08-12 14:23 ` [PATCH v9 0/6] Support PMT features in Xe Hans de Goede
@ 2024-08-12 18:13 ` Ruhl, Michael J
0 siblings, 0 replies; 20+ messages in thread
From: Ruhl, Michael J @ 2024-08-12 18:13 UTC (permalink / raw)
To: Hans de Goede, intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, david.e.box@linux.intel.com,
ilpo.jarvinen@linux.intel.com, Brost, Matthew,
andriy.shevchenko@linux.intel.com
> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, August 12, 2024 10:23 AM
> To: Ruhl, Michael J <michael.j.ruhl@intel.com>; 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
> Subject: Re: [PATCH v9 0/6] Support PMT features in Xe
>
> Hi,
>
> On 7/25/24 2:23 PM, Michael J. Ruhl wrote:
> > DG2 and Battlemage have the Intel Platform Monitoring Technology (PMT)
> > feature available, but not in the "standard" (pci endpoint) way.
> >
> > Add support to the vsec and Xe drivers to allow access to the PMT
> > space for the DG2 and BMG devices.
> >
> > The intel_vsec_register() function allows drivers to provide telemetry
> > header information (usually found at probe time), to allow the PMT
> > driver to probe the telemetry features.
> >
> > Battlemage has a shared memory area (selected by index), so a callback
> > function is required to access the appropriate PMT data.
>
> Thank you for your patch-series, I've applied patches 1-3 to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-
> x86.git/log/?h=review-hans
>
> For patches 4 - 6 please address the review remarks and post a v10 based on
> top of my review-hans branch.
>
> Once I've run some tests on this branch the patches there will be added to the
> platform-drivers-x86/for-next branch and eventually will be included in the
> pdx86 pull-request to Linus for the next merge-window.
Hi Hans,
I just post V10 of patch 4 (from the above branch "review-hans"
5 and 6 need some additional work/testing that I need to defer for now.
The commit message looks different in the posted email, than it does in my tree.
Not sure what happened to the <cr>s...
Please let me know if I missed anything.
Thanks!
M
> Regards,
>
> Hans
>
>
>
>
>
> >
> > V2:
> > Re-worked DG2 support patches using a base_adjust rather than a
> > quirk.
> > Updated GUID decode, for correct decode.
> > v3:
> > Fixed a documentation issue for the pmt struct.
> > v4:
> > Fixed a documentation issue in the xe_vsec.c module
> > v5:
> > Addressed review comments for patch 4 (Xe driver)
> > Add r/b for the first three patches
> > v6:
> > Added kernel doc to moved data structure
> > Added required include files
> > Correct usage for FIELD_PREP()/FIELD_GET()
> > Whitespace clean up
> > Removed unnecessary type cast
> > v7:
> > Commit message updates
> > v8:
> > Added some r/b (patch 2 and 3).
> > Updated kernel doc patch 2 (priv_data) patch 5 (base_adjust)
> > v9:
> > Add r/b for the Xe driver patches
> >
> > David E. Box (3):
> > platform/x86/intel/vsec.h: Move to include/linux
> > platform/x86/intel/vsec: Add PMT read callbacks
> > platform/x86/intel/pmt: Use PMT callbacks
> >
> > Michael J. Ruhl (3):
> > drm/xe/vsec: Support BMG devices
> > platform/x86/intel/pmt: Add support for PMT base adjust
> > drm/xe/vsec: Add support for DG2
> >
> > MAINTAINERS | 3 +-
> > drivers/gpu/drm/xe/Makefile | 1 +
> > drivers/gpu/drm/xe/xe_device.c | 5 +
> > drivers/gpu/drm/xe/xe_device_types.h | 6 +
> > drivers/gpu/drm/xe/xe_vsec.c | 300 ++++++++++++++++++
> > drivers/gpu/drm/xe/xe_vsec.h | 13 +
> > drivers/platform/x86/intel/pmc/core_ssram.c | 2 +-
> > drivers/platform/x86/intel/pmt/class.c | 28 +-
> > drivers/platform/x86/intel/pmt/class.h | 11 +-
> > drivers/platform/x86/intel/pmt/crashlog.c | 2 +-
> > drivers/platform/x86/intel/pmt/telemetry.c | 21 +-
> > drivers/platform/x86/intel/sdsi.c | 3 +-
> > drivers/platform/x86/intel/tpmi.c | 3 +-
> > drivers/platform/x86/intel/vsec.c | 9 +-
> > .../vsec.h => include/linux/intel_vsec.h | 50 ++-
> > 15 files changed, 428 insertions(+), 29 deletions(-) create mode
> > 100644 drivers/gpu/drm/xe/xe_vsec.c create mode 100644
> > drivers/gpu/drm/xe/xe_vsec.h rename drivers/platform/x86/intel/vsec.h
> > => include/linux/intel_vsec.h (61%)
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust
2024-08-12 9:22 ` Ilpo Järvinen
@ 2024-08-13 16:59 ` Rodrigo Vivi
0 siblings, 0 replies; 20+ messages in thread
From: Rodrigo Vivi @ 2024-08-13 16:59 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Ruhl, Michael J, Hans de Goede, david.e.box@linux.intel.com,
intel-xe@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org, Brost, Matthew,
andriy.shevchenko@linux.intel.com
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2024-08-13 16:59 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-25 12:23 [PATCH v9 0/6] Support PMT features in Xe Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 1/6] platform/x86/intel/vsec.h: Move to include/linux Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 2/6] platform/x86/intel/vsec: Add PMT read callbacks Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 3/6] platform/x86/intel/pmt: Use PMT callbacks Michael J. Ruhl
2024-07-25 12:23 ` [PATCH v9 4/6] drm/xe/vsec: Support BMG devices Michael J. Ruhl
2024-08-07 19:23 ` David E. Box
2024-08-12 9:01 ` Ilpo Järvinen
2024-08-12 17:14 ` Ruhl, Michael J
2024-07-25 12:23 ` [PATCH v9 5/6] platform/x86/intel/pmt: Add support for PMT base adjust Michael J. Ruhl
2024-07-30 12:29 ` Ruhl, Michael J
2024-07-30 13:08 ` Ilpo Järvinen
2024-08-08 19:49 ` David E. Box
2024-08-08 21:01 ` Rodrigo Vivi
2024-08-09 0:57 ` David E. Box
2024-08-09 18:21 ` Ruhl, Michael J
2024-08-12 9:22 ` Ilpo Järvinen
2024-08-13 16:59 ` Rodrigo Vivi
2024-07-25 12:23 ` [PATCH v9 6/6] drm/xe/vsec: Add support for DG2 Michael J. Ruhl
2024-08-12 14:23 ` [PATCH v9 0/6] Support PMT features in Xe Hans de Goede
2024-08-12 18:13 ` Ruhl, Michael J
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox