All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: "Onur Özkan" <work@onurozkan.dev>,
	"Mark Brown" <broonie@kernel.org>,
	daniel.almeida@collabora.com, aliceryhl@google.com,
	dakr@kernel.org, airlied@gmail.com, simona@ffwll.ch,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	lgirdwood@gmail.com, ojeda@kernel.org,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v2 1/1] drm/tyr: make SRAM supply optional like panthor
Date: Fri, 13 Feb 2026 13:42:15 +0100	[thread overview]
Message-ID: <20260213134215.27141dfe@fedora> (raw)
In-Reply-To: <aY8V8xlTv0UAdyQz@e142607>

Hi,

On Fri, 13 Feb 2026 12:15:47 +0000
Liviu Dudau <Liviu.Dudau@arm.com> wrote:

> On Thu, Feb 12, 2026 at 09:30:10PM +0300, Onur Özkan wrote:
> > On Thu, 12 Feb 2026 14:51:34 +0100
> > Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >   
> > > On Thu, 12 Feb 2026 13:13:31 +0000
> > > Mark Brown <broonie@kernel.org> wrote:
> > >   
> > > > On Thu, Feb 12, 2026 at 01:46:01PM +0100, Boris Brezillon wrote:  
> > > > > Mark Brown <broonie@kernel.org> wrote:    
> > > >   
> > > > > > The panthor driver is buggy here and should be fixed, the
> > > > > > driver should treat the supply as mandatory and let the system
> > > > > > integration work out how it's actually made available.    
> > > >   
> > > > > > Trying to open code this just breaks the error handling.    
> > > >   
> > > > > Maybe, but the thing is, the DT bindings have been accepted
> > > > > already, and it's not something we can easily change. What we can
> > > > > do is make this sram-supply mandatory for new compatibles, but we
> > > > > can't force it on older/existing SoCs without breaking
> > > > > backward-DT compat.    
> > > > 
> > > > In practice you can because we do sub in a dummy regulator for
> > > > missing supplies, it produces a warning but works fine.  If we
> > > > didn't do this it'd be basically impossible to add regulator
> > > > support to anything at any point after the original merge which is
> > > > clearly not reasonable.  
> > > 
> > > Okay, I guess we need to fix panthor then...
> > >   
> > 
> > That + updating the log to something like "sram-supply is missing in
> > the DT" would be quite better I think. It would make the issue more
> > obvious and convey that the DT file is expected to configure that field
> > explicitly. With the current log message, not many people will
> > understand the problem at a glance.
> > 
> > As for the bug I described in this patch, we can proceed with the
> > alternative solution (updating the DT file) that I mentioned in the
> > Zulip thread (the link is included in the patch). Which is this simple
> > diff:
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > index dafad29f9854..a30339fd2c10 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > @@ -177,6 +177,7 @@ &gmac1_rgmii_clk
> > 
> >  &gpu {
> >         mali-supply = <&vdd_gpu_s0>;
> > +       sram-supply = <&vdd_gpu_mem_s0>;
> >         status = "okay";
> >  };
> > 
> > @@ -537,7 +538,7 @@ rk806_dvs3_null: dvs3-null-pins {
> >                 };
> > 
> >                 regulators {
> > -                       vdd_gpu_s0: dcdc-reg1 {
> > +                       vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {  
> 
> You don't need to define a new label, using the same supply for mali-supply and
> sram-supply should be fine.
> 
> >                                 regulator-name = "vdd_gpu_s0";
> >                                 regulator-boot-on;
> >                                 regulator-min-microvolt = <550000>;
> > 
> > Note that this only fixes the issue for the Orange Pi 5. If we want
> > to go further, the same approach should be applied to many other boards
> > as well. I can generate a list of the DT files (using a simple Python
> > script) that need this update over the weekend.  
> 
> Yes, please, but bias the script towards using the same regulator as mali-supply.
> 
> > 
> > If we want to go even further and fix all DT files to properly include
> > sram-supply we could also enforce that DT files do not omit sram-supply
> > in the future. I am not sure this is strictly necessary but it also
> > doesn't seem consistent to leave things as they are. Right now, some DT
> > files include sram-supply even when there is no separate SRAM rail,
> > while others do not. As a result, some boards will continue to print
> > that annoying log message.
> > 
> > It's not very clear which approach is best.  
> 
> I'm in favor of the proposal here, where we make sram-supply mandatory for non-"mt8196-mali"
> SoCs and we patch the DTs to add the sram-supply for those.

Works for me. Thanks for chiming in Liviu.

Regards,

Boris

  reply	other threads:[~2026-02-13 12:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-12 10:05 [PATCH v2 0/1] drm/tyr: make SRAM supply optional like panthor Onur Özkan
2026-02-12 10:05 ` [PATCH v2 1/1] " Onur Özkan
2026-02-12 11:34   ` Mark Brown
2026-02-12 12:16     ` Onur Özkan
2026-02-12 12:21       ` Mark Brown
2026-02-12 12:46         ` Boris Brezillon
2026-02-12 13:13           ` Mark Brown
2026-02-12 13:51             ` Boris Brezillon
2026-02-12 18:30               ` Onur Özkan
2026-02-13 12:15                 ` Liviu Dudau
2026-02-13 12:42                   ` Boris Brezillon [this message]
2026-02-13 12:59                   ` Onur Özkan
2026-02-13 10:57         ` Liviu Dudau
2026-02-13 15:54           ` Mark Brown
2026-02-12 12:22       ` Boris Brezillon
2026-02-12 13:10         ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260213134215.27141dfe@fedora \
    --to=boris.brezillon@collabora.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=broonie@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.