[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aAZskvNjYjSYEN-s@intel.com>
Date: Mon, 21 Apr 2025 12:04:34 -0400
From: Rodrigo Vivi <rodrigo.vivi@...el.com>
To: Alexander Usyskin <alexander.usyskin@...el.com>
CC: Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger
<richard@....at>, Vignesh Raghavendra <vigneshr@...com>, Lucas De Marchi
<lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin
<tursulin@...ulin.net>, Karthik Poosa <karthik.poosa@...el.com>, "Reuven
Abliyev" <reuven.abliyev@...el.com>, Oren Weil <oren.jer.weil@...el.com>,
<linux-mtd@...ts.infradead.org>, <intel-xe@...ts.freedesktop.org>,
<dri-devel@...ts.freedesktop.org>, <intel-gfx@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>, <Abliyev@...edesktop.org>
Subject: Re: [PATCH v8 12/12] drm/xe/nvm: add support for non-posted erase
On Mon, Apr 14, 2025 at 12:38:03PM +0300, Alexander Usyskin wrote:
> From: "Abliyev, Reuven" <reuven.abliyev@...el.com>
>
> Erase command is slow on discrete graphics storage
> and may overshot PCI completion timeout.
> BMG introduces the ability to have non-posted erase.
> Add driver support for non-posted erase with polling
> for erase completion.
>
> Signed-off-by: Abliyev, Reuven <reuven.abliyev@...el.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@...el.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@...el.com>
and
Acked-by: Rodrigo Vivi <rodrigo.vivi@...el.com>
on all xe and i915 patches to get merged through another trees if needed.
> ---
> drivers/gpu/drm/xe/xe_nvm.c | 25 +++++++++++++++++
> drivers/mtd/devices/mtd_intel_dg.c | 43 ++++++++++++++++++++++++++++--
> include/linux/intel_dg_nvm_aux.h | 2 ++
> 3 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_nvm.c b/drivers/gpu/drm/xe/xe_nvm.c
> index 8aec20bc629a..dd91f2e37661 100644
> --- a/drivers/gpu/drm/xe/xe_nvm.c
> +++ b/drivers/gpu/drm/xe/xe_nvm.c
> @@ -14,7 +14,15 @@
> #include "xe_sriov.h"
>
> #define GEN12_GUNIT_NVM_BASE 0x00102040
> +#define GEN12_DEBUG_NVM_BASE 0x00101018
> +
> +#define GEN12_CNTL_PROTECTED_NVM_REG 0x0010100C
> +
> #define GEN12_GUNIT_NVM_SIZE 0x80
> +#define GEN12_DEBUG_NVM_SIZE 0x4
> +
> +#define NVM_NON_POSTED_ERASE_CHICKEN_BIT BIT(13)
> +
> #define HECI_FW_STATUS_2_NVM_ACCESS_MODE BIT(3)
>
> static const struct intel_dg_nvm_region regions[INTEL_DG_NVM_REGIONS] = {
> @@ -28,6 +36,16 @@ static void xe_nvm_release_dev(struct device *dev)
> {
> }
>
> +static bool xe_nvm_non_posted_erase(struct xe_device *xe)
> +{
> + struct xe_gt *gt = xe_root_mmio_gt(xe);
> +
> + if (xe->info.platform != XE_BATTLEMAGE)
> + return false;
> + return !(xe_mmio_read32(>->mmio, XE_REG(GEN12_CNTL_PROTECTED_NVM_REG)) &
> + NVM_NON_POSTED_ERASE_CHICKEN_BIT);
> +}
> +
> static bool xe_nvm_writable_override(struct xe_device *xe)
> {
> struct xe_gt *gt = xe_root_mmio_gt(xe);
> @@ -85,6 +103,7 @@ void xe_nvm_init(struct xe_device *xe)
> nvm = xe->nvm;
>
> nvm->writable_override = xe_nvm_writable_override(xe);
> + nvm->non_posted_erase = xe_nvm_non_posted_erase(xe);
> 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;
> @@ -92,6 +111,12 @@ void xe_nvm_init(struct xe_device *xe)
> nvm->bar.desc = IORES_DESC_NONE;
> nvm->regions = regions;
>
> + nvm->bar2.parent = &pdev->resource[0];
> + nvm->bar2.start = GEN12_DEBUG_NVM_BASE + pdev->resource[0].start;
> + nvm->bar2.end = nvm->bar2.start + GEN12_DEBUG_NVM_SIZE - 1;
> + nvm->bar2.flags = IORESOURCE_MEM;
> + nvm->bar2.desc = IORES_DESC_NONE;
> +
> aux_dev = &nvm->aux_dev;
>
> aux_dev->name = "nvm";
> diff --git a/drivers/mtd/devices/mtd_intel_dg.c b/drivers/mtd/devices/mtd_intel_dg.c
> index 9f4bb15a03b8..c898107a588f 100644
> --- a/drivers/mtd/devices/mtd_intel_dg.c
> +++ b/drivers/mtd/devices/mtd_intel_dg.c
> @@ -28,6 +28,9 @@ struct intel_dg_nvm {
> struct mtd_info mtd;
> struct mutex lock; /* region access lock */
> void __iomem *base;
> + void __iomem *base2;
> + bool non_posted_erase;
> +
> size_t size;
> unsigned int nregions;
> struct {
> @@ -44,6 +47,7 @@ struct intel_dg_nvm {
> #define NVM_VALSIG_REG 0x00000010
> #define NVM_ADDRESS_REG 0x00000040
> #define NVM_REGION_ID_REG 0x00000044
> +#define NVM_DEBUG_REG 0x00000000
> /*
> * [15:0]-Erase size = 0x0010 4K 0x0080 32K 0x0100 64K
> * [23:16]-Reserved
> @@ -75,6 +79,9 @@ struct intel_dg_nvm {
> #define NVM_FREG_ADDR_SHIFT 12
> #define NVM_FREG_MIN_REGION_SIZE 0xFFF
>
> +#define NVM_NON_POSTED_ERASE_DONE BIT(23)
> +#define NVM_NON_POSTED_ERASE_DONE_ITER 3000
> +
> static inline void idg_nvm_set_region_id(struct intel_dg_nvm *nvm, u8 region)
> {
> iowrite32((u32)region, nvm->base + NVM_REGION_ID_REG);
> @@ -370,11 +377,30 @@ idg_erase(struct intel_dg_nvm *nvm, u8 region, loff_t from, u64 len, u64 *fail_a
> {
> u64 i;
> const u32 block = 0x10;
> + u32 reg;
> + u32 iter = 0;
> void __iomem *base = nvm->base;
> + void __iomem *base2 = nvm->base2;
>
> for (i = 0; i < len; i += SZ_4K) {
> iowrite32(from + i, base + NVM_ADDRESS_REG);
> iowrite32(region << 24 | block, base + NVM_ERASE_REG);
> + if (nvm->non_posted_erase) {
> + /* Wait for Erase Done */
> + reg = ioread32(base2 + NVM_DEBUG_REG);
> + while (!(reg & NVM_NON_POSTED_ERASE_DONE) &&
> + ++iter < NVM_NON_POSTED_ERASE_DONE_ITER) {
> + msleep(10);
> + reg = ioread32(base2 + NVM_DEBUG_REG);
> + }
> + if (reg & NVM_NON_POSTED_ERASE_DONE) {
> + /* Clear Erase Done */
> + iowrite32(reg, base2 + NVM_DEBUG_REG);
> + } else {
> + *fail_addr = from + i;
> + return -ETIME;
> + }
> + }
> /* Since the writes are via sguint
> * we cannot do back to back erases.
> */
> @@ -383,7 +409,8 @@ idg_erase(struct intel_dg_nvm *nvm, u8 region, loff_t from, u64 len, u64 *fail_a
> return len;
> }
>
> -static int intel_dg_nvm_init(struct intel_dg_nvm *nvm, struct device *device)
> +static int intel_dg_nvm_init(struct intel_dg_nvm *nvm, struct device *device,
> + bool non_posted_erase)
> {
> int ret;
> unsigned int i, n;
> @@ -443,7 +470,10 @@ static int intel_dg_nvm_init(struct intel_dg_nvm *nvm, struct device *device)
> n++;
> }
>
> + nvm->non_posted_erase = non_posted_erase;
> +
> dev_dbg(device, "Registered %d regions\n", n);
> + dev_dbg(device, "Non posted erase %d\n", nvm->non_posted_erase);
>
> /* Need to add 1 to the amount of memory
> * so it is reported as an even block
> @@ -778,7 +808,16 @@ static int intel_dg_mtd_probe(struct auxiliary_device *aux_dev,
> goto err;
> }
>
> - ret = intel_dg_nvm_init(nvm, device);
> + if (invm->non_posted_erase) {
> + nvm->base2 = devm_ioremap_resource(device, &invm->bar2);
> + if (IS_ERR(nvm->base2)) {
> + dev_err(device, "base2 mmio not mapped\n");
> + ret = PTR_ERR(nvm->base2);
> + goto err;
> + }
> + }
> +
> + ret = intel_dg_nvm_init(nvm, device, invm->non_posted_erase);
> if (ret < 0) {
> dev_err(device, "cannot initialize nvm %d\n", ret);
> goto err;
> diff --git a/include/linux/intel_dg_nvm_aux.h b/include/linux/intel_dg_nvm_aux.h
> index 68df634c994c..bee25dfc6982 100644
> --- a/include/linux/intel_dg_nvm_aux.h
> +++ b/include/linux/intel_dg_nvm_aux.h
> @@ -17,7 +17,9 @@ struct intel_dg_nvm_region {
> struct intel_dg_nvm_dev {
> struct auxiliary_device aux_dev;
> bool writable_override;
> + bool non_posted_erase;
> struct resource bar;
> + struct resource bar2;
> const struct intel_dg_nvm_region *regions;
> };
>
> --
> 2.43.0
>
Powered by blists - more mailing lists