Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-10-02 13:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4Oyb3KBWhGgKfwNop7+cwR=p=ZpVs-jfH0iQwm5scfOJ-A@mail.gmail.com>

On 2 October 2014 15:27, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>
>>>>> You need both.
>>>>>
>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>> process -- the same thing happens with the early printk UART driver.
>>>>> EARLYPRINTK on the command line enables this.
>>>>>
>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>> set_console() but it's been a while.
>>>>
>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>
>>> No, that is where you get into trouble. The device specific driver has
>>> to go onto initrd where it can be loaded as early in the boot process
>>> as possible.
>>
>> This is an argument in the "you cannot do that" / "your use case is not valid"
>> category, IOW this is not a technical argument. You say I cannot do that I
>> say I can, deadlock.
>
> It is certainly possible to extend an earlyframebuffer to be able to
> run as a user space console. It is just going to turn into a
> Frankenmonster driver with piles of device specific, special case code
> in it.

What is device specific about code that reads a list of clocks and
just asks tells kernel it uses them all?

That's been discussed to death in this thread already.

>
> I think that device specific code belongs in a device specific driver
> and earlyframebuffer should handoff to it as soon as possible.

Even it that case it needs to tell the kernel it needs the clocks so
that they are not shut of until that handoff happens.

>
>>
>> I've already explained that we not only can do that (we already have working
>> code proving that), but also that this is something which we absolutely need:
>>
>>>> One example why this is necessary is e.g. to debug things where the problem
>>>> is that the right module is not included in the initrd.
>
> A generic earlyframebuffer would show this error.
>
> Just use earlyprintk as a guideline, if earlyprintk shows the error
> earlyframebuffer would also show it.
>

It does not. It hands off to an uart driver built into the kernel. The
handoff is broken on sunxi and the earlyprintk continues to work
indefinitely replicating all kernel messages twice on the serial
console unless something happens to reconfigure the uart used for
earlyprink later on. When that does happen the early console can fail
very early, even way before the time you would load an initrd. It just
happens to work most of the time because when you use earlyprintk most
of the time you also use serial console with the same parameters on
the same pins.

Thanks

Michal

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-02 14:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OxTmNRxSJum17RZGLLku5dAFPauzqyP0BOF-VC=bD=Hng@mail.gmail.com>

Hi,

On 10/02/2014 03:40 PM, jonsmirl@gmail.com wrote:
> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 03:27 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl@gmail.com wrote:
>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>> ...
>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>
>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>
>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>
>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>
>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>
>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>
>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>
>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>
>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>
>>>>>>> You need both.
>>>>>>>
>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>
>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>> set_console() but it's been a while.
>>>>>>
>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>
>>>>> No, that is where you get into trouble. The device specific driver has
>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>> as possible.
>>>>
>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>> say I can, deadlock.
>>>
>>> It is certainly possible to extend an earlyframebuffer to be able to
>>> run as a user space console. It is just going to turn into a
>>> Frankenmonster driver with piles of device specific, special case code
>>> in it.
>>
>> There is nothing hardware specific about a framebuffer needing some
>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>> that is hardware specific, but the framebuffer driver does not need to
>> worry about that, it just sees a clocks property with some random clocks
>> in there, and that is as generic as it gets.
>>
>>> I think that device specific code belongs in a device specific driver
>>> and earlyframebuffer should handoff to it as soon as possible.
>>>
>>>>
>>>> I've already explained that we not only can do that (we already have working
>>>> code proving that), but also that this is something which we absolutely need:
>>>>
>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>> is that the right module is not included in the initrd.
>>>
>>> A generic earlyframebuffer would show this error.
>>
>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>> be disabled before the initrd starts, and the screen will be black from then
> 
> I thought the clock/regulator clean up happened after initrd loading,
> but maybe that is not the case.
> 
> A cleaner solution would then be to modify the clock/regulator clean
> up to happen after driver loading is finished from initrd. Deferring
> until after that completes is a fixed limit, everything is sitting
> there in RAM. I would not propose extending it until harddisk based
> loading happens.
> 
> So there are two ways to do this...
> 1) modify things like earlyconsole to protect device specific resource
> (I think this is a bad idea)

Why is this a bad idea? If the bootloader tells us exactly which resources
are needed, then earlyconsole can claim them, and release them on
handover to the real display driver.

> 2) delay the clock/regulator cleanup until after there is a fixed
> window for device specific drivers to load in. Loading from initrd is
> a fixed window.

As I already explained by example in another mail, this won't work:

"delaying over the initrd is not helpful. Not having the real driver
load for whatever reasons, is not necessarily a boot blocking event,
and if it us just missing will not lead to any error messages.

So the boot will continue normally with a black screen, and things are
still impossible to debug."

We've been down the whole delay till $random point in time thing in the
past with storage enumeration, where you need to wait for say all members
of a raid set to show up. The lesson learned from that is that you should
not wait $random time / event, but wait for the actual storage device to
show up.

And this is just like that, we need to wait for the actual display driver
to have loaded and taken over before "cleaning up" the clocks used by
the display engine.

I guess we could just delay all clock cleanup until then, not really
pretty, and I still prefer to just list the clocks in the simplefb
dtnode, but if this version would be acceptable to all involved, I can
live with it.

Mike, would a patch adding 2 calls like these to the clock core be
acceptable ?  :

clk_block_disable_unused()
clk_unblock_disable_unused()

Where an earlyconsole driver can then call clk_block_disable_unused(),
which will turn any calls to clk_disable_unused into a nop.

And when the earlyconsole gets unregistered because the real driver
takes over, it calls clk_unblock_disable_unused(), which then checks if
clk_disable_unused() has already been called (iow check if we're post
late_init), and if it has calls clk_disable_unused() at that time.

If this is acceptable I can whip up a patch for this.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-02 14:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542D5C74.4050602@redhat.com>

On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 10/02/2014 03:40 PM, jonsmirl@gmail.com wrote:
>> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 03:27 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl@gmail.com wrote:
>>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>> ...
>>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>>
>>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>>
>>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>>
>>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>>
>>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>>
>>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>>
>>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>>
>>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>>
>>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>>
>>>>>>>> You need both.
>>>>>>>>
>>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>>
>>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>>> set_console() but it's been a while.
>>>>>>>
>>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>>
>>>>>> No, that is where you get into trouble. The device specific driver has
>>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>>> as possible.
>>>>>
>>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>>> say I can, deadlock.
>>>>
>>>> It is certainly possible to extend an earlyframebuffer to be able to
>>>> run as a user space console. It is just going to turn into a
>>>> Frankenmonster driver with piles of device specific, special case code
>>>> in it.
>>>
>>> There is nothing hardware specific about a framebuffer needing some
>>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>>> that is hardware specific, but the framebuffer driver does not need to
>>> worry about that, it just sees a clocks property with some random clocks
>>> in there, and that is as generic as it gets.
>>>
>>>> I think that device specific code belongs in a device specific driver
>>>> and earlyframebuffer should handoff to it as soon as possible.
>>>>
>>>>>
>>>>> I've already explained that we not only can do that (we already have working
>>>>> code proving that), but also that this is something which we absolutely need:
>>>>>
>>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>>> is that the right module is not included in the initrd.
>>>>
>>>> A generic earlyframebuffer would show this error.
>>>
>>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>>> be disabled before the initrd starts, and the screen will be black from then
>>
>> I thought the clock/regulator clean up happened after initrd loading,
>> but maybe that is not the case.
>>
>> A cleaner solution would then be to modify the clock/regulator clean
>> up to happen after driver loading is finished from initrd. Deferring
>> until after that completes is a fixed limit, everything is sitting
>> there in RAM. I would not propose extending it until harddisk based
>> loading happens.
>>
>> So there are two ways to do this...
>> 1) modify things like earlyconsole to protect device specific resource
>> (I think this is a bad idea)
>
> Why is this a bad idea? If the bootloader tells us exactly which resources
> are needed, then earlyconsole can claim them, and release them on
> handover to the real display driver.
>
>> 2) delay the clock/regulator cleanup until after there is a fixed
>> window for device specific drivers to load in. Loading from initrd is
>> a fixed window.
>
> As I already explained by example in another mail, this won't work:
>
> "delaying over the initrd is not helpful. Not having the real driver
> load for whatever reasons, is not necessarily a boot blocking event,
> and if it us just missing will not lead to any error messages.
>
> So the boot will continue normally with a black screen, and things are
> still impossible to debug."
>
> We've been down the whole delay till $random point in time thing in the
> past with storage enumeration, where you need to wait for say all members
> of a raid set to show up. The lesson learned from that is that you should
> not wait $random time / event, but wait for the actual storage device to
> show up.
>
> And this is just like that, we need to wait for the actual display driver
> to have loaded and taken over before "cleaning up" the clocks used by
> the display engine.
>
> I guess we could just delay all clock cleanup until then, not really
> pretty, and I still prefer to just list the clocks in the simplefb
> dtnode, but if this version would be acceptable to all involved, I can
> live with it.
>
> Mike, would a patch adding 2 calls like these to the clock core be
> acceptable ?  :
>
> clk_block_disable_unused()
> clk_unblock_disable_unused()
>
> Where an earlyconsole driver can then call clk_block_disable_unused(),
> which will turn any calls to clk_disable_unused into a nop.

Is there a way to use the existing eprobe_defer system to do this?

>
> And when the earlyconsole gets unregistered because the real driver
> takes over, it calls clk_unblock_disable_unused(), which then checks if
> clk_disable_unused() has already been called (iow check if we're post
> late_init), and if it has calls clk_disable_unused() at that time.
>
> If this is acceptable I can whip up a patch for this.
>
> Regards,
>
> Hans
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-10-02 14:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OxTmNRxSJum17RZGLLku5dAFPauzqyP0BOF-VC=bD=Hng@mail.gmail.com>

On 2 October 2014 15:40, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 03:27 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl@gmail.com wrote:
>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>> ...
>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>
>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>
>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>
>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>
>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>
>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>
>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>
>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>
>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>
>>>>>>> You need both.
>>>>>>>
>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>
>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>> set_console() but it's been a while.
>>>>>>
>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>
>>>>> No, that is where you get into trouble. The device specific driver has
>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>> as possible.
>>>>
>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>> say I can, deadlock.
>>>
>>> It is certainly possible to extend an earlyframebuffer to be able to
>>> run as a user space console. It is just going to turn into a
>>> Frankenmonster driver with piles of device specific, special case code
>>> in it.
>>
>> There is nothing hardware specific about a framebuffer needing some
>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>> that is hardware specific, but the framebuffer driver does not need to
>> worry about that, it just sees a clocks property with some random clocks
>> in there, and that is as generic as it gets.
>>
>>> I think that device specific code belongs in a device specific driver
>>> and earlyframebuffer should handoff to it as soon as possible.
>>>
>>>>
>>>> I've already explained that we not only can do that (we already have working
>>>> code proving that), but also that this is something which we absolutely need:
>>>>
>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>> is that the right module is not included in the initrd.
>>>
>>> A generic earlyframebuffer would show this error.
>>
>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>> be disabled before the initrd starts, and the screen will be black from then
>
> I thought the clock/regulator clean up happened after initrd loading,
> but maybe that is not the case.
>
> A cleaner solution would then be to modify the clock/regulator clean
> up to happen after driver loading is finished from initrd. Deferring
> until after that completes is a fixed limit, everything is sitting
> there in RAM. I would not propose extending it until harddisk based
> loading happens.
>
> So there are two ways to do this...
> 1) modify things like earlyconsole to protect device specific resource
> (I think this is a bad idea)
> 2) delay the clock/regulator cleanup until after there is a fixed
> window for device specific drivers to load in. Loading from initrd is
> a fixed window.

Fixed window is timeout.

Time and time again timeouts break.

How is loading a module from harddisk different form loading from initrd?

How is even the kernel going to tell?

How should systems that run completely off initrd behave?

How should the kernel behave during this 'fixed window'? If it cannot
touch resources that were in enabled or unknown state at boot how can
it allocate resources for newly loaded drivers?

Or to put it differently: when the firmware inserts dynamically at
boot time or the DT writer statically at compile time the resources
which are needed by console then kernel knows that the *other* stuff
that has been enabled by firmware or for which it cannot be determined
if it's enabled or not can be reused.

For example, if the firmware loaded the kernel from mmc or just probed
mmc the mmc clocks have been likely left enabled. You need to
reprogram mmc clocks to access mmc most of the time. If you assume no
knowledge of hardware then you cannot know that mmc clocks are not
used for console and cannot touch them. Hence if loading a kms driver
fails in initrd you are needlessly locked out of loading system from
mmc.

I am sure you can find other examples for both clocks or other
resources so the solution to properly name what you need for the
console to keep running is more scalable.

If your board is simple you might get away with a static DT. If you do
not care about early graphics you can just drop it.

BTW this has *also* been discussed here, at length.

Thanks

Michal

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-10-02 14:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542D5C74.4050602@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

On Thu, Oct 02, 2014 at 04:08:52PM +0200, Hans de Goede wrote:
> > 2) delay the clock/regulator cleanup until after there is a fixed
> > window for device specific drivers to load in. Loading from initrd is
> > a fixed window.
> 
> As I already explained by example in another mail, this won't work:
> 
> "delaying over the initrd is not helpful. Not having the real driver
> load for whatever reasons, is not necessarily a boot blocking event,
> and if it us just missing will not lead to any error messages.
> 
> So the boot will continue normally with a black screen, and things are
> still impossible to debug."

Plus:

1) you might not have an initrd, which doesn't change a thing: your
clock get disabled before you can load your driver

2) you might not have a rootfs, and no driver to load, which would
leave the clock enabled forever.

> We've been down the whole delay till $random point in time thing in the
> past with storage enumeration, where you need to wait for say all members
> of a raid set to show up. The lesson learned from that is that you should
> not wait $random time / event, but wait for the actual storage device to
> show up.
> 
> And this is just like that, we need to wait for the actual display driver
> to have loaded and taken over before "cleaning up" the clocks used by
> the display engine.
> 
> I guess we could just delay all clock cleanup until then, not really
> pretty, and I still prefer to just list the clocks in the simplefb
> dtnode, but if this version would be acceptable to all involved, I can
> live with it.
> 
> Mike, would a patch adding 2 calls like these to the clock core be
> acceptable ?  :
> 
> clk_block_disable_unused()
> clk_unblock_disable_unused()

Thierry actually already made such a patch somewhere in this
thread. The thing is that it should also block clk_disable (and all
the potential users of clk_disable: clk_set_rate, clk_set_parent,
etc.) from actually disabling them. Otherwise, you might still end up
with your clock disabled.

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: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-02 14:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OxHeHszmcbMw_LRJbcaFW5JP7Atsa4RMRwWfqMrCueAnQ@mail.gmail.com>

Hi,

On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 03:40 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 03:27 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl@gmail.com wrote:
>>>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>>> ...
>>>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>>>
>>>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>>>
>>>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>>>
>>>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>>>
>>>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>>>
>>>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>>>
>>>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>>>
>>>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>>>
>>>>>>>>> You need both.
>>>>>>>>>
>>>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>>>
>>>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>>>> set_console() but it's been a while.
>>>>>>>>
>>>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>>>
>>>>>>> No, that is where you get into trouble. The device specific driver has
>>>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>>>> as possible.
>>>>>>
>>>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>>>> say I can, deadlock.
>>>>>
>>>>> It is certainly possible to extend an earlyframebuffer to be able to
>>>>> run as a user space console. It is just going to turn into a
>>>>> Frankenmonster driver with piles of device specific, special case code
>>>>> in it.
>>>>
>>>> There is nothing hardware specific about a framebuffer needing some
>>>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>>>> that is hardware specific, but the framebuffer driver does not need to
>>>> worry about that, it just sees a clocks property with some random clocks
>>>> in there, and that is as generic as it gets.
>>>>
>>>>> I think that device specific code belongs in a device specific driver
>>>>> and earlyframebuffer should handoff to it as soon as possible.
>>>>>
>>>>>>
>>>>>> I've already explained that we not only can do that (we already have working
>>>>>> code proving that), but also that this is something which we absolutely need:
>>>>>>
>>>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>>>> is that the right module is not included in the initrd.
>>>>>
>>>>> A generic earlyframebuffer would show this error.
>>>>
>>>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>>>> be disabled before the initrd starts, and the screen will be black from then
>>>
>>> I thought the clock/regulator clean up happened after initrd loading,
>>> but maybe that is not the case.
>>>
>>> A cleaner solution would then be to modify the clock/regulator clean
>>> up to happen after driver loading is finished from initrd. Deferring
>>> until after that completes is a fixed limit, everything is sitting
>>> there in RAM. I would not propose extending it until harddisk based
>>> loading happens.
>>>
>>> So there are two ways to do this...
>>> 1) modify things like earlyconsole to protect device specific resource
>>> (I think this is a bad idea)
>>
>> Why is this a bad idea? If the bootloader tells us exactly which resources
>> are needed, then earlyconsole can claim them, and release them on
>> handover to the real display driver.

Jon, can you please answer this ? I really really want to know why people
think this is such a bad idea. Understanding why people think this is a bad
idea is necessary to be able to come up with an alternative solution.

>>
>>> 2) delay the clock/regulator cleanup until after there is a fixed
>>> window for device specific drivers to load in. Loading from initrd is
>>> a fixed window.
>>
>> As I already explained by example in another mail, this won't work:
>>
>> "delaying over the initrd is not helpful. Not having the real driver
>> load for whatever reasons, is not necessarily a boot blocking event,
>> and if it us just missing will not lead to any error messages.
>>
>> So the boot will continue normally with a black screen, and things are
>> still impossible to debug."
>>
>> We've been down the whole delay till $random point in time thing in the
>> past with storage enumeration, where you need to wait for say all members
>> of a raid set to show up. The lesson learned from that is that you should
>> not wait $random time / event, but wait for the actual storage device to
>> show up.
>>
>> And this is just like that, we need to wait for the actual display driver
>> to have loaded and taken over before "cleaning up" the clocks used by
>> the display engine.
>>
>> I guess we could just delay all clock cleanup until then, not really
>> pretty, and I still prefer to just list the clocks in the simplefb
>> dtnode, but if this version would be acceptable to all involved, I can
>> live with it.
>>
>> Mike, would a patch adding 2 calls like these to the clock core be
>> acceptable ?  :
>>
>> clk_block_disable_unused()
>> clk_unblock_disable_unused()
>>
>> Where an earlyconsole driver can then call clk_block_disable_unused(),
>> which will turn any calls to clk_disable_unused into a nop.
> 
> Is there a way to use the existing eprobe_defer system to do this?

No, that is not what it is intended for (at all). If we go this way
2 subsystem specific calls for this is the best way to deal with this.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-02 14:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20141002141849.GD4039@lukather>

Hi,

On 10/02/2014 04:18 PM, Maxime Ripard wrote:
> On Thu, Oct 02, 2014 at 04:08:52PM +0200, Hans de Goede wrote:
>>> 2) delay the clock/regulator cleanup until after there is a fixed
>>> window for device specific drivers to load in. Loading from initrd is
>>> a fixed window.
>>
>> As I already explained by example in another mail, this won't work:
>>
>> "delaying over the initrd is not helpful. Not having the real driver
>> load for whatever reasons, is not necessarily a boot blocking event,
>> and if it us just missing will not lead to any error messages.
>>
>> So the boot will continue normally with a black screen, and things are
>> still impossible to debug."
> 
> Plus:
> 
> 1) you might not have an initrd, which doesn't change a thing: your
> clock get disabled before you can load your driver
> 
> 2) you might not have a rootfs, and no driver to load, which would
> leave the clock enabled forever.
> 
>> We've been down the whole delay till $random point in time thing in the
>> past with storage enumeration, where you need to wait for say all members
>> of a raid set to show up. The lesson learned from that is that you should
>> not wait $random time / event, but wait for the actual storage device to
>> show up.
>>
>> And this is just like that, we need to wait for the actual display driver
>> to have loaded and taken over before "cleaning up" the clocks used by
>> the display engine.
>>
>> I guess we could just delay all clock cleanup until then, not really
>> pretty, and I still prefer to just list the clocks in the simplefb
>> dtnode, but if this version would be acceptable to all involved, I can
>> live with it.
>>
>> Mike, would a patch adding 2 calls like these to the clock core be
>> acceptable ?  :
>>
>> clk_block_disable_unused()
>> clk_unblock_disable_unused()
> 
> Thierry actually already made such a patch somewhere in this
> thread. The thing is that it should also block clk_disable (and all
> the potential users of clk_disable: clk_set_rate, clk_set_parent,
> etc.) from actually disabling them. Otherwise, you might still end up
> with your clock disabled.

A valid point, which brings us back to simply adding the clocks to the
dt node really being both the simplest solution, as well as the only
solution without any pitfalls.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-02 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542D5F62.60907@redhat.com>

On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 03:40 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 03:27 PM, jonsmirl@gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 10/02/2014 02:56 PM, jonsmirl@gmail.com wrote:
>>>>>>>> On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 10/02/2014 02:22 PM, jonsmirl@gmail.com wrote:
>>>>>>>>>> On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> On 10/01/2014 08:12 PM, Stephen Warren wrote:
>>>>>>>>>>>> On 10/01/2014 11:54 AM, jonsmirl@gmail.com wrote:
>>>>>>>>>>>>> On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>>>> ...
>>>>>>>>>>>>>> We've been over all this again and again and again.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> AAAARRRRRGGGGGGGGGGGGGGGGGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> All solutions provided sofar are both tons more complicated, then the
>>>>>>>>>>>>>> simple solution of simply having the simplefb dt node declare which
>>>>>>>>>>>>>> clocks it needs. And to make things worse all of them sofar have
>>>>>>>>>>>>>> unresolved issues (due to their complexity mostly).
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> With the clocks in the simplefb node, then all a real driver has to do,
>>>>>>>>>>>>>> is claim those same clocks before unregistering the simplefb driver,
>>>>>>>>>>>>>> and everything will just work.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Yet we've been discussing this for months, all because of some
>>>>>>>>>>>>>> vague worries from Thierry, and *only* from Thierry that this will
>>>>>>>>>>>>>> make simplefb less generic / not abstract enough, while a simple
>>>>>>>>>>>>>> generic clocks property is about as generic as things come.
>>>>>>>>>>>>
>>>>>>>>>>>> Note: I haven't been following this thread, and really don't have the time to get involved, but I did want to point out one thing:
>>>>>>>>>>>>
>>>>>>>>>>>> As I think I mentioned very early on in this thread, one of the big concerns when simplefb was merged was that it would slowly grow and become a monster. As such, a condition of merging it was that it would not grow features like resource management at all. That means no clock/regulator/... support. It's intended as a simple stop-gap between early platform bringup and whenever a real driver exists for the HW. If you need resource management, write a HW-specific driver. The list archives presumably have a record of the discussion, but I don't know the links off the top of my head. If nobody
>>>>>>>>>>>> other than Thierry is objecting, presumably the people who originally objected simply haven't noticed this patch/thread. I suppose it's possible they changed their mind.
>>>>>>>>>>>>
>>>>>>>>>>>> BTW, there's no reason that the simplefb code couldn't be refactored out into a support library that's used by both the simplefb we currently have and any new HW-specific driver. It's just that the simplefb binding and driver shouldn't grow.
>>>>>>>>>>>
>>>>>>>>>>> The whole reason why we want to use simplefb is not just to get things
>>>>>>>>>>> running until HW specific driver is in place, but also to have early console
>>>>>>>>>>> output (to help debugging boot problems on devices without a serial console),
>>>>>>>>>>> in a world where most video drivers are build as loadable modules, so we
>>>>>>>>>>> won't have video output until quite late into the boot process.
>>>>>>>>>>
>>>>>>>>>> You need both.
>>>>>>>>>>
>>>>>>>>>> 1) temporary early boot console -- this is nothing but an address in
>>>>>>>>>> RAM and the x/y layout. The character set from framebuffer is built
>>>>>>>>>> into the kernel.  The parallel to this is early-printk and how it uses
>>>>>>>>>> the UARTs without interrupts. This console vaporizes late in the boot
>>>>>>>>>> process -- the same thing happens with the early printk UART driver.
>>>>>>>>>> EARLYPRINTK on the command line enables this.
>>>>>>>>>>
>>>>>>>>>> 2) a device specific driver -- this sits on initrd and it loaded as
>>>>>>>>>> soon as possible. The same thing happens with the real UART driver for
>>>>>>>>>> the console. CONSOLE= on the command line causes the transition. There
>>>>>>>>>> is an API in the kernel to do this transition, I believe it is called
>>>>>>>>>> set_console() but it's been a while.
>>>>>>>>>
>>>>>>>>> Eventually we need both, yes. But 1) should stay working until 2) loads,
>>>>>>>>> not until some phase of the bootup is completed, but simply until 2) loads.
>>>>>>>>
>>>>>>>> No, that is where you get into trouble. The device specific driver has
>>>>>>>> to go onto initrd where it can be loaded as early in the boot process
>>>>>>>> as possible.
>>>>>>>
>>>>>>> This is an argument in the "you cannot do that" / "your use case is not valid"
>>>>>>> category, IOW this is not a technical argument. You say I cannot do that I
>>>>>>> say I can, deadlock.
>>>>>>
>>>>>> It is certainly possible to extend an earlyframebuffer to be able to
>>>>>> run as a user space console. It is just going to turn into a
>>>>>> Frankenmonster driver with piles of device specific, special case code
>>>>>> in it.
>>>>>
>>>>> There is nothing hardware specific about a framebuffer needing some
>>>>> clocks to not be disabled. Tons of SoC's will have this. Which clocks,
>>>>> that is hardware specific, but the framebuffer driver does not need to
>>>>> worry about that, it just sees a clocks property with some random clocks
>>>>> in there, and that is as generic as it gets.
>>>>>
>>>>>> I think that device specific code belongs in a device specific driver
>>>>>> and earlyframebuffer should handoff to it as soon as possible.
>>>>>>
>>>>>>>
>>>>>>> I've already explained that we not only can do that (we already have working
>>>>>>> code proving that), but also that this is something which we absolutely need:
>>>>>>>
>>>>>>>>> One example why this is necessary is e.g. to debug things where the problem
>>>>>>>>> is that the right module is not included in the initrd.
>>>>>>
>>>>>> A generic earlyframebuffer would show this error.
>>>>>
>>>>> If it reserves the clocks it needs, yes. If it does not then the clocks will
>>>>> be disabled before the initrd starts, and the screen will be black from then
>>>>
>>>> I thought the clock/regulator clean up happened after initrd loading,
>>>> but maybe that is not the case.
>>>>
>>>> A cleaner solution would then be to modify the clock/regulator clean
>>>> up to happen after driver loading is finished from initrd. Deferring
>>>> until after that completes is a fixed limit, everything is sitting
>>>> there in RAM. I would not propose extending it until harddisk based
>>>> loading happens.
>>>>
>>>> So there are two ways to do this...
>>>> 1) modify things like earlyconsole to protect device specific resource
>>>> (I think this is a bad idea)
>>>
>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>> are needed, then earlyconsole can claim them, and release them on
>>> handover to the real display driver.
>
> Jon, can you please answer this ? I really really want to know why people
> think this is such a bad idea. Understanding why people think this is a bad
> idea is necessary to be able to come up with an alternative solution.

The list of resources should not be duplicated in the device tree -
once in the simplefb node and again in the real device node. Device
tree is a hardware description and it is being twisted to solve a
software issue. This problem is not limited to clocks, same problem
exists with regulators. On SGI systems this would exist with entire
bus controllers (but they are x86 based, console is not on the root
bus). This is a very messy problem and will lead to a Frankenstein
sized driver over time.

But... I think this is a red herring which is masking the real
problem. The real problem seems to be that there is no window for
loading device specific drivers before the resource clean up phase
happens. That's a real problem -- multi architecture distros are going
to have lots of loadable device specific drivers.

This leads me into thinking that the resource cleanup is not being
done at the right time. Maybe it should be part of the housekeeping
that goes on in the system idle task or something like that. I don't
work enough with the user space transition to know the right place to
move it to.

If we can get this resource clean up sorted out, the problem with
simplefb needing a protected resource list disappears. It make sense
to me that we have to have some way of loading device specific drivers
before this clean up happens.

I'll add Linus to the list, maybe he'll give us some guidance on how
to handle this.

>
>>>
>>>> 2) delay the clock/regulator cleanup until after there is a fixed
>>>> window for device specific drivers to load in. Loading from initrd is
>>>> a fixed window.
>>>
>>> As I already explained by example in another mail, this won't work:
>>>
>>> "delaying over the initrd is not helpful. Not having the real driver
>>> load for whatever reasons, is not necessarily a boot blocking event,
>>> and if it us just missing will not lead to any error messages.
>>>
>>> So the boot will continue normally with a black screen, and things are
>>> still impossible to debug."
>>>
>>> We've been down the whole delay till $random point in time thing in the
>>> past with storage enumeration, where you need to wait for say all members
>>> of a raid set to show up. The lesson learned from that is that you should
>>> not wait $random time / event, but wait for the actual storage device to
>>> show up.
>>>
>>> And this is just like that, we need to wait for the actual display driver
>>> to have loaded and taken over before "cleaning up" the clocks used by
>>> the display engine.
>>>
>>> I guess we could just delay all clock cleanup until then, not really
>>> pretty, and I still prefer to just list the clocks in the simplefb
>>> dtnode, but if this version would be acceptable to all involved, I can
>>> live with it.
>>>
>>> Mike, would a patch adding 2 calls like these to the clock core be
>>> acceptable ?  :
>>>
>>> clk_block_disable_unused()
>>> clk_unblock_disable_unused()
>>>
>>> Where an earlyconsole driver can then call clk_block_disable_unused(),
>>> which will turn any calls to clk_disable_unused into a nop.
>>
>> Is there a way to use the existing eprobe_defer system to do this?
>
> No, that is not what it is intended for (at all). If we go this way
> 2 subsystem specific calls for this is the best way to deal with this.
>
> Regards,
>
> Hans
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-02 14:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4Oz8mL5jGzBHZeznd5NbPqq93q7PpTDqHHcV_HYSfqBVHA@mail.gmail.com>

Hi,

On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:

<snip>

>>>>> So there are two ways to do this...
>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>> (I think this is a bad idea)
>>>>
>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>> are needed, then earlyconsole can claim them, and release them on
>>>> handover to the real display driver.
>>
>> Jon, can you please answer this ? I really really want to know why people
>> think this is such a bad idea. Understanding why people think this is a bad
>> idea is necessary to be able to come up with an alternative solution.
> 
> The list of resources should not be duplicated in the device tree -
> once in the simplefb node and again in the real device node.

It is not duplicated, the simplefb node will list the clocks used for the
mode / output as setup by the firmware, which are often not all clocks
which the display engine supports. Where as the real device node will list
all clocks the display engine may use.

> Device
> tree is a hardware description and it is being twisted to solve a
> software issue.

This is not true, various core devicetree developers have already said
that storing other info in the devicetree is fine, and being able to do so
is part of the original design.

> This problem is not limited to clocks, same problem
> exists with regulators. On SGI systems this would exist with entire
> bus controllers (but they are x86 based, console is not on the root
> bus). This is a very messy problem and will lead to a Frankenstein
> sized driver over time.

This is a "what if ..." argument, we can discuss potential hypothetical
problems all day long, what happens if the sky falls down?

> But... I think this is a red herring which is masking the real
> problem. The real problem seems to be that there is no window for
> loading device specific drivers before the resource clean up phase
> happens. That's a real problem -- multi architecture distros are going
> to have lots of loadable device specific drivers.

As Maxime pointed out to my alternative solution to fixing the clocks
problem, this is not strictly a when to do cleanup problem. If another
driver uses the same clocks, and does a clk_disable call after probing
(because the device is put in low power mode until used by userspace),
then the clk will be disabled even without any cleanup running at all.

The real problem here is simply that to work the simplefb needs certain
resources, just like any other device. And while for any other device
simply listing the needed resources is an accepted practice, for simplefb
for some reason (which I still do not understand) people all of a sudden
see listing resources as a problem.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-02 15:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542D6633.20907@redhat.com>

On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>
> <snip>
>
>>>>>> So there are two ways to do this...
>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>> (I think this is a bad idea)
>>>>>
>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>> handover to the real display driver.
>>>
>>> Jon, can you please answer this ? I really really want to know why people
>>> think this is such a bad idea. Understanding why people think this is a bad
>>> idea is necessary to be able to come up with an alternative solution.
>>
>> The list of resources should not be duplicated in the device tree -
>> once in the simplefb node and again in the real device node.
>
> It is not duplicated, the simplefb node will list the clocks used for the
> mode / output as setup by the firmware, which are often not all clocks
> which the display engine supports. Where as the real device node will list
> all clocks the display engine may use.
>
>> Device
>> tree is a hardware description and it is being twisted to solve a
>> software issue.
>
> This is not true, various core devicetree developers have already said
> that storing other info in the devicetree is fine, and being able to do so
> is part of the original design.
>
>> This problem is not limited to clocks, same problem
>> exists with regulators. On SGI systems this would exist with entire
>> bus controllers (but they are x86 based, console is not on the root
>> bus). This is a very messy problem and will lead to a Frankenstein
>> sized driver over time.
>
> This is a "what if ..." argument, we can discuss potential hypothetical
> problems all day long, what happens if the sky falls down?
>
>> But... I think this is a red herring which is masking the real
>> problem. The real problem seems to be that there is no window for
>> loading device specific drivers before the resource clean up phase
>> happens. That's a real problem -- multi architecture distros are going
>> to have lots of loadable device specific drivers.
>
> As Maxime pointed out to my alternative solution to fixing the clocks
> problem, this is not strictly a when to do cleanup problem. If another
> driver uses the same clocks, and does a clk_disable call after probing
> (because the device is put in low power mode until used by userspace),
> then the clk will be disabled even without any cleanup running at all.
>
> The real problem here is simply that to work the simplefb needs certain
> resources, just like any other device. And while for any other device
> simply listing the needed resources is an accepted practice, for simplefb
> for some reason (which I still do not understand) people all of a sudden
> see listing resources as a problem.

Because you are creating two different device tree nodes describing a
single piece of hardware and that's not suppose to happen in a device
tree.  The accurate description of the hardware is being perverted to
solve a software problem.

One node describes the hardware in a format to make simplefb happy.
Another node describes the same hardware in a format to make the
device specific driver happy.


>
> Regards,
>
> Hans
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-10-02 15:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OwMf-zXC_B=VqdVNrD_ExC3zGiGUm4j5KKgS-U4cPB2zg@mail.gmail.com>

On Thu, Oct 2, 2014 at 11:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> <snip>
>>
>>>>>>> So there are two ways to do this...
>>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>>> (I think this is a bad idea)
>>>>>>
>>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>>> handover to the real display driver.
>>>>
>>>> Jon, can you please answer this ? I really really want to know why people
>>>> think this is such a bad idea. Understanding why people think this is a bad
>>>> idea is necessary to be able to come up with an alternative solution.
>>>
>>> The list of resources should not be duplicated in the device tree -
>>> once in the simplefb node and again in the real device node.
>>
>> It is not duplicated, the simplefb node will list the clocks used for the
>> mode / output as setup by the firmware, which are often not all clocks
>> which the display engine supports. Where as the real device node will list
>> all clocks the display engine may use.
>>
>>> Device
>>> tree is a hardware description and it is being twisted to solve a
>>> software issue.
>>
>> This is not true, various core devicetree developers have already said
>> that storing other info in the devicetree is fine, and being able to do so
>> is part of the original design.
>>
>>> This problem is not limited to clocks, same problem
>>> exists with regulators. On SGI systems this would exist with entire
>>> bus controllers (but they are x86 based, console is not on the root
>>> bus). This is a very messy problem and will lead to a Frankenstein
>>> sized driver over time.
>>
>> This is a "what if ..." argument, we can discuss potential hypothetical
>> problems all day long, what happens if the sky falls down?
>>
>>> But... I think this is a red herring which is masking the real
>>> problem. The real problem seems to be that there is no window for
>>> loading device specific drivers before the resource clean up phase
>>> happens. That's a real problem -- multi architecture distros are going
>>> to have lots of loadable device specific drivers.
>>
>> As Maxime pointed out to my alternative solution to fixing the clocks
>> problem, this is not strictly a when to do cleanup problem. If another
>> driver uses the same clocks, and does a clk_disable call after probing
>> (because the device is put in low power mode until used by userspace),
>> then the clk will be disabled even without any cleanup running at all.
>>
>> The real problem here is simply that to work the simplefb needs certain
>> resources, just like any other device. And while for any other device
>> simply listing the needed resources is an accepted practice, for simplefb
>> for some reason (which I still do not understand) people all of a sudden
>> see listing resources as a problem.
>
> Because you are creating two different device tree nodes describing a
> single piece of hardware and that's not suppose to happen in a device
> tree.  The accurate description of the hardware is being perverted to
> solve a software problem.
>
> One node describes the hardware in a format to make simplefb happy.
> Another node describes the same hardware in a format to make the
> device specific driver happy.

But... I think all of this device tree stuff is a red herring and not
the core problem.

Core problem.....

Bios sets stuff up
Built-in drivers initialize
Bios settings get cleaned up (display goes blank)
Loadable drivers initialize (display comes back)

In multi-architecture kernels almost all of the drivers are loadable.
We need to figure out how to change the order....

Bios sets stuff up
Built-in drivers initialize
Loadable drivers initialize
Bios settings get cleaned up

Maybe the Bios cleanup turns into a small app you place at the end of
your init scripts. It's just a power saving cleanup and shouldn't be
causing this much trouble.

I don't think leaving the order as is and using the device tree to
construct a big list of exceptions to the clean up process is the
right approach.

>
>
>>
>> Regards,
>>
>> Hans
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>
>
> --
> Jon Smirl
> jonsmirl@gmail.com



-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-10-02 15:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OwMf-zXC_B=VqdVNrD_ExC3zGiGUm4j5KKgS-U4cPB2zg@mail.gmail.com>

On 2 October 2014 17:30, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Oct 2, 2014 at 11:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> <snip>
>>>
>>>>>>>> So there are two ways to do this...
>>>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>>>> (I think this is a bad idea)
>>>>>>>
>>>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>>>> handover to the real display driver.
>>>>>
>>>>> Jon, can you please answer this ? I really really want to know why people
>>>>> think this is such a bad idea. Understanding why people think this is a bad
>>>>> idea is necessary to be able to come up with an alternative solution.
>>>>
>>>> The list of resources should not be duplicated in the device tree -
>>>> once in the simplefb node and again in the real device node.
>>>
>>> It is not duplicated, the simplefb node will list the clocks used for the
>>> mode / output as setup by the firmware, which are often not all clocks
>>> which the display engine supports. Where as the real device node will list
>>> all clocks the display engine may use.
>>>
>>>> Device
>>>> tree is a hardware description and it is being twisted to solve a
>>>> software issue.
>>>
>>> This is not true, various core devicetree developers have already said
>>> that storing other info in the devicetree is fine, and being able to do so
>>> is part of the original design.
>>>
>>>> This problem is not limited to clocks, same problem
>>>> exists with regulators. On SGI systems this would exist with entire
>>>> bus controllers (but they are x86 based, console is not on the root
>>>> bus). This is a very messy problem and will lead to a Frankenstein
>>>> sized driver over time.
>>>
>>> This is a "what if ..." argument, we can discuss potential hypothetical
>>> problems all day long, what happens if the sky falls down?
>>>
>>>> But... I think this is a red herring which is masking the real
>>>> problem. The real problem seems to be that there is no window for
>>>> loading device specific drivers before the resource clean up phase
>>>> happens. That's a real problem -- multi architecture distros are going
>>>> to have lots of loadable device specific drivers.
>>>
>>> As Maxime pointed out to my alternative solution to fixing the clocks
>>> problem, this is not strictly a when to do cleanup problem. If another
>>> driver uses the same clocks, and does a clk_disable call after probing
>>> (because the device is put in low power mode until used by userspace),
>>> then the clk will be disabled even without any cleanup running at all.
>>>
>>> The real problem here is simply that to work the simplefb needs certain
>>> resources, just like any other device. And while for any other device
>>> simply listing the needed resources is an accepted practice, for simplefb
>>> for some reason (which I still do not understand) people all of a sudden
>>> see listing resources as a problem.
>>
>> Because you are creating two different device tree nodes describing a
>> single piece of hardware and that's not suppose to happen in a device
>> tree.  The accurate description of the hardware is being perverted to
>> solve a software problem.
>>
>> One node describes the hardware in a format to make simplefb happy.
>> Another node describes the same hardware in a format to make the
>> device specific driver happy.

No.

one node describes the state in which the hardware was left by u-boot
and other node describes the display hardware in full.

Obviously, this will overlap but is not duplication.

Or as pointed out the simplefb node is not hardware description but
'other information' which is part of the DT design. If your system
does not use simplefb it can be ignored with no ill side effect (once
the part with claiming memory for simplefb cleanly is resolved).

Did we not discuss that several times already?

>
> But... I think all of this device tree stuff is a red herring and not
> the core problem.
>
> Core problem.....
>
> Bios sets stuff up
> Built-in drivers initialize
> Bios settings get cleaned up (display goes blank)
> Loadable drivers initialize (display comes back)
>
> In multi-architecture kernels almost all of the drivers are loadable.
> We need to figure out how to change the order....
>
> Bios sets stuff up
> Built-in drivers initialize
> Loadable drivers initialize
> Bios settings get cleaned up
>
> Maybe the Bios cleanup turns into a small app you place at the end of
> your init scripts. It's just a power saving cleanup and shouldn't be
> causing this much trouble.

No. You need to do cleanup during driver loading as well. So not doing
the cleanup early gives you more bizarre problems.

Did we not discuss that several times as well?

Thanks

Michal

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-02 15:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OyvUeSidyR8OWk9BM4X3BmvzSLyZtbhnMaB75kr-AFuew@mail.gmail.com>

Hi,

On 10/02/2014 05:30 PM, jonsmirl@gmail.com wrote:
> On Thu, Oct 2, 2014 at 11:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> <snip>
>>>
>>>>>>>> So there are two ways to do this...
>>>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>>>> (I think this is a bad idea)
>>>>>>>
>>>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>>>> handover to the real display driver.
>>>>>
>>>>> Jon, can you please answer this ? I really really want to know why people
>>>>> think this is such a bad idea. Understanding why people think this is a bad
>>>>> idea is necessary to be able to come up with an alternative solution.
>>>>
>>>> The list of resources should not be duplicated in the device tree -
>>>> once in the simplefb node and again in the real device node.
>>>
>>> It is not duplicated, the simplefb node will list the clocks used for the
>>> mode / output as setup by the firmware, which are often not all clocks
>>> which the display engine supports. Where as the real device node will list
>>> all clocks the display engine may use.
>>>
>>>> Device
>>>> tree is a hardware description and it is being twisted to solve a
>>>> software issue.
>>>
>>> This is not true, various core devicetree developers have already said
>>> that storing other info in the devicetree is fine, and being able to do so
>>> is part of the original design.
>>>
>>>> This problem is not limited to clocks, same problem
>>>> exists with regulators. On SGI systems this would exist with entire
>>>> bus controllers (but they are x86 based, console is not on the root
>>>> bus). This is a very messy problem and will lead to a Frankenstein
>>>> sized driver over time.
>>>
>>> This is a "what if ..." argument, we can discuss potential hypothetical
>>> problems all day long, what happens if the sky falls down?
>>>
>>>> But... I think this is a red herring which is masking the real
>>>> problem. The real problem seems to be that there is no window for
>>>> loading device specific drivers before the resource clean up phase
>>>> happens. That's a real problem -- multi architecture distros are going
>>>> to have lots of loadable device specific drivers.
>>>
>>> As Maxime pointed out to my alternative solution to fixing the clocks
>>> problem, this is not strictly a when to do cleanup problem. If another
>>> driver uses the same clocks, and does a clk_disable call after probing
>>> (because the device is put in low power mode until used by userspace),
>>> then the clk will be disabled even without any cleanup running at all.
>>>
>>> The real problem here is simply that to work the simplefb needs certain
>>> resources, just like any other device. And while for any other device
>>> simply listing the needed resources is an accepted practice, for simplefb
>>> for some reason (which I still do not understand) people all of a sudden
>>> see listing resources as a problem.
>>
>> Because you are creating two different device tree nodes describing a
>> single piece of hardware and that's not suppose to happen in a device
>> tree.

That again is a very narrow reading of what is a very generic descriptive
language. Also note that we are already in this situation with simplefb,
all we're advocating is extending the info which is in the simplefb node
so that it is actually usable in a much wider range of scenarios.

>> The accurate description of the hardware is being perverted to
>> solve a software problem.

Again, devicetree is not strictly a hardware description language.

>> One node describes the hardware in a format to make simplefb happy.
>> Another node describes the same hardware in a format to make the
>> device specific driver happy.
> 
> But... I think all of this device tree stuff is a red herring and not
> the core problem.

Actually the red herring is people focussing on the init ordering
solution, which as already explained will simply never work, quoting
myself (from above):

>>> As Maxime pointed out to my alternative solution to fixing the clocks
>>> problem, this is not strictly a when to do cleanup problem. If another
>>> driver uses the same clocks, and does a clk_disable call after probing
>>> (because the device is put in low power mode until used by userspace),
>>> then the clk will be disabled even without any cleanup running at all.

And to repeat myself yet again:

"The real problem here is simply that to work the simplefb needs certain
resources, just like any other device."

So the logical thing to do is to just put the clocks in the node. The only
counter argument I hear you make is "this is not a hardware description,
so it does not belong in devicetree". An argument which has already
been discussed earlier in this thread (about a month ago) and one of the
officialo devicetree maintainers responded to that saying that it is fine
to have non hardware info in the dt.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Stephen Warren @ 2014-10-02 15:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542CF3C3.2050708@redhat.com>

On 10/02/2014 12:42 AM, Hans de Goede wrote:
...
> The whole reason why we want to use simplefb is not just to get things
> running until HW specific driver is in place, but also to have early console
> output (to help debugging boot problems on devices without a serial console),
> in a world where most video drivers are build as loadable modules, so we
> won't have video output until quite late into the boot process.

That's a very different use-case than what was originally envisaged.

...
> You indicate that you don't have the time for this discussion, and I note that
> there is no MAINTAINERS entry for drivers/video/fbdev/simplefb.c . So how about
> the following, I pick up drivers/video/fbdev/simplefb.c maintainership, adding
> MAINTAINERS entry for it with my name in it. Then as the maintainer it will be
> my responsibility (and in my own benefit) to stop this from growing into
> a monster ?

I have no issue with you being the maintainer.

I would suggest you track down whoever it was that was involved in the 
original discussion and objected to simplefb performing resource 
management, and get their explicit ack for the addition of 
clocks/regulators/... to it. But, that's just a suggestion.

^ permalink raw reply

* FOSDEM15: Graphics DevRoom: call for speakers.
From: Luc Verhaegen @ 2014-10-02 17:44 UTC (permalink / raw)
  To: mesa-dev, xorg-devel, dri-devel, xorg, wayland-devel,
	xorg-announce, mir-devel, directfb-dev, linux-fbdev, linux-media

Hi,

At FOSDEM on the 31st of january and the 1st of February 2015, there 
will be another graphics DevRoom. URL: https://fosdem.org/2015/

The focus of this DevRoom is of course the same as last year, namely:
* Graphics drivers: from display to media to 3d drivers, both in kernel 
  or userspace. Be it part of DRM, KMS, (direct)FB, V4L, Xorg, Mesa...
* Input drivers: kernel and userspace.
* Windowing systems: X, Wayland, Mir, directFB, ...
* Even colour management and other areas which i might have overlooked 
  above are accepted.

Slots are 50 minutes long, and scheduled hourly. This partly to avoid 
confusion and people running all over the place all the time. As a 
speaker, you do not have to fill your whole hour, gaps are never wasted 
time.

Slots will be handed out on a first come, first serve basis. The best 
slots will go to those who apply the earliest. The amount of slots is 
currently not known yet, but i expect there to be around 16 available (8 
on each day), so act quickly.

Talk Submission:
----------------

Like last year, the pentabarf system will be used for talk submission. 
It is not perfect from a devroom organizer and talk submitters usability 
point-of-view, but the fosdem organizers are working on it. It is 
however workable and it ended up working out pretty well last year.

https://penta.fosdem.org/submission/FOSDEM15

Remember that FOSDEM is not like XDC, it's not some 50 odd people 
meeting with a sliding schedule which only gets filled out on the last 
day. Upwards of 8000 people are visiting this event, and most of them 
get a printed booklet or use the schedule on the FOSDEM website or an 
app for their phone to figure out what to watch or participate in next. 
So please put some effort in your talk submission and details.

Since this an open source community event, please refrain from turning 
in a talk that is a pure corporate or product commercial. Also, if you 
are unsure on whether you can come or not (this is FOSDEM, why are you 
not there anyway?), please wait with submitting your talk. Submitting a 
talk and then not turning up because you could not be bothered is a 
sure-fire way to get larted and then to never be allowed to talk again.

As for deadlines, i hope to have a pretty much complete schedule between 
christmas and the new year. The rockhard printed schedule deadline is 
probably January 9th, after that you will not be featured in the booklet 
and you will have a lot less visitors. I will hopefully be able to lock 
down entries and descriptions after that date.

Don't count on this deadline: first come first serve! There are perhaps 
only 16 slots. And the worst slots will be assigned to those who come 
last. Do you really want to talk on saturday at 10:00 when people are 
still in zombie mode after the beer event, if they are there at all?

Use your account from last year, so you can try to recycle some of your 
data from last year. If you have forgotten your password, then you can 
reset it here: https://penta.fosdem.org/user/forgot_password

Necessary information:
----------------------

Below is a list of what i need to see filled in when you apply for a 
devroom before i consider it a valid submission. Remember: first come, 
first serve. The best slots are for the earliest submissions and there 
are only around 16 slots.

On your personal page:
* General:
  * First and last name
  * Nickname
  * Image
* Contact:
  * email
  * mobile number (this is a very hard requirement as there will be no 
    other reliable form of emergency communication on the day)
* Description:
  * Abstract
  * Description

Create an event:
* On the General page:
  * Event title
  * Event subtitle.
  * Track: Graphics Devroom
  * Event type: Lecture (talk) or Meeting (BoF)
* Persons:
  * Add yourself as speaker.
* Description:
  * Abstract:
  * Full Description
* Links:
  * Add relevant links.

Everything else can be ignored or will be filled in by me or the FOSDEM 
organizers.

That's about it. Hope to see you all at FOSDEM :)

Luc Verhaegen.

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Geert Uytterhoeven @ 2014-10-02 19:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4Oy8sZ-d6JzWLi7pUeNFE5ge0fr5Tgoc_PSpKcOoiN-0yQ@mail.gmail.com>

On Thu, Oct 2, 2014 at 2:22 PM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> 1) temporary early boot console -- this is nothing but an address in
> RAM and the x/y layout. The character set from framebuffer is built
> into the kernel.  The parallel to this is early-printk and how it uses
> the UARTs without interrupts. This console vaporizes late in the boot
> process -- the same thing happens with the early printk UART driver.
> EARLYPRINTK on the command line enables this.

JFYI, the early serial console can also vanish, even very early in the boot
process, before the unused clocks are disabled.
Cfr. http://marc.info/?l=linux-sh&m\x141227657322649&w=2

So there's no safety in this world without calling clk_prepare_enable().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Julian Calaby @ 2014-10-02 23:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKON4OwMf-zXC_B=VqdVNrD_ExC3zGiGUm4j5KKgS-U4cPB2zg@mail.gmail.com>

Hi,

On Fri, Oct 3, 2014 at 1:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
> On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> <snip>
>>
>>>>>>> So there are two ways to do this...
>>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>>> (I think this is a bad idea)
>>>>>>
>>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>>> handover to the real display driver.
>>>>
>>>> Jon, can you please answer this ? I really really want to know why people
>>>> think this is such a bad idea. Understanding why people think this is a bad
>>>> idea is necessary to be able to come up with an alternative solution.
>>>
>>> The list of resources should not be duplicated in the device tree -
>>> once in the simplefb node and again in the real device node.
>>
>> It is not duplicated, the simplefb node will list the clocks used for the
>> mode / output as setup by the firmware, which are often not all clocks
>> which the display engine supports. Where as the real device node will list
>> all clocks the display engine may use.
>>
>>> Device
>>> tree is a hardware description and it is being twisted to solve a
>>> software issue.
>>
>> This is not true, various core devicetree developers have already said
>> that storing other info in the devicetree is fine, and being able to do so
>> is part of the original design.
>>
>>> This problem is not limited to clocks, same problem
>>> exists with regulators. On SGI systems this would exist with entire
>>> bus controllers (but they are x86 based, console is not on the root
>>> bus). This is a very messy problem and will lead to a Frankenstein
>>> sized driver over time.
>>
>> This is a "what if ..." argument, we can discuss potential hypothetical
>> problems all day long, what happens if the sky falls down?
>>
>>> But... I think this is a red herring which is masking the real
>>> problem. The real problem seems to be that there is no window for
>>> loading device specific drivers before the resource clean up phase
>>> happens. That's a real problem -- multi architecture distros are going
>>> to have lots of loadable device specific drivers.
>>
>> As Maxime pointed out to my alternative solution to fixing the clocks
>> problem, this is not strictly a when to do cleanup problem. If another
>> driver uses the same clocks, and does a clk_disable call after probing
>> (because the device is put in low power mode until used by userspace),
>> then the clk will be disabled even without any cleanup running at all.
>>
>> The real problem here is simply that to work the simplefb needs certain
>> resources, just like any other device. And while for any other device
>> simply listing the needed resources is an accepted practice, for simplefb
>> for some reason (which I still do not understand) people all of a sudden
>> see listing resources as a problem.
>
> Because you are creating two different device tree nodes describing a
> single piece of hardware and that's not suppose to happen in a device
> tree.  The accurate description of the hardware is being perverted to
> solve a software problem.
>
> One node describes the hardware in a format to make simplefb happy.
> Another node describes the same hardware in a format to make the
> device specific driver happy.

Stupid question: What about mangling an existing device node to be
simplefb compatible, and writing simplefb to be binding agnostic?

I listed some psuedo-code to do the latter earlier in the thread.

I.e. changing something like this:

vopb: vopb@ff930000 {
    compatible = "rockchip,rk3288-vop";
    reg = <0xff930000 0x19c>;
    interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
    clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
    resets = <&cru SRST_LCDC1_AXI>, <&cru SRST_LCDC1_AHB>, <&cru
SRST_LCDC1_DCLK>;
    reset-names = "axi", "ahb", "dclk";
    iommus = <&vopb_mmu>;
    vopb_out: port {
        #address-cells = <1>;
        #size-cells = <0>;
        vopb_out_edp: endpoint@0 {
            reg = <0>;
            remote-endpoint=<&edp_in_vopb>;
        };
        vopb_out_hdmi: endpoint@1 {
            reg = <1>;
            remote-endpoint=<&hdmi_in_vopb>;
        };
    };
};

into something like this:

vopb: vopb@ff930000 {
    compatible = "rockchip,rk3288-vop", "simple-framebuffer";
    framebuffer {
        reg = <0x1d385000 (1600 * 1200 * 2)>;
        width = <1600>;
        height = <1200>;
        stride = <(1600 * 2)>;
        format = "r5g6b5";
    };
    reg = <0xff930000 0x19c>;
    interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&cru ACLK_VOP0>, <&cru DCLK_VOP0>, <&cru HCLK_VOP0>;
    clock-names = "aclk_vop", "dclk_vop", "hclk_vop";
    resets = <&cru SRST_LCDC1_AXI>, <&cru SRST_LCDC1_AHB>, <&cru
SRST_LCDC1_DCLK>;
    reset-names = "axi", "ahb", "dclk";
    iommus = <&vopb_mmu>;
    vopb_out: port {
        #address-cells = <1>;
        #size-cells = <0>;
        vopb_out_edp: endpoint@0 {
            reg = <0>;
            remote-endpoint=<&edp_in_vopb>;
        };
        vopb_out_hdmi: endpoint@1 {
            reg = <1>;
            remote-endpoint=<&hdmi_in_vopb>;
        };
    };
};

And if the clocks are output port specific, we could add some lines
like "simple-framebuffer,ignore-node" to it so simplefb doesn't enable
HDMI clocks when we're using a build-in LCD.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-03  7:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGRGNgUUG6TOzC5YJgqqKfCM_iK9KcDXW9DPEr1P5OoO1qwd8w@mail.gmail.com>

Hi,

On 10/03/2014 01:31 AM, Julian Calaby wrote:
> Hi,
> 
> On Fri, Oct 3, 2014 at 1:14 AM, jonsmirl@gmail.com <jonsmirl@gmail.com> wrote:
>> On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 10/02/2014 04:41 PM, jonsmirl@gmail.com wrote:
>>>> On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 10/02/2014 04:16 PM, jonsmirl@gmail.com wrote:
>>>>>> On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> <snip>
>>>
>>>>>>>> So there are two ways to do this...
>>>>>>>> 1) modify things like earlyconsole to protect device specific resource
>>>>>>>> (I think this is a bad idea)
>>>>>>>
>>>>>>> Why is this a bad idea? If the bootloader tells us exactly which resources
>>>>>>> are needed, then earlyconsole can claim them, and release them on
>>>>>>> handover to the real display driver.
>>>>>
>>>>> Jon, can you please answer this ? I really really want to know why people
>>>>> think this is such a bad idea. Understanding why people think this is a bad
>>>>> idea is necessary to be able to come up with an alternative solution.
>>>>
>>>> The list of resources should not be duplicated in the device tree -
>>>> once in the simplefb node and again in the real device node.
>>>
>>> It is not duplicated, the simplefb node will list the clocks used for the
>>> mode / output as setup by the firmware, which are often not all clocks
>>> which the display engine supports. Where as the real device node will list
>>> all clocks the display engine may use.
>>>
>>>> Device
>>>> tree is a hardware description and it is being twisted to solve a
>>>> software issue.
>>>
>>> This is not true, various core devicetree developers have already said
>>> that storing other info in the devicetree is fine, and being able to do so
>>> is part of the original design.
>>>
>>>> This problem is not limited to clocks, same problem
>>>> exists with regulators. On SGI systems this would exist with entire
>>>> bus controllers (but they are x86 based, console is not on the root
>>>> bus). This is a very messy problem and will lead to a Frankenstein
>>>> sized driver over time.
>>>
>>> This is a "what if ..." argument, we can discuss potential hypothetical
>>> problems all day long, what happens if the sky falls down?
>>>
>>>> But... I think this is a red herring which is masking the real
>>>> problem. The real problem seems to be that there is no window for
>>>> loading device specific drivers before the resource clean up phase
>>>> happens. That's a real problem -- multi architecture distros are going
>>>> to have lots of loadable device specific drivers.
>>>
>>> As Maxime pointed out to my alternative solution to fixing the clocks
>>> problem, this is not strictly a when to do cleanup problem. If another
>>> driver uses the same clocks, and does a clk_disable call after probing
>>> (because the device is put in low power mode until used by userspace),
>>> then the clk will be disabled even without any cleanup running at all.
>>>
>>> The real problem here is simply that to work the simplefb needs certain
>>> resources, just like any other device. And while for any other device
>>> simply listing the needed resources is an accepted practice, for simplefb
>>> for some reason (which I still do not understand) people all of a sudden
>>> see listing resources as a problem.
>>
>> Because you are creating two different device tree nodes describing a
>> single piece of hardware and that's not suppose to happen in a device
>> tree.  The accurate description of the hardware is being perverted to
>> solve a software problem.
>>
>> One node describes the hardware in a format to make simplefb happy.
>> Another node describes the same hardware in a format to make the
>> device specific driver happy.
> 
> Stupid question: What about mangling an existing device node to be
> simplefb compatible, and writing simplefb to be binding agnostic?

That will not work, with simplefb a single node represents the currently
active video output. While in real hardware that may involve multiple
blocks, e.g on sunxi for hdmi out this involves the compositor (which
generates video data from 1 or more layers) which feeds into the
lcd-controller (which in this case is only used to generate hsync + vsync)
signals really, which feeds into the hdmi encoder, all 3 of which are
separate hardware blocks with their own clocks, etc.

And which hardware blocks exactly are involved will differ depending
on which video output of the device has been setup by the firmware.

The simplefb node is a virtual device, describing what the firmware
has setup, where as the hardware nodes are describing the real hardware.

I can see that people see this as duplicate info, but really it is not,
one is runtime information passed from the bootloader/firmware to the
kernel, the other is standard hardware description as people are used
to seeing in devicetree.

Regards,

Hans


^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-10-03 11:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542D73F5.9040605@wwwdotorg.org>

Hi,

On 10/02/2014 05:49 PM, Stephen Warren wrote:
> On 10/02/2014 12:42 AM, Hans de Goede wrote:
> ...
>> The whole reason why we want to use simplefb is not just to get things
>> running until HW specific driver is in place, but also to have early console
>> output (to help debugging boot problems on devices without a serial console),
>> in a world where most video drivers are build as loadable modules, so we
>> won't have video output until quite late into the boot process.
> 
> That's a very different use-case than what was originally envisaged.

I realize that, it is always amazing what sort of uses code finds once it is
out there in the wild :)

> 
> ...
>> You indicate that you don't have the time for this discussion, and I note that
>> there is no MAINTAINERS entry for drivers/video/fbdev/simplefb.c . So how about
>> the following, I pick up drivers/video/fbdev/simplefb.c maintainership, adding
>> MAINTAINERS entry for it with my name in it. Then as the maintainer it will be
>> my responsibility (and in my own benefit) to stop this from growing into
>> a monster ?
> 
> I have no issue with you being the maintainer.

Great, I'll send a patch for MAINTAINERS for this then.

> I would suggest you track down whoever it was that was involved in the original discussion and objected to simplefb performing resource management, and get their explicit ack for the addition of clocks/regulators/... to it. But, that's just a suggestion.

I appreciate the suggestion, but various people involved have been discussing this
for over a month now. All arguments in favor and against have been made many times
now, and I doubt that those people (in sofar as they are not already involved in
the discussion) will have anything new to add.

IMHO we've come at a point here where a decision needs to be made how to deal with
this, because any decision, is better then no decision (and thus no progress) at all.

I fully commit myself to actively maintain simplefb, and deal with any fallout, for
the foreseeable future.

Regards,

Hans

^ permalink raw reply

* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Tomi Valkeinen @ 2014-10-03 11:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <542E8590.70206@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On 03/10/14 14:16, Hans de Goede wrote:

>>> You indicate that you don't have the time for this discussion, and I note that
>>> there is no MAINTAINERS entry for drivers/video/fbdev/simplefb.c . So how about
>>> the following, I pick up drivers/video/fbdev/simplefb.c maintainership, adding
>>> MAINTAINERS entry for it with my name in it. Then as the maintainer it will be
>>> my responsibility (and in my own benefit) to stop this from growing into
>>> a monster ?
>>
>> I have no issue with you being the maintainer.
> 
> Great, I'll send a patch for MAINTAINERS for this then.

In that case, could you, as the maintainer, summarize the plans and/or
biggest issues for simplefb? =)

I've been planning to read this monster thread, but I just haven't found
time.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH v2 0/5] simplefb: Add clock support to simplefb
From: Hans de Goede @ 2014-10-03 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tomi,

Here is v2 of the patch-set to add clocks support to simplefb.

Changes in v2:
-Added "simplefb: Add simplefb MAINTAINERS entry" patch

If you've followed the discussion sofar, then you know that not everyone
agrees 100% with this. The primary concern being that this will turn simplefb
into not-so-simplefb.

However no-one has been able to come up with a workable alternative solution.

We've asked input from Mike Turquette, the clk maintainer, and he agrees
that the solution in this patch-set is the right way to deal with clocks for
simplefb, and he has given his Reviewed-by for the dt-bindings patch.

As for the simplefb code becoming to complex for this, I noticed that
simplefb currently does not have an active maintainer. I've discussed this
with Stephen Warren <swarren@wwwdotorg.org>, the original author of simplefb,
and with his permisson I'm picking up maintainership of simplefb.

And as the new maintainer of simplefb I agree that the approach in this
patch-set is the best way forward, and I've added my Reviewed-by to Luc's
patches adding the clocks property support. I fully commit myself to
actively maintain simplefb for the foreseeable future.

I believe that having the acks of both the clk and simplefb maintainer
is more then good enough to get this set merged despite not everyone
agreeing 100% with it (note some people have expressed concerns, but no one
has actually nacked it).

Therefor I would like to ask you to merge this set for 3.19, or if there is
still time for 3.18.

Thanks & Regards,

Hans

^ permalink raw reply

* [PATCH v2 1/5] simplefb: Add simplefb MAINTAINERS entry
From: Hans de Goede @ 2014-10-03 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1412337125-14388-1-git-send-email-hdegoede@redhat.com>

During the discussion about adding clock handling code to simplefb, it became
clear that simplefb currently does not have an active maintainer.

I've discussed this with Stephen Warren <swarren@wwwdotorg.org>, the original
author of simplefb, and with his permisson I'm picking up maintainership of
simplefb.

Cc: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3705430..01a5fd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8289,6 +8289,13 @@ F:	drivers/media/usb/siano/
 F:	drivers/media/usb/siano/
 F:	drivers/media/mmc/siano/
 
+SIMPLEFB FB DRIVER
+M:	Hans de Goede <hdegoede@redhat.com>
+L:	linux-fbdev@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/video/simple-framebuffer.txt
+F:	drivers/video/fbdev/simplefb.c
+
 SH_VEU V4L2 MEM2MEM DRIVER
 L:	linux-media@vger.kernel.org
 S:	Orphan
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 2/5] dt-bindings: Add a clocks property to the simple-framebuffer binding
From: Hans de Goede @ 2014-10-03 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1412337125-14388-1-git-send-email-hdegoede@redhat.com>

A simple-framebuffer node represents a framebuffer setup by the firmware /
bootloader. Such a framebuffer may have a number of clocks in use, add a
property to communicate this to the OS.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Mike Turquette <mturquette@linaro.org>
---
 Documentation/devicetree/bindings/video/simple-framebuffer.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
index 70c26f3..e75478e 100644
--- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
+++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
@@ -14,6 +14,9 @@ Required properties:
   - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b).
   - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
 
+Optional properties:
+- clocks : List of clocks used by the framebuffer
+
 Example:
 
 	framebuffer {
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 3/5] simplefb: formalize pseudo palette handling
From: Hans de Goede @ 2014-10-03 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1412337125-14388-1-git-send-email-hdegoede@redhat.com>

From: Luc Verhaegen <libv@skynet.be>

Signed-off-by: Luc Verhaegen <libv@skynet.be>
Acked-by: Stephen Warren <swarren@nvidia.com>
[hdegoede@redhat.com: drop unnecessary void * cast]
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/fbdev/simplefb.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 210f3a0..ec112c1 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -41,6 +41,8 @@ static struct fb_var_screeninfo simplefb_var = {
 	.vmode		= FB_VMODE_NONINTERLACED,
 };
 
+#define PSEUDO_PALETTE_SIZE 16
+
 static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 			      u_int transp, struct fb_info *info)
 {
@@ -50,7 +52,7 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 	u32 cb = blue >> (16 - info->var.blue.length);
 	u32 value;
 
-	if (regno >= 16)
+	if (regno >= PSEUDO_PALETTE_SIZE)
 		return -EINVAL;
 
 	value = (cr << info->var.red.offset) |
@@ -163,11 +165,16 @@ static int simplefb_parse_pd(struct platform_device *pdev,
 	return 0;
 }
 
+struct simplefb_par {
+	u32 palette[PSEUDO_PALETTE_SIZE];
+};
+
 static int simplefb_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct simplefb_params params;
 	struct fb_info *info;
+	struct simplefb_par *par;
 	struct resource *mem;
 
 	if (fb_get_options("simplefb", NULL))
@@ -188,11 +195,13 @@ static int simplefb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	info = framebuffer_alloc(sizeof(u32) * 16, &pdev->dev);
+	info = framebuffer_alloc(sizeof(struct simplefb_par), &pdev->dev);
 	if (!info)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, info);
 
+	par = info->par;
+
 	info->fix = simplefb_fix;
 	info->fix.smem_start = mem->start;
 	info->fix.smem_len = resource_size(mem);
@@ -225,7 +234,7 @@ static int simplefb_probe(struct platform_device *pdev)
 		framebuffer_release(info);
 		return -ENODEV;
 	}
-	info->pseudo_palette = (void *)(info + 1);
+	info->pseudo_palette = par->palette;
 
 	dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
 			     info->fix.smem_start, info->fix.smem_len,
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 4/5] simplefb: add goto error path to probe
From: Hans de Goede @ 2014-10-03 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1412337125-14388-1-git-send-email-hdegoede@redhat.com>

From: Luc Verhaegen <libv@skynet.be>

Signed-off-by: Luc Verhaegen <libv@skynet.be>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/video/fbdev/simplefb.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index ec112c1..b7d5c08 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -220,8 +220,8 @@ static int simplefb_probe(struct platform_device *pdev)
 
 	info->apertures = alloc_apertures(1);
 	if (!info->apertures) {
-		framebuffer_release(info);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto error_fb_release;
 	}
 	info->apertures->ranges[0].base = info->fix.smem_start;
 	info->apertures->ranges[0].size = info->fix.smem_len;
@@ -231,8 +231,8 @@ static int simplefb_probe(struct platform_device *pdev)
 	info->screen_base = ioremap_wc(info->fix.smem_start,
 				       info->fix.smem_len);
 	if (!info->screen_base) {
-		framebuffer_release(info);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto error_fb_release;
 	}
 	info->pseudo_palette = par->palette;
 
@@ -247,14 +247,20 @@ static int simplefb_probe(struct platform_device *pdev)
 	ret = register_framebuffer(info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret);
-		iounmap(info->screen_base);
-		framebuffer_release(info);
-		return ret;
+		goto error_unmap;
 	}
 
 	dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
 
 	return 0;
+
+ error_unmap:
+	iounmap(info->screen_base);
+
+ error_fb_release:
+	framebuffer_release(info);
+
+	return ret;
 }
 
 static int simplefb_remove(struct platform_device *pdev)
-- 
2.1.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox