* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930075258.GH4081@lukather>
[-- Attachment #1: Type: text/plain, Size: 3828 bytes --]
On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
[...]
> > > > What happened in the Snow example is that regulators that were
> > > > previously on would all of a sudden be automatically disabled on boot
> > > > because there was now a driver that registered them with a generic
> > > > framework.
> > > >
> > > > The same thing is going to happen with simplefb for your device. If you
> > > > later realize that you need a regulator to keep the panel going, you'll
> > > > have to add code to your firmware to populate the corresponding
> > > > properties, otherwise the regulator will end up unused and will be
> > > > automatically disabled. At the same time you're going to break upstream
> > > > for all users of your old firmware because it doesn't add that property
> > > > yet.
> > > >
> > > > And the same will continue to happen for every new type of resource
> > > > you're going to add.
> > >
> > > Sure, we can add any resources we will need. Regulators, reset lines,
> > > pm domains, allocated memory, but I'm not really sure this is what you
> > > want, right?
> >
> > No it's not what I want. *You* want to add resource management to this
> > driver. What I'm saying is that if we start adding clocks then we can no
> > longer say no to resets or regulators or power domains either.
>
> Yes, because resource management can be more than just "keep the thing
> enabled". It might also be about not modifying anything, just like we
> saw for the clocks, but that might also apply to regulators voltage.
We've already determined that simplefb can't do anything meaningful with
the resources other than keep them in the status quo. It simply doesn't
have enough knowledge to do so. It doesn't know the exact pixel clock or
what voltage the attached panel needs.
> And the only way I can think of to deal with that properly is to have
> resources management in the driver.
My point is that if we had a proper way to tell the kernel not to do
anything with resources owned by firmware, then the driver wouldn't
have to do anything with the resources.
> > > I really start to consider adding a sunxi-uboot-fb, that would just
> > > duplicate the source code of simplefb but also taking the
> > > clocks. Somehow, I feel like it will be easier (and definitely less of
> > > a hack than using the generic common clock API).
> >
> > You're not getting it are you? What makes you think sunxi-uboot-fb is
> > going to be any different? This isn't about a name.
>
> At least, we would get to do any binding and resource management we
> want. And that's a big win.
So instead of trying to understand the concerns that I've expressed and
constructively contribute to finding a solution that works for everybody
you'd rather go and write a driver from scratch. Way to go.
I've already said that I'm not saying strictly no to these patches, but
what I want to see happen is some constructive discussion about whether
we can find better ways to do it. If we can't then I'm all for merging
these patches. Fortunately other (sub)threads have been somewhat more
constructive and actually come up with alternatives that should make
everyone happier.
If you're going to do SoC-specific bindings and resource management you
are in fact implementing what Grant suggested in a subthread. You're
implementing a dummy driver only for resource management, which isn't
really a bad thing. It can serve as a placeholder for now until you add
the real driver. And you can also use the simplefb driver to provide
the framebuffer.
Thierry
[-- 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: Michal Suchanek @ 2014-09-30 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930085429.GB4059@ulmo>
On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
>> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
>> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
>> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> [...]
>> > > > What happened in the Snow example is that regulators that were
>> > > > previously on would all of a sudden be automatically disabled on boot
>> > > > because there was now a driver that registered them with a generic
>> > > > framework.
>> > > >
>> > > > The same thing is going to happen with simplefb for your device. If you
>> > > > later realize that you need a regulator to keep the panel going, you'll
>> > > > have to add code to your firmware to populate the corresponding
>> > > > properties, otherwise the regulator will end up unused and will be
>> > > > automatically disabled. At the same time you're going to break upstream
>> > > > for all users of your old firmware because it doesn't add that property
>> > > > yet.
>> > > >
>> > > > And the same will continue to happen for every new type of resource
>> > > > you're going to add.
>> > >
>> > > Sure, we can add any resources we will need. Regulators, reset lines,
>> > > pm domains, allocated memory, but I'm not really sure this is what you
>> > > want, right?
>> >
>> > No it's not what I want. *You* want to add resource management to this
>> > driver. What I'm saying is that if we start adding clocks then we can no
>> > longer say no to resets or regulators or power domains either.
>>
>> Yes, because resource management can be more than just "keep the thing
>> enabled". It might also be about not modifying anything, just like we
>> saw for the clocks, but that might also apply to regulators voltage.
>
> We've already determined that simplefb can't do anything meaningful with
> the resources other than keep them in the status quo. It simply doesn't
> have enough knowledge to do so. It doesn't know the exact pixel clock or
> what voltage the attached panel needs.
>
>> And the only way I can think of to deal with that properly is to have
>> resources management in the driver.
>
> My point is that if we had a proper way to tell the kernel not to do
> anything with resources owned by firmware, then the driver wouldn't
> have to do anything with the resources.
The firmware on sunxi does not own any resources whatsoever. It ceases
running once it executes the kernel. This is different from ACPI and
UEFI where you have pieces of the firmware lingering indefinitely and
potentially getting invoked by user pressing some button or some other
hardware event. It is also different from rpi where the Linux kernel
effectively runs in a virtual environment created by the firmware
hypervisor.
So on sunxi and many other ARM machines the Linux kernel is the sole
owner of any resources that might happen to be available on the
machine. There is no firmware owning them when the Linux kernel is
running, ever.
And we do have a proper way to tell to the kernel what these resources
are used for - inserting description of them into the simplefb DT
node. Sure the simplefb cannot manage the resources in any way and but
it does own them. When it is running they are in use, when it stops
they are free to be reclaimed by the platform driver.
But you refuse to marge the kernel change for this proper management to happen.
The alternate proposal to stop the kernel from managing resources
while simplefb is running and refernce count drivers that cannot
manage resources is indeed a workable, general alternative.
It does however switch the kernel into special mode when resources are
not managed and will needlessly hinder eg. development and testing of
powermanagement and hotplug for drivers unrelated to kms in parallel
to kms.
But let's look at this special mode closer.
Under normal boot sequence when the built-in drivers are initialized
or around that time the kernel does a pass in which it disables unused
clocks and possibly reclaims other resources.
The boot has finished for the kernel and now it hands over to
userspace and it is up to the userspace to load any more drivers (such
as kms) or not. If at that point a driver like simplefb exists it
should have called that stop_managing_resources() which should replace
clk_ignore_unused() so that other resources are properly handled
without the driver ever having to know about them.
For clocks this should be simple. At least on sunxi the clock driver
can tell if the clock is gated and can potentially be in use. It can
even mark clocks that are used at this point to know not to ever
disable them. Also it should refuse to ever change a clock frequency
on these clocks since if one of them was used for simplefb it should
break. It does not matter it's a mmc bus clock completely unrelated to
display. If you assume no knowledge you cannot change the mmc clock
when a different card is inserted or when a card is inserted into an
empty slot. If the firmware probed the slot the clock might be running
anyway.
Other resources might be more difficult to tackle. Is it possible to
change voltage regulators? Even the fact that it is set to 0 does not
necessarily mean that it is unused and then other driver claiming it
later can change the voltage. So no changing the regulators, ever.
And there goes gpio. You cannot assume that you can change any pin
muxing or any pin level whatsoever. The pin might be connected to the
display. So no loading of drivers that need any pin assignment
changes.
I guess by now it's clear that if this mode that assumes no knowledge
of the underlying hardware and requires the kernel to not manage
resources in order to keep dumb drivers running completely paralyzes
the kernel and will among other things prevent loading of the proper
kms driver.
>
>> > > I really start to consider adding a sunxi-uboot-fb, that would just
>> > > duplicate the source code of simplefb but also taking the
>> > > clocks. Somehow, I feel like it will be easier (and definitely less of
>> > > a hack than using the generic common clock API).
>> >
>> > You're not getting it are you? What makes you think sunxi-uboot-fb is
>> > going to be any different? This isn't about a name.
>>
>> At least, we would get to do any binding and resource management we
>> want. And that's a big win.
>
> So instead of trying to understand the concerns that I've expressed and
> constructively contribute to finding a solution that works for everybody
> you'd rather go and write a driver from scratch. Way to go.
It's the constructive thing to do when the existing driver cannot be
extended to work for everybody.
>
> I've already said that I'm not saying strictly no to these patches, but
> what I want to see happen is some constructive discussion about whether
> we can find better ways to do it. If we can't then I'm all for merging
> these patches. Fortunately other (sub)threads have been somewhat more
> constructive and actually come up with alternatives that should make
> everyone happier.
What are those alternatives?
>
> If you're going to do SoC-specific bindings and resource management you
> are in fact implementing what Grant suggested in a subthread. You're
> implementing a dummy driver only for resource management, which isn't
> really a bad thing. It can serve as a placeholder for now until you add
> the real driver. And you can also use the simplefb driver to provide
> the framebuffer.
Oh, so back to the proposal to make a driver that claims the required
resources and then instantiates an unextended simplefb that is
oblivious to the resources to be kept simple?
Did I not propose that way back?
Was it not already rejected?
Thanks
Michal
^ permalink raw reply
* Re: [PATCH] fonts: Add 6x10 font
From: Tomi Valkeinen @ 2014-09-30 9:41 UTC (permalink / raw)
To: Maarten ter Huurne, Jean-Christophe Plagniol-Villard,
Geert Uytterhoeven
Cc: Apelete Seketeli, linux-fbdev, linux-kernel
In-Reply-To: <1410263188-10532-1-git-send-email-maarten@treewalker.org>
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
Hi,
On 09/09/14 14:46, Maarten ter Huurne wrote:
> This font is suitable for framebuffer consoles on devices with a
> 320x240 screen, to get a reasonable number of characters (53x24) that
> are still at a readable size.
>
> The font is derived from the existing 6x11 font, but gets 3 extra
> lines without sacrificing readability. Also I redesigned a some glyhps
> so they are more distinct and better fill the available space.
>
> Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
> ---
> include/linux/font.h | 4 +-
> lib/fonts/Kconfig | 9 +
> lib/fonts/Makefile | 1 +
> lib/fonts/font_6x10.c | 3086 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/fonts/fonts.c | 4 +
> 5 files changed, 3103 insertions(+), 1 deletion(-)
> create mode 100644 lib/fonts/font_6x10.c
I think the patch is fine, but there hasn't been any comments or acks
for this. I'm a bit reluctant to add this much data to the kernel if
there are no users (except you) for this.
So I'd like to hear at least one "yeah, I could maybe use that font"
from someone =).
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: valkyriefb: Fix unused variable warning in set_valkyrie_clock()
From: Tomi Valkeinen @ 2014-09-30 9:43 UTC (permalink / raw)
To: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-m68k, linuxppc-dev
In-Reply-To: <1410688110-11042-1-git-send-email-geert@linux-m68k.org>
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
On 14/09/14 12:48, Geert Uytterhoeven wrote:
> If CONFIG_ADB_CUDA=n:
>
> drivers/video/fbdev/valkyriefb.c: In function ‘set_valkyrie_clock’:
> drivers/video/fbdev/valkyriefb.c:267: warning: unused variable ‘i’
> drivers/video/fbdev/valkyriefb.c:266: warning: unused variable ‘req’
>
> Move the variable declarations inside the existing #ifdef section to fix
> this.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> drivers/video/fbdev/valkyriefb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
> index 97cb9bd1d1dd..b6ed09f16355 100644
> --- a/drivers/video/fbdev/valkyriefb.c
> +++ b/drivers/video/fbdev/valkyriefb.c
> @@ -263,10 +263,10 @@ static inline int valkyrie_vram_reqd(int video_mode, int color_mode)
>
> static void set_valkyrie_clock(unsigned char *params)
> {
> +#ifdef CONFIG_ADB_CUDA
> struct adb_request req;
> int i;
>
> -#ifdef CONFIG_ADB_CUDA
> for (i = 0; i < 3; ++i) {
> cuda_request(&req, NULL, 5, CUDA_PACKET, CUDA_GET_SET_IIC,
> 0x50, i + 1, params[i]);
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: sis: sis_main.c: Cleaning up missing null-terminate in conjunction with s
From: Tomi Valkeinen @ 2014-09-30 10:01 UTC (permalink / raw)
To: Rickard Strandqvist, Thomas Winischhofer,
Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <1410716148-16243-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
On 14/09/14 20:35, Rickard Strandqvist wrote:
> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/video/fbdev/sis/sis_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
> index 3f12a2d..1f4c216 100644
> --- a/drivers/video/fbdev/sis/sis_main.c
> +++ b/drivers/video/fbdev/sis/sis_main.c
> @@ -5830,7 +5830,7 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> ivideo->cardnumber++;
> }
>
> - strncpy(ivideo->myid, chipinfo->chip_name, 30);
> + strlcpy(ivideo->myid, chipinfo->chip_name, sizeof(ivideo->myid));
>
> ivideo->warncount = 0;
> ivideo->chip_id = pdev->device;
>
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] video: fbdev: stifb.c: use container_of to resolve stifb_info from fb_info
From: Tomi Valkeinen @ 2014-09-30 10:08 UTC (permalink / raw)
To: Fabian Frederick, linux-kernel
Cc: daniel.vetter, James E.J. Bottomley, Helge Deller,
Jean-Christophe Plagniol-Villard, linux-parisc, linux-fbdev
In-Reply-To: <1410980419-5326-2-git-send-email-fabf@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Hi,
On 17/09/14 22:00, Fabian Frederick wrote:
> Use container_of instead of casting first structure member.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> PARISC cross-compiled but untested.
>
> drivers/video/fbdev/stifb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/stifb.c b/drivers/video/fbdev/stifb.c
> index cfe8a2f..86621fa 100644
> --- a/drivers/video/fbdev/stifb.c
> +++ b/drivers/video/fbdev/stifb.c
> @@ -918,7 +918,7 @@ static int
> stifb_setcolreg(u_int regno, u_int red, u_int green,
> u_int blue, u_int transp, struct fb_info *info)
> {
> - struct stifb_info *fb = (struct stifb_info *) info;
> + struct stifb_info *fb = container_of(info, struct stifb_info, info);
> u32 color;
>
> if (regno >= NR_PALETTE)
> @@ -978,7 +978,7 @@ stifb_setcolreg(u_int regno, u_int red, u_int green,
> static int
> stifb_blank(int blank_mode, struct fb_info *info)
> {
> - struct stifb_info *fb = (struct stifb_info *) info;
> + struct stifb_info *fb = container_of(info, struct stifb_info, info);
> int enable = (blank_mode == 0) ? ENABLE : DISABLE;
>
> switch (fb->id) {
>
Thanks, queuing the series for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] LLVMLinux: Patches to enable the kernel to be compiled with clang/LLVM
From: Tomi Valkeinen @ 2014-09-30 10:15 UTC (permalink / raw)
To: Behan Webster, plagnioj
Cc: daniel.vetter, geert, laurent.pinchart, linux-fbdev, linux-kernel,
linux-omap, robdclark
In-Reply-To: <1411780253-4492-1-git-send-email-behanw@converseincode.com>
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
On 27/09/14 04:10, Behan Webster wrote:
> Replace the use of nested functions where a normal function will suffice.
>
> Nested functions are not liked by upstream kernel developers in general. Their
> use breaks the use of clang as a compiler, and doesn't make the code any
> better.
>
> This code now works for both gcc and clang.
>
> The LLVMLinux project aims to fully build the Linux kernel using both gcc and
> clang (the C front end for the LLVM compiler infrastructure project).
>
> Behan Webster (2):
> arm, fbdev, omap2, LLVMLinux: Remove nested function from omap2 dss
> arm, fbdev, omap2, LLVMLinux: Remove nested function from omapfb
>
> drivers/video/fbdev/omap2/dss/dispc-compat.c | 9 +++++----
> drivers/video/fbdev/omap2/dss/manager-sysfs.c | 16 +++++++++-------
> drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 14 +++++++-------
> 3 files changed, 21 insertions(+), 18 deletions(-)
>
I have to say I do like small helper funcs as nested functions, as they
are restricted inside the parent function's scope. But, of course,
nested funcs have their issues.
So looks fine to me, queuing for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Tomi Valkeinen @ 2014-09-30 10:24 UTC (permalink / raw)
To: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson
Cc: Guennadi Liakhovetski, linux-fbdev, linux-pm, linux-sh
In-Reply-To: <1411474918-2955-1-git-send-email-geert+renesas@glider.be>
[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]
Hi Geert, Ulf,
On 23/09/14 15:21, Geert Uytterhoeven wrote:
> When the PM domain containing the HDMI hardware block is powered down,
> the HDMI register values (incl. interrupt polarity settings) are lost.
> During resume, after powering up the PM domain, interrupts are
> re-enabled, and an interrupt storm happens due to incorrect interrupt
> polarity settings:
>
> irq 163: nobody cared (try booting with the "irqpoll" option)
> ...
> Disabling IRQ #163
>
> To fix this, re-initialize the interrupt polarity settings, and the
> htop1 register block (if present), during resume.
>
> As the .suspend_noirq() and .resume_noirq() callbacks are not called
> when using the generic PM domain, the normal .resume() callback is used,
> and the device interrupt needs to be disabled/enabled manually.
>
> This fixes resume from s2ram with power down of the A4MP PM domain on
> r8a7740/Armadillo.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
> callbacks are not called when using genpd, unlike .suspend(),
> .suspend_late(), .resume_early(), and .resume()?
> ---
> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
Should I queue this version for 3.18, or is there some modifications you
think needs to be done first?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] framebuffer: fix border color
From: Tomi Valkeinen @ 2014-09-30 10:41 UTC (permalink / raw)
To: Mikulas Patocka, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1409161238560.3150@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
On 16/09/14 19:40, Mikulas Patocka wrote:
> The framebuffer code uses the current background color to fill the border
> when switching consoles, however, this results in inconsistent behavior.
> For example:
> - start Midnigh Commander
> - the border is black
> - switch to another console and switch back
> - the border is cyan
> - type something into the command line in mc
> - the border is cyan
> - switch to another console and switch back
> - the border is black
> - press F9 to go to menu
> - the border is black
> - switch to another console and switch back
> - the border is dark blue
>
> When switching to a console with Midnight Commander, the border is random
> color that was left selected by the slang subsystem.
>
> This patch fixes this inconsistency by always using black as the
> background color when switching consoles.
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Geert Uytterhoeven @ 2014-09-30 10:41 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson, Guennadi Liakhovetski,
Linux Fbdev development list, Linux PM list, Linux-sh list
In-Reply-To: <542A84F7.5020005@ti.com>
Hi Tomi,
On Tue, Sep 30, 2014 at 12:24 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>> When the PM domain containing the HDMI hardware block is powered down,
>> the HDMI register values (incl. interrupt polarity settings) are lost.
>> During resume, after powering up the PM domain, interrupts are
>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>> polarity settings:
>>
>> irq 163: nobody cared (try booting with the "irqpoll" option)
>> ...
>> Disabling IRQ #163
>>
>> To fix this, re-initialize the interrupt polarity settings, and the
>> htop1 register block (if present), during resume.
>>
>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>> when using the generic PM domain, the normal .resume() callback is used,
>> and the device interrupt needs to be disabled/enabled manually.
>>
>> This fixes resume from s2ram with power down of the A4MP PM domain on
>> r8a7740/Armadillo.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>> callbacks are not called when using genpd, unlike .suspend(),
>> .suspend_late(), .resume_early(), and .resume()?
>> ---
>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> Should I queue this version for 3.18, or is there some modifications you
> think needs to be done first?
Given the lack of documentation, I cannot do more improvements.
Please apply this version, so the interrupt storm on resume is fixed.
Thanks for applying!
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: [PATCH] framebuffer: fix screen corruption when copying
From: Tomi Valkeinen @ 2014-09-30 10:41 UTC (permalink / raw)
To: Mikulas Patocka, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <alpine.LRH.2.02.1409161236550.3150@file01.intranet.prod.int.rdu2.redhat.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
On 16/09/14 19:38, Mikulas Patocka wrote:
> The function bitcpy_rev has a bug that may result in screen corruption.
> The bug happens under these conditions:
> * the end of the destination area of a copy operation is aligned on a long
> word boundary
> * the end of the source area is not aligned on a long word boundary
> * we are copying more than one long word
>
> In this case, the variable shift is non-zero and the variable first is
> zero. The statements FB_WRITEL(comp(d0, FB_READL(dst), first), dst) reads
> the last long word of the destination and writes it back unchanged
> (because first is zero). Correctly, we should write the variable d0 to the
> last word of the destination in this case.
>
> This patch fixes the bug by introducing and extra test if first is zero.
>
> The patch also removes the references to fb_memmove in the code that is
> commented out because fb_memmove was removed from framebuffer subsystem.
Thanks, queued for 3.18.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Tomi Valkeinen @ 2014-09-30 10:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Ulf Hansson, Guennadi Liakhovetski,
Linux Fbdev development list, Linux PM list, Linux-sh list
In-Reply-To: <CAMuHMdV-PLgG6yC=e9=qJNKbyYbUcVS-QeOaAMtf53AUOvXBvA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]
On 30/09/14 13:41, Geert Uytterhoeven wrote:
> Hi Tomi,
>
> On Tue, Sep 30, 2014 at 12:24 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>>> When the PM domain containing the HDMI hardware block is powered down,
>>> the HDMI register values (incl. interrupt polarity settings) are lost.
>>> During resume, after powering up the PM domain, interrupts are
>>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>>> polarity settings:
>>>
>>> irq 163: nobody cared (try booting with the "irqpoll" option)
>>> ...
>>> Disabling IRQ #163
>>>
>>> To fix this, re-initialize the interrupt polarity settings, and the
>>> htop1 register block (if present), during resume.
>>>
>>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>>> when using the generic PM domain, the normal .resume() callback is used,
>>> and the device interrupt needs to be disabled/enabled manually.
>>>
>>> This fixes resume from s2ram with power down of the A4MP PM domain on
>>> r8a7740/Armadillo.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>>> callbacks are not called when using genpd, unlike .suspend(),
>>> .suspend_late(), .resume_early(), and .resume()?
>>> ---
>>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> Should I queue this version for 3.18, or is there some modifications you
>> think needs to be done first?
>
> Given the lack of documentation, I cannot do more improvements.
>
> Please apply this version, so the interrupt storm on resume is fixed.
Ok, queued. Thanks!
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH/RFC] fbdev: sh_mobile_hdmi: Re-init regs before irq re-enable on resume
From: Ulf Hansson @ 2014-09-30 10:43 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
Rafael J. Wysocki, Guennadi Liakhovetski,
Linux Fbdev development list, linux-pm@vger.kernel.org,
Linux-sh list
In-Reply-To: <542A84F7.5020005@ti.com>
On 30 September 2014 12:24, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi Geert, Ulf,
>
> On 23/09/14 15:21, Geert Uytterhoeven wrote:
>> When the PM domain containing the HDMI hardware block is powered down,
>> the HDMI register values (incl. interrupt polarity settings) are lost.
>> During resume, after powering up the PM domain, interrupts are
>> re-enabled, and an interrupt storm happens due to incorrect interrupt
>> polarity settings:
>>
>> irq 163: nobody cared (try booting with the "irqpoll" option)
>> ...
>> Disabling IRQ #163
>>
>> To fix this, re-initialize the interrupt polarity settings, and the
>> htop1 register block (if present), during resume.
>>
>> As the .suspend_noirq() and .resume_noirq() callbacks are not called
>> when using the generic PM domain, the normal .resume() callback is used,
>> and the device interrupt needs to be disabled/enabled manually.
>>
>> This fixes resume from s2ram with power down of the A4MP PM domain on
>> r8a7740/Armadillo.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Is there a specific reason why the .suspend_noirq() and .resume_noirq()
>> callbacks are not called when using genpd, unlike .suspend(),
>> .suspend_late(), .resume_early(), and .resume()?
>> ---
>> drivers/video/fbdev/sh_mobile_hdmi.c | 44 ++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> Should I queue this version for 3.18, or is there some modifications you
> think needs to be done first?
I have no objections for this patch. Please go ahead.
The reason for my comments was to try to figure out if there were any
impact to the generic PM domain.
Kind regards
Uffe
^ permalink raw reply
* Re: [RFC] fbdev: mxsfb: Add support for mx6sl and mx6sx
From: Tomi Valkeinen @ 2014-09-30 10:54 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]
On 19/08/14 21:23, Fabio Estevam wrote:
> mx6sl and mx6sx share the same LCD controller as mx23 and mx28.
>
> Add support for it.
>
> The basic difference is the number of clocks that are required:
>
> - mx23/mx28: only one clock
> - mx6sl: two clocks
> - mx6sx: three clocks
>
> Tested on a mx28evk and mx6slevk.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> I am sending it as RFC because the scrolling is showing some artifacts when
> I boot with console=tty0. The kernel messages are displayed correctly, but when
> it scrolls I see some garbage.
Is the garbage on the framebuffer memory? If so, I don't think this
patch can cause it.
> Appreciate any comment/suggestion to improve this.
>
> drivers/video/fbdev/Kconfig | 2 +-
> drivers/video/fbdev/mxsfb.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index e911b9c..fd28987 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -2420,7 +2420,7 @@ config FB_JZ4740
>
> config FB_MXS
> tristate "MXS LCD framebuffer support"
> - depends on FB && ARCH_MXS
> + depends on FB && (ARCH_MXS || ARCH_MXC)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> index accf48a2..a567cc6 100644
> --- a/drivers/video/fbdev/mxsfb.c
> +++ b/drivers/video/fbdev/mxsfb.c
> @@ -172,6 +172,10 @@ struct mxsfb_info {
> struct fb_info fb_info;
> struct platform_device *pdev;
> struct clk *clk;
> + struct clk *clk_axi;
> + struct clk *clk_disp_axi;
> + bool clk_axi_enabled;
> + bool clk_disp_axi_enabled;
> void __iomem *base; /* registers */
> unsigned allocated_size;
> int enabled;
> @@ -185,6 +189,42 @@ struct mxsfb_info {
> #define mxsfb_is_v3(host) (host->devdata->ipversion == 3)
> #define mxsfb_is_v4(host) (host->devdata->ipversion == 4)
>
> +/* Enable lcdif axi clock */
> +static inline void clk_enable_axi(struct mxsfb_info *host)
> +{
> + if (!host->clk_axi_enabled && (!IS_ERR(host->clk_axi))) {
> + clk_prepare_enable(host->clk_axi);
> + host->clk_axi_enabled = true;
> + }
> +}
> +
> +/* Disable lcdif axi clock */
> +static inline void clk_disable_axi(struct mxsfb_info *host)
> +{
> + if (host->clk_axi_enabled && (!IS_ERR(host->clk_axi))) {
> + clk_disable_unprepare(host->clk_axi);
> + host->clk_axi_enabled = false;
> + }
I think you could do the IS_ERR(clk_axi) check when getting the clock,
and if there's an error, do host->clk_axi = NULL. Then above you can do
just:
if (host->clk_axi)
clk_disable_unprepare(host->clk_axi);
And no need to clk_axi_enabled. And probably the helper functions can be
left out and have the code inline.
And similarly for clk_disp_axi, of course.
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: Thierry Reding @ 2014-09-30 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930080354.GI4081@lukather>
[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]
On Tue, Sep 30, 2014 at 10:03:54AM +0200, Maxime Ripard wrote:
> On Tue, Sep 30, 2014 at 07:39:02AM +0200, Thierry Reding wrote:
> > You keep bringing up the Raspberry Pi for some reason and suggest that
> > it is somehow inferior to sunxi. What makes you think it's less entitled
> > to be supported on Linux than sunxi? I don't care about the Raspberry Pi
> > and I equally don't care about sunxi. I don't own a Raspberry Pi and I
> > don't own any Allwinner hardware. What I do care about is Linux and I
> > want it to work well for all SoCs equally.
> >
> > Perhaps if you could put aside your crusade against the Raspberry Pi for
> > just a second you'll realize that we're all on the same team. This isn't
> > a competition and I'm not trying to put a spoke in your wheel. On the
> > contrary, I'm actually trying to help you.
>
> We've been over this already, and I'll tell you again that you're
> getting this wrong.
>
> No platform is more entitled to get merged than another one. I do care
> about the Allwinner SoCs, and I care just as much about the broader
> Linux support for all the other SoCs, be it from nvidia, samsung or
> whatever vendor you can come up with.
Okay, I'm glad our goals aren't that far apart then. It would be helpful
to stop dragging the Raspberry Pi into this, though, since it isn't at
all relevant.
> But you can't hide the fact that the bcm2835 still has a very limited
> clock support, and I really don't know about its clock tree, but I
> guess that if the times come when they add a more complete clock
> support, they will face the same issue.
This isn't at all relevant. And that's exactly why I think it's good to
hide all the resource management behind firmware. That way it becomes
easy to support any SoC with any firmware.
> If the driver would have been developped initially to create a
> framebuffer on the Allwinner SoCs, at a time when we didn't have any
> clock support too, calling it only usable on sunxi wouldn't have
> shocked me tbh.
The functionality that it provides is still very generic. And the
firmware interface is generic too. It is this abstraction that allows
it to be generic. You on the other hand seem to be arguing that by
making it abstract we've made it less generic.
Abstraction is about hiding details to capture commonality.
Thierry
[-- 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-09-30 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMqctRM1YZBR+p=0R39BSX_GT6XagPcb4HBG1BqW1OqyB48zw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10430 bytes --]
On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote:
> On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> >> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> >> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> >> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> > [...]
> >> > > > What happened in the Snow example is that regulators that were
> >> > > > previously on would all of a sudden be automatically disabled on boot
> >> > > > because there was now a driver that registered them with a generic
> >> > > > framework.
> >> > > >
> >> > > > The same thing is going to happen with simplefb for your device. If you
> >> > > > later realize that you need a regulator to keep the panel going, you'll
> >> > > > have to add code to your firmware to populate the corresponding
> >> > > > properties, otherwise the regulator will end up unused and will be
> >> > > > automatically disabled. At the same time you're going to break upstream
> >> > > > for all users of your old firmware because it doesn't add that property
> >> > > > yet.
> >> > > >
> >> > > > And the same will continue to happen for every new type of resource
> >> > > > you're going to add.
> >> > >
> >> > > Sure, we can add any resources we will need. Regulators, reset lines,
> >> > > pm domains, allocated memory, but I'm not really sure this is what you
> >> > > want, right?
> >> >
> >> > No it's not what I want. *You* want to add resource management to this
> >> > driver. What I'm saying is that if we start adding clocks then we can no
> >> > longer say no to resets or regulators or power domains either.
> >>
> >> Yes, because resource management can be more than just "keep the thing
> >> enabled". It might also be about not modifying anything, just like we
> >> saw for the clocks, but that might also apply to regulators voltage.
> >
> > We've already determined that simplefb can't do anything meaningful with
> > the resources other than keep them in the status quo. It simply doesn't
> > have enough knowledge to do so. It doesn't know the exact pixel clock or
> > what voltage the attached panel needs.
> >
> >> And the only way I can think of to deal with that properly is to have
> >> resources management in the driver.
> >
> > My point is that if we had a proper way to tell the kernel not to do
> > anything with resources owned by firmware, then the driver wouldn't
> > have to do anything with the resources.
>
> The firmware on sunxi does not own any resources whatsoever. It ceases
> running once it executes the kernel. This is different from ACPI and
> UEFI where you have pieces of the firmware lingering indefinitely and
> potentially getting invoked by user pressing some button or some other
> hardware event. It is also different from rpi where the Linux kernel
> effectively runs in a virtual environment created by the firmware
> hypervisor.
You know all that because you of course wrote every single firmware
implementation that does and will ever exist for sunxi. There's nothing
keeping anyone from running UEFI on a sunxi SoC.
> So on sunxi and many other ARM machines the Linux kernel is the sole
> owner of any resources that might happen to be available on the
> machine. There is no firmware owning them when the Linux kernel is
> running, ever.
Of course this is part of the abstraction. The idea is that the device
is a virtual one created by firmware. Therefore firmware owns the
resources until the virtual device has been handed over to the kernel.
If you're into splitting hairs, then the simplefb device shouldn't exist
in the first place.
> And we do have a proper way to tell to the kernel what these resources
> are used for - inserting description of them into the simplefb DT
> node. Sure the simplefb cannot manage the resources in any way and but
> it does own them. When it is running they are in use, when it stops
> they are free to be reclaimed by the platform driver.
Yes. And again I'm not saying anything different. What I'm saying is
that we shouldn't need to know about the resources and instead hide that
within the firmware, for the same reason that we're already hiding the
register programming in hardware, namely to create an abstraction that
works irrespective of the underlying hardware.
> But you refuse to marge the kernel change for this proper management
> to happen.
I have no authority to merge these changes nor have I any way of vetoing
them. All I'm saying is that I think it's a bad idea. If nobody else
thinks it is then it will eventually get merged irrespective of what I'm
saying. And when that happens I'll shut up and live happily ever after.
But it doesn't magically convince me that it's a good idea.
> The alternate proposal to stop the kernel from managing resources
> while simplefb is running and refernce count drivers that cannot
> manage resources is indeed a workable, general alternative.
>
> It does however switch the kernel into special mode when resources are
> not managed and will needlessly hinder eg. development and testing of
> powermanagement and hotplug for drivers unrelated to kms in parallel
> to kms.
>
> But let's look at this special mode closer.
>
> Under normal boot sequence when the built-in drivers are initialized
> or around that time the kernel does a pass in which it disables unused
> clocks and possibly reclaims other resources.
>
> The boot has finished for the kernel and now it hands over to
> userspace and it is up to the userspace to load any more drivers (such
> as kms) or not. If at that point a driver like simplefb exists it
> should have called that stop_managing_resources() which should replace
> clk_ignore_unused() so that other resources are properly handled
> without the driver ever having to know about them.
>
> For clocks this should be simple. At least on sunxi the clock driver
> can tell if the clock is gated and can potentially be in use. It can
> even mark clocks that are used at this point to know not to ever
> disable them. Also it should refuse to ever change a clock frequency
> on these clocks since if one of them was used for simplefb it should
> break. It does not matter it's a mmc bus clock completely unrelated to
> display. If you assume no knowledge you cannot change the mmc clock
> when a different card is inserted or when a card is inserted into an
> empty slot. If the firmware probed the slot the clock might be running
> anyway.
>
> Other resources might be more difficult to tackle. Is it possible to
> change voltage regulators? Even the fact that it is set to 0 does not
> necessarily mean that it is unused and then other driver claiming it
> later can change the voltage. So no changing the regulators, ever.
>
> And there goes gpio. You cannot assume that you can change any pin
> muxing or any pin level whatsoever. The pin might be connected to the
> display. So no loading of drivers that need any pin assignment
> changes.
The original patches don't tackle any of these problems. And in addition
to what's been mentioned elsewhere you're adding two new types of
resources that need to be potentially claimed here. Fortunately for GPIO
and pinmux the default is to not touch them at all unless explicitly
claimed and controlled. At least as far as I know.
> I guess by now it's clear that if this mode that assumes no knowledge
> of the underlying hardware and requires the kernel to not manage
> resources in order to keep dumb drivers running completely paralyzes
> the kernel and will among other things prevent loading of the proper
> kms driver.
Yes, Geert already made that argument and I admit that it exposes a flaw
in the solution that I proposed.
> >> > > I really start to consider adding a sunxi-uboot-fb, that would just
> >> > > duplicate the source code of simplefb but also taking the
> >> > > clocks. Somehow, I feel like it will be easier (and definitely less of
> >> > > a hack than using the generic common clock API).
> >> >
> >> > You're not getting it are you? What makes you think sunxi-uboot-fb is
> >> > going to be any different? This isn't about a name.
> >>
> >> At least, we would get to do any binding and resource management we
> >> want. And that's a big win.
> >
> > So instead of trying to understand the concerns that I've expressed and
> > constructively contribute to finding a solution that works for everybody
> > you'd rather go and write a driver from scratch. Way to go.
>
> It's the constructive thing to do when the existing driver cannot be
> extended to work for everybody.
No, it isn't. If a generic driver doesn't work for everybody then it
isn't generic and we should fix it. Not duplicate it and add platform
specific quirks.
> > I've already said that I'm not saying strictly no to these patches, but
> > what I want to see happen is some constructive discussion about whether
> > we can find better ways to do it. If we can't then I'm all for merging
> > these patches. Fortunately other (sub)threads have been somewhat more
> > constructive and actually come up with alternatives that should make
> > everyone happier.
>
> What are those alternatives?
Sorry, you've got to do some of the work yourself. They've been
mentioned in this thread and the one that Maxime pointed to the other
day.
> > If you're going to do SoC-specific bindings and resource management you
> > are in fact implementing what Grant suggested in a subthread. You're
> > implementing a dummy driver only for resource management, which isn't
> > really a bad thing. It can serve as a placeholder for now until you add
> > the real driver. And you can also use the simplefb driver to provide
> > the framebuffer.
>
> Oh, so back to the proposal to make a driver that claims the required
> resources and then instantiates an unextended simplefb that is
> oblivious to the resources to be kept simple?
Pretty much, yes.
> Did I not propose that way back?
Yes, I think you did.
> Was it not already rejected?
No, I don't think it was. In fact I don't think anything was really
rejected yet, we're still in the middle of a discussion.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
From: Tomi Valkeinen @ 2014-09-30 11:40 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
linux-fbdev
In-Reply-To: <1407329562-10832-1-git-send-email-daniel.vetter@ffwll.ch>
[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]
Hi,
On 06/08/14 15:52, Daniel Vetter wrote:
> In drm/i915 we want to get at the video= cmdline modes even when we
> don't have fbdev support enabled, so that users can always override
> the kernel's initial mode selection.
>
> But that gives us a direct depency upon the parsing code in the fbdev
> subsystem. Since it's so little code just extract these 2 functions
> and always build them in.
>
> Whiel at it fix the checkpatch fail in this code.
>
> v2: Also move fb_mode_option. Spotted by the kbuild.
>
> v3: Review from Geert:
> - Keep the old copyright notice from fb_mem.c, although I have no
> idea what exactly applies.
> - Only compile this when needed.
>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> --
>
> I prefer if we can merge this through drm-next since we'll use it
> there in follow-up patches.
> -Daniel
> ---
> drivers/video/fbdev/Kconfig | 4 ++
> drivers/video/fbdev/core/Makefile | 1 +
> drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
> drivers/video/fbdev/core/fbmem.c | 92 ----------------------------
> drivers/video/fbdev/core/modedb.c | 3 -
> 5 files changed, 115 insertions(+), 95 deletions(-)
> create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
Sorry for late response.
Looks fine for me, and I'm fine merging it via drm-next.
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
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: Hans de Goede @ 2014-09-30 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930045957.GA29874@ulmo>
Hi,
On 09/30/2014 06:59 AM, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
<snip>
>> But sure, you can still try to point new issues, get an obvious and
>> robust solution, and then discard the issue when the solution doesn't
>> go your way...
>
> And you've already proven that you're completely unwilling to even
> consider any other solution than what was originally proposed, so I
> really don't see how discussing this further with you is going to be
> productive.
That is not true, we have seriously considered various other alternatives,
as you know since you've participated in the discussion about them.
And we've found them all lacking, mostly because they are 10 times as
complicated.
You've made your point that you don't like this solution quite loudly
already, and we've all heard you. However you seem to be mostly alone in
this. Even the clk maintainer has said that what we want to do is
exactly how clocks are supposed to be used in dt.
If you don't like this no-one is forcing you to use the clocks property
in your own code. If it is not there, simplefb will behave exactly as
before.
Now since you're the only one very vocally against this, and a lot
of people are in favor of this and have a need for this, can we
please just get this merged and get this over with ?
Regards,
Hans
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-09-30 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <542A9771.2030401@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]
On Tue, Sep 30, 2014 at 01:43:45PM +0200, Hans de Goede wrote:
> Hi,
>
> On 09/30/2014 06:59 AM, Thierry Reding wrote:
> > On Mon, Sep 29, 2014 at 05:57:18PM +0200, Maxime Ripard wrote:
>
> <snip>
>
> >> But sure, you can still try to point new issues, get an obvious and
> >> robust solution, and then discard the issue when the solution doesn't
> >> go your way...
> >
> > And you've already proven that you're completely unwilling to even
> > consider any other solution than what was originally proposed, so I
> > really don't see how discussing this further with you is going to be
> > productive.
>
> That is not true, we have seriously considered various other alternatives,
> as you know since you've participated in the discussion about them.
>
> And we've found them all lacking, mostly because they are 10 times as
> complicated.
>
> You've made your point that you don't like this solution quite loudly
> already, and we've all heard you. However you seem to be mostly alone in
> this. Even the clk maintainer has said that what we want to do is
> exactly how clocks are supposed to be used in dt.
>
> If you don't like this no-one is forcing you to use the clocks property
> in your own code. If it is not there, simplefb will behave exactly as
> before.
>
> Now since you're the only one very vocally against this, and a lot
> of people are in favor of this and have a need for this, can we
> please just get this merged and get this over with ?
Whatever. I no longer care.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] video/fbdev: Always built-in video= cmdline parsing
From: Daniel Vetter @ 2014-09-30 11:53 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Plagniol-Villard, Intel Graphics Development, Geert Uytterhoeven,
Linux Fbdev development list, DRI Development
In-Reply-To: <542A96BF.7080303@ti.com>
On Tue, Sep 30, 2014 at 1:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 06/08/14 15:52, Daniel Vetter wrote:
>> In drm/i915 we want to get at the video= cmdline modes even when we
>> don't have fbdev support enabled, so that users can always override
>> the kernel's initial mode selection.
>>
>> But that gives us a direct depency upon the parsing code in the fbdev
>> subsystem. Since it's so little code just extract these 2 functions
>> and always build them in.
>>
>> Whiel at it fix the checkpatch fail in this code.
>>
>> v2: Also move fb_mode_option. Spotted by the kbuild.
>>
>> v3: Review from Geert:
>> - Keep the old copyright notice from fb_mem.c, although I have no
>> idea what exactly applies.
>> - Only compile this when needed.
>>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> --
>>
>> I prefer if we can merge this through drm-next since we'll use it
>> there in follow-up patches.
>> -Daniel
>> ---
>> drivers/video/fbdev/Kconfig | 4 ++
>> drivers/video/fbdev/core/Makefile | 1 +
>> drivers/video/fbdev/core/fb_cmdline.c | 110 ++++++++++++++++++++++++++++++++++
>> drivers/video/fbdev/core/fbmem.c | 92 ----------------------------
>> drivers/video/fbdev/core/modedb.c | 3 -
>> 5 files changed, 115 insertions(+), 95 deletions(-)
>> create mode 100644 drivers/video/fbdev/core/fb_cmdline.c
>
> Sorry for late response.
>
> Looks fine for me, and I'm fine merging it via drm-next.
>
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Well totally forgotten about this one here - I even forgot to drop the
"pls ack this" text from the commit message before sending the pull
request to Dave :(
Anyway, thanks for having a look.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Maxime Ripard @ 2014-09-30 12:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930085429.GB4059@ulmo>
[-- Attachment #1: Type: text/plain, Size: 5963 bytes --]
On Tue, Sep 30, 2014 at 10:54:32AM +0200, Thierry Reding wrote:
> On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
> > On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
> > > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
> > > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
> [...]
> > > > > What happened in the Snow example is that regulators that were
> > > > > previously on would all of a sudden be automatically disabled on boot
> > > > > because there was now a driver that registered them with a generic
> > > > > framework.
> > > > >
> > > > > The same thing is going to happen with simplefb for your device. If you
> > > > > later realize that you need a regulator to keep the panel going, you'll
> > > > > have to add code to your firmware to populate the corresponding
> > > > > properties, otherwise the regulator will end up unused and will be
> > > > > automatically disabled. At the same time you're going to break upstream
> > > > > for all users of your old firmware because it doesn't add that property
> > > > > yet.
> > > > >
> > > > > And the same will continue to happen for every new type of resource
> > > > > you're going to add.
> > > >
> > > > Sure, we can add any resources we will need. Regulators, reset lines,
> > > > pm domains, allocated memory, but I'm not really sure this is what you
> > > > want, right?
> > >
> > > No it's not what I want. *You* want to add resource management to this
> > > driver. What I'm saying is that if we start adding clocks then we can no
> > > longer say no to resets or regulators or power domains either.
> >
> > Yes, because resource management can be more than just "keep the thing
> > enabled". It might also be about not modifying anything, just like we
> > saw for the clocks, but that might also apply to regulators voltage.
>
> We've already determined that simplefb can't do anything meaningful with
> the resources other than keep them in the status quo. It simply doesn't
> have enough knowledge to do so. It doesn't know the exact pixel clock or
> what voltage the attached panel needs.
We do agree that it doesn't care, doesn't need to know it, or doesn't
need to do anything about it, but what it needs is that they stay the
same. That means both keeping a clock or a regulator enabled, but also
preventing any other user (direct, as in a shared regulator, or
indirect, as in two clocks sharing the same parent) to change that
voltage or pixel clock.
You were trying to address the former in your patch, but you
completely ignore the second one, which is just as important.
> > And the only way I can think of to deal with that properly is to have
> > resources management in the driver.
>
> My point is that if we had a proper way to tell the kernel not to do
> anything with resources owned by firmware, then the driver wouldn't
> have to do anything with the resources.
Yes, but at least for the clocks, and I guess it might be true in some
sick way for regulators too, the fact that it's a tree doesn't make
this easy at all. If they were completely independant clocks, yeah,
sure, we could do that. But it's almost never the case, and all clocks
share parents with other at some degree.
I guess you could do it using clock flags of some sort, but that would
require traversing the clock tree for almost any operation, which
doesn't look very reasonable.
> > > > I really start to consider adding a sunxi-uboot-fb, that would just
> > > > duplicate the source code of simplefb but also taking the
> > > > clocks. Somehow, I feel like it will be easier (and definitely less of
> > > > a hack than using the generic common clock API).
> > >
> > > You're not getting it are you? What makes you think sunxi-uboot-fb is
> > > going to be any different? This isn't about a name.
> >
> > At least, we would get to do any binding and resource management we
> > want. And that's a big win.
>
> So instead of trying to understand the concerns that I've expressed and
> constructively contribute to finding a solution that works for everybody
> you'd rather go and write a driver from scratch. Way to go.
Hey, you haven't really contributed either to finding a solution to
the fact that we need not only to prevent the clocks from being
touched during the framework initcall, but also from other related
users.
> I've already said that I'm not saying strictly no to these patches, but
> what I want to see happen is some constructive discussion about whether
> we can find better ways to do it. If we can't then I'm all for merging
> these patches. Fortunately other (sub)threads have been somewhat more
> constructive and actually come up with alternatives that should make
> everyone happier.
I didn't see where you said that you were not strictly against
them. But ok.
I guess your concerns all boil down to 1) do not break DT backward
compatibility, 2) do not break what the firmware has set up, even on
older firmwares.
We got 1 covered already, but in order to cover 2, I guess we would
need both our solutions, that I don't really see as orthogonal.
How would something like adding optional clocks property, and if
found, doing the regular clk_* calls, and if not found, relying on
your solution work for you?
> If you're going to do SoC-specific bindings and resource management
> you are in fact implementing what Grant suggested in a
> subthread. You're implementing a dummy driver only for resource
> management, which isn't really a bad thing. It can serve as a
> placeholder for now until you add the real driver. And you can also
> use the simplefb driver to provide the framebuffer.
That could be an option too, but I'd rather avoid it if possible.
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: Mark Brown @ 2014-09-30 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140929220250.GD5599@skynet.be>
[-- Attachment #1: Type: text/plain, Size: 553 bytes --]
On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote:
> 2) Simplefb will only have a single user: the rpi. As the only other
> users i can think of, which does not have a full driver and which does
> not have clocks automatically disabled, are discrete cards. And they do
> not really tend to happen with dt or platform devices.
I thought the goal was for other platforms to use simplefb while waiting
for the real drivers to be loaded (so you can get get console output as
early as possible from a built in driver for example)?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: jonsmirl @ 2014-09-30 12:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930124140.GC4273@sirena.org.uk>
On Tue, Sep 30, 2014 at 8:41 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Sep 30, 2014 at 12:02:50AM +0200, Luc Verhaegen wrote:
>
>> 2) Simplefb will only have a single user: the rpi. As the only other
>> users i can think of, which does not have a full driver and which does
>> not have clocks automatically disabled, are discrete cards. And they do
>> not really tend to happen with dt or platform devices.
>
> I thought the goal was for other platforms to use simplefb while waiting
> for the real drivers to be loaded (so you can get get console output as
> early as possible from a built in driver for example)?
That is an option that might work. Stop trying to make simplefb work
after the system is fully booted. Instead just let it run until the
clocks get shut off. That allows it to go back to being nothing but a
simple pointer to the video buffer.
Then if you want to keep your display going and don't have a KMS
driver written, whip together a device specific framebuffer driver for
your hardware that does the right thing with the clocks, etc. The
device specific framebuffer driver can load later in the boot process
so that it doesn't have to be built into the kernel. This device
specfic driver matches on a compatible string and knows what to do
with all of the device tree info.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Michal Suchanek @ 2014-09-30 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930113144.GA19694@ulmo>
On 30 September 2014 13:31, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Tue, Sep 30, 2014 at 11:38:50AM +0200, Michal Suchanek wrote:
>> On 30 September 2014 10:54, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > On Tue, Sep 30, 2014 at 09:52:58AM +0200, Maxime Ripard wrote:
>> >> On Tue, Sep 30, 2014 at 07:21:11AM +0200, Thierry Reding wrote:
>> >> > On Mon, Sep 29, 2014 at 06:28:14PM +0200, Maxime Ripard wrote:
>> >> > > On Mon, Sep 29, 2014 at 03:47:15PM +0200, Thierry Reding wrote:
>> > [...]
>> >> > > > What happened in the Snow example is that regulators that were
>> >> > > > previously on would all of a sudden be automatically disabled on boot
>> >> > > > because there was now a driver that registered them with a generic
>> >> > > > framework.
>> >> > > >
>> >> > > > The same thing is going to happen with simplefb for your device. If you
>> >> > > > later realize that you need a regulator to keep the panel going, you'll
>> >> > > > have to add code to your firmware to populate the corresponding
>> >> > > > properties, otherwise the regulator will end up unused and will be
>> >> > > > automatically disabled. At the same time you're going to break upstream
>> >> > > > for all users of your old firmware because it doesn't add that property
>> >> > > > yet.
>> >> > > >
>> >> > > > And the same will continue to happen for every new type of resource
>> >> > > > you're going to add.
>> >> > >
>> >> > > Sure, we can add any resources we will need. Regulators, reset lines,
>> >> > > pm domains, allocated memory, but I'm not really sure this is what you
>> >> > > want, right?
>> >> >
>> >> > No it's not what I want. *You* want to add resource management to this
>> >> > driver. What I'm saying is that if we start adding clocks then we can no
>> >> > longer say no to resets or regulators or power domains either.
>> >>
>> >> Yes, because resource management can be more than just "keep the thing
>> >> enabled". It might also be about not modifying anything, just like we
>> >> saw for the clocks, but that might also apply to regulators voltage.
>> >
>> > We've already determined that simplefb can't do anything meaningful with
>> > the resources other than keep them in the status quo. It simply doesn't
>> > have enough knowledge to do so. It doesn't know the exact pixel clock or
>> > what voltage the attached panel needs.
>> >
>> >> And the only way I can think of to deal with that properly is to have
>> >> resources management in the driver.
>> >
>> > My point is that if we had a proper way to tell the kernel not to do
>> > anything with resources owned by firmware, then the driver wouldn't
>> > have to do anything with the resources.
>>
>> The firmware on sunxi does not own any resources whatsoever. It ceases
>> running once it executes the kernel. This is different from ACPI and
>> UEFI where you have pieces of the firmware lingering indefinitely and
>> potentially getting invoked by user pressing some button or some other
>> hardware event. It is also different from rpi where the Linux kernel
>> effectively runs in a virtual environment created by the firmware
>> hypervisor.
>
> You know all that because you of course wrote every single firmware
> implementation that does and will ever exist for sunxi. There's nothing
> keeping anyone from running UEFI on a sunxi SoC.
The existing 'firmware' or rather loader for sunxi is u-boot.
I am not saying other solutions cannot exist. I am describing the
current situation.
>
>> So on sunxi and many other ARM machines the Linux kernel is the sole
>> owner of any resources that might happen to be available on the
>> machine. There is no firmware owning them when the Linux kernel is
>> running, ever.
>
> Of course this is part of the abstraction. The idea is that the device
> is a virtual one created by firmware. Therefore firmware owns the
> resources until the virtual device has been handed over to the kernel.
>
> If you're into splitting hairs, then the simplefb device shouldn't exist
> in the first place.
Why shoudn't it?
It is properly created by u-boot and handed over to the kernel with
all the required information for the kernel to keep it running or shut
it down as it sees fit.
>
>> And we do have a proper way to tell to the kernel what these resources
>> are used for - inserting description of them into the simplefb DT
>> node. Sure the simplefb cannot manage the resources in any way and but
>> it does own them. When it is running they are in use, when it stops
>> they are free to be reclaimed by the platform driver.
>
> Yes. And again I'm not saying anything different. What I'm saying is
> that we shouldn't need to know about the resources and instead hide that
> within the firmware, for the same reason that we're already hiding the
> register programming in hardware, namely to create an abstraction that
> works irrespective of the underlying hardware.
So then hide those resources in the Linux kernel. Because if you are
into hair splitting then on sunxi currently the Linux kernel is the
firmware and u-boot is only one of the loader stages that ultimately
executes the final firmware which is Linux.
>> >> > > I really start to consider adding a sunxi-uboot-fb, that would just
>> >> > > duplicate the source code of simplefb but also taking the
>> >> > > clocks. Somehow, I feel like it will be easier (and definitely less of
>> >> > > a hack than using the generic common clock API).
>> >> >
>> >> > You're not getting it are you? What makes you think sunxi-uboot-fb is
>> >> > going to be any different? This isn't about a name.
>> >>
>> >> At least, we would get to do any binding and resource management we
>> >> want. And that's a big win.
>> >
>> > So instead of trying to understand the concerns that I've expressed and
>> > constructively contribute to finding a solution that works for everybody
>> > you'd rather go and write a driver from scratch. Way to go.
>>
>> It's the constructive thing to do when the existing driver cannot be
>> extended to work for everybody.
>
> No, it isn't. If a generic driver doesn't work for everybody then it
> isn't generic and we should fix it. Not duplicate it and add platform
> specific quirks.
What was proposed originally is not platform specific quirk.
It was a generic solution what works on every platform when you can
provide a list of resources that are needed for simplefb to run.
It's not like sunxi is the only platform that has exposed display clocks, it it?
>
>> > I've already said that I'm not saying strictly no to these patches, but
>> > what I want to see happen is some constructive discussion about whether
>> > we can find better ways to do it. If we can't then I'm all for merging
>> > these patches. Fortunately other (sub)threads have been somewhat more
>> > constructive and actually come up with alternatives that should make
>> > everyone happier.
>>
>> What are those alternatives?
>
> Sorry, you've got to do some of the work yourself. They've been
> mentioned in this thread and the one that Maxime pointed to the other
> day.
I did not notice any in this thread, sorry.
>
>> > If you're going to do SoC-specific bindings and resource management you
>> > are in fact implementing what Grant suggested in a subthread. You're
>> > implementing a dummy driver only for resource management, which isn't
>> > really a bad thing. It can serve as a placeholder for now until you add
>> > the real driver. And you can also use the simplefb driver to provide
>> > the framebuffer.
>>
>> Oh, so back to the proposal to make a driver that claims the required
>> resources and then instantiates an unextended simplefb that is
>> oblivious to the resources to be kept simple?
>
> Pretty much, yes.
>
>> Did I not propose that way back?
>
> Yes, I think you did.
>
>> Was it not already rejected?
>
> No, I don't think it was. In fact I don't think anything was really
> rejected yet, we're still in the middle of a discussion.
>
> Thierry
You personally were against it ...
Can we agree on that as acceptable solution then?
Thanks
Michal
On 28 August 2014 12:08, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Wed, Aug 27, 2014 at 10:57:29PM +0200, Michal Suchanek wrote:
>> On 27 August 2014 17:42, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>> > So, how would the clock driver would know about which use case we're
>> > in? How would it know about which display engine is currently running?
>> > How would it know about which video output is being set?
>> >
>> > Currently, we have two separate display engines, which can each output
>> > either to 4 different outputs (HDMI, RGB/LVDS, 2 DSI). Each and every
>> > one of these combinations would require different clocks. What clocks
>> > will we put in the driver? All of them?
>> >
>>
>> since simplefb cannot be extended how about adding, say, dtfb which
>> claims the resources from dt and then instantiates a simplefb once the
>> resources are claimed? That is have a dtfb which has the clocks
>> assigned and has simplefb as child dt node.
>
> I don't see how that changes anything. All you do is add another layer
> of indirection. The fundamental problem remains the same and isn't
> solved.
>
> Thierry
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 4/4] simplefb: add clock handling code
From: Mark Brown @ 2014-09-30 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140930050923.GB29874@ulmo>
[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]
On Tue, Sep 30, 2014 at 07:09:24AM +0200, Thierry Reding wrote:
> On Mon, Sep 29, 2014 at 04:55:17PM +0100, Mark Brown wrote:
> > So long as we're ensuring that when we don't start supporting resources
> > without DT additions or at least require DT additions to actively manage
> > them (which can then include simplefb hookup) we should be fine.
> I'm not sure I understand what you mean. If we add a driver for the PMIC
> that exposes these regulators and then add a DT node for the PMIC, we'd
> still need to fix the firmware to generate the appropriate DT properties
> to allow simplefb to enable the regulators.
No, you don't. It's only if you start describing the regulators in the
PMIC in DT that you run into problems. Unconfigured regulators won't be
touched.
> So unless firmware is updated at the same time, regulators will get
> disabled because they are unused.
That won't happen unless the regulators are explicitly described, if
they are described as unused then this will of course happen.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox