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

[Thomas Zimmermann <tzimmermann@suse.de>]

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@suse.de>
>>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>>> ---
>>>>>>     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@kernel.org>
>
> 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)