* Re: [PATCH] video: da8xx-fb: preserve display width when changing HSYNC
From: Tomi Valkeinen @ 2014-08-26 12:37 UTC (permalink / raw)
To: Ian Abbott, linux-fbdev; +Cc: Jean-Christophe Plagniol-Villard, linux-kernel
In-Reply-To: <1404395199-5726-1-git-send-email-abbotti@mev.co.uk>
[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]
On 03/07/14 16:46, Ian Abbott wrote:
> When looking at this driver for a client, I noticed the code that
> configures the HSYNC pulse clobbers the display width in the same
> register. It only preserves the MS part of the width in bit 3 and zeros
> the LS part of the width in bits 9 to 4. This doesn't matter during
> initialization as the width is configured afterwards, but subsequent use
> of the FBIPUT_HSYNC ioctl would clobber the width.
>
> Preserve bits 9 to 0 of LCD_RASTER_TIMING_0_REG when configuring the
> horizontal sync.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> I haven't tested this change, but it's pretty trivial.
> ---
> drivers/video/fbdev/da8xx-fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
> index 788f6b3..10c876c 100644
> --- a/drivers/video/fbdev/da8xx-fb.c
> +++ b/drivers/video/fbdev/da8xx-fb.c
> @@ -419,7 +419,7 @@ static void lcd_cfg_horizontal_sync(int back_porch, int pulse_width,
> {
> u32 reg;
>
> - reg = lcdc_read(LCD_RASTER_TIMING_0_REG) & 0xf;
> + reg = lcdc_read(LCD_RASTER_TIMING_0_REG) & 0x3ff;
> reg |= (((back_porch-1) & 0xff) << 24)
> | (((front_porch-1) & 0xff) << 16)
> | (((pulse_width-1) & 0x3f) << 10);
>
Thanks, queued for 3.17 fixes.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] video: vermilion: remove unnecessary break after goto
From: Tomi Valkeinen @ 2014-08-26 12:41 UTC (permalink / raw)
To: Fabian Frederick, linux-kernel
Cc: Jean-Christophe Plagniol-Villard, Jingoo Han, linux-fbdev
In-Reply-To: <1404836838-6953-1-git-send-email-fabf@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 869 bytes --]
On 08/07/14 19:27, Fabian Frederick wrote:
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> drivers/video/fbdev/vermilion/vermilion.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/vermilion/vermilion.c b/drivers/video/fbdev/vermilion/vermilion.c
> index 048a666..5f930ae 100644
> --- a/drivers/video/fbdev/vermilion/vermilion.c
> +++ b/drivers/video/fbdev/vermilion/vermilion.c
> @@ -481,7 +481,6 @@ static int vml_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> default:
> err = -ENODEV;
> goto out_err_1;
> - break;
> }
>
> info = &vinfo->info;
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 trivial 5/6] video: mx3fb: Update comment for dmaengine_prep_slave_sg() API
From: Tomi Valkeinen @ 2014-08-26 12:42 UTC (permalink / raw)
To: Geert Uytterhoeven, Vinod Koul, Dan Williams
Cc: dmaengine, linux-kernel, Jean-Christophe Plagniol-Villard,
Jiri Kosina, linux-fbdev
In-Reply-To: <1405095208-14642-5-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]
On 11/07/14 19:13, Geert Uytterhoeven wrote:
> Commit 16052827d98fbc13c31ebad560af4bd53e2b4dd5 ("dmaengine/dma_slave:
> introduce inline wrappers") changed the code to use the new API, but forgot
> to update a comment.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Jiri Kosina <trivial@kernel.org>
> Cc: linux-fbdev@vger.kernel.org
> --
> v2:
> - New
> ---
> drivers/video/fbdev/mx3fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/mx3fb.c b/drivers/video/fbdev/mx3fb.c
> index c645a0a0c341..5e97baf92721 100644
> --- a/drivers/video/fbdev/mx3fb.c
> +++ b/drivers/video/fbdev/mx3fb.c
> @@ -1179,7 +1179,7 @@ static int mx3fb_pan_display(struct fb_var_screeninfo *var,
>
> /*
> * We enable the End of Frame interrupt, which will free a tx-descriptor,
> - * which we will need for the next device_prep_slave_sg(). The
> + * which we will need for the next dmaengine_prep_slave_sg(). The
> * IRQ-handler will disable the IRQ again.
> */
> init_completion(&mx3_fbi->flip_cmpl);
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] msm: msm_fb: Move to using managed resources of kzalloc
From: Pramod Gurav @ 2014-08-26 12:49 UTC (permalink / raw)
To: linux-kernel, linux-fbdev
Cc: Pramod Gurav, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Stephen Boyd, Jingoo Han, Rob Clark
Move to managed verion of kzalloc. Also checks return for failure case
which was missing.
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
CC: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
CC: Stephen Boyd <sboyd@codeaurora.org>
CC: Jingoo Han <jg1.han@samsung.com>
CC: Rob Clark <robdclark@gmail.com>
---
drivers/video/fbdev/msm/msm_fb.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/msm/msm_fb.c b/drivers/video/fbdev/msm/msm_fb.c
index 4640188..2979d7e 100644
--- a/drivers/video/fbdev/msm/msm_fb.c
+++ b/drivers/video/fbdev/msm/msm_fb.c
@@ -569,8 +569,13 @@ static int msmfb_probe(struct platform_device *pdev)
mutex_init(&msmfb->panel_init_lock);
init_waitqueue_head(&msmfb->frame_wq);
INIT_WORK(&msmfb->resume_work, power_on_panel);
- msmfb->black = kzalloc(msmfb->fb->var.bits_per_pixel*msmfb->xres,
- GFP_KERNEL);
+ msmfb->black = devm_kzalloc(&pdev->dev,
+ msmfb->fb->var.bits_per_pixel*msmfb->xres,
+ GFP_KERNEL);
+ if (!msmfb->black) {
+ ret = -ENOMEM;
+ goto error_register_framebuffer;
+ }
printk(KERN_INFO "msmfb_probe() installing %d x %d panel\n",
msmfb->xres, msmfb->yres);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 1/2] msm: msm_fb: Add remove function for platform driver for clean unloading
From: Tomi Valkeinen @ 2014-08-26 12:52 UTC (permalink / raw)
To: Pramod Gurav, linux-kernel, linux-fbdev
Cc: Jean-Christophe Plagniol-Villard, Stephen Boyd, Jingoo Han,
Rob Clark
In-Reply-To: <1409055038-9204-1-git-send-email-pramod.gurav@smartplayin.com>
[-- Attachment #1: Type: text/plain, Size: 283 bytes --]
On 26/08/14 15:10, Pramod Gurav wrote:
> This adds a remove function to platform driver structure so that
> resources are released when driver is unloaded.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>
Thanks, queued this series for 3.18.
Tomi
[-- Attachment #2: OpenPGP 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: Maxime Ripard @ 2014-08-26 13:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FB555E.5040409@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]
Hi,
On Mon, Aug 25, 2014 at 05:25:18PM +0200, Andreas Färber wrote:
> 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.
Leaving *all* the clocks enabled just to workaround a driver failing
to grab its clocks doesn't look like a solution either.
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: Maxime Ripard @ 2014-08-26 13:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826084026.GF17263@ulmo>
[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]
On Tue, Aug 26, 2014 at 10:40:27AM +0200, Thierry Reding wrote:
> 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.
So we have to write a DRM/KMS driver in order to have display working
while waiting for a DRM/KMS driver to be worked on? Come on...
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-26 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826132207.GL15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]
On Tue, Aug 26, 2014 at 03:22:07PM +0200, Maxime Ripard wrote:
> On Tue, Aug 26, 2014 at 10:40:27AM +0200, Thierry Reding wrote:
> > 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.
>
> So we have to write a DRM/KMS driver in order to have display working
> while waiting for a DRM/KMS driver to be worked on? Come on...
Perhaps you should go and read this thread again. That's not at all what
I've been saying.
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-26 13:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826080432.GD17263@ulmo>
[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]
On Tue, Aug 26, 2014 at 10:04:33AM +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. 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.
No. What I mean is that it seems like we are somehow punished, or at
least blamed, for having a better and more complete kernel support.
> > 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.
And in this case, with this patch, simplefb will not claim any clock,
nor will fail probing.
> 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.
I guess such a hand over if it were to happen in the kernel would
involve the second driver claiming the resources before the first one
release them. How is that different in this case?
> 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.
Mike Turquette repeatedly said that he was against such a DT property:
https://lkml.org/lkml/2014/5/12/693
--
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-26 14:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826135341.GM15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 3850 bytes --]
On Tue, Aug 26, 2014 at 03:53:41PM +0200, Maxime Ripard wrote:
> On Tue, Aug 26, 2014 at 10:04:33AM +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. 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.
>
> No. What I mean is that it seems like we are somehow punished, or at
> least blamed, for having a better and more complete kernel support.
This isn't a competition. Nobody's punishing or blaming anyone. This is
about finding the best solution for the problem at hand.
> > > 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.
>
> And in this case, with this patch, simplefb will not claim any clock,
> nor will fail probing.
>
> > 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.
>
> I guess such a hand over if it were to happen in the kernel would
> involve the second driver claiming the resources before the first one
> release them. How is that different in this case?
It's different in that that driver will be hardware specific and know
exactly what clock and other resources are required. It will have a
device-specific binding.
> > 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.
>
> Mike Turquette repeatedly said that he was against such a DT property:
> https://lkml.org/lkml/2014/5/12/693
Mike says in that email that he's opposing the addition of a property
for clocks that is the equivalent of regulator-always-on. That's not
what this is about. If at all it'd be a property to mark a clock that
should not be disabled by default because it's essential.
Adding Mike on this subthread too.
Either way, Mark already suggested a different alternative in another
subthread, namely to add a new kind of checkpoint at which subsystems
can call a "disable unused" function that's called later than a late
initcall. This is not going to fix things for you immediately because
the clocks will still be switched off (only later) if you don't have
a real driver that's claiming the clocks. But you can work around that
for now by making the relevant clocks always on and remove that
workaround once a real driver is loaded that knows how to handle them
properly.
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 14:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826123357.GB22395@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]
On Tue, Aug 26, 2014 at 02:33:57PM +0200, Luc Verhaegen wrote:
> On Tue, Aug 26, 2014 at 02:21:16PM +0200, Thierry Reding wrote:
> > On Tue, Aug 26, 2014 at 02:02:58PM +0200, Luc Verhaegen wrote:
> > > On Tue, Aug 26, 2014 at 09:52:49AM +0200, Thierry Reding wrote:
> > > > 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
> > >
> > > Oh, i had to go wrestle with UBoot options and reserve 8MB off the top,
> > > which the kernel never gets told about. A nice throwback to x86 IGP
> > > bioses, a past i had thought i had left behind forgood.
> >
> > Can you not use the reserved memory code (drivers/of/of_reserved_mem.c
> > in the kernel)? I think that's the generally accepted way to do this
> > with DT.
> >
> > Thierry
>
> It was mentioned to me, and I probably could do that, but this seemed
> even more DT wrangling from within u-boot, and it didn't seem finished
> yet. Plus, all of this already was way more wrangling than i bargained
> for, especially with the promise of "simple"-fb. Then there is the
> really awkward way in which one gets to define clocks outside of a
> dts/compiler (you've seen the code, right?), i really did not feel like
> repeating something like that.
It could most probably be turned into fairly generic and reusable code
so that others can use it. And it would give you a way of recovering
those 8 MiB of memory that you'd otherwise loose.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv2 27/27] OMAPDSS: connector-analog-tv: Add DT support
From: Laurent Pinchart @ 2014-08-26 16:58 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1387205794-32246-28-git-send-email-tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Hi Tomi,
On Monday 16 December 2013 16:56:34 Tomi Valkeinen wrote:
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> .../video/omap2/displays-new/connector-analog-tv.c | 66 ++++++++++++++++++-
> 1 file changed, 65 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/omap2/displays-new/connector-analog-tv.c
> b/drivers/video/omap2/displays-new/connector-analog-tv.c index
> ccd9073f706f..ebed25a86487 100644
> --- a/drivers/video/omap2/displays-new/connector-analog-tv.c
> +++ b/drivers/video/omap2/displays-new/connector-analog-tv.c
> @@ -12,6 +12,7 @@
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/of.h>
>
> #include <video/omapdss.h>
> #include <video/omap-panel-data.h>
> @@ -42,6 +43,12 @@ static const struct omap_video_timings tvc_pal_timings > { .interlace = true,
> };
>
> +static const struct of_device_id tvc_of_match[];
> +
> +struct tvc_of_data {
> + enum omap_dss_venc_type connector_type;
> +};
> +
> #define to_panel_data(x) container_of(x, struct panel_drv_data, dssdev)
>
> static int tvc_connect(struct omap_dss_device *dssdev)
> @@ -92,7 +99,10 @@ static int tvc_enable(struct omap_dss_device *dssdev)
> in->ops.atv->set_timings(in, &ddata->timings);
>
> in->ops.atv->set_type(in, ddata->connector_type);
> - in->ops.atv->invert_vid_out_polarity(in, ddata->invert_polarity);
> +
> + if (!ddata->dev->of_node)
> + in->ops.atv->invert_vid_out_polarity(in,
> + ddata->invert_polarity);
>
> r = in->ops.atv->enable(in);
> if (r)
> @@ -205,6 +215,35 @@ static int tvc_probe_pdata(struct platform_device
> *pdev) return 0;
> }
>
> +static int tvc_probe_of(struct platform_device *pdev)
> +{
> + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct device_node *node = pdev->dev.of_node;
> + struct omap_dss_device *in;
> + const struct of_device_id *match;
> + const struct tvc_of_data *data;
> +
> + match = of_match_node(tvc_of_match, pdev->dev.of_node);
> + if (!match) {
> + dev_err(&pdev->dev, "unsupported device\n");
> + return -ENODEV;
> + }
> +
> + data = match->data;
> +
> + in = omapdss_of_find_source_for_first_ep(node);
> + if (IS_ERR(in)) {
> + dev_err(&pdev->dev, "failed to find video source\n");
> + return PTR_ERR(in);
> + }
> +
> + ddata->in = in;
> +
> + ddata->connector_type = data->connector_type;
> +
> + return 0;
> +}
> +
> static int tvc_probe(struct platform_device *pdev)
> {
> struct panel_drv_data *ddata;
> @@ -222,6 +261,10 @@ static int tvc_probe(struct platform_device *pdev)
> r = tvc_probe_pdata(pdev);
> if (r)
> return r;
> + } else if (pdev->dev.of_node) {
> + r = tvc_probe_of(pdev);
> + if (r)
> + return r;
> } else {
> return -ENODEV;
> }
> @@ -263,12 +306,33 @@ static int __exit tvc_remove(struct platform_device
> *pdev) return 0;
> }
>
> +static const struct tvc_of_data tv_svideo_data = {
> + .connector_type = OMAP_DSS_VENC_TYPE_SVIDEO,
> +};
> +
> +static const struct tvc_of_data tv_composite_video_data = {
> + .connector_type = OMAP_DSS_VENC_TYPE_COMPOSITE,
> +};
> +
> +static const struct of_device_id tvc_of_match[] = {
> + {
> + .compatible = "svideo-connector",
> + .data = &tv_svideo_data,
> + },
> + {
> + .compatible = "composite-video-connector",
I've just noticed that this doesn't match the bindings that document the
compatible value to be "composite-connector".
> + .data = &tv_composite_video_data,
> + },
> + {},
> +};
> +
> static struct platform_driver tvc_connector_driver = {
> .probe = tvc_probe,
> .remove = __exit_p(tvc_remove),
> .driver = {
> .name = "connector-analog-tv",
> .owner = THIS_MODULE,
> + .of_match_table = tvc_of_match,
> },
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Henrik Nordström @ 2014-08-26 18:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826105451.GD31124@ulmo>
tis 2014-08-26 klockan 12:55 +0200 skrev Thierry Reding:
> So the firmware does control the resources to a point where the display
> hardware is initialized and then it will happily forget about them. The
> device will keep scanning out the framebuffer and simplefb can update it
> with boot messages. To prevent the console from suddenly disappearing
> the kernel needs to make sure that the resources aren't inadvertently
> turned off at some more or less random point in time.
Which imho is until simplefb is unbound from the device.
Separately to this there needs to be a in-kernel handover procedure
where a new driver (i.e. video kms driver) can bind to a device and take
over resources, but it's completely unrelated to who owns the hardware
resources while simplefb is the active driver for the device.
It is not clear to me where the hardware resources should be listed in
DT, being it a simplefb node or part of the actual hardware device node
properly marked as dynamic boot defaults or something else? It's
somewhere inbetween hardware and virtual device, and somewhat volatile.
As far as simplefb is concerned it is a hardware desription of the
framebuffer, but for a kms driver it's no more than firmware handover of
boottime settings and ceases to exists once the kms driver have
reconfigured the hardware.
What I can do is to list some properties that these settings need to
provide:
1. Handover of hardware settings from bootloader to kernel.
2. Hardware description for simplefb use. It's at this level not really
any different from any other hardware.
3. Volatile and removed when kms driver have reconfigured the hardware
or it's been decided that the (virtual) device should not be active.
Regarding 3 and kexec as mentioned earlier. A new set of properties may
be added again for kexec usage to describe the current framebuffer for
the new kernel but op to the system which prepares for kexec to
determine. Should be no different in mechanism from bootloader to kernel
handover (the kexecing kernel is the bootloader in kexec case).
Regards
Henrik
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-26 19:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826143550.GB3027@ulmo>
Hi,
On 08/26/2014 04:35 PM, Thierry Reding wrote:
> On Tue, Aug 26, 2014 at 03:53:41PM +0200, Maxime Ripard wrote:
>> On Tue, Aug 26, 2014 at 10:04:33AM +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. 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.
>>
>> No. What I mean is that it seems like we are somehow punished, or at
>> least blamed, for having a better and more complete kernel support.
>
> This isn't a competition. Nobody's punishing or blaming anyone. This is
> about finding the best solution for the problem at hand.
>
>>>> 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.
>>
>> And in this case, with this patch, simplefb will not claim any clock,
>> nor will fail probing.
>>
>>> 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.
>>
>> I guess such a hand over if it were to happen in the kernel would
>> involve the second driver claiming the resources before the first one
>> release them. How is that different in this case?
>
> It's different in that that driver will be hardware specific and know
> exactly what clock and other resources are required. It will have a
> device-specific binding.
>
>>> 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.
>>
>> Mike Turquette repeatedly said that he was against such a DT property:
>> https://lkml.org/lkml/2014/5/12/693
>
> Mike says in that email that he's opposing the addition of a property
> for clocks that is the equivalent of regulator-always-on. That's not
> what this is about. If at all it'd be a property to mark a clock that
> should not be disabled by default because it's essential.
>
> Adding Mike on this subthread too.
>
> Either way, Mark already suggested a different alternative in another
> subthread, namely to add a new kind of checkpoint at which subsystems
> can call a "disable unused" function that's called later than a late
> initcall. This is not going to fix things for you immediately because
> the clocks will still be switched off (only later) if you don't have
> a real driver that's claiming the clocks. But you can work around that
> for now by making the relevant clocks always on and remove that
> workaround once a real driver is loaded that knows how to handle them
> properly.
This will simply not work, and is a ton more complicated then
simply teaching simplefb about a clocks property, which is a really simple
and clean patch.
First of all let me explain why this won't work. When should those
subsystems call this "later then a late initcall" call ? the kms driver
may very well be a kernel module, which will be loaded by userspace,
so we would need this to be triggered by userspace, but when ?
When the initrd is done? What then if the kms driver is not in the initrd,
so maybe when udev is done enumerating devices, but what then if the kernel
is still enumerating hardware at this time? Will we just put a sleep 30
in our initscripts to make sure the kernel is done scanning all busses,
will that be long enough? Maybe we need to re-introduce the scsi_wait_done
kernel module which was added as an ugly hack to allow userspace to wait
for the kernel to have finished scanning scsi (and ata / sata) busses,
for controllers found at the time the module was load. Which never worked
reliable, because it would be possible that the controller itself still
needed to be discovered. We've spend years cleaning up userspace enough
to be able to kill scsi_wait_done. We've already been down this road of
waiting for hw enumeration to be done, and the problem is that on
modern systems *it is never done*.
So second of all, Thierry, what exactly is the technical argument against
adding support for dealing with clocks to simplefb ? I've heard a lot of
people in favor of it, and only pretty much you against it, and your
argument so far seems to boil down to "I don't like it", which is not
really a technical sound argument IMHO. Moreover all the alternatives
seem to be horribly complicated and most of them also seem to have
several issues which will make them simply not work reliable.
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-26 21:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826143550.GB3027@ulmo>
[-- Attachment #1: Type: text/plain, Size: 4447 bytes --]
On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> > > > 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.
> >
> > And in this case, with this patch, simplefb will not claim any clock,
> > nor will fail probing.
> >
> > > 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.
> >
> > I guess such a hand over if it were to happen in the kernel would
> > involve the second driver claiming the resources before the first one
> > release them. How is that different in this case?
>
> It's different in that that driver will be hardware specific and know
> exactly what clock and other resources are required. It will have a
> device-specific binding.
Except that you made simplefb a generic driver. So we have two choices
here: either we don't anything but the trivial case, and no one with a
rather more advanced hardware will be able to use it, or we try to
grab any resource that might be of use, which means clocks,
regulators, reserved memory region, or whatever, so that we pretty
much cover all cases. It really is as simple as that.
> > > 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.
> >
> > Mike Turquette repeatedly said that he was against such a DT property:
> > https://lkml.org/lkml/2014/5/12/693
>
> Mike says in that email that he's opposing the addition of a property
> for clocks that is the equivalent of regulator-always-on. That's not
> what this is about. If at all it'd be a property to mark a clock that
> should not be disabled by default because it's essential.
It's just semantic. How is "a clock that should not be disabled by
default because it's essential" not a clock that stays always on?
Plus, you should read the mail further. It's clearly said that
consumer drivers should call clk_prepare_enable themselves, and that
the only exception to that is the case where we don't have such a
driver (and only valid as a temporary exception, which I guess you'll
soon turn into temporary-until-a-drm-kms-driver-is-merged).
> Adding Mike on this subthread too.
>
> Either way, Mark already suggested a different alternative in another
> subthread, namely to add a new kind of checkpoint at which subsystems
> can call a "disable unused" function that's called later than a late
> initcall. This is not going to fix things for you immediately because
> the clocks will still be switched off (only later) if you don't have
> a real driver that's claiming the clocks.
Great, I'm glad we found a solution for a completely unrelated issue.
> But you can work around that for now by making the relevant clocks
> always on and remove that workaround once a real driver is loaded
> that knows how to handle them properly.
So, let me get this straight. The clock provider driver should behave
as a clock consumer because it knows that in some cases, it might not
have any willingful enough consumer? Doesn't that even ring your
this-is-a-clear-abstraction-violation-bell just a tiny bit?
--
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-27 6:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826210248.GO15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 6360 bytes --]
On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
> On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> > > > > 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.
> > >
> > > And in this case, with this patch, simplefb will not claim any clock,
> > > nor will fail probing.
> > >
> > > > 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.
> > >
> > > I guess such a hand over if it were to happen in the kernel would
> > > involve the second driver claiming the resources before the first one
> > > release them. How is that different in this case?
> >
> > It's different in that that driver will be hardware specific and know
> > exactly what clock and other resources are required. It will have a
> > device-specific binding.
>
> Except that you made simplefb a generic driver. So we have two choices
> here: either we don't anything but the trivial case, and no one with a
> rather more advanced hardware will be able to use it, or we try to
> grab any resource that might be of use, which means clocks,
> regulators, reserved memory region, or whatever, so that we pretty
> much cover all cases. It really is as simple as that.
No, it should be even simpler. simplefb shouldn't have to know any of
this. It's just taking what firmware has set up and uses that. It's a
stop-gap solution to provide information on the display until a real
driver can be loaded and initializes the display hardware properly.
> > > > 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.
> > >
> > > Mike Turquette repeatedly said that he was against such a DT property:
> > > https://lkml.org/lkml/2014/5/12/693
> >
> > Mike says in that email that he's opposing the addition of a property
> > for clocks that is the equivalent of regulator-always-on. That's not
> > what this is about. If at all it'd be a property to mark a clock that
> > should not be disabled by default because it's essential.
>
> It's just semantic. How is "a clock that should not be disabled by
> default because it's essential" not a clock that stays always on?
Because a clock that should not be disabled by default can be turned off
when appropriate. A clock that is always on can't be turned off.
> Plus, you should read the mail further. It's clearly said that
> consumer drivers should call clk_prepare_enable themselves, and that
> the only exception to that is the case where we don't have such a
> driver (and only valid as a temporary exception, which I guess you'll
> soon turn into temporary-until-a-drm-kms-driver-is-merged).
Exactly. simplefb is only a temporary measure. It's not meant to be used
as a full-blown framebuffer driver. There are two use-cases where it is
an appropriate solution:
1) As a stop-gap solution for when the platform doesn't support a full
display driver yet. In that case you will want simplefb to stay
around forever.
2) To get early boot output before a full display driver can be loaded
in which case simplefb should go away after the display driver has
taken over.
In case of 2) the most straightforward solution is to not disable any
clocks until all drivers have had a chance to claim them. When the full
driver has been probed it should have claimed the clocks so they would
no longer get disabled.
For 1) I think it's fair to say that it's only a temporary solution to
get something on the screen. There won't be any kind of acceleration at
all, no power saving. Nothing but a dumb framebuffer. In that case it
should be a simple matter of keeping a select number of clocks always on
in the clock driver until support is more complete and it can be
properly handled. It's not like it's going to make a difference anyway
since simplefb won't ever disable them either.
> > Adding Mike on this subthread too.
> >
> > Either way, Mark already suggested a different alternative in another
> > subthread, namely to add a new kind of checkpoint at which subsystems
> > can call a "disable unused" function that's called later than a late
> > initcall. This is not going to fix things for you immediately because
> > the clocks will still be switched off (only later) if you don't have
> > a real driver that's claiming the clocks.
>
> Great, I'm glad we found a solution for a completely unrelated issue.
It's not at all unrelated. See above.
> > But you can work around that for now by making the relevant clocks
> > always on and remove that workaround once a real driver is loaded
> > that knows how to handle them properly.
>
> So, let me get this straight. The clock provider driver should behave
> as a clock consumer because it knows that in some cases, it might not
> have any willingful enough consumer? Doesn't that even ring your
> this-is-a-clear-abstraction-violation-bell just a tiny bit?
No. The clock driver should simply not allow the clocks to be disabled
if it isn't safe to do so.
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: Mark Brown @ 2014-08-27 7:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1409078409.2701.17.camel@localhost>
[-- Attachment #1: Type: text/plain, Size: 813 bytes --]
On Tue, Aug 26, 2014 at 08:40:09PM +0200, Henrik Nordström wrote:
> It is not clear to me where the hardware resources should be listed in
> DT, being it a simplefb node or part of the actual hardware device node
> properly marked as dynamic boot defaults or something else? It's
> somewhere inbetween hardware and virtual device, and somewhat volatile.
> As far as simplefb is concerned it is a hardware desription of the
> framebuffer, but for a kms driver it's no more than firmware handover of
> boottime settings and ceases to exists once the kms driver have
> reconfigured the hardware.
Is simplefb something that should be in the device tree distinctly in
the first place - shouldn't it be a subset of the functionality of the
video nodes? It's the same hardware being driven differently.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Hans de Goede @ 2014-08-27 8:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827065440.GG15640@ulmo>
Hi,
On 08/27/2014 08:54 AM, Thierry Reding wrote:
> On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
>> On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
>>>>>> 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.
>>>>
>>>> And in this case, with this patch, simplefb will not claim any clock,
>>>> nor will fail probing.
>>>>
>>>>> 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.
>>>>
>>>> I guess such a hand over if it were to happen in the kernel would
>>>> involve the second driver claiming the resources before the first one
>>>> release them. How is that different in this case?
>>>
>>> It's different in that that driver will be hardware specific and know
>>> exactly what clock and other resources are required. It will have a
>>> device-specific binding.
>>
>> Except that you made simplefb a generic driver. So we have two choices
>> here: either we don't anything but the trivial case, and no one with a
>> rather more advanced hardware will be able to use it, or we try to
>> grab any resource that might be of use, which means clocks,
>> regulators, reserved memory region, or whatever, so that we pretty
>> much cover all cases. It really is as simple as that.
>
> No, it should be even simpler. simplefb shouldn't have to know any of
> this. It's just taking what firmware has set up and uses that. It's a
> stop-gap solution to provide information on the display until a real
> driver can be loaded and initializes the display hardware properly.
>
>>>>> 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.
>>>>
>>>> Mike Turquette repeatedly said that he was against such a DT property:
>>>> https://lkml.org/lkml/2014/5/12/693
>>>
>>> Mike says in that email that he's opposing the addition of a property
>>> for clocks that is the equivalent of regulator-always-on. That's not
>>> what this is about. If at all it'd be a property to mark a clock that
>>> should not be disabled by default because it's essential.
>>
>> It's just semantic. How is "a clock that should not be disabled by
>> default because it's essential" not a clock that stays always on?
>
> Because a clock that should not be disabled by default can be turned off
> when appropriate. A clock that is always on can't be turned off.
>
>> Plus, you should read the mail further. It's clearly said that
>> consumer drivers should call clk_prepare_enable themselves, and that
>> the only exception to that is the case where we don't have such a
>> driver (and only valid as a temporary exception, which I guess you'll
>> soon turn into temporary-until-a-drm-kms-driver-is-merged).
>
> Exactly. simplefb is only a temporary measure. It's not meant to be used
> as a full-blown framebuffer driver. There are two use-cases where it is
> an appropriate solution:
>
> 1) As a stop-gap solution for when the platform doesn't support a full
> display driver yet. In that case you will want simplefb to stay
> around forever.
>
> 2) To get early boot output before a full display driver can be loaded
> in which case simplefb should go away after the display driver has
> taken over.
>
> In case of 2) the most straightforward solution is to not disable any
> clocks until all drivers have had a chance to claim them. When the full
> driver has been probed it should have claimed the clocks so they would
> no longer get disabled.
Please read my long reply from yesterday evening why this will never
work, as there is not a well defined moment when "all drivers have had a
chance to claim them"
I've been doing low level Linux development for 10 years now and I've
worked a lot on storage (raid, iscsi, fcoe support, etc.) in the past.
We've been down this road before, and all it leads to is pain, some more
pain and then even more pain. Followed by the conclusion that everything
needed to be refactored to use a hotplug model, and that instead of
waiting for hardware / disk-enumeration to be done (so we can mount
filesystems), the right thing to do is to actually wait for the block
device(s) holding said filesystems to show up.
The same applies here, the right thing to do is to wait for the kms
driver claiming the clocks to actually show up. And the easiest and
cleanest way to do that is to have simplefb claim the clocks, and
have it release them when the kms driver loads and asks simplefb
to unload.
Oh and one more thing, this whole make a list of special clocks which
should be disabled later model is flawed too, because it assumes a
fixed list of clocks, but which clocks actually get used may very well
depend on whether e.g. a vga or hdmi monitor is attached, so u-boot
would then still need to communicate the list of clocks actually used
somehow, and if it needs to pass a list of clocks, the most natural
way to do so is through a clocks property...
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Henrik Nordström @ 2014-08-27 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140826135341.GM15297@lukather>
tis 2014-08-26 klockan 15:53 +0200 skrev Maxime Ripard:
> > 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.
>
> Mike Turquette repeatedly said that he was against such a DT property:
> https://lkml.org/lkml/2014/5/12/693
And not really what we want here. The clocks should be turned off if not
in use, i.e. when simplefb is unbound from the device and no other
driver has taken over. Maybe even in the case where there is no simplefb
driver in-kernel, but if the use case is to be generalized then this
kind of resources need to be held until explicitly released to allow
resources to be held until their driver is loaded, which may happen long
after kernel bootup.
But again, for many use cases it is a virtual hardware property, not
really hardware in it's own but still smells a lot like hardware. These
parameters is only valid until the hardware have been reconfigured.
Regards
Henrik
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-27 8:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827074057.GS17528@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
On Wed, Aug 27, 2014 at 08:40:57AM +0100, Mark Brown wrote:
> On Tue, Aug 26, 2014 at 08:40:09PM +0200, Henrik Nordström wrote:
>
> > It is not clear to me where the hardware resources should be listed in
> > DT, being it a simplefb node or part of the actual hardware device node
> > properly marked as dynamic boot defaults or something else? It's
> > somewhere inbetween hardware and virtual device, and somewhat volatile.
> > As far as simplefb is concerned it is a hardware desription of the
> > framebuffer, but for a kms driver it's no more than firmware handover of
> > boottime settings and ceases to exists once the kms driver have
> > reconfigured the hardware.
>
> Is simplefb something that should be in the device tree distinctly in
> the first place - shouldn't it be a subset of the functionality of the
> video nodes? It's the same hardware being driven differently.
Therorically, yes, but that would mean knowing beforehand what the
final binding will look like, even before submitting the driver. Since
the bindings are always reviewed, and most of the time changed
slightly, that wouldn't work very well with the DT as a stable ABI
policy I guess.
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: Geert Uytterhoeven @ 2014-08-27 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827082208.GQ15297@lukather>
Hi Maxime,
On Wed, Aug 27, 2014 at 10:22 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, Aug 27, 2014 at 08:40:57AM +0100, Mark Brown wrote:
>> On Tue, Aug 26, 2014 at 08:40:09PM +0200, Henrik Nordström wrote:
>>
>> > It is not clear to me where the hardware resources should be listed in
>> > DT, being it a simplefb node or part of the actual hardware device node
>> > properly marked as dynamic boot defaults or something else? It's
>> > somewhere inbetween hardware and virtual device, and somewhat volatile.
>> > As far as simplefb is concerned it is a hardware desription of the
>> > framebuffer, but for a kms driver it's no more than firmware handover of
>> > boottime settings and ceases to exists once the kms driver have
>> > reconfigured the hardware.
>>
>> Is simplefb something that should be in the device tree distinctly in
>> the first place - shouldn't it be a subset of the functionality of the
>> video nodes? It's the same hardware being driven differently.
>
> Therorically, yes, but that would mean knowing beforehand what the
> final binding will look like, even before submitting the driver. Since
> the bindings are always reviewed, and most of the time changed
> slightly, that wouldn't work very well with the DT as a stable ABI
> policy I guess.
If you don't know how the bindings for a device will look like at the time of
writing your DTS, you're always screwed, whether you add a simpefb
node or not.
If you know how the bindings look like, just add the device, with an extra
"linux,simplefb" compatibility value.
If you don't know how the bindings look like, do your utter best in
guessing. Your DTS must be amended later anyway, either because
you guessed wrong[*] (in case you added a node to have simplefb
working), or because you have to add a real device node (in case you
didn't add one for simplefb).
[*] Actually you may have guessed right, in which case it'll continue
working as-is, and everybody will be happy.
Whether you want to keep backwards-compatibility in your future driver
with the "guessed wrong" node is up to you.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-08-27 8:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827065440.GG15640@ulmo>
[-- Attachment #1: Type: text/plain, Size: 9067 bytes --]
On Wed, Aug 27, 2014 at 08:54:41AM +0200, Thierry Reding wrote:
> On Tue, Aug 26, 2014 at 11:02:48PM +0200, Maxime Ripard wrote:
> > On Tue, Aug 26, 2014 at 04:35:51PM +0200, Thierry Reding wrote:
> > > > > > 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.
> > > >
> > > > And in this case, with this patch, simplefb will not claim any clock,
> > > > nor will fail probing.
> > > >
> > > > > 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.
> > > >
> > > > I guess such a hand over if it were to happen in the kernel would
> > > > involve the second driver claiming the resources before the first one
> > > > release them. How is that different in this case?
> > >
> > > It's different in that that driver will be hardware specific and know
> > > exactly what clock and other resources are required. It will have a
> > > device-specific binding.
> >
> > Except that you made simplefb a generic driver. So we have two choices
> > here: either we don't anything but the trivial case, and no one with a
> > rather more advanced hardware will be able to use it, or we try to
> > grab any resource that might be of use, which means clocks,
> > regulators, reserved memory region, or whatever, so that we pretty
> > much cover all cases. It really is as simple as that.
>
> No, it should be even simpler. simplefb shouldn't have to know any of
> this. It's just taking what firmware has set up and uses that. It's a
> stop-gap solution to provide information on the display until a real
> driver can be loaded and initializes the display hardware properly.
That was not the original intention of this driver then. Stephen's
commit log (commit 26549c8d36a6) was even mentionning some use cases:
"
Examples use-cases include:
* The built-in LCD panels on the Samsung ARM chromebook, and Tegra
devices, and likely many other ARM or embedded systems. These cannot
yet be supported using a full graphics driver, since the panel control
should be provided by the CDF (Common Display Framework), which has been
stuck in design/review for quite some time. One could support these
panels using custom SoC-specific code, but there is a desire to use
common infra-structure rather than having each SoC vendor invent their
own code, hence the desire to wait for CDF.
* Hardware for which a full graphics driver is not yet available, and
the path to obtain one upstream isn't yet clear. For example, the
Raspberry Pi.
* Any hardware in early stages of upstreaming, before a full graphics
driver has been tackled. This driver can provide a graphical boot
console (even full X support) much earlier in the upstreaming process,
thus making new SoC or board support more generally useful earlier.
"
We're clearly in the use cases 2 and 3.
I understand that you're suggesting a 4th use-case, which seems
totally valid, but let's not forget the reasons why it has been merged
in the first place.
> > > > > 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.
> > > >
> > > > Mike Turquette repeatedly said that he was against such a DT property:
> > > > https://lkml.org/lkml/2014/5/12/693
> > >
> > > Mike says in that email that he's opposing the addition of a property
> > > for clocks that is the equivalent of regulator-always-on. That's not
> > > what this is about. If at all it'd be a property to mark a clock that
> > > should not be disabled by default because it's essential.
> >
> > It's just semantic. How is "a clock that should not be disabled by
> > default because it's essential" not a clock that stays always on?
>
> Because a clock that should not be disabled by default can be turned off
> when appropriate. A clock that is always on can't be turned off.
If a clock is essential, then it should never be disabled. Or we don't
share the same meaning of essential.
> > Plus, you should read the mail further. It's clearly said that
> > consumer drivers should call clk_prepare_enable themselves, and that
> > the only exception to that is the case where we don't have such a
> > driver (and only valid as a temporary exception, which I guess you'll
> > soon turn into temporary-until-a-drm-kms-driver-is-merged).
>
> Exactly. simplefb is only a temporary measure. It's not meant to be used
> as a full-blown framebuffer driver. There are two use-cases where it is
> an appropriate solution:
>
> 1) As a stop-gap solution for when the platform doesn't support a full
> display driver yet. In that case you will want simplefb to stay
> around forever.
>
> 2) To get early boot output before a full display driver can be loaded
> in which case simplefb should go away after the display driver has
> taken over.
>
> In case of 2) the most straightforward solution is to not disable any
> clocks until all drivers have had a chance to claim them. When the full
> driver has been probed it should have claimed the clocks so they would
> no longer get disabled.
>
> For 1) I think it's fair to say that it's only a temporary solution to
> get something on the screen. There won't be any kind of acceleration at
> all, no power saving. Nothing but a dumb framebuffer. In that case it
> should be a simple matter of keeping a select number of clocks always on
> in the clock driver until support is more complete and it can be
> properly handled. It's not like it's going to make a difference anyway
> since simplefb won't ever disable them either.
We do agree on that. The only thing we seem to disagree on is how to
keep these clocks running.
> > > Adding Mike on this subthread too.
> > >
> > > Either way, Mark already suggested a different alternative in another
> > > subthread, namely to add a new kind of checkpoint at which subsystems
> > > can call a "disable unused" function that's called later than a late
> > > initcall. This is not going to fix things for you immediately because
> > > the clocks will still be switched off (only later) if you don't have
> > > a real driver that's claiming the clocks.
> >
> > Great, I'm glad we found a solution for a completely unrelated issue.
>
> It's not at all unrelated. See above.
Kind of, these patches were about the first use case you mentionned,
and your solution tackles the second one.
> > > But you can work around that for now by making the relevant clocks
> > > always on and remove that workaround once a real driver is loaded
> > > that knows how to handle them properly.
> >
> > So, let me get this straight. The clock provider driver should behave
> > as a clock consumer because it knows that in some cases, it might not
> > have any willingful enough consumer? Doesn't that even ring your
> > this-is-a-clear-abstraction-violation-bell just a tiny bit?
>
> No. The clock driver should simply not allow the clocks to be disabled
> if it isn't safe to do so.
Again, we do seem to differ on our interpretation of an essential
clock. To me, it is perfectly safe to disable the clocks. The system
will still be responding, the memory will be there, the CPU will keep
running, and we do have a way to recover from that disabled to clock
(for example to enable it back).
Plus, again, in Mike's mail, it's clearly said that adding hacks like
this to the clock driver should only be considered in the case where
we don't have a consuming driver, which is not our case here.
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: Maxime Ripard @ 2014-08-27 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdVkEPBtVT0dcDqQOQ_J0p+zYJwaUW8LqQX0Z+h6CHihRg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2938 bytes --]
Hi!
On Wed, Aug 27, 2014 at 10:37:36AM +0200, Geert Uytterhoeven wrote:
> Hi Maxime,
>
> On Wed, Aug 27, 2014 at 10:22 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, Aug 27, 2014 at 08:40:57AM +0100, Mark Brown wrote:
> >> On Tue, Aug 26, 2014 at 08:40:09PM +0200, Henrik Nordström wrote:
> >>
> >> > It is not clear to me where the hardware resources should be listed in
> >> > DT, being it a simplefb node or part of the actual hardware device node
> >> > properly marked as dynamic boot defaults or something else? It's
> >> > somewhere inbetween hardware and virtual device, and somewhat volatile.
> >> > As far as simplefb is concerned it is a hardware desription of the
> >> > framebuffer, but for a kms driver it's no more than firmware handover of
> >> > boottime settings and ceases to exists once the kms driver have
> >> > reconfigured the hardware.
> >>
> >> Is simplefb something that should be in the device tree distinctly in
> >> the first place - shouldn't it be a subset of the functionality of the
> >> video nodes? It's the same hardware being driven differently.
> >
> > Therorically, yes, but that would mean knowing beforehand what the
> > final binding will look like, even before submitting the driver. Since
> > the bindings are always reviewed, and most of the time changed
> > slightly, that wouldn't work very well with the DT as a stable ABI
> > policy I guess.
>
> If you don't know how the bindings for a device will look like at the time of
> writing your DTS, you're always screwed, whether you add a simpefb
> node or not.
>
> If you know how the bindings look like, just add the device, with an extra
> "linux,simplefb" compatibility value.
> If you don't know how the bindings look like, do your utter best in
> guessing. Your DTS must be amended later anyway, either because
> you guessed wrong[*] (in case you added a node to have simplefb
> working), or because you have to add a real device node (in case you
> didn't add one for simplefb).
Let's be conservative and consider the case where we would guess
wrong.
If we just rely on a simplefb node, when reviewing and integrating the
"new" bindings to describe accureately the various IPs involved in the
display path, we would obviously create new compatibles for
them. Since it's new compatibles, we can come up with any binding we'd
like, without have to consider the backward compatibility, since it's
a new binding.
Then, we just remove the simplefb, all is good.
If we were to try to create our bindings for all the IPs involved, and
were not pleased with the binding anymore when merging the driver,
then we would have to break the bindings, since we don't introduce a
new compatible anymore, but modifying an existing one.
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: Geert Uytterhoeven @ 2014-08-27 9:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140827085538.GS15297@lukather>
On Wed, Aug 27, 2014 at 10:55 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
>> >> Is simplefb something that should be in the device tree distinctly in
>> >> the first place - shouldn't it be a subset of the functionality of the
>> >> video nodes? It's the same hardware being driven differently.
>> >
>> > Therorically, yes, but that would mean knowing beforehand what the
>> > final binding will look like, even before submitting the driver. Since
>> > the bindings are always reviewed, and most of the time changed
>> > slightly, that wouldn't work very well with the DT as a stable ABI
>> > policy I guess.
>>
>> If you don't know how the bindings for a device will look like at the time of
>> writing your DTS, you're always screwed, whether you add a simpefb
>> node or not.
>>
>> If you know how the bindings look like, just add the device, with an extra
>> "linux,simplefb" compatibility value.
>> If you don't know how the bindings look like, do your utter best in
>> guessing. Your DTS must be amended later anyway, either because
>> you guessed wrong[*] (in case you added a node to have simplefb
>> working), or because you have to add a real device node (in case you
>> didn't add one for simplefb).
>
> Let's be conservative and consider the case where we would guess
> wrong.
>
> If we just rely on a simplefb node, when reviewing and integrating the
> "new" bindings to describe accureately the various IPs involved in the
> display path, we would obviously create new compatibles for
> them. Since it's new compatibles, we can come up with any binding we'd
> like, without have to consider the backward compatibility, since it's
> a new binding.
>
> Then, we just remove the simplefb, all is good.
I would keep the simplefb compatible value. Else you break compatibility
with old kernels that don't have your new driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-27 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53FCE704.4030103@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 8086 bytes --]
Can you please fix you mail setup. You're addressing me directly in this
email, yet I'm neither in To: nor Cc: headers. That's really annoying.
On Tue, Aug 26, 2014 at 09:59:00PM +0200, Hans de Goede wrote:
> Hi,
>
> On 08/26/2014 04:35 PM, Thierry Reding wrote:
> >On Tue, Aug 26, 2014 at 03:53:41PM +0200, Maxime Ripard wrote:
> >>On Tue, Aug 26, 2014 at 10:04:33AM +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. 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.
> >>
> >>No. What I mean is that it seems like we are somehow punished, or at
> >>least blamed, for having a better and more complete kernel support.
> >
> >This isn't a competition. Nobody's punishing or blaming anyone. This is
> >about finding the best solution for the problem at hand.
> >
> >>>>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.
> >>
> >>And in this case, with this patch, simplefb will not claim any clock,
> >>nor will fail probing.
> >>
> >>>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.
> >>
> >>I guess such a hand over if it were to happen in the kernel would
> >>involve the second driver claiming the resources before the first one
> >>release them. How is that different in this case?
> >
> >It's different in that that driver will be hardware specific and know
> >exactly what clock and other resources are required. It will have a
> >device-specific binding.
> >
> >>>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.
> >>
> >>Mike Turquette repeatedly said that he was against such a DT property:
> >>https://lkml.org/lkml/2014/5/12/693
> >
> >Mike says in that email that he's opposing the addition of a property
> >for clocks that is the equivalent of regulator-always-on. That's not
> >what this is about. If at all it'd be a property to mark a clock that
> >should not be disabled by default because it's essential.
> >
> >Adding Mike on this subthread too.
> >
> >Either way, Mark already suggested a different alternative in another
> >subthread, namely to add a new kind of checkpoint at which subsystems
> >can call a "disable unused" function that's called later than a late
> >initcall. This is not going to fix things for you immediately because
> >the clocks will still be switched off (only later) if you don't have
> >a real driver that's claiming the clocks. But you can work around that
> >for now by making the relevant clocks always on and remove that
> >workaround once a real driver is loaded that knows how to handle them
> >properly.
>
> This will simply not work, and is a ton more complicated then
> simply teaching simplefb about a clocks property, which is a really simple
> and clean patch.
>
> First of all let me explain why this won't work. When should those
> subsystems call this "later then a late initcall" call ? the kms driver
> may very well be a kernel module, which will be loaded by userspace,
> so we would need this to be triggered by userspace, but when ?
>
> When the initrd is done? What then if the kms driver is not in the initrd,
> so maybe when udev is done enumerating devices, but what then if the kernel
> is still enumerating hardware at this time?
Usually an initrd knows how to wait for all or a given set of devices to
be probed. udev is pretty good for that.
> Will we just put a sleep 30
> in our initscripts to make sure the kernel is done scanning all busses,
> will that be long enough? Maybe we need to re-introduce the scsi_wait_done
> kernel module which was added as an ugly hack to allow userspace to wait
> for the kernel to have finished scanning scsi (and ata / sata) busses,
> for controllers found at the time the module was load. Which never worked
> reliable, because it would be possible that the controller itself still
> needed to be discovered. We've spend years cleaning up userspace enough
> to be able to kill scsi_wait_done. We've already been down this road of
> waiting for hw enumeration to be done, and the problem is that on
> modern systems *it is never done*.
Those are mostly integration issues. If you don't load the driver from
initrd then I don't think anybody will complain when there's flicker
when it finally gets loaded. The whole point of this is to have early
access to the framebuffer and get display and graphics going as soon as
possible, so please don't pull in hypothetical scenarios where people
manually load drivers half an hour after the system has booted.
> So second of all, Thierry, what exactly is the technical argument against
> adding support for dealing with clocks to simplefb ?
I've already stated technical arguments against it. You could just go
back and read the thread again, or would you rather want me to repeat
them for your convenience?
> I've heard a lot of people in favor of it,
Of course you have, all the people in favour of it are sunxi people that
really want to see this merged because it gives them working display
after U-Boot.
> and only pretty much you against it,
At least Stephen also spoke out about it. But this quickly devolved into
the kind of thread you want to get out of as quickly as possible, so I
can't blame him for not continuing this discussion. In fact I'm very
much regretting getting into this in the first place. You clearly have
no intention to even consider any possible other solution that what was
posted, so it's pretty useless to try and convince you.
> and your argument so far seems to boil down to "I don't like it",
I'm not surprised that you think so since you seem to be very selective
in what you read (or register) and what you don't.
> is not really a technical sound argument IMHO. Moreover all the
> alternatives seem to be horribly complicated and most of them also
> seem to have several issues which will make them simply not work
> reliable.
I'm not an unreasonable person, or at least I like to think so, and if
there really isn't a better solution then I won't object to this patch
any longer. But as it stands I am not convinced that this is the best
solution, so I think we should keep the discussion going for a bit and
see if we really can't come up with something that will fix this for a
more general case rather than just your particular use-case. It's
evidently a recurring problem that others suffer from, too.
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