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

[Hans de Goede <hdegoede@redhat.com>]

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.

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.

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.

So as the maintainer of the simplefb kernel driver for over a decade
I strongly advice the DT maintainers to accept this bindings patch
and from my my side this still is:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans