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.
+
width:
$ref: /schemas/types.yaml#/definitions/uint32
description: Width of the framebuffer in pixels
--
2.50.0
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. Best regards, Krzysztof
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 on the actual hardware. Think of 'VGA-for-ARM'. We need these resources to keep the display working. Best regards Thomas > > Best regards, > Krzysztof > > -- -- 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)
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. Best regards, Krzysztof
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.) Best regards Thomas > > Best regards, > Krzysztof -- -- 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)
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
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)
Hi Krzysztof, On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: > 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. The bindings support an arbitrary number of clocks, regulators, power-domains. Why should I artificially limit the interconnects to only one? The driver code also has that support added in this series. Regards Luca > > Best regards, > Krzysztof
On 27/06/2025 11:48, Luca Weiss wrote: > Hi Krzysztof, > > On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >> 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. > > The bindings support an arbitrary number of clocks, regulators, > power-domains. Why should I artificially limit the interconnects to only > one? And IMO they should not. Bindings are not supposed to be generic. > > The driver code also has that support added in this series. That's not the problem here. Best regards, Krzysztof
Hi Am 28.06.25 um 13:49 schrieb Krzysztof Kozlowski: > On 27/06/2025 11:48, Luca Weiss wrote: >> Hi Krzysztof, >> >> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>> 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. >> The bindings support an arbitrary number of clocks, regulators, >> power-domains. Why should I artificially limit the interconnects to only >> one? > And IMO they should not. Bindings are not supposed to be generic. > >> The driver code also has that support added in this series. > That's not the problem here. Then could you please state the problem in clear terms? We're obviously not all on the same page here. Best regards Thomas > > > Best regards, > Krzysztof > -- -- 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)
Hi Krzysztof, On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: > On 27/06/2025 11:48, Luca Weiss wrote: >> Hi Krzysztof, >> >> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>> 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. >> >> The bindings support an arbitrary number of clocks, regulators, >> power-domains. Why should I artificially limit the interconnects to only >> one? > > And IMO they should not. Bindings are not supposed to be generic. The simplefb binding is a binding to allow keeping the firmware, e.g. uboot setup framebuffer alive to e.g. show a boot splash until the native display-engine drive loads. Needing display-engine specific bindings totally contradicts the whole goal of It is generic by nature and I really do not see how clocks and regulators are any different then interconnects here. Note that we had a huge discussion about adding clock and regulators to simplefb many years ago with pretty much the same arguments against doing so. In the end it was decided to add regulator and clocks support to the simplefb bindings and non of the feared problems with e.g. ordening of turning things on happened. A big part of this is that the claiming of clks / regulators / interconnects by the simplefb driver is there to keep things on, so it happens before the kernel starts tuning off unused resources IOW everything is already up and running and this really is about avoiding turning things off. Regards, Hans
On 29/06/2025 14:07, Hans de Goede wrote: > Hi Krzysztof, > > On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: >> On 27/06/2025 11:48, Luca Weiss wrote: >>> Hi Krzysztof, >>> >>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>>> 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. >>> >>> The bindings support an arbitrary number of clocks, regulators, >>> power-domains. Why should I artificially limit the interconnects to only >>> one? >> >> And IMO they should not. Bindings are not supposed to be generic. > > The simplefb binding is a binding to allow keeping the firmware, e.g. > uboot setup framebuffer alive to e.g. show a boot splash until > the native display-engine drive loads. Needing display-engine > specific bindings totally contradicts the whole goal of No, it does not. DT is well designed for that through expressing compatibility. I did not say you cannot have generic fallback for simple use case. But this (and previous patchset) grows this into generic binding ONLY and that is not correct. > > It is generic by nature and I really do not see how clocks and > regulators are any different then interconnects here. Yeah, they are also wrong. I already commented on this. > > Note that we had a huge discussion about adding clock > and regulators to simplefb many years ago with pretty > much the same arguments against doing so. In the end it was > decided to add regulator and clocks support to the simplefb > bindings and non of the feared problems with e.g. ordening > of turning things on happened. > > A big part of this is that the claiming of clks / regulators / > interconnects by the simplefb driver is there to keep things on, > so it happens before the kernel starts tuning off unused resources > IOW everything is already up and running and this really is about > avoiding turning things off. No one asks to drop them from the driver. I only want specific front compatible which will list and constrain the properties. It is not contradictory to your statements, U-boot support, driver support. I really do not see ANY argument why this cannot follow standard DT rules. Best regards, Krzysztof
Hi, On 30-Jun-25 10:24 AM, Krzysztof Kozlowski wrote: > On 29/06/2025 14:07, Hans de Goede wrote: >> Hi Krzysztof, >> >> On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: >>> On 27/06/2025 11:48, Luca Weiss wrote: >>>> Hi Krzysztof, >>>> >>>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>>>> 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. >>>> >>>> The bindings support an arbitrary number of clocks, regulators, >>>> power-domains. Why should I artificially limit the interconnects to only >>>> one? >>> >>> And IMO they should not. Bindings are not supposed to be generic. >> >> The simplefb binding is a binding to allow keeping the firmware, e.g. >> uboot setup framebuffer alive to e.g. show a boot splash until >> the native display-engine drive loads. Needing display-engine >> specific bindings totally contradicts the whole goal of > > No, it does not. DT is well designed for that through expressing > compatibility. I did not say you cannot have generic fallback for simple > use case. > > But this (and previous patchset) grows this into generic binding ONLY > and that is not correct. I think that it is important here to notice that this is not a generic fallback binding, this is not and will never be intended to replace have a proper binding for the display-engine. This is just a way to give the kernel access to the firmware setup framebuffer to e.g. show a bootsplash but also fatal kernel errors until the real display-engine driver loads. Note sometimes the whole framebuffer memory is not touched at all and the sole reason for having a driver attach to the simplefb node early on is just to keep the resources needed to keep the panel lit up around (on) until the real display-engine driver comes along to claim those resources. This avoids the display going black if the display-engine driver only binds after the kernel starts turning off unused resources, this typically happens when the display-engine driver is a module. >> It is generic by nature and I really do not see how clocks and >> regulators are any different then interconnects here. > > Yeah, they are also wrong. I already commented on this. > >> >> Note that we had a huge discussion about adding clock >> and regulators to simplefb many years ago with pretty >> much the same arguments against doing so. In the end it was >> decided to add regulator and clocks support to the simplefb >> bindings and non of the feared problems with e.g. ordening >> of turning things on happened. >> >> A big part of this is that the claiming of clks / regulators / >> interconnects by the simplefb driver is there to keep things on, >> so it happens before the kernel starts tuning off unused resources >> IOW everything is already up and running and this really is about >> avoiding turning things off. > > No one asks to drop them from the driver. I only want specific front > compatible which will list and constrain the properties. It is not > contradictory to your statements, U-boot support, driver support. I > really do not see ANY argument why this cannot follow standard DT rules. So what you are saying is that you want something like: framebuffer0: framebuffer@1d385000 { compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; } and that the binding for qcom.simple-framebuffer-sm8650-mdss can then list interconnects ? Regards, Hans
On 30/06/2025 10:40, Hans de Goede wrote: >> >> No one asks to drop them from the driver. I only want specific front >> compatible which will list and constrain the properties. It is not >> contradictory to your statements, U-boot support, driver support. I >> really do not see ANY argument why this cannot follow standard DT rules. > > So what you are saying is that you want something like: > > framebuffer0: framebuffer@1d385000 { > compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; > } > > and that the binding for qcom.simple-framebuffer-sm8650-mdss > can then list interconnects ? IMO yes (after adjusting above to coding style), but as mentioned in other response you can just get an ack or opinion from Rob or Conor. Best regards, Krzysztof
On Wed, Jul 02, 2025 at 10:43:27PM +0200, Krzysztof Kozlowski wrote: > On 30/06/2025 10:40, Hans de Goede wrote: > >> > >> No one asks to drop them from the driver. I only want specific front > >> compatible which will list and constrain the properties. It is not > >> contradictory to your statements, U-boot support, driver support. I > >> really do not see ANY argument why this cannot follow standard DT rules. > > > > So what you are saying is that you want something like: > > > > framebuffer0: framebuffer@1d385000 { > > compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; > > } > > > > and that the binding for qcom.simple-framebuffer-sm8650-mdss > > can then list interconnects ? > IMO yes (after adjusting above to coding style), but as mentioned in > other response you can just get an ack or opinion from Rob or Conor. But, this way we end up describing MDSS hardware block twice: once with the proper device structure and once more in the simple-framebuffer definition. I think this is a bit strange. I understand your point of having a device-specific compatible string and also having a verifiable schema, but I think it's an overkill here. Consider regulator supplies of this simple-framebuffer. Obviously some of them supply the panel and not the SoC parts. Should we also include the panel into the respective compat string? What about describing the device with two different DSI panels? I think this explodes too quickly to be useful. I'd cast my (small) vote into continuing using the simple-framebuffer as is, without additional compatible strings and extend the bindings allowing unbound number of interconnects. -- With best wishes Dmitry
Hi Krzysztof, On Sun Jul 6, 2025 at 1:24 PM CEST, Dmitry Baryshkov wrote: > On Wed, Jul 02, 2025 at 10:43:27PM +0200, Krzysztof Kozlowski wrote: >> On 30/06/2025 10:40, Hans de Goede wrote: >> >> >> >> No one asks to drop them from the driver. I only want specific front >> >> compatible which will list and constrain the properties. It is not >> >> contradictory to your statements, U-boot support, driver support. I >> >> really do not see ANY argument why this cannot follow standard DT rules. >> > >> > So what you are saying is that you want something like: >> > >> > framebuffer0: framebuffer@1d385000 { >> > compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; >> > } >> > >> > and that the binding for qcom.simple-framebuffer-sm8650-mdss >> > can then list interconnects ? >> IMO yes (after adjusting above to coding style), but as mentioned in >> other response you can just get an ack or opinion from Rob or Conor. > > But, this way we end up describing MDSS hardware block twice: once with > the proper device structure and once more in the simple-framebuffer > definition. I think this is a bit strange. > > I understand your point of having a device-specific compatible string > and also having a verifiable schema, but I think it's an overkill here. > > Consider regulator supplies of this simple-framebuffer. Obviously some > of them supply the panel and not the SoC parts. Should we also include > the panel into the respective compat string? What about describing the > device with two different DSI panels? > > I think this explodes too quickly to be useful. I'd cast my (small) vote > into continuing using the simple-framebuffer as is, without additional > compatible strings and extend the bindings allowing unbound number of > interconnects. How do we continue on this? If the current solution is not acceptable, can you suggest one that is? I'd like to keep this moving to not block the dts upstreaming unnecessarily - or otherwise I need to drop simple-framebuffer from the dts patch and keep this out-of-tree along with a patch like this. Regards Luca
On 11/07/2025 09:49, Luca Weiss wrote: >> I think this explodes too quickly to be useful. I'd cast my (small) vote >> into continuing using the simple-framebuffer as is, without additional >> compatible strings and extend the bindings allowing unbound number of >> interconnects. > > How do we continue on this? > > If the current solution is not acceptable, can you suggest one that is? > > I'd like to keep this moving to not block the dts upstreaming > unnecessarily - or otherwise I need to drop simple-framebuffer from the > dts patch and keep this out-of-tree along with a patch like this. I gave another alternative already (in this thread!) - get an ack or opinion from @Rob or @Conor. For the cases I am not sure or I got something wrong, I always defer to @Rob. Best regards, Krzysztof
Hi Am 02.07.25 um 22:43 schrieb Krzysztof Kozlowski: > On 30/06/2025 10:40, Hans de Goede wrote: >>> No one asks to drop them from the driver. I only want specific front >>> compatible which will list and constrain the properties. It is not >>> contradictory to your statements, U-boot support, driver support. I >>> really do not see ANY argument why this cannot follow standard DT rules. >> So what you are saying is that you want something like: >> >> framebuffer0: framebuffer@1d385000 { >> compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; >> } >> >> and that the binding for qcom.simple-framebuffer-sm8650-mdss >> can then list interconnects ? > IMO yes (after adjusting above to coding style), but as mentioned in > other response you can just get an ack or opinion from Rob or Conor. But does that work with *any* device that requires interconnects? The next such simple-framebuffer device should work out of the box *without* the kernel knowing anything about it. That's one of the key features of the simple-framebuffer. If we have to maintainer per-device feature sets, it breaks that assumption. Best regards Thomas > > Best regards, > Krzysztof -- -- 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)
Hi Thomas, On 3-Jul-25 8:47 AM, Thomas Zimmermann wrote: > Hi > > Am 02.07.25 um 22:43 schrieb Krzysztof Kozlowski: >> On 30/06/2025 10:40, Hans de Goede wrote: >>>> No one asks to drop them from the driver. I only want specific front >>>> compatible which will list and constrain the properties. It is not >>>> contradictory to your statements, U-boot support, driver support. I >>>> really do not see ANY argument why this cannot follow standard DT rules. >>> So what you are saying is that you want something like: >>> >>> framebuffer0: framebuffer@1d385000 { >>> compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; >>> } >>> >>> and that the binding for qcom.simple-framebuffer-sm8650-mdss >>> can then list interconnects ? >> IMO yes (after adjusting above to coding style), but as mentioned in >> other response you can just get an ack or opinion from Rob or Conor. > > But does that work with *any* device that requires interconnects? The next such simple-framebuffer device should work out of the box *without* the kernel knowing anything about it. That's one of the key features of the simple-framebuffer. If we have to maintainer per-device feature sets, it breaks that assumption. The driver code for this can still be generic and since the driver will bind to the fallback plain "simple-framebuffer" compatible this should also work for new platforms. The e.g. "qcom.simple-framebuffer-sm8650-mdss" compatible would purely be something in the dt-bindings to document which simplefb implementations will have interconnects and which ones will not. The driver does not necessarily need to check these more precise compatibles, it can still just check for the generic presence of interconnects. Regards, Hans
On Thu, Jul 03, 2025 at 10:34:23AM +0200, Hans de Goede wrote: > Hi Thomas, > > On 3-Jul-25 8:47 AM, Thomas Zimmermann wrote: > > Hi > > > > Am 02.07.25 um 22:43 schrieb Krzysztof Kozlowski: > >> On 30/06/2025 10:40, Hans de Goede wrote: > >>>> No one asks to drop them from the driver. I only want specific front > >>>> compatible which will list and constrain the properties. It is not > >>>> contradictory to your statements, U-boot support, driver support. I > >>>> really do not see ANY argument why this cannot follow standard DT rules. > >>> So what you are saying is that you want something like: > >>> > >>> framebuffer0: framebuffer@1d385000 { > >>> compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; > >>> } > >>> > >>> and that the binding for qcom.simple-framebuffer-sm8650-mdss > >>> can then list interconnects ? > >> IMO yes (after adjusting above to coding style), but as mentioned in > >> other response you can just get an ack or opinion from Rob or Conor. > > > > But does that work with *any* device that requires interconnects? The next such simple-framebuffer device should work out of the box *without* the kernel knowing anything about it. That's one of the key features of the simple-framebuffer. If we have to maintainer per-device feature sets, it breaks that assumption. > > The driver code for this can still be generic and since the driver > will bind to the fallback plain "simple-framebuffer" compatible > this should also work for new platforms. > > The e.g. "qcom.simple-framebuffer-sm8650-mdss" compatible would > purely be something in the dt-bindings to document which simplefb > implementations will have interconnects and which ones will not. > > The driver does not necessarily need to check these more > precise compatibles, it can still just check for the generic > presence of interconnects. This ship has kind of sailed though. This binding has been used by plenty of firmwares and bootloaders over the years, and has been deployed on plenty of devices already. Good luck fixing it in all of them, and then updating every device. Maxime
On 03/07/2025 11:41, Maxime Ripard wrote: >>> But does that work with *any* device that requires interconnects? The next such simple-framebuffer device should work out of the box *without* the kernel knowing anything about it. That's one of the key features of the simple-framebuffer. If we have to maintainer per-device feature sets, it breaks that assumption. >> >> The driver code for this can still be generic and since the driver >> will bind to the fallback plain "simple-framebuffer" compatible >> this should also work for new platforms. >> >> The e.g. "qcom.simple-framebuffer-sm8650-mdss" compatible would >> purely be something in the dt-bindings to document which simplefb >> implementations will have interconnects and which ones will not. >> >> The driver does not necessarily need to check these more >> precise compatibles, it can still just check for the generic >> presence of interconnects. > > This ship has kind of sailed though. This binding has been used by > plenty of firmwares and bootloaders over the years, and has been > deployed on plenty of devices already. > > Good luck fixing it in all of them, and then updating every device. No one suggested that... We speak about new devices, although maybe this one SM7635 new device runs plenty of firmwares and bootloaders? Best regards, Krzysztof
Hi Am 03.07.25 um 10:34 schrieb Hans de Goede: > Hi Thomas, > > On 3-Jul-25 8:47 AM, Thomas Zimmermann wrote: >> Hi >> >> Am 02.07.25 um 22:43 schrieb Krzysztof Kozlowski: >>> On 30/06/2025 10:40, Hans de Goede wrote: >>>>> No one asks to drop them from the driver. I only want specific front >>>>> compatible which will list and constrain the properties. It is not >>>>> contradictory to your statements, U-boot support, driver support. I >>>>> really do not see ANY argument why this cannot follow standard DT rules. >>>> So what you are saying is that you want something like: >>>> >>>> framebuffer0: framebuffer@1d385000 { >>>> compatible = "qcom.simple-framebuffer-sm8650-mdss", "simple-framebuffer"; >>>> } >>>> >>>> and that the binding for qcom.simple-framebuffer-sm8650-mdss >>>> can then list interconnects ? >>> IMO yes (after adjusting above to coding style), but as mentioned in >>> other response you can just get an ack or opinion from Rob or Conor. >> But does that work with *any* device that requires interconnects? The next such simple-framebuffer device should work out of the box *without* the kernel knowing anything about it. That's one of the key features of the simple-framebuffer. If we have to maintainer per-device feature sets, it breaks that assumption. > The driver code for this can still be generic and since the driver > will bind to the fallback plain "simple-framebuffer" compatible > this should also work for new platforms. > > The e.g. "qcom.simple-framebuffer-sm8650-mdss" compatible would > purely be something in the dt-bindings to document which simplefb > implementations will have interconnects and which ones will not. > > The driver does not necessarily need to check these more > precise compatibles, it can still just check for the generic > presence of interconnects. Thanks, that's good to hear. Best regards Thomas > > 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)
On Mon, Jun 30, 2025 at 10:24:06AM +0200, Krzysztof Kozlowski wrote: > On 29/06/2025 14:07, Hans de Goede wrote: > > Hi Krzysztof, > > > > On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: > >> On 27/06/2025 11:48, Luca Weiss wrote: > >>> Hi Krzysztof, > >>> > >>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: > >>>> 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. > >>> > >>> The bindings support an arbitrary number of clocks, regulators, > >>> power-domains. Why should I artificially limit the interconnects to only > >>> one? > >> > >> And IMO they should not. Bindings are not supposed to be generic. > > > > The simplefb binding is a binding to allow keeping the firmware, e.g. > > uboot setup framebuffer alive to e.g. show a boot splash until > > the native display-engine drive loads. Needing display-engine > > specific bindings totally contradicts the whole goal of > > No, it does not. DT is well designed for that through expressing > compatibility. I did not say you cannot have generic fallback for simple > use case. > > But this (and previous patchset) grows this into generic binding ONLY > and that is not correct. Can we have a proper definition of what a correct device tree binding is then? It's a bit surprising to have *that* discussion over a binding that is now well older than a decade now, and while there is definitely some generic bindings in ePAPR/DT spec, like the CPU ones. If you don't consider that spec to be correct DT bindings, please provide a definition of what that is, and / or reasonable alternatives. Also, no, a device specific binding isn't reasonable here, because we *don't* have a device. From a technical standpoint, the firmware creates the framebuffer, Linux just uses it. Just like you don't have a device/platform specific compatible for PSCI, SCPI, et al. And from a process standpoint, that driver is typically used years before we even get to writing a driver for the actual display driver. And since bindings are far from standard and actually pretty opionionated, even if we submitted a binding to use a proper binding without having a clear idea of what the hardware is, or what a driver would want, we would end up with either a broken binding, or a broken driver. Maxime
On 30/06/2025 10:38, Maxime Ripard wrote: > On Mon, Jun 30, 2025 at 10:24:06AM +0200, Krzysztof Kozlowski wrote: >> On 29/06/2025 14:07, Hans de Goede wrote: >>> Hi Krzysztof, >>> >>> On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: >>>> On 27/06/2025 11:48, Luca Weiss wrote: >>>>> Hi Krzysztof, >>>>> >>>>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>>>>> 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. >>>>> >>>>> The bindings support an arbitrary number of clocks, regulators, >>>>> power-domains. Why should I artificially limit the interconnects to only >>>>> one? >>>> >>>> And IMO they should not. Bindings are not supposed to be generic. >>> >>> The simplefb binding is a binding to allow keeping the firmware, e.g. >>> uboot setup framebuffer alive to e.g. show a boot splash until >>> the native display-engine drive loads. Needing display-engine >>> specific bindings totally contradicts the whole goal of >> >> No, it does not. DT is well designed for that through expressing >> compatibility. I did not say you cannot have generic fallback for simple >> use case. >> >> But this (and previous patchset) grows this into generic binding ONLY >> and that is not correct. > > Can we have a proper definition of what a correct device tree binding is > then? > > It's a bit surprising to have *that* discussion over a binding that is > now well older than a decade now, and while there is definitely some > generic bindings in ePAPR/DT spec, like the CPU ones. Hm? In ARM world at least they are specific, e.g. they have specific compatibles. > > If you don't consider that spec to be correct DT bindings, please > provide a definition of what that is, and / or reasonable alternatives. > > Also, no, a device specific binding isn't reasonable here, because we > *don't* have a device. From a technical standpoint, the firmware creates You touch internal parts of the SoC and you list very specific SoC parts. Interconnect is internal part of the SoC and only specific devices are using it. You define here generic SW construct for something which is opposite of generic: the interconnect connecting two specific, unique components of one, given SoC. > the framebuffer, Linux just uses it. Just like you don't have a > device/platform specific compatible for PSCI, SCPI, et al. They follow some sort of spec and still they do not reference chosen SoC-design-specific properties. Best regards, Krzysztof
On Mon, Jun 30, 2025 at 11:36:51AM +0200, Krzysztof Kozlowski wrote: > On 30/06/2025 10:38, Maxime Ripard wrote: > > On Mon, Jun 30, 2025 at 10:24:06AM +0200, Krzysztof Kozlowski wrote: > >> On 29/06/2025 14:07, Hans de Goede wrote: > >>> Hi Krzysztof, > >>> > >>> On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: > >>>> On 27/06/2025 11:48, Luca Weiss wrote: > >>>>> Hi Krzysztof, > >>>>> > >>>>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: > >>>>>> 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. > >>>>> > >>>>> The bindings support an arbitrary number of clocks, regulators, > >>>>> power-domains. Why should I artificially limit the interconnects to only > >>>>> one? > >>>> > >>>> And IMO they should not. Bindings are not supposed to be generic. > >>> > >>> The simplefb binding is a binding to allow keeping the firmware, e.g. > >>> uboot setup framebuffer alive to e.g. show a boot splash until > >>> the native display-engine drive loads. Needing display-engine > >>> specific bindings totally contradicts the whole goal of > >> > >> No, it does not. DT is well designed for that through expressing > >> compatibility. I did not say you cannot have generic fallback for simple > >> use case. > >> > >> But this (and previous patchset) grows this into generic binding ONLY > >> and that is not correct. > > > > Can we have a proper definition of what a correct device tree binding is > > then? > > > > It's a bit surprising to have *that* discussion over a binding that is > > now well older than a decade now, and while there is definitely some > > generic bindings in ePAPR/DT spec, like the CPU ones. > > Hm? In ARM world at least they are specific, e.g. they have specific > compatibles. > > > > > If you don't consider that spec to be correct DT bindings, please > > provide a definition of what that is, and / or reasonable alternatives. > > > > Also, no, a device specific binding isn't reasonable here, because we > > *don't* have a device. From a technical standpoint, the firmware creates > > You touch internal parts of the SoC and you list very specific SoC > parts. Interconnect is internal part of the SoC and only specific > devices are using it. > > You define here generic SW construct for something which is opposite of > generic: the interconnect connecting two specific, unique components of > one, given SoC. > > > the framebuffer, Linux just uses it. Just like you don't have a > > device/platform specific compatible for PSCI, SCPI, et al. > > They follow some sort of spec and still they do not reference chosen > SoC-design-specific properties. ish. I mean, on theory, you're absolutely correct. In practice, assigned-clock-parents, assigned-clock-rates, or protected-clocks for example exist and are *only* about SoC-design specific behaviours. Maxime
Hi Krzysztof, On 30-Jun-25 11:36 AM, Krzysztof Kozlowski wrote: > On 30/06/2025 10:38, Maxime Ripard wrote: >> On Mon, Jun 30, 2025 at 10:24:06AM +0200, Krzysztof Kozlowski wrote: >>> On 29/06/2025 14:07, Hans de Goede wrote: >>>> Hi Krzysztof, >>>> >>>> On 28-Jun-25 1:49 PM, Krzysztof Kozlowski wrote: >>>>> On 27/06/2025 11:48, Luca Weiss wrote: >>>>>> Hi Krzysztof, >>>>>> >>>>>> On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >>>>>>> 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. >>>>>> >>>>>> The bindings support an arbitrary number of clocks, regulators, >>>>>> power-domains. Why should I artificially limit the interconnects to only >>>>>> one? >>>>> >>>>> And IMO they should not. Bindings are not supposed to be generic. >>>> >>>> The simplefb binding is a binding to allow keeping the firmware, e.g. >>>> uboot setup framebuffer alive to e.g. show a boot splash until >>>> the native display-engine drive loads. Needing display-engine >>>> specific bindings totally contradicts the whole goal of >>> >>> No, it does not. DT is well designed for that through expressing >>> compatibility. I did not say you cannot have generic fallback for simple >>> use case. >>> >>> But this (and previous patchset) grows this into generic binding ONLY >>> and that is not correct. >> >> Can we have a proper definition of what a correct device tree binding is >> then? >> >> It's a bit surprising to have *that* discussion over a binding that is >> now well older than a decade now, and while there is definitely some >> generic bindings in ePAPR/DT spec, like the CPU ones. > > Hm? In ARM world at least they are specific, e.g. they have specific > compatibles. > >> >> If you don't consider that spec to be correct DT bindings, please >> provide a definition of what that is, and / or reasonable alternatives. >> >> Also, no, a device specific binding isn't reasonable here, because we >> *don't* have a device. From a technical standpoint, the firmware creates > > You touch internal parts of the SoC and you list very specific SoC > parts. Interconnect is internal part of the SoC and only specific > devices are using it. > > You define here generic SW construct for something which is opposite of > generic: the interconnect connecting two specific, unique components of > one, given SoC. > >> the framebuffer, Linux just uses it. Just like you don't have a >> device/platform specific compatible for PSCI, SCPI, et al. > > They follow some sort of spec and still they do not reference chosen > SoC-design-specific properties. It does not look like this discussion is going anywhere, despite 2 drm subsystem maintainers and the simplefb maintainer telling you that this is what is necessary and also that we believe this is the right thing todo. IOW despite 3 domain experts telling you we want this, you keep coming up with vague, not really technical argument about this not being generic / simple enough. Looking at this from a driver pov interconnects are just another resource we need to avoid from turning off. And this is simple and generic, the actual display-engine drivers are very complex and when powering things up this needs to be done in a very specific order with specific delays. That is hw-specific. The simplefb/simpledrm code does not need any of this knowledge everything is already setup. The simple* drivers just needs to claim all listed resources in an arbitrary order and without any delays as someone who has written many many drivers this is about as simple and generic as it can get. But as mentioned it looks like this discussion is going anywhere. Is there some sort of arbitration / appeal process which we can use when DT-maintainers block a binding which has been acked and is seen as necessary by the subsystem maintainers of the subsystem for which the bindings are ? Regards, Hans
"Luca Weiss" <luca.weiss@fairphone.com> writes: > Hi Krzysztof, > > On Fri Jun 27, 2025 at 10:08 AM CEST, Krzysztof Kozlowski wrote: >> 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. > > The bindings support an arbitrary number of clocks, regulators, > power-domains. Why should I artificially limit the interconnects to only > one? > I agree with Luca here. There are device specific bindings for the device specific drivers. But this is about the generic drivers that are able to scan out using a system provided framebuffer. The display controller is setup by the firmware but it might need a set of clocks, power domains, regulators, etc left enabled in order to work. It's true that the "simple" is a misnomer, probably these drivers should had been named sysfb and sysfbdrm, or something along those lines. > The driver code also has that support added in this series. > > Regards > Luca > >> >> Best regards, >> Krzysztof > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Luca Weiss <luca.weiss@fairphone.com> writes: Hello Luca, > 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> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
© 2016 - 2025 Red Hat, Inc.