* Re: [PATCH v3 00/14] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-08-14 19:10 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood, tomi.valkeinen, detheridge
In-Reply-To: <53EC8127.7090405@ti.com>
[-- Attachment #1: Type: text/plain, Size: 672 bytes --]
On Thu, Aug 14, 2014 at 12:28:07PM +0300, Jyri Sarha wrote:
> After discussing with Peter and Tomi I decided to change the approach a bit.
> There should not be any new device registered, just ASoC components that are
> implemented under sound/soc as a library rather than a device driver.
> The library would export a register and unregister functions to be called
> from video driver directory. The functions will then register the necessary
> ASoC components under the video device.
> If there are no objections I'll go ahead implementing this approach, first
> for OMAP HDMI audio, and later for SiI9022 driver we are cooking.
That also sounds perfectly sensible.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 3/4] simplefb: disable dt node upon remove
From: Henrik Nordström @ 2014-08-15 0:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4OypgToTSXxjShCt6uTGZBU69LpJxRwVoz=PnotpL=zm4w@mail.gmail.com>
tor 2014-08-14 klockan 08:07 -0400 skrev jonsmirl@gmail.com:
> So what are the rules going to be? Once the OS is up and starts
> changing things the state in the DT and the OS are going to diverge.
> How does this get handled for a kernel driver that can load/unload?
> Should the kernel remove those state nodes as soon as it alters the
> state?
hardware that is no longer there should also not be represented/active
in DT.
A simple fb that have been torn down is no more, no different from never
having been there. It can be argued that it never was in the first place
(i.e. that it is not actually hardware) but that it another can of worms
and both have their benefits and drawbacks. One thing is certain
however, as far as simplefb is concerned it is hardware, not really any
different from a persistent framebuffer with hardwired settings in
hardware.
Regarding kexec, it's the responsibility of the kernel doing kexec to
make sure DT and hardware matches for the next started kernel. With
hardware that can be reconfigured it is not safe to assume that the DT
can be passed as-is from before the kernel reconfigured hardware. If the
currently running kernel wants to set up a framebuffer for simplefb use
by the next kernel than it's free to do so, but in either case it needs
to provide the right information to next kernel.
If it was given a simplefb node, but then killed the simplefb then no
simplefb node should be provided to the next kernel. If it was not given
a simplefb node, but have configured the hardware suitable for simplefb
node then it may provide a simplefb node.
A UART that have changed rate no longer have that rate.
Same applies to numerous other items as well. If a kernel remaps IRQs,
bus addresses, or whatever in hardware aspects where such changes is
possible then DT needs to be updated as well. A bit less common than a
simplefb being destroyed by driver reconfiguring the framebuffer, but if
you look at it from a little distance then the problem is exactly the
same. Hardware is not 100% static, and therefore hardware description
also can not be 100% static. And hardware is getting more and more
dynamically reconfigurable.
A DT that contains false information is generally worse than no DT / not
having that information at all.
Regards
Henrik
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 3/4] simplefb: disable dt node upon remove
From: jonsmirl @ 2014-08-15 1:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1408063512.890.22.camel@localhost>
On Thu, Aug 14, 2014 at 8:45 PM, Henrik Nordström
<henrik@henriknordstrom.net> wrote:
> tor 2014-08-14 klockan 08:07 -0400 skrev jonsmirl@gmail.com:
>
>> So what are the rules going to be? Once the OS is up and starts
>> changing things the state in the DT and the OS are going to diverge.
>> How does this get handled for a kernel driver that can load/unload?
>> Should the kernel remove those state nodes as soon as it alters the
>> state?
>
> hardware that is no longer there should also not be represented/active
> in DT.
>
> A simple fb that have been torn down is no more, no different from never
> having been there. It can be argued that it never was in the first place
> (i.e. that it is not actually hardware) but that it another can of worms
> and both have their benefits and drawbacks. One thing is certain
> however, as far as simplefb is concerned it is hardware, not really any
> different from a persistent framebuffer with hardwired settings in
> hardware.
How does the kernel know what clocks to protect? A list in the
simplefb node? This list of clocks is a reason for adding simplefb to
the compatible list for the real hardware. That way it won't be
duplicated.
The issues with parameter conflicts between simplefb and the real
hardware can be dealt with. If the real hardware wants to add the
simplefb compatible field it will need to get its parameters sorted
out so that they don't conflict. Clock syntax is standardized in the
DTS so it shouldn't be a problem.
You can also argue that simplefb should never occur in a DTS file. It
is something that uboot will add. And then the kernel will remove when
simplefb loads KMS.
That logic should apply to all of these dynamic fields. You don't want
stale state data in the DT.
>
> Regarding kexec, it's the responsibility of the kernel doing kexec to
> make sure DT and hardware matches for the next started kernel. With
> hardware that can be reconfigured it is not safe to assume that the DT
> can be passed as-is from before the kernel reconfigured hardware. If the
> currently running kernel wants to set up a framebuffer for simplefb use
> by the next kernel than it's free to do so, but in either case it needs
> to provide the right information to next kernel.
>
> If it was given a simplefb node, but then killed the simplefb then no
> simplefb node should be provided to the next kernel. If it was not given
> a simplefb node, but have configured the hardware suitable for simplefb
> node then it may provide a simplefb node.
>
> A UART that have changed rate no longer have that rate.
>
> Same applies to numerous other items as well. If a kernel remaps IRQs,
> bus addresses, or whatever in hardware aspects where such changes is
> possible then DT needs to be updated as well. A bit less common than a
> simplefb being destroyed by driver reconfiguring the framebuffer, but if
> you look at it from a little distance then the problem is exactly the
> same. Hardware is not 100% static, and therefore hardware description
> also can not be 100% static. And hardware is getting more and more
> dynamically reconfigurable.
>
> A DT that contains false information is generally worse than no DT / not
> having that information at all.
>
> Regards
> Henrik
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH 3/4] simplefb: disable dt node upon remove
From: Maxime Ripard @ 2014-08-15 6:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKON4Oyp0qCucAhTNeXu7LELv_MRXPkYum+jOH2QADHb4Lw_HA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2493 bytes --]
On Thu, Aug 14, 2014 at 09:27:14PM -0400, jonsmirl@gmail.com wrote:
> On Thu, Aug 14, 2014 at 8:45 PM, Henrik Nordström
> <henrik@henriknordstrom.net> wrote:
> > tor 2014-08-14 klockan 08:07 -0400 skrev jonsmirl@gmail.com:
> >
> >> So what are the rules going to be? Once the OS is up and starts
> >> changing things the state in the DT and the OS are going to diverge.
> >> How does this get handled for a kernel driver that can load/unload?
> >> Should the kernel remove those state nodes as soon as it alters the
> >> state?
> >
> > hardware that is no longer there should also not be represented/active
> > in DT.
> >
> > A simple fb that have been torn down is no more, no different from never
> > having been there. It can be argued that it never was in the first place
> > (i.e. that it is not actually hardware) but that it another can of worms
> > and both have their benefits and drawbacks. One thing is certain
> > however, as far as simplefb is concerned it is hardware, not really any
> > different from a persistent framebuffer with hardwired settings in
> > hardware.
>
> How does the kernel know what clocks to protect? A list in the
> simplefb node? This list of clocks is a reason for adding simplefb to
> the compatible list for the real hardware. That way it won't be
> duplicated.
>
> The issues with parameter conflicts between simplefb and the real
> hardware can be dealt with. If the real hardware wants to add the
> simplefb compatible field it will need to get its parameters sorted
> out so that they don't conflict. Clock syntax is standardized in the
> DTS so it shouldn't be a problem.
It shouldn't be, but apparently, some disagree.
Anyway, I'm not sure having the simplefb compatible would work in this
use-case, mainly for two reasons:
- Most likely, the bindings are going to be very different. Not only
about which properties you'll have, but also what you will place
in these properties. reg for example have the memory address of
the buffer in the simplefb case, while in the KMS driver case, it
would have the memory address of the registers.
- There's will be a single driver probed. So if you want to go down
the hand over road (which is a bit premature at this point if you
ask me, but anyway), you will have no driver probed to hand over
to.
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 3/4] simplefb: disable dt node upon remove
From: jonsmirl @ 2014-08-15 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140815064336.GX15297@lukather>
On Fri, Aug 15, 2014 at 2:43 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Thu, Aug 14, 2014 at 09:27:14PM -0400, jonsmirl@gmail.com wrote:
>> On Thu, Aug 14, 2014 at 8:45 PM, Henrik Nordström
>> <henrik@henriknordstrom.net> wrote:
>> > tor 2014-08-14 klockan 08:07 -0400 skrev jonsmirl@gmail.com:
>> >
>> >> So what are the rules going to be? Once the OS is up and starts
>> >> changing things the state in the DT and the OS are going to diverge.
>> >> How does this get handled for a kernel driver that can load/unload?
>> >> Should the kernel remove those state nodes as soon as it alters the
>> >> state?
>> >
>> > hardware that is no longer there should also not be represented/active
>> > in DT.
>> >
>> > A simple fb that have been torn down is no more, no different from never
>> > having been there. It can be argued that it never was in the first place
>> > (i.e. that it is not actually hardware) but that it another can of worms
>> > and both have their benefits and drawbacks. One thing is certain
>> > however, as far as simplefb is concerned it is hardware, not really any
>> > different from a persistent framebuffer with hardwired settings in
>> > hardware.
>>
>> How does the kernel know what clocks to protect? A list in the
>> simplefb node? This list of clocks is a reason for adding simplefb to
>> the compatible list for the real hardware. That way it won't be
>> duplicated.
>>
>> The issues with parameter conflicts between simplefb and the real
>> hardware can be dealt with. If the real hardware wants to add the
>> simplefb compatible field it will need to get its parameters sorted
>> out so that they don't conflict. Clock syntax is standardized in the
>> DTS so it shouldn't be a problem.
>
> It shouldn't be, but apparently, some disagree.
>
> Anyway, I'm not sure having the simplefb compatible would work in this
> use-case, mainly for two reasons:
> - Most likely, the bindings are going to be very different. Not only
> about which properties you'll have, but also what you will place
> in these properties. reg for example have the memory address of
> the buffer in the simplefb case, while in the KMS driver case, it
> would have the memory address of the registers.
> - There's will be a single driver probed. So if you want to go down
> the hand over road (which is a bit premature at this point if you
> ask me, but anyway), you will have no driver probed to hand over
> to.
The graphic drivers have already been down this road before. Simplefb
should be a driver library not a driver. DRM is also a driver library.
So to implement simplefb whip up a real skeleton driver for the video
hardware that parses the DT and then pokes the appropriate info into
the simplefb library. Initially that driver will just include the
calls out to the simplefb library. Later on it can get KMS
implemented. This real driver will get bound to the hardware and have
a dependency that loads the simplefb driver library.
Now there is no handover problem. Only a single driver is ever
attached to the hardware. When it wants to go into KMS mode it makes a
call into the simplefb library to shut down whatever it is doing.
But you still need a mechanism for uboot to signal that it has set up
simplefb. Adding a dynamic node like this might work...
video0: video@01c21800 {
compatible = "allwinner,sun4i-a10-video";
clocks = <&apb0_gates 6>, <&ir0_clk>;
clock-names = "apb", "video";
interrupts = <0 5 4>;
reg = <0x01c21800 0x40>;
status = "enabled";
simplefb {
new namespace for simplefb parameters
}
};
I would have suggested a driver library earlier but it has been a
while since I worked on DRM and I forgot about it.
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] powerpc: Fix build error for vga16fb
From: Pranith Kumar @ 2014-08-18 21:13 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Arnd Bergmann,
Peter Griffin, Alexander Shiyan, Jean Delvare, Kukjin Kim,
Mark Brown, open list:FRAMEBUFFER LAYER, open list
Cc: geert, Andrew Morton
In-Reply-To: <1408396422-32658-1-git-send-email-bobby.prani@gmail.com>
Fix a build error caused by
drivers/built-in.o: In function `vga16fb_probe':
drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
make: *** [vmlinux] Error 1
This is occuring as vgacon_remap_base is exported only when
CONFIG_VGA_CONSOLE=y. Fix the error by making FB_VBA16 depend on
CONFIG_VGA_CONSOLE.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index e911b9c..09df564 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -547,7 +547,7 @@ config FB_IMSTT
config FB_VGA16
tristate "VGA 16-color graphics support"
- depends on FB && (X86 || PPC)
+ depends on FB && (X86 || PPC) && CONFIG_VGA_CONSOLE
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] powerpc: Fix build error for vga16fb
From: Geert Uytterhoeven @ 2014-08-18 21:28 UTC (permalink / raw)
To: Pranith Kumar
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Arnd Bergmann,
Peter Griffin, Alexander Shiyan, Jean Delvare, Kukjin Kim,
Mark Brown, open list:FRAMEBUFFER LAYER, open list, Andrew Morton
In-Reply-To: <1408396422-32658-2-git-send-email-bobby.prani@gmail.com>
Hi Pranith,
On Mon, Aug 18, 2014 at 11:13 PM, Pranith Kumar <bobby.prani@gmail.com> wrote:
> Fix a build error caused by
>
> drivers/built-in.o: In function `vga16fb_probe':
> drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
> drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
> make: *** [vmlinux] Error 1
>
> This is occuring as vgacon_remap_base is exported only when
> CONFIG_VGA_CONSOLE=y. Fix the error by making FB_VBA16 depend on
> CONFIG_VGA_CONSOLE.
>
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> CC: Andrew Morton <akpm@linux-foundation.org>
> ---
> drivers/video/fbdev/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index e911b9c..09df564 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -547,7 +547,7 @@ config FB_IMSTT
>
> config FB_VGA16
> tristate "VGA 16-color graphics support"
> - depends on FB && (X86 || PPC)
> + depends on FB && (X86 || PPC) && CONFIG_VGA_CONSOLE
The dependency on CONFIG_VGA_CONSOLE only exists on PPC, not on x86.
So this is not correct.
Digging a bit deeper: There's no code left in arch/powerpc that actually
sets vgacon_remap_base, so it's not gonna work anyway.
It seems vga16 support on PPC died with the removal of arch/ppc/.
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] powerpc: Fix build error for vga16fb
From: Pranith Kumar @ 2014-08-18 21:34 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Arnd Bergmann,
Peter Griffin, Alexander Shiyan, Jean Delvare, Kukjin Kim,
Mark Brown, open list:FRAMEBUFFER LAYER, open list, Andrew Morton
In-Reply-To: <CAMuHMdV_vAPCPh1JdCxtf0iz6haHULUV8t0oYt0jrEAky2WSLw@mail.gmail.com>
On Mon, Aug 18, 2014 at 5:28 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Digging a bit deeper: There's no code left in arch/powerpc that actually
> sets vgacon_remap_base, so it's not gonna work anyway.
> It seems vga16 support on PPC died with the removal of arch/ppc/.
OK. I will remove the dependency on PPC for vga16fb.
--
Pranith
^ permalink raw reply
* [PATCH] powerpc: Fix build error caused by vga16fb
From: Pranith Kumar @ 2014-08-18 21:40 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Arnd Bergmann,
Peter Griffin, Jean Delvare, Jingoo Han, Alexander Shiyan,
Kukjin Kim, Pawel Moll, open list:FRAMEBUFFER LAYER, open list
Cc: geert, Andrew Morton
vga16fb causes a build failure on PPC 32 as follows
drivers/built-in.o: In function `vga16fb_probe':
drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
drivers/video/fbdev/vga16fb.c:1326: undefined reference to `vgacon_remap_base'
make: *** [vmlinux] Error 1
It was noted by Geert that in PPC, no one is setting vgacon_remap_base because
of which it working on PPC is suspect.
Fix the build failure by disabling vga16fb on PPC.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
drivers/video/fbdev/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index e911b9c..d236fe4 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -547,7 +547,7 @@ config FB_IMSTT
config FB_VGA16
tristate "VGA 16-color graphics support"
- depends on FB && (X86 || PPC)
+ depends on FB && X86
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.9.1
^ permalink raw reply related
* [PATCH] video: ARM CLCD: Ensure bits-per-pixel is a power of 2 and <= 32
From: Jon Medhurst (Tixy) @ 2014-08-19 13:07 UTC (permalink / raw)
To: linux-arm-kernel
If the device-tree specifies a max-memory-bandwidth property then
the CLCD driver uses that to calculate the bits-per-pixel supported,
however, it doesn't ensure that the result is a sane value, i.e. a
power of 2 and <= 32 as the rest of the code assumes.
Acked-by: Pawel Moll <pawel.moll@arm.com>
Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
This fixes code which is new in 3.17 (commit d10715be03) and so I assume
is a candidate for adding to a coming -rc ? Without the fix, people can
be left (as I was) with a blank non-functioning screen even if they
create a valid device-tree for the new driver functionality.
drivers/video/fbdev/amba-clcd.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index beadd3e..98b66b7 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -24,6 +24,7 @@
#include <linux/list.h>
#include <linux/amba/bus.h>
#include <linux/amba/clcd.h>
+#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/hardirq.h>
#include <linux/dma-mapping.h>
@@ -650,6 +651,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
{
struct device_node *endpoint;
int err;
+ int bpp;
u32 max_bandwidth;
u32 tft_r0b0g0[3];
@@ -667,11 +669,15 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
err = of_property_read_u32(fb->dev->dev.of_node, "max-memory-bandwidth",
&max_bandwidth);
- if (!err)
- fb->panel->bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
+ if (!err) {
+ bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
fb->panel->mode.yres * fb->panel->mode.refresh);
- else
- fb->panel->bpp = 32;
+ bpp = rounddown_pow_of_two(bpp);
+ if (bpp > 32)
+ bpp = 32;
+ } else
+ bpp = 32;
+ fb->panel->bpp = bpp;
#ifdef CONFIG_CPU_BIG_ENDIAN
fb->panel->cntl |= CNTL_BEBO;
--
2.0.1
^ permalink raw reply related
* Re: [PATCH] video: ARM CLCD: Ensure bits-per-pixel is a power of 2 and <= 32
From: Russell King - ARM Linux @ 2014-08-19 14:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1408453651.3746.41.camel@linaro1.home>
On Tue, Aug 19, 2014 at 02:07:31PM +0100, Jon Medhurst (Tixy) wrote:
> If the device-tree specifies a max-memory-bandwidth property then
> the CLCD driver uses that to calculate the bits-per-pixel supported,
> however, it doesn't ensure that the result is a sane value, i.e. a
> power of 2 and <= 32 as the rest of the code assumes.
>
> Acked-by: Pawel Moll <pawel.moll@arm.com>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
>
> This fixes code which is new in 3.17 (commit d10715be03) and so I assume
> is a candidate for adding to a coming -rc ? Without the fix, people can
> be left (as I was) with a blank non-functioning screen even if they
> create a valid device-tree for the new driver functionality.
>
> drivers/video/fbdev/amba-clcd.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
> index beadd3e..98b66b7 100644
> --- a/drivers/video/fbdev/amba-clcd.c
> +++ b/drivers/video/fbdev/amba-clcd.c
> @@ -24,6 +24,7 @@
> #include <linux/list.h>
> #include <linux/amba/bus.h>
> #include <linux/amba/clcd.h>
> +#include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/hardirq.h>
> #include <linux/dma-mapping.h>
> @@ -650,6 +651,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
> {
> struct device_node *endpoint;
> int err;
> + int bpp;
> u32 max_bandwidth;
> u32 tft_r0b0g0[3];
>
> @@ -667,11 +669,15 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
>
> err = of_property_read_u32(fb->dev->dev.of_node, "max-memory-bandwidth",
> &max_bandwidth);
> - if (!err)
> - fb->panel->bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
> + if (!err) {
> + bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
> fb->panel->mode.yres * fb->panel->mode.refresh);
This calculation is wrong in any case - this is the naïeve calculation
which assumes that the bandwidth is:
x * y * (bpp / 8) * refresh
That isn't the maximum bandwidth, it's the average bandwidth across a
full frame.
If we're interested in limiting the maximum bandwidth, because the
hardware can't cope with fetching the data above a certain rate, then
we need a different method.
We know the pixel rate. We know how many memory bits are fetched for
each pixel. So:
peak_bandwidth = pixel_clock * bpp / 8
Let's take 32bpp, 1024x768@60Hz, which has a pixel clock of 65MHz. Using
the first, we get:
1024 * 768 * 4 * 60 = 180MiB/s.
Using the second:
65 * 4 = 248MiB/s.
To see why there's this discrepency, realise that there's 320 clocks where
there's no pixel activity (so no memory fetches) per line, and 38 lines
where there are no memory fetches. That's 320 * 768 + 1344 * 38 = 296832
pixel clocks where there's no memory fetches, out of a total of 1344 * 806
= 1083264 total pixel clocks in a frame. So, that's about 27% of the frame
where the memory subsystem is inactive - which accounts for the difference
in the above figure.
So, if you are limiting the bandwidth of the mode due to the available bus
bandwidth to the controller (which is implied by the property name), then
you want to use the second equation to limit the peak bandwidth, and not
the average bandwidth.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [RFC] fbdev: mxsfb: Add support for mx6sl and mx6sx
From: Fabio Estevam @ 2014-08-19 18:23 UTC (permalink / raw)
To: linux-fbdev
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.
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;
+ }
+}
+
+/* Enable DISP axi clock */
+static inline void clk_enable_disp_axi(struct mxsfb_info *host)
+{
+ if (!host->clk_disp_axi_enabled && (!IS_ERR(host->clk_disp_axi))) {
+ clk_prepare_enable(host->clk_disp_axi);
+ host->clk_disp_axi_enabled = true;
+ }
+}
+
+/* Disable DISP axi clock */
+static inline void clk_disable_disp_axi(struct mxsfb_info *host)
+{
+ if (host->clk_disp_axi_enabled && (!IS_ERR(host->clk_disp_axi))) {
+ clk_disable_unprepare(host->clk_disp_axi);
+ host->clk_disp_axi_enabled = false;
+ }
+}
+
static const struct mxsfb_devdata mxsfb_devdata[] = {
[MXSFB_V3] = {
.transfer_count = LCDC_V3_TRANSFER_COUNT,
@@ -331,6 +371,8 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
}
}
+ clk_enable_axi(host);
+ clk_enable_disp_axi(host);
clk_prepare_enable(host->clk);
clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
@@ -374,6 +416,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
clk_disable_unprepare(host->clk);
+ clk_disable_disp_axi(host);
+ clk_disable_axi(host);
host->enabled = 0;
@@ -867,6 +911,9 @@ static int mxsfb_probe(struct platform_device *pdev)
goto fb_release;
}
+ host->clk_axi = devm_clk_get(&host->pdev->dev, "axi");
+ host->clk_disp_axi = devm_clk_get(&host->pdev->dev, "disp_axi");
+
host->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
if (IS_ERR(host->reg_lcd))
host->reg_lcd = NULL;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] video: ARM CLCD: Ensure bits-per-pixel is a power of 2 and <= 32
From: Jon Medhurst (Tixy) @ 2014-08-20 8:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140819144032.GD30401@n2100.arm.linux.org.uk>
On Tue, 2014-08-19 at 15:40 +0100, Russell King - ARM Linux wrote:
> On Tue, Aug 19, 2014 at 02:07:31PM +0100, Jon Medhurst (Tixy) wrote:
> > If the device-tree specifies a max-memory-bandwidth property then
> > the CLCD driver uses that to calculate the bits-per-pixel supported,
> > however, it doesn't ensure that the result is a sane value, i.e. a
> > power of 2 and <= 32 as the rest of the code assumes.
> >
> > Acked-by: Pawel Moll <pawel.moll@arm.com>
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > ---
> >
> > This fixes code which is new in 3.17 (commit d10715be03) and so I assume
> > is a candidate for adding to a coming -rc ? Without the fix, people can
> > be left (as I was) with a blank non-functioning screen even if they
> > create a valid device-tree for the new driver functionality.
> >
> > drivers/video/fbdev/amba-clcd.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
> > index beadd3e..98b66b7 100644
> > --- a/drivers/video/fbdev/amba-clcd.c
> > +++ b/drivers/video/fbdev/amba-clcd.c
> > @@ -24,6 +24,7 @@
> > #include <linux/list.h>
> > #include <linux/amba/bus.h>
> > #include <linux/amba/clcd.h>
> > +#include <linux/bitops.h>
> > #include <linux/clk.h>
> > #include <linux/hardirq.h>
> > #include <linux/dma-mapping.h>
> > @@ -650,6 +651,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
> > {
> > struct device_node *endpoint;
> > int err;
> > + int bpp;
> > u32 max_bandwidth;
> > u32 tft_r0b0g0[3];
> >
> > @@ -667,11 +669,15 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
> >
> > err = of_property_read_u32(fb->dev->dev.of_node, "max-memory-bandwidth",
> > &max_bandwidth);
> > - if (!err)
> > - fb->panel->bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
> > + if (!err) {
> > + bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
> > fb->panel->mode.yres * fb->panel->mode.refresh);
>
> This calculation is wrong in any case - this is the naïeve calculation
> which assumes that the bandwidth is:
>
> x * y * (bpp / 8) * refresh
>
> That isn't the maximum bandwidth, it's the average bandwidth across a
> full frame.
>
> If we're interested in limiting the maximum bandwidth, because the
> hardware can't cope with fetching the data above a certain rate, then
> we need a different method.
>
> We know the pixel rate. We know how many memory bits are fetched for
> each pixel. So:
>
> peak_bandwidth = pixel_clock * bpp / 8
That all sounds logical and reasonable to me, especially as the binding
doc describes the property as "maximum bandwidth in bytes per second
that the cell's memory interface can handle".
I'll update the patch to also correct the calculation as you suggest.
--
Tixy
^ permalink raw reply
* [PATCH v2] video: ARM CLCD: Fix calculation of bits-per-pixel
From: Jon Medhurst (Tixy) @ 2014-08-20 12:41 UTC (permalink / raw)
To: linux-arm-kernel
If the device-tree specifies a max-memory-bandwidth property then the
CLCD driver uses that to calculate the bits-per-pixel supported,
however, this calculation is faulty for two reasons.
1. It doesn't ensure that the result is a sane value, i.e. a power of 2
and <= 32 as the rest of the code assumes.
2. It uses the displayed resolution and calculates the average bandwidth
across the whole frame. It should instead calculate the peak
bandwidth based on the pixel clock.
This patch fixes both the above.
Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
Changes since v1:
- Changed formula for calculating bits per pixel
- Changed commit message and subject, was:
"video: ARM CLCD: Ensure bits-per-pixel is a power of 2 and <= 32"
drivers/video/fbdev/amba-clcd.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index beadd3e..a7b6217 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -24,6 +24,7 @@
#include <linux/list.h>
#include <linux/amba/bus.h>
#include <linux/amba/clcd.h>
+#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/hardirq.h>
#include <linux/dma-mapping.h>
@@ -650,6 +651,7 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
{
struct device_node *endpoint;
int err;
+ unsigned int bpp;
u32 max_bandwidth;
u32 tft_r0b0g0[3];
@@ -667,11 +669,22 @@ static int clcdfb_of_init_display(struct clcd_fb *fb)
err = of_property_read_u32(fb->dev->dev.of_node, "max-memory-bandwidth",
&max_bandwidth);
- if (!err)
- fb->panel->bpp = 8 * max_bandwidth / (fb->panel->mode.xres *
- fb->panel->mode.yres * fb->panel->mode.refresh);
- else
- fb->panel->bpp = 32;
+ if (!err) {
+ /*
+ * max_bandwidth is in bytes per second and pixclock in
+ * pico-seconds, so the maximum allowed bits per pixel is
+ * 8 * max_bandwidth / (PICOS2KHZ(pixclock) * 1000)
+ * Rearrange this calculation to avoid overflow and then ensure
+ * result is a valid format.
+ */
+ bpp = max_bandwidth / (1000 / 8)
+ / PICOS2KHZ(fb->panel->mode.pixclock);
+ bpp = rounddown_pow_of_two(bpp);
+ if (bpp > 32)
+ bpp = 32;
+ } else
+ bpp = 32;
+ fb->panel->bpp = bpp;
#ifdef CONFIG_CPU_BIG_ENDIAN
fb->panel->cntl |= CNTL_BEBO;
--
2.1.0.rc1
^ permalink raw reply related
* Re: [PATCH v2] video: ARM CLCD: Fix calculation of bits-per-pixel
From: Pawel Moll @ 2014-08-20 13:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1408538464.3858.43.camel@linaro1.home>
On Wed, 2014-08-20 at 13:41 +0100, Jon Medhurst (Tixy) wrote:
> If the device-tree specifies a max-memory-bandwidth property then the
> CLCD driver uses that to calculate the bits-per-pixel supported,
> however, this calculation is faulty for two reasons.
>
> 1. It doesn't ensure that the result is a sane value, i.e. a power of 2
> and <= 32 as the rest of the code assumes.
>
> 2. It uses the displayed resolution and calculates the average bandwidth
> across the whole frame. It should instead calculate the peak
> bandwidth based on the pixel clock.
>
> This patch fixes both the above.
>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
Acked-by: Pawel Moll <pawel.moll@arm.com>
^ permalink raw reply
* [PATCH] fbdev: Remove __init from chips_hw_init() to fix build failure
From: Pranith Kumar @ 2014-08-21 12:23 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Rob Clark,
Jingoo Han, Laurent Pinchart, Daniel Vetter,
open list:FRAMEBUFFER LAYER, open list
Fix build failure caused as follows:
The function chipsfb_pci_init() references
the function __init chips_hw_init().
This is often because chipsfb_pci_init lacks a __init
annotation or the annotation of chips_hw_init is wrong.
make: *** [drivers] Error 2
by removing the __init annotation from chips_hw_init(). The other thing that
could have been done was annotating chipsfb_pci_init(). But that cannot be done
since chipsfb_pci_init() is called from non __init functions.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
drivers/video/fbdev/chipsfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 206a66b..59abdc6 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -273,7 +273,7 @@ static struct chips_init_reg chips_init_xr[] = {
{ 0xa8, 0x00 }
};
-static void __init chips_hw_init(void)
+static void chips_hw_init(void)
{
int i;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v7][ 3/5] video: mx3fb: Introduce regulator support.
From: Mark Brown @ 2014-08-22 22:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5397083D.4090400@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]
On Tue, Jun 10, 2014 at 03:29:33PM +0200, Denis Carikli wrote:
> On 03/17/2014 07:20 AM, Sascha Hauer wrote:
> >This patch adds regulator support for the display of a i.MX3 IPU. The
> >problem Denis has to solve here is that he needs to get the regulator,
> >but the display devicenode doesn't have a struct device associated with
> >it, so he cannot provide one to regulator_get(). One way out here could
> >be a of_regulator_get(struct device_node *). Mark, would this be ok with
> >you?
> Here, the display devicenode has no struct device associated with it like
> mentioned above.
> Because of that, retriving the regulator from the devicetree, for instance
> like regulator_dev_lookup() does, would require a different approach.
> As I understand it, what happen in regulator_dev_lookup when
> regulator_get(NULL, regulator_name) is used is the following:
> Since the struct device is NULL, it skips the struct device based lookup,
> but also the devicetree lookup (it uses dev->of_node for that)
> At the end it falls back on a match on the regulator name to find it.
> would keeping regulator_get(NULL, regulator_name); in the driver instead be
> better for now?
It would be much better to have an actual device to get things for,
using a hard coded regulator name is very much deprecated. Using a hard
coded name is fragile and we don't have support for mapping this
properly in the device tree.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 8/9 v2] video: fbdev: aty: use c99 initializers in structures
From: Julia Lawall @ 2014-08-23 15:50 UTC (permalink / raw)
To: Josh Triplett
Cc: Paul Mackerras, kernel-janitors, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <20140823152015.GB2074@thin>
From nobody Sat Aug 23 12:48:19 CEST 2014
From: Julia Lawall <Julia.Lawall@lip6.fr>
To: Paul Mackerras <paulus@samba.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,Tomi Valkeinen <tomi.valkeinen@ti.com>,linux-fbdev@vger.kernel.org,linux-kernel@vger.kernel.org
Subject: [PATCH 8/9] video: fbdev: aty: use c99 initializers in structures
From: Julia Lawall <Julia.Lawall@lip6.fr>
Use c99 initializers for structures.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@decl@
identifier i1,fld;
type T;
field list[n] fs;
@@
struct i1 {
fs
T fld;
...};
@bad@
identifier decl.i1,i2;
expression e;
initializer list[decl.n] is;
@@
struct i1 i2 = { is,
+ .fld = e
- e
,...};
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The patches in this series do not depend on each other.
v2: Move close braces down to the next line and add trailing commas, as
suggested by Josh Triplett.
diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
index ff60701..aedf2fb 100644
--- a/drivers/video/fbdev/aty/aty128fb.c
+++ b/drivers/video/fbdev/aty/aty128fb.c
@@ -324,14 +324,61 @@ struct aty128_meminfo {
};
/* various memory configurations */
-static const struct aty128_meminfo sdr_128 - { 4, 4, 3, 3, 1, 3, 1, 16, 30, 16, "128-bit SDR SGRAM (1:1)" };
-static const struct aty128_meminfo sdr_64 - { 4, 8, 3, 3, 1, 3, 1, 17, 46, 17, "64-bit SDR SGRAM (1:1)" };
-static const struct aty128_meminfo sdr_sgram - { 4, 4, 1, 2, 1, 2, 1, 16, 24, 16, "64-bit SDR SGRAM (2:1)" };
-static const struct aty128_meminfo ddr_sgram - { 4, 4, 3, 3, 2, 3, 1, 16, 31, 16, "64-bit DDR SGRAM" };
+static const struct aty128_meminfo sdr_128 = {
+ .ML = 4,
+ .MB = 4,
+ .Trcd = 3,
+ .Trp = 3,
+ .Twr = 1,
+ .CL = 3,
+ .Tr2w = 1,
+ .LoopLatency = 16,
+ .DspOn = 30,
+ .Rloop = 16,
+ .name = "128-bit SDR SGRAM (1:1)",
+};
+
+static const struct aty128_meminfo sdr_64 = {
+ .ML = 4,
+ .MB = 8,
+ .Trcd = 3,
+ .Trp = 3,
+ .Twr = 1,
+ .CL = 3,
+ .Tr2w = 1,
+ .LoopLatency = 17,
+ .DspOn = 46,
+ .Rloop = 17,
+ .name = "64-bit SDR SGRAM (1:1)",
+};
+
+static const struct aty128_meminfo sdr_sgram = {
+ .ML = 4,
+ .MB = 4,
+ .Trcd = 1,
+ .Trp = 2,
+ .Twr = 1,
+ .CL = 2,
+ .Tr2w = 1,
+ .LoopLatency = 16,
+ .DspOn = 24,
+ .Rloop = 16,
+ .name = "64-bit SDR SGRAM (2:1)",
+};
+
+static const struct aty128_meminfo ddr_sgram = {
+ .ML = 4,
+ .MB = 4,
+ .Trcd = 3,
+ .Trp = 3,
+ .Twr = 2,
+ .CL = 3,
+ .Tr2w = 1,
+ .LoopLatency = 16,
+ .DspOn = 31,
+ .Rloop = 16,
+ .name = "64-bit DDR SGRAM",
+};
static struct fb_fix_screeninfo aty128fb_fix = {
.id = "ATY Rage128",
^ permalink raw reply related
* Re: [PATCH 8/9 v2] video: fbdev: aty: use c99 initializers in structures
From: Josh Triplett @ 2014-08-23 16:03 UTC (permalink / raw)
To: Julia Lawall
Cc: Paul Mackerras, kernel-janitors, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.02.1408231749030.2068@localhost6.localdomain6>
On Sat, Aug 23, 2014 at 05:50:28PM +0200, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Use c99 initializers for structures.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @decl@
> identifier i1,fld;
> type T;
> field list[n] fs;
> @@
>
> struct i1 {
> fs
> T fld;
> ...};
>
> @bad@
> identifier decl.i1,i2;
> expression e;
> initializer list[decl.n] is;
> @@
>
> struct i1 i2 = { is,
> + .fld = e
> - e
> ,...};
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> v2: Move close braces down to the next line and add trailing commas, as
> suggested by Josh Triplett.
>
> diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
> index ff60701..aedf2fb 100644
> --- a/drivers/video/fbdev/aty/aty128fb.c
> +++ b/drivers/video/fbdev/aty/aty128fb.c
> @@ -324,14 +324,61 @@ struct aty128_meminfo {
> };
>
> /* various memory configurations */
> -static const struct aty128_meminfo sdr_128 > - { 4, 4, 3, 3, 1, 3, 1, 16, 30, 16, "128-bit SDR SGRAM (1:1)" };
> -static const struct aty128_meminfo sdr_64 > - { 4, 8, 3, 3, 1, 3, 1, 17, 46, 17, "64-bit SDR SGRAM (1:1)" };
> -static const struct aty128_meminfo sdr_sgram > - { 4, 4, 1, 2, 1, 2, 1, 16, 24, 16, "64-bit SDR SGRAM (2:1)" };
> -static const struct aty128_meminfo ddr_sgram > - { 4, 4, 3, 3, 2, 3, 1, 16, 31, 16, "64-bit DDR SGRAM" };
> +static const struct aty128_meminfo sdr_128 = {
> + .ML = 4,
> + .MB = 4,
> + .Trcd = 3,
> + .Trp = 3,
> + .Twr = 1,
> + .CL = 3,
> + .Tr2w = 1,
> + .LoopLatency = 16,
> + .DspOn = 30,
> + .Rloop = 16,
> + .name = "128-bit SDR SGRAM (1:1)",
> +};
> +
> +static const struct aty128_meminfo sdr_64 = {
> + .ML = 4,
> + .MB = 8,
> + .Trcd = 3,
> + .Trp = 3,
> + .Twr = 1,
> + .CL = 3,
> + .Tr2w = 1,
> + .LoopLatency = 17,
> + .DspOn = 46,
> + .Rloop = 17,
> + .name = "64-bit SDR SGRAM (1:1)",
> +};
> +
> +static const struct aty128_meminfo sdr_sgram = {
> + .ML = 4,
> + .MB = 4,
> + .Trcd = 1,
> + .Trp = 2,
> + .Twr = 1,
> + .CL = 2,
> + .Tr2w = 1,
> + .LoopLatency = 16,
> + .DspOn = 24,
> + .Rloop = 16,
> + .name = "64-bit SDR SGRAM (2:1)",
> +};
> +
> +static const struct aty128_meminfo ddr_sgram = {
> + .ML = 4,
> + .MB = 4,
> + .Trcd = 3,
> + .Trp = 3,
> + .Twr = 2,
> + .CL = 3,
> + .Tr2w = 1,
> + .LoopLatency = 16,
> + .DspOn = 31,
> + .Rloop = 16,
> + .name = "64-bit DDR SGRAM",
> +};
>
> static struct fb_fix_screeninfo aty128fb_fix = {
> .id = "ATY Rage128",
^ permalink raw reply
* [PATCH 2/7] video: fbdev: intelfb: delete double assignment
From: Julia Lawall @ 2014-08-23 18:33 UTC (permalink / raw)
To: Maik Broemme
Cc: joe, kernel-janitors, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1408818808-18850-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Delete successive assignments to the same location.
In the second case, = is converted to |=, which looks appropriate based on
the values involved.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression i;
@@
*i = ...;
i = ...;
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The patches in this series do not depend on each other.
This changes the behavior of the code in the second case and is not tested.
drivers/video/fbdev/intelfb/intelfbhw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbhw.c b/drivers/video/fbdev/intelfb/intelfbhw.c
index fbad61d..d31ed4e 100644
--- a/drivers/video/fbdev/intelfb/intelfbhw.c
+++ b/drivers/video/fbdev/intelfb/intelfbhw.c
@@ -1191,7 +1191,6 @@ int intelfbhw_mode_to_hw(struct intelfb_info *dinfo,
vsync_end = vsync_start + var->vsync_len;
vtotal = vsync_end + var->upper_margin;
vblank_start = vactive;
- vblank_end = vtotal;
vblank_end = vsync_end + 1;
DBG_MSG("V: act %d, ss %d, se %d, tot %d bs %d, be %d\n",
@@ -1859,7 +1858,7 @@ void intelfbhw_cursor_init(struct intelfb_info *dinfo)
tmp = INREG(CURSOR_CONTROL);
tmp &= ~(CURSOR_FORMAT_MASK | CURSOR_GAMMA_ENABLE |
CURSOR_ENABLE | CURSOR_STRIDE_MASK);
- tmp = CURSOR_FORMAT_3C;
+ tmp |= CURSOR_FORMAT_3C;
OUTREG(CURSOR_CONTROL, tmp);
OUTREG(CURSOR_A_BASEADDR, dinfo->cursor.offset << 12);
tmp = (64 << CURSOR_SIZE_H_SHIFT) |
^ permalink raw reply related
* [PATCH 6/7] video: fbdev: sis: delete double assignment
From: Julia Lawall @ 2014-08-23 18:33 UTC (permalink / raw)
To: Thomas Winischhofer
Cc: joe, kernel-janitors, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1408818808-18850-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Delete successive assignments to the same location. The second assignment
is changed to update a different field, as done in other nearby code.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression i;
@@
*i = ...;
i = ...;
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The patches in this series do not depend on each other.
This changes the semantics of the code and is not tested.
drivers/video/fbdev/sis/init301.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
index a89e3ca..295e0de 100644
--- a/drivers/video/fbdev/sis/init301.c
+++ b/drivers/video/fbdev/sis/init301.c
@@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
} else {
SiS_Pr->PanelHT = 1688; SiS_Pr->PanelVT = 802;
- SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRS = 112;
+ SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRE = 112;
SiS_Pr->PanelVRS = 3; SiS_Pr->PanelVRE = 6;
SiS_Pr->PanelVCLKIdx300 = VCLK81_300;
SiS_Pr->PanelVCLKIdx315 = VCLK81_315;
^ permalink raw reply related
* Re: [PATCH 6/7] video: fbdev: sis: delete double assignment
From: Geert Uytterhoeven @ 2014-08-24 18:21 UTC (permalink / raw)
To: Julia Lawall
Cc: Thomas Winischhofer, Joe Perches, kernel-janitors@vger.kernel.org,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <1408818808-18850-7-git-send-email-Julia.Lawall@lip6.fr>
On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> ---
> The patches in this series do not depend on each other.
>
> This changes the semantics of the code and is not tested.
Hence I think you should change the subject of the patch, so it's obvious
some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".
> drivers/video/fbdev/sis/init301.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> index a89e3ca..295e0de 100644
> --- a/drivers/video/fbdev/sis/init301.c
> +++ b/drivers/video/fbdev/sis/init301.c
> @@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
> SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
> } else {
> SiS_Pr->PanelHT = 1688; SiS_Pr->PanelVT = 802;
> - SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRS = 112;
> + SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRE = 112;
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 6/7] video: fbdev: sis: delete double assignment
From: Julia Lawall @ 2014-08-24 18:40 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Julia Lawall, Thomas Winischhofer, Joe Perches,
kernel-janitors@vger.kernel.org, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Linux Fbdev development list,
linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdVERt9XBN5OW8QqT_PvQVHd6xgnCdjRpiSNp7ZSEgXWSg@mail.gmail.com>
On Sun, 24 Aug 2014, Geert Uytterhoeven wrote:
> On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > ---
> > The patches in this series do not depend on each other.
> >
> > This changes the semantics of the code and is not tested.
>
> Hence I think you should change the subject of the patch, so it's obvious
> some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".
Do you want me to resend this patch with the new subject, or just keep it
in mind for the future?
thanks,
julia
>
> > drivers/video/fbdev/sis/init301.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/sis/init301.c b/drivers/video/fbdev/sis/init301.c
> > index a89e3ca..295e0de 100644
> > --- a/drivers/video/fbdev/sis/init301.c
> > +++ b/drivers/video/fbdev/sis/init301.c
> > @@ -1714,7 +1714,7 @@ SiS_GetLCDResInfo(struct SiS_Private *SiS_Pr, unsigned short ModeNo, unsigned sh
> > SiS_Pr->PanelVCLKIdx315 = VCLK81_315; /* ? */
> > } else {
> > SiS_Pr->PanelHT = 1688; SiS_Pr->PanelVT = 802;
> > - SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRS = 112;
> > + SiS_Pr->PanelHRS = 48; SiS_Pr->PanelHRE = 112;
>
> 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
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 6/7] video: fbdev: sis: delete double assignment
From: Geert Uytterhoeven @ 2014-08-25 11:36 UTC (permalink / raw)
To: Julia Lawall
Cc: Thomas Winischhofer, Joe Perches, kernel-janitors@vger.kernel.org,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Linux Fbdev development list, linux-kernel@vger.kernel.org
In-Reply-To: <alpine.DEB.2.02.1408242040020.2066@localhost6.localdomain6>
Hi Julia,
On Sun, Aug 24, 2014 at 8:40 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> On Sat, Aug 23, 2014 at 8:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
>> > ---
>> > The patches in this series do not depend on each other.
>> >
>> > This changes the semantics of the code and is not tested.
>>
>> Hence I think you should change the subject of the patch, so it's obvious
>> some bug is fixed, e.g. "video: fbdev: sis: Fix double assignment".
>
> Do you want me to resend this patch with the new subject, or just keep it
> in mind for the future?
That's up to the fbdev maintainer.
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 4/4] simplefb: add clock handling code
From: Thierry Reding @ 2014-08-25 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140813170106.GT15297@lukather>
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]
On Wed, Aug 13, 2014 at 07:01:06PM +0200, Maxime Ripard wrote:
> On Wed, Aug 13, 2014 at 10:38:09AM -0600, Stephen Warren wrote:
[...]
> > If not, perhaps the clock driver should force the clock to be
> > enabled (perhaps only if the DRM/KMS driver isn't enabled?).
>
> I'm sorry, but I'm not going to take any code that will do that in our
> clock driver.
>
> I'm not going to have a huge list of ifdef depending on configuration
> options to know which clock to enable, especially when clk_get should
> have the consumer device as an argument.
Are you saying is that you want to solve a platform-specific problem by
pushing code into simple, generic drivers so that your platform code can
stay "clean"?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox