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