* Re: [PATCH 4/8] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Maxime Ripard @ 2015-02-14 15:36 UTC (permalink / raw)
To: Thomas Niederprüm
Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150214152212.1643da7d@maestro.intranet>
[-- Attachment #1: Type: text/plain, Size: 2579 bytes --]
On Sat, Feb 14, 2015 at 03:22:12PM +0100, Thomas Niederprüm wrote:
> Am Thu, 12 Feb 2015 16:11:21 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
> > On Sat, Feb 07, 2015 at 04:35:41PM +0100, Thomas Niederprüm wrote:
> > > Am Sat, 7 Feb 2015 12:18:21 +0100
> > > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> > >
> > > > Hi,
> > > >
> > > > On Fri, Feb 06, 2015 at 11:28:10PM +0100, niederp@physik.uni-kl.de
> > > > wrote:
> > > > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > >
> > > > > It makes sense to use vmalloc to allocate the video buffer
> > > > > since it has to be page aligned memory for using it with mmap.
> > > >
> > > > Please wrap your commit log at 80 chars.
> > >
> > > I'll try to do so in future, sorry for that.
> > >
> > > >
> > > > It looks like there's numerous fbdev drivers using this
> > > > (especially since you copy pasted that code, without mentionning
> > > > it).
> > >
> > > Yes, I should have mentioned that in the commit message. As
> > > implicitly indicated in the cover letter the rvmalloc() and
> > > rvfree() are copy pasted from the vfb driver. Honestly, I didn't
> > > give this one too much thought. It seemed a viable solution to the
> > > mmap problem. For a bit more history on that, see my comment below.
> > >
> > > >
> > > > That should be turned into an allocator so that drivers all get
> > > > this right.
> > > >
> > > > > Also deffered io seems buggy in combination with kmalloc'ed
> > > > > memory (crash on unloading the module).
> > > >
> > > > And maybe that's the real issue to fix.
> > >
> > > The problem is solved by using vmalloc ;)
> >
> > Yep, but why do you need to mark the reserved pages?
> >
> > ...
>
> As far as I understood mmaped memory is marked as userspace memory in
> the page table and is therefore subject to swapping. The pages are
> marked reserved to make clear that this memory can not be swapped and
> thus lock the pages in memory. See discussions [0,1,2].
>
> [0]http://stackoverflow.com/questions/10760479/mmap-kernel-buffer-to-user-space
> [1]http://www.linuxquestions.org/questions/linux-kernel-70/why-why-setpagereserved-is-needed-when-map-a-kernel-space-to-user-space-885176/
> [2]https://sites.google.com/site/skartikeyan/mmap.html
Hmmm, both https://lwn.net/Articles/28746/ and
http://linux-mm.org/DeviceDriverMmap tell otherwise :)
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: [PATCH 5/8] fbdev: ssd1307fb: Add module parameter bitsperpixel.
From: Maxime Ripard @ 2015-02-14 15:54 UTC (permalink / raw)
To: Thomas Niederprüm
Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150207170503.045490aa@maestro.intranet>
[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]
On Sat, Feb 07, 2015 at 05:05:03PM +0100, Thomas Niederprüm wrote:
> Am Sat, 7 Feb 2015 12:20:43 +0100
> schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
>
> > On Fri, Feb 06, 2015 at 11:28:11PM +0100, niederp@physik.uni-kl.de
> > wrote:
> > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > >
> > > This patch adds a module parameter 'bitsperpixel' to adjust the
> > > colordepth of the framebuffer. All values >1 will result in memory
> > > map of the requested color depth. However only the MSB of each
> > > pixel will be sent to the device. The framebuffer identifies itself
> > > as a grayscale display with the specified depth.
> >
> > I'm not sure this is the right thing to do.
> >
> > The bits per pixel for this display is rightfully defined, used and
> > reported to the userspace, why would you want to change that?
>
> You are right of course. The display is 1bpp and it reports to be 1
> bpp. The problem is that there is almost no userspace library that can
> handle 1 bit framebuffers correctly. So it is nice if the framebuffer
> (optionally) can expose itself as 8 bits per pixel grayscale to the
> userspace program. As an example this allows to run DirectFB on the
> framebuffer, which is not possible out of the box for 1bpp.
>
> Also note that if do not set the module parameter at load time
> the framebuffer will be 1bpp. So you have to actively set that module
> parameter to make the framebuffer pretend to be more than 1bpp.
>
> In any case I don't cling to that patch, I just thought it was a nice
> feature.
I'd say that the right fix would be to patch DirectFB, instead of
faking that in the kernel.
But again, that's probably Tomi's call, not mine.
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: [PATCH 2/8] fbdev: ssd1307fb: Unify init code and make controller configurable from device tree
From: Thomas Niederprüm @ 2015-02-14 16:12 UTC (permalink / raw)
To: Maxime Ripard; +Cc: linux-fbdev, plagnioj, tomi.valkeinen, linux-kernel
In-Reply-To: <20150212164147.GK2079@lukather>
Am Thu, 12 Feb 2015 17:41:47 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> On Sat, Feb 07, 2015 at 03:59:33PM +0100, Thomas Niederprüm wrote:
> > Am Sat, 7 Feb 2015 11:42:25 +0100
> > schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:
> >
> > > Hi,
> > >
> > > On Fri, Feb 06, 2015 at 11:28:08PM +0100, niederp@physik.uni-kl.de
> > > wrote:
> > > > From: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > >
> > > > This patches unifies the init code for the ssd130X chips and
> > > > adds device tree bindings to describe the hardware configuration
> > > > of the used controller. This gets rid of the magic bit values
> > > > used in the init code so far.
> > > >
> > > > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > > > ---
> > > > .../devicetree/bindings/video/ssd1307fb.txt | 11 +
> > > > drivers/video/fbdev/ssd1307fb.c | 243
> > > > ++++++++++++++------- 2 files changed, 174 insertions(+), 80
> > > > deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/video/ssd1307fb.txt
> > > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt index
> > > > 7a12542..1230f68 100644 ---
> > > > a/Documentation/devicetree/bindings/video/ssd1307fb.txt +++
> > > > b/Documentation/devicetree/bindings/video/ssd1307fb.txt @@
> > > > -15,6 +15,17 @@ Required properties: Optional properties:
> > > > - reset-active-low: Is the reset gpio is active on physical
> > > > low?
> > > > + - solomon,segment-remap: Invert the order of data column to
> > > > segment mapping
> > > > + - solomon,offset: Map the display start line to one of COM0 -
> > > > COM63
> > > > + - solomon,contrast: Set initial contrast of the display
> > > > + - solomon,prechargep1: Set the duration of the precharge
> > > > period phase1
> > > > + - solomon,prechargep2: Set the duration of the precharge
> > > > period phase2
> > > > + - solomon,com-alt: Enable/disable alternate COM pin
> > > > configuration
> > > > + - solomon,com-lrremap: Enable/disable left-right remap of COM
> > > > pins
> > > > + - solomon,com-invdir: Invert COM scan direction
> > > > + - solomon,vcomh: Set VCOMH regulator voltage
> > > > + - solomon,dclk-div: Set display clock divider
> > > > + - solomon,dclk-frq: Set display clock frequency
> > >
> > > I'm sorry, but this is the wrong approach, for at least two
> > > reasons: you broke all existing users of that driver, which is a
> > > clear no-go,
> >
> > Unfortunately this is true. The problem is that the SSD130X
> > controllers allow for a very versatile wiring of the display to the
> > controller. It's over to the manufacturer of the OLED module
> > (disp+controller) to decide how it's actually wired and during
> > device initialization the driver has to take care to configure the
> > SSD130X controller according to that wiring. If the driver fails to
> > do so you will end up having your display showing
> > garbage.
>
> How so?
One good example is the segment remap. It basically allows to invert the
order of the output pins connecting to the oled panel. This gives the
manufacturer of the module the freedom wire it the one way or the
other, depending on the PCB restrictions/panel layout (Section 10.1.12
of [0], 10.1.8 in [1], 9.1.8 in [2]). However, once the panel is
connected to the controller it's determined whether the segment remap
is needed or not. Setting the segment remap as done in the current
initialization code for ssd1306 and ssd1307 makes my display module
show it's contents mirrored left to right, probably since the
manufacturer decided not to connect the panel in an inverted order.
The same applies to the com-alt, com-lrremap and com-invdir values,
which define different possibilities for the COM signals pin
configuration (Section 10.1.26 of [0], 10.1.18 in [1], 9.1.18 in [2])
and readout direction of the video memory (Section 10.1.21 of [0],
10.1.14 in [1], 9.1.14 in [2]). Setting com-alt incorrectly leaves
every other line of the display blank. Setting com-lrremap incorrectly
produces a very distorted image. Setting com-invdir incorrectly flips
the image upside down.
IMHO at least these four hardware-specific properties need to be known
to the driver in order to initialize the hardware correctly.
>
> Does it depend on the X, or can it change from one same controller to
> another? to what extent?
Unfortunately I do not posses any hardware utilizing a ssd1306 or
ssd1307 controller. My primary and only target device is a Newhaven
NHD-3.12-25664UCB2 OLED display module using an SSD1305 controller. I
just inferred from the datasheets of ssd1306/7 [1,2] that they should
behave the same since the registers are bit to bit identical (except
for the VHCOM register). Maybe that was a bit too naive :/
>
> The 1306 for example seems to not be using these values at all, while
> the 1307 does.
That is surprising. In that case I would like to ask the guys from
Solomon why they describe all these options in the SSD1306 datasheet
[1]. But in any case, isn't that good news for the problem of setting
the default values. When the 1306 isn't using these values anyway we can
not break the initialization by setting different default values. In
this case the problem of the default values boils down to the segment
remap only since this is set in the init code of the 1307, while the
default would be to leave it off.
>
> > Unfortunately the current sate of the initialization code of the
> > ssd1307fb driver is not very flexible in that respect. Taking a look
> > at the initialization code for the ssd1306 shows that it was written
> > with one very special display module in mind. Most of the magic bit
> > values set there are non-default values according to the
> > datasheet. The result is that the driver works with that one
> > particular display module but many other (differently wired) display
> > modules using a ssd1306 controller won't work without changing the
> > hardcoded magic bit values.
> >
> > My idea here was to set all configuration to the default values (as
> > given in the datasheet) unless it is overwritten by DT. Of course,
> > without a change in DT, this breaks the driver for all existing
> > users. The only alternative would be to set the current values as
> > default. Somehow this feels wrong to me as these values look
> > arbitrary when you don't know what exact display module they were
> > set for. But if you insist, I will change the default values.
>
> Unfortunately, the DT bindings are to be considered an ABI, and we
> should support booting with older DTs (not that I personally care
> about it, but that's another issue). So we really don't have much
> choice here.
>
> Moreover, that issue left aside, modifying bindings like this without
> fixing up the in-tree users is considered quite rude :)
I didn't intend to be rude, sorry. A quick search revealed that there
is luckily only one in-tree user, which is imx28-cfa10036.dts. In case
it will be necessary I will include a patch to fix this.
> > > and the DT itself should not contain any direct mapping of the
> > > registers.
> > >
> >
> > I think I don't get what you mean here. Is it because I do no sanity
> > checks of the numbers set in DT? I was just looking for a way to
> > hand over the information about the wiring of display to the
> > driver. How would you propose to solve this?
>
> What I meant was that replicating direct registers value is usually a
> recipe for a later failure, especially if we can have the information
> under a generic and easy to understand manner.
>
> For example, replacing the solomon,dclk-div and solomon,dclk-frq
> properties by a clock-frequency property in Hz, and computing the
> divider and that register in your driver is usually better, also
> because it allows to have different requirements / algorithms to
> compute that if some other chip needs it.
I'll give that a try, even though that particular one is not trivial
since the documentation on the actual frequency that is set by the
dclk-freq is very poor (not present for 1306/1307 [1,2], just a graph
for 1305 [0]).
For the properties describing the hardware pin configuration (see above)
I see no real alternative. Maybe they can all be covered by one DT
property like:
solomon,com-cfg = PINCFG_SEGREMAP | PINCFG_COMALT | PINCFG_COMINV |
PINCFG_COMLRRM
each PINCFG_* setting one bit. The driver will then translate this into
the correct settings for the 130X registers. The only problem here is
that this implicitly assumes the default values of each bit to be 0.
>
> Maxime
>
[0]http://www.newhavendisplay.com/app_notes/SSD1305.pdf
[1]http://www.adafruit.com/datasheets/SSD1306.pdf
[2]http://www.displayfuture.com/Display/datasheet/controller/SSD1307.pdf
Thomas
^ permalink raw reply
* [PATCH] gxt4500: enable on non-PPC architectures
From: Ondrej Zary @ 2015-02-14 22:30 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
These chips can be present at least on x86 too - Fire GL2 AGP has GXT6000P but
this driver is currently limited to PPC.
Enable it for all architectures and add chip configuration for little-endian.
Tested on x86 with Fire GL2 AGP.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/Kconfig | 5 +++--
drivers/video/fbdev/gxt4500.c | 7 +++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 08a7a04..cb15734 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2143,7 +2143,7 @@ config FB_UDL
config FB_IBM_GXT4500
tristate "Framebuffer support for IBM GXT4000P/4500P/6000P/6500P adaptors"
- depends on FB && PPC
+ depends on FB
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2151,7 +2151,8 @@ config FB_IBM_GXT4500
Say Y here to enable support for the IBM GXT4000P/6000P and
GXT4500P/6500P display adaptor based on Raster Engine RC1000,
found on some IBM System P (pSeries) machines. This driver
- doesn't use Geometry Engine GT1000.
+ doesn't use Geometry Engine GT1000. This driver also supports
+ AGP Fire GL2/3/4 cards on x86.
config FB_PS3
tristate "PS3 GPU framebuffer driver"
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a..1bf9894 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -670,8 +670,15 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_drvdata(pdev, info);
+#ifdef __BIG_ENDIAN
/* Set byte-swapping for DFA aperture for all pixel sizes */
pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
+#else /* __LITTLE_ENDIAN */
+ /* not sure what this means but fgl23 driver does that */
+ pci_write_config_dword(pdev, CFG_ENDIAN0, 0x2300);
+/* pci_write_config_dword(pdev, CFG_ENDIAN0 + 4, 0x400000);*/
+ pci_write_config_dword(pdev, CFG_ENDIAN0 + 8, 0x98530000);
+#endif
info->fbops = &gxt4500_ops;
info->flags = FBINFO_FLAG_DEFAULT;
--
Ondrej Zary
^ permalink raw reply related
* [PATCH] gxt4500: fix 16bpp 565 mode
From: Ondrej Zary @ 2015-02-14 23:09 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
Fix wrong colors in 16bpp 565 mode.
Not sure if this is OK for big-endian systems or it breaks them.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/gxt4500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1bf9894..1f2fd5b 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -525,7 +525,7 @@ static int gxt4500_setcolreg(unsigned int reg, unsigned int red,
u32 val = reg;
switch (par->pixfmt) {
case DFA_PIX_16BIT_565:
- val |= (reg << 11) | (reg << 6);
+ val |= (reg << 11) | (reg << 5);
break;
case DFA_PIX_16BIT_1555:
val |= (reg << 10) | (reg << 5);
--
Ondrej Zary
^ permalink raw reply related
* [PATCH] framebuffer: don't link fb_devio into kernel image unconditionally
From: Harald Geyer @ 2015-02-16 18:34 UTC (permalink / raw)
To: linux-fbdev
CONFIG_FB_DEFERRED_IO is defined as bool while CONFIG_FB is defined as
tristate. Currently fb_defio.o is linked into the kernel image even if
CONFIG_FB=m.
I fix this by updating the Makefile to link fb_defio.o into fb.o and thus
go into one place with the other core framebuffer code.
This has been tested on arm/sunxi.
Signed-off-by: Harald Geyer <harald@ccbib.org>
---
drivers/video/fbdev/core/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 67f28e2..23d86a8 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_FB_CMDLINE) += fb_cmdline.o
obj-$(CONFIG_FB) += fb.o
fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
modedb.o fbcvt.o
+fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
fb-objs := $(fb-y)
obj-$(CONFIG_FB_CFB_FILLRECT) += cfbfillrect.o
@@ -14,4 +15,3 @@ obj-$(CONFIG_FB_SYS_IMAGEBLIT) += sysimgblt.o
obj-$(CONFIG_FB_SYS_FOPS) += fb_sys_fops.o
obj-$(CONFIG_FB_SVGALIB) += svgalib.o
obj-$(CONFIG_FB_DDC) += fb_ddc.o
-obj-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
--
2.1.4
^ permalink raw reply related
* [PATCH 1/5] gxt4500: enable on non-PPC architectures
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
These chips can be present at least on x86 too - Fire GL2 AGP has GXT6000P but
this driver is currently limited to PPC.
Enable it for all architectures and add chip configuration for little-endian.
Tested on x86 with Fire GL2 AGP.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/Kconfig | 5 +++--
drivers/video/fbdev/gxt4500.c | 7 +++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 08a7a04..cb15734 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2143,7 +2143,7 @@ config FB_UDL
config FB_IBM_GXT4500
tristate "Framebuffer support for IBM GXT4000P/4500P/6000P/6500P adaptors"
- depends on FB && PPC
+ depends on FB
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2151,7 +2151,8 @@ config FB_IBM_GXT4500
Say Y here to enable support for the IBM GXT4000P/6000P and
GXT4500P/6500P display adaptor based on Raster Engine RC1000,
found on some IBM System P (pSeries) machines. This driver
- doesn't use Geometry Engine GT1000.
+ doesn't use Geometry Engine GT1000. This driver also supports
+ AGP Fire GL2/3/4 cards on x86.
config FB_PS3
tristate "PS3 GPU framebuffer driver"
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a..1bf9894 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -670,8 +670,15 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_drvdata(pdev, info);
+#ifdef __BIG_ENDIAN
/* Set byte-swapping for DFA aperture for all pixel sizes */
pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
+#else /* __LITTLE_ENDIAN */
+ /* not sure what this means but fgl23 driver does that */
+ pci_write_config_dword(pdev, CFG_ENDIAN0, 0x2300);
+/* pci_write_config_dword(pdev, CFG_ENDIAN0 + 4, 0x400000);*/
+ pci_write_config_dword(pdev, CFG_ENDIAN0 + 8, 0x98530000);
+#endif
info->fbops = &gxt4500_ops;
info->flags = FBINFO_FLAG_DEFAULT;
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 2/5] gxt4500: fix 16bpp 565 mode
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1424212434-12614-1-git-send-email-linux@rainbow-software.org>
Fix wrong colors in 16bpp 565 mode.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/gxt4500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1bf9894..1f2fd5b 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -525,7 +525,7 @@ static int gxt4500_setcolreg(unsigned int reg, unsigned int red,
u32 val = reg;
switch (par->pixfmt) {
case DFA_PIX_16BIT_565:
- val |= (reg << 11) | (reg << 6);
+ val |= (reg << 11) | (reg << 5);
break;
case DFA_PIX_16BIT_1555:
val |= (reg << 10) | (reg << 5);
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 3/5] gxt4500: fix color order
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1424212434-12614-1-git-send-email-linux@rainbow-software.org>
The color order in truecolor modes is wrong. This does not affect console but
is visible e.g. in X11 which has wrong colors.
Swap blue and red colors to fix the problem.
Fixes https://forums.gentoo.org/viewtopic-t-692740-start-0.html
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/gxt4500.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1f2fd5b..442b07c 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -347,11 +347,12 @@ static void gxt4500_unpack_pixfmt(struct fb_var_screeninfo *var,
break;
}
if (pixfmt != DFA_PIX_8BIT) {
- var->green.offset = var->red.length;
- var->blue.offset = var->green.offset + var->green.length;
+ var->blue.offset = 0;
+ var->green.offset = var->blue.length;
+ var->red.offset = var->green.offset + var->green.length;
if (var->transp.length)
var->transp.offset - var->blue.offset + var->blue.length;
+ var->red.offset + var->red.length;
}
}
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 4/5] gxt4500: Use write-combining for framebuffer (PAT and MTRR)
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1424212434-12614-1-git-send-email-linux@rainbow-software.org>
Use write-combining for framebuffer to greatly improve performance on x86.
Add both ioremap_wc (for systems with PAT) and MTRR setup for non-PAT systems.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/gxt4500.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 442b07c..f278c82 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -13,6 +13,9 @@
#include <linux/pci_ids.h>
#include <linux/delay.h>
#include <linux/string.h>
+#ifdef CONFIG_MTRR
+#include <asm/mtrr.h>
+#endif
#define PCI_DEVICE_ID_IBM_GXT4500P 0x21c
#define PCI_DEVICE_ID_IBM_GXT6500P 0x21b
@@ -142,7 +145,9 @@ static const unsigned char watfmt[] = {
struct gxt4500_par {
void __iomem *regs;
-
+#ifdef CONFIG_MTRR
+ int mtrr_reg;
+#endif
int pixfmt; /* pixel format, see DFA_PIX_* values */
/* PLL parameters */
@@ -158,6 +163,10 @@ struct gxt4500_par {
/* mode requested by user */
static char *mode_option;
+#ifdef CONFIG_MTRR
+static int mtrr = 1;
+#endif
+
/* default mode: 1280x1024 @ 60 Hz, 8 bpp */
static const struct fb_videomode defaultmode = {
.refresh = 60,
@@ -663,7 +672,7 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
info->fix.smem_start = fb_phys;
info->fix.smem_len = pci_resource_len(pdev, 1);
- info->screen_base = pci_ioremap_bar(pdev, 1);
+ info->screen_base = ioremap_wc(fb_phys, info->fix.smem_len);
if (!info->screen_base) {
dev_err(&pdev->dev, "gxt4500: cannot map framebuffer\n");
goto err_unmap_regs;
@@ -671,6 +680,14 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_drvdata(pdev, info);
+#ifdef CONFIG_MTRR
+ if (mtrr) {
+ par->mtrr_reg = -1;
+ par->mtrr_reg = mtrr_add(info->fix.smem_start,
+ info->fix.smem_len, MTRR_TYPE_WRCOMB, 1);
+ }
+#endif
+
#ifdef __BIG_ENDIAN
/* Set byte-swapping for DFA aperture for all pixel sizes */
pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
@@ -735,6 +752,12 @@ static void gxt4500_remove(struct pci_dev *pdev)
return;
par = info->par;
unregister_framebuffer(info);
+#ifdef CONFIG_MTRR
+ if (par->mtrr_reg >= 0) {
+ mtrr_del(par->mtrr_reg, 0, 0);
+ par->mtrr_reg = -1;
+ }
+#endif
fb_dealloc_cmap(&info->cmap);
iounmap(par->regs);
iounmap(info->screen_base);
@@ -789,3 +812,8 @@ MODULE_DESCRIPTION("FBDev driver for IBM GXT4500P/6500P and GXT4000P/6000P");
MODULE_LICENSE("GPL");
module_param(mode_option, charp, 0);
MODULE_PARM_DESC(mode_option, "Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\"");
+
+#ifdef CONFIG_MTRR
+module_param(mtrr, int, 0444);
+MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
+#endif
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 5/5] gxt4500: enable panning
From: Ondrej Zary @ 2015-02-17 22:33 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1424212434-12614-1-git-send-email-linux@rainbow-software.org>
The driver implements pan_display but the corresponding flags are not set.
Add FBINFO_HWACCEL_XPAN and FBINFO_HWACCEL_YPAN to flags to allow HW
accelerated panning (for fast scrolling).
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
drivers/video/fbdev/gxt4500.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index f278c82..7f97c89 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -699,7 +699,8 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
#endif
info->fbops = &gxt4500_ops;
- info->flags = FBINFO_FLAG_DEFAULT;
+ info->flags = FBINFO_FLAG_DEFAULT | FBINFO_HWACCEL_XPAN |
+ FBINFO_HWACCEL_YPAN;
err = fb_alloc_cmap(&info->cmap, 256, 0);
if (err) {
--
Ondrej Zary
^ permalink raw reply related
* [PATCH 1/3] Printk() is replaced with pr_* respective functions in efifb.c
From: Parmeshwr Prasad @ 2015-02-18 8:29 UTC (permalink / raw)
To: pjones, plagnioj; +Cc: linux-fbdev, linux-kernel
This is a trivial patch:
I have replaced printk() with respective pr_* functions.
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..facfbb3 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -142,10 +142,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_err("efifb: invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("efifb: probing for efifb\n");
/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -193,14 +193,14 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
+ pr_warn(
"efifb: cannot reserve video memory at 0x%lx\n",
efifb_fix.smem_start);
}
info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -218,18 +218,18 @@ static int efifb_probe(struct platform_device *dev)
info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
+ pr_err("efifb: abort, cannot ioremap video memory "
"0x%x @ 0x%lx\n",
efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
+ pr_info("efifb: framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
efifb_fix.smem_start, info->screen_base,
size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);
@@ -237,7 +237,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.xres_virtual = efifb_defined.xres;
efifb_defined.yres_virtual = efifb_fix.smem_len /
efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ pr_info("efifb: scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;
/* some dummy values for timing to make fbset happy */
@@ -255,7 +255,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;
- printk(KERN_INFO "efifb: %s: "
+ pr_info("efifb: %s: "
"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
"Truecolor",
screen_info.rsvd_size,
@@ -276,11 +276,11 @@ static int efifb_probe(struct platform_device *dev)
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ pr_err("efifb: cannot allocate colormap\n");
goto err_unmap;
}
if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ pr_err("efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
--
1.9.3
-Parmeshwr
^ permalink raw reply related
* [PATCH 2/3] Static should not be initialize so removing static initialization from efifb.c
From: Parmeshwr Prasad @ 2015-02-18 8:33 UTC (permalink / raw)
To: pjones, plagnioj, tomi.valkeinen; +Cc: linux-kernel, linux-fbdev
This patch depend on my earlier patch.
Static should not be initialize so removing initialization.
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index facfbb3..cc1a156 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -17,7 +17,7 @@
#include <video/vga.h>
#include <asm/sysfb.h>
-static bool request_mem_succeeded = false;
+static bool request_mem_succeeded;
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
--
1.9.3
^ permalink raw reply related
* [PATCH 3/3] Moved assignment out of if() statement in efifb.c
From: Parmeshwr Prasad @ 2015-02-18 8:36 UTC (permalink / raw)
To: pjones, plagnioj, tomi.valkeinen; +Cc: linux-fbdev, linux-kernel
This patch depend on my earlier patchs.
Moving assignment out of if() statement.
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index cc1a156..9124ba9 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -275,11 +275,13 @@ static int efifb_probe(struct platform_device *dev)
info->fix = efifb_fix;
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
- if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
+ err = fb_alloc_cmap(&info->cmap, 256, 0);
+ if (err < 0) {
pr_err("efifb: cannot allocate colormap\n");
goto err_unmap;
}
- if ((err = register_framebuffer(info)) < 0) {
+ err = register_framebuffer(info);
+ if (err < 0) {
pr_err("efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
--
1.9.3
-Parmeshwr
^ permalink raw reply related
* Re: [PATCH 1/3] Printk() is replaced with pr_* respective functions in efifb.c
From: Joe Perches @ 2015-02-18 8:51 UTC (permalink / raw)
To: Parmeshwr Prasad; +Cc: pjones, plagnioj, linux-fbdev, linux-kernel
In-Reply-To: <20150218082942.GB9820@linuxteamdev.amer.dell.com>
On Wed, 2015-02-18 at 03:29 -0500, Parmeshwr Prasad wrote:
> This is a trivial patch:
>
> I have replaced printk() with respective pr_* functions.
Hi.
A few suggestions:
o Add this #define before any include
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
o Strip the embedded "efifb: " prefixes from the formats
o Coalesce the formats
o Don't change KERN_<LEVEL> uses unnecessarily
If you do change logging levels, state why in the change log
o Align the multiline arguments
> diff --git a/drivers/video/fbdev/efifb.c b/diriveom rrs/video/fbdev/efifb.c
[]
> @@ -142,10 +142,10 @@ static int efifb_probe(struct platform_device *dev)
> if (!screen_info.pages)
> screen_info.pages = 1;
> if (!screen_info.lfb_base) {
> - printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
> + pr_err("efifb: invalid framebuffer address\n");
Strip the "efifb: " prefix from all the pr_<level> uses
pr_err("invalid framebuffer address\n");
> @@ -193,14 +193,14 @@ static int efifb_probe(struct platform_device *dev)
> } else {
> /* We cannot make this fatal. Sometimes this comes from magic
> spaces our resource handlers simply don't know about */
> - printk(KERN_WARNING
> + pr_warn(
> "efifb: cannot reserve video memory at 0x%lx\n",
> efifb_fix.smem_start);
Move the format to the same line as pr_warn
pr_warn("cannot reserve video memory at 0x%lx\n",
> @@ -218,18 +218,18 @@ static int efifb_probe(struct platform_device *dev)
>
> info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
> if (!info->screen_base) {
> - printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
> + pr_err("efifb: abort, cannot ioremap video memory "
> "0x%x @ 0x%lx\n",
> efifb_fix.smem_len, efifb_fix.smem_start);
Coalesce the format and align the arguments
pr_err("abort, cannot ioremap video memory 0x%x @ 0x%lx\n"
efifb_fix.smem_len, efifb_fix.smem_start);
You should end up with something like:
---
drivers/video/fbdev/efifb.c | 57 +++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..2769bc9 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -6,6 +6,8 @@
*
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
@@ -142,10 +144,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_debug("invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("probing for efifb\n");
/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -193,14 +195,13 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
- "efifb: cannot reserve video memory at 0x%lx\n",
+ pr_warn("cannot reserve video memory at 0x%lx\n",
efifb_fix.smem_start);
}
info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -218,26 +219,24 @@ static int efifb_probe(struct platform_device *dev)
info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
- efifb_fix.smem_len, efifb_fix.smem_start);
+ pr_err("abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
+ efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
- "using %dk, total %dk\n",
- efifb_fix.smem_start, info->screen_base,
- size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
- efifb_defined.xres, efifb_defined.yres,
- efifb_defined.bits_per_pixel, efifb_fix.line_length,
- screen_info.pages);
+ pr_info("framebuffer at 0x%lx, mapped to 0x%p, using %dk, total %dk\n",
+ efifb_fix.smem_start, info->screen_base,
+ size_remap / 1024, size_total / 1024);
+ pr_info("mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ efifb_defined.xres, efifb_defined.yres,
+ efifb_defined.bits_per_pixel, efifb_fix.line_length,
+ screen_info.pages);
efifb_defined.xres_virtual = efifb_defined.xres;
efifb_defined.yres_virtual = efifb_fix.smem_len /
efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ pr_info("scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;
/* some dummy values for timing to make fbset happy */
@@ -255,17 +254,15 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;
- printk(KERN_INFO "efifb: %s: "
- "size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
- "Truecolor",
- screen_info.rsvd_size,
- screen_info.red_size,
- screen_info.green_size,
- screen_info.blue_size,
- screen_info.rsvd_pos,
- screen_info.red_pos,
- screen_info.green_pos,
- screen_info.blue_pos);
+ pr_info("Truecolor: size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
+ screen_info.rsvd_size,
+ screen_info.red_size,
+ screen_info.green_size,
+ screen_info.blue_size,
+ screen_info.rsvd_pos,
+ screen_info.red_pos,
+ screen_info.green_pos,
+ screen_info.blue_pos);
efifb_fix.ypanstep = 0;
efifb_fix.ywrapstep = 0;
@@ -276,11 +273,11 @@ static int efifb_probe(struct platform_device *dev)
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ pr_err("cannot allocate colormap\n");
goto err_unmap;
}
if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ pr_err("cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
^ permalink raw reply related
* Re: [PATCH 1/3] Printk() is replaced with pr_* respective functions in efifb.c
From: Parmeshwr Prasad @ 2015-02-18 9:51 UTC (permalink / raw)
To: Joe Perches
Cc: pjones@redhat.com, plagnioj@jcrosoft.com,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-efi
In-Reply-To: <1424249497.25416.23.camel@perches.com>
Thanks Joe for your suggestion.
I have changes patch as you said.
Signed-off-by: Parmeshwr Prasad <parmeshwr_prasad@dell.com>
---
drivers/video/fbdev/efifb.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 4bfff34..7557991 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -6,6 +6,8 @@
*
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
@@ -142,10 +144,10 @@ static int efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
- printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
+ pr_debug("invalid framebuffer address\n");
return -ENODEV;
}
- printk(KERN_INFO "efifb: probing for efifb\n");
+ pr_info("probing for efifb\n");
/* just assume they're all unset if any are */
if (!screen_info.blue_size) {
@@ -193,14 +195,14 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
- "efifb: cannot reserve video memory at 0x%lx\n",
+ pr_warn(
+ "cannot reserve video memory at 0x%lx\n",
efifb_fix.smem_start);
}
info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -218,18 +220,18 @@ static int efifb_probe(struct platform_device *dev)
info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
+ pr_err("abort, cannot ioremap video memory "
"0x%x @ 0x%lx\n",
efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
- printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
+ pr_info("framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
efifb_fix.smem_start, info->screen_base,
size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ pr_info("mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);
@@ -237,7 +239,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.xres_virtual = efifb_defined.xres;
efifb_defined.yres_virtual = efifb_fix.smem_len /
efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ pr_info("scrolling: redraw\n");
efifb_defined.yres_virtual = efifb_defined.yres;
/* some dummy values for timing to make fbset happy */
@@ -255,7 +257,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.transp.offset = screen_info.rsvd_pos;
efifb_defined.transp.length = screen_info.rsvd_size;
- printk(KERN_INFO "efifb: %s: "
+ pr_info("%s: "
"size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
"Truecolor",
screen_info.rsvd_size,
@@ -276,11 +278,11 @@ static int efifb_probe(struct platform_device *dev)
info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
- printk(KERN_ERR "efifb: cannot allocate colormap\n");
+ pr_err("cannot allocate colormap\n");
goto err_unmap;
}
if ((err = register_framebuffer(info)) < 0) {
- printk(KERN_ERR "efifb: cannot register framebuffer\n");
+ pr_err("cannot register framebuffer\n");
goto err_fb_dealoc;
}
fb_info(info, "%s frame buffer device\n", info->fix.id);
--
1.9.3
-Parmeshwr
On Wed, Feb 18, 2015 at 02:51:37AM -0600, Joe Perches wrote:
> On Wed, 2015-02-18 at 03:29 -0500, Parmeshwr Prasad wrote:
> > This is a trivial patch:
> >
> > I have replaced printk() with respective pr_* functions.
>
> Hi.
>
> A few suggestions:
>
> o Add this #define before any include
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> o Strip the embedded "efifb: " prefixes from the formats
> o Coalesce the formats
> o Don't change KERN_<LEVEL> uses unnecessarily
> If you do change logging levels, state why in the change log
> o Align the multiline arguments
>
> > diff --git a/drivers/video/fbdev/efifb.c b/diriveom rrs/video/fbdev/efifb.c
> []
> > @@ -142,10 +142,10 @@ static int efifb_probe(struct platform_device *dev)
> > if (!screen_info.pages)
> > screen_info.pages = 1;
> > if (!screen_info.lfb_base) {
> > - printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
> > + pr_err("efifb: invalid framebuffer address\n");
>
> Strip the "efifb: " prefix from all the pr_<level> uses
>
> pr_err("invalid framebuffer address\n");
>
> > @@ -193,14 +193,14 @@ static int efifb_probe(struct platform_device *dev)
> > } else {
> > /* We cannot make this fatal. Sometimes this comes from magic
> > spaces our resource handlers simply don't know about */
> > - printk(KERN_WARNING
> > + pr_warn(
> > "efifb: cannot reserve video memory at 0x%lx\n",
> > efifb_fix.smem_start);
>
> Move the format to the same line as pr_warn
>
> pr_warn("cannot reserve video memory at 0x%lx\n",
>
> > @@ -218,18 +218,18 @@ static int efifb_probe(struct platform_device *dev)
> >
> > info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
> > if (!info->screen_base) {
> > - printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
> > + pr_err("efifb: abort, cannot ioremap video memory "
> > "0x%x @ 0x%lx\n",
> > efifb_fix.smem_len, efifb_fix.smem_start);
>
> Coalesce the format and align the arguments
>
> pr_err("abort, cannot ioremap video memory 0x%x @ 0x%lx\n"
> efifb_fix.smem_len, efifb_fix.smem_start);
>
> You should end up with something like:
>
> ---
> drivers/video/fbdev/efifb.c | 57 +++++++++++++++++++++------------------------
> 1 file changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 4bfff34..2769bc9 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -6,6 +6,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/errno.h>
> @@ -142,10 +144,10 @@ static int efifb_probe(struct platform_device *dev)
> if (!screen_info.pages)
> screen_info.pages = 1;
> if (!screen_info.lfb_base) {
> - printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
> + pr_debug("invalid framebuffer address\n");
> return -ENODEV;
> }
> - printk(KERN_INFO "efifb: probing for efifb\n");
> + pr_info("probing for efifb\n");
>
> /* just assume they're all unset if any are */
> if (!screen_info.blue_size) {
> @@ -193,14 +195,13 @@ static int efifb_probe(struct platform_device *dev)
> } else {
> /* We cannot make this fatal. Sometimes this comes from magic
> spaces our resource handlers simply don't know about */
> - printk(KERN_WARNING
> - "efifb: cannot reserve video memory at 0x%lx\n",
> + pr_warn("cannot reserve video memory at 0x%lx\n",
> efifb_fix.smem_start);
> }
>
> info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
> if (!info) {
> - printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
> + pr_err("cannot allocate framebuffer\n");
> err = -ENOMEM;
> goto err_release_mem;
> }
> @@ -218,26 +219,24 @@ static int efifb_probe(struct platform_device *dev)
>
> info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
> if (!info->screen_base) {
> - printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
> - "0x%x @ 0x%lx\n",
> - efifb_fix.smem_len, efifb_fix.smem_start);
> + pr_err("abort, cannot ioremap video memory 0x%x @ 0x%lx\n",
> + efifb_fix.smem_len, efifb_fix.smem_start);
> err = -EIO;
> goto err_release_fb;
> }
>
> - printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
> - "using %dk, total %dk\n",
> - efifb_fix.smem_start, info->screen_base,
> - size_remap/1024, size_total/1024);
> - printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
> - efifb_defined.xres, efifb_defined.yres,
> - efifb_defined.bits_per_pixel, efifb_fix.line_length,
> - screen_info.pages);
> + pr_info("framebuffer at 0x%lx, mapped to 0x%p, using %dk, total %dk\n",
> + efifb_fix.smem_start, info->screen_base,
> + size_remap / 1024, size_total / 1024);
> + pr_info("mode is %dx%dx%d, linelength=%d, pages=%d\n",
> + efifb_defined.xres, efifb_defined.yres,
> + efifb_defined.bits_per_pixel, efifb_fix.line_length,
> + screen_info.pages);
>
> efifb_defined.xres_virtual = efifb_defined.xres;
> efifb_defined.yres_virtual = efifb_fix.smem_len /
> efifb_fix.line_length;
> - printk(KERN_INFO "efifb: scrolling: redraw\n");
> + pr_info("scrolling: redraw\n");
> efifb_defined.yres_virtual = efifb_defined.yres;
>
> /* some dummy values for timing to make fbset happy */
> @@ -255,17 +254,15 @@ static int efifb_probe(struct platform_device *dev)
> efifb_defined.transp.offset = screen_info.rsvd_pos;
> efifb_defined.transp.length = screen_info.rsvd_size;
>
> - printk(KERN_INFO "efifb: %s: "
> - "size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
> - "Truecolor",
> - screen_info.rsvd_size,
> - screen_info.red_size,
> - screen_info.green_size,
> - screen_info.blue_size,
> - screen_info.rsvd_pos,
> - screen_info.red_pos,
> - screen_info.green_pos,
> - screen_info.blue_pos);
> + pr_info("Truecolor: size=%d:%d:%d:%d, shift=%d:%d:%d:%d\n",
> + screen_info.rsvd_size,
> + screen_info.red_size,
> + screen_info.green_size,
> + screen_info.blue_size,
> + screen_info.rsvd_pos,
> + screen_info.red_pos,
> + screen_info.green_pos,
> + screen_info.blue_pos);
>
> efifb_fix.ypanstep = 0;
> efifb_fix.ywrapstep = 0;
> @@ -276,11 +273,11 @@ static int efifb_probe(struct platform_device *dev)
> info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
>
> if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
> - printk(KERN_ERR "efifb: cannot allocate colormap\n");
> + pr_err("cannot allocate colormap\n");
> goto err_unmap;
> }
> if ((err = register_framebuffer(info)) < 0) {
> - printk(KERN_ERR "efifb: cannot register framebuffer\n");
> + pr_err("cannot register framebuffer\n");
> goto err_fb_dealoc;
> }
> fb_info(info, "%s frame buffer device\n", info->fix.id);
>
>
^ permalink raw reply related
* Re: [PATCH v6 2/2] fbcon: expose cursor blink interval via sysfs
From: Tomi Valkeinen @ 2015-02-20 12:04 UTC (permalink / raw)
To: Scot Doyle, Jean-Christophe Plagniol-Villard
Cc: Geert Uytterhoeven, Richard Weinberger, linux-fbdev, linux-kernel
In-Reply-To: <alpine.DEB.2.11.1501300938070.2359@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]
On 30/01/15 11:40, Scot Doyle wrote:
> fbcon toggles cursor display state every 200 milliseconds when blinking.
> Since users prefer different toggle intervals, expose the interval via
> /sys/class/graphics/fbcon/cursor_blink_ms so that it may be customized.
>
> Values written to the interface set the approximate time interval in
> milliseconds between cursor toggles, from 1 to 32767. Since the interval
> is stored internally as a number of jiffies, the millisecond value read
> from the interface may not exactly match the entered value.
>
> An outstanding blink timer is reset after a new value is entered.
>
> If the cursor blink is disabled, either via the 'cursor_blink' boolean
> setting or some other mechanism, the 'cursor_blink_ms' setting may still
> be modified. The new value will be used if the blink is reactivated.
>
> Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> ---
> drivers/video/console/fbcon.c | 65 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 7a2030b..7baa333 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -3495,11 +3495,76 @@ err:
> return count;
> }
>
> +static ssize_t show_cursor_blink_ms(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + struct fbcon_ops *ops;
> + int idx, ms = -1;
> +
> + if (fbcon_has_exited)
> + return -ENODEV;
> +
> + console_lock();
> + idx = con2fb_map[fg_console];
> +
> + if (idx != -1 && registered_fb[idx] != NULL) {
> + ops = ((struct fb_info *)registered_fb[idx])->fbcon_par;
I don't think the above typecast is needed. I can remove the typecast
when applying to my tree.
So queuing for 3.21. Thanks!
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: sm501fb: use memset_io
From: Tomi Valkeinen @ 2015-02-20 12:10 UTC (permalink / raw)
To: Sudip Mukherjee, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <1423571852-14511-1-git-send-email-sudipm.mukherjee@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]
On 10/02/15 14:37, Sudip Mukherjee wrote:
> we should really be using memset_io() instead of using memset() as
> this is actually io space mapped into our memory.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> it has only been build tested. I only have the hardware of sm712 and sm750.
>
> drivers/video/fbdev/sm501fb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
> index e8d4121..d0a4e2f 100644
> --- a/drivers/video/fbdev/sm501fb.c
> +++ b/drivers/video/fbdev/sm501fb.c
> @@ -1606,7 +1606,7 @@ static int sm501fb_start(struct sm501fb_info *info,
> info->fbmem_len = resource_size(res);
>
> /* clear framebuffer memory - avoids garbage data on unused fb */
> - memset(info->fbmem, 0, info->fbmem_len);
> + memset_io(info->fbmem, 0, info->fbmem_len);
>
> /* clear palette ram - undefined at power on */
> for (k = 0; k < (256 * 3); k++)
>
Thanks, queued for 3.21.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: Add missing error check for devm_kzalloc
From: Tomi Valkeinen @ 2015-02-20 12:12 UTC (permalink / raw)
To: Kiran Padwal
Cc: Russell King, Jean-Christophe Plagniol-Villard, linux-fbdev,
linux-kernel, Kiran Padwal
In-Reply-To: <1423647406-858-1-git-send-email-kiran.padwal@smartplayin.com>
[-- Attachment #1: Type: text/plain, Size: 909 bytes --]
On 11/02/15 11:36, Kiran Padwal wrote:
> This patch add a missing check on the return value of devm_kzalloc,
> which would cause a NULL pointer dereference in a OOM situation.
>
> Signed-off-by: Kiran Padwal <kiran.padwal@smartplayin.com>
> ---
> drivers/video/fbdev/amba-clcd.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
> index 32c0b6b..9362424 100644
> --- a/drivers/video/fbdev/amba-clcd.c
> +++ b/drivers/video/fbdev/amba-clcd.c
> @@ -599,6 +599,9 @@ static int clcdfb_of_get_mode(struct device *dev, struct device_node *endpoint,
>
> len = clcdfb_snprintf_mode(NULL, 0, mode);
> name = devm_kzalloc(dev, len + 1, GFP_KERNEL);
> + if (!name)
> + return -ENOMEM;
> +
> clcdfb_snprintf_mode(name, len + 1, mode);
> mode->name = name;
>
>
Thanks, queued for 3.20.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: fix possible null dereference
From: Tomi Valkeinen @ 2015-02-20 12:13 UTC (permalink / raw)
To: Sudip Mukherjee, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel
In-Reply-To: <1423756056-15108-1-git-send-email-sudipm.mukherjee@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]
On 12/02/15 17:47, Sudip Mukherjee wrote:
> we were dereferencing edid first and the NULL check was after
> accessing that. now we are using edid only if we know that
> it is not NULL.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/video/fbdev/core/fbmon.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
> index 9533859..868facd 100644
> --- a/drivers/video/fbdev/core/fbmon.c
> +++ b/drivers/video/fbdev/core/fbmon.c
> @@ -624,9 +624,6 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
> int num = 0, i, first = 1;
> int ver, rev;
>
> - ver = edid[EDID_STRUCT_VERSION];
> - rev = edid[EDID_STRUCT_REVISION];
> -
> mode = kzalloc(50 * sizeof(struct fb_videomode), GFP_KERNEL);
> if (mode == NULL)
> return NULL;
> @@ -637,6 +634,9 @@ static struct fb_videomode *fb_create_modedb(unsigned char *edid, int *dbsize,
> return NULL;
> }
>
> + ver = edid[EDID_STRUCT_VERSION];
> + rev = edid[EDID_STRUCT_REVISION];
> +
> *dbsize = 0;
>
> DPRINTK(" Detailed Timings\n");
>
Thanks, queued for 3.20.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: make of_device_id array const
From: Tomi Valkeinen @ 2015-02-20 12:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1423479747-17899-1-git-send-email-sanjeev_sharma@mentor.com>
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]
On 09/02/15 13:02, Sanjeev Sharma wrote:
> Make of_device_id array const.
>
> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
> ---
> drivers/video/fbdev/imxfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
> index 3b6a3c8..84d1d29 100644
> --- a/drivers/video/fbdev/imxfb.c
> +++ b/drivers/video/fbdev/imxfb.c
> @@ -183,7 +183,7 @@ static struct platform_device_id imxfb_devtype[] = {
> };
> MODULE_DEVICE_TABLE(platform, imxfb_devtype);
>
> -static struct of_device_id imxfb_of_dev_id[] = {
> +static const struct of_device_id imxfb_of_dev_id[] = {
> {
> .compatible = "fsl,imx1-fb",
> .data = &imxfb_devtype[IMX1_FB],
>
Thanks, queued for 3.21.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: use msecs_to_jiffies for time conversion
From: Tomi Valkeinen @ 2015-02-20 12:24 UTC (permalink / raw)
To: Nicholas Mc Guire, Jean-Christophe Plagniol-Villard
Cc: Jingoo Han, Daniel Vetter, Fabian Frederick, Laurent Pinchart,
Wolfram Sang, linux-fbdev, linux-kernel
In-Reply-To: <1423214041-15818-1-git-send-email-hofrat@osadl.org>
[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]
On 06/02/15 11:14, Nicholas Mc Guire wrote:
> This is only an API consolidation and should make things more readable
> it replaces var * HZ / 1000 by msecs_to_jiffies(var).
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
>
> Patch was only compile tested with viper_defconfig (implies CONFIG_FB_PXA=m)
>
> Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
>
> drivers/video/fbdev/pxafb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
> index da2431e..d8b4743 100644
> --- a/drivers/video/fbdev/pxafb.c
> +++ b/drivers/video/fbdev/pxafb.c
> @@ -1285,7 +1285,7 @@ static int pxafb_smart_thread(void *arg)
> mutex_unlock(&fbi->ctrlr_lock);
>
> set_current_state(TASK_INTERRUPTIBLE);
> - schedule_timeout(30 * HZ / 1000);
> + schedule_timeout(msecs_to_jiffies(30));
> }
>
> pr_debug("%s(): task ending\n", __func__);
> @@ -1460,7 +1460,7 @@ static void pxafb_disable_controller(struct pxafb_info *fbi)
> #ifdef CONFIG_FB_PXA_SMARTPANEL
> if (fbi->lccr0 & LCCR0_LCDT) {
> wait_for_completion_timeout(&fbi->refresh_done,
> - 200 * HZ / 1000);
> + msecs_to_jiffies(200);
That will not compile.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: via/via_clock: fix sparse warning
From: Tomi Valkeinen @ 2015-02-20 12:26 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Florian Tobias Schandinat, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <1423140848-5989-1-git-send-email-prabhakar.csengg@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]
On 05/02/15 14:54, Lad Prabhakar wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> this patch fixes following sparse warning:
>
> via_clock.c:33:12: warning: symbol 'via_slap' was not declared. Should it be static?
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---
> Found this issue on linux-next (gcc version 4.9.2,
> sparse version 0.4.5-rc1)and applies on top linux-next.
>
> drivers/video/fbdev/via/via_clock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/via/via_clock.c b/drivers/video/fbdev/via/via_clock.c
> index db1e392..bf269fa 100644
> --- a/drivers/video/fbdev/via/via_clock.c
> +++ b/drivers/video/fbdev/via/via_clock.c
> @@ -30,7 +30,7 @@
> #include "global.h"
> #include "debug.h"
>
> -const char *via_slap = "Please slap VIA Technologies to motivate them "
> +static const char *via_slap = "Please slap VIA Technologies to motivate them "
> "releasing full documentation for your platform!\n";
>
> static inline u32 cle266_encode_pll(struct via_pll_config pll)
>
Thanks, queued for 3.21.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: fbdev: use msecs_to_jiffies for time conversion
From: Nicholas Mc Guire @ 2015-02-20 14:01 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Nicholas Mc Guire, Jean-Christophe Plagniol-Villard, Jingoo Han,
Daniel Vetter, Fabian Frederick, Laurent Pinchart, Wolfram Sang,
linux-fbdev, linux-kernel
In-Reply-To: <54E72774.1070809@ti.com>
On Fri, 20 Feb 2015, Tomi Valkeinen wrote:
> On 06/02/15 11:14, Nicholas Mc Guire wrote:
> > This is only an API consolidation and should make things more readable
> > it replaces var * HZ / 1000 by msecs_to_jiffies(var).
> >
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > ---
> >
> > Patch was only compile tested with viper_defconfig (implies CONFIG_FB_PXA=m)
> >
> > Patch is against 3.19.0-rc7 (localversion-next is -next-20150204)
> >
> > drivers/video/fbdev/pxafb.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
> > index da2431e..d8b4743 100644
> > --- a/drivers/video/fbdev/pxafb.c
> > +++ b/drivers/video/fbdev/pxafb.c
> > @@ -1285,7 +1285,7 @@ static int pxafb_smart_thread(void *arg)
> > mutex_unlock(&fbi->ctrlr_lock);
> >
> > set_current_state(TASK_INTERRUPTIBLE);
> > - schedule_timeout(30 * HZ / 1000);
> > + schedule_timeout(msecs_to_jiffies(30));
> > }
> >
> > pr_debug("%s(): task ending\n", __func__);
> > @@ -1460,7 +1460,7 @@ static void pxafb_disable_controller(struct pxafb_info *fbi)
> > #ifdef CONFIG_FB_PXA_SMARTPANEL
> > if (fbi->lccr0 & LCCR0_LCDT) {
> > wait_for_completion_timeout(&fbi->refresh_done,
> > - 200 * HZ / 1000);
> > + msecs_to_jiffies(200);
>
> That will not compile.
>
Just reran it on linux-next 3.19.0 -next-20150220
for viper_defconfig using gcc version 4.9.2 20140904 (prerelease)
(crosstool-NG linaro-1.13.1-4.9-2014.09 - Linaro GCC 4.9-2014.09)
root@debian:~/linux-next# patch -p1 < 0001-video-fbdev-use-msecs_to_jiffies-for-time-convrsion.patch
patching file drivers/video/fbdev/pxafb.c
root@debian:~/linux-next# make viper_defconfig ARCH=arm
#
# configuration written to .config
#
root@debian:~/linux-next# LANG=en_US make modules ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-
...
CC [M] drivers/video/console/softcursor.o
CC [M] drivers/video/fbdev/pxafb.o
CC [M] drivers/video/fbdev/core/cfbfillrect.o
CC [M] drivers/video/fbdev/core/cfbcopyarea.o
CC [M] drivers/video/fbdev/core/cfbimgblt.o
CC [M] sound/sound_core.o
LD [M] sound/soundcore.o
...
CC drivers/video/console/softcursor.mod.o
LDFINAL [M] drivers/video/console/softcursor.ko
CC drivers/video/fbdev/core/cfbcopyarea.mod.o
LDFINAL [M] drivers/video/fbdev/core/cfbcopyarea.ko
CC drivers/video/fbdev/core/cfbfillrect.mod.o
LDFINAL [M] drivers/video/fbdev/core/cfbfillrect.ko
CC drivers/video/fbdev/core/cfbimgblt.mod.o
LDFINAL [M] drivers/video/fbdev/core/cfbimgblt.ko
CC drivers/video/fbdev/pxafb.mod.o
LDFINAL [M] drivers/video/fbdev/pxafb.ko
CC fs/configfs/configfs.mod.o
LDFINAL [M] fs/configfs/configfs.ko
...
No compile warning or errors
could you send me the compile error message and the toolchain you
are using - this change should not really have any noticable impact.
Is this a toolchain issue ?
thx!
hofrat
^ permalink raw reply
* Re: [PATCH] video: fbdev: use msecs_to_jiffies for time conversion
From: Tomi Valkeinen @ 2015-02-20 14:06 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Nicholas Mc Guire, Jean-Christophe Plagniol-Villard, Jingoo Han,
Daniel Vetter, Fabian Frederick, Laurent Pinchart, Wolfram Sang,
linux-fbdev, linux-kernel
In-Reply-To: <20150220140110.GA25767@opentech.at>
[-- Attachment #1: Type: text/plain, Size: 810 bytes --]
On 20/02/15 16:01, Nicholas Mc Guire wrote:
>>> pr_debug("%s(): task ending\n", __func__);
>>> @@ -1460,7 +1460,7 @@ static void pxafb_disable_controller(struct pxafb_info *fbi)
>>> #ifdef CONFIG_FB_PXA_SMARTPANEL
>>> if (fbi->lccr0 & LCCR0_LCDT) {
>>> wait_for_completion_timeout(&fbi->refresh_done,
>>> - 200 * HZ / 1000);
>>> + msecs_to_jiffies(200);
>>
>> That will not compile.
> No compile warning or errors
>
> could you send me the compile error message and the toolchain you
> are using - this change should not really have any noticable impact.
I didn't compile it. It's missing a closing parenthesis.
This is one reason I'm not very fond of cleanups to drivers that the
patch sender cannot test... They may cause more problems than they help.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- 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