* Re: [PATCH 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Maxime Ripard @ 2014-11-18 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <546B0026.2010107@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 4765 bytes --]
On Tue, Nov 18, 2014 at 09:15:34AM +0100, Hans de Goede wrote:
> Hi,
>
> On 11/17/2014 04:55 PM, Maxime Ripard wrote:
> > On Mon, Nov 17, 2014 at 01:39:18PM +0000, Grant Likely wrote:
> >> On Mon, Nov 17, 2014 at 12:47 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >>> On Mon, Nov 17, 2014 at 12:34:46PM +0100, Hans de Goede wrote:
> >>>> If pre-filled framebuffer nodes are used, the firmware may need extra
> >>>> properties to find the right node. This documents the properties to use
> >>>> for this on sunxi platforms.
> >>>>
> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>> ---
> >>>> .../bindings/video/simple-framebuffer-sunxi.txt | 33 ++++++++++++++++++++++
> >>>> 1 file changed, 33 insertions(+)
> >>>> create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >>>> new file mode 100644
> >>>> index 0000000..84ca264
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >>>> @@ -0,0 +1,33 @@
> >>>> +Sunxi specific Simple Framebuffer bindings
> >>>> +
> >>>> +This binding documents sunxi specific extensions to the simple-framebuffer
> >>>> +bindings. The sunxi simplefb u-boot code relies on the devicetree containing
> >>>> +pre-populated simplefb nodes.
> >>>> +
> >>>> +These extensions are intended so that u-boot can select the right node based
> >>>> +on which pipeline and output is being used. As such they are solely intended
> >>>> +for firmware / bootloader use, and the OS should ignore them.
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: "sunxi,framebuffer"
> >>>> +- sunxi,pipeline:
> >>>> + <0> for the de_be0 -> lcdc0 -> output pipeline
> >>>> + <1> for the de_be1 -> lcdc1 -> output pipeline
> >>>> +- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
> >>>> +
> >>>> +Example:
> >>>> +
> >>>> +chosen {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <1>;
> >>>> + ranges;
> >>>> +
> >>>> + framebuffer@0 {
> >>>> + compatible = "sunxi,framebuffer", "simple-framebuffer";
> >>>> + sunxi,pipeline = <0>;
> >>>> + sunxi,output = "hdmi";
> >>>> + clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
> >>>> + <&ahb_gates 44>;
> >>>
> >>> If we're going that way, then maybe having to specify clock-names
> >>> would be better in order to know which clock is what?
> >>
> >> I wouldn't go that way with this binding since the driver has no need
> >> to differentiate between the clocks, and driver authors shouldn't be
> >> encouraged to do so. The purpose of the clocks in this node is only
> >> for itemizing dependencies, not for how to configure the clocks.
> >> Firmware shouldn't care at all about the clocks list, it only needs to
> >> find the correct pre-populated node to fill in and enable.
> >
> > Well, if we want to play the "DT as an ABI" stuff, you have no
> > guarantee that in the future, simplefb will still be the driver bound
> > to "sunxi,framebuffer"
>
> Huh? We do have that guarantee, or at least that a simplefb bindings
> compatible driver is, the "sunxi,framebuffer" bindings live in:
>
> Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>
> With the filename already being a big hint, further more it states that:
Yeah, well, that can change. The compatible itself can't, but we're
free to move around the drivers however we see fit (as long as the
user-space is not impacted)
> "This binding documents sunxi specific extensions to the simple-framebuffer
> bindings. The sunxi simplefb u-boot code relies on the devicetree containing
> pre-populated simplefb nodes.
>
> These extensions are intended so that u-boot can select the right node based
> on which pipeline is being used. As such they are solely intended for
> firmware / bootloader use, and the OS should ignore them."
>
> Which to me seems to guarantee that we're talking about a simple-framebuffer
> compatible node here. The scope of these extensions are very narrow, there
> only purpose is to allow the bootloader to find the right pre-populated simplefb
> node, everything else is specified by the simplefb bindings, not these
> extensions.
My point was that what we might need to know these clocks at some
point in the future. But if you don't believe we will, it's fine for
me.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Thierry Reding @ 2014-11-18 10:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdXZ4V4GHvyDLOP8FYvPuGArc5G0aO6NCE9FKHT24YRGPg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]
On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> >> -module_init(simplefb_init);
> >> +/*
> >> + * While this can be a module, if builtin it's most likely the console
> >> + * So let's leave module_exit but move module_init to an earlier place
> >> + */
> >
> > Not really related to this patch itself, but do we want to support
> > simplefb as a module? It seems like it's going to be most of the time
> > broken.
>
> If it depends on clocks, it won't work as a module, as CCF will have disabled
> all unused clocks at that point.
If it does depend on anything beyond clocks it won't work at all. Clocks
are special because they get set up very early at boot time. If it turns
out that a simplefb ever needs a regulator to remain on, and that's even
quite likely to happen eventually, it's going to fail miserably, because
those regulators will typically be provided by a PMIC on an I2C bus. The
regulator won't be registered until very late into the boot process and
a regulator_get() call will almost certainly cause the simplefb driver
to defer probing.
Now deferring probing is a real showstopper for simplefb, because not
only does it make the framebuffer useless as early boot console, once
probing is attempted again the clocks that it would have needed to
acquire to keep going will already have been switched off, too.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Hans de Goede @ 2014-11-18 11:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141118101911.GA15769@ulmo>
Hi,
On 11/18/2014 11:19 AM, Thierry Reding wrote:
> On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
>> Hi Maxime,
>>
>> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>>>> -module_init(simplefb_init);
>>>> +/*
>>>> + * While this can be a module, if builtin it's most likely the console
>>>> + * So let's leave module_exit but move module_init to an earlier place
>>>> + */
>>>
>>> Not really related to this patch itself, but do we want to support
>>> simplefb as a module? It seems like it's going to be most of the time
>>> broken.
>>
>> If it depends on clocks, it won't work as a module, as CCF will have disabled
>> all unused clocks at that point.
>
> If it does depend on anything beyond clocks it won't work at all. Clocks
> are special because they get set up very early at boot time. If it turns
> out that a simplefb ever needs a regulator to remain on, and that's even
> quite likely to happen eventually, it's going to fail miserably, because
> those regulators will typically be provided by a PMIC on an I2C bus. The
> regulator won't be registered until very late into the boot process and
> a regulator_get() call will almost certainly cause the simplefb driver
> to defer probing.
Right, this has been discussed already and the plan is to have simplefb
continue its probe function and return success from it if it encounters
any -eprobe_defer errors, while tracking which resources it misses.
And then have a late_initcall which will claim any resources which failed
with -eprobe beforehand.
> Now deferring probing is a real showstopper for simplefb, because not
> only does it make the framebuffer useless as early boot console, once
> probing is attempted again the clocks that it would have needed to
> acquire to keep going will already have been switched off, too.
That is not true, even with the current implementation, if all necessary
drivers are built in, then simplefb will come up later, but it will still
come up before the late_initcall which disables the clocks.
Once we do the split probing described above (which is something which
we plan to do when it becomes necessary), then simplefb will still come
up early.
Regards,
Hans
^ permalink raw reply
* [PATCH v3 0/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes
From: Hans de Goede @ 2014-11-18 11:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomi,
So it turns out we needed a v3, as I mistakenly went with "sunxi," as vendor
prefix for the allwinner specific properties, but the registered vendor prefix
for allwinner is "allwinnner,", this version fixes this.
Changes in v3: Use proper "allwinnner," for the compatible string and vendor
specific properties.
Changes in v2: Changed the simplefb-sunxi bindings to use a single
sunxi,pipeline string property instead of a sunxi,pipeline int and a
sunxi,output string property.
This patch set is all acked up, and a fix to the binding changes you've
already queued for 3.19, so please queue these 2 patches for 3.19.
Thanks & Regards,
Hans
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: simplefb: Drop the advice about using a specific path for nodes
From: Hans de Goede @ 2014-11-18 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1416309051-26784-1-git-send-email-hdegoede@redhat.com>
This goes contrary to how devicetree usually works, so drop it. Instead if
the firmware needs to be able to find a specific node it should use a
platform specific compatible + properties for this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
Documentation/devicetree/bindings/video/simple-framebuffer.txt | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
index f8fb7e6..4474ef6 100644
--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
@@ -31,12 +31,10 @@ enable them. This way if e.g. later on support for more display clocks get
added, the simplefb nodes will already contain this info and the firmware
does not need to be updated.
-If pre-filled framebuffer nodes are used, they should be named
-"framebuffer#-<output>", e.g. "framebuffer0-hdmi". The output should be
-included in the name since different outputs typically require different
-clocks and the clocks are part of the pre-populated nodes. The firmware must
-rename the nodes to the standard "framebuffer@<address>" name using the
-runtime chosen address when enabling the nodes.
+If pre-filled framebuffer nodes are used, the firmware may need extra
+information to find the right node. In that case an extra platform specific
+compatible and platform specific properties should be used and documented,
+see e.g. simple-framebuffer-sunxi.txt .
Required properties:
- compatible: "simple-framebuffer"
--
2.1.0
^ permalink raw reply related
* [PATCH v3 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Hans de Goede @ 2014-11-18 11:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1416309051-26784-1-git-send-email-hdegoede@redhat.com>
If pre-filled framebuffer nodes are used, the firmware may need extra
properties to find the right node. This documents the properties to use
for this on sunxi platforms.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
.../bindings/video/simple-framebuffer-sunxi.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
new file mode 100644
index 0000000..c46ba64
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
@@ -0,0 +1,33 @@
+Sunxi specific Simple Framebuffer bindings
+
+This binding documents sunxi specific extensions to the simple-framebuffer
+bindings. The sunxi simplefb u-boot code relies on the devicetree containing
+pre-populated simplefb nodes.
+
+These extensions are intended so that u-boot can select the right node based
+on which pipeline is being used. As such they are solely intended for
+firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "allwinner,simple-framebuffer"
+- allwinner,pipeline, one of:
+ "de_be0-lcd0"
+ "de_be1-lcd1"
+ "de_be0-lcd0-hdmi"
+ "de_be1-lcd1-hdmi"
+
+Example:
+
+chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer@0 {
+ compatible = "allwinner,simple-framebuffer", "simple-framebuffer";
+ allwinner,pipeline = "de_be0-lcd0-hdmi";
+ clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
+ <&ahb_gates 44>;
+ status = "disabled";
+ };
+};
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Thierry Reding @ 2014-11-18 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <546B26F4.50302@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3375 bytes --]
On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:
> Hi,
>
> On 11/18/2014 11:19 AM, Thierry Reding wrote:
> > On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
> >> Hi Maxime,
> >>
> >> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >>>> -module_init(simplefb_init);
> >>>> +/*
> >>>> + * While this can be a module, if builtin it's most likely the console
> >>>> + * So let's leave module_exit but move module_init to an earlier place
> >>>> + */
> >>>
> >>> Not really related to this patch itself, but do we want to support
> >>> simplefb as a module? It seems like it's going to be most of the time
> >>> broken.
> >>
> >> If it depends on clocks, it won't work as a module, as CCF will have disabled
> >> all unused clocks at that point.
> >
> > If it does depend on anything beyond clocks it won't work at all. Clocks
> > are special because they get set up very early at boot time. If it turns
> > out that a simplefb ever needs a regulator to remain on, and that's even
> > quite likely to happen eventually, it's going to fail miserably, because
> > those regulators will typically be provided by a PMIC on an I2C bus. The
> > regulator won't be registered until very late into the boot process and
> > a regulator_get() call will almost certainly cause the simplefb driver
> > to defer probing.
>
> Right, this has been discussed already and the plan is to have simplefb
> continue its probe function and return success from it if it encounters
> any -eprobe_defer errors, while tracking which resources it misses.
>
> And then have a late_initcall which will claim any resources which failed
> with -eprobe beforehand.
How do you ensure that the late_initcall gets run before any of the
other late_initcalls that disable the resources? Also my recollection is
that deferred probing will first be triggered the first time from a
late_initcall, so chances aren't very high that all resources have shown
up by that time.
> > Now deferring probing is a real showstopper for simplefb, because not
> > only does it make the framebuffer useless as early boot console, once
> > probing is attempted again the clocks that it would have needed to
> > acquire to keep going will already have been switched off, too.
>
> That is not true, even with the current implementation, if all necessary
> drivers are built in, then simplefb will come up later, but it will still
> come up before the late_initcall which disables the clocks.
Yes, in the current implementation because clocks typically are
registered very early and thus you don't hit the deferred probe. The
same is not true for other types of resources where it's actually quite
common to hit deferred probing (regulators is a very notorious one).
It doesn't matter whether a driver is built-in or not, once you hit
deferred probing you lose.
> Once we do the split probing described above (which is something which
> we plan to do when it becomes necessary), then simplefb will still come
> up early.
It will come up early but won't have acquired all the resources that it
needs, so unless you somehow manage to order late_initcalls in exactly
the way that you need them, the frameworks will still turn off what you
haven't managed to claim yet.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Hans de Goede @ 2014-11-18 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141118112123.GA3873@ulmo>
Hi,
On 11/18/2014 12:21 PM, Thierry Reding wrote:
> On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 11/18/2014 11:19 AM, Thierry Reding wrote:
>>> On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
>>>> Hi Maxime,
>>>>
>>>> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>> -module_init(simplefb_init);
>>>>>> +/*
>>>>>> + * While this can be a module, if builtin it's most likely the console
>>>>>> + * So let's leave module_exit but move module_init to an earlier place
>>>>>> + */
>>>>>
>>>>> Not really related to this patch itself, but do we want to support
>>>>> simplefb as a module? It seems like it's going to be most of the time
>>>>> broken.
>>>>
>>>> If it depends on clocks, it won't work as a module, as CCF will have disabled
>>>> all unused clocks at that point.
>>>
>>> If it does depend on anything beyond clocks it won't work at all. Clocks
>>> are special because they get set up very early at boot time. If it turns
>>> out that a simplefb ever needs a regulator to remain on, and that's even
>>> quite likely to happen eventually, it's going to fail miserably, because
>>> those regulators will typically be provided by a PMIC on an I2C bus. The
>>> regulator won't be registered until very late into the boot process and
>>> a regulator_get() call will almost certainly cause the simplefb driver
>>> to defer probing.
>>
>> Right, this has been discussed already and the plan is to have simplefb
>> continue its probe function and return success from it if it encounters
>> any -eprobe_defer errors, while tracking which resources it misses.
>>
>> And then have a late_initcall which will claim any resources which failed
>> with -eprobe beforehand.
>
> How do you ensure that the late_initcall gets run before any of the
> other late_initcalls that disable the resources?
> Also my recollection is
> that deferred probing will first be triggered the first time from a
> late_initcall, so chances aren't very high that all resources have shown
> up by that time.
So I just looked up the relevant code, and your right, this means that
the whole model of "disable unused resources once probing is done" which
we use for e.g. clocks, is already somewhat broken since there is
no guarantee probing is really done when the cleanup code runs.
>>> Now deferring probing is a real showstopper for simplefb, because not
>>> only does it make the framebuffer useless as early boot console, once
>>> probing is attempted again the clocks that it would have needed to
>>> acquire to keep going will already have been switched off, too.
>>
>> That is not true, even with the current implementation, if all necessary
>> drivers are built in, then simplefb will come up later, but it will still
>> come up before the late_initcall which disables the clocks.
>
> Yes, in the current implementation because clocks typically are
> registered very early and thus you don't hit the deferred probe. The
> same is not true for other types of resources where it's actually quite
> common to hit deferred probing (regulators is a very notorious one).
>
> It doesn't matter whether a driver is built-in or not, once you hit
> deferred probing you lose.
>
>> Once we do the split probing described above (which is something which
>> we plan to do when it becomes necessary), then simplefb will still come
>> up early.
>
> It will come up early but won't have acquired all the resources that it
> needs, so unless you somehow manage to order late_initcalls in exactly
> the way that you need them, the frameworks will still turn off what you
> haven't managed to claim yet.
If it is a resource which only shows up as a result of deferred probing,
then it may very well not have been probed & registered yet, when the
framework cleanup functions runs, and thus will not get turned off...
So yes you're right that deferred probing may cause issues, but it seems
that this is not something simplefb specific, but rather a generic problem
with deferred-probing vs subsys cleanup functions.
My view on this is simple, lets worry about this when we actually have
a board which hits these issues, and then we'll see from there.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Hans de Goede @ 2014-11-18 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <546B31A7.9020804@redhat.com>
Hi,
On 11/18/2014 12:46 PM, Hans de Goede wrote:
> Hi,
>
> On 11/18/2014 12:21 PM, Thierry Reding wrote:
>> On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 11/18/2014 11:19 AM, Thierry Reding wrote:
>>>> On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
>>>>> Hi Maxime,
>>>>>
>>>>> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
>>>>> <maxime.ripard@free-electrons.com> wrote:
>>>>>>> -module_init(simplefb_init);
>>>>>>> +/*
>>>>>>> + * While this can be a module, if builtin it's most likely the console
>>>>>>> + * So let's leave module_exit but move module_init to an earlier place
>>>>>>> + */
>>>>>>
>>>>>> Not really related to this patch itself, but do we want to support
>>>>>> simplefb as a module? It seems like it's going to be most of the time
>>>>>> broken.
>>>>>
>>>>> If it depends on clocks, it won't work as a module, as CCF will have disabled
>>>>> all unused clocks at that point.
>>>>
>>>> If it does depend on anything beyond clocks it won't work at all. Clocks
>>>> are special because they get set up very early at boot time. If it turns
>>>> out that a simplefb ever needs a regulator to remain on, and that's even
>>>> quite likely to happen eventually, it's going to fail miserably, because
>>>> those regulators will typically be provided by a PMIC on an I2C bus. The
>>>> regulator won't be registered until very late into the boot process and
>>>> a regulator_get() call will almost certainly cause the simplefb driver
>>>> to defer probing.
>>>
>>> Right, this has been discussed already and the plan is to have simplefb
>>> continue its probe function and return success from it if it encounters
>>> any -eprobe_defer errors, while tracking which resources it misses.
>>>
>>> And then have a late_initcall which will claim any resources which failed
>>> with -eprobe beforehand.
>>
>> How do you ensure that the late_initcall gets run before any of the
>> other late_initcalls that disable the resources?
>
>> Also my recollection is
>> that deferred probing will first be triggered the first time from a
>> late_initcall, so chances aren't very high that all resources have shown
>> up by that time.
>
> So I just looked up the relevant code, and your right, this means that
> the whole model of "disable unused resources once probing is done" which
> we use for e.g. clocks, is already somewhat broken since there is
> no guarantee probing is really done when the cleanup code runs.
>
>>>> Now deferring probing is a real showstopper for simplefb, because not
>>>> only does it make the framebuffer useless as early boot console, once
>>>> probing is attempted again the clocks that it would have needed to
>>>> acquire to keep going will already have been switched off, too.
>>>
>>> That is not true, even with the current implementation, if all necessary
>>> drivers are built in, then simplefb will come up later, but it will still
>>> come up before the late_initcall which disables the clocks.
>>
>> Yes, in the current implementation because clocks typically are
>> registered very early and thus you don't hit the deferred probe. The
>> same is not true for other types of resources where it's actually quite
>> common to hit deferred probing (regulators is a very notorious one).
>>
>> It doesn't matter whether a driver is built-in or not, once you hit
>> deferred probing you lose.
>>
>>> Once we do the split probing described above (which is something which
>>> we plan to do when it becomes necessary), then simplefb will still come
>>> up early.
>>
>> It will come up early but won't have acquired all the resources that it
>> needs, so unless you somehow manage to order late_initcalls in exactly
>> the way that you need them, the frameworks will still turn off what you
>> haven't managed to claim yet.
>
> If it is a resource which only shows up as a result of deferred probing,
> then it may very well not have been probed & registered yet, when the
> framework cleanup functions runs, and thus will not get turned off...
>
> So yes you're right that deferred probing may cause issues, but it seems
> that this is not something simplefb specific, but rather a generic problem
> with deferred-probing vs subsys cleanup functions.
>
> My view on this is simple, lets worry about this when we actually have
> a board which hits these issues, and then we'll see from there.
So thinking more about this, I think this is not that hard to fix.
First lets fix the generic conflict between eprobedefer and subsys cleanup
functions. This can be done by:
1) Having a linked list of subsys cleanup functions
to call in drivers/base/dd.c
2) Have subsystems register their cleanup function rather then using
late_initcall to get it called
3) Have deferred_probe_work_func iterate over the list and call the cleanup
functions once deferred_probe_active_list goes empty. It should also remove
them once called, so that they only get called the first time
deferred_probe_active_list goes empty (iow when the deferred probing of'
build-in drivers is done).
This way cleanup functions actually get run when all probing of (buildin)
drivers is done.
Then when we move simplefb to a 2 fase probe, we can simply register the
framebuffer at the first probe call, and claim any resources we get, but
return -eprobe_defer if some resources returned that themselves.
Then when simplefb_probe gets re-called, it can first check if it did not
already register a fb, and if it did, it can use that and see which
resources are missing, and only try to claim those. If some resources
still return probe_defer, return eprobe_defer again, otherwise success.
This way simplefb_probe will get called as long as resources are missing
and other drivers are successfully completing deferred probes (if no
driver successfully completes a deferred probe, deferred_probe_active_list
will go empty).
And then when the subsys cleanup functions run, simplefb will have been
able to claim any resources registered by buildin drivers.
Regards,
Hans
^ permalink raw reply
* RE: SSD1306 OLED driver
From: Ssagarr Patil @ 2014-11-18 14:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <BAY176-W246A4D5C3A23D1928FC5D3D38C0@phx.gbl>
Hi Maxime,
>
> On Fri, Nov 14, 2014 at 05:09:09PM +0530, Ssagarr Patil wrote:
>> Hi,
>>
>> I have the SSD1306 oled device connected via spi, will the same
>> driver work ? as the current one is based on i2c what all mods would
>> be required to have it working with SPI ?
>
> It won't work out of the box, but it should be fairly easy to support.
>
> You need to had support for SPI probing, and then provide some generic
> accessors the rest of the code can use to wrap around the i2c and spi
> calls.
>
> I'm not sure regmap can be used for this, but this would be something
> worth looking into to do that.
>
Thanks for the inputs :)
Does the driver work as a standard fbdev driver, is there any library on top
of it to ease the user space ?
Thanks & Regards,
--Sagar
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Thierry Reding @ 2014-11-18 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <546B3F3C.6050202@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6518 bytes --]
On Tue, Nov 18, 2014 at 01:44:44PM +0100, Hans de Goede wrote:
> Hi,
>
> On 11/18/2014 12:46 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 11/18/2014 12:21 PM, Thierry Reding wrote:
> >> On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 11/18/2014 11:19 AM, Thierry Reding wrote:
> >>>> On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
> >>>>> Hi Maxime,
> >>>>>
> >>>>> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
> >>>>> <maxime.ripard@free-electrons.com> wrote:
> >>>>>>> -module_init(simplefb_init);
> >>>>>>> +/*
> >>>>>>> + * While this can be a module, if builtin it's most likely the console
> >>>>>>> + * So let's leave module_exit but move module_init to an earlier place
> >>>>>>> + */
> >>>>>>
> >>>>>> Not really related to this patch itself, but do we want to support
> >>>>>> simplefb as a module? It seems like it's going to be most of the time
> >>>>>> broken.
> >>>>>
> >>>>> If it depends on clocks, it won't work as a module, as CCF will have disabled
> >>>>> all unused clocks at that point.
> >>>>
> >>>> If it does depend on anything beyond clocks it won't work at all. Clocks
> >>>> are special because they get set up very early at boot time. If it turns
> >>>> out that a simplefb ever needs a regulator to remain on, and that's even
> >>>> quite likely to happen eventually, it's going to fail miserably, because
> >>>> those regulators will typically be provided by a PMIC on an I2C bus. The
> >>>> regulator won't be registered until very late into the boot process and
> >>>> a regulator_get() call will almost certainly cause the simplefb driver
> >>>> to defer probing.
> >>>
> >>> Right, this has been discussed already and the plan is to have simplefb
> >>> continue its probe function and return success from it if it encounters
> >>> any -eprobe_defer errors, while tracking which resources it misses.
> >>>
> >>> And then have a late_initcall which will claim any resources which failed
> >>> with -eprobe beforehand.
> >>
> >> How do you ensure that the late_initcall gets run before any of the
> >> other late_initcalls that disable the resources?
> >
> >> Also my recollection is
> >> that deferred probing will first be triggered the first time from a
> >> late_initcall, so chances aren't very high that all resources have shown
> >> up by that time.
> >
> > So I just looked up the relevant code, and your right, this means that
> > the whole model of "disable unused resources once probing is done" which
> > we use for e.g. clocks, is already somewhat broken since there is
> > no guarantee probing is really done when the cleanup code runs.
> >
> >>>> Now deferring probing is a real showstopper for simplefb, because not
> >>>> only does it make the framebuffer useless as early boot console, once
> >>>> probing is attempted again the clocks that it would have needed to
> >>>> acquire to keep going will already have been switched off, too.
> >>>
> >>> That is not true, even with the current implementation, if all necessary
> >>> drivers are built in, then simplefb will come up later, but it will still
> >>> come up before the late_initcall which disables the clocks.
> >>
> >> Yes, in the current implementation because clocks typically are
> >> registered very early and thus you don't hit the deferred probe. The
> >> same is not true for other types of resources where it's actually quite
> >> common to hit deferred probing (regulators is a very notorious one).
> >>
> >> It doesn't matter whether a driver is built-in or not, once you hit
> >> deferred probing you lose.
> >>
> >>> Once we do the split probing described above (which is something which
> >>> we plan to do when it becomes necessary), then simplefb will still come
> >>> up early.
> >>
> >> It will come up early but won't have acquired all the resources that it
> >> needs, so unless you somehow manage to order late_initcalls in exactly
> >> the way that you need them, the frameworks will still turn off what you
> >> haven't managed to claim yet.
> >
> > If it is a resource which only shows up as a result of deferred probing,
> > then it may very well not have been probed & registered yet, when the
> > framework cleanup functions runs, and thus will not get turned off...
> >
> > So yes you're right that deferred probing may cause issues, but it seems
> > that this is not something simplefb specific, but rather a generic problem
> > with deferred-probing vs subsys cleanup functions.
> >
> > My view on this is simple, lets worry about this when we actually have
> > a board which hits these issues, and then we'll see from there.
>
> So thinking more about this, I think this is not that hard to fix.
>
> First lets fix the generic conflict between eprobedefer and subsys cleanup
> functions. This can be done by:
>
> 1) Having a linked list of subsys cleanup functions
> to call in drivers/base/dd.c
>
> 2) Have subsystems register their cleanup function rather then using
> late_initcall to get it called
>
> 3) Have deferred_probe_work_func iterate over the list and call the cleanup
> functions once deferred_probe_active_list goes empty. It should also remove
> them once called, so that they only get called the first time
> deferred_probe_active_list goes empty (iow when the deferred probing of'
> build-in drivers is done).
>
> This way cleanup functions actually get run when all probing of (buildin)
> drivers is done.
That sounds like a reasonable solution to me.
> Then when we move simplefb to a 2 fase probe, we can simply register the
> framebuffer at the first probe call, and claim any resources we get, but
> return -eprobe_defer if some resources returned that themselves.
>
> Then when simplefb_probe gets re-called, it can first check if it did not
> already register a fb, and if it did, it can use that and see which
> resources are missing, and only try to claim those. If some resources
> still return probe_defer, return eprobe_defer again, otherwise success.
I'm not sure you can check that the fb was already registered. When the
driver probe fails the driver core will pretty much remove any trace of
the driver, so checking for an already-reagistered framebuffer isn't
going to be easy.
But if you can manage to make that work it should give us a pretty good
solution that should be easy to make use of by other drivers as well.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: SSD1306 OLED driver
From: maxime.ripard @ 2014-11-18 15:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <BAY176-W246A4D5C3A23D1928FC5D3D38C0@phx.gbl>
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
On Tue, Nov 18, 2014 at 08:29:51PM +0530, Ssagarr Patil wrote:
> Hi Maxime,
>
> >
> > On Fri, Nov 14, 2014 at 05:09:09PM +0530, Ssagarr Patil wrote:
> >> Hi,
> >>
> >> I have the SSD1306 oled device connected via spi, will the same
> >> driver work ? as the current one is based on i2c what all mods would
> >> be required to have it working with SPI ?
> >
> > It won't work out of the box, but it should be fairly easy to support.
> >
> > You need to had support for SPI probing, and then provide some generic
> > accessors the rest of the code can use to wrap around the i2c and spi
> > calls.
> >
> > I'm not sure regmap can be used for this, but this would be something
> > worth looking into to do that.
> >
> Thanks for the inputs :)
>
> Does the driver work as a standard fbdev driver, is there any library on top
> of it to ease the user space ?
It exposes a monochrome fbdev, so the usual framebuffer tools and
libraries are supposed to work quite well, provided they've been
written to handle monochrome displays.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Maxime Ripard @ 2014-11-18 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141118151603.GA19426@ulmo>
[-- Attachment #1: Type: text/plain, Size: 7185 bytes --]
On Tue, Nov 18, 2014 at 04:16:04PM +0100, Thierry Reding wrote:
> On Tue, Nov 18, 2014 at 01:44:44PM +0100, Hans de Goede wrote:
> > Hi,
> >
> > On 11/18/2014 12:46 PM, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 11/18/2014 12:21 PM, Thierry Reding wrote:
> > >> On Tue, Nov 18, 2014 at 12:01:08PM +0100, Hans de Goede wrote:
> > >>> Hi,
> > >>>
> > >>> On 11/18/2014 11:19 AM, Thierry Reding wrote:
> > >>>> On Thu, Nov 13, 2014 at 09:58:41AM +0100, Geert Uytterhoeven wrote:
> > >>>>> Hi Maxime,
> > >>>>>
> > >>>>> On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
> > >>>>> <maxime.ripard@free-electrons.com> wrote:
> > >>>>>>> -module_init(simplefb_init);
> > >>>>>>> +/*
> > >>>>>>> + * While this can be a module, if builtin it's most likely the console
> > >>>>>>> + * So let's leave module_exit but move module_init to an earlier place
> > >>>>>>> + */
> > >>>>>>
> > >>>>>> Not really related to this patch itself, but do we want to support
> > >>>>>> simplefb as a module? It seems like it's going to be most of the time
> > >>>>>> broken.
> > >>>>>
> > >>>>> If it depends on clocks, it won't work as a module, as CCF will have disabled
> > >>>>> all unused clocks at that point.
> > >>>>
> > >>>> If it does depend on anything beyond clocks it won't work at all. Clocks
> > >>>> are special because they get set up very early at boot time. If it turns
> > >>>> out that a simplefb ever needs a regulator to remain on, and that's even
> > >>>> quite likely to happen eventually, it's going to fail miserably, because
> > >>>> those regulators will typically be provided by a PMIC on an I2C bus. The
> > >>>> regulator won't be registered until very late into the boot process and
> > >>>> a regulator_get() call will almost certainly cause the simplefb driver
> > >>>> to defer probing.
> > >>>
> > >>> Right, this has been discussed already and the plan is to have simplefb
> > >>> continue its probe function and return success from it if it encounters
> > >>> any -eprobe_defer errors, while tracking which resources it misses.
> > >>>
> > >>> And then have a late_initcall which will claim any resources which failed
> > >>> with -eprobe beforehand.
> > >>
> > >> How do you ensure that the late_initcall gets run before any of the
> > >> other late_initcalls that disable the resources?
> > >
> > >> Also my recollection is
> > >> that deferred probing will first be triggered the first time from a
> > >> late_initcall, so chances aren't very high that all resources have shown
> > >> up by that time.
> > >
> > > So I just looked up the relevant code, and your right, this means that
> > > the whole model of "disable unused resources once probing is done" which
> > > we use for e.g. clocks, is already somewhat broken since there is
> > > no guarantee probing is really done when the cleanup code runs.
> > >
> > >>>> Now deferring probing is a real showstopper for simplefb, because not
> > >>>> only does it make the framebuffer useless as early boot console, once
> > >>>> probing is attempted again the clocks that it would have needed to
> > >>>> acquire to keep going will already have been switched off, too.
> > >>>
> > >>> That is not true, even with the current implementation, if all necessary
> > >>> drivers are built in, then simplefb will come up later, but it will still
> > >>> come up before the late_initcall which disables the clocks.
> > >>
> > >> Yes, in the current implementation because clocks typically are
> > >> registered very early and thus you don't hit the deferred probe. The
> > >> same is not true for other types of resources where it's actually quite
> > >> common to hit deferred probing (regulators is a very notorious one).
> > >>
> > >> It doesn't matter whether a driver is built-in or not, once you hit
> > >> deferred probing you lose.
> > >>
> > >>> Once we do the split probing described above (which is something which
> > >>> we plan to do when it becomes necessary), then simplefb will still come
> > >>> up early.
> > >>
> > >> It will come up early but won't have acquired all the resources that it
> > >> needs, so unless you somehow manage to order late_initcalls in exactly
> > >> the way that you need them, the frameworks will still turn off what you
> > >> haven't managed to claim yet.
> > >
> > > If it is a resource which only shows up as a result of deferred probing,
> > > then it may very well not have been probed & registered yet, when the
> > > framework cleanup functions runs, and thus will not get turned off...
> > >
> > > So yes you're right that deferred probing may cause issues, but it seems
> > > that this is not something simplefb specific, but rather a generic problem
> > > with deferred-probing vs subsys cleanup functions.
> > >
> > > My view on this is simple, lets worry about this when we actually have
> > > a board which hits these issues, and then we'll see from there.
> >
> > So thinking more about this, I think this is not that hard to fix.
> >
> > First lets fix the generic conflict between eprobedefer and subsys cleanup
> > functions. This can be done by:
> >
> > 1) Having a linked list of subsys cleanup functions
> > to call in drivers/base/dd.c
> >
> > 2) Have subsystems register their cleanup function rather then using
> > late_initcall to get it called
> >
> > 3) Have deferred_probe_work_func iterate over the list and call the cleanup
> > functions once deferred_probe_active_list goes empty. It should also remove
> > them once called, so that they only get called the first time
> > deferred_probe_active_list goes empty (iow when the deferred probing of'
> > build-in drivers is done).
> >
> > This way cleanup functions actually get run when all probing of (buildin)
> > drivers is done.
>
> That sounds like a reasonable solution to me.
>
> > Then when we move simplefb to a 2 fase probe, we can simply register the
> > framebuffer at the first probe call, and claim any resources we get, but
> > return -eprobe_defer if some resources returned that themselves.
> >
> > Then when simplefb_probe gets re-called, it can first check if it did not
> > already register a fb, and if it did, it can use that and see which
> > resources are missing, and only try to claim those. If some resources
> > still return probe_defer, return eprobe_defer again, otherwise success.
>
> I'm not sure you can check that the fb was already registered. When the
> driver probe fails the driver core will pretty much remove any trace of
> the driver, so checking for an already-reagistered framebuffer isn't
> going to be easy.
>
> But if you can manage to make that work it should give us a pretty good
> solution that should be easy to make use of by other drivers as well.
What about handling priorities in the suggested cleanup mechanism, and
just add the second phase as the highest priority cleanup callback?
That way, we would be guaranteed to be run just before any cleanup
function.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH] video: fbdev: Add CVT timing calculations.
From: David Ung @ 2014-11-18 23:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1415746455-28701-1-git-send-email-davidu@nvidia.com>
> -----Original Message-----
> From: David Ung [mailto:davidu@nvidia.com]
> Sent: Tuesday, November 11, 2014 2:54 PM
> To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com
> Cc: linux-fbdev@vger.kernel.org; David Ung
> Subject: [PATCH] video: fbdev: Add CVT timing calculations.
>
> Currently fbmon is still relying on the old GTF timings when parsing the
> standard timings of the EDID.
> This causes problem with some monitor eg DELL U2410 which advertises
> high resolutions like 1920x1200@60 and GTF timing gives it 193mhz clock
> which is out of spec of the monitor which has dclkmax of 170mhz.
> This patch address the above problem by adding support for CVT with
> reduced timings.
>
> Signed-off-by: David Ung <davidu@nvidia.com>
Ping?
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: Remove duplicated include in amba-clcd.c
From: Tomi Valkeinen @ 2014-11-19 12:51 UTC (permalink / raw)
To: Qiang Chen
Cc: plagnioj, linux, pawel.moll, linux-fbdev, linux-kernel, qcwj.lk
In-Reply-To: <1416216301-15854-1-git-send-email-qiang2.chen@sonymobile.com>
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
On 17/11/14 11:25, Qiang Chen wrote:
> This patch fixes duplicated include dma-mapping.h in
> drivers/video/fbdev/amba-clcd.c
>
> Signed-off-by: Qiang Chen <qiang2.chen@sonymobile.com>
> ---
> drivers/video/fbdev/amba-clcd.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
> index 6ad23bd..32c0b6b 100644
> --- a/drivers/video/fbdev/amba-clcd.c
> +++ b/drivers/video/fbdev/amba-clcd.c
> @@ -27,7 +27,6 @@
> #include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/hardirq.h>
> -#include <linux/dma-mapping.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_graph.h>
>
Thanks, queued for 3.19.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Grant Likely @ 2014-11-19 14:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141117155514.GU6414@lukather>
On Mon, Nov 17, 2014 at 3:55 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Nov 17, 2014 at 01:39:18PM +0000, Grant Likely wrote:
>> On Mon, Nov 17, 2014 at 12:47 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > On Mon, Nov 17, 2014 at 12:34:46PM +0100, Hans de Goede wrote:
>> >> If pre-filled framebuffer nodes are used, the firmware may need extra
>> >> properties to find the right node. This documents the properties to use
>> >> for this on sunxi platforms.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> >> ---
>> >> .../bindings/video/simple-framebuffer-sunxi.txt | 33 ++++++++++++++++++++++
>> >> 1 file changed, 33 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>> >> new file mode 100644
>> >> index 0000000..84ca264
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
>> >> @@ -0,0 +1,33 @@
>> >> +Sunxi specific Simple Framebuffer bindings
>> >> +
>> >> +This binding documents sunxi specific extensions to the simple-framebuffer
>> >> +bindings. The sunxi simplefb u-boot code relies on the devicetree containing
>> >> +pre-populated simplefb nodes.
>> >> +
>> >> +These extensions are intended so that u-boot can select the right node based
>> >> +on which pipeline and output is being used. As such they are solely intended
>> >> +for firmware / bootloader use, and the OS should ignore them.
>> >> +
>> >> +Required properties:
>> >> +- compatible: "sunxi,framebuffer"
>> >> +- sunxi,pipeline:
>> >> + <0> for the de_be0 -> lcdc0 -> output pipeline
>> >> + <1> for the de_be1 -> lcdc1 -> output pipeline
>> >> +- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
>> >> +
>> >> +Example:
>> >> +
>> >> +chosen {
>> >> + #address-cells = <1>;
>> >> + #size-cells = <1>;
>> >> + ranges;
>> >> +
>> >> + framebuffer@0 {
>> >> + compatible = "sunxi,framebuffer", "simple-framebuffer";
>> >> + sunxi,pipeline = <0>;
>> >> + sunxi,output = "hdmi";
>> >> + clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
>> >> + <&ahb_gates 44>;
>> >
>> > If we're going that way, then maybe having to specify clock-names
>> > would be better in order to know which clock is what?
>>
>> I wouldn't go that way with this binding since the driver has no need
>> to differentiate between the clocks, and driver authors shouldn't be
>> encouraged to do so. The purpose of the clocks in this node is only
>> for itemizing dependencies, not for how to configure the clocks.
>> Firmware shouldn't care at all about the clocks list, it only needs to
>> find the correct pre-populated node to fill in and enable.
>
> Well, if we want to play the "DT as an ABI" stuff, you have no
> guarantee that in the future, simplefb will still be the driver bound
> to "sunxi,framebuffer", and that this driver doesn't need to
> differentiate which clocks it needs to protect and which are not
> needed. Especially with the combination of the output that you now
> hardcode.
As long as the node claims compatibility with "simple-framebuffer",
then it must abide by the simple framebuffer rules. It cannot depend
on the driver having special knowledge about what to do with the
clocks. It is fine for a driver that understands "sunxi,framebuffer"
to enable additional features over and above simple-framebuffer,
providing it doesn't break the vanilla "simple-framebuffer" interface.
The simple-framebuffer binding is clear that it will treat the clocks
as dependencies only.
> Of course, if now we don't care at all about this ABI thing, it's
> completely fine for me. But I'll archive this email preciously.
I do not in any way mean to say it is okay to break ABI.
Also, my comments above are advice that I would give to U-Boot
authors, not something that should go in the binding. The
simple-framebuffer binding doesn't need to know about the clock names
at all, and if the assumption is that firmware will merely enable a
pre-populated framebuffer node in the tree, then it probably doesn't
need to parse the clocks property either. However, if it actually is
better for the U-Boot implementation to have clocks with names in the
node, then I'm not going to object.
g.
^ permalink raw reply
* Re: [PATCH v2 1/5] video: omapdss: Add opa362 driver
From: Dr. H. Nikolaus Schaller @ 2014-11-19 15:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5464DF26.2010707@ti.com>
Am 13.11.2014 um 17:41 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
> On 13/11/14 18:25, Dr. H. Nikolaus Schaller wrote:
>> Hi,
>>
>> Am 13.11.2014 um 12:51 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
>>
>>> On 13/11/14 00:10, Marek Belisko wrote:
>>>> opa362 is amplifier for video and can be connected to the tvout pads
>>>> of the OMAP3. It has one gpio control for enable/disable of the output
>>>> (high impedance).
>>>>
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>> drivers/video/fbdev/omap2/displays-new/Kconfig | 6 +
>>>> drivers/video/fbdev/omap2/displays-new/Makefile | 1 +
>>>> .../fbdev/omap2/displays-new/amplifier-opa362.c | 343 +++++++++++++++++++++
>>>
>>> I think it would be better to rename this to encoder-opa362.c. It's not
>>
>> When developing this driver we did simply rename the encoder-tfp410 file,
>> but thent hough that it does not fit into the „encoder“ category, because we
>> would expect something digital or digital to analog „encoding“ which it does not.
>
> That is true, but we already have other "encoders" like
> encoder-tpd12s015.c, which also do not encode. "encoder" in this context
> means something that takes a video input, and has a video output. In
> contrast to a panel or a connector.
>
>>>> +
>>>> + in->ops.atv->set_timings(in, &ddata->timings);
>>>> + /* fixme: should we receive the invert from our consumer, i.e. the connector? */
>>>> + in->ops.atv->invert_vid_out_polarity(in, true);
>>>
>>> Well this does seem to be broken. I don't know what the answer to the
>>> question above is, but the code doesn't work properly.
>>>
>>> In the opa362_invert_vid_out_polarity function below, you get the invert
>>> boolean from the connector. This you pass to the OMAP venc. However,
>>> above you always override that value in venc with true.
>>>
>>> So, either the invert_vid_out_polarity value has to be always true or
>>> false, because _OPA362_ requires it to be true or false, OR you need use
>>> the value from the connector.
>>>
>>> Seeing the comment in opa362_invert_vid_out_polarity, my guess is the
>>> latter, and the call above should be removed.
>>
>> Yes, you are right - this is not systematic.
>>
>> But the problem is that we can’t ask the connector here what it wants
>> to see. It might (or might not) call our opa362_invert_vid_out_polarity() later
>> which we can then forward to overwrite the inital state of this opa362_enable().
>
> You don't need to ask. The connector calls invert_vid_out_polarity
> before enabling the output.
Unfortunately it doesn’t. At least not always.
It does only for pdata systems but not for DT based systems:
if (!ddata->dev->of_node) {
in->ops.atv->set_type(in, ddata->connector_type);
in->ops.atv->invert_vid_out_polarity(in,
ddata->invert_polarity);
}
Not calling is in our case different from calling with ddata->invert_polarity = 0.
> You can just pass it forward inverted, as
> you already do in this driver. If it doesn't, it's either a bug or you
> can just rely on the value that is already programmed to venc.
Therefore it is not called with “false” which would make our invert_vid_out_polarity
invert it and send “true” towards the VENC. So VENC remains non-inverted.
We will also add a patch for the connector-analog.c
>>> We are going to support only DT boot at some point. Thus I think the
>>> whole platform data code should be left out.
>>
>> Is there already a decision? I think it should not be done before. And it
>> does not harm to still have it.
>
> It's just a matter of time. I don't accept any new boards using platform
> data for display, or new display drivers using platform data, because I
> don't want to spend my time converting them later. And as this is a new
> driver, no existing board can be using the opa362 platform_data. So we
> can support DT only.
Ok, that looks reasonable - as long as we can rely on that all mainline DSS
drivers are already fully converted to DT :)
BR,
Nikolaus
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Tomi Valkeinen @ 2014-11-20 10:59 UTC (permalink / raw)
To: Mark Brown, Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <54646648.5030807@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
Hi Mark,
On 13/11/14 10:05, Tomi Valkeinen wrote:
> Hi Mark,
>
> On 13/11/14 00:23, Mark Brown wrote:
>> On Wed, Nov 12, 2014 at 04:40:51PM +0200, Jyri Sarha wrote:
>>
>>> It would make the most sense to get these in trough fbdev tree. So it
>>> would be nice to get acked-bys (if the patches are Ok) for ASoC side
>>> changes from appropriate maintainers.
>>
>> So, this is a very large series which gets reposted every so often to no
>> apparent interest from the video side, there's been no response at all
>
> Sorry for the lack of communication. We've been discussing this series
> on irc. It's been mostly about how to manage the device/driver split
> between drivers/video/ and sound/ sides.
>
>> that I can remember and even the earlier bits of the series before it
>> starts touching audio don't seem to be getting merged. What's going on
>> here?
>
> The series is all audio in terms of functionality. The first few patches
> could probably be merged independently, but I've wanted this whole OMAP
> HDMI audio rewrite to be in one series.
>
> I'll start testing this latest series, and I hope we can merge it for
> the next merge window so that we'll finally get the OMAP HDMI audio working.
>
> I don't have much knowledge of the asoc architecture, so I probably
> can't comment much on the sound/ side design. For me the most important
> things are that 1) it works 2) I can easily unload/load the modules
> (which was broken in some of the earlier versions).
The series looks good to me, and works for me.
Do you have any comments for the sound/ parts? If not, I can merge this
series via fbdev tree, and for that I'd like your ack on the sound/ patches.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: Add CVT timing calculations.
From: Tomi Valkeinen @ 2014-11-20 12:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1415746455-28701-1-git-send-email-davidu@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 9984 bytes --]
Hi,
On 12/11/14 00:54, David Ung wrote:
> Currently fbmon is still relying on the old GTF timings when parsing
> the standard timings of the EDID.
> This causes problem with some monitor eg DELL U2410 which advertises
> high resolutions like 1920x1200@60 and GTF timing gives it 193mhz
> clock which is out of spec of the monitor which has dclkmax of 170mhz.
> This patch address the above problem by adding support for CVT with
> reduced timings.
These timings are quite complex, and I don't claim to fully understand
all the details, so I have a few questions:
So you say the monitor has a standard timing for 1920x1200@60. If I read
the EDID standard right, a standard timing entry either has to match a
timing from VESA DMT, or it shall be calculated using GTF.
Don't we have or shouldn't we have a table for the VESA DMT modes, from
which to search for standard timings? 1920x1200@60 should be found there.
> Signed-off-by: David Ung <davidu@nvidia.com>
> ---
> drivers/video/fbdev/core/fbmon.c | 92 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 77 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 5b0e313..8d4ec9f 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -21,6 +21,11 @@
> * GTF Spreadsheet by Andy Morrish (1/5/97)
> * available at http://www.vesa.org
> *
> + * Coordinated Video Timing is derived from:
> + *
> + * CVT Spreadsheet by Graham Loveridge (9/Apr/2003)
> + * available at http://www.vesa.org
> + *
> * This file is subject to the terms and conditions of the GNU General Public
> * License. See the file COPYING in the main directory of this archive
> * for more details.
> @@ -383,17 +388,21 @@ static void get_chroma(unsigned char *block, struct fb_monspecs *specs)
> }
>
> static void calc_mode_timings(int xres, int yres, int refresh,
> - struct fb_videomode *mode)
> + struct fb_videomode *mode,
> + struct fb_monspecs *specs)
> {
> struct fb_var_screeninfo *var;
> + struct fb_info info;
>
> var = kzalloc(sizeof(struct fb_var_screeninfo), GFP_KERNEL);
> + if (specs)
> + memcpy(&info.monspecs, specs, sizeof(struct fb_monspecs));
>
> if (var) {
> var->xres = xres;
> var->yres = yres;
> - fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON,
> - refresh, var, NULL);
> + fb_get_mode(FB_VSYNCTIMINGS | FB_IGNOREMON, refresh, var,
> + specs && !specs->gtf ? &info : NULL);
This juggling with fb_monspecs, creating a temporary fb_info (and even
leaving most of it uninitialized) doesn't look very nice.
Why allow NULL specs in any case? Don't we always have it?
If the specs is really only used for the GTF flag, could we just pass
that flag, not the whole specs?
> mode->xres = xres;
> mode->yres = yres;
> mode->pixclock = var->pixclock;
> @@ -417,12 +426,12 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
>
> c = block[0];
> if (c&0x80) {
> - calc_mode_timings(720, 400, 70, &mode[num]);
> + calc_mode_timings(720, 400, 70, &mode[num], NULL);
> mode[num++].flag = FB_MODE_IS_CALCULATED;
> DPRINTK(" 720x400@70Hz\n");
> }
> if (c&0x40) {
> - calc_mode_timings(720, 400, 88, &mode[num]);
> + calc_mode_timings(720, 400, 88, &mode[num], NULL);
> mode[num++].flag = FB_MODE_IS_CALCULATED;
> DPRINTK(" 720x400@88Hz\n");
> }
> @@ -431,7 +440,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
> DPRINTK(" 640x480@60Hz\n");
> }
> if (c&0x10) {
> - calc_mode_timings(640, 480, 67, &mode[num]);
> + calc_mode_timings(640, 480, 67, &mode[num], NULL);
> mode[num++].flag = FB_MODE_IS_CALCULATED;
> DPRINTK(" 640x480@67Hz\n");
> }
> @@ -462,7 +471,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
> DPRINTK(" 800x600@75Hz\n");
> }
> if (c&0x20) {
> - calc_mode_timings(832, 624, 75, &mode[num]);
> + calc_mode_timings(832, 624, 75, &mode[num], NULL);
> mode[num++].flag = FB_MODE_IS_CALCULATED;
> DPRINTK(" 832x624@75Hz\n");
> }
> @@ -496,7 +505,7 @@ static int get_est_timing(unsigned char *block, struct fb_videomode *mode)
> }
>
> static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
> - int ver, int rev)
> + int ver, int rev, struct fb_monspecs *specs)
> {
> int xres, yres = 0, refresh, ratio, i;
>
> @@ -535,7 +544,7 @@ static int get_std_timing(unsigned char *block, struct fb_videomode *mode,
> return 1;
> }
> }
> - calc_mode_timings(xres, yres, refresh, mode);
> + calc_mode_timings(xres, yres, refresh, mode, specs);
> return 1;
> }
>
> @@ -545,7 +554,7 @@ static int get_dst_timing(unsigned char *block,
> int j, num = 0;
>
> for (j = 0; j < 6; j++, block += STD_TIMING_DESCRIPTION_SIZE)
> - num += get_std_timing(block, &mode[num], ver, rev);
> + num += get_std_timing(block, &mode[num], ver, rev, NULL);
Hmm, why is this call using NULL?
>
> return num;
> }
> @@ -601,7 +610,8 @@ static void get_detailed_timing(unsigned char *block,
> * This function builds a mode database using the contents of the EDID
> * data
> */
> -static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
> +static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
> + struct fb_monspecs *specs)
> {
> struct fb_videomode *mode, *m;
> unsigned char *block;
> @@ -643,7 +653,7 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize)
> DPRINTK(" Standard Timings\n");
> block = edid + STD_TIMING_DESCRIPTIONS_START;
> for (i = 0; i < STD_TIMING; i++, block += STD_TIMING_DESCRIPTION_SIZE)
> - num += get_std_timing(block, &mode[num], ver, rev);
> + num += get_std_timing(block, &mode[num], ver, rev, specs);
>
> block = edid + DETAILED_TIMING_DESCRIPTIONS_START;
> for (i = 0; i < 4; i++, block+= DETAILED_TIMING_DESCRIPTION_SIZE) {
> @@ -707,7 +717,7 @@ static int fb_get_monitor_limits(unsigned char *edid, struct fb_monspecs *specs)
> int num_modes, hz, hscan, pixclock;
> int vtotal, htotal;
>
> - modes = fb_create_modedb(edid, &num_modes);
> + modes = fb_create_modedb(edid, &num_modes, NULL);
Same here.
> if (!modes) {
> DPRINTK("None Available\n");
> return 1;
> @@ -964,7 +974,7 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
> DPRINTK(" Display Characteristics:\n");
> get_monspecs(edid, specs);
>
> - specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
> + specs->modedb = fb_create_modedb(edid, &specs->modedb_len, specs);
>
> /*
> * Workaround for buggy EDIDs that sets that the first
> @@ -1255,6 +1265,55 @@ static void fb_timings_dclk(struct __fb_timings *timings)
> }
>
> /*
> + * All calculations are based on the VESA CVT Spreadsheet
> + * available at VESA's public ftp (http://www.vesa.org).
> + *
> + * The names of variables are kept to match as closely to the names
> + * that appears in the spreadsheet.
> + */
> +#define RB_MIN_V_BLANK 460
> +#define RB_H_BLANK 160
> +#define CELL_GRAN 8
> +#define CLOCK_STEP 250
> +#define RB_V_FPORCH 3
> +#define V_SYNC_RND 6
> +#define MIN_V_BPORCH 6
> +
> +static void fb_timings_vfreq_cvt(struct __fb_timings *timings)
> +{
> + u32 v_lines_rnd, h_period_est, vbi_line, rb_min_vbi, act_vbi_lines;
> + u32 total_v_lines, h_pixels_rnd, total_active_pixels, total_pixels;
> + u32 act_pixel_freq;
> + u32 v_field_rate_rqd = timings->vfreq;
> + u32 h_pixels = timings->hactive;
> + u32 v_lines = timings->vactive;
> +
> + v_lines_rnd = v_lines;
> + h_period_est = 1000000 / v_field_rate_rqd - RB_MIN_V_BLANK;
> + h_period_est = h_period_est / v_lines_rnd;
> + vbi_line = RB_MIN_V_BLANK / h_period_est;
> + rb_min_vbi = RB_V_FPORCH + V_SYNC_RND + MIN_V_BPORCH;
> + if (vbi_line < rb_min_vbi)
> + act_vbi_lines = rb_min_vbi;
> + else
> + act_vbi_lines = vbi_line;
> + total_v_lines = act_vbi_lines + v_lines_rnd;
> + h_pixels_rnd = (h_pixels / CELL_GRAN) * CELL_GRAN;
> + total_active_pixels = h_pixels_rnd;
> + total_pixels = RB_H_BLANK + total_active_pixels;
> + act_pixel_freq = v_field_rate_rqd * total_v_lines * total_pixels;
> + act_pixel_freq /= 1000;
> + act_pixel_freq = act_pixel_freq / CLOCK_STEP * CLOCK_STEP * 1000;
> +
> + timings->hfreq = act_pixel_freq / total_pixels;
> + timings->vblank = act_vbi_lines;
> + timings->vtotal = timings->vactive + timings->vblank;
> + timings->hblank = RB_H_BLANK;
> + timings->htotal = timings->hactive + timings->hblank;
> + timings->dclk = timings->htotal * timings->hfreq;
> +}
What about fbcvt.c? You add fb_timings_vfreq_cvt function, but it sounds
something which we either have in fbcvt.c, or should be in fbcvt.c.
> +
> +/*
> * fb_get_mode - calculates video mode using VESA GTF
> * @flags: if: 0 - maximize vertical refresh rate
> * 1 - vrefresh-driven calculation;
> @@ -1347,7 +1406,10 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
> break;
> case FB_VSYNCTIMINGS: /* vrefresh driven */
> timings->vfreq = val;
> - fb_timings_vfreq(timings);
> + if (info && !info->monspecs.gtf)
> + fb_timings_vfreq_cvt(timings);
> + else
> + fb_timings_vfreq(timings);
> break;
> case FB_HSYNCTIMINGS: /* hsync driven */
> timings->hfreq = val;
>
With your patch, fb_get_mode() does something else than the comments
say. It explicitly talks about calculating with GTF. Does any of the
callers expect it to use GTF, and won't work with CVT? In the minimum
the comments should be changed to reflect he new behavior.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH] video: fbdev: Add CVT timing calculations.
From: David Ung @ 2014-11-20 21:00 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1415746455-28701-1-git-send-email-davidu@nvidia.com>
> On 12/11/14 00:54, David Ung wrote:
> > Currently fbmon is still relying on the old GTF timings when parsing
> > the standard timings of the EDID.
> > This causes problem with some monitor eg DELL U2410 which advertises
> > high resolutions like 1920x1200@60 and GTF timing gives it 193mhz
> > clock which is out of spec of the monitor which has dclkmax of 170mhz.
> > This patch address the above problem by adding support for CVT with
> > reduced timings.
>
> These timings are quite complex, and I don't claim to fully understand all
> the details, so I have a few questions:
>
> So you say the monitor has a standard timing for 1920x1200@60. If I read the
> EDID standard right, a standard timing entry either has to match a timing
> from VESA DMT, or it shall be calculated using GTF.
>
> Don't we have or shouldn't we have a table for the VESA DMT modes, from
> which to search for standard timings? 1920x1200@60 should be found there.
Agree, there ought to be a DMT table of modes that it should match against, though this table does not exist currently in fbdev.
After reading your comment, I had a look at the edid again for DELL U2410 and in the standard timing, it defines - (D1, 00)h
According to the DMT document, that matches DMT ID 45h - which the table defines as 193.25mhz pixel clock.
Clearly my patch is wrong in this approach which tries to correct the problem with cvt reduce timing.
The correct approach should be to invalidate this mode since the monspec defines a max clock of 170mhz.
This should make the patch much more simpler. I shall abandon this patch and post a different one.
David
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-11-21 11:23 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <546DC9A0.8060700@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]
On Thu, Nov 20, 2014 at 12:59:44PM +0200, Tomi Valkeinen wrote:
> The series looks good to me, and works for me.
> Do you have any comments for the sound/ parts? If not, I can merge this
> series via fbdev tree, and for that I'd like your ack on the sound/ patches.
I've not reviewed it yet and I'm still seeing some fairly basic
discussion between Jiri and Jean-Francois about approaches to
integrating HDMI which seem to have ground to a halt (I've not been
reading them in any detail). The fact that we're getting no sharing at
all between all the different HDMI devices people are supporting and
very limited dialogue between them is really setting off alarm bells.
As far as I can tell in order to figure out what to do with all this
HDMI stuff I'm going to need to go to square one, get an overview of the
hardware that's out there for myself and try to work out what to do with
it.
With this specific series I also need to figure out what all the video
side is about (like I said earlier a lot of the patches look like
they're supposed to be simple fixes for the video code not terribly
closely tied to the rest of the series but none of them are getting
applied) and what the end goal is beyond mechanically moving code.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Jyri Sarha @ 2014-11-21 12:10 UTC (permalink / raw)
To: Mark Brown, Tomi Valkeinen
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <20141121112317.GQ3815@sirena.org.uk>
On 11/21/2014 01:23 PM, Mark Brown wrote:
> On Thu, Nov 20, 2014 at 12:59:44PM +0200, Tomi Valkeinen wrote:
>
>> The series looks good to me, and works for me.
>
>> Do you have any comments for the sound/ parts? If not, I can merge this
>> series via fbdev tree, and for that I'd like your ack on the sound/ patches.
>
> I've not reviewed it yet and I'm still seeing some fairly basic
> discussion between Jiri and Jean-Francois about approaches to
> integrating HDMI which seem to have ground to a halt (I've not been
> reading them in any detail). The fact that we're getting no sharing at
> all between all the different HDMI devices people are supporting and
> very limited dialogue between them is really setting off alarm bells.
>
OMAP HDMI audio is fundamentally different to the case on Armada or on
BBB. In omap the whole HDMI IP is integrated to the SoC and there really
is no codec in the ASoC sense. The the cpu-dai transmits the audio
directly to hdmi wire and there is no i2s bus involved. So this case
should not be mixed with the patches Jean-Francois working on. The code
is also orthogonal in that sense that the latest omap-hdmi-audio uses
the generic dymmy codec.
The issue about generic HDMI codec, that Jean-Francois (and soon me) is
trying to solve - applies to the cases where a generic cpu-dai is
connected to an external HDMI encoder with i2s (or s/pdif, unfortunately
do not have such HW). In these cases the structure of the ASoC setup
resembles closely the usual pattern of ASoC cards. The main difference
is just that the "codec" IP also handling the video and there are no
mixers, etc.
I am currently trying to find the common denominator between tda998x and
SiI9022 HDMI encoder chips to come up with a generic solution for the
external HDMI encoder case. However, this work is completely separate to
the omap-hdmi-audio and its review should not be delayed because of the
hdmi codec work.
Best regards,
Jyri
> As far as I can tell in order to figure out what to do with all this
> HDMI stuff I'm going to need to go to square one, get an overview of the
> hardware that's out there for myself and try to work out what to do with
> it.
>
> With this specific series I also need to figure out what all the video
> side is about (like I said earlier a lot of the patches look like
> they're supposed to be simple fixes for the video code not terribly
> closely tied to the rest of the series but none of them are getting
> applied) and what the end goal is beyond mechanically moving code.
>
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Jyri Sarha @ 2014-11-21 12:35 UTC (permalink / raw)
To: Mark Brown, Tomi Valkeinen
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <20141121112317.GQ3815@sirena.org.uk>
On 11/21/2014 01:23 PM, Mark Brown wrote:
> With this specific series I also need to figure out what all the video
> side is about (like I said earlier a lot of the patches look like
> they're supposed to be simple fixes for the video code not terribly
> closely tied to the rest of the series but none of them are getting
> applied) and what the end goal is beyond mechanically moving code.
(Sorry, I forgot to comment this part.)
The end goal of this series is to fix OMAP HDMI audio, that got broken
couple of releases ago. At the same time I cleaned up the old complex
scheme to make the connection between the video and audio parts and
allow multiple HDMI devices (DSS side is not ready for this yet, but
audio side is).
Another target was to make configuring the audio as simple as possible.
Because everything needed for HDMI audio is always there if the video
side is correctly configured, there should be no need for any additional
configuration (in DT or otherwise) to get the audio working. Now simply
selecting omap-hdmi-audio is enough. There is no complex cross
dependencies to the video side any more, the audio driver simply does
find its device if the HDMI video driver is not probed.
But indeed, in functional sense the whole series - apart from the couple
of fixes in the beginning - is just moving the code around.
Best regards,
Jyri
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: simplefb-sunxi: Add sunxi simplefb extensions
From: Maxime Ripard @ 2014-11-21 14:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6tqRS=oCeqbrez86uLLXtQ_C4ETEesNqgxKGb38QOQ8SQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5753 bytes --]
On Wed, Nov 19, 2014 at 02:05:48PM +0000, Grant Likely wrote:
> On Mon, Nov 17, 2014 at 3:55 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Nov 17, 2014 at 01:39:18PM +0000, Grant Likely wrote:
> >> On Mon, Nov 17, 2014 at 12:47 PM, Maxime Ripard
> >> <maxime.ripard@free-electrons.com> wrote:
> >> > On Mon, Nov 17, 2014 at 12:34:46PM +0100, Hans de Goede wrote:
> >> >> If pre-filled framebuffer nodes are used, the firmware may need extra
> >> >> properties to find the right node. This documents the properties to use
> >> >> for this on sunxi platforms.
> >> >>
> >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> >> ---
> >> >> .../bindings/video/simple-framebuffer-sunxi.txt | 33 ++++++++++++++++++++++
> >> >> 1 file changed, 33 insertions(+)
> >> >> create mode 100644 Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >> >> new file mode 100644
> >> >> index 0000000..84ca264
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer-sunxi.txt
> >> >> @@ -0,0 +1,33 @@
> >> >> +Sunxi specific Simple Framebuffer bindings
> >> >> +
> >> >> +This binding documents sunxi specific extensions to the simple-framebuffer
> >> >> +bindings. The sunxi simplefb u-boot code relies on the devicetree containing
> >> >> +pre-populated simplefb nodes.
> >> >> +
> >> >> +These extensions are intended so that u-boot can select the right node based
> >> >> +on which pipeline and output is being used. As such they are solely intended
> >> >> +for firmware / bootloader use, and the OS should ignore them.
> >> >> +
> >> >> +Required properties:
> >> >> +- compatible: "sunxi,framebuffer"
> >> >> +- sunxi,pipeline:
> >> >> + <0> for the de_be0 -> lcdc0 -> output pipeline
> >> >> + <1> for the de_be1 -> lcdc1 -> output pipeline
> >> >> +- sunxi,output: One of: "hdmi", "lcd", "vga", and "composite"
> >> >> +
> >> >> +Example:
> >> >> +
> >> >> +chosen {
> >> >> + #address-cells = <1>;
> >> >> + #size-cells = <1>;
> >> >> + ranges;
> >> >> +
> >> >> + framebuffer@0 {
> >> >> + compatible = "sunxi,framebuffer", "simple-framebuffer";
> >> >> + sunxi,pipeline = <0>;
> >> >> + sunxi,output = "hdmi";
> >> >> + clocks = <&pll5 1>, <&ahb_gates 36>, <&ahb_gates 43>,
> >> >> + <&ahb_gates 44>;
> >> >
> >> > If we're going that way, then maybe having to specify clock-names
> >> > would be better in order to know which clock is what?
> >>
> >> I wouldn't go that way with this binding since the driver has no need
> >> to differentiate between the clocks, and driver authors shouldn't be
> >> encouraged to do so. The purpose of the clocks in this node is only
> >> for itemizing dependencies, not for how to configure the clocks.
> >> Firmware shouldn't care at all about the clocks list, it only needs to
> >> find the correct pre-populated node to fill in and enable.
> >
> > Well, if we want to play the "DT as an ABI" stuff, you have no
> > guarantee that in the future, simplefb will still be the driver bound
> > to "sunxi,framebuffer", and that this driver doesn't need to
> > differentiate which clocks it needs to protect and which are not
> > needed. Especially with the combination of the output that you now
> > hardcode.
>
> As long as the node claims compatibility with "simple-framebuffer",
> then it must abide by the simple framebuffer rules. It cannot depend
> on the driver having special knowledge about what to do with the
> clocks. It is fine for a driver that understands "sunxi,framebuffer"
> to enable additional features over and above simple-framebuffer,
> providing it doesn't break the vanilla "simple-framebuffer" interface.
>
> The simple-framebuffer binding is clear that it will treat the clocks
> as dependencies only.
Isn't the compatible list precisely made to be able to define more
specific behaviours if needed?
If I get a list with "sunxi,framebuffer", "simple-framebuffer", I've
always thought that sunxi,framebuffer was defining exactly what the
device was about, and that simple-framebuffer was only aimed at
providing roughly the same functionnality, possibly with degraded
features, and with a compatible DT bindings.
I don't see how if we require a clock-names list in sunxi,framebuffer
that would break the binding of simple-framebuffer. It's just more
information, that might prove useful in the future.
> > Of course, if now we don't care at all about this ABI thing, it's
> > completely fine for me. But I'll archive this email preciously.
>
> I do not in any way mean to say it is okay to break ABI.
>
> Also, my comments above are advice that I would give to U-Boot
> authors, not something that should go in the binding. The
> simple-framebuffer binding doesn't need to know about the clock names
> at all, and if the assumption is that firmware will merely enable a
> pre-populated framebuffer node in the tree, then it probably doesn't
> need to parse the clocks property either. However, if it actually is
> better for the U-Boot implementation to have clocks with names in the
> node, then I'm not going to object.
I'm not speaking about u-boot here, but the kernel itself.
What if we want from that pipeline property to enable (or rather keep
enabled) only a few clocks, and not all of them?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-11-21 16:14 UTC (permalink / raw)
To: Jyri Sarha
Cc: Tomi Valkeinen, alsa-devel, linux-fbdev, linux-omap,
peter.ujfalusi, liam.r.girdwood
In-Reply-To: <546F317B.9030004@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
On Fri, Nov 21, 2014 at 02:35:07PM +0200, Jyri Sarha wrote:
> On 11/21/2014 01:23 PM, Mark Brown wrote:
> >With this specific series I also need to figure out what all the video
> >side is about (like I said earlier a lot of the patches look like
> >they're supposed to be simple fixes for the video code not terribly
> >closely tied to the rest of the series but none of them are getting
> >applied) and what the end goal is beyond mechanically moving code.
> The end goal of this series is to fix OMAP HDMI audio, that got broken
> couple of releases ago. At the same time I cleaned up the old complex scheme
> to make the connection between the video and audio parts and allow multiple
> HDMI devices (DSS side is not ready for this yet, but audio side is).
But in what way is it broken and how did this happen? Why are none of
the patches which look like they're supposed to be bug fixes early on in
the series getting applied? I had thought this was just a lack of
interest on the video side but it seems there's some other problems
since the series has apparently been discussed off-list and still it's
just as big as it was initially.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox