[PATCH 0/4] Make MT8196 get its Mali GPU shader_present from nvmem

[PATCH 0/4] Make MT8196 get its Mali GPU shader_present from nvmem

[Nicolas Frattaroli]

The MediaTek MT8196 SoC's Mali SHADER_PRESENT register does not list
only functional shader cores, but also those that are fused off to
improve yield.

The SHADER_PRESENT bitmask with the one fused off core omitted is to be
found in an efuse. However, the efuse address is considered
confidential, and is not public knowledge.

The MT8196 GPUEB MCU, which does the power management for the Mali GPU
on this SoC, knows and reads the efuse however, and exposes it in the
shared memory intended to communicate state to the application
processor. Reading the bitmask from this shared memory area is the
vendor's intended solution.

This series models this in the binding and implements it in the
corresponding Linux drivers:
- the mali-valhall-csf binding gets an nvmem-cells/nvmem-cell-names
  property to declare that shader-present is in a different castle
- the mt8196-gpufreq binding requires nodes to expose the shader-present
  cell
- panthor checks for the presence of the shader-present cell and uses it
  as the shader-present value if it's found, instead of the Mali GPU
  register contents
- mtk-mfg-pmdomain becomes an nvmem provider and will happily serve
  queries for the shader-present cell

While it would be preferable if we could read the efuse directly, it's
not possible as things stand, and insisting on it will just keep this
hardware from working in mainline. Running a GPU workload with a
SHADER_PRESENT bitmask that includes a faulty core results in corrupt
GPU rendering output.

Modelling the mt8196-gpufreq device as a nvmem-cell provider however is
not lying about the hardware's capabilities, as it truly does provide
access to the nvmem-cell, even if it acts as a proxy.

From a bindings and panthor perspective, this is also generic enough to
where hypothetical other vendors doing the same thing (even with direct
efuse access) can rely on the same cell name and implementation.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Nicolas Frattaroli (4):
      dt-bindings: gpu: mali-valhall-csf: Add shader-present nvmem cell
      dt-bindings: power: mt8196-gpufreq: Describe nvmem provider ability
      drm/panthor: Implement reading shader_present from nvmem
      pmdomain: mediatek: mtk-mfg: Expose shader_present as nvmem cell

 .../bindings/gpu/arm,mali-valhall-csf.yaml         | 14 +++++
 .../bindings/power/mediatek,mt8196-gpufreq.yaml    | 13 +++++
 drivers/gpu/drm/panthor/panthor_hw.c               | 63 +++++++++++++++++++---
 drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c       | 57 ++++++++++++++++++++
 4 files changed, 141 insertions(+), 6 deletions(-)
---
base-commit: 16f014a645fb35303b8fd3305f23f8ecd3f2f2a6
change-id: 20251217-mt8196-shader-present-e2dc3f97fc74

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH 1/4] dt-bindings: gpu: mali-valhall-csf: Add shader-present nvmem cell

[Nicolas Frattaroli]

On the MediaTek MT8196 SoC, the bitmask for which shader cores are
present and functional is not the one in the Mali GPU's registers, but
in an external efuse.

Add the nvmem cell properties to describe such a setup, and make them
required on MT8196.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml      | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
index bee9faf1d3f8..8eccd4338a2b 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
@@ -51,6 +51,14 @@ properties:
           - stacks
       - const: stacks
 
+  nvmem-cells:
+    items:
+      - description: bitmask of functional shader cores
+
+  nvmem-cell-names:
+    items:
+      - const: shader-present
+
   mali-supply: true
 
   operating-points-v2: true
@@ -108,6 +116,8 @@ allOf:
       properties:
         clocks:
           minItems: 3
+        nvmem-cells: false
+        nvmem-cell-names: false
         power-domains:
           maxItems: 1
         power-domain-names: false
@@ -133,6 +143,8 @@ allOf:
             - const: core
             - const: stacks
       required:
+        - nvmem-cells
+        - nvmem-cell-names
         - power-domains
 
 examples:
@@ -179,6 +191,8 @@ examples:
                      <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH 0>,
                      <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH 0>;
         interrupt-names = "job", "mmu", "gpu";
+        nvmem-cells = <&shader_present>;
+        nvmem-cell-names = "shader-present";
         power-domains = <&gpufreq>;
     };
 

-- 
2.52.0

[PATCH 2/4] dt-bindings: power: mt8196-gpufreq: Describe nvmem provider ability

[Nicolas Frattaroli]

On the MediaTek MT8196 SoC, the Mali GPU's "shader_present" hardware
register may also include a non-functional shader core, along with the
present shader cores. An efuse elsewhere in the SoC provides the
shader_present mask with the fused off core omitted.

However, the efuse address is not publicly disclosed. What is known
though is that the GPUEB MCU reads this efuse, and exposes its contents
in the memory it shares with the application processor.

We can therefore describe the mediatek,mt8196-gpufreq device as being an
nvmem provider for this purpose, as it does provide nvmem access in an
indirect way.

The shader-present child node is left out of the list of required
properties as we may one day be able to describe the actual efuse region
this value comes from, so the gpufreq device isn't necessarily the only
device that can provide this cell, and implementations shouldn't need to
implement this functionality once this is the case.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml  | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml b/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml
index b9e43abaf8a4..66fc59b3c8b4 100644
--- a/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml
@@ -74,9 +74,18 @@ properties:
   "#clock-cells":
     const: 1
 
+  "#nvmem-cell-cells":
+    const: 0
+
   "#power-domain-cells":
     const: 0
 
+  shader-present:
+    type: object
+
+dependencies:
+  shader-present: [ "#nvmem-cell-cells" ]
+
 required:
   - compatible
   - reg
@@ -113,5 +122,9 @@ examples:
                      "ccf", "fast-dvfs";
         memory-region = <&gpueb_shared_memory>;
         #clock-cells = <1>;
+        #nvmem-cell-cells = <0>;
         #power-domain-cells = <0>;
+
+        shader-present {
+        };
     };

-- 
2.52.0

[PATCH 3/4] drm/panthor: Implement reading shader_present from nvmem

[Nicolas Frattaroli]

On some platforms, notably MediaTek MT8196, the shader_present bitmask
in the Mali GPU register for it has cores enabled that may be faulty.
The true shader_present bitmask is found in an efuse instead.

Implement reading shader_present from an nvmem cell if one is present,
falling back to the Mali register if it's absent. The error codes are
trickled up through to the probe function so that probe deferral works.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_hw.c | 63 ++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
index 87ebb7ae42c4..eb44c8b108aa 100644
--- a/drivers/gpu/drm/panthor/panthor_hw.c
+++ b/drivers/gpu/drm/panthor/panthor_hw.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 /* Copyright 2025 ARM Limited. All rights reserved. */
 
+#include <linux/nvmem-consumer.h>
 #include <drm/drm_print.h>
 
 #include "panthor_device.h"
@@ -109,7 +110,52 @@ static char *get_gpu_model_name(struct panthor_device *ptdev)
 	return "(Unknown Mali GPU)";
 }
 
-static void panthor_gpu_info_init(struct panthor_device *ptdev)
+static int overload_shader_present(struct panthor_device *ptdev)
+{
+	struct device *dev = ptdev->base.dev;
+	struct nvmem_cell *cell = nvmem_cell_get(dev, "shader-present");
+	ssize_t len;
+	void *buf;
+	int ret;
+
+	if (IS_ERR(cell)) {
+		/* On platforms without this cell, use the Mali register */
+		if (PTR_ERR(cell) == -ENOENT)
+			return 0;
+
+		return dev_err_probe(dev, PTR_ERR(cell),
+				     "Failed to get shader-present nvmem cell\n");
+	}
+
+	buf = nvmem_cell_read(cell, &len);
+	if (IS_ERR(buf)) {
+		ret = dev_err_probe(dev, PTR_ERR(buf),
+				    "Failed to read shader-present nvmem cell\n");
+		goto err_put_cell;
+	}
+
+	if (!len || len > 8) {
+		ret = dev_err_probe(dev, -EINVAL, "shader-present cell can't be length %ld\n",
+				    len);
+		goto err_free;
+	}
+
+	memcpy(&ptdev->gpu_info.shader_present, buf, len);
+
+	kfree(buf);
+	nvmem_cell_put(cell);
+
+	return 0;
+
+err_free:
+	kfree(buf);
+err_put_cell:
+	nvmem_cell_put(cell);
+
+	return ret;
+}
+
+static int panthor_gpu_info_init(struct panthor_device *ptdev)
 {
 	unsigned int i;
 
@@ -143,13 +189,18 @@ static void panthor_gpu_info_init(struct panthor_device *ptdev)
 		ptdev->gpu_info.tiler_present = gpu_read64(ptdev, GPU_TILER_PRESENT);
 		ptdev->gpu_info.l2_present = gpu_read64(ptdev, GPU_L2_PRESENT);
 	}
+
+	return overload_shader_present(ptdev);
 }
 
-static void panthor_hw_info_init(struct panthor_device *ptdev)
+static int panthor_hw_info_init(struct panthor_device *ptdev)
 {
 	u32 major, minor, status;
+	int ret;
 
-	panthor_gpu_info_init(ptdev);
+	ret = panthor_gpu_info_init(ptdev);
+	if (ret)
+		return ret;
 
 	major = GPU_VER_MAJOR(ptdev->gpu_info.gpu_id);
 	minor = GPU_VER_MINOR(ptdev->gpu_info.gpu_id);
@@ -172,6 +223,8 @@ static void panthor_hw_info_init(struct panthor_device *ptdev)
 		 "shader_present=0x%0llx l2_present=0x%0llx tiler_present=0x%0llx",
 		 ptdev->gpu_info.shader_present, ptdev->gpu_info.l2_present,
 		 ptdev->gpu_info.tiler_present);
+
+	return 0;
 }
 
 static int panthor_hw_bind_device(struct panthor_device *ptdev)
@@ -218,7 +271,5 @@ int panthor_hw_init(struct panthor_device *ptdev)
 	if (ret)
 		return ret;
 
-	panthor_hw_info_init(ptdev);
-
-	return 0;
+	return panthor_hw_info_init(ptdev);
 }

-- 
2.52.0

[PATCH 4/4] pmdomain: mediatek: mtk-mfg: Expose shader_present as nvmem cell

[Nicolas Frattaroli]

Implement nvmem-provider functionality in mtk-mfg-pmdomain, such that it
can expose its GF_REG_SHADER_PRESENT value in the shared memory as an
nvmem cell for panthor.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c | 57 ++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
index 9bad577b3ae4..725ebc678f1b 100644
--- a/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
+++ b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
@@ -10,6 +10,7 @@
 #include <linux/clk-provider.h>
 #include <linux/container_of.h>
 #include <linux/iopoll.h>
+#include <linux/nvmem-provider.h>
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -872,6 +873,58 @@ static int mtk_mfg_init_clk_provider(struct mtk_mfg *mfg)
 	return 0;
 }
 
+static int mtk_mfg_read_nvmem(void *priv, unsigned int offset, void *val, size_t bytes)
+{
+	struct mtk_mfg *mfg = priv;
+	u32 *buf = val;
+
+	if (bytes != 4)
+		return -EINVAL;
+
+	if (!mfg->shared_mem)
+		return -ENODEV;
+
+	if (offset + bytes >= mfg->shared_mem_size)
+		return -EINVAL;
+
+	*buf = readl(mfg->shared_mem + offset);
+
+	return 0;
+}
+
+static int mtk_mfg_init_nvmem_provider(struct mtk_mfg *mfg)
+{
+	struct device *dev = &mfg->pdev->dev;
+	struct nvmem_cell_info cell = {};
+	struct nvmem_config config = {};
+	struct nvmem_device *nvdev;
+	int ret;
+
+	config.reg_read = mtk_mfg_read_nvmem;
+	config.dev = dev;
+	config.read_only = true;
+	config.priv = mfg;
+	config.size = 4;
+	config.word_size = 4;
+
+	nvdev = devm_nvmem_register(dev, &config);
+	of_node_put(config.of_node);
+	if (IS_ERR(nvdev))
+		return dev_err_probe(dev, PTR_ERR(nvdev), "Couldn't register nvmem provider\n");
+
+	cell.name = "shader-present";
+	cell.offset = GF_REG_SHADER_PRESENT;
+	cell.bytes = 4;
+	cell.np = of_get_child_by_name(dev->of_node, cell.name);
+
+	ret = nvmem_add_one_cell(nvdev, &cell);
+	/* cell.np purposefully not put as nvmem_add_one_cell does not increase refcount */
+	if (ret)
+		return dev_err_probe(dev, ret, "Couldn't add cell %s\n", cell.name);
+
+	return 0;
+}
+
 static int mtk_mfg_probe(struct platform_device *pdev)
 {
 	struct mtk_mfg *mfg;
@@ -984,6 +1037,10 @@ static int mtk_mfg_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_power_off;
 
+	ret = mtk_mfg_init_nvmem_provider(mfg);
+	if (ret)
+		goto err_power_off;
+
 	ret = of_genpd_add_provider_simple(dev->of_node, &mfg->pd);
 	if (ret) {
 		dev_err_probe(dev, ret, "Failed to add pmdomain provider\n");

-- 
2.52.0

Re: [PATCH 3/4] drm/panthor: Implement reading shader_present from nvmem

[Boris Brezillon]

On Wed, 17 Dec 2025 18:03:29 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:

> On some platforms, notably MediaTek MT8196, the shader_present bitmask
> in the Mali GPU register for it has cores enabled that may be faulty.
> The true shader_present bitmask is found in an efuse instead.
> 
> Implement reading shader_present from an nvmem cell if one is present,
> falling back to the Mali register if it's absent. The error codes are
> trickled up through to the probe function so that probe deferral works.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/panthor/panthor_hw.c | 63 ++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> index 87ebb7ae42c4..eb44c8b108aa 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.c
> +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0 or MIT
>  /* Copyright 2025 ARM Limited. All rights reserved. */
>  
> +#include <linux/nvmem-consumer.h>
>  #include <drm/drm_print.h>
>  
>  #include "panthor_device.h"
> @@ -109,7 +110,52 @@ static char *get_gpu_model_name(struct panthor_device *ptdev)
>  	return "(Unknown Mali GPU)";
>  }
>  
> -static void panthor_gpu_info_init(struct panthor_device *ptdev)
> +static int overload_shader_present(struct panthor_device *ptdev)
> +{
> +	struct device *dev = ptdev->base.dev;
> +	struct nvmem_cell *cell = nvmem_cell_get(dev, "shader-present");
> +	ssize_t len;
> +	void *buf;
> +	int ret;
> +
> +	if (IS_ERR(cell)) {
> +		/* On platforms without this cell, use the Mali register */
> +		if (PTR_ERR(cell) == -ENOENT)
> +			return 0;
> +
> +		return dev_err_probe(dev, PTR_ERR(cell),
> +				     "Failed to get shader-present nvmem cell\n");
> +	}
> +
> +	buf = nvmem_cell_read(cell, &len);
> +	if (IS_ERR(buf)) {
> +		ret = dev_err_probe(dev, PTR_ERR(buf),
> +				    "Failed to read shader-present nvmem cell\n");
> +		goto err_put_cell;
> +	}
> +
> +	if (!len || len > 8) {
> +		ret = dev_err_probe(dev, -EINVAL, "shader-present cell can't be length %ld\n",
> +				    len);
> +		goto err_free;
> +	}
> +
> +	memcpy(&ptdev->gpu_info.shader_present, buf, len);
> +
> +	kfree(buf);
> +	nvmem_cell_put(cell);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(buf);
> +err_put_cell:
> +	nvmem_cell_put(cell);
> +
> +	return ret;
> +}
> +
> +static int panthor_gpu_info_init(struct panthor_device *ptdev)
>  {
>  	unsigned int i;
>  
> @@ -143,13 +189,18 @@ static void panthor_gpu_info_init(struct panthor_device *ptdev)
>  		ptdev->gpu_info.tiler_present = gpu_read64(ptdev, GPU_TILER_PRESENT);
>  		ptdev->gpu_info.l2_present = gpu_read64(ptdev, GPU_L2_PRESENT);
>  	}
> +
> +	return overload_shader_present(ptdev);
>  }
>  
> -static void panthor_hw_info_init(struct panthor_device *ptdev)
> +static int panthor_hw_info_init(struct panthor_device *ptdev)
>  {
>  	u32 major, minor, status;
> +	int ret;
>  
> -	panthor_gpu_info_init(ptdev);
> +	ret = panthor_gpu_info_init(ptdev);
> +	if (ret)
> +		return ret;
>  
>  	major = GPU_VER_MAJOR(ptdev->gpu_info.gpu_id);
>  	minor = GPU_VER_MINOR(ptdev->gpu_info.gpu_id);
> @@ -172,6 +223,8 @@ static void panthor_hw_info_init(struct panthor_device *ptdev)
>  		 "shader_present=0x%0llx l2_present=0x%0llx tiler_present=0x%0llx",
>  		 ptdev->gpu_info.shader_present, ptdev->gpu_info.l2_present,
>  		 ptdev->gpu_info.tiler_present);
> +
> +	return 0;
>  }
>  
>  static int panthor_hw_bind_device(struct panthor_device *ptdev)
> @@ -218,7 +271,5 @@ int panthor_hw_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> -	panthor_hw_info_init(ptdev);
> -
> -	return 0;
> +	return panthor_hw_info_init(ptdev);
>  }
> 

Re: [PATCH 3/4] drm/panthor: Implement reading shader_present from nvmem

[Steven Price]

On 17/12/2025 17:03, Nicolas Frattaroli wrote:
> On some platforms, notably MediaTek MT8196, the shader_present bitmask
> in the Mali GPU register for it has cores enabled that may be faulty.
> The true shader_present bitmask is found in an efuse instead.
> 
> Implement reading shader_present from an nvmem cell if one is present,
> falling back to the Mali register if it's absent. The error codes are
> trickled up through to the probe function so that probe deferral works.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

[Although I really hope other vendors don't do this - the hardware is
broken!]

Although one NIT below if you respin for other reasons...

> ---
>  drivers/gpu/drm/panthor/panthor_hw.c | 63 ++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> index 87ebb7ae42c4..eb44c8b108aa 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.c
> +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0 or MIT
>  /* Copyright 2025 ARM Limited. All rights reserved. */
>  
> +#include <linux/nvmem-consumer.h>
>  #include <drm/drm_print.h>
>  
>  #include "panthor_device.h"
> @@ -109,7 +110,52 @@ static char *get_gpu_model_name(struct panthor_device *ptdev)
>  	return "(Unknown Mali GPU)";
>  }
>  
> -static void panthor_gpu_info_init(struct panthor_device *ptdev)
> +static int overload_shader_present(struct panthor_device *ptdev)
> +{
> +	struct device *dev = ptdev->base.dev;
> +	struct nvmem_cell *cell = nvmem_cell_get(dev, "shader-present");
> +	ssize_t len;
> +	void *buf;
> +	int ret;
> +
> +	if (IS_ERR(cell)) {
> +		/* On platforms without this cell, use the Mali register */
> +		if (PTR_ERR(cell) == -ENOENT)
> +			return 0;
> +
> +		return dev_err_probe(dev, PTR_ERR(cell),
> +				     "Failed to get shader-present nvmem cell\n");
> +	}
> +
> +	buf = nvmem_cell_read(cell, &len);
> +	if (IS_ERR(buf)) {
> +		ret = dev_err_probe(dev, PTR_ERR(buf),
> +				    "Failed to read shader-present nvmem cell\n");
> +		goto err_put_cell;
> +	}
> +
> +	if (!len || len > 8) {
> +		ret = dev_err_probe(dev, -EINVAL, "shader-present cell can't be length %ld\n",
> +				    len);
> +		goto err_free;
> +	}
> +
> +	memcpy(&ptdev->gpu_info.shader_present, buf, len);
> +
> +	kfree(buf);
> +	nvmem_cell_put(cell);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(buf);
> +err_put_cell:
> +	nvmem_cell_put(cell);
> +
> +	return ret;
> +}

Rather than repeating the clean up, you can do something like:

{
	void *buf = NULL;
	int ret = 0

	if (IS_ERR(cell)) {
		ret = dev_err_probe(...);
		goto out;
	}

	buf = nvmem_cell_read();
	if (IS_ERR(buf)) {
		ret = dev_err_probe(...);
		goto out;
	}

	if (!len || len > 8) {
		ret = dev_err_probe(...);
		goto out;
	}

	memcpy();

out:
	if (!IS_ERR(buf))
		kfree(buf);
	if (!IS_ERR(cell))
		nvmem_cell_put(cell);

	return ret;
}

That avoids mistakes when adding a new operation into the sequence. Or
you can use the fancy new cleanup helpers, but that feels overkill here.
But equally I'm ok if you leave the code as is - it's simple enough and
the conversation can be done later if we need it.

Thanks,
Steve

> +
> +static int panthor_gpu_info_init(struct panthor_device *ptdev)
>  {
>  	unsigned int i;
>  
> @@ -143,13 +189,18 @@ static void panthor_gpu_info_init(struct panthor_device *ptdev)
>  		ptdev->gpu_info.tiler_present = gpu_read64(ptdev, GPU_TILER_PRESENT);
>  		ptdev->gpu_info.l2_present = gpu_read64(ptdev, GPU_L2_PRESENT);
>  	}
> +
> +	return overload_shader_present(ptdev);
>  }
>  
> -static void panthor_hw_info_init(struct panthor_device *ptdev)
> +static int panthor_hw_info_init(struct panthor_device *ptdev)
>  {
>  	u32 major, minor, status;
> +	int ret;
>  
> -	panthor_gpu_info_init(ptdev);
> +	ret = panthor_gpu_info_init(ptdev);
> +	if (ret)
> +		return ret;
>  
>  	major = GPU_VER_MAJOR(ptdev->gpu_info.gpu_id);
>  	minor = GPU_VER_MINOR(ptdev->gpu_info.gpu_id);
> @@ -172,6 +223,8 @@ static void panthor_hw_info_init(struct panthor_device *ptdev)
>  		 "shader_present=0x%0llx l2_present=0x%0llx tiler_present=0x%0llx",
>  		 ptdev->gpu_info.shader_present, ptdev->gpu_info.l2_present,
>  		 ptdev->gpu_info.tiler_present);
> +
> +	return 0;
>  }
>  
>  static int panthor_hw_bind_device(struct panthor_device *ptdev)
> @@ -218,7 +271,5 @@ int panthor_hw_init(struct panthor_device *ptdev)
>  	if (ret)
>  		return ret;
>  
> -	panthor_hw_info_init(ptdev);
> -
> -	return 0;
> +	return panthor_hw_info_init(ptdev);
>  }
> 

Re: [PATCH 3/4] drm/panthor: Implement reading shader_present from nvmem

[Chia-I Wu]

On Fri, Dec 19, 2025 at 7:31 AM Steven Price <steven.price@arm.com> wrote:
>
> On 17/12/2025 17:03, Nicolas Frattaroli wrote:
> > On some platforms, notably MediaTek MT8196, the shader_present bitmask
> > in the Mali GPU register for it has cores enabled that may be faulty.
> > The true shader_present bitmask is found in an efuse instead.
> >
> > Implement reading shader_present from an nvmem cell if one is present,
> > falling back to the Mali register if it's absent. The error codes are
> > trickled up through to the probe function so that probe deferral works.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
>
> Reviewed-by: Steven Price <steven.price@arm.com>
>
> [Although I really hope other vendors don't do this - the hardware is
> broken!]
>
> Although one NIT below if you respin for other reasons...
>
> > ---
> >  drivers/gpu/drm/panthor/panthor_hw.c | 63 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 57 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> > index 87ebb7ae42c4..eb44c8b108aa 100644
> > --- a/drivers/gpu/drm/panthor/panthor_hw.c
> > +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0 or MIT
> >  /* Copyright 2025 ARM Limited. All rights reserved. */
> >
> > +#include <linux/nvmem-consumer.h>
> >  #include <drm/drm_print.h>
> >
> >  #include "panthor_device.h"
> > @@ -109,7 +110,52 @@ static char *get_gpu_model_name(struct panthor_device *ptdev)
> >       return "(Unknown Mali GPU)";
> >  }
> >
> > -static void panthor_gpu_info_init(struct panthor_device *ptdev)
> > +static int overload_shader_present(struct panthor_device *ptdev)
> > +{
> > +     struct device *dev = ptdev->base.dev;
> > +     struct nvmem_cell *cell = nvmem_cell_get(dev, "shader-present");
> > +     ssize_t len;
> > +     void *buf;
> > +     int ret;
> > +
> > +     if (IS_ERR(cell)) {
> > +             /* On platforms without this cell, use the Mali register */
> > +             if (PTR_ERR(cell) == -ENOENT)
> > +                     return 0;
> > +
> > +             return dev_err_probe(dev, PTR_ERR(cell),
> > +                                  "Failed to get shader-present nvmem cell\n");
> > +     }
> > +
> > +     buf = nvmem_cell_read(cell, &len);
> > +     if (IS_ERR(buf)) {
> > +             ret = dev_err_probe(dev, PTR_ERR(buf),
> > +                                 "Failed to read shader-present nvmem cell\n");
> > +             goto err_put_cell;
> > +     }
> > +
> > +     if (!len || len > 8) {
> > +             ret = dev_err_probe(dev, -EINVAL, "shader-present cell can't be length %ld\n",
> > +                                 len);
> > +             goto err_free;
> > +     }
> > +
> > +     memcpy(&ptdev->gpu_info.shader_present, buf, len);
When len is 4, this is neither overriding nor masking. I might suggest
switching to nvmem_cell_read_variable_le_u64 for simplicity.

Either way, Reviewed-by: Chia-I Wu <olvaffe@gmail.com>


> > +
> > +     kfree(buf);
> > +     nvmem_cell_put(cell);
> > +
> > +     return 0;
> > +
> > +err_free:
> > +     kfree(buf);
> > +err_put_cell:
> > +     nvmem_cell_put(cell);
> > +
> > +     return ret;
> > +}
>
> Rather than repeating the clean up, you can do something like:
>
> {
>         void *buf = NULL;
>         int ret = 0
>
>         if (IS_ERR(cell)) {
>                 ret = dev_err_probe(...);
>                 goto out;
>         }
>
>         buf = nvmem_cell_read();
>         if (IS_ERR(buf)) {
>                 ret = dev_err_probe(...);
>                 goto out;
>         }
>
>         if (!len || len > 8) {
>                 ret = dev_err_probe(...);
>                 goto out;
>         }
>
>         memcpy();
>
> out:
>         if (!IS_ERR(buf))
>                 kfree(buf);
>         if (!IS_ERR(cell))
>                 nvmem_cell_put(cell);
>
>         return ret;
> }
>
> That avoids mistakes when adding a new operation into the sequence. Or
> you can use the fancy new cleanup helpers, but that feels overkill here.
> But equally I'm ok if you leave the code as is - it's simple enough and
> the conversation can be done later if we need it.
>
> Thanks,
> Steve
>
> > +
> > +static int panthor_gpu_info_init(struct panthor_device *ptdev)
> >  {
> >       unsigned int i;
> >
> > @@ -143,13 +189,18 @@ static void panthor_gpu_info_init(struct panthor_device *ptdev)
> >               ptdev->gpu_info.tiler_present = gpu_read64(ptdev, GPU_TILER_PRESENT);
> >               ptdev->gpu_info.l2_present = gpu_read64(ptdev, GPU_L2_PRESENT);
> >       }
> > +
> > +     return overload_shader_present(ptdev);
> >  }
> >
> > -static void panthor_hw_info_init(struct panthor_device *ptdev)
> > +static int panthor_hw_info_init(struct panthor_device *ptdev)
> >  {
> >       u32 major, minor, status;
> > +     int ret;
> >
> > -     panthor_gpu_info_init(ptdev);
> > +     ret = panthor_gpu_info_init(ptdev);
> > +     if (ret)
> > +             return ret;
> >
> >       major = GPU_VER_MAJOR(ptdev->gpu_info.gpu_id);
> >       minor = GPU_VER_MINOR(ptdev->gpu_info.gpu_id);
> > @@ -172,6 +223,8 @@ static void panthor_hw_info_init(struct panthor_device *ptdev)
> >                "shader_present=0x%0llx l2_present=0x%0llx tiler_present=0x%0llx",
> >                ptdev->gpu_info.shader_present, ptdev->gpu_info.l2_present,
> >                ptdev->gpu_info.tiler_present);
> > +
> > +     return 0;
> >  }
> >
> >  static int panthor_hw_bind_device(struct panthor_device *ptdev)
> > @@ -218,7 +271,5 @@ int panthor_hw_init(struct panthor_device *ptdev)
> >       if (ret)
> >               return ret;
> >
> > -     panthor_hw_info_init(ptdev);
> > -
> > -     return 0;
> > +     return panthor_hw_info_init(ptdev);
> >  }
> >
>

Re: [PATCH 4/4] pmdomain: mediatek: mtk-mfg: Expose shader_present as nvmem cell

[Chia-I Wu]

On Wed, Dec 17, 2025 at 9:04 AM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> Implement nvmem-provider functionality in mtk-mfg-pmdomain, such that it
> can expose its GF_REG_SHADER_PRESENT value in the shared memory as an
> nvmem cell for panthor.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c | 57 ++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>
> diff --git a/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
> index 9bad577b3ae4..725ebc678f1b 100644
> --- a/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
> +++ b/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/container_of.h>
>  #include <linux/iopoll.h>
> +#include <linux/nvmem-provider.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> @@ -872,6 +873,58 @@ static int mtk_mfg_init_clk_provider(struct mtk_mfg *mfg)
>         return 0;
>  }
>
> +static int mtk_mfg_read_nvmem(void *priv, unsigned int offset, void *val, size_t bytes)
> +{
> +       struct mtk_mfg *mfg = priv;
> +       u32 *buf = val;
> +
> +       if (bytes != 4)
> +               return -EINVAL;
> +
> +       if (!mfg->shared_mem)
> +               return -ENODEV;
> +
> +       if (offset + bytes >= mfg->shared_mem_size)
> +               return -EINVAL;
> +
> +       *buf = readl(mfg->shared_mem + offset);
> +
> +       return 0;
> +}
> +
> +static int mtk_mfg_init_nvmem_provider(struct mtk_mfg *mfg)
> +{
> +       struct device *dev = &mfg->pdev->dev;
> +       struct nvmem_cell_info cell = {};
> +       struct nvmem_config config = {};
> +       struct nvmem_device *nvdev;
> +       int ret;
> +
> +       config.reg_read = mtk_mfg_read_nvmem;
> +       config.dev = dev;
> +       config.read_only = true;
> +       config.priv = mfg;
> +       config.size = 4;
> +       config.word_size = 4;
> +
> +       nvdev = devm_nvmem_register(dev, &config);
> +       of_node_put(config.of_node);
This looks like a dead line.
> +       if (IS_ERR(nvdev))
> +               return dev_err_probe(dev, PTR_ERR(nvdev), "Couldn't register nvmem provider\n");
> +
> +       cell.name = "shader-present";
> +       cell.offset = GF_REG_SHADER_PRESENT;
> +       cell.bytes = 4;
> +       cell.np = of_get_child_by_name(dev->of_node, cell.name);
> +
> +       ret = nvmem_add_one_cell(nvdev, &cell);
> +       /* cell.np purposefully not put as nvmem_add_one_cell does not increase refcount */
I think you still need to of_node_put on errors.
> +       if (ret)
> +               return dev_err_probe(dev, ret, "Couldn't add cell %s\n", cell.name);
> +
> +       return 0;
> +}
> +
>  static int mtk_mfg_probe(struct platform_device *pdev)
>  {
>         struct mtk_mfg *mfg;
> @@ -984,6 +1037,10 @@ static int mtk_mfg_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_power_off;
>
> +       ret = mtk_mfg_init_nvmem_provider(mfg);
> +       if (ret)
> +               goto err_power_off;
> +
>         ret = of_genpd_add_provider_simple(dev->of_node, &mfg->pd);
>         if (ret) {
>                 dev_err_probe(dev, ret, "Failed to add pmdomain provider\n");
>
> --
> 2.52.0
>

Re: [PATCH 1/4] dt-bindings: gpu: mali-valhall-csf: Add shader-present nvmem cell

[Rob Herring (Arm)]

On Wed, 17 Dec 2025 18:03:27 +0100, Nicolas Frattaroli wrote:
> On the MediaTek MT8196 SoC, the bitmask for which shader cores are
> present and functional is not the one in the Mali GPU's registers, but
> in an external efuse.
> 
> Add the nvmem cell properties to describe such a setup, and make them
> required on MT8196.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-valhall-csf.yaml      | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 2/4] dt-bindings: power: mt8196-gpufreq: Describe nvmem provider ability

[Rob Herring (Arm)]

On Wed, 17 Dec 2025 18:03:28 +0100, Nicolas Frattaroli wrote:
> On the MediaTek MT8196 SoC, the Mali GPU's "shader_present" hardware
> register may also include a non-functional shader core, along with the
> present shader cores. An efuse elsewhere in the SoC provides the
> shader_present mask with the fused off core omitted.
> 
> However, the efuse address is not publicly disclosed. What is known
> though is that the GPUEB MCU reads this efuse, and exposes its contents
> in the memory it shares with the application processor.
> 
> We can therefore describe the mediatek,mt8196-gpufreq device as being an
> nvmem provider for this purpose, as it does provide nvmem access in an
> indirect way.
> 
> The shader-present child node is left out of the list of required
> properties as we may one day be able to describe the actual efuse region
> this value comes from, so the gpufreq device isn't necessarily the only
> device that can provide this cell, and implementations shouldn't need to
> implement this functionality once this is the case.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../devicetree/bindings/power/mediatek,mt8196-gpufreq.yaml  | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>