Re: [PATCH v2 1/5] dt-bindings: display: simple-framebuffer: Add interconnects property

[Thomas Zimmermann]

Hi

Am 30.06.25 um 09:26 schrieb Hans de Goede:
> Hi,
>
> On 30-Jun-25 8:34 AM, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 28.06.25 um 13:50 schrieb Krzysztof Kozlowski:
> > > On 27/06/2025 13:34, Thomas Zimmermann wrote:
> > > > Hi
> > > >
> > > > Am 27.06.25 um 10:08 schrieb Krzysztof Kozlowski:
> > > > > On Mon, Jun 23, 2025 at 08:44:45AM +0200, Luca Weiss wrote:
> > > > > > Document the interconnects property which is a list of interconnect
> > > > > > paths that is used by the framebuffer and therefore needs to be kept
> > > > > > alive when the framebuffer is being used.
> > > > > >
> > > > > > Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > > > > > Signed-off-by: Luca Weiss <luca.weiss@xxxxxxxxxxxxx>
> > > > > > ---
> > > > > >     Documentation/devicetree/bindings/display/simple-framebuffer.yaml | 3 +++
> > > > > >     1 file changed, 3 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> > > > > > index 296500f9da05e296dbbeec50ba5186b6b30aaffc..f0fa0ef23d91043dfb2b220c654b80e2e80850cd 100644
> > > > > > --- a/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/display/simple-framebuffer.yaml
> > > > > > @@ -79,6 +79,9 @@ properties:
> > > > > >       power-domains:
> > > > > >         description: List of power domains used by the framebuffer.
> > > > > >     +  interconnects:
> > > > > > +    description: List of interconnect paths used by the framebuffer.
> > > > > > +
> > > > > maxItems: 1, or this is not a simple FB anymore. Anything which needs
> > > > > some sort of resources in unknown way is not simple anymore. You need
> > > > > device specific bindings.
> > > > In this context, 'simple' means that this device cannot change display
> > > > modes or do graphics acceleration. The hardware itself is not
> > > > necessarily simple. As Javier pointed out, it's initialized by firmware
> > > If hardware is not simple, then it needs specific bindings.
> > >
> > > > on the actual hardware. Think of 'VGA-for-ARM'. We need these resources
> > > > to keep the display working.
> > > I don't claim you do not need these resources. I claim device is not
> > > simple thus does not suit rules for generic bindings. Generic bindings
> > > are in general not allowed and we have them only for very, very simple
> > > devices.
> > >
> > > You say this is not simple device, so there you go - specific binding
> > > for this complex (not-simple) device.
> > No, I didn't. I said that the device is simple. I did not say that the device's hardware is simple. Sounds nonsensical, but makes sense here. The simple-framebuffer is just the range of display memory that the firmware configured for printing boot-up messages. We use it for the kernel's output as well.  Being generic and simple is the exact raison d'etre for simple-framebuffer.  (The display property points to the actual hardware, but we don't need it.)
> I believe part of the problem here is the simple part of the simplefb
> name in hindsight that is a mistake and we should have called the thing
> firmware-framebuffer since its goal is to pass along a firmware setup
> framebuffer to the OS for displaying stuff.

I totally feel you. In DRM land, we've also been upset about the naming. 
But well...


> As for the argument for having a firmware-framebuffer not being allowed
> because framebuffers are to complex to have a generic binding, that
> ship has long sailed since we already have the simplefb binding.
>
> And since we already have the binding I do not find this not being
> simple a valid technical argument. That is an argument to allow
> having a generic binding at all or to not have it at all, but here
> we already have the binding and this is just about evolving the binding
> with changing hw needs.

Exactly my point.

> And again this reminds me very much of the whole clocks / regulators
> addition to simplefb discussion we had over a decade ago. Back then
> we had a huge thread, almost a flamefest with in my memory over
> a 100 emails and back then the only argument against adding them
> was also "it is not simple", which IMHO really is a non argument for
> an already existing binding. Certainly it is not a good technical
> argument.
>
> During the last decade, after clocks and regulators were added to
> the binding. simplefb has been used successfully on millions (billions?)
> handover the firmware framebuffer to the OS for bootsplash use,
> replacing various vendor hacks for this. Disallowing the addition of
> interconnect support to the simplefb binding will only result in
> various vendor hacks appearing in vendor kernels for this, which
> I believe is something which we should try to avoid.

Exactly. And I'd also add that the current way of handling the situation 
is the only feasible one. Simple-framebuffer needs to be generic and 
compatible with existing and future hardware at minimal cost. The way of 
doing so, is to have a few properties, such as clocks, regulators and 
now interconnects, that the firmware clearly tells us about.  If we go 
with per-hardware/per-vendor nodes, simple-framebuffer loses its usefulness.


> So as the maintainer of the simplefb kernel driver for over a decade
> I strongly advice the DT maintainers to accept this bindings patch

As the maintainer of the simpledrm driver, I second this.

Best regards
Thomas

> and from my my side this still is:
>
> Reviewed-by: Hans de Goede <hansg@xxxxxxxxxx>
>
> Regards,
>
> Hans

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)