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

[Boris Brezillon <boris.brezillon@collabora.com>]

Hi Nicolas,

On Mon, 16 Feb 2026 13:43:19 +0100
Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:

> > >>
> > >> I wonder if a more generic device tree flag would be better here.  
> > > 
> > > No, we don't want it as a separate DT flag. This is all stuff we can
> > > hide behind the compat, and every bit we add to the DT we don't
> > > strictly need turns out to be a liability in the long run in general.
> > >   
> > >> What happens if others do the same as Mediatek or Mediatek decides to
> > >> do this with more processors and this list grows?  
> > > 
> > > That's what panthor_soc_data is for: you can attach per-compat
> > > properties without polluting the DT with more stuff that can be
> > > directly inferred from the compatible.
> > >   
> > >> It seems like a
> > >> panthor binding might be useful to prevent future bloat.  
> > > 
> > > It's actually the opposite, the more we add to the DT, the trickier it
> > > gets to maintain, because we tend to get those things wrong (is the
> > > SRAM really not needed on mt8196, or is this just a workaround to hide
> > > the fact the PM is deferred to some FW?).
> > >   
> > 
> > MT8196 has three supplies: core, stack, sram.
> > 
> > For example, the Google Rauru Chromebooks use those:
> > 
> >         core-supply = <&mt6373_vbuck7>;
> >         stack-supply = <&mt6316dp_vbuck0>;
> >         sram-supply = <&mt6316kp_vbuck1>;
> > 
> > As of now (in our midstream trees), these supplies are declared in the gpufreq
> > node (the performance domain controller), and required to be on whenever GPUEB
> > interaction is needed, other than whenever the GPU itself is, well, needed to
> > be powered.
> > 
> > As of the current model, these supplies are getting powered on and off along
> > with the MFG power domain.
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pmdomain/mediatek/mtk-mfg-pmdomain.c#n1005
> > 
> > I'm not sure what happens if we also add those to the GPU node... for this, I'm
> > adding Nicolas to the Ccs, as he is the one who developed support for EB.  
> 
> Fairly sure they need to be on as part of any of the operations the MFG stuff
> does, but I also am not 100% sure on this because I didn't take notes at the
> time.
> 
> Either way, this patch shouldn't exist, it doesn't do anything useful, as a
> missing supply from the DT can be caught with `make dtbs_check`.

Well, the fact DTs lacking the sram-supply definition went through
is a clear sign that `make dtbs_check` is not bulletproof. Not saying
the script doesn't work, but if maintainers don't run it automatically
before merging DTs, it's going to fail again, I'm afraid. If we had
enforced that "supply is mandatory" rule at runtime, we wouldn't be in
position where we have invalid DTs/DTBs merged/deployed, and I'm saying
that as the person git blame points to for introducing this logic :-).

> It does not
> need to be booted on each device to then have the driver abort probe at runtime.

Now that we've merged those DTs we can't really fail the probe
anyway, because that would break DT-backward compat, but flagging those
DTBs as broken (with an error message) would be better than pretending
it's all good, IMHO.

Regards,

Boris