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.cb/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, Onurquoted
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);