Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/03/2014 01:31 AM, Julian Calaby wrote: Hi, On Fri, Oct 3, 2014 at 1:14 AM, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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 -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
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 -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Tomi, On 10/03/2014 01:45 PM, Tomi Valkeinen wrote: 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. See the cover letter of the patch-set I've just send. 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
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 signature.asc Description: OpenPGP digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. This is also the reason why we're working on adding hdmi console support to u-boot in the first place, to debug boot problems. So the whole write a HW specific driver answer just does not cut it. Just like we have vgacon / efifb on x86, we need something similar on ARM, and since ARM does not have a generic hw interface like vga, we need a firmware solution like efifb. So as said the whole write a HW specific driver just will not work, so that means using something like simplefb. Now I can take simplefb, copy it to rename it to firmwarefb or ubootfb or something, and then add the clocks support, but that is just silly. 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 ? To me that seems better then adding a new drivers/video/fbdev/firmwarefb.c which will be just a copy with the clocks added. 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 11:17:23AM -0700, Mike Turquette wrote: On Wed, Oct 1, 2014 at 12:30 AM, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 02:37:53PM -0700, Mike Turquette wrote: Quoting Thierry Reding (2014-09-29 06:54:00) On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001 From: Mike Turquette mturque...@linaro.org Date: Tue, 30 Sep 2014 14:24:38 -0700 Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused Refine the definition around clk_ignore_unused, which caused some confusion recently on the linux-fbdev and linux-arm-kernel mailing lists[0]. [0] http://lkml.kernel.org/r/20140929135358.GC30998@ulmo Signed-off-by: Mike Turquette mturque...@linaro.org --- Thierry, Please let me know if this wording makes the feature more clear. I think that's better than before, but I don't think it's accurate yet. As pointed out by Maxime unused clock may still be disabled if it's part of a tree and that tree is being disabled because there are no users left. It is entirely accurate. This feature does in fact prevent the clock framework from *automatically* gating clock According to what Maxime said if an unused clock is a sibling (has the same parent) of a clock that is used and then gets disabled, then if the parent has no other clocks that are enabled, the unused clock will still be disabled. That's still counts as automatically to me. Not automatically would mean that the clock needs to be disabled explicitly for it to become disabled. Disabling it as a side-effect of its parent getting disabled is still automatic. And it was merged by Olof so that he could use simplefb with the Chromebook! And presumably it does work for that specific Chromebook. It seems, though that for hardware with a somewhat whackier clock tree it doesn't work so well. As far as I can tell that's the reason for this patch and the ensuing discussion in the first place. Although, perhaps nobody ever really tested whether or not the above scenario was actually a problem for sunxi and maybe clk_ignore_unused would work for them. But as I understand they don't want to use it, so this whole debate about this kernel parameter is a bit moot. What I had argued is that it's unexpected behavior, because the clock is still unused (or becomes unused again), therefore shouldn't be disabled at that point either. Leaving clocks enabled because nobody claimed them is not an option. But that's exactly what clk_ignore_unused is, isn't it? I'm now totally confused. So if you want to keep the current behaviour where an unused clock can still be disabled depending on what other users do, then I think it'd be good to mention that as a potential caveat. Do you have a suggestion on the wording? Perhaps something like this: Note that if an unused clock shares a parent with clocks that are used, the unused clock may still become disabled as a side- effect of the parent clock being disabled when none of the children that are used remain enabled. ? Thierry pgpdRD2TSuRb3.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 06:17:04PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 02:48:53PM +0200, Thierry Reding wrote: On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: [...] and that the DT must not contain any hint of simplefb as shipped separately. Well, I don't think it should because it describes the same resources that the device tree node for the real device already describes. But perhaps this is one of the cases where duplication isn't all that bad? If we were worried about this wecould also do it by referring to those nodes and saying get all the resources these things need rather than duplicating the references (this might make it easier to work out when the system is ready to hand off to the real drivers). That's problematic to some degree because not all resource types may have a binding that allows them to be automatically probed, so it could be difficult to implement get all the resources this thing needs. But perhaps we can come up with good enough heuristics to make that work reliably. One downside of that is that there may be a lot of components involved in getting display to work and not all resources may be needed to keep the current state running, so we may be claiming too many. But given that we'd eventually release all of them anyway this shouldn't be too much of an issue. That's never going to work well as far as I can see but doesn't seem like an ABI stability issue, or at least not a reasonable one. It would work well under the assumption that the kernel wouldn't be touching any of the resources that simplefb depends on. If that's not a reasonable assumption then I think we can't make simplefb work the way it's currently written. I can't see how that's reasonable unless the kernel has some way of figuring out what it shouldn't be touching. Agreed. It's become clear in this discussion that we can't do this in the way x86 and other more firmware-oriented architectures do it. They get away with it because they in fact hide all of this in the firmware or don't provide a way to control the resources in such a fine-grained manner to begin with. Thierry pgpg3RGNrnKTX.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 08:43:27PM +0200, Geert Uytterhoeven wrote: On Wed, Oct 1, 2014 at 7:17 PM, Mark Brown broo...@kernel.org wrote: Well, I don't think it should because it describes the same resources that the device tree node for the real device already describes. But perhaps this is one of the cases where duplication isn't all that bad? If we were worried about this wecould also do it by referring to those nodes and saying get all the resources these things need rather than duplicating the references (this might make it easier to work out when the system is ready to hand off to the real drivers). You can have a single node for both simplefb and the later real driver. DT describes the hardware, not the software ecosystem running on the hardware. Clock, regulators, etc. don't change from a hardware point of view. If the firmware initialized a suitable graphics mode, it just has to add linux,simplefb to the compatible property (and perhaps a few other simplefb-specific properties). Unfortunately I don't think that's going to work. Especially on ARM SoCs there is no single node for a display device. The display device is typically composed of several subdevices. Thierry pgpuH2r96AUtv.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. This is also the reason why we're working on adding hdmi console support to u-boot in the first place, to debug boot problems. So the whole write a HW specific driver answer just does not cut it. Just like we have vgacon / efifb on x86, we need something similar on ARM, and since ARM does not have a generic hw interface like vga, we need a firmware solution like efifb. So as said the whole write a HW specific driver just will not work, so that means using something like simplefb. Now I can take simplefb, copy it to rename it to firmwarefb or ubootfb or something, and then add the clocks support, but that is just silly. 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 ? To me that seems better then adding a new drivers/video/fbdev/firmwarefb.c which will be just a copy with the clocks added. Regards, Hans -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Which means we must reserve necessary resources so that they don't get disabled until 2 loads. 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. 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Trying to indefinitely extend the life of the earlyprintk or earlyframeuffer is what causes problems. Doing that forces you to basically turn them into device specific drivers which do things like claiming device specific resources and gaining device specific dependency knowledge, things that shouldn't be in earlyframebuffer. Which means we must reserve necessary resources so that they don't get disabled until 2 loads. 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. Regards, Hans -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. 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. If we ever want ARM support to stop being about cute embedded non-sense hacks, we must be able to have users get some meaningful output in failure cases like this without needing to first solder a serial console to some test pads. 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 2 October 2014 14:56, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Trying to indefinitely extend the life of the earlyprintk or earlyframeuffer is what causes problems. Doing that forces you to basically turn them into device specific drivers which do things like claiming device specific resources and gaining device specific dependency knowledge, things that shouldn't be in earlyframebuffer. No. When initrd is running boot has already finished as far as kernel is concerned. And you have to extend the life of the simplefb from the time boot has finished through the time kernel mounts initrd (or other root) and hands over to userspace found on the initrd, through the time this userspace searches for the kms driver and until the time it has finally loaded if that ever succeeds. From the point of view of kernel once it has handed over to init in initrd the boot is finished. The init is normal userspace running off normal filesystem backed by a device-specific driver (initrd). That some systems do not continue to run off this filesystem indefinitely and in fact go out of their way to expunge the initrd filesystem and reclaim its resources by exercising some syscalls
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. 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. Just use earlyprintk as a guideline, if earlyprintk shows the error earlyframebuffer would also show it. If we ever want ARM support to stop being about cute embedded non-sense hacks, we must be
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 9:23 AM, Michal Suchanek hramr...@gmail.com wrote: On 2 October 2014 14:56, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Trying to indefinitely extend the life of the earlyprintk or earlyframeuffer is what causes problems. Doing that forces you to basically turn them into device specific drivers which do things like claiming device specific resources and gaining device specific dependency knowledge, things that shouldn't be in earlyframebuffer. No. When initrd is running boot has already finished as far as kernel is concerned. And you have to extend the life of the simplefb from the time boot has finished through the time kernel mounts initrd (or other root) and hands over to userspace found on the initrd, through the time this userspace searches for the kms driver and until the time it has finally loaded if that ever succeeds. Does the clock and regulator cleanup happen before drivers can load off from initrd? I didn't think it did but I might be wrong. So maybe a solution to this is to delay that cleanup until after initrd drivers have a chance to load. Of course it is not possible to delay it indefinitely (like for disk based
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 03:34 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:23 AM, Michal Suchanek hramr...@gmail.com wrote: On 2 October 2014 14:56, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Trying to indefinitely extend the life of the earlyprintk or earlyframeuffer is what causes problems. Doing that forces you to basically turn them into device specific drivers which do things like claiming device specific resources and gaining device specific dependency knowledge, things that shouldn't be in earlyframebuffer. No. When initrd is running boot has already finished as far as kernel is concerned. And you have to extend the life of the simplefb from the time boot has finished through the time kernel mounts initrd (or other root) and hands over to userspace found on the initrd, through the time this userspace searches for the kms driver and until the time it has finally loaded if that ever succeeds. Does the clock and regulator cleanup happen before drivers can load off from initrd? I didn't think it did but I might be wrong. Yes the cleanup happens before the first userspace process starts, be that the fake /sbin/init from the initrd, or the
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 9:40 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:34 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:23 AM, Michal Suchanek hramr...@gmail.com wrote: On 2 October 2014 14:56, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Trying to indefinitely extend the life of the earlyprintk or earlyframeuffer is what causes problems. Doing that forces you to basically turn them into device specific drivers which do things like claiming device specific resources and gaining device specific dependency knowledge, things that shouldn't be in earlyframebuffer. No. When initrd is running boot has already finished as far as kernel is concerned. And you have to extend the life of the simplefb from the time boot has finished through the time kernel mounts initrd (or other root) and hands over to userspace found on the initrd, through the time this userspace searches for the kms driver and until the time it has finally loaded if that ever succeeds. Does the clock and regulator cleanup happen before drivers can load off from initrd? I didn't think it did but I might be wrong. Yes the cleanup happens before the first
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 3:34 PM, jonsm...@gmail.com jonsm...@gmail.com wrote:
Does the clock and regulator cleanup happen before drivers can load
off from initrd? I didn't think it did but I might be wrong.
Yes
drivers/base/power/domain.c:late_initcall(genpd_poweroff_unused);
drivers/clk/clk.c:late_initcall_sync(clk_disable_unused);
drivers/regulator/core.c:late_initcall_sync(regulator_init_complete);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 03:40 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:40 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 2 October 2014 15:40, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
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 signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:40 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
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 -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:40 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:33 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 03:27 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 9:14 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:56 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 8:39 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 02:22 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 2:42 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 08:12 PM, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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 -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 11:14 AM, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. -- Jon Smirl jonsm...@gmail.com -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 2 October 2014 17:30, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 11:14 AM, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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 -- You received this message because you are subscribed to the Google Groups
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/02/2014 05:30 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 11:14 AM, jonsm...@gmail.com jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote: On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
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. -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Thu, Oct 2, 2014 at 2:22 PM, jonsm...@gmail.com jonsm...@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-shm=141227657322649w=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 -- ge...@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
--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi,
On Fri, Oct 3, 2014 at 1:14 AM, jonsm...@gmail.com jonsm...@gmail.com wrote:
On Thu, Oct 2, 2014 at 10:50 AM, Hans de Goede hdego...@redhat.com wrote:
Hi,
On 10/02/2014 04:41 PM, jonsm...@gmail.com wrote:
On Thu, Oct 2, 2014 at 10:21 AM, Hans de Goede hdego...@redhat.com wrote:
Hi,
On 10/02/2014 04:16 PM, jonsm...@gmail.com wrote:
On Thu, Oct 2, 2014 at 10:08 AM, Hans de Goede hdego...@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@ff93 {
compatible = rockchip,rk3288-vop;
reg = 0xff93 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@ff93 {
compatible = rockchip,rk3288-vop, simple-framebuffer;
framebuffer {
reg = 0x1d385000 (1600 * 1200 * 2);
width = 1600;
height = 1200;
stride = (1600 * 2);
format = r5g6b5;
};
reg = 0xff93 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;
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 02:37:53PM -0700, Mike Turquette wrote: Quoting Thierry Reding (2014-09-29 06:54:00) On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001 From: Mike Turquette mturque...@linaro.org Date: Tue, 30 Sep 2014 14:24:38 -0700 Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused Refine the definition around clk_ignore_unused, which caused some confusion recently on the linux-fbdev and linux-arm-kernel mailing lists[0]. [0] http://lkml.kernel.org/r/20140929135358.GC30998@ulmo Signed-off-by: Mike Turquette mturque...@linaro.org --- Thierry, Please let me know if this wording makes the feature more clear. I think that's better than before, but I don't think it's accurate yet. As pointed out by Maxime unused clock may still be disabled if it's part of a tree and that tree is being disabled because there are no users left. What I had argued is that it's unexpected behaviour, because the clock is still unused (or becomes unused again), therefore shouldn't be disabled at that point either. So if you want to keep the current behaviour where an unused clock can still be disabled depending on what other users do, then I think it'd be good to mention that as a potential caveat. Thierry pgpC8LuOkv3Rr.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote: Not really thought this through fully yet but would having phandles to the relevant devices do the job? Kind of lines up with Grant's idea of having dummy drivers. One of the arguments that came up during the discussion of the sunxi patches is that simplefb is going to be used precisely because there is no real driver for the display part of the SoC yet and nobody knows what the binding will look like. So there's nothing to point at currently and for the same reason having a dummy driver won't work. There's simply no definition of what resources are needed. You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. We might want to do that in the future, though it's not always the case that reset is the lowest power state. That proves my point. If we ever decide to assert resets by default we'll have yet another subsystem that can potentially break existing DTs. OTOH given the level of breakage that's like to introduce we might just decide not to do that... It might be the sensible thing to do in most cases. I think there's a legitimate reason not to trust firmware. However in case of simplefb we already do, so I think having a sort of flag to signal that we do trust firmware would allow us to cope with these situation much better. In the end it brings us back to the very fundamental principles of DT that's been causing so much pain. For things to work properly and in a stable way you have to get the bindings right and complete from the start. That is, it needs to describe every aspect of the hardware block and all links to other components. Or we ned to introduce things in a conservative fashion which does cope with backwards compatibility; it's definitely more work but it is doable. Is it? I thought the only way to keep backwards compatibility was by making any new properties optional. But if those properties add vital information for the device to work you have to come up with a sensible default to keep existing setups working that lack the new properties. Doing that is not going to scale very well. It has a chance of working for hardware-specific drivers because we may be able to derive the default from the SoC generation or even the machine compatible. But I don't see how it could work for something that's supposed to be generic like simplefb. I'm hoping that there's a better way that I don't know about, because it would certainly make a lot of things much easier. One thing that makes me a bit nervous about this approach in the context of the regulator API is the frequency with which one finds shared supplies. I'm not sure if it's actually a big problem in practice but it makes me worry a bit. We could probably just do something like make refcounting down to zero not actually disable anything for standard regulators to deal with it which might be an idea anyway in the context of this sort of dodge. Yes, that's sort of how I expected clk_ignore_unused to work. The way I understood it, it would cause all unused clocks to be ignored (that is stay enabled if they are). Of course as Geert pointed out in another subthread, taking this all the way means that we have to disable all power management because the firmware device may be sharing resources with other devices and which therefore are not unused. That's a pretty strong argument and I don't have a solution for that. It is only really a problem for cases where the firmware virtual device isn't taken over by a proper driver at some point, though. Indeed, and we also run into trouble for things where we actually need to really turn off the
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote: [...] Of course as Geert pointed out in another subthread, taking this all the way means that we have to disable all power management because the firmware device may be sharing resources with other devices and which therefore are not unused. That's a pretty strong argument and I don't have a solution for that. It is only really a problem for cases where the firmware virtual device isn't taken over by a proper driver at some point, though. Indeed, and we also run into trouble for things where we actually need to really turn off the resource for some reason (MMC has some needs here for example). Perhaps an alternative would be to just keep power management going and hope for the best. This may turn out not to be as much of a problem for many SoCs or boards as people make it out to be. Thierry pgpGUTF5zy5fp.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 9:41 AM, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 06:39:28PM +0100, Mark Brown wrote: On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote: So long as we're ensuring that when we don't start supporting resources without DT additions or at least require DT additions to actively manage them (which can then include simplefb hookup) we should be fine. I'm not sure I understand what you mean. If we add a driver for the PMIC that exposes these regulators and then add a DT node for the PMIC, we'd still need to fix the firmware to generate the appropriate DT properties to allow simplefb to enable the regulators. No, you don't. It's only if you start describing the regulators in the PMIC in DT that you run into problems. Unconfigured regulators won't be touched. Okay, that's what I meant. It seems rather odd to add a PMIC DT node but omit the description of the regulators that it exposes. Unless the regulators are truly unused, as in not connected to any peripherals. Agreed, I added similar PMIC support to other Chromebooks (Peach Pit and Pi) DTS and for me it made totally sense to add nodes for all the regulators that are connected to peripherals according to the board schematic. Specially since the framework is smart enough to disable any regulator that is not used. After all, a DT is meant to describe the hardware, so how can possibly be an issue to add more details about the hw in a DTS? If something is working relying on parts of the hw on not being described, then is essentially relying on side-effects and implementation details which are bound to be broken anyways. So unless firmware is updated at the same time, regulators will get disabled because they are unused. That won't happen unless the regulators are explicitly described, if they are described as unused then this will of course happen. With described as unused you mean they have a node in DT, so constraints are applied and all that, but no driver actually uses them? Adding your resources (clock, regulators, etc) incrementally and only when the driver for the device that use these resources is available, will only make adding support for a new platform slower IMHO since there will be more patches to be posted, reviewed and merged. The fundamental issue is that if we start describing simplefb nodes with an incomplete set of resources then we're bound to run into problems where it'll break once these new resources are described in the DTS. If the simplefb node was described in the DTS then this would be less of a problem because the resources could be added to the simplefb node at the same time. Agreed, the assumptions made by simplefb are quite fragile so we should either document somewhere that simplefb ignores all the resources and that is a best effort so users should not consider the display breaking a regression or make it robust enough so users can expect that it will always work. Just adding the clocks is a partial solution which I think will make the situation even worst since it will give a false illusion of robustness but as you said it will break anyways due other resources. However given that simplefb is supposed to be generated by firmware this is no longer possible. It will inevitably break unless you upgrade the DTB and the firmware at the same time. And it was already decided long ago that upgrading the firmware should never be a requirement for keeping things working. AFAICT in practice most platforms' firmware do not generate the simplefb by default. In the case of Chromebooks for example, a custom U-boot needs to be flashed in order to have simplefb support. In fact most people working on mainline support for Chromebooks do not use simplefb and that is why the issue was not spot when adding the support for clocks and regulators. Personally I didn't even know how simplefb worked before Will reported that his display used to work on Snow before 3.16. So I assume that his reasonable to expect that users using simplefb are able to update their bootloader. Which brings a more general question about DT and backward compatibility. Should we have backward compatibility only with the official firmware that is ship on a device when is bought or should we maintain backward compatibility against any firmware out there that someone re-built and added logic to mangle the FDT before is passed to the kernel? Going back to simplefb, I think the fact that the simplefb is not in the DTS is the fundamental issue here. For me, the most reasonable approach to solve this is the one suggested by Doug Anderson. That is to have the simplefb node in the DTS so all the references to the resources it uses can be added in the DTS but keep the simplefb node as status = disabled. The bootloader can find the simplefb node and fill all the information about the framebuffer
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader and that the DT must not contain any hint of simplefb as shipped separately. That's never going to work well as far as I can see but doesn't seem like an ABI stability issue, or at least not a reasonable one. Either the bootloader needs to be updated along with the DT or the DT needs to offer the bootloader a stable interface of its own for adding the description of what it has set up (like a default disabled node with the FB description, I'm sure other ideas are possible). Obviously the goal with the stable ABI is that the DT will be distributed along with the platform. Of course as Geert pointed out in another subthread, taking this all the way means that we have to disable all power management because the firmware device may be sharing resources with other devices and which therefore are not unused. That's a pretty strong argument and I don't have a solution for that. It is only really a problem for cases where the firmware virtual device isn't taken over by a proper driver at some point, though. Indeed, and we also run into trouble for things where we actually need to really turn off the resource for some reason (MMC has some needs here for example). So if disabling power management wholesale isn't going to be an option, what's the alternative? I originally proposed that the clock drivers could be modified to not disable clocks that are known to be problematic with simplefb. People objected to that because they thought it would be impractical to determine which clocks are involved with display across various boards. Handling this in the clock driver has worked remarkably well for us on Tegra, but perhaps that's just because Tegra is an unusually sane design to begin with. It's probably more that you've just not run into the corner cases yet - if the display is mostly driven from the standard controller on the SoC you've got a pretty good idea what's going to be happening. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 01:10:46PM +0200, Javier Martinez Canillas wrote: On Wed, Oct 1, 2014 at 9:41 AM, Thierry Reding thierry.red...@gmail.com wrote: Okay, that's what I meant. It seems rather odd to add a PMIC DT node but omit the description of the regulators that it exposes. Unless the regulators are truly unused, as in not connected to any peripherals. Agreed, I added similar PMIC support to other Chromebooks (Peach Pit and Pi) DTS and for me it made totally sense to add nodes for all the regulators that are connected to peripherals according to the board schematic. Specially since the framework is smart enough to disable any regulator that is not used. After all, a DT is meant to describe the hardware, so how can possibly be an issue to add more details about the hw in a DTS? If something is working relying on parts of the hw on not being described, then is essentially relying on side-effects and implementation details which are bound to be broken anyways. It's not a problem to describe the hardware, it's a problem to describe the hardware inaccurately. If you add something and explicitly tell the kernel that nothing needs it then it shouldn't be a surprise that it gets turned off. With described as unused you mean they have a node in DT, so constraints are applied and all that, but no driver actually uses them? Adding your resources (clock, regulators, etc) incrementally and only when the driver for the device that use these resources is available, will only make adding support for a new platform slower IMHO since there will be more patches to be posted, reviewed and merged. So don't do that if you're worried about it then, provide the bits of DT that hook everything up from the start or otherwise describe things as being in use. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 01:10:46PM +0200, Javier Martinez Canillas wrote: [...] Adding your resources (clock, regulators, etc) incrementally and only when the driver for the device that use these resources is available, will only make adding support for a new platform slower IMHO since there will be more patches to be posted, reviewed and merged. So don't do that if you're worried about it then, provide the bits of DT that hook everything up from the start or otherwise describe things as being in use. Otherwise describe things as being in use doesn't work for clocks for example. And Mike already said he wasn't willing to add something like an always-on DT property for clocks. Thierry pgpB5ykJ9w8oI.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader No, what I'm saying is that what the simplefb driver expects and what the bootloader sets up may diverge as resource drivers are added to the kernel. The DT /could/ be updated without the bootloader. You may only be able to replace the DTB but not the bootloader on a given platform. and that the DT must not contain any hint of simplefb as shipped separately. Well, I don't think it should because it describes the same resources that the device tree node for the real device already describes. But perhaps this is one of the cases where duplication isn't all that bad? That's never going to work well as far as I can see but doesn't seem like an ABI stability issue, or at least not a reasonable one. It would work well under the assumption that the kernel wouldn't be touching any of the resources that simplefb depends on. If that's not a reasonable assumption then I think we can't make simplefb work the way it's currently written. Either the bootloader needs to be updated along with the DT I thought we had decided that this was one of the big no-nos. But perhaps I'm misremembering. or the DT needs to offer the bootloader a stable interface of its own for adding the description of what it has set up (like a default disabled node with the FB description, I'm sure other ideas are possible). Obviously the goal with the stable ABI is that the DT will be distributed along with the platform. So instead of pretending that this is in any way generic, maybe a better idea would be to provide code in DRM/KMS drivers that is called early, grabs all the resources as defined in the binding for the device and then instantiates simplefb using the parsed information. Which is kind of the stub driver that Grant had suggested. Of course that means duplicating most of the resource handling from the real driver into this stub driver. And it means that this part of the driver would have to be built into the kernel and bloat it some more. Thierry pgpvcvXAotbWP.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding thierry.red...@gmail.com wrote: On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader No, what I'm saying is that what the simplefb driver expects and what the bootloader sets up may diverge as resource drivers are added to the kernel. The DT /could/ be updated without the bootloader. You may only be able to replace the DTB but not the bootloader on a given platform. simplefb should be a boot console and not survive past the boot process. Trying to get a 'generic' console driver to survive the boot process is not a generic problem. There are about 1,000 messages in these threads explaining why this is not a generic problem. All of these clock and regulator issues would go away by building a device specific framebuffer driver. A device specific framebuffer driver can be written in a day or two, it is far simpler than a KMS driver. This driver would how to parse the device specific device tree node and do the right thing with the regulators/clocks. So simplefb is built-in and used for early boot. During the boot process a device specific framebuffer driver loads. This device specific driver takes over for simplefb and can become the user space console. If the device specific framebuffer does not get loaded, then simplefb is going to stop working when the clocks and regulators get shut off. But that is what should happen. and that the DT must not contain any hint of simplefb as shipped separately. Well, I don't think it should because it describes the same resources that the device tree node for the real device already describes. But perhaps this is one of the cases where duplication isn't all that bad? That's never going to work well as far as I can see but doesn't seem like an ABI stability issue, or at least not a reasonable one. It would work well under the assumption that the kernel wouldn't be touching any of the resources that simplefb depends on. If that's not a reasonable assumption then I think we can't make simplefb work the way it's currently written. Either the bootloader needs to be updated along with the DT I thought we had decided that this was one of the big no-nos. But perhaps I'm misremembering. or the DT needs to offer the bootloader a stable interface of its own for adding the description of what it has set up (like a default disabled node with the FB description, I'm sure other ideas are possible). Obviously the goal with the stable ABI is that the DT will be distributed along with the platform. So instead of pretending that this is in any way generic, maybe a better idea would be to provide code in DRM/KMS drivers that is called early, grabs all the resources as defined in the binding for the device and then instantiates simplefb using the parsed information. Which is kind of the stub driver that Grant had suggested. Of course that means duplicating most of the resource handling from the real driver into this stub driver. And it means that this part of the driver would have to be built into the kernel and bloat it some more. Thierry -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 1 October 2014 15:01, jonsm...@gmail.com jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding thierry.red...@gmail.com wrote: On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader No, what I'm saying is that what the simplefb driver expects and what the bootloader sets up may diverge as resource drivers are added to the kernel. The DT /could/ be updated without the bootloader. You may only be able to replace the DTB but not the bootloader on a given platform. simplefb should be a boot console and not survive past the boot process. Trying to get a 'generic' console driver to survive the boot process is not a generic problem. There are about 1,000 messages in these threads explaining why this is not a generic problem. All of these clock and regulator issues would go away by building a device specific framebuffer driver. A device specific framebuffer driver can be written in a day or two, it is far simpler than a KMS driver. This driver would how to parse the device specific device tree node and do the right thing with the regulators/clocks. How it would know? You need different clocks for LCD and different clocks for HDMI. Unless it is a real driver that can drive either it can tell which is enabled or u-boot has to tell it or you have to write a fixed entry for the configuration you want in the DT and configure u-boot accordingly by hand as well. So simplefb is built-in and used for early boot. During the boot process a device specific framebuffer driver loads. This device specific driver takes over for simplefb and can become the user space console. If the device specific framebuffer does not get loaded, then simplefb is going to stop working when the clocks and regulators get shut off. But that is what should happen. Why it should be so? It is reasonable to want working console on device which has u-boot or other firmware graphics support but the support in kernel is still under development. Also the 'boot end' for kernel when it frees the clocks is way earlier than the 'boot end' for the distribution which ends when you reach certain init goal like multiuser environment. There is a lot between and once the kernel hands over to init it can never tell what's going on. Since a modular KMS driver would load way later than the moment when 'boot end' is reached for kernel the simple function as early console would break. Also if you prevented resource management from happening during this 'booting' stage you could not safely load drivers during that time which kind of defeats the purpose of this stage. Because either the kernel can do resource management and give resources to drivers that are loaded or it cannot do either. Thanks Michal -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 9:17 AM, Michal Suchanek hramr...@gmail.com wrote: On 1 October 2014 15:01, jonsm...@gmail.com jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 8:48 AM, Thierry Reding thierry.red...@gmail.com wrote: On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: On Wed, Oct 01, 2014 at 10:14:44AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 07:00:36PM +0100, Mark Brown wrote: You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. I agree from a technical point of view. However given the dynamically generated nature of the node the problem is more of a logistical nature. As we've seen U-Boot is being enabled to add clocks to the simplefb node but I'm fairly certain that there's a regulator somewhere that needs to be enabled too, be it for powering the display controller, the panel or the backlight. I wouldn't even be surprised if there were one for each of those. If so simplefb on this board will break when the regulators are described in the kernel's DTS. If we don't consider this a problem then the whole DT ABI stability business is a farce. I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader No, what I'm saying is that what the simplefb driver expects and what the bootloader sets up may diverge as resource drivers are added to the kernel. The DT /could/ be updated without the bootloader. You may only be able to replace the DTB but not the bootloader on a given platform. simplefb should be a boot console and not survive past the boot process. Trying to get a 'generic' console driver to survive the boot process is not a generic problem. There are about 1,000 messages in these threads explaining why this is not a generic problem. All of these clock and regulator issues would go away by building a device specific framebuffer driver. A device specific framebuffer driver can be written in a day or two, it is far simpler than a KMS driver. This driver would how to parse the device specific device tree node and do the right thing with the regulators/clocks. How it would know? You need different clocks for LCD and different clocks for HDMI. Unless it is a real driver that can drive either it can tell which is enabled or u-boot has to tell it or you have to write a fixed entry for the configuration you want in the DT and configure u-boot accordingly by hand as well. Start building all of that very device specific support inside the device specific framebuffer driver. The device specific framebuffer driver will be on initrd and it will get loaded as soon as possible by the kernel. Inside the device node for the video device there should be a sub-node or phandle indicating the presence of the LCD or HDMI jack. That is a valid hardware description and it should always be there. You can also add a 'chosen' node to indicate how these have been programmed. Two solutions -- 1) Build in all of the device specific KMS/framebuffer drivers into the kernel. Now there is no need for simplefb. But that wastes a lot of memory with code that will never get executed. 2) Early boot off from simplefb. Have all of the graphics drivers on initrd. Load the right one. Device specific graphic driver now owns hardware. When KMS is missing, write a much smaller framebuffer driver. You can start by copying simplefb and then add in the device specific bits. The option of fully booting on simplefb up to user space console is not a good one. It requires that simplefb be taught about all of the crazy and very complex clock and regulator environments for all of the random graphics systems. And we're just getting started on enumerating all of those crazy configurations. You haven't wandered into the area of the video hardware living on a different bus and having a bus controller in the way yet. Now you have to figure out how to keep that bus from being turned off (there are SGI systems like this). So simplefb is built-in and used for early boot. During the boot process a device specific framebuffer driver loads. This device specific driver takes over for simplefb and can become the user space console. If the device specific framebuffer does not get loaded, then simplefb is going to stop working when the clocks and regulators get shut off. But that is what should happen. Why it should be so? It is reasonable to want working console on device which has u-boot or other firmware graphics support but the support in kernel is still
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 02:48:02PM +0200, Thierry Reding wrote: On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote: So don't do that if you're worried about it then, provide the bits of DT that hook everything up from the start or otherwise describe things as being in use. Otherwise describe things as being in use doesn't work for clocks for example. And Mike already said he wasn't willing to add something like an always-on DT property for clocks. That's not the only way of doing things - another way would be to have a stub driver that just holds the resources while working on getting a full one in place for example. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 02:48:53PM +0200, Thierry Reding wrote: On Wed, Oct 01, 2014 at 01:20:08PM +0100, Mark Brown wrote: I think you're setting constraints on the implementation you want to see which make it unworkable but I don't think those constraints are needed. You're starting from the position that the DT needs to be updated without the bootloader No, what I'm saying is that what the simplefb driver expects and what the bootloader sets up may diverge as resource drivers are added to the kernel. The DT /could/ be updated without the bootloader. You may only be able to replace the DTB but not the bootloader on a given platform. Sure, but doing that and also having the bootloader write part of the DT from scratch with no cooperation from the rest of the DT doesn't seem like the way to robustness. and that the DT must not contain any hint of simplefb as shipped separately. Well, I don't think it should because it describes the same resources that the device tree node for the real device already describes. But perhaps this is one of the cases where duplication isn't all that bad? If we were worried about this wecould also do it by referring to those nodes and saying get all the resources these things need rather than duplicating the references (this might make it easier to work out when the system is ready to hand off to the real drivers). That's never going to work well as far as I can see but doesn't seem like an ABI stability issue, or at least not a reasonable one. It would work well under the assumption that the kernel wouldn't be touching any of the resources that simplefb depends on. If that's not a reasonable assumption then I think we can't make simplefb work the way it's currently written. I can't see how that's reasonable unless the kernel has some way of figuring out what it shouldn't be touching. Either the bootloader needs to be updated along with the DT I thought we had decided that this was one of the big no-nos. But perhaps I'm misremembering. It makes things more fragile so it's not desirable, no. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 10/01/2014 07:05 PM, Mark Brown wrote: On Wed, Oct 01, 2014 at 02:48:02PM +0200, Thierry Reding wrote: On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote: So don't do that if you're worried about it then, provide the bits of DT that hook everything up from the start or otherwise describe things as being in use. Otherwise describe things as being in use doesn't work for clocks for example. And Mike already said he wasn't willing to add something like an always-on DT property for clocks. That's not the only way of doing things - another way would be to have a stub driver that just holds the resources while working on getting a full one in place for example. That won't work because the real driver which will eventually replace the stub one will likely be a module, and then we will loose video output between the kernel finalizing the initial boot, and the module actually loading. We've been over all this again and again and again. RGH! 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. This madness has to end! Thierry can we please have a clear and unambiguous NACK from you on having the clocks property in the simplefb dt node, and if you do so I expect a proof of concept patch from you with an alternative solution within a week, or can you please stop blocking this from getting merged? And again, if you believe this will cause some sort of dt compat issues or whatever, no one is making you use this property for your boards! 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: Hi, On 10/01/2014 07:05 PM, Mark Brown wrote: On Wed, Oct 01, 2014 at 02:48:02PM +0200, Thierry Reding wrote: On Wed, Oct 01, 2014 at 01:32:50PM +0100, Mark Brown wrote: So don't do that if you're worried about it then, provide the bits of DT that hook everything up from the start or otherwise describe things as being in use. Otherwise describe things as being in use doesn't work for clocks for example. And Mike already said he wasn't willing to add something like an always-on DT property for clocks. That's not the only way of doing things - another way would be to have a stub driver that just holds the resources while working on getting a full one in place for example. That won't work because the real driver which will eventually replace the stub one will likely be a module, and then we will loose video output between the kernel finalizing the initial boot, and the module actually loading. Is this correct? Do the clocks really get shut off before a driver on initrd can load? I agree this is true if you wait until user space is fully up and stick this driver out on a disk drive. We've been over all this again and again and again. RGH! 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. This madness has to end! Thierry can we please have a clear and unambiguous NACK from you on having the clocks property in the simplefb dt node, and if you do so I expect a proof of concept patch from you with an alternative solution within a week, or can you please stop blocking this from getting merged? And again, if you believe this will cause some sort of dt compat issues or whatever, no one is making you use this property for your boards! 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 01, 2014 at 12:12:20PM -0600, Stephen Warren wrote: On 10/01/2014 11:54 AM, jonsm...@gmail.com wrote: On Wed, Oct 1, 2014 at 1:26 PM, Hans de Goede hdego...@redhat.com wrote: ... We've been over all this again and again and again. RGH! 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. Define resource management. Simplefb should never alter resources. It should never alter anything that $bootloader set up. It should however claim resources to prevent them from being altered. Perhaps the word managing should be split up in claiming and altering here. Luc Verhaegen. -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 12:30 AM, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 02:37:53PM -0700, Mike Turquette wrote: Quoting Thierry Reding (2014-09-29 06:54:00) On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001 From: Mike Turquette mturque...@linaro.org Date: Tue, 30 Sep 2014 14:24:38 -0700 Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused Refine the definition around clk_ignore_unused, which caused some confusion recently on the linux-fbdev and linux-arm-kernel mailing lists[0]. [0] http://lkml.kernel.org/r/20140929135358.GC30998@ulmo Signed-off-by: Mike Turquette mturque...@linaro.org --- Thierry, Please let me know if this wording makes the feature more clear. I think that's better than before, but I don't think it's accurate yet. As pointed out by Maxime unused clock may still be disabled if it's part of a tree and that tree is being disabled because there are no users left. It is entirely accurate. This feature does in fact prevent the clock framework from *automatically* gating clock And it was merged by Olof so that he could use simplefb with the Chromebook! What I had argued is that it's unexpected behavior, because the clock is still unused (or becomes unused again), therefore shouldn't be disabled at that point either. Leaving clocks enabled because nobody claimed them is not an option. So if you want to keep the current behaviour where an unused clock can still be disabled depending on what other users do, then I think it'd be good to mention that as a potential caveat. Do you have a suggestion on the wording? Thanks, Mike Thierry -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Wed, Oct 1, 2014 at 7:17 PM, Mark Brown broo...@kernel.org wrote:
Well, I don't think it should because it describes the same resources
that the device tree node for the real device already describes. But
perhaps this is one of the cases where duplication isn't all that bad?
If we were worried about this wecould also do it by referring to
those nodes and saying get all the resources these things need rather
than duplicating the references (this might make it easier to work out
when the system is ready to hand off to the real drivers).
You can have a single node for both simplefb and the later real driver.
DT describes the hardware, not the software ecosystem running on the
hardware. Clock, regulators, etc. don't change from a hardware point of
view.
If the firmware initialized a suitable graphics mode, it just has to add
linux,simplefb to the compatible property (and perhaps a few other
simplefb-specific properties).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. Not really thought this through fully yet but would having phandles to the relevant devices do the job? Kind of lines up with Grant's idea of having dummy drivers. One of the arguments that came up during the discussion of the sunxi patches is that simplefb is going to be used precisely because there is no real driver for the display part of the SoC yet and nobody knows what the binding will look like. So there's nothing to point at currently and for the same reason having a dummy driver won't work. There's simply no definition of what resources are needed. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. We might want to do that in the future, though it's not always the case that reset is the lowest power state. That proves my point. If we ever decide to assert resets by default we'll have yet another subsystem that can potentially break existing DTs. In the end it brings us back to the very fundamental principles of DT that's been causing so much pain. For things to work properly and in a stable way you have to get the bindings right and complete from the start. That is, it needs to describe every aspect of the hardware block and all links to other components. But there is no way you can do that for a virtual device like simplefb because it is a generic abstraction for hardware that varies wildly. The only way to make it work is to abstract away all the resource management and consider it to be done elsewhere. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. One thing that makes me a bit nervous about this approach in the context of the regulator API is the frequency with which one finds shared supplies. I'm not sure if it's actually a big problem in practice but it makes me worry a bit. We could probably just do something like make refcounting down to zero not actually disable anything for standard regulators to deal with it which might be an idea anyway in the context of this sort of dodge. Yes, that's sort of how I expected clk_ignore_unused to work. The way I understood it, it would cause all unused clocks to be ignored (that is stay enabled if they are). Of course as Geert pointed out in another subthread, taking this all the way means that we have to disable all power management because the firmware device may be sharing resources with other devices and which therefore are
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 06:59:59AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:54:00PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. Well, it never says that it also prevent them from being disabled later on. But it's true it's not very explicit. It says: Keep all clocks already enabled by bootloader on, even if no driver has claimed them. ... There's no until or anything there, so I interpret that as indefinitely. Well, then, sorry, but that's not how it works. Either way, if there are other users of a clock then they will just as likely want to modify the rate at which point simplefb will break just as badly. And this can be handled just as well. Register a clock notifier, refuse any rate change, done. But of course, that would require having a clock handle. Now, how would *you* prevent such a change? Like I said in the other thread. If you have two drivers that use the same clock but need different frequencies you've lost anyway. Except that the driver that has the most logic (ie not simplefb) will have a way to recover and deal with that. You're making an assumption here. Why would the driver have such logic if nothing ever prevented it from setting the rate properly before. I'm not saying it has, but it something that can be done. You usually have several strategies, which depending on the device, might or might not be possible, such as reparenting, trying to use an additional divider. Worst case scenario, you're indeed doomed. But you do have a best case scenario, which isn't the case with your approach. And you didn't screw the framebuffer silently. But sure, you can still try to point new issues, get an obvious and robust solution, and then discard the issue when the solution doesn't go your way... And you've already proven that you're completely unwilling to even consider any other solution than what was originally proposed, so I really don't see how discussing this further with you is going to be productive. You haven't express *what* you wanted to achieve for quite some time, but only *how*. And your how has some deficiencies that have already been pointed out numerous times. However, I do come to the same conclusion. I really don't see how we can be productive. Just like I really don't see how we will ever be able to get any DRM/KMS driver in when the time comes. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. Perhaps read the sentence again. I said that we would *otherwise* be able to do much easier with a text editor.. My point remains that there shouldn't be a need to generate DTB content of this complexity at all. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? That's exactly the point. And it's going to break again and again as simplefb is extended with new things. Generally DT bindings should be backwards compatible. When extended they should provide a way to fall back to a reasonable default. There's simply no way you can do that with simplefb. This one *is* backward compatible. It doesn't even change the simplefb behaviour, compared to what it was doing before, if you don't have this clocks property. What can be a more reasonable default that the way it used to behave? My point is that if we decide not to consider all resources handled by firmware then we need to go all the way. At that point you start having problems as evidenced by the Snow example. Agreed. What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. And the only way I can think of to deal with that properly is to have resources management in the driver. You know that you are going to call that for regulator, reset, power domains, just as you would have needed to with the proper API, unless that with this kind of solution, you would have to modify *every* framework that might interact with any resource involved in getting simplefb running? We have to add handling for every kind of resource either way. Also if this evolves into a common pattern we can easily wrap it up in a single function call. Unless that in one case, we already have everything needed to handle everything properly, and in another, you keep hacking more and more into the involved frameworks. This is a fundamental issue that we are facing and I'm trying to come up with a solution that is future-proof and will work for drivers other than simplefb. Just because we currently lack this
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 07:39:02AM +0200, Thierry Reding wrote: You keep bringing up the Raspberry Pi for some reason and suggest that it is somehow inferior to sunxi. What makes you think it's less entitled to be supported on Linux than sunxi? I don't care about the Raspberry Pi and I equally don't care about sunxi. I don't own a Raspberry Pi and I don't own any Allwinner hardware. What I do care about is Linux and I want it to work well for all SoCs equally. Perhaps if you could put aside your crusade against the Raspberry Pi for just a second you'll realize that we're all on the same team. This isn't a competition and I'm not trying to put a spoke in your wheel. On the contrary, I'm actually trying to help you. We've been over this already, and I'll tell you again that you're getting this wrong. No platform is more entitled to get merged than another one. I do care about the Allwinner SoCs, and I care just as much about the broader Linux support for all the other SoCs, be it from nvidia, samsung or whatever vendor you can come up with. But you can't hide the fact that the bcm2835 still has a very limited clock support, and I really don't know about its clock tree, but I guess that if the times come when they add a more complete clock support, they will face the same issue. If the driver would have been developped initially to create a framebuffer on the Allwinner SoCs, at a time when we didn't have any clock support too, calling it only usable on sunxi wouldn't have shocked me tbh. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: [...] What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. We've already determined that simplefb can't do anything meaningful with the resources other than keep them in the status quo. It simply doesn't have enough knowledge to do so. It doesn't know the exact pixel clock or what voltage the attached panel needs. And the only way I can think of to deal with that properly is to have resources management in the driver. My point is that if we had a proper way to tell the kernel not to do anything with resources owned by firmware, then the driver wouldn't have to do anything with the resources. I really start to consider adding a sunxi-uboot-fb, that would just duplicate the source code of simplefb but also taking the clocks. Somehow, I feel like it will be easier (and definitely less of a hack than using the generic common clock API). You're not getting it are you? What makes you think sunxi-uboot-fb is going to be any different? This isn't about a name. At least, we would get to do any binding and resource management we want. And that's a big win. So instead of trying to understand the concerns that I've expressed and constructively contribute to finding a solution that works for everybody you'd rather go and write a driver from scratch. Way to go. I've already said that I'm not saying strictly no to these patches, but what I want to see happen is some constructive discussion about whether we can find better ways to do it. If we can't then I'm all for merging these patches. Fortunately other (sub)threads have been somewhat more constructive and actually come up with alternatives that should make everyone happier. If you're going to do SoC-specific bindings and resource management you are in fact implementing what Grant suggested in a subthread. You're implementing a dummy driver only for resource management, which isn't really a bad thing. It can serve as a placeholder for now until you add the real driver. And you can also use the simplefb driver to provide the framebuffer. Thierry pgpwc7ByQwhIp.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 30 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: [...] What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. We've already determined that simplefb can't do anything meaningful with the resources other than keep them in the status quo. It simply doesn't have enough knowledge to do so. It doesn't know the exact pixel clock or what voltage the attached panel needs. And the only way I can think of to deal with that properly is to have resources management in the driver. My point is that if we had a proper way to tell the kernel not to do anything with resources owned by firmware, then the driver wouldn't have to do anything with the resources. The firmware on sunxi does not own any resources whatsoever. It ceases running once it executes the kernel. This is different from ACPI and UEFI where you have pieces of the firmware lingering indefinitely and potentially getting invoked by user pressing some button or some other hardware event. It is also different from rpi where the Linux kernel effectively runs in a virtual environment created by the firmware hypervisor. So on sunxi and many other ARM machines the Linux kernel is the sole owner of any resources that might happen to be available on the machine. There is no firmware owning them when the Linux kernel is running, ever. And we do have a proper way to tell to the kernel what these resources are used for - inserting description of them into the simplefb DT node. Sure the simplefb cannot manage the resources in any way and but it does own them. When it is running they are in use, when it stops they are free to be reclaimed by the platform driver. But you refuse to marge the kernel change for this proper management to happen. The alternate proposal to stop the kernel from managing resources while simplefb is running and refernce count drivers that cannot manage resources is indeed a workable, general alternative. It does however switch the kernel into special mode when resources are not managed and will needlessly hinder eg. development and testing of powermanagement and hotplug for drivers unrelated to kms in parallel to kms. But let's look at this special mode closer. Under normal boot sequence when the built-in drivers are initialized or around that time the kernel does a pass in which it disables unused clocks and possibly reclaims other resources. The boot has finished for the kernel and now it hands over to userspace and it is up to the userspace to load any more drivers (such as kms) or not. If at that point a driver like simplefb exists it should have called that stop_managing_resources() which should replace clk_ignore_unused() so that other resources are properly handled without the driver ever having to know about them. For clocks this should be simple. At least on sunxi the clock driver can tell if the clock is gated and can potentially be in use. It can even mark clocks that are used at this point to know not to ever disable them. Also it should refuse to ever change a clock frequency on these clocks since if one of them was used for simplefb it should break. It does not matter it's a mmc bus clock completely unrelated to display. If you assume no knowledge you cannot change the mmc clock when a different card is inserted or when a card is inserted into an empty slot. If the firmware probed the slot the clock might be running anyway. Other
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 10:03:54AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:39:02AM +0200, Thierry Reding wrote: You keep bringing up the Raspberry Pi for some reason and suggest that it is somehow inferior to sunxi. What makes you think it's less entitled to be supported on Linux than sunxi? I don't care about the Raspberry Pi and I equally don't care about sunxi. I don't own a Raspberry Pi and I don't own any Allwinner hardware. What I do care about is Linux and I want it to work well for all SoCs equally. Perhaps if you could put aside your crusade against the Raspberry Pi for just a second you'll realize that we're all on the same team. This isn't a competition and I'm not trying to put a spoke in your wheel. On the contrary, I'm actually trying to help you. We've been over this already, and I'll tell you again that you're getting this wrong. No platform is more entitled to get merged than another one. I do care about the Allwinner SoCs, and I care just as much about the broader Linux support for all the other SoCs, be it from nvidia, samsung or whatever vendor you can come up with. Okay, I'm glad our goals aren't that far apart then. It would be helpful to stop dragging the Raspberry Pi into this, though, since it isn't at all relevant. But you can't hide the fact that the bcm2835 still has a very limited clock support, and I really don't know about its clock tree, but I guess that if the times come when they add a more complete clock support, they will face the same issue. This isn't at all relevant. And that's exactly why I think it's good to hide all the resource management behind firmware. That way it becomes easy to support any SoC with any firmware. If the driver would have been developped initially to create a framebuffer on the Allwinner SoCs, at a time when we didn't have any clock support too, calling it only usable on sunxi wouldn't have shocked me tbh. The functionality that it provides is still very generic. And the firmware interface is generic too. It is this abstraction that allows it to be generic. You on the other hand seem to be arguing that by making it abstract we've made it less generic. Abstraction is about hiding details to capture commonality. Thierry pgpAFZSGpCS4j.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote: On 30 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: [...] What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. We've already determined that simplefb can't do anything meaningful with the resources other than keep them in the status quo. It simply doesn't have enough knowledge to do so. It doesn't know the exact pixel clock or what voltage the attached panel needs. And the only way I can think of to deal with that properly is to have resources management in the driver. My point is that if we had a proper way to tell the kernel not to do anything with resources owned by firmware, then the driver wouldn't have to do anything with the resources. The firmware on sunxi does not own any resources whatsoever. It ceases running once it executes the kernel. This is different from ACPI and UEFI where you have pieces of the firmware lingering indefinitely and potentially getting invoked by user pressing some button or some other hardware event. It is also different from rpi where the Linux kernel effectively runs in a virtual environment created by the firmware hypervisor. You know all that because you of course wrote every single firmware implementation that does and will ever exist for sunxi. There's nothing keeping anyone from running UEFI on a sunxi SoC. So on sunxi and many other ARM machines the Linux kernel is the sole owner of any resources that might happen to be available on the machine. There is no firmware owning them when the Linux kernel is running, ever. Of course this is part of the abstraction. The idea is that the device is a virtual one created by firmware. Therefore firmware owns the resources until the virtual device has been handed over to the kernel. If you're into splitting hairs, then the simplefb device shouldn't exist in the first place. And we do have a proper way to tell to the kernel what these resources are used for - inserting description of them into the simplefb DT node. Sure the simplefb cannot manage the resources in any way and but it does own them. When it is running they are in use, when it stops they are free to be reclaimed by the platform driver. Yes. And again I'm not saying anything different. What I'm saying is that we shouldn't need to know about the resources and instead hide that within the firmware, for the same reason that we're already hiding the register programming in hardware, namely to create an abstraction that works irrespective of the underlying hardware. But you refuse to marge the kernel change for this proper management to happen. I have no authority to merge these changes nor have I any way of vetoing them. All I'm saying is that I think it's a bad idea. If nobody else thinks it is then it will eventually get merged irrespective of what I'm saying. And when that happens I'll shut up and live happily ever after. But it doesn't magically convince me that it's a good idea. The alternate proposal to stop the kernel from managing resources while simplefb is running and refernce count drivers that cannot manage resources is indeed a workable, general alternative. It does however switch the kernel into special mode when resources are not managed and will needlessly hinder eg. development and testing of
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi, On 09/30/2014 06:59 AM, Thierry Reding wrote: On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote: snip But sure, you can still try to point new issues, get an obvious and robust solution, and then discard the issue when the solution doesn't go your way... And you've already proven that you're completely unwilling to even consider any other solution than what was originally proposed, so I really don't see how discussing this further with you is going to be productive. That is not true, we have seriously considered various other alternatives, as you know since you've participated in the discussion about them. And we've found them all lacking, mostly because they are 10 times as complicated. You've made your point that you don't like this solution quite loudly already, and we've all heard you. However you seem to be mostly alone in this. Even the clk maintainer has said that what we want to do is exactly how clocks are supposed to be used in dt. If you don't like this no-one is forcing you to use the clocks property in your own code. If it is not there, simplefb will behave exactly as before. Now since you're the only one very vocally against this, and a lot of people are in favor of this and have a need for this, can we please just get this merged and get this over with ? 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 01:43:45PM +0200, Hans de Goede wrote: Hi, On 09/30/2014 06:59 AM, Thierry Reding wrote: On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote: snip But sure, you can still try to point new issues, get an obvious and robust solution, and then discard the issue when the solution doesn't go your way... And you've already proven that you're completely unwilling to even consider any other solution than what was originally proposed, so I really don't see how discussing this further with you is going to be productive. That is not true, we have seriously considered various other alternatives, as you know since you've participated in the discussion about them. And we've found them all lacking, mostly because they are 10 times as complicated. You've made your point that you don't like this solution quite loudly already, and we've all heard you. However you seem to be mostly alone in this. Even the clk maintainer has said that what we want to do is exactly how clocks are supposed to be used in dt. If you don't like this no-one is forcing you to use the clocks property in your own code. If it is not there, simplefb will behave exactly as before. Now since you're the only one very vocally against this, and a lot of people are in favor of this and have a need for this, can we please just get this merged and get this over with ? Whatever. I no longer care. Thierry pgpUQ_XRIPqPR.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 10:54:32AM +0200, Thierry Reding wrote: On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: [...] What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. We've already determined that simplefb can't do anything meaningful with the resources other than keep them in the status quo. It simply doesn't have enough knowledge to do so. It doesn't know the exact pixel clock or what voltage the attached panel needs. We do agree that it doesn't care, doesn't need to know it, or doesn't need to do anything about it, but what it needs is that they stay the same. That means both keeping a clock or a regulator enabled, but also preventing any other user (direct, as in a shared regulator, or indirect, as in two clocks sharing the same parent) to change that voltage or pixel clock. You were trying to address the former in your patch, but you completely ignore the second one, which is just as important. And the only way I can think of to deal with that properly is to have resources management in the driver. My point is that if we had a proper way to tell the kernel not to do anything with resources owned by firmware, then the driver wouldn't have to do anything with the resources. Yes, but at least for the clocks, and I guess it might be true in some sick way for regulators too, the fact that it's a tree doesn't make this easy at all. If they were completely independant clocks, yeah, sure, we could do that. But it's almost never the case, and all clocks share parents with other at some degree. I guess you could do it using clock flags of some sort, but that would require traversing the clock tree for almost any operation, which doesn't look very reasonable. I really start to consider adding a sunxi-uboot-fb, that would just duplicate the source code of simplefb but also taking the clocks. Somehow, I feel like it will be easier (and definitely less of a hack than using the generic common clock API). You're not getting it are you? What makes you think sunxi-uboot-fb is going to be any different? This isn't about a name. At least, we would get to do any binding and resource management we want. And that's a big win. So instead of trying to understand the concerns that I've expressed and constructively contribute to finding a solution that works for everybody you'd rather go and write a driver from scratch. Way to go. Hey, you haven't really contributed either to finding a solution to the fact that we need not only to prevent the clocks from being touched during the framework initcall, but also from other related users. I've already said that I'm not saying strictly no to these patches, but what I want to see happen is some constructive discussion about whether we can find better ways to do it. If we can't then I'm all for merging these patches. Fortunately other (sub)threads have been somewhat more constructive and actually come up with alternatives that should make everyone happier. I didn't see where you said that you were not strictly against them. But ok. I guess your concerns all boil down to 1) do not break DT backward compatibility, 2) do not break what the firmware has set up, even on older firmwares. We got 1 covered already, but in order to cover 2, I guess we would need both our solutions, that I don't really see as orthogonal. How would something like adding
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote: 2) Simplefb will only have a single user: the rpi. As the only other users i can think of, which does not have a full driver and which does not have clocks automatically disabled, are discrete cards. And they do not really tend to happen with dt or platform devices. I thought the goal was for other platforms to use simplefb while waiting for the real drivers to be loaded (so you can get get console output as early as possible from a built in driver for example)? signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 8:41 AM, Mark Brown broo...@kernel.org wrote: On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote: 2) Simplefb will only have a single user: the rpi. As the only other users i can think of, which does not have a full driver and which does not have clocks automatically disabled, are discrete cards. And they do not really tend to happen with dt or platform devices. I thought the goal was for other platforms to use simplefb while waiting for the real drivers to be loaded (so you can get get console output as early as possible from a built in driver for example)? That is an option that might work. Stop trying to make simplefb work after the system is fully booted. Instead just let it run until the clocks get shut off. That allows it to go back to being nothing but a simple pointer to the video buffer. Then if you want to keep your display going and don't have a KMS driver written, whip together a device specific framebuffer driver for your hardware that does the right thing with the clocks, etc. The device specific framebuffer driver can load later in the boot process so that it doesn't have to be built into the kernel. This device specfic driver matches on a compatible string and knows what to do with all of the device tree info. -- Jon Smirl jonsm...@gmail.com -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 30 September 2014 13:31, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote: On 30 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote: On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: [...] What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. Sure, we can add any resources we will need. Regulators, reset lines, pm domains, allocated memory, but I'm not really sure this is what you want, right? No it's not what I want. *You* want to add resource management to this driver. What I'm saying is that if we start adding clocks then we can no longer say no to resets or regulators or power domains either. Yes, because resource management can be more than just keep the thing enabled. It might also be about not modifying anything, just like we saw for the clocks, but that might also apply to regulators voltage. We've already determined that simplefb can't do anything meaningful with the resources other than keep them in the status quo. It simply doesn't have enough knowledge to do so. It doesn't know the exact pixel clock or what voltage the attached panel needs. And the only way I can think of to deal with that properly is to have resources management in the driver. My point is that if we had a proper way to tell the kernel not to do anything with resources owned by firmware, then the driver wouldn't have to do anything with the resources. The firmware on sunxi does not own any resources whatsoever. It ceases running once it executes the kernel. This is different from ACPI and UEFI where you have pieces of the firmware lingering indefinitely and potentially getting invoked by user pressing some button or some other hardware event. It is also different from rpi where the Linux kernel effectively runs in a virtual environment created by the firmware hypervisor. You know all that because you of course wrote every single firmware implementation that does and will ever exist for sunxi. There's nothing keeping anyone from running UEFI on a sunxi SoC. The existing 'firmware' or rather loader for sunxi is u-boot. I am not saying other solutions cannot exist. I am describing the current situation. So on sunxi and many other ARM machines the Linux kernel is the sole owner of any resources that might happen to be available on the machine. There is no firmware owning them when the Linux kernel is running, ever. Of course this is part of the abstraction. The idea is that the device is a virtual one created by firmware. Therefore firmware owns the resources until the virtual device has been handed over to the kernel. If you're into splitting hairs, then the simplefb device shouldn't exist in the first place. Why shoudn't it? It is properly created by u-boot and handed over to the kernel with all the required information for the kernel to keep it running or shut it down as it sees fit. And we do have a proper way to tell to the kernel what these resources are used for - inserting description of them into the simplefb DT node. Sure the simplefb cannot manage the resources in any way and but it does own them. When it is running they are in use, when it stops they are free to be reclaimed by the platform driver. Yes. And again I'm not saying anything different. What I'm saying is that we shouldn't need to know about the resources and instead hide that within the firmware, for the same reason that we're already hiding the register programming in hardware, namely to create an abstraction that works irrespective of the underlying hardware. So then hide those resources in the Linux kernel. Because if you are into hair splitting then on sunxi currently the Linux kernel is the firmware and u-boot is only one of the loader stages that ultimately executes the final firmware which is Linux. I really start to consider adding a sunxi-uboot-fb, that would just duplicate the
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote: So long as we're ensuring that when we don't start supporting resources without DT additions or at least require DT additions to actively manage them (which can then include simplefb hookup) we should be fine. I'm not sure I understand what you mean. If we add a driver for the PMIC that exposes these regulators and then add a DT node for the PMIC, we'd still need to fix the firmware to generate the appropriate DT properties to allow simplefb to enable the regulators. No, you don't. It's only if you start describing the regulators in the PMIC in DT that you run into problems. Unconfigured regulators won't be touched. So unless firmware is updated at the same time, regulators will get disabled because they are unused. That won't happen unless the regulators are explicitly described, if they are described as unused then this will of course happen. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 08:03:14AM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 05:11:01PM +0100, Mark Brown wrote: Not really thought this through fully yet but would having phandles to the relevant devices do the job? Kind of lines up with Grant's idea of having dummy drivers. One of the arguments that came up during the discussion of the sunxi patches is that simplefb is going to be used precisely because there is no real driver for the display part of the SoC yet and nobody knows what the binding will look like. So there's nothing to point at currently and for the same reason having a dummy driver won't work. There's simply no definition of what resources are needed. You may well need to extend the binding in future for an actual driver but from the point of view of what's going into the block it really should just be a case of reading the datasheet and mechanically typing that in. If we can work out how to say where the framebuffer is we really ought to be able to work this stuff out. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. We might want to do that in the future, though it's not always the case that reset is the lowest power state. That proves my point. If we ever decide to assert resets by default we'll have yet another subsystem that can potentially break existing DTs. OTOH given the level of breakage that's like to introduce we might just decide not to do that... In the end it brings us back to the very fundamental principles of DT that's been causing so much pain. For things to work properly and in a stable way you have to get the bindings right and complete from the start. That is, it needs to describe every aspect of the hardware block and all links to other components. Or we ned to introduce things in a conservative fashion which does cope with backwards compatibility; it's definitely more work but it is doable. One thing that makes me a bit nervous about this approach in the context of the regulator API is the frequency with which one finds shared supplies. I'm not sure if it's actually a big problem in practice but it makes me worry a bit. We could probably just do something like make refcounting down to zero not actually disable anything for standard regulators to deal with it which might be an idea anyway in the context of this sort of dodge. Yes, that's sort of how I expected clk_ignore_unused to work. The way I understood it, it would cause all unused clocks to be ignored (that is stay enabled if they are). Of course as Geert pointed out in another subthread, taking this all the way means that we have to disable all power management because the firmware device may be sharing resources with other devices and which therefore are not unused. That's a pretty strong argument and I don't have a solution for that. It is only really a problem for cases where the firmware virtual device isn't taken over by a proper driver at some point, though. Indeed, and we also run into trouble for things where we actually need to really turn off the resource for some reason (MMC has some needs here for example). signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Quoting Thierry Reding (2014-09-29 06:54:00) On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. From e156ee56cbe26c9e8df6619dac1a993245afc1d5 Mon Sep 17 00:00:00 2001 From: Mike Turquette mturque...@linaro.org Date: Tue, 30 Sep 2014 14:24:38 -0700 Subject: [PATCH] doc/kernel-parameters.txt: clarify clk_ignore_unused Refine the definition around clk_ignore_unused, which caused some confusion recently on the linux-fbdev and linux-arm-kernel mailing lists[0]. [0] http://lkml.kernel.org/r/20140929135358.GC30998@ulmo Signed-off-by: Mike Turquette mturque...@linaro.org --- Thierry, Please let me know if this wording makes the feature more clear. Documentation/kernel-parameters.txt | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 10d51c2..0ce01fb 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -605,11 +605,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted. See Documentation/s390/CommonIO for details. clk_ignore_unused [CLK] - Keep all clocks already enabled by bootloader on, - even if no driver has claimed them. This is useful - for debug and development, but should not be - needed on a platform with proper driver support. - For more information, see Documentation/clk.txt. + Prevents the clock framework from automatically gating + clocks that have not been explicitly enabled by a Linux + device driver but are enabled in hardware at reset or + by the bootloader/firmware. Note that this does not + force such clocks to be always-on nor does it reserve + those clocks in any way. This parameter is useful for + debug and development, but should not be needed on a + platform with proper driver support. For more + information, see Documentation/clk.txt. clock= [BUGS=X86-32, HW] gettimeofday clocksource override. [Deprecated] -- 1.8.3.2 -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. Passing in clock information via the device tree already requires a non- trivial amount of code in the firmware. A similar amount of code would be necessary for each type of
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry, (CC linux-pm, as PM is the real reason behind disabling unused clocks) (CC gregkh and lkml, for driver core) On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding thierry.red...@gmail.com wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many It doesn't deal with hardware details for hardware components for which no driver is available (yet). That's why the hardware is still in a usable state, after the firmware has set it up. Clocks, regulators, PM domains typically have system-wide implications, and thus need system-wide drivers (in the absence of such drivers, things would work as-is). Note that the driver still requests resources (ioremap the frame buffer), so it needs to know about that tiny piece of hardware detail. people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. The latter indeed complicates things. But see below... [*] If we start extending the binding with board-level details we end up duplicating the device tree node for
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote: Hi Thierry, (CC linux-pm, as PM is the real reason behind disabling unused clocks) (CC gregkh and lkml, for driver core) On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding thierry.red...@gmail.com wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many It doesn't deal with hardware details for hardware components for which no driver is available (yet). That's why the hardware is still in a usable state, after the firmware has set it up. Clocks, regulators, PM domains typically have system-wide implications, and thus need system-wide drivers (in the absence of such drivers, things would work as-is). Note that the driver still requests resources (ioremap the frame buffer), so it needs to know about that tiny piece of hardware detail. That's not a hardware detail. Or at least it isn't hardware specific. It is needed and the same irrespective of display hardware. Clocks, power domains, regulators and all those are not always the same. people. What is passed in via its device tree
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry,
On Mon, Sep 29, 2014 at 10:54 AM, Thierry Reding
thierry.red...@gmail.com wrote:
On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote:
(CC linux-pm, as PM is the real reason behind disabling unused clocks)
(CC gregkh and lkml, for driver core)
On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding
thierry.red...@gmail.com wrote:
If we start extending the binding with board-level details we end up
duplicating the device tree node for the proper video device. Also note
that it won't stop at clocks. Other setups will require regulators to be
listed in this device tree node as well so that they don't get disabled
at late_initcall. And the regulator bindings don't provide a method to
list an arbitrary number of clocks in a single property in the way that
the clocks property works.
Then (optional) regulator support needs to be added.
Can you elaborate?
I'm not so familiar with regulators, but I guess it's similar to clocks?
There may be also resets involved. Fortunately the reset framework is
minimalistic enough not to care about asserting all unused resets at
late_initcall. And other things like power domains may also need to be
kept on.
Fortunately, unlike clocks, PM domains are first class citizens in the
device framework, as they're handled by the driver core.
So just adding a power-domains property to DTS will work, without any
driver change.
Well, the device driver would also need to call into the PM runtime
framework to enable all the first class citizen magic. But even if it
were to do that, you'd still need to add all the domains to the DTB.
Powering up domains can be done solely by the device-specific PM
domain code, without PM runtime. If simplefb is tied to the PM domain
and doesn't do any PM runtime, the domain will stay powered on
(only unused PM domains are powered down via late_initcall).
Note that I'm saying DT*B* here, because the firmware needs to fill in
those properties after the DTS has been compiled. And since most of
these resources are linked via phandle you actually need to resolve
these first before you can fill in the new properties of this
dynamically created node.
So firmware needs to know exactly what device tree node to look for,
find a corresponding phandle and then put the phandle value in the
simplefb device tree node. And it needs to know intimate details about
the clock provider binding because it needs to add an appropriate
specifier, too.
Indeed. Complicated.
And then all of a sudden something that was supposed to be simple and
generic needs to know the specifics of some hardware device.
And suddenly we wish we could write a real driver and put the stuff in
the DTS, not DTB...
The only reasonable thing for simplefb to do is not deal with any kind
of resource at all (except perhaps area that contains the framebuffer
memory).
So how about instead of requiring resources to be explicitly claimed we
introduce something like the below patch? The intention being to give
firmware device drivers a way of signalling to the clock framework
that they need rely on clocks set up by firmware and when they no longer
need them. This implements essentially what Mark (CC'ing again on this
subthread) suggested earlier in this thread. Basically, it will allow
drivers to determine the time when unused clocks are really unused. It
will of course only work when used correctly by drivers. For the case of
simplefb I'd expect its .probe() implementation to call the new
clk_ignore_unused() function and once it has handed over control of the
display hardware to the real driver it can call clk_unignore_unused() to
signal that all unused clocks that it cares about have now been claimed.
This is reference counted and can therefore be used by more than a
single driver if necessary. Similar functionality could be added for
other resource subsystems as needed.
This still won't work for modules, right? Or am I missing something?
With modules you will never know in advance what will be used and what
won't be used, so you need to keep all clocks, regulators, PM domains, ...
up and running?
No. The way this works is that your firmware shim driver, simplefb in
this case, will call clk_ignore_unused() to tell the clock framework
that it uses clocks set up by the firmware, and therefore requests that
no clocks should be considered unused (for now). Later on when the
proper driver has successfully taken over from the shim driver, the shim
driver can unregister itself and call clk_unignore_unused(), which will
drop its reference on the unused clocks. When all references have been
dropped the clock framework will then disable all remaining unused
clocks.
So the shim must be built-in, not modular.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. There may be also resets involved. Fortunately the reset framework is minimalistic enough
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 11:10:53AM +0200, Geert Uytterhoeven wrote:
Hi Thierry,
On Mon, Sep 29, 2014 at 10:54 AM, Thierry Reding
thierry.red...@gmail.com wrote:
On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote:
(CC linux-pm, as PM is the real reason behind disabling unused clocks)
(CC gregkh and lkml, for driver core)
On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding
thierry.red...@gmail.com wrote:
If we start extending the binding with board-level details we end up
duplicating the device tree node for the proper video device. Also note
that it won't stop at clocks. Other setups will require regulators to be
listed in this device tree node as well so that they don't get disabled
at late_initcall. And the regulator bindings don't provide a method to
list an arbitrary number of clocks in a single property in the way that
the clocks property works.
Then (optional) regulator support needs to be added.
Can you elaborate?
I'm not so familiar with regulators, but I guess it's similar to clocks?
The bindings are different. Essentially what you use is a *-supply
property per regulator. There is no way to specify more than one
regulator in a single property.
So if you want to keep that generic you have to do crazy things like:
simplefb {
enable-0-supply = reg1;
enable-1-supply = reg2;
...
};
I suppose a more generic supplies property could be created to support
this use-case, but I think this kind of proves my point. The only way
that the original proposal is going to work for other resources is if
they follow the same kind of binding. I don't think it makes sense to
introduce such a prerequisite merely because it would make life easy
for some exotic driver with a very specific application.
And then all of a sudden something that was supposed to be simple and
generic needs to know the specifics of some hardware device.
And suddenly we wish we could write a real driver and put the stuff in
the DTS, not DTB...
Oh, there's no doubt a real driver would be preferrable. Note that
simplefb is only meant to be a shim to pass a framebuffer from firmware
to kernel. In some cases it can be used with longer lifetime, like for
example if you really want to have graphical output but the real driver
isn't there yet.
Being a shim driver is precisely the reason why I think the binding
shouldn't be extended to cover all possible types of resources. That
should all go into the binding for the real device.
The only reasonable thing for simplefb to do is not deal with any kind
of resource at all (except perhaps area that contains the framebuffer
memory).
So how about instead of requiring resources to be explicitly claimed we
introduce something like the below patch? The intention being to give
firmware device drivers a way of signalling to the clock framework
that they need rely on clocks set up by firmware and when they no longer
need them. This implements essentially what Mark (CC'ing again on this
subthread) suggested earlier in this thread. Basically, it will allow
drivers to determine the time when unused clocks are really unused. It
will of course only work when used correctly by drivers. For the case of
simplefb I'd expect its .probe() implementation to call the new
clk_ignore_unused() function and once it has handed over control of the
display hardware to the real driver it can call clk_unignore_unused() to
signal that all unused clocks that it cares about have now been claimed.
This is reference counted and can therefore be used by more than a
single driver if necessary. Similar functionality could be added for
other resource subsystems as needed.
This still won't work for modules, right? Or am I missing something?
With modules you will never know in advance what will be used and what
won't be used, so you need to keep all clocks, regulators, PM domains, ...
up and running?
No. The way this works is that your firmware shim driver, simplefb in
this case, will call clk_ignore_unused() to tell the clock framework
that it uses clocks set up by the firmware, and therefore requests that
no clocks should be considered unused (for now). Later on when the
proper driver has successfully taken over from the shim driver, the shim
driver can unregister itself and call clk_unignore_unused(), which will
drop its reference on the unused clocks. When all references have been
dropped the clock framework will then disable all remaining unused
clocks.
So the shim must be built-in, not modular.
Correct. Making it a module isn't very useful in my opinion. You'd loose
all the advantages.
Thierry
pgpKAyGHybPqz.pgp
Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 29 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote: Hi Thierry, (CC linux-pm, as PM is the real reason behind disabling unused clocks) (CC gregkh and lkml, for driver core) On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding thierry.red...@gmail.com wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many It doesn't deal with hardware details for hardware components for which no driver is available (yet). That's why the hardware is still in a usable state, after the firmware has set it up. Clocks, regulators, PM domains typically have system-wide implications, and thus need system-wide drivers (in the absence of such drivers, things would work as-is). Note that the driver still requests resources (ioremap the frame buffer), so it needs to know about that tiny piece of hardware detail. That's not a hardware detail. Or at least it isn't hardware specific. It is needed and the same irrespective of display hardware. Clocks, power domains,
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. Passing in clock information via the device tree already requires a non- trivial amount of code in the firmware. A similar amount of code would be necessary for each type of resource that needs to be kept enabled. In addition to the above some devices may also require resources that have no generic bindings. That just doesn't scale. The only reasonable thing for simplefb to do is not deal with any kind of resource at all (except perhaps area that contains the framebuffer memory). You should really read that thread: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/284726.html It's quite interesting, because you'll see that: A) Your approach, even on the platform you're working on, doesn't work. Or at least, isn't reliable. What platform exactly do you think I'm working on? Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. B) Other maintainers, precisely like Mark, came to the same conclusion than Mike. Well, and others didn't. Also I think if you read that thread and look at my proposal it matches exactly what was discussed as one of the solutions at one point in the thread. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic clk_get and clk_prepare_enable, you're willing to do a just as much generic clk_ignore_unused call? Yes. How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point, so duplicating what the real driver does within the simplefb driver is just that, duplication. We don't allow duplication anywhere else in the kernel, why should simplefb be an exception? You know that you are going to call that for regulator, reset, power domains, just as you would have needed to with the proper API, unless that with this kind of solution, you would have to modify *every* framework that might interact with any resource involved in getting simplefb running? We have to add handling for every kind of resource either way. Also if this evolves into a common pattern we can easily wrap it up in a single function call. Plus, speaking more specifically about the clocks,
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry,
On Mon, Sep 29, 2014 at 12:18 PM, Thierry Reding
thierry.red...@gmail.com wrote:
How is that less generic?
It's more generic. That's the whole point.
The difference is that with the solution I proposed we don't have to
keep track of all the resources. We know that firmware has set them up
and we know that a real driver will properly take them over at some
point, so duplicating what the real driver does within the simplefb
driver is just that, duplication. We don't allow duplication anywhere
else in the kernel, why should simplefb be an exception?
You know that you are going to call that for regulator, reset, power
domains, just as you would have needed to with the proper API, unless
that with this kind of solution, you would have to modify *every*
framework that might interact with any resource involved in getting
simplefb running?
We have to add handling for every kind of resource either way. Also if
this evolves into a common pattern we can easily wrap it up in a single
function call.
disable_all_power_management(), as this is not limited to clocks.
Plus, speaking more specifically about the clocks, that won't prevent
your clock to be shut down as a side effect of a later clk_disable
call from another driver.
Furthermore isn't it a bug for a driver to call clk_disable() before a
preceding clk_enable()? There are patches being worked on that will
enable per-user clocks and as I understand it they will specifically
disallow drivers to disable the hardware clock if other drivers are
still keeping them on via their own referenc.
Calling clk_disable() preceding clk_enable() is a bug.
Calling clk_disable() after clk_enable() will disable the clock (and
its parents)
if the clock subsystem thinks there are no other users, which is what will
happen here.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 11:44:19AM +0200, Michal Suchanek wrote: On 29 September 2014 10:54, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 10:27:41AM +0200, Geert Uytterhoeven wrote: Hi Thierry, (CC linux-pm, as PM is the real reason behind disabling unused clocks) (CC gregkh and lkml, for driver core) On Mon, Sep 29, 2014 at 10:06 AM, Thierry Reding thierry.red...@gmail.com wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: Quoting Maxime Ripard (2014-09-02 02:25:08) On Fri, Aug 29, 2014 at 04:38:14PM +0200, Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:12:44PM +0200, Maxime Ripard wrote: On Fri, Aug 29, 2014 at 09:01:17AM +0200, Thierry Reding wrote: I would think the memory should still be reserved anyway to make sure nothing else is writing over it. And it's in the device tree anyway because the driver needs to know where to put framebuffer content. So the point I was trying to make is that we can't treat the memory in the same way as clocks because it needs to be explicitly managed. Whereas clocks don't. The driver is simply too generic to know what to do with the clocks. You agreed on the fact that the only thing we need to do with the clocks is claim them. Really, I don't find what's complicated there (or not generic). That's not what I agreed on. What I said is that the only thing we need to do with the clocks is nothing. They are already in the state that they need to be. Claim was probably a poor choice of words, but still. We have to keep the clock running, and both the solution you've been giving and this patch do so in a generic way. It doesn't know what frequency they should be running at We don't care about that. Just like we don't care about which frequency is the memory bus running at. It will just run at whatever frequency is appropriate. Exactly. And you shouldn't have to care about them at all. Firmware has already configured the clocks to run at the correct frequencies, and it has made sure that they are enabled. or what they're used for And we don't care about that either. You're not interested in what output the framebuffer is setup to use, which is pretty much the same here, this is the same thing here. That's precisely what I've been saying. The only thing that simplefb cares about is the memory it should be using and the format of the pixels (and how many of them) it writes into that memory. Everything else is assumed to have been set up. Including clocks. We're really discussing in circles here. Mike? -EHIGHLATENCYRESPONSE I forgot about this thread. Sorry. In an attempt to provide the least helpful answer possible, I will stay clear of all of the stuff relating to how simple should simplefb be and the related reserved memory discussion. A few times in this thread it is stated that we can't prevent unused clocks from being disabled. That is only partially true. The clock framework DOES provide a flag to prevent UNUSED clocks from being disabled at late_initcall-time by a clock garbage collector mechanism. Maxime and others familiar with the clock framework are aware of this. What the framework doesn't do is to allow for a magic flag in DT, in platform_data or elsewhere that says, don't let me get turned off until the right driver claims me. That would be an external or alternative method for preventing a clock from being disabled. We have a method for preventing clocks from being disabled. It is as follows: struct clk *my_clk = clk_get(...); clk_prepare_enable(my_clk); That is how it should be done. Period. With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many It doesn't deal with hardware details for hardware components for which no driver is available (yet). That's why the hardware is still in a usable state, after the firmware has set it up. Clocks, regulators, PM domains typically have system-wide implications, and thus need system-wide drivers (in the absence of such drivers, things would work as-is). Note that the driver still requests resources (ioremap the frame buffer), so it needs to know
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 12:35:17PM +0200, Geert Uytterhoeven wrote: Hi Thierry, On Mon, Sep 29, 2014 at 12:18 PM, Thierry Reding thierry.red...@gmail.com wrote: How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point, so duplicating what the real driver does within the simplefb driver is just that, duplication. We don't allow duplication anywhere else in the kernel, why should simplefb be an exception? You know that you are going to call that for regulator, reset, power domains, just as you would have needed to with the proper API, unless that with this kind of solution, you would have to modify *every* framework that might interact with any resource involved in getting simplefb running? We have to add handling for every kind of resource either way. Also if this evolves into a common pattern we can easily wrap it up in a single function call. disable_all_power_management(), as this is not limited to clocks. Right. But it isn't all power management either. It just shouldn't turn everything unused off. Clocks, regulators, power domains and so on which are used can very well be power managed. Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. Either way, if there are other users of a clock then they will just as likely want to modify the rate at which point simplefb will break just as badly. Thierry pgpxfSBHORkJm.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry, On Mon, Sep 29, 2014 at 8:18 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. As far as the kernel is concerned, this is a solved problem. Firmware is going to be doing some dark magic to set up the hardware to be a dumb frame buffer and some other stuff to add the simplefb device node - so by this point, adding the clocks (or whatever) required by the hardware should be fairly uncomplicated - the firmware already knows the hardware intimately. As for the actual device tree manipulations, U-boot (or whatever) will probably just grow some helper functions to make this simple. Alternatively, it could simply add the relevant data to an existing device node and munge it's compatible property so simplefb picks it up. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. Passing in clock information via the device tree already requires a non- trivial amount of code in the firmware. A similar amount of code would be necessary for each type of resource that needs to be kept enabled. In addition to the above some devices may also require resources that have no generic bindings. That just doesn't scale. The only reasonable thing for simplefb to do is not deal with any kind of resource at all (except perhaps area that contains the framebuffer memory). You should really read that thread: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/284726.html It's quite interesting, because you'll see that: A) Your approach, even on the platform you're working on, doesn't work. Or at least, isn't reliable. What platform exactly do you think I'm working on? Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. B) Other maintainers, precisely like Mark, came to the same conclusion than Mike. Well, and others didn't. Also I think if you read that thread and look at my proposal it matches exactly what was discussed as one of the solutions at one point in the thread. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic clk_get and clk_prepare_enable, you're willing to do a just as much generic clk_ignore_unused call? Yes. How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry,
On Mon, Sep 29, 2014 at 12:44 PM, Thierry Reding
thierry.red...@gmail.com wrote:
You know that you are going to call that for regulator, reset, power
domains, just as you would have needed to with the proper API, unless
that with this kind of solution, you would have to modify *every*
framework that might interact with any resource involved in getting
simplefb running?
We have to add handling for every kind of resource either way. Also if
this evolves into a common pattern we can easily wrap it up in a single
function call.
disable_all_power_management(), as this is not limited to clocks.
Right. But it isn't all power management either. It just shouldn't turn
everything unused off. Clocks, regulators, power domains and so on which
are used can very well be power managed.
No they can't, as the clock/regulator/PM domain core cannot know if any
of the used ones are also used by a shim driver like simplefb.
Clocks and regulators may be shared. PM domains can contain multiple
hardware blocks. Without more knowledge, the only safe thing is not
disabling anything.
Plus, speaking more specifically about the clocks, that won't prevent
your clock to be shut down as a side effect of a later clk_disable
call from another driver.
Furthermore isn't it a bug for a driver to call clk_disable() before a
preceding clk_enable()? There are patches being worked on that will
enable per-user clocks and as I understand it they will specifically
disallow drivers to disable the hardware clock if other drivers are
still keeping them on via their own referenc.
Calling clk_disable() preceding clk_enable() is a bug.
Calling clk_disable() after clk_enable() will disable the clock (and
its parents)
if the clock subsystem thinks there are no other users, which is what will
happen here.
Right. I'm not sure this is really applicable to this situation, though.
Yes it is: if all users of a clock/regulator/PM domain are gone, it will
be disabled. Bad luck for simplefb still needing them.
Either way, if there are other users of a clock then they will just as
likely want to modify the rate at which point simplefb will break just
as badly.
BTW, this can also happen for clocks that are properly used.
I guess the clock core code does some arbitration to handle such cases.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
--
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+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Either way, if there are other users of a clock then they will just as likely want to modify the rate at which point simplefb will break just as badly. And this can be handled just as well. Register a clock notifier, refuse any rate change, done. But of course, that would require having a clock handle. Now, how would *you* prevent such a change? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. Passing in clock information via the device tree already requires a non- trivial amount of code in the firmware. A similar amount of code would be necessary for each type of resource that needs to be kept enabled. In addition to the above some devices may also require resources that have no generic bindings. That just doesn't scale. The only reasonable thing for simplefb to do is not deal with any kind of resource at all (except perhaps area that contains the framebuffer memory). You should really read that thread: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/284726.html It's quite interesting, because you'll see that: A) Your approach, even on the platform you're working on, doesn't work. Or at least, isn't reliable. What platform exactly do you think I'm working on? My bad, I thought it was a tegra SoC in there. I should have read more carefully obviously. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? B) Other maintainers, precisely like Mark, came to the same conclusion than Mike. Well, and others didn't. We've been talking about both clocks and regulators up to now. I can see Mike and Mark both suggesting to use the usual clocks and regulators APIs, either in that thread or the one I pointed out. Also I think if you read that thread and look at my proposal it matches exactly what was discussed as one of the solutions at one point in the thread. I've seen it, and replied to that already. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic clk_get and clk_prepare_enable, you're willing to do a just as much generic clk_ignore_unused call? Yes. How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point You keep saying that... and you know that you can't make
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 09:00:01PM +1000, Julian Calaby wrote: Hi Thierry, If you address people directly please make sure they are in the To: line. Or at least Cc. On Mon, Sep 29, 2014 at 8:18 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. As far as the kernel is concerned, this is a solved problem. It's not completely solved. There's still the issue of no generic way to specify regulators like you can do for clocks, resets or power domains. But the kernel isn't the real issue here. The issue is the firmware that now has to go out of its way not only to initialize display hardware but also create device tree content just to make Linux not turn everything off. Firmware is going to be doing some dark magic to set up the hardware to be a dumb frame buffer and some other stuff to add the simplefb device node - so by this point, adding the clocks (or whatever) required by the hardware should be fairly uncomplicated - the firmware already knows the hardware intimately. As for the actual device tree manipulations, U-boot (or whatever) will probably just grow some helper functions to make this simple. Have you looked at the code needed to do this? It's not at all trivial. And the point is really that all this information is there already, so we're completely duplicating this into a dynamically created device tree node and for what reason? Only to have one driver request all these resources and have them forcefully released a few seconds later. Alternatively, it could simply add the relevant data to an existing device node and munge it's compatible property so simplefb picks it up. Yes, I think that'd be a much better solution. Of course it's going to be very difficult to make that work with a generic driver because now that generic driver needs to parse the DT binding for any number of compatible devices. Thierry pgpXeaVvCkSNz.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. Perhaps read the sentence again. I said that we would *otherwise* be able to do much easier with a text editor.. My point remains that there shouldn't be a need to generate DTB content of this complexity at all. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? That's exactly the point. And it's going to break again and again as simplefb is extended with new things. Generally DT bindings should be backwards compatible. When extended they should provide a way to fall back to a reasonable default. There's simply no way you can do that with simplefb. What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic clk_get and clk_prepare_enable, you're willing to do a just as much generic clk_ignore_unused call? Yes. How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point You keep saying that... and you know that you can't make this assumption. Why not? Are you really expecting to keep running with simplefb forever? Nobody is going to seriously use an upstream kernel in any product with only simplefb as a framebuffer. I've said before that this is a hack to get you working display. And that's all it is. If you want to do it properly go and write a DRM/KMS driver. You know that you are going to call that for regulator, reset, power domains, just as you would have needed to with the proper API, unless that with this kind of solution, you would have to modify *every* framework that might interact with any resource involved in getting simplefb running? We have to add handling for every kind of resource either way. Also if this evolves into a common pattern we can easily wrap it up in a single function call. Unless
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. Either way, if there are other users of a clock then they will just as likely want to modify the rate at which point simplefb will break just as badly. And this can be handled just as well. Register a clock notifier, refuse any rate change, done. But of course, that would require having a clock handle. Now, how would *you* prevent such a change? Like I said in the other thread. If you have two drivers that use the same clock but need different frequencies you've lost anyway. Thierry pgpKPih4ESVw2.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 01:32:44PM +0200, Geert Uytterhoeven wrote: Hi Thierry, On Mon, Sep 29, 2014 at 12:44 PM, Thierry Reding thierry.red...@gmail.com wrote: You know that you are going to call that for regulator, reset, power domains, just as you would have needed to with the proper API, unless that with this kind of solution, you would have to modify *every* framework that might interact with any resource involved in getting simplefb running? We have to add handling for every kind of resource either way. Also if this evolves into a common pattern we can easily wrap it up in a single function call. disable_all_power_management(), as this is not limited to clocks. Right. But it isn't all power management either. It just shouldn't turn everything unused off. Clocks, regulators, power domains and so on which are used can very well be power managed. No they can't, as the clock/regulator/PM domain core cannot know if any of the used ones are also used by a shim driver like simplefb. Clocks and regulators may be shared. PM domains can contain multiple hardware blocks. Without more knowledge, the only safe thing is not disabling anything. Indeed. That's a shame. In the most common case that probably won't matter all that much, given that the real driver can be expected to load within a reasonable amount of time. Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. Yes it is: if all users of a clock/regulator/PM domain are gone, it will be disabled. Bad luck for simplefb still needing them. Hmm... if all users are gone, then aren't the resources unused again and should therefore be ignored? Thierry pgpIoQc_lLPua.pgp Description: PGP signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
Hi Thierry, On Mon, Sep 29, 2014 at 11:21 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 09:00:01PM +1000, Julian Calaby wrote: Hi Thierry, If you address people directly please make sure they are in the To: line. Or at least Cc. Sorry about that, the mailing list I received this through (Google Groups based) generally strips to: and CC: lines, so my mail client (Gmail) doesn't do it automatically. I'm still getting used to it. On Mon, Sep 29, 2014 at 8:18 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. As far as the kernel is concerned, this is a solved problem. It's not completely solved. There's still the issue of no generic way to specify regulators like you can do for clocks, resets or power domains. But the kernel isn't the real issue here. The issue is the firmware that now has to go out of its way not only to initialize display hardware but also create device tree content just to make Linux not turn everything off. My point is that the firmware is going to be doing complicated stuff already, adding and using some helpers to configure a device tree node is relatively simple in comparison to dealing with the actual hardware. It wouldn't surprise me if u-boot, for example, ended up with a set of functions to handle this exact case as more graphics hardware gets brought up. Firmware is going to be doing some dark magic to set up the hardware to be a dumb frame buffer and some other stuff to add the simplefb device node - so by this point, adding the clocks (or whatever) required by the hardware should be fairly uncomplicated - the firmware already knows the hardware intimately. As for the actual device tree manipulations, U-boot (or whatever) will probably just grow some helper functions to make this simple. Have you looked at the code needed to do this? It's not at all trivial. And the point is really that all this information is there already, so we're completely duplicating this into a dynamically created device tree node and for what reason? Only to have one driver request all these resources and have them forcefully released a few seconds later. Alternatively, it could simply add the relevant data to an existing device node and munge it's compatible property so simplefb picks it up. Yes, I think that'd be a much better solution. Of course it's going to be very difficult to make that work with a generic driver because now that generic driver needs to parse the DT binding for any number of compatible devices. Not necessarily. The patch that started this discussion can work with any number of clocks specified in a clocks property. Therefore all that needs to happen is that the final hardware binding specifies it's clocks that way. This is how, for example, the ahci_platform driver's clock code works. I'm sure that as hardware diversifies, the other subsystems will grow in similar directions and eventually be dealt with using similarly generic code. Thanks, -- Julian Calaby Email: julian.cal...@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 29 September 2014 15:47, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. Perhaps read the sentence again. I said that we would *otherwise* be able to do much easier with a text editor.. My point remains that there shouldn't be a need to generate DTB content of this complexity at all. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? That's exactly the point. And it's going to break again and again as simplefb is extended with new things. Generally DT bindings should be backwards compatible. When extended they should provide a way to fall back to a reasonable default. There's simply no way you can do that with simplefb. What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. So what? You get a driver for regulators and suddenly find that nothing registered regulators because they were on all the time anyway and everything breaks? What a surprise! The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. Sure. And what can you do about that? It's not like the original Snow firmware writes anything of use to the DT at all. So to run a development kernel you need a development firmware. If you add new features to the kernel that involve intefacing to the firmware you need to update the firmware as well. Once support for Snow platform is stable you can expect that users can use a stable release of firmware indefinitely. And the same will continue to happen for every new type of resource you're going to add. The like 5 resource types, yes. Some of which may not even apply to simplefb. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic clk_get and clk_prepare_enable, you're willing to do a just as much generic clk_ignore_unused call? Yes. How is that less generic? It's more generic. That's the whole point. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point You keep saying that... and you know that you can't make this assumption. Why not?
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Tue, Sep 30, 2014 at 12:46:11AM +1000, Julian Calaby wrote: Hi Thierry, On Mon, Sep 29, 2014 at 11:21 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 09:00:01PM +1000, Julian Calaby wrote: Hi Thierry, If you address people directly please make sure they are in the To: line. Or at least Cc. Sorry about that, the mailing list I received this through (Google Groups based) generally strips to: and CC: lines, so my mail client (Gmail) doesn't do it automatically. I'm still getting used to it. Yeah, I wish mailing lists would stop doing that. On Mon, Sep 29, 2014 at 8:18 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. As far as the kernel is concerned, this is a solved problem. It's not completely solved. There's still the issue of no generic way to specify regulators like you can do for clocks, resets or power domains. But the kernel isn't the real issue here. The issue is the firmware that now has to go out of its way not only to initialize display hardware but also create device tree content just to make Linux not turn everything off. My point is that the firmware is going to be doing complicated stuff already, adding and using some helpers to configure a device tree node is relatively simple in comparison to dealing with the actual hardware. It wouldn't surprise me if u-boot, for example, ended up with a set of functions to handle this exact case as more graphics hardware gets brought up. Not all firmware is based on U-Boot. Essentially whatever binding changes we make will need to be implemented in all firmware. And the complexity isn't so much about writing the actual DT data, but more about figuring out which data to write. Every firmware image needs to know exactly which clocks and other resources to transcribe for a given board. It'll essentially need to contain some sort of driver for each device that parses a DTB, correlates the data to what it knows of the device internals and write a subset of that data back into the DTB in a slightly different format. That's just whacky. DT was meant to simplify things. Firmware is going to be doing some dark magic to set up the hardware to be a dumb frame buffer and some other stuff to add the simplefb device node - so by this point, adding the clocks (or whatever) required by the hardware should be fairly uncomplicated - the firmware already knows the hardware intimately. As for the actual device tree manipulations, U-boot (or whatever) will probably just grow some helper functions to make this simple. Have you looked at the code needed to do this? It's not at all trivial. And the point is really that all this information is there already, so we're completely duplicating this into a dynamically created device tree node and for what reason? Only to have one driver request all these resources and have them forcefully released a few seconds later. Alternatively, it could simply add the relevant data to an existing device node and munge it's compatible property so simplefb picks it up. Yes, I think that'd be a much better solution. Of course it's going to be very difficult to make that work with a generic driver because now that generic driver needs to parse the DT binding for any number of compatible devices. Not necessarily. The patch that started this discussion can work with any number of clocks specified in a clocks property. Therefore all that needs to happen is that the final hardware binding specifies it's clocks that way. Are you suggesting that we should be modeling the hardware specific binding to match what the simplefb binding does? I'm sure that as hardware diversifies, the other subsystems will grow in similar directions and eventually be dealt with using similarly generic code. For regulators this already works very differently. As opposed to the clocks/clock-names type of binding it uses one where the consumer name of the regulator comes from the prefix of a -supply property. That is you'd get
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 29 September 2014 17:19, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Sep 30, 2014 at 12:46:11AM +1000, Julian Calaby wrote: On Mon, Sep 29, 2014 at 8:18 PM, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. As far as the kernel is concerned, this is a solved problem. It's not completely solved. There's still the issue of no generic way to specify regulators like you can do for clocks, resets or power domains. But the kernel isn't the real issue here. The issue is the firmware that now has to go out of its way not only to initialize display hardware but also create device tree content just to make Linux not turn everything off. My point is that the firmware is going to be doing complicated stuff already, adding and using some helpers to configure a device tree node is relatively simple in comparison to dealing with the actual hardware. It wouldn't surprise me if u-boot, for example, ended up with a set of functions to handle this exact case as more graphics hardware gets brought up. Not all firmware is based on U-Boot. Essentially whatever binding changes we make will need to be implemented in all firmware. And the complexity isn't so much about writing the actual DT data, but more about figuring out which data to write. Every firmware image needs to know exactly which clocks and other resources to transcribe for a given board. It'll essentially need to contain some sort of driver for each device that parses a DTB, correlates the data to what it knows of the device internals and write a subset of that data back into the DTB in a slightly different format. That's just whacky. DT was meant to simplify things. The firmware only needs to implement DT parsing and writing if it wants to communicate the configuration it set up to the kernel. If you do not have control over the firmware or do not want to write support for the firmware to generate the DT you can produce a fixed configuration DT and have the firmware load that with the kernel. It is fully backwards compatible with dumb firmware that does not support DT modification during boot. You will just need a different DT for slightly different devices (eg. tablets with different display) or different configurations of same device (eg. different displays connected to the HDMI port of your devboard). I'm sure that as hardware diversifies, the other subsystems will grow in similar directions and eventually be dealt with using similarly generic code. For regulators this already works very differently. As opposed to the clocks/clock-names type of binding it uses one where the consumer name of the regulator comes from the prefix of a -supply property. That is you'd get something like this: foo-supply = reg1; bar-supply = reg2; And since you don't have enough information in the kernel simplefb driver to attach any meaning to these, the best you can do would be iterating over a range and have: 0-supply = reg0; 1-supply = reg1; ... n-supply = reg2; This is made more difficult by the fact that these regulators may be required by components not immediately related to the display engine. They could be for an attached panel, a video bridge or the +5V pin on the HDMI connector. So you are saying that listing the properties of the simplefb node and filtering foo-supply and bar-supply out of that is too difficult? I hope kernel development did not get this dumb. Thanks Michal -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 05:04:32PM +0200, Michal Suchanek wrote: On 29 September 2014 15:47, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. Perhaps read the sentence again. I said that we would *otherwise* be able to do much easier with a text editor.. My point remains that there shouldn't be a need to generate DTB content of this complexity at all. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? That's exactly the point. And it's going to break again and again as simplefb is extended with new things. Generally DT bindings should be backwards compatible. When extended they should provide a way to fall back to a reasonable default. There's simply no way you can do that with simplefb. What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. So what? You get a driver for regulators and suddenly find that nothing registered regulators because they were on all the time anyway and everything breaks? What a surprise! I agree, this is not a surprise at all. But like I pointed out this is bound to happen again and again. So how about we learn from it and add the functionality needed to prevent it from happening? The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. Sure. And what can you do about that? It's not like the original Snow firmware writes anything of use to the DT at all. So to run a development kernel you need a development firmware. If you add new features to the kernel that involve intefacing to the firmware you need to update the firmware as well. Once support for Snow platform is stable you can expect that users can use a stable release of firmware indefinitely. Feel free to take that up with the DT ABI stability committee. And the same will continue to happen for every new type of resource you're going to add. The like 5 resource types, yes. Some of which may not even apply to simplefb. Today it's 5, tomorrow 6, next year, who knows. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. So, just to be clear, instead of doing a generic
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. Not quite - there was also DT added for them. With just the driver the regulator API shouldn't have touched them, it'd just have exposed them read only. The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. And the same will continue to happen for every new type of resource you're going to add. So long as we're ensuring that when we don't start supporting resources without DT additions or at least require DT additions to actively manage them (which can then include simplefb hookup) we should be fine. The difference is that with the solution I proposed we don't have to keep track of all the resources. We know that firmware has set them up and we know that a real driver will properly take them over at some point You keep saying that... and you know that you can't make this assumption. Why not? Are you really expecting to keep running with simplefb forever? Nobody is going to seriously use an upstream kernel in any product with only simplefb as a framebuffer. I've said before that this is a hack to get you working display. And that's all it is. If you want to do it properly go and write a DRM/KMS driver. Well, for product probably not. But in terms of the runtime of a kernel I'd not be so sure - people do dogfood and we should be allowing that. I've used unaccelerated displays with reasonable success for a large part of my work in the past. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: On Sat, Sep 27, 2014 at 04:56:01PM -0700, Mike Turquette wrote: With that said I think that Luc's approach is very sensible. I'm not sure what purpose in the universe DT is supposed to serve if not for _this_exact_case_. We have a nice abstracted driver, usable by many people. The hardware details of how it is hooked up at the board level can all be hidden behind stable DT bindings that everyone already uses. simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. If we start extending the binding with board-level details we end up duplicating the device tree node for the proper video device. Also note that it won't stop at clocks. Other setups will require regulators to be listed in this device tree node as well so that they don't get disabled at late_initcall. And the regulator bindings don't provide a method to list an arbitrary number of clocks in a single property in the way that the clocks property works. Not really thought this through fully yet but would having phandles to the relevant devices do the job? Kind of lines up with Grant's idea of having dummy drivers. There may be also resets involved. Fortunately the reset framework is minimalistic enough not to care about asserting all unused resets at late_initcall. And other things like power domains may also need to be kept on. We might want to do that in the future, though it's not always the case that reset is the lowest power state. So how about instead of requiring resources to be explicitly claimed we introduce something like the below patch? The intention being to give firmware device drivers a way of signalling to the clock framework that they need rely on clocks set up by firmware and when they no longer need them. This implements essentially what Mark (CC'ing again on this subthread) suggested earlier in this thread. Basically, it will allow drivers to determine the time when unused clocks are really unused. It will of course only work when used correctly by drivers. For the case of simplefb I'd expect its .probe() implementation to call the new clk_ignore_unused() function and once it has handed over control of the display hardware to the real driver it can call clk_unignore_unused() to signal that all unused clocks that it cares about have now been claimed. This is reference counted and can therefore be used by more than a single driver if necessary. Similar functionality could be added for other resource subsystems as needed. One thing that makes me a bit nervous about this approach in the context of the regulator API is the frequency with which one finds shared supplies. I'm not sure if it's actually a big problem in practice but it makes me worry a bit. We could probably just do something like make refcounting down to zero not actually disable anything for standard regulators to deal with it which might be an idea anyway in the context of this sort of dodge. signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On 29 September 2014 17:49, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 05:04:32PM +0200, Michal Suchanek wrote: On 29 September 2014 15:47, Thierry Reding thierry.red...@gmail.com wrote: On Mon, Sep 29, 2014 at 01:46:43PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:18:08PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 11:23:01AM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 10:06:39AM +0200, Thierry Reding wrote: [...] simplefb doesn't deal at all with hardware details. It simply uses what firmware has set up, which is the only reason why it will work for many people. What is passed in via its device tree node is the minimum amount of information needed to draw something into the framebuffer. Also note that the simplefb device tree node is not statically added to a DTS file but needs to be dynamically generated by firmware at runtime. Which makes the whole even simpler, since the firmware already knows all about which clocks it had to enable. It makes things very complicated in the firmware because it now needs to be able to generate DTB content that we would otherwise be able to do much easier with a text editor. Didn't you just say that it was dynamically generated at runtime? So we can just ignore the text editor case. Perhaps read the sentence again. I said that we would *otherwise* be able to do much easier with a text editor.. My point remains that there shouldn't be a need to generate DTB content of this complexity at all. Why do you think what I proposed isn't going to work or be reliable? I don't see any arguments in the thread that would imply that. The fact that it broke in the first place? That's exactly the point. And it's going to break again and again as simplefb is extended with new things. Generally DT bindings should be backwards compatible. When extended they should provide a way to fall back to a reasonable default. There's simply no way you can do that with simplefb. What happened in the Snow example is that regulators that were previously on would all of a sudden be automatically disabled on boot because there was now a driver that registered them with a generic framework. So what? You get a driver for regulators and suddenly find that nothing registered regulators because they were on all the time anyway and everything breaks? What a surprise! I agree, this is not a surprise at all. But like I pointed out this is bound to happen again and again. So how about we learn from it and add the functionality needed to prevent it from happening? The same thing is going to happen with simplefb for your device. If you later realize that you need a regulator to keep the panel going, you'll have to add code to your firmware to populate the corresponding properties, otherwise the regulator will end up unused and will be automatically disabled. At the same time you're going to break upstream for all users of your old firmware because it doesn't add that property yet. Sure. And what can you do about that? It's not like the original Snow firmware writes anything of use to the DT at all. So to run a development kernel you need a development firmware. If you add new features to the kernel that involve intefacing to the firmware you need to update the firmware as well. Once support for Snow platform is stable you can expect that users can use a stable release of firmware indefinitely. Feel free to take that up with the DT ABI stability committee. As pointed out you can also generate by hand a DT that exactly describes what the preinstalled firmware sets up - so long as the preinstalled firmware sets upa any usable framebuffer at all and always sets up the same thing. If the preinstalled firmware can set up the framebuffer in different ways and is not willing to communicate to the kernel in what way it set up the framebuffer then it is unusable with simplefb. That sucks but there is no way around that. Why not? Are you really expecting to keep running with simplefb forever? Nobody is going to seriously use an upstream kernel in any product with only simplefb as a framebuffer. I've said before that this is a hack to Why not? You can use shadowfb (software acceleration) with simplefb all right. Shadowfb is hands down the fastest video acceleration we have on anything but hardware that has _very_ slow CPU or that can run Intel UXA drivers. With manufacturers adding more and more superfluous cores to the CPUs shadowfb is actually not too stressing on the system, either. On hardware like Allwinner A13 (single core) the use of shadowfb over actual video acceleration hardware has its benefits and drawbacks and is neither clearly better nor worse. The same hardware tends to have only one fixed video output - a tablet LCD panel. On such hardware modesetting
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 03:54:00PM +0200, Thierry Reding wrote: On Mon, Sep 29, 2014 at 01:34:36PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 12:44:57PM +0200, Thierry Reding wrote: Plus, speaking more specifically about the clocks, that won't prevent your clock to be shut down as a side effect of a later clk_disable call from another driver. Furthermore isn't it a bug for a driver to call clk_disable() before a preceding clk_enable()? There are patches being worked on that will enable per-user clocks and as I understand it they will specifically disallow drivers to disable the hardware clock if other drivers are still keeping them on via their own referenc. Calling clk_disable() preceding clk_enable() is a bug. Calling clk_disable() after clk_enable() will disable the clock (and its parents) if the clock subsystem thinks there are no other users, which is what will happen here. Right. I'm not sure this is really applicable to this situation, though. It's actually very easy to do. Have a driver that probes, enables its clock, fails to probe for any reason, call clk_disable in its exit path. If there's no other user at that time of this particular clock tree, it will be shut down. Bam. You just lost your framebuffer. Really, it's just that simple, and relying on the fact that some other user of the same clock tree will always be their is beyond fragile. Perhaps the meaning clk_ignore_unused should be revised, then. What you describe isn't at all what I'd expect from such an option. And it does not match the description in Documentation/kernel-parameters.txt either. Well, it never says that it also prevent them from being disabled later on. But it's true it's not very explicit. Either way, if there are other users of a clock then they will just as likely want to modify the rate at which point simplefb will break just as badly. And this can be handled just as well. Register a clock notifier, refuse any rate change, done. But of course, that would require having a clock handle. Now, how would *you* prevent such a change? Like I said in the other thread. If you have two drivers that use the same clock but need different frequencies you've lost anyway. Except that the driver that has the most logic (ie not simplefb) will have a way to recover and deal with that. You prevented the clock rate *and* your system can react, I guess you actually won. But sure, you can still try to point new issues, get an obvious and robust solution, and then discard the issue when the solution doesn't go your way... Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote: On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote: This is a fundamental issue that we are facing and I'm trying to come up with a solution that is future-proof and will work for drivers other than simplefb. Just because we currently lack this functionality doesn't make it a hack trying to add it. Or maybe it's just showing that the trend to rely more and more on the firmware is a bad idea? I really start to consider adding a sunxi-uboot-fb, that would just duplicate the source code of simplefb but also taking the clocks. Somehow, I feel like it will be easier (and definitely less of a hack than using the generic common clock API). In the 2nd round of this discussion, i stated that another fb or even a drm driver altogether seemed to be the sensible way out of this mess. I suggest drm_rescue. Then instead of hacking existing drivers to work on your particular platform you should start looking into hacking your platform drivers to cope with the lack of a proper display driver. Or alternatively spend the time getting a proper display driver merged. Whether simplefb is used as primary framebuffer or just during only boot until hand-off to a real driver, it still remains a stop-gap solution. Then really, simplefb deserves a more appropriate name. Like uselessfb, tegrafb, DONOTUSEITYOUSTUPIDDEVfb or whatever makes it not look generic. Very early on, now almost two months back, i used the word denialfb. rpifb is the real name of this thing though, but then the dt binding names would have to change and whatnot. I don't get the resistance, at least not from a technical point of view. And i do not care enough to get properly involved in this pointless and endless discussion. drm_rescue it is. Luc Verhaegen. -- 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+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
