* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-26 8:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825151822.GC21020@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 796 bytes --]
On Mon, Aug 25, 2014 at 05:18:22PM +0200, Luc Verhaegen wrote:
> On Mon, Aug 25, 2014 at 05:12:58PM +0200, Thierry Reding wrote:
> >
> > Out of curiosity, how does this work in practice? How does the
> > bootloader create this entry? Does it scan the DT to see which clocks
> > the real hardware device references and then simply copies them to the
> > simplefb node?
> >
> > Thierry
>
> https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg06619.html
That looks like a royal pain. Again, I think it'd be much simpler (but
not less code, unfortunately) to do this on a per-resource basis. That
way these low-level firmware drivers in the kernel can stay trivial,
keeping the real complexity where they belong: in hardware-specific
drivers such as DRM/KMS.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-26 8:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825150705.GB15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 2793 bytes --]
On Mon, Aug 25, 2014 at 05:07:05PM +0200, Maxime Ripard wrote:
> On Mon, Aug 25, 2014 at 04:53:06PM +0200, Thierry Reding wrote:
> > Hmm... that's true. But we already have a way to deal with exactly this
> > situation for regulators. There's a property called regulator-boot-on
> > which a bootloader should set whet it has enabled a given regulator. It
> > can of course also be set statically in a DTS if it's know upfront that
> > a bootloader will always enable it. Perhaps what we need is a similar
> > property for clocks so that the clock framework will not inadvertently
> > turn off a clock that's still being used.
>
> Except that such a property won't work either.
Of course it won't work if it's designed not to work. The solution to
that is to design it in a way that it works and does exactly what we
want it to do.
> Regulators with regulator-boot-on will still be disabled if there's no
> one to claim it. Just like what happens currently for the clocks.
I was somewhat surprised by this, but it can indeed easily be verified.
It seems to me somewhat at odds with the definition of such a property.
Mark,
You've probably not read the whole backlog, but the discussion revolves
around hand-off of resources from firmware to kernel (via DT in this
case). If firmware initializes a device (display controller in this
particular case) and enables resources needed by the device to work
properly then in order to properly hand over resources from firmware to
kernel we need a way to communicate the state of these resources. For
regulators the regulator-boot-on property is specified to do exactly
that. However the implementation will automatically disable a regulator
marked boot-on if it hasn't been claimed by any driver after the
initcall stage completes.
I find that rather odd since I always assumed that a regulator marked
boot-on would not be touched by the core at all, assuming that firmware
set it up properly and that it would be required (even if not explicitly
claimed).
The issue that this causes is that we can't properly hand-off devices
initialized by firmware because the regulator will be disabled after the
initcall stage and then enabled when the driver loads. In case of
display the result will usually be flicker. The same applies to other
types of resources (in this case clocks).
Two categories of drivers have this issue: drivers built as modules (or
that defer probing) and therefore won't be probed until after initcalls
have run and generic low-level drivers that take over firmware devices
(simplefb in this case) that don't know anything about the resource that
the devices need.
Also Cc'ing Mike, perhaps he has ideas on how to solve this problem for
clocks specifically.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-26 8:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825152232.GE15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 4277 bytes --]
On Mon, Aug 25, 2014 at 05:22:32PM +0200, Maxime Ripard wrote:
> On Mon, Aug 25, 2014 at 05:05:04PM +0200, Thierry Reding wrote:
> > On Mon, Aug 25, 2014 at 04:58:54PM +0200, Maxime Ripard wrote:
> > > On Mon, Aug 25, 2014 at 04:16:29PM +0200, Thierry Reding wrote:
> > > > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> > > > > On 08/25/2014 03:39 PM, Thierry Reding wrote:
> > > > > > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> > > > > >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> > > > > >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > > > > >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> > > > > >>> [...]
> > > > > >>>>> If not, perhaps the clock driver should force the clock to be
> > > > > >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> > > > > >>>>
> > > > > >>>> I'm sorry, but I'm not going to take any code that will do that in our
> > > > > >>>> clock driver.
> > > > > >>>>
> > > > > >>>> I'm not going to have a huge list of ifdef depending on configuration
> > > > > >>>> options to know which clock to enable, especially when clk_get should
> > > > > >>>> have the consumer device as an argument.
> > > > > >>>
> > > > > >>> Are you saying is that you want to solve a platform-specific problem by
> > > > > >>> pushing code into simple, generic drivers so that your platform code can
> > > > > >>> stay "clean"?
> > > > > >>
> > > > > >> Are you saying that this driver would become "dirty" with such a patch?
> > > > > >
> > > > > > Yes. Others have said the same and even provided alternative solutions
> > > > > > on how to solve what's seemingly a platform-specific problem in a
> > > > > > platform-specific way.
> > > > >
> > > > > This is not platform specific, any platform with a complete clock driver
> > > > > will suffer from the same problem (the clock driver disabling unclaimed
> > > > > ahb gates, and thus killing the video output) if it wants to use simplefb
> > > > > for early console support.
> > > >
> > > > It is platform specific in that your platform may require certain clocks
> > > > to remain on.
> > >
> > > The platform doesn't. simplefb does. simplefb is the obvious consumer
> > > for these clocks, and given the current API and abstraction we have,
> > > it should be the one claiming the clocks too.
> >
> > No. simplefb just wants to write to some memory that hardware has been
> > set up to scan out. The platform requires that the clocks be on. Other
> > platforms may not even allow turning off the clocks.
>
> Like what? the rpi? Come on. Just because the videocore is some black
> box we know nothing about doesn't mean we should use it as an example.
You make it sound like the Raspberry Pi is somehow less important than
sunxi.
> Any decent enough SoC, with a decent support in the kernel will have
> clocks for this, and I really wonder how simplefb will behave once its
> clocks will be turned off...
There are other devices besides ARM SoCs that may want to use this
driver and that don't have clock support.
But you're missing my point. What I'm saying is that the simplefb driver
is meant to serve as a way to take over whatever framebuffer a firmware
set up. Therefore I think it makes the most sense to assume that nothing
needs to be controlled in any way since already been set up by firmware.
Eventually there should be a driver that takes over from simplefb that
knows how to properly handle the device's specifics, but that's not
simplefb.
The goal of this patch series is to keep clocks from being turned off.
But that's not what it does. What it does is turn clocks on to prevent
them from being turned off. In my opinion that's a workaround for a
deficiency in the kernel (and the firmware/kernel interface) and I think
it should be fixed at the root. So a much better solution would be to
establish a way for firmware to communicate to the kernel that a given
resource has been enabled by firmware and shouldn't be disabled. Such a
solution can be implement for all types of resources and can be reused
by all drivers since they don't have to worry about these details.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-26 7:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825150900.GB21020@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
On Mon, Aug 25, 2014 at 05:09:00PM +0200, Luc Verhaegen wrote:
> On Mon, Aug 25, 2014 at 05:05:04PM +0200, Thierry Reding wrote:
> >
> > No. simplefb just wants to write to some memory that hardware has been
> > set up to scan out. The platform requires that the clocks be on.
>
> Simplefb also requires that the memory is there and is persistent. Fine
> for discrete graphics cards, fine for rpi where most things are hidden
> from the ARM core anyway, not so fine for anybody else.
I don't understand. This patch series isn't changing anything about the
memory aspects of the driver yet it's working for you on sunxi, isn't
it? So it can't be all that broken.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v4 00/15] Rework OMAP4+ HDMI audio support
From: Jyri Sarha @ 2014-08-25 19:04 UTC (permalink / raw)
To: alsa-devel, linux-fbdev, linux-omap
Cc: peter.ujfalusi, broonie, liam.r.girdwood, tomi.valkeinen,
detheridge, Jyri Sarha
The patches are based on:
git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git for-next
The base, the patches, and couple of additional not-to-be-merged
omap2plus_defconfig patches can be found here:
git://git.ti.com/~jyrisarha/ti-linux-kernel/jyrisarhas-audio-video-linux-feature-tree.git omap-hdmi-audio
Changes since v3:
- Move ASoC parts into library module under sound/soc/omap
- Come up with API for the library
- Some cleaning up
The patch set fixes OMAP4+ HDMI audio. The structure of the
implementation looks a bit different than before. Instead of creating
a driver specific API for a separate ASoC component driver to connect
to, these patches implement a single audio library module under
sound/soc/omap for ASoC side implementation. The library exports
omap_hdmi_audio_register() and omap_hdmi_audio_unregister()
functions. With the functions OMAPDSS HDMI implementation registers
and unregisters all ASoC components needed for OMAP HDMI audio.
The library implements cpu-dai component using the callbacks provided
by OMAPDSS. Omap-pcm is registered for platform component, dummy
hdmi-audio-codec is registered for codec component, and
asoc-simple-card is registered for machine driver.
Big part of the HDMI audio code is still unchanged and there is a need
for a cleanup there. Also there is still probably something wrong with
speaker mapping of multi-channel streams. I will get back to cleaning
up these issues later.
Best regards,
Jyri
Jyri Sarha (15):
OMAPDSS: hdmi.h: Add HDMI_AUDIO_LAYOUT_6CH enum value
OMAPDSS: hdmi: Remove most of OMAP[45]_DSS_HDMI_AUDIO ifdefs
OMAPDSS: hdmi4_core: Remove unused hdmi4_audio_get_dma_port()
OMAPDSS: hdmi_wp: Add function for getting audio dma address
OMAPDSS: hdmi: Make hdmi structure public
OMAPDSS: hdmi: Add exported functions for storing HDMI audio data
OMAPDSS: hdmi: Make hdmi_mode_has_audio() more user friedly
ASoC: omap-hdmi-audio: Add OMAP HDMI audio support library
OMAPDSS: Kconfig: Implement options for OMAP4 and OMAP5 HDMI audio
support
OMAPDSS: hdmi4: Register HDMI audio with omap_hdmi_audio_register()
OMAPDSS: hdmi5: Register HDMI audio with omap_hdmi_audio_register()
ASoC: omap: Remove obsolete HDMI audio code and Kconfig options
OMAPDSS: hdmi4: Remove callbacks for an external ASoC DAI driver
OMAPDSS: hdmi5: Remove callbacks for an external ASoC DAI driver
OMAPDSS: Remove all references to obsolete HDMI audio callbacks
.../fbdev/omap2/displays-new/connector-hdmi.c | 99 ------
.../fbdev/omap2/displays-new/encoder-tpd12s015.c | 56 ---
drivers/video/fbdev/omap2/dss/Kconfig | 28 +-
drivers/video/fbdev/omap2/dss/hdmi.h | 35 +-
drivers/video/fbdev/omap2/dss/hdmi4.c | 233 ++++++-------
drivers/video/fbdev/omap2/dss/hdmi4_core.c | 14 -
drivers/video/fbdev/omap2/dss/hdmi4_core.h | 4 -
drivers/video/fbdev/omap2/dss/hdmi5.c | 216 +++++-------
drivers/video/fbdev/omap2/dss/hdmi5_core.c | 6 -
drivers/video/fbdev/omap2/dss/hdmi5_core.h | 2 -
drivers/video/fbdev/omap2/dss/hdmi_common.c | 18 +-
drivers/video/fbdev/omap2/dss/hdmi_wp.c | 8 +-
include/sound/omap-hdmi-audio.h | 50 +++
include/video/omapdss.h | 34 +-
sound/soc/omap/Kconfig | 15 +-
sound/soc/omap/Makefile | 6 +-
sound/soc/omap/omap-hdmi-audio.c | 357 +++++++++++++++++++
sound/soc/omap/omap-hdmi-card.c | 87 -----
sound/soc/omap/omap-hdmi.c | 364 --------------------
sound/soc/omap/omap-hdmi.h | 38 --
20 files changed, 676 insertions(+), 994 deletions(-)
create mode 100644 include/sound/omap-hdmi-audio.h
create mode 100644 sound/soc/omap/omap-hdmi-audio.c
delete mode 100644 sound/soc/omap/omap-hdmi-card.c
delete mode 100644 sound/soc/omap/omap-hdmi.c
delete mode 100644 sound/soc/omap/omap-hdmi.h
--
1.7.9.5
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-25 15:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825151255.GA14967@ulmo.nvidia.com>
On Mon, Aug 25, 2014 at 11:12 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Aug 25, 2014 at 04:27:04PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 08/25/2014 04:23 PM, jonsmirl@gmail.com wrote:
>> > On Mon, Aug 25, 2014 at 10:16 AM, Thierry Reding
>> > <thierry.reding@gmail.com> wrote:
>> >> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
>> >>> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>> >>>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>> >>>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>> >>>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>> >>>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>> >>>>>> [...]
>> >>>>>>>> If not, perhaps the clock driver should force the clock to be
>> >>>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>> >>>>>>>
>> >>>>>>> I'm sorry, but I'm not going to take any code that will do that in our
>> >>>>>>> clock driver.
>> >>>>>>>
>> >>>>>>> I'm not going to have a huge list of ifdef depending on configuration
>> >>>>>>> options to know which clock to enable, especially when clk_get should
>> >>>>>>> have the consumer device as an argument.
>> >>>>>>
>> >>>>>> Are you saying is that you want to solve a platform-specific problem by
>> >>>>>> pushing code into simple, generic drivers so that your platform code can
>> >>>>>> stay "clean"?
>> >>>>>
>> >>>>> Are you saying that this driver would become "dirty" with such a patch?
>> >>>>
>> >>>> Yes. Others have said the same and even provided alternative solutions
>> >>>> on how to solve what's seemingly a platform-specific problem in a
>> >>>> platform-specific way.
>> >>>
>> >>> This is not platform specific, any platform with a complete clock driver
>> >>> will suffer from the same problem (the clock driver disabling unclaimed
>> >>> ahb gates, and thus killing the video output) if it wants to use simplefb
>> >>> for early console support.
>> >>
>> >> It is platform specific in that your platform may require certain clocks
>> >> to remain on. The next platform may require power domains to remain on
>> >> during boot and yet another one may rely on regulators to stay on during
>> >> boot. By your argument simplefb will need to be taught to handle pretty
>> >> much every type of resource that the kernel has.
>> >
>> > Why can't simplefb be a driver library that is called from a device
>> > specific device driver that only claims the clocks (or regulators)?
>> > Then build all of these device specific drivers into the generic ARM
>> > kernel. They will be quite small since all they do is claim the clocks
>> > (or regulator). Maybe we can even figure out some protocol for
>> > removing the unused ones from memory later.
>> >
>> > Later during the boot process the device specific driver can load its
>> > KMS code which has also been implemented as a driver library. Maybe
>> > use E_PROBE_DEFER to do this. Match on the device ID, claim the
>> > clocks, defer until the full KMS library can be loaded.
>>
>> There is no need for all this complexity, all that is needed is for the
>> simplefb driver to be thought to claim + enable any clocks listed in
>> its dt node.
>
> Out of curiosity, how does this work in practice? How does the
> bootloader create this entry? Does it scan the DT to see which clocks
> the real hardware device references and then simply copies them to the
> simplefb node?
That's why this is such a problem. There is a general rule in Linux -
one device, one driver. All of the kernel device driver code is
written around this assumption. We're trying to make two drivers for
one device and the kernel is not designed to support that. There
shouldn't be an independent 'simplefb' node. That is creating two
device descriptions for a single device.
Adding 'chosen' information to the devices nodes might work.
video@1345 {
compatible = "sunxi-a20-video";
reg = <...>;
clocks = <...>;
chosen {
compatible = "simplefb"; // dynamically added by uboot
buffer = <0x4564>; // dynamically added by uboot
};
};
This will initialize the simplefb driver without attaching it to any
specific hardware. It can then look in its parent node for clocks and
regulators and claim them.
>
> Thierry
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Andreas Färber @ 2014-08-25 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FB3E7F.4000503@redhat.com>
Hi,
Am 25.08.2014 15:47, schrieb Hans de Goede:
> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>>>>>> If not, perhaps the clock driver should force the clock to be
>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
[...]
> As for the suggestion to simply never disable the plls / ahb gates by blocking
> them from ever being disabled in the sunxi clock driver, that is not really
> a solution either, as we want to be able to turn these things off to safe
> power on screen blank once control has been turned over to the kms driver.
Without wanting to take sides on the simplefb matter, can't you just use
clk_ignore_unused in bootargs to work around the issue at hand?
That's what I do on the Spring Chromebook.
Cheers,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-25 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825150501.GE14763@ulmo.nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 3587 bytes --]
On Mon, Aug 25, 2014 at 05:05:04PM +0200, Thierry Reding wrote:
> On Mon, Aug 25, 2014 at 04:58:54PM +0200, Maxime Ripard wrote:
> > On Mon, Aug 25, 2014 at 04:16:29PM +0200, Thierry Reding wrote:
> > > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> > > > On 08/25/2014 03:39 PM, Thierry Reding wrote:
> > > > > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> > > > >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> > > > >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > > > >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> > > > >>> [...]
> > > > >>>>> If not, perhaps the clock driver should force the clock to be
> > > > >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> > > > >>>>
> > > > >>>> I'm sorry, but I'm not going to take any code that will do that in our
> > > > >>>> clock driver.
> > > > >>>>
> > > > >>>> I'm not going to have a huge list of ifdef depending on configuration
> > > > >>>> options to know which clock to enable, especially when clk_get should
> > > > >>>> have the consumer device as an argument.
> > > > >>>
> > > > >>> Are you saying is that you want to solve a platform-specific problem by
> > > > >>> pushing code into simple, generic drivers so that your platform code can
> > > > >>> stay "clean"?
> > > > >>
> > > > >> Are you saying that this driver would become "dirty" with such a patch?
> > > > >
> > > > > Yes. Others have said the same and even provided alternative solutions
> > > > > on how to solve what's seemingly a platform-specific problem in a
> > > > > platform-specific way.
> > > >
> > > > This is not platform specific, any platform with a complete clock driver
> > > > will suffer from the same problem (the clock driver disabling unclaimed
> > > > ahb gates, and thus killing the video output) if it wants to use simplefb
> > > > for early console support.
> > >
> > > It is platform specific in that your platform may require certain clocks
> > > to remain on.
> >
> > The platform doesn't. simplefb does. simplefb is the obvious consumer
> > for these clocks, and given the current API and abstraction we have,
> > it should be the one claiming the clocks too.
>
> No. simplefb just wants to write to some memory that hardware has been
> set up to scan out. The platform requires that the clocks be on. Other
> platforms may not even allow turning off the clocks.
Like what? the rpi? Come on. Just because the videocore is some black
box we know nothing about doesn't mean we should use it as an example.
Any decent enough SoC, with a decent support in the kernel will have
clocks for this, and I really wonder how simplefb will behave once its
clocks will be turned off...
> > > The next platform may require power domains to remain on during boot
> > > and yet another one may rely on regulators to stay on during
> > > boot. By your argument simplefb will need to be taught to handle
> > > pretty much every type of resource that the kernel has.
> >
> > And I wouldn't find anything wrong with that. We're already doing so
> > for any generic driver in the kernel (AHCI, EHCI comes to my mind
> > first, there's probably a lot of others). Why wouldn't we do as such
> > for this one?
>
> Yes, and we've had similar discussions in those subsystems too.
Similar discussion, with different outcomes it seems.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Luc Verhaegen @ 2014-08-25 15:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825151255.GA14967@ulmo.nvidia.com>
On Mon, Aug 25, 2014 at 05:12:58PM +0200, Thierry Reding wrote:
>
> Out of curiosity, how does this work in practice? How does the
> bootloader create this entry? Does it scan the DT to see which clocks
> the real hardware device references and then simply copies them to the
> simplefb node?
>
> Thierry
https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg06619.html
Luc Verhaegen.
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FB47B8.2090104@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]
On Mon, Aug 25, 2014 at 04:27:04PM +0200, Hans de Goede wrote:
> Hi,
>
> On 08/25/2014 04:23 PM, jonsmirl@gmail.com wrote:
> > On Mon, Aug 25, 2014 at 10:16 AM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> >> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> >>> On 08/25/2014 03:39 PM, Thierry Reding wrote:
> >>>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> >>>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> >>>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> >>>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> >>>>>> [...]
> >>>>>>>> If not, perhaps the clock driver should force the clock to be
> >>>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> >>>>>>>
> >>>>>>> I'm sorry, but I'm not going to take any code that will do that in our
> >>>>>>> clock driver.
> >>>>>>>
> >>>>>>> I'm not going to have a huge list of ifdef depending on configuration
> >>>>>>> options to know which clock to enable, especially when clk_get should
> >>>>>>> have the consumer device as an argument.
> >>>>>>
> >>>>>> Are you saying is that you want to solve a platform-specific problem by
> >>>>>> pushing code into simple, generic drivers so that your platform code can
> >>>>>> stay "clean"?
> >>>>>
> >>>>> Are you saying that this driver would become "dirty" with such a patch?
> >>>>
> >>>> Yes. Others have said the same and even provided alternative solutions
> >>>> on how to solve what's seemingly a platform-specific problem in a
> >>>> platform-specific way.
> >>>
> >>> This is not platform specific, any platform with a complete clock driver
> >>> will suffer from the same problem (the clock driver disabling unclaimed
> >>> ahb gates, and thus killing the video output) if it wants to use simplefb
> >>> for early console support.
> >>
> >> It is platform specific in that your platform may require certain clocks
> >> to remain on. The next platform may require power domains to remain on
> >> during boot and yet another one may rely on regulators to stay on during
> >> boot. By your argument simplefb will need to be taught to handle pretty
> >> much every type of resource that the kernel has.
> >
> > Why can't simplefb be a driver library that is called from a device
> > specific device driver that only claims the clocks (or regulators)?
> > Then build all of these device specific drivers into the generic ARM
> > kernel. They will be quite small since all they do is claim the clocks
> > (or regulator). Maybe we can even figure out some protocol for
> > removing the unused ones from memory later.
> >
> > Later during the boot process the device specific driver can load its
> > KMS code which has also been implemented as a driver library. Maybe
> > use E_PROBE_DEFER to do this. Match on the device ID, claim the
> > clocks, defer until the full KMS library can be loaded.
>
> There is no need for all this complexity, all that is needed is for the
> simplefb driver to be thought to claim + enable any clocks listed in
> its dt node.
Out of curiosity, how does this work in practice? How does the
bootloader create this entry? Does it scan the DT to see which clocks
the real hardware device references and then simply copies them to the
simplefb node?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Luc Verhaegen @ 2014-08-25 15:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825150501.GE14763@ulmo.nvidia.com>
On Mon, Aug 25, 2014 at 05:05:04PM +0200, Thierry Reding wrote:
>
> No. simplefb just wants to write to some memory that hardware has been
> set up to scan out. The platform requires that the clocks be on.
Simplefb also requires that the memory is there and is persistent. Fine
for discrete graphics cards, fine for rpi where most things are hidden
from the ARM core anyway, not so fine for anybody else.
Luc Verhaegen.
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-25 15:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825145303.GC14763@ulmo.nvidia.com>
On Mon, Aug 25, 2014 at 10:53 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Aug 25, 2014 at 04:23:59PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 08/25/2014 04:16 PM, Thierry Reding wrote:
>> > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
>> >> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>> >>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>> >>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>> >>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>> >>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>> >>>>> [...]
>> >>>>>>> If not, perhaps the clock driver should force the clock to be
>> >>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>> >>>>>>
>> >>>>>> I'm sorry, but I'm not going to take any code that will do that in our
>> >>>>>> clock driver.
>> >>>>>>
>> >>>>>> I'm not going to have a huge list of ifdef depending on configuration
>> >>>>>> options to know which clock to enable, especially when clk_get should
>> >>>>>> have the consumer device as an argument.
>> >>>>>
>> >>>>> Are you saying is that you want to solve a platform-specific problem by
>> >>>>> pushing code into simple, generic drivers so that your platform code can
>> >>>>> stay "clean"?
>> >>>>
>> >>>> Are you saying that this driver would become "dirty" with such a patch?
>> >>>
>> >>> Yes. Others have said the same and even provided alternative solutions
>> >>> on how to solve what's seemingly a platform-specific problem in a
>> >>> platform-specific way.
>> >>
>> >> This is not platform specific, any platform with a complete clock driver
>> >> will suffer from the same problem (the clock driver disabling unclaimed
>> >> ahb gates, and thus killing the video output) if it wants to use simplefb
>> >> for early console support.
>> >
>> > It is platform specific in that your platform may require certain clocks
>> > to remain on. The next platform may require power domains to remain on
>> > during boot and yet another one may rely on regulators to stay on during
>> > boot. By your argument simplefb will need to be taught to handle pretty
>> > much every type of resource that the kernel has.
>> >
>> >> As for the suggestion to simply never disable the plls / ahb gates by blocking
>> >> them from ever being disabled in the sunxi clock driver, that is not really
>> >> a solution either, as we want to be able to turn these things off to safe
>> >> power on screen blank once control has been turned over to the kms driver.
>> >
>> > Then perhaps part of the hand-off procedure between simplefb and DRM/KMS
>> > should involve marking PLLs or "gates" as properly managed.
>>
>> And by your earlier argument also power domains, regulators, etc. So now we need
>> to add code to each of the clock core, power-domain core, regulator core, etc. to
>> have them now about this initially unmanaged state thing you're introducing, as
>> well as modify all involved clock / regulator / etc. drivers to mark certain
>> resources as unmanaged.
>
> Hmm... that's true. But we already have a way to deal with exactly this
> situation for regulators. There's a property called regulator-boot-on
> which a bootloader should set whet it has enabled a given regulator. It
> can of course also be set statically in a DTS if it's know upfront that
> a bootloader will always enable it. Perhaps what we need is a similar
> property for clocks so that the clock framework will not inadvertently
> turn off a clock that's still being used.
There should probably be a generic 'boot-initialized;' property that
can be added to any DT device node. Then uboot can add that property
to the device node for anything it has turned on.
You could even use it to add more info 'boot-initialized = <"9600 8 N 1">;'
That passes the info in an OS agnostic manner.
>
> Thierry
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-25 15:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825145303.GC14763@ulmo.nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 867 bytes --]
On Mon, Aug 25, 2014 at 04:53:06PM +0200, Thierry Reding wrote:
> Hmm... that's true. But we already have a way to deal with exactly this
> situation for regulators. There's a property called regulator-boot-on
> which a bootloader should set whet it has enabled a given regulator. It
> can of course also be set statically in a DTS if it's know upfront that
> a bootloader will always enable it. Perhaps what we need is a similar
> property for clocks so that the clock framework will not inadvertently
> turn off a clock that's still being used.
Except that such a property won't work either. Regulators with
regulator-boot-on will still be disabled if there's no one to claim
it. Just like what happens currently for the clocks.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825145854.GA15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 2924 bytes --]
On Mon, Aug 25, 2014 at 04:58:54PM +0200, Maxime Ripard wrote:
> On Mon, Aug 25, 2014 at 04:16:29PM +0200, Thierry Reding wrote:
> > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> > > On 08/25/2014 03:39 PM, Thierry Reding wrote:
> > > > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> > > >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> > > >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > > >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> > > >>> [...]
> > > >>>>> If not, perhaps the clock driver should force the clock to be
> > > >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> > > >>>>
> > > >>>> I'm sorry, but I'm not going to take any code that will do that in our
> > > >>>> clock driver.
> > > >>>>
> > > >>>> I'm not going to have a huge list of ifdef depending on configuration
> > > >>>> options to know which clock to enable, especially when clk_get should
> > > >>>> have the consumer device as an argument.
> > > >>>
> > > >>> Are you saying is that you want to solve a platform-specific problem by
> > > >>> pushing code into simple, generic drivers so that your platform code can
> > > >>> stay "clean"?
> > > >>
> > > >> Are you saying that this driver would become "dirty" with such a patch?
> > > >
> > > > Yes. Others have said the same and even provided alternative solutions
> > > > on how to solve what's seemingly a platform-specific problem in a
> > > > platform-specific way.
> > >
> > > This is not platform specific, any platform with a complete clock driver
> > > will suffer from the same problem (the clock driver disabling unclaimed
> > > ahb gates, and thus killing the video output) if it wants to use simplefb
> > > for early console support.
> >
> > It is platform specific in that your platform may require certain clocks
> > to remain on.
>
> The platform doesn't. simplefb does. simplefb is the obvious consumer
> for these clocks, and given the current API and abstraction we have,
> it should be the one claiming the clocks too.
No. simplefb just wants to write to some memory that hardware has been
set up to scan out. The platform requires that the clocks be on. Other
platforms may not even allow turning off the clocks.
> > The next platform may require power domains to remain on during boot
> > and yet another one may rely on regulators to stay on during
> > boot. By your argument simplefb will need to be taught to handle
> > pretty much every type of resource that the kernel has.
>
> And I wouldn't find anything wrong with that. We're already doing so
> for any generic driver in the kernel (AHCI, EHCI comes to my mind
> first, there's probably a lot of others). Why wouldn't we do as such
> for this one?
Yes, and we've had similar discussions in those subsystems too.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4OyjJ+QcBGOzrUDH9J+DxBEKLmXeOcnUyOHHHV_DtCG0zA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3567 bytes --]
On Mon, Aug 25, 2014 at 10:23:26AM -0400, jonsmirl@gmail.com wrote:
> On Mon, Aug 25, 2014 at 10:16 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> >> On 08/25/2014 03:39 PM, Thierry Reding wrote:
> >> > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> >> >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> >> >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> >> >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> >> >>> [...]
> >> >>>>> If not, perhaps the clock driver should force the clock to be
> >> >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> >> >>>>
> >> >>>> I'm sorry, but I'm not going to take any code that will do that in our
> >> >>>> clock driver.
> >> >>>>
> >> >>>> I'm not going to have a huge list of ifdef depending on configuration
> >> >>>> options to know which clock to enable, especially when clk_get should
> >> >>>> have the consumer device as an argument.
> >> >>>
> >> >>> Are you saying is that you want to solve a platform-specific problem by
> >> >>> pushing code into simple, generic drivers so that your platform code can
> >> >>> stay "clean"?
> >> >>
> >> >> Are you saying that this driver would become "dirty" with such a patch?
> >> >
> >> > Yes. Others have said the same and even provided alternative solutions
> >> > on how to solve what's seemingly a platform-specific problem in a
> >> > platform-specific way.
> >>
> >> This is not platform specific, any platform with a complete clock driver
> >> will suffer from the same problem (the clock driver disabling unclaimed
> >> ahb gates, and thus killing the video output) if it wants to use simplefb
> >> for early console support.
> >
> > It is platform specific in that your platform may require certain clocks
> > to remain on. The next platform may require power domains to remain on
> > during boot and yet another one may rely on regulators to stay on during
> > boot. By your argument simplefb will need to be taught to handle pretty
> > much every type of resource that the kernel has.
>
> Why can't simplefb be a driver library that is called from a device
> specific device driver that only claims the clocks (or regulators)?
> Then build all of these device specific drivers into the generic ARM
> kernel. They will be quite small since all they do is claim the clocks
> (or regulator). Maybe we can even figure out some protocol for
> removing the unused ones from memory later.
>
> Later during the boot process the device specific driver can load its
> KMS code which has also been implemented as a driver library. Maybe
> use E_PROBE_DEFER to do this. Match on the device ID, claim the
> clocks, defer until the full KMS library can be loaded.
That sounds like the most scalable solution so far. On the other hand,
as I understand it, the simplefb driver was designed to take over the
framebuffer set up by firmware, so it's somewhat odd that the driver
would have to deal with resources in the first place. If we push the
resource problem into the respective subsystems we keep the simplefb
driver completely hardware agnostic.
And we'll also be solving this problem for other types of drivers at the
same time. Firmware may after all initialize clocks and other resources
for other types of devices too. Handling resources in the drivers would
therefore imply that every driver needs to cope with this.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-25 14:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825141600.GA14763@ulmo.nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]
On Mon, Aug 25, 2014 at 04:16:29PM +0200, Thierry Reding wrote:
> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> > On 08/25/2014 03:39 PM, Thierry Reding wrote:
> > > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> > >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> > >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> > >>> [...]
> > >>>>> If not, perhaps the clock driver should force the clock to be
> > >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> > >>>>
> > >>>> I'm sorry, but I'm not going to take any code that will do that in our
> > >>>> clock driver.
> > >>>>
> > >>>> I'm not going to have a huge list of ifdef depending on configuration
> > >>>> options to know which clock to enable, especially when clk_get should
> > >>>> have the consumer device as an argument.
> > >>>
> > >>> Are you saying is that you want to solve a platform-specific problem by
> > >>> pushing code into simple, generic drivers so that your platform code can
> > >>> stay "clean"?
> > >>
> > >> Are you saying that this driver would become "dirty" with such a patch?
> > >
> > > Yes. Others have said the same and even provided alternative solutions
> > > on how to solve what's seemingly a platform-specific problem in a
> > > platform-specific way.
> >
> > This is not platform specific, any platform with a complete clock driver
> > will suffer from the same problem (the clock driver disabling unclaimed
> > ahb gates, and thus killing the video output) if it wants to use simplefb
> > for early console support.
>
> It is platform specific in that your platform may require certain clocks
> to remain on.
The platform doesn't. simplefb does. simplefb is the obvious consumer
for these clocks, and given the current API and abstraction we have,
it should be the one claiming the clocks too.
> The next platform may require power domains to remain on during boot
> and yet another one may rely on regulators to stay on during
> boot. By your argument simplefb will need to be taught to handle
> pretty much every type of resource that the kernel has.
And I wouldn't find anything wrong with that. We're already doing so
for any generic driver in the kernel (AHCI, EHCI comes to my mind
first, there's probably a lot of others). Why wouldn't we do as such
for this one?
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FB46FF.1010208@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3479 bytes --]
On Mon, Aug 25, 2014 at 04:23:59PM +0200, Hans de Goede wrote:
> Hi,
>
> On 08/25/2014 04:16 PM, Thierry Reding wrote:
> > On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> >> On 08/25/2014 03:39 PM, Thierry Reding wrote:
> >>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> >>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> >>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> >>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> >>>>> [...]
> >>>>>>> If not, perhaps the clock driver should force the clock to be
> >>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> >>>>>>
> >>>>>> I'm sorry, but I'm not going to take any code that will do that in our
> >>>>>> clock driver.
> >>>>>>
> >>>>>> I'm not going to have a huge list of ifdef depending on configuration
> >>>>>> options to know which clock to enable, especially when clk_get should
> >>>>>> have the consumer device as an argument.
> >>>>>
> >>>>> Are you saying is that you want to solve a platform-specific problem by
> >>>>> pushing code into simple, generic drivers so that your platform code can
> >>>>> stay "clean"?
> >>>>
> >>>> Are you saying that this driver would become "dirty" with such a patch?
> >>>
> >>> Yes. Others have said the same and even provided alternative solutions
> >>> on how to solve what's seemingly a platform-specific problem in a
> >>> platform-specific way.
> >>
> >> This is not platform specific, any platform with a complete clock driver
> >> will suffer from the same problem (the clock driver disabling unclaimed
> >> ahb gates, and thus killing the video output) if it wants to use simplefb
> >> for early console support.
> >
> > It is platform specific in that your platform may require certain clocks
> > to remain on. The next platform may require power domains to remain on
> > during boot and yet another one may rely on regulators to stay on during
> > boot. By your argument simplefb will need to be taught to handle pretty
> > much every type of resource that the kernel has.
> >
> >> As for the suggestion to simply never disable the plls / ahb gates by blocking
> >> them from ever being disabled in the sunxi clock driver, that is not really
> >> a solution either, as we want to be able to turn these things off to safe
> >> power on screen blank once control has been turned over to the kms driver.
> >
> > Then perhaps part of the hand-off procedure between simplefb and DRM/KMS
> > should involve marking PLLs or "gates" as properly managed.
>
> And by your earlier argument also power domains, regulators, etc. So now we need
> to add code to each of the clock core, power-domain core, regulator core, etc. to
> have them now about this initially unmanaged state thing you're introducing, as
> well as modify all involved clock / regulator / etc. drivers to mark certain
> resources as unmanaged.
Hmm... that's true. But we already have a way to deal with exactly this
situation for regulators. There's a property called regulator-boot-on
which a bootloader should set whet it has enabled a given regulator. It
can of course also be set statically in a DTS if it's know upfront that
a bootloader will always enable it. Perhaps what we need is a similar
property for clocks so that the clock framework will not inadvertently
turn off a clock that's still being used.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-25 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4OyjJ+QcBGOzrUDH9J+DxBEKLmXeOcnUyOHHHV_DtCG0zA@mail.gmail.com>
Hi,
On 08/25/2014 04:23 PM, jonsmirl@gmail.com wrote:
> On Mon, Aug 25, 2014 at 10:16 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
>>> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>>>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>>>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>>>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>>>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>>>>>> [...]
>>>>>>>> If not, perhaps the clock driver should force the clock to be
>>>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>>>>>>>
>>>>>>> I'm sorry, but I'm not going to take any code that will do that in our
>>>>>>> clock driver.
>>>>>>>
>>>>>>> I'm not going to have a huge list of ifdef depending on configuration
>>>>>>> options to know which clock to enable, especially when clk_get should
>>>>>>> have the consumer device as an argument.
>>>>>>
>>>>>> Are you saying is that you want to solve a platform-specific problem by
>>>>>> pushing code into simple, generic drivers so that your platform code can
>>>>>> stay "clean"?
>>>>>
>>>>> Are you saying that this driver would become "dirty" with such a patch?
>>>>
>>>> Yes. Others have said the same and even provided alternative solutions
>>>> on how to solve what's seemingly a platform-specific problem in a
>>>> platform-specific way.
>>>
>>> This is not platform specific, any platform with a complete clock driver
>>> will suffer from the same problem (the clock driver disabling unclaimed
>>> ahb gates, and thus killing the video output) if it wants to use simplefb
>>> for early console support.
>>
>> It is platform specific in that your platform may require certain clocks
>> to remain on. The next platform may require power domains to remain on
>> during boot and yet another one may rely on regulators to stay on during
>> boot. By your argument simplefb will need to be taught to handle pretty
>> much every type of resource that the kernel has.
>
> Why can't simplefb be a driver library that is called from a device
> specific device driver that only claims the clocks (or regulators)?
> Then build all of these device specific drivers into the generic ARM
> kernel. They will be quite small since all they do is claim the clocks
> (or regulator). Maybe we can even figure out some protocol for
> removing the unused ones from memory later.
>
> Later during the boot process the device specific driver can load its
> KMS code which has also been implemented as a driver library. Maybe
> use E_PROBE_DEFER to do this. Match on the device ID, claim the
> clocks, defer until the full KMS library can be loaded.
There is no need for all this complexity, all that is needed is for the
simplefb driver to be thought to claim + enable any clocks listed in
its dt node.
Then once we want to do a handover, all is needed is a single simplefb
unregister call, at which point simplefb will disable the clocks and
release them. Note that this will be a nop as they should already be
claimed and enabled by the kms driver at this time.
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-25 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825141600.GA14763@ulmo.nvidia.com>
Hi,
On 08/25/2014 04:16 PM, Thierry Reding wrote:
> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
>> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>>> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>>>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>>>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>>>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>>>>> [...]
>>>>>>> If not, perhaps the clock driver should force the clock to be
>>>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>>>>>>
>>>>>> I'm sorry, but I'm not going to take any code that will do that in our
>>>>>> clock driver.
>>>>>>
>>>>>> I'm not going to have a huge list of ifdef depending on configuration
>>>>>> options to know which clock to enable, especially when clk_get should
>>>>>> have the consumer device as an argument.
>>>>>
>>>>> Are you saying is that you want to solve a platform-specific problem by
>>>>> pushing code into simple, generic drivers so that your platform code can
>>>>> stay "clean"?
>>>>
>>>> Are you saying that this driver would become "dirty" with such a patch?
>>>
>>> Yes. Others have said the same and even provided alternative solutions
>>> on how to solve what's seemingly a platform-specific problem in a
>>> platform-specific way.
>>
>> This is not platform specific, any platform with a complete clock driver
>> will suffer from the same problem (the clock driver disabling unclaimed
>> ahb gates, and thus killing the video output) if it wants to use simplefb
>> for early console support.
>
> It is platform specific in that your platform may require certain clocks
> to remain on. The next platform may require power domains to remain on
> during boot and yet another one may rely on regulators to stay on during
> boot. By your argument simplefb will need to be taught to handle pretty
> much every type of resource that the kernel has.
>
>> As for the suggestion to simply never disable the plls / ahb gates by blocking
>> them from ever being disabled in the sunxi clock driver, that is not really
>> a solution either, as we want to be able to turn these things off to safe
>> power on screen blank once control has been turned over to the kms driver.
>
> Then perhaps part of the hand-off procedure between simplefb and DRM/KMS
> should involve marking PLLs or "gates" as properly managed.
And by your earlier argument also power domains, regulators, etc. So now we need
to add code to each of the clock core, power-domain core, regulator core, etc. to
have them now about this initially unmanaged state thing you're introducing, as
well as modify all involved clock / regulator / etc. drivers to mark certain
resources as unmanaged.
Or we add a single simple and clean patch to the simplefb driver for dealing
with clocks, and worry about all the other hypothetical problems later...
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-08-25 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825141600.GA14763@ulmo.nvidia.com>
On Mon, Aug 25, 2014 at 10:16 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
>> On 08/25/2014 03:39 PM, Thierry Reding wrote:
>> > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>> >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>> >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>> >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>> >>> [...]
>> >>>>> If not, perhaps the clock driver should force the clock to be
>> >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>> >>>>
>> >>>> I'm sorry, but I'm not going to take any code that will do that in our
>> >>>> clock driver.
>> >>>>
>> >>>> I'm not going to have a huge list of ifdef depending on configuration
>> >>>> options to know which clock to enable, especially when clk_get should
>> >>>> have the consumer device as an argument.
>> >>>
>> >>> Are you saying is that you want to solve a platform-specific problem by
>> >>> pushing code into simple, generic drivers so that your platform code can
>> >>> stay "clean"?
>> >>
>> >> Are you saying that this driver would become "dirty" with such a patch?
>> >
>> > Yes. Others have said the same and even provided alternative solutions
>> > on how to solve what's seemingly a platform-specific problem in a
>> > platform-specific way.
>>
>> This is not platform specific, any platform with a complete clock driver
>> will suffer from the same problem (the clock driver disabling unclaimed
>> ahb gates, and thus killing the video output) if it wants to use simplefb
>> for early console support.
>
> It is platform specific in that your platform may require certain clocks
> to remain on. The next platform may require power domains to remain on
> during boot and yet another one may rely on regulators to stay on during
> boot. By your argument simplefb will need to be taught to handle pretty
> much every type of resource that the kernel has.
Why can't simplefb be a driver library that is called from a device
specific device driver that only claims the clocks (or regulators)?
Then build all of these device specific drivers into the generic ARM
kernel. They will be quite small since all they do is claim the clocks
(or regulator). Maybe we can even figure out some protocol for
removing the unused ones from memory later.
Later during the boot process the device specific driver can load its
KMS code which has also been implemented as a driver library. Maybe
use E_PROBE_DEFER to do this. Match on the device ID, claim the
clocks, defer until the full KMS library can be loaded.
>
>> As for the suggestion to simply never disable the plls / ahb gates by blocking
>> them from ever being disabled in the sunxi clock driver, that is not really
>> a solution either, as we want to be able to turn these things off to safe
>> power on screen blank once control has been turned over to the kms driver.
>
> Then perhaps part of the hand-off procedure between simplefb and DRM/KMS
> should involve marking PLLs or "gates" as properly managed.
>
>> And while at it let me also tackle the don't use simplefb only use kms argument,
>> that means that the clocks will be turned off until the kms module loads, which
>> will cause noticable screen flicker / video output resync, something which we've
>> been trying to get rid of for years now.
>>
>> And no, build in the kms driver is not an answer either. That works nicely for
>> firmware, but not for generic Linux distributions supporting a wide range
>> of boards.
>
> Odd... I didn't offer any of those two as solutions to the problem.
>
> Thierry
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 14:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FB3E7F.4000503@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]
On Mon, Aug 25, 2014 at 03:47:43PM +0200, Hans de Goede wrote:
> On 08/25/2014 03:39 PM, Thierry Reding wrote:
> > On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> >> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> >>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> >>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> >>> [...]
> >>>>> If not, perhaps the clock driver should force the clock to be
> >>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> >>>>
> >>>> I'm sorry, but I'm not going to take any code that will do that in our
> >>>> clock driver.
> >>>>
> >>>> I'm not going to have a huge list of ifdef depending on configuration
> >>>> options to know which clock to enable, especially when clk_get should
> >>>> have the consumer device as an argument.
> >>>
> >>> Are you saying is that you want to solve a platform-specific problem by
> >>> pushing code into simple, generic drivers so that your platform code can
> >>> stay "clean"?
> >>
> >> Are you saying that this driver would become "dirty" with such a patch?
> >
> > Yes. Others have said the same and even provided alternative solutions
> > on how to solve what's seemingly a platform-specific problem in a
> > platform-specific way.
>
> This is not platform specific, any platform with a complete clock driver
> will suffer from the same problem (the clock driver disabling unclaimed
> ahb gates, and thus killing the video output) if it wants to use simplefb
> for early console support.
It is platform specific in that your platform may require certain clocks
to remain on. The next platform may require power domains to remain on
during boot and yet another one may rely on regulators to stay on during
boot. By your argument simplefb will need to be taught to handle pretty
much every type of resource that the kernel has.
> As for the suggestion to simply never disable the plls / ahb gates by blocking
> them from ever being disabled in the sunxi clock driver, that is not really
> a solution either, as we want to be able to turn these things off to safe
> power on screen blank once control has been turned over to the kms driver.
Then perhaps part of the hand-off procedure between simplefb and DRM/KMS
should involve marking PLLs or "gates" as properly managed.
> And while at it let me also tackle the don't use simplefb only use kms argument,
> that means that the clocks will be turned off until the kms module loads, which
> will cause noticable screen flicker / video output resync, something which we've
> been trying to get rid of for years now.
>
> And no, build in the kms driver is not an answer either. That works nicely for
> firmware, but not for generic Linux distributions supporting a wide range
> of boards.
Odd... I didn't offer any of those two as solutions to the problem.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-25 13:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825133953.GJ4163@ulmo.nvidia.com>
Hi,
On 08/25/2014 03:39 PM, Thierry Reding wrote:
> On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
>> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
>>> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
>>>> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
>>> [...]
>>>>> If not, perhaps the clock driver should force the clock to be
>>>>> enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>>>>
>>>> I'm sorry, but I'm not going to take any code that will do that in our
>>>> clock driver.
>>>>
>>>> I'm not going to have a huge list of ifdef depending on configuration
>>>> options to know which clock to enable, especially when clk_get should
>>>> have the consumer device as an argument.
>>>
>>> Are you saying is that you want to solve a platform-specific problem by
>>> pushing code into simple, generic drivers so that your platform code can
>>> stay "clean"?
>>
>> Are you saying that this driver would become "dirty" with such a patch?
>
> Yes. Others have said the same and even provided alternative solutions
> on how to solve what's seemingly a platform-specific problem in a
> platform-specific way.
This is not platform specific, any platform with a complete clock driver
will suffer from the same problem (the clock driver disabling unclaimed
ahb gates, and thus killing the video output) if it wants to use simplefb
for early console support.
I can only assume that this problem was never hit on tegra because when
kms support (and thus also a clock driver for the video plls) was introduced
simplefb support was dropped at the same time, or the gates are not being
disabled for some other reason.
As for the suggestion to simply never disable the plls / ahb gates by blocking
them from ever being disabled in the sunxi clock driver, that is not really
a solution either, as we want to be able to turn these things off to safe
power on screen blank once control has been turned over to the kms driver.
And while at it let me also tackle the don't use simplefb only use kms argument,
that means that the clocks will be turned off until the kms module loads, which
will cause noticable screen flicker / video output resync, something which we've
been trying to get rid of for years now.
And no, build in the kms driver is not an answer either. That works nicely for
firmware, but not for generic Linux distributions supporting a wide range
of boards.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 13:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825124410.GZ15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]
On Mon, Aug 25, 2014 at 02:44:10PM +0200, Maxime Ripard wrote:
> On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> > On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > > On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> > [...]
> > > > If not, perhaps the clock driver should force the clock to be
> > > > enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> > >
> > > I'm sorry, but I'm not going to take any code that will do that in our
> > > clock driver.
> > >
> > > I'm not going to have a huge list of ifdef depending on configuration
> > > options to know which clock to enable, especially when clk_get should
> > > have the consumer device as an argument.
> >
> > Are you saying is that you want to solve a platform-specific problem by
> > pushing code into simple, generic drivers so that your platform code can
> > stay "clean"?
>
> Are you saying that this driver would become "dirty" with such a patch?
Yes. Others have said the same and even provided alternative solutions
on how to solve what's seemingly a platform-specific problem in a
platform-specific way.
> If so, we really have an issue in the kernel.
Can you elaborate?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-25 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140825121228.GB4163@ulmo.nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]
On Mon, Aug 25, 2014 at 02:12:30PM +0200, Thierry Reding wrote:
> On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> > On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
> [...]
> > > If not, perhaps the clock driver should force the clock to be
> > > enabled (perhaps only if the DRM/KMS driver isn't enabled?).
> >
> > I'm sorry, but I'm not going to take any code that will do that in our
> > clock driver.
> >
> > I'm not going to have a huge list of ifdef depending on configuration
> > options to know which clock to enable, especially when clk_get should
> > have the consumer device as an argument.
>
> Are you saying is that you want to solve a platform-specific problem by
> pushing code into simple, generic drivers so that your platform code can
> stay "clean"?
Are you saying that this driver would become "dirty" with such a patch?
If so, we really have an issue in the kernel.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140813170106.GT15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]
On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
[...]
> > If not, perhaps the clock driver should force the clock to be
> > enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>
> I'm sorry, but I'm not going to take any code that will do that in our
> clock driver.
>
> I'm not going to have a huge list of ifdef depending on configuration
> options to know which clock to enable, especially when clk_get should
> have the consumer device as an argument.
Are you saying is that you want to solve a platform-specific problem by
pushing code into simple, generic drivers so that your platform code can
stay "clean"?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox