diff mbox series

[v10,06/10] drm/i915/nvm: add nvm device for discrete graphics

Message ID 20250515133345.2805031-7-alexander.usyskin@intel.com
State New
Headers show
Series mtd: add driver for Intel discrete graphics | expand

Commit Message

Alexander Usyskin May 15, 2025, 1:33 p.m. UTC 
Enable access to internal non-volatile memory on
DGFX devices via a child device.
The nvm child device is exposed via auxiliary bus.

CC: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Co-developed-by: Tomas Winkler <tomasw@gmail.com>
Signed-off-by: Tomas Winkler <tomasw@gmail.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/gpu/drm/i915/Makefile      |  4 ++
 drivers/gpu/drm/i915/i915_driver.c |  6 ++
 drivers/gpu/drm/i915/i915_drv.h    |  3 +
 drivers/gpu/drm/i915/i915_reg.h    |  1 +
 drivers/gpu/drm/i915/intel_nvm.c   | 92 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_nvm.h   | 15 +++++
 6 files changed, 121 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/intel_nvm.c
 create mode 100644 drivers/gpu/drm/i915/intel_nvm.h

Comments

Raag Jadav May 24, 2025, 10:20 a.m. UTC | #1 
On Thu, May 15, 2025 at 04:33:41PM +0300, Alexander Usyskin wrote:
> Enable access to internal non-volatile memory on
> DGFX devices via a child device.
> The nvm child device is exposed via auxiliary bus.

...

> +void intel_nvm_init(struct drm_i915_private *i915)
> +{

Lucas recently revamped xe driver to address this, so let's not hide bugs
and return an error where possible.

> +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> +	struct intel_dg_nvm_dev *nvm;
> +	struct auxiliary_device *aux_dev;
> +	int ret;
> +
> +	/* Only the DGFX devices have internal NVM */
> +	if (!IS_DGFX(i915))
> +		return;
> +
> +	/* Nvm pointer should be NULL here */
> +	if (WARN_ON(i915->nvm))
> +		return;
> +
> +	i915->nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> +	if (!i915->nvm)
> +		return;
> +
> +	nvm = i915->nvm;
> +
> +	nvm->writeable_override = true;
> +	nvm->bar.parent = &pdev->resource[0];
> +	nvm->bar.start = GEN12_GUNIT_NVM_BASE + pdev->resource[0].start;
> +	nvm->bar.end = nvm->bar.start + GEN12_GUNIT_NVM_SIZE - 1;
> +	nvm->bar.flags = IORESOURCE_MEM;
> +	nvm->bar.desc = IORES_DESC_NONE;
> +	nvm->regions = regions;
> +
> +	aux_dev = &nvm->aux_dev;
> +
> +	aux_dev->name = "nvm";
> +	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
> +		       PCI_DEVID(pdev->bus->number, pdev->devfn);

Why not just pci_dev_id()?

> +	aux_dev->dev.parent = &pdev->dev;
> +	aux_dev->dev.release = i915_nvm_release_dev;
> +
> +	ret = auxiliary_device_init(aux_dev);
> +	if (ret) {
> +		drm_err(&i915->drm, "i915-nvm aux init failed %d\n", ret);
> +		return;
> +	}
> +
> +	ret = auxiliary_device_add(aux_dev);
> +	if (ret) {
> +		drm_err(&i915->drm, "i915-nvm aux add failed %d\n", ret);
> +		auxiliary_device_uninit(aux_dev);
> +		return;
> +	}
> +}

Raag
Alexander Usyskin May 27, 2025, 6 a.m. UTC | #2 
> Subject: Re: [PATCH v10 06/10] drm/i915/nvm: add nvm device for discrete
> graphics
> 
> On Thu, May 15, 2025 at 04:33:41PM +0300, Alexander Usyskin wrote:
> > Enable access to internal non-volatile memory on
> > DGFX devices via a child device.
> > The nvm child device is exposed via auxiliary bus.
> 
> ...
> 
> > +void intel_nvm_init(struct drm_i915_private *i915)
> > +{
> 
> Lucas recently revamped xe driver to address this, so let's not hide bugs
> and return an error where possible.
> 
I can return error from this call, but the SPI failure is non-fatal for Xe.
Caller should ignore error from this init.

> > +	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > +	struct intel_dg_nvm_dev *nvm;
> > +	struct auxiliary_device *aux_dev;
> > +	int ret;
> > +
> > +	/* Only the DGFX devices have internal NVM */
> > +	if (!IS_DGFX(i915))
> > +		return;
> > +
> > +	/* Nvm pointer should be NULL here */
> > +	if (WARN_ON(i915->nvm))
> > +		return;
> > +
> > +	i915->nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> > +	if (!i915->nvm)
> > +		return;
> > +
> > +	nvm = i915->nvm;
> > +
> > +	nvm->writeable_override = true;
> > +	nvm->bar.parent = &pdev->resource[0];
> > +	nvm->bar.start = GEN12_GUNIT_NVM_BASE + pdev-
> >resource[0].start;
> > +	nvm->bar.end = nvm->bar.start + GEN12_GUNIT_NVM_SIZE - 1;
> > +	nvm->bar.flags = IORESOURCE_MEM;
> > +	nvm->bar.desc = IORES_DESC_NONE;
> > +	nvm->regions = regions;
> > +
> > +	aux_dev = &nvm->aux_dev;
> > +
> > +	aux_dev->name = "nvm";
> > +	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
> > +		       PCI_DEVID(pdev->bus->number, pdev->devfn);
> 
> Why not just pci_dev_id()?
Will do
> 
> > +	aux_dev->dev.parent = &pdev->dev;
> > +	aux_dev->dev.release = i915_nvm_release_dev;
> > +
> > +	ret = auxiliary_device_init(aux_dev);
> > +	if (ret) {
> > +		drm_err(&i915->drm, "i915-nvm aux init failed %d\n", ret);
> > +		return;
> > +	}
> > +
> > +	ret = auxiliary_device_add(aux_dev);
> > +	if (ret) {
> > +		drm_err(&i915->drm, "i915-nvm aux add failed %d\n", ret);
> > +		auxiliary_device_uninit(aux_dev);
> > +		return;
> > +	}
> > +}
> 
> Raag
Raag Jadav May 27, 2025, 6:35 p.m. UTC | #3 
On Tue, May 27, 2025 at 11:30:20AM +0530, Usyskin, Alexander wrote:
> > Subject: Re: [PATCH v10 06/10] drm/i915/nvm: add nvm device for discrete
> > graphics
> > 
> > On Thu, May 15, 2025 at 04:33:41PM +0300, Alexander Usyskin wrote:
> > > Enable access to internal non-volatile memory on
> > > DGFX devices via a child device.
> > > The nvm child device is exposed via auxiliary bus.
> > 
> > ...
> > 
> > > +void intel_nvm_init(struct drm_i915_private *i915)
> > > +{
> > 
> > Lucas recently revamped xe driver to address this, so let's not hide bugs
> > and return an error where possible.
> > 
> I can return error from this call, but the SPI failure is non-fatal for Xe.
> Caller should ignore error from this init.

Fair. Let's atleast return error and leave the handling to the caller,
so we don't have to come back revamping it in the future.

Raag
Alexander Usyskin May 28, 2025, 6:29 a.m. UTC | #4 
> Subject: Re: [PATCH v10 06/10] drm/i915/nvm: add nvm device for discrete
> graphics
> 
> On Tue, May 27, 2025 at 11:30:20AM +0530, Usyskin, Alexander wrote:
> > > Subject: Re: [PATCH v10 06/10] drm/i915/nvm: add nvm device for discrete
> > > graphics
> > >
> > > On Thu, May 15, 2025 at 04:33:41PM +0300, Alexander Usyskin wrote:
> > > > Enable access to internal non-volatile memory on
> > > > DGFX devices via a child device.
> > > > The nvm child device is exposed via auxiliary bus.
> > >
> > > ...
> > >
> > > > +void intel_nvm_init(struct drm_i915_private *i915)
> > > > +{
> > >
> > > Lucas recently revamped xe driver to address this, so let's not hide bugs
> > > and return an error where possible.
> > >
> > I can return error from this call, but the SPI failure is non-fatal for Xe.
> > Caller should ignore error from this init.
> 
> Fair. Let's atleast return error and leave the handling to the caller,
> so we don't have to come back revamping it in the future.

Ok, will add return values here

> 
> Raag
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1a90eb1f180a..2e0fa85491a8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -212,6 +212,10 @@  i915-y += \
 i915-y += \
 	gt/intel_gsc.o
 
+# graphics nvm device (DGFX) support
+i915-y += \
+	intel_nvm.o
+
 # graphics hardware monitoring (HWMON) support
 i915-$(CONFIG_HWMON) += \
 	i915_hwmon.o
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 2b0bcb9aa3c7..c7ab0352d31d 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -82,6 +82,8 @@ 
 #include "soc/intel_dram.h"
 #include "soc/intel_gmch.h"
 
+#include "intel_nvm.h"
+
 #include "i915_debugfs.h"
 #include "i915_driver.h"
 #include "i915_drm_client.h"
@@ -646,6 +648,8 @@  static int i915_driver_register(struct drm_i915_private *dev_priv)
 	/* Depends on sysfs having been initialized */
 	i915_perf_register(dev_priv);
 
+	intel_nvm_init(dev_priv);
+
 	for_each_gt(gt, dev_priv, i)
 		intel_gt_driver_register(gt);
 
@@ -688,6 +692,8 @@  static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 
 	i915_hwmon_unregister(dev_priv);
 
+	intel_nvm_fini(dev_priv);
+
 	i915_perf_unregister(dev_priv);
 	i915_pmu_unregister(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 93b4c504b710..1212117d07ef 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -65,6 +65,7 @@ 
 struct drm_i915_clock_gating_funcs;
 struct vlv_s0ix_state;
 struct intel_pxp;
+struct intel_dg_nvm_dev;
 
 #define GEM_QUIRK_PIN_SWIZZLED_PAGES	BIT(0)
 
@@ -310,6 +311,8 @@  struct drm_i915_private {
 
 	struct i915_perf perf;
 
+	struct intel_dg_nvm_dev *nvm;
+
 	struct i915_hwmon *hwmon;
 
 	struct intel_gt *gt[I915_MAX_GT];
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2e4190da3e0d..7f538ef3cbb5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -321,6 +321,7 @@ 
 #define DG2_GSC_HECI2_BASE	0x00374000
 #define MTL_GSC_HECI1_BASE	0x00116000
 #define MTL_GSC_HECI2_BASE	0x00117000
+#define GEN12_GUNIT_NVM_BASE	0x00102040
 
 #define HECI_H_CSR(base)	_MMIO((base) + 0x4)
 #define   HECI_H_CSR_IE		REG_BIT(0)
diff --git a/drivers/gpu/drm/i915/intel_nvm.c b/drivers/gpu/drm/i915/intel_nvm.c
new file mode 100644
index 000000000000..75d3ebe669ff
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_nvm.c
@@ -0,0 +1,92 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2019-2024, Intel Corporation. All rights reserved.
+ */
+
+#include <linux/intel_dg_nvm_aux.h>
+#include <linux/irq.h>
+#include "i915_reg.h"
+#include "i915_drv.h"
+#include "intel_nvm.h"
+
+#define GEN12_GUNIT_NVM_SIZE 0x80
+
+static const struct intel_dg_nvm_region regions[INTEL_DG_NVM_REGIONS] = {
+	[0] = { .name = "DESCRIPTOR", },
+	[2] = { .name = "GSC", },
+	[11] = { .name = "OptionROM", },
+	[12] = { .name = "DAM", },
+};
+
+static void i915_nvm_release_dev(struct device *dev)
+{
+}
+
+void intel_nvm_init(struct drm_i915_private *i915)
+{
+	struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
+	struct intel_dg_nvm_dev *nvm;
+	struct auxiliary_device *aux_dev;
+	int ret;
+
+	/* Only the DGFX devices have internal NVM */
+	if (!IS_DGFX(i915))
+		return;
+
+	/* Nvm pointer should be NULL here */
+	if (WARN_ON(i915->nvm))
+		return;
+
+	i915->nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
+	if (!i915->nvm)
+		return;
+
+	nvm = i915->nvm;
+
+	nvm->writeable_override = true;
+	nvm->bar.parent = &pdev->resource[0];
+	nvm->bar.start = GEN12_GUNIT_NVM_BASE + pdev->resource[0].start;
+	nvm->bar.end = nvm->bar.start + GEN12_GUNIT_NVM_SIZE - 1;
+	nvm->bar.flags = IORESOURCE_MEM;
+	nvm->bar.desc = IORES_DESC_NONE;
+	nvm->regions = regions;
+
+	aux_dev = &nvm->aux_dev;
+
+	aux_dev->name = "nvm";
+	aux_dev->id = (pci_domain_nr(pdev->bus) << 16) |
+		       PCI_DEVID(pdev->bus->number, pdev->devfn);
+	aux_dev->dev.parent = &pdev->dev;
+	aux_dev->dev.release = i915_nvm_release_dev;
+
+	ret = auxiliary_device_init(aux_dev);
+	if (ret) {
+		drm_err(&i915->drm, "i915-nvm aux init failed %d\n", ret);
+		return;
+	}
+
+	ret = auxiliary_device_add(aux_dev);
+	if (ret) {
+		drm_err(&i915->drm, "i915-nvm aux add failed %d\n", ret);
+		auxiliary_device_uninit(aux_dev);
+		return;
+	}
+}
+
+void intel_nvm_fini(struct drm_i915_private *i915)
+{
+	struct intel_dg_nvm_dev *nvm = i915->nvm;
+
+	/* Only the DGFX devices have internal NVM */
+	if (!IS_DGFX(i915))
+		return;
+
+	/* Nvm pointer should not be NULL here */
+	if (WARN_ON(!nvm))
+		return;
+
+	auxiliary_device_delete(&nvm->aux_dev);
+	auxiliary_device_uninit(&nvm->aux_dev);
+	kfree(nvm);
+	i915->nvm = NULL;
+}
diff --git a/drivers/gpu/drm/i915/intel_nvm.h b/drivers/gpu/drm/i915/intel_nvm.h
new file mode 100644
index 000000000000..7bc3d1114a3f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_nvm.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright(c) 2019-2024 Intel Corporation. All rights reserved.
+ */
+
+#ifndef __INTEL_NVM_H__
+#define __INTEL_NVM_H__
+
+struct drm_i915_private;
+
+void intel_nvm_init(struct drm_i915_private *i915);
+
+void intel_nvm_fini(struct drm_i915_private *i915);
+
+#endif /* __INTEL_NVM_H__ */