Thread (15 messages) 15 messages, 9 authors, 2026-02-24

Re: [PATCH v1 2/2] drm/panthor: treat sram as mandatory except mt8196

From: Steven Price <steven.price@arm.com>
Date: 2026-02-16 15:20:42
Also in: dri-devel, linux-mediatek, lkml

On 16/02/2026 14:41, Onur Özkan wrote:
On Mon, 16 Feb 2026 10:37:43 +0100
Boris Brezillon [off-list ref] wrote:
quoted
On Sun, 15 Feb 2026 13:02:51 +0300
Onur Özkan [off-list ref] wrote:
quoted
If sram-supply is missing, Panthor falls back to a
dummy regulator with a warning. This implicit behavior
hides missing DT wiring behind regulator core fallback.

Make SRAM handling explicit: require sram-supply for all
Panthor compatibles except mt8196-mali where GPU supplies
are intentionally managed outside Panthor and DT does not
model sram-supply for that compatible.

This keeps DT power modeling explicit and avoids relying on
dummy-regulator fallback.

Link: https://lore.kernel.org/all/20260213155937.6af75786@nimda/ (local)
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 drivers/gpu/drm/panthor/panthor_devfreq.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_devfreq.c
b/drivers/gpu/drm/panthor/panthor_devfreq.c index
2249b41ca4af..5f6075f18fe3 100644 ---
a/drivers/gpu/drm/panthor/panthor_devfreq.c +++
b/drivers/gpu/drm/panthor/panthor_devfreq.c @@ -206,12 +206,17 @@
int panthor_devfreq_init(struct panthor_device *ptdev)
 	 * But without knowing if it's beneficial or not (in term
of power
 	 * consumption), or how much it slows down the
suspend/resume steps,
 	 * let's just keep regulators enabled for the device
lifetime.
+	 *
+	 * Treat sram-supply as mandatory except for mt8196-mali.
It manages
+	 * SRAM outside Panthor so this driver must not require
direct control
+	 * over it.
 	 */
-	ret = devm_regulator_get_enable_optional(dev, "sram");
-	if (ret && ret != -ENODEV) {
-		if (ret != -EPROBE_DEFER)
+	if (!of_device_is_compatible(dev->of_node,
"mediatek,mt8196-mali")) {
Rather than checking for specific compats here, let's go for
a dont_need_sram_supply bool in panthor_soc_data.
Makes sense.
quoted
quoted
+		ret = devm_regulator_get_enable_optional(dev,
"sram");
If we assume SRAM supply is mandatory, should this be
devm_regulator_get_enable() instead?
That was the first thing I did but when I tested it, it didn't work as
expected because devm_regulator_get_enable() fell back to the dummy
regulator without returning an error.
My understanding was that devm_regulator_get_enable() would output a
warning in that case.

I'm not sure it really makes sense to deliberately fail the probe just
because the regulator isn't there: we know in practice it often works
and it can be useful for bring-up purposes. The issue was that we don't
want DTs submitted without the "sram" regulator and the dt-check hasn't
been sufficient in the past to catch such errors.

So a dmesg warning seems (to me) like the right sort of balance -
there's obviously something to fix, but it doesn't block bring-up
(unless of course the hardware actually requires it of course).

I also note that you've lost the -EPROBE_DEFER check, meaning that the
log message you've added will spam the logs if a defer happens.

My reading for these threads is that we need:

 * A new field in panthor_soc_data for the mt8196 case.

 * To check for new field, and if it's not set then use
devm_regulator_get_enable() so that it generates a warning message if
the regulator isn't present. Otherwise we skip the call.

So no need for panthor itself to log any messages, and we just return
any errors from devm_regulator_get_enable(). A missing regulator will
generate a warning but no error.

What am I missing?

Thanks,
Steve
Regards,
Onur
 
quoted
quoted
+		if (ret) {
 			DRM_DEV_ERROR(dev, "Couldn't
retrieve/enable sram supply\n");
-		return ret;
+			return ret;
+		}
 	}
 
 	opp = devfreq_recommended_opp(dev, &cur_freq, 0);
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help