* Re: [PATCH] offb: Fix little-endian support
From: Cedric Le Goater @ 2014-05-14 17:04 UTC (permalink / raw)
To: Takashi Iwai
Cc: Dinar Valeev, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <s5hha4sh20d.wl%tiwai@suse.de>
On 05/14/2014 04:24 PM, Takashi Iwai wrote:
> At Wed, 14 May 2014 16:01:17 +0200,
> Cedric Le Goater wrote:
>>
>> Hi Iwai-san,
>>
>> On 05/14/2014 03:21 PM, Takashi Iwai wrote:
>>> Although the color palette was corrected for little endian by the
>>> commit [e1edf18b: offb: Add palette hack for little endian], the
>>> graphics mode is still shown in psychedelic colors.
>>
>> Are you referring to the linux logo colors ? If so, could you please
>> try the patch below, it should be a fix.
>
> Not only penguin logo but the whole X graphics got strange colors,
> too, according to the bug report. I put the original reporter/tester
> (Dinar Valeev) to Cc.
> I'm merely a person who tries to fix this mess ;)
>
> BTW, did you try to run X on fbdev?
Not at the time, I was working on the console only, BE and LE.
I just tried fbdev and indeed this is a psychedelic mess :)
Your fix has also issues on BE and console and the patch of mine
below is of no use for fbdev. Damn, this is a nightmare.
C.
>>> For fixing this
>>> properly, we rather need to correct the RGB offsets depending on
>>> endianess.
>>>
>>> Since the RGB base offsets are corrected, we don't need the hack for
>>> pallette color entries. This patch reverts that, too.
>>
>> Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
>> the depth to 8,15,16,32 ?
>
> Yes, it was with qemu -vga std -vnc :x.
> About different color depths, Dinar can test / clarify better, I
> suppose.
>
>> I think the patch might be breaking big endian
>> too.
>
> Big endian should work as is because my patch uses the original
> offsets when fb_be_math() is true. It corrects the RGB offsets if
> !fb_be_math().
>
> But, I'm also entirely not sure whether this is 100% correct, either.
> Namely, if the RGB offsets were correct for some little endian
> machines with offb, my patch would break it, of course. But, then
> your previous fix must have already broken it as well, so I took the
> same fb_be_math() check.
>
>
> thanks,
>
> Takashi
>
>> Now, I am far from being an expert on frame buffers. It would be glad
>> to have some insights on that topic.
>>
>> Thanks,
>>
>> C.
>>
>>
>> [PATCH] fb: fix logo palette entries for little endian
>>
>> The offb_cmap_byteswap() routine helps byteswapping the color map
>> entries when required. This patch externalizes and renames the helper
>> routine to adjust the pseudo palette of the logo when running on
>> little endian.
>>
>> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
>> ---
>> drivers/video/fbmem.c | 6 ++++--
>> drivers/video/offb.c | 11 +----------
>> include/linux/fb.h | 8 ++++++++
>> 3 files changed, 13 insertions(+), 12 deletions(-)
>>
>> Index: linux.git/drivers/video/fbmem.c
>> =================================>> --- linux.git.orig/drivers/video/fbmem.c
>> +++ linux.git/drivers/video/fbmem.c
>> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
>> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
>>
>> for ( i = 0; i < logo->clutsize; i++) {
>> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
>> + palette[i+32] = fb_cmap_byteswap(info,
>> + safe_shift((clut[0] & redmask), redshift) |
>> safe_shift((clut[1] & greenmask), greenshift) |
>> safe_shift((clut[2] & bluemask), blueshift));
>> clut += 3;
>> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
>> blueshift = info->var.blue.offset;
>>
>> for (i = 32; i < 32 + logo->clutsize; i++)
>> - palette[i] = i << redshift | i << greenshift | i << blueshift;
>> + palette[i] = fb_cmap_byteswap(info, i << redshift |
>> + i << greenshift | i << blueshift);
>> }
>>
>> static void fb_set_logo(struct fb_info *info,
>> Index: linux.git/drivers/video/offb.c
>> =================================>> --- linux.git.orig/drivers/video/offb.c
>> +++ linux.git/drivers/video/offb.c
>> @@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
>> #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
>> #define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
>>
>> -#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
>> -
>> -static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
>> -{
>> - u32 bpp = info->var.bits_per_pixel;
>> -
>> - return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
>> -}
>> -
>> /*
>> * Set a single color register. The values supplied are already
>> * rounded down to the hardware's capabilities (according to the
>> @@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
>> mask <<= info->var.transp.offset;
>> value |= mask;
>> }
>> - pal[regno] = offb_cmap_byteswap(info, value);
>> + pal[regno] = fb_cmap_byteswap(info, value);
>> return 0;
>> }
>>
>> Index: linux.git/include/linux/fb.h
>> =================================>> --- linux.git.orig/include/linux/fb.h
>> +++ linux.git/include/linux/fb.h
>> @@ -582,6 +582,7 @@ static inline struct apertures_struct *a
>>
>> #endif
>>
>> +#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
>> #define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
>> #define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
>> (val) << (bits))
>> @@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
>> #endif /* CONFIG_FB_FOREIGN_ENDIAN */
>> }
>>
>> +static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
>> +{
>> + u32 bpp = info->var.bits_per_pixel;
>> +
>> + return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
>> +}
>> +
>> /* drivers/video/fbsysfs.c */
>> extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
>> extern void framebuffer_release(struct fb_info *info);
>>
>
^ permalink raw reply
* Re: [PATCH] offb: Fix little-endian support
From: Takashi Iwai @ 2014-05-14 17:57 UTC (permalink / raw)
To: Cedric Le Goater
Cc: Dinar Valeev, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
linux-fbdev, linux-kernel
In-Reply-To: <5373A200.6080301@fr.ibm.com>
At Wed, 14 May 2014 19:04:00 +0200,
Cedric Le Goater wrote:
>
> On 05/14/2014 04:24 PM, Takashi Iwai wrote:
> > At Wed, 14 May 2014 16:01:17 +0200,
> > Cedric Le Goater wrote:
> >>
> >> Hi Iwai-san,
> >>
> >> On 05/14/2014 03:21 PM, Takashi Iwai wrote:
> >>> Although the color palette was corrected for little endian by the
> >>> commit [e1edf18b: offb: Add palette hack for little endian], the
> >>> graphics mode is still shown in psychedelic colors.
> >>
> >> Are you referring to the linux logo colors ? If so, could you please
> >> try the patch below, it should be a fix.
> >
> > Not only penguin logo but the whole X graphics got strange colors,
> > too, according to the bug report. I put the original reporter/tester
> > (Dinar Valeev) to Cc.
> > I'm merely a person who tries to fix this mess ;)
> >
> > BTW, did you try to run X on fbdev?
>
> Not at the time, I was working on the console only, BE and LE.
> I just tried fbdev and indeed this is a psychedelic mess :)
>
> Your fix has also issues on BE and console and the patch of mine
> below is of no use for fbdev. Damn, this is a nightmare.
Hm, so it actually regressed on BE?
It's strange because fb_math_be() should be true and the patch won't
change the values in that case...
Takashi
>
> C.
>
> >>> For fixing this
> >>> properly, we rather need to correct the RGB offsets depending on
> >>> endianess.
> >>>
> >>> Since the RGB base offsets are corrected, we don't need the hack for
> >>> pallette color entries. This patch reverts that, too.
> >>
> >> Are you testing using qemu -vga std -vnc :x ? If so, did you try changing
> >> the depth to 8,15,16,32 ?
> >
> > Yes, it was with qemu -vga std -vnc :x.
> > About different color depths, Dinar can test / clarify better, I
> > suppose.
> >
> >> I think the patch might be breaking big endian
> >> too.
> >
> > Big endian should work as is because my patch uses the original
> > offsets when fb_be_math() is true. It corrects the RGB offsets if
> > !fb_be_math().
> >
> > But, I'm also entirely not sure whether this is 100% correct, either.
> > Namely, if the RGB offsets were correct for some little endian
> > machines with offb, my patch would break it, of course. But, then
> > your previous fix must have already broken it as well, so I took the
> > same fb_be_math() check.
> >
> >
> > thanks,
> >
> > Takashi
> >
> >> Now, I am far from being an expert on frame buffers. It would be glad
> >> to have some insights on that topic.
> >>
> >> Thanks,
> >>
> >> C.
> >>
> >>
> >> [PATCH] fb: fix logo palette entries for little endian
> >>
> >> The offb_cmap_byteswap() routine helps byteswapping the color map
> >> entries when required. This patch externalizes and renames the helper
> >> routine to adjust the pseudo palette of the logo when running on
> >> little endian.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> >> ---
> >> drivers/video/fbmem.c | 6 ++++--
> >> drivers/video/offb.c | 11 +----------
> >> include/linux/fb.h | 8 ++++++++
> >> 3 files changed, 13 insertions(+), 12 deletions(-)
> >>
> >> Index: linux.git/drivers/video/fbmem.c
> >> =================================> >> --- linux.git.orig/drivers/video/fbmem.c
> >> +++ linux.git/drivers/video/fbmem.c
> >> @@ -252,7 +252,8 @@ static void fb_set_logo_truepalette(str
> >> blueshift = info->var.blue.offset - (8 - info->var.blue.length);
> >>
> >> for ( i = 0; i < logo->clutsize; i++) {
> >> - palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
> >> + palette[i+32] = fb_cmap_byteswap(info,
> >> + safe_shift((clut[0] & redmask), redshift) |
> >> safe_shift((clut[1] & greenmask), greenshift) |
> >> safe_shift((clut[2] & bluemask), blueshift));
> >> clut += 3;
> >> @@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
> >> blueshift = info->var.blue.offset;
> >>
> >> for (i = 32; i < 32 + logo->clutsize; i++)
> >> - palette[i] = i << redshift | i << greenshift | i << blueshift;
> >> + palette[i] = fb_cmap_byteswap(info, i << redshift |
> >> + i << greenshift | i << blueshift);
> >> }
> >>
> >> static void fb_set_logo(struct fb_info *info,
> >> Index: linux.git/drivers/video/offb.c
> >> =================================> >> --- linux.git.orig/drivers/video/offb.c
> >> +++ linux.git/drivers/video/offb.c
> >> @@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
> >> #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN 0x6cd4
> >> #define AVIVO_DC_LUTB_WHITE_OFFSET_RED 0x6cd8
> >>
> >> -#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> >> -
> >> -static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
> >> -{
> >> - u32 bpp = info->var.bits_per_pixel;
> >> -
> >> - return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> >> -}
> >> -
> >> /*
> >> * Set a single color register. The values supplied are already
> >> * rounded down to the hardware's capabilities (according to the
> >> @@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
> >> mask <<= info->var.transp.offset;
> >> value |= mask;
> >> }
> >> - pal[regno] = offb_cmap_byteswap(info, value);
> >> + pal[regno] = fb_cmap_byteswap(info, value);
> >> return 0;
> >> }
> >>
> >> Index: linux.git/include/linux/fb.h
> >> =================================> >> --- linux.git.orig/include/linux/fb.h
> >> +++ linux.git/include/linux/fb.h
> >> @@ -582,6 +582,7 @@ static inline struct apertures_struct *a
> >>
> >> #endif
> >>
> >> +#define FB_RIGHT_POS(p, bpp) (fb_be_math(p) ? 0 : (32 - (bpp)))
> >> #define FB_LEFT_POS(p, bpp) (fb_be_math(p) ? (32 - (bpp)) : 0)
> >> #define FB_SHIFT_HIGH(p, val, bits) (fb_be_math(p) ? (val) >> (bits) : \
> >> (val) << (bits))
> >> @@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
> >> #endif /* CONFIG_FB_FOREIGN_ENDIAN */
> >> }
> >>
> >> +static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
> >> +{
> >> + u32 bpp = info->var.bits_per_pixel;
> >> +
> >> + return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
> >> +}
> >> +
> >> /* drivers/video/fbsysfs.c */
> >> extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
> >> extern void framebuffer_release(struct fb_info *info);
> >>
> >
>
^ permalink raw reply
* [PATCH] video : remove redundant error check
From: Daeseok Youn @ 2014-05-15 7:55 UTC (permalink / raw)
To: plagnioj
Cc: tomi.valkeinen, jg1.han, daeseok.youn, daniel.vetter,
Julia.Lawall, linux-fbdev, linux-kernel
From 060757f81f85babf393cc76714d49af25af7791d Mon Sep 17 00:00:00 2001
From: Daeseok Youn <daeseok.youn@gmail.com>
Date: Thu, 15 May 2014 16:51:35 +0900
Subject: [PATCH] video : remove redundant error check
It doesn't need to check "err" for printing info.
And also use pr_info instead of printk.
Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
drivers/video/fbdev/i810/i810_main.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/video/fbdev/i810/i810_main.c b/drivers/video/fbdev/i810/i810_main.c
index bb674e4..15cb397 100644
--- a/drivers/video/fbdev/i810/i810_main.c
+++ b/drivers/video/fbdev/i810/i810_main.c
@@ -1910,13 +1910,12 @@ static void i810fb_find_init_mode(struct fb_info *info)
for (i = 0; i < par->ddc_num + 1; i++) {
err = i810_probe_i2c_connector(info, &par->edid, i);
- if (!err)
+ if (!err) {
+ pr_info("i810fb_init_pci: DDC probe successful\n");
break;
+ }
}
- if (!err)
- printk("i810fb_init_pci: DDC probe successful\n");
-
fb_edid_to_monspecs(par->edid, specs);
if (specs->modedb = NULL)
--
1.7.1
^ permalink raw reply related
* [PATCH] OMAPDSS: move 'compatible' converter to omapdss driver
From: Tomi Valkeinen @ 2014-05-15 8:05 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap, linux-fbdev, Archit Taneja, Tomi Valkeinen
Move the panel/encoder driver compatible-string converter from
arch/arm/mach-omap2/display.c to omapdss driver. That is a more logical
place for it, as it's really an omapdss internal hack.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
arch/arm/mach-omap2/display.c | 56 -----------
drivers/video/fbdev/omap2/Makefile | 2 +-
drivers/video/fbdev/omap2/dss/Kconfig | 4 +
drivers/video/fbdev/omap2/dss/Makefile | 1 +
drivers/video/fbdev/omap2/dss/omapdss-boot-init.c | 112 ++++++++++++++++++++++
5 files changed, 118 insertions(+), 57 deletions(-)
create mode 100644 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 16d33d831287..519a20fc0432 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -555,65 +555,9 @@ int omap_dss_reset(struct omap_hwmod *oh)
return r;
}
-/* list of 'compatible' nodes to convert to omapdss specific */
-static const char * const dss_compat_conv_list[] __initconst = {
- "composite-connector",
- "dvi-connector",
- "hdmi-connector",
- "panel-dpi",
- "panel-dsi-cm",
- "sony,acx565akm",
- "svideo-connector",
- "ti,tfp410",
- "ti,tpd12s015",
-};
-
-/* prepend compatible string with "omapdss," */
-static __init void omapdss_omapify_node(struct device_node *node,
- const char *compat)
-{
- char *new_compat;
- struct property *prop;
-
- new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
-
- prop = kzalloc(sizeof(*prop), GFP_KERNEL);
-
- if (!prop) {
- pr_err("omapdss_omapify_node: kzalloc failed\n");
- return;
- }
-
- prop->name = "compatible";
- prop->value = new_compat;
- prop->length = strlen(new_compat) + 1;
-
- of_update_property(node, prop);
-}
-
-/*
- * As omapdss panel drivers are omapdss specific, but we want to define the
- * DT-data in generic manner, we convert the compatible strings of the panel
- * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
- * correct DT data and omapdss specific drivers.
- *
- * When we get generic panel drivers to the kernel, this will be removed.
- */
void __init omapdss_early_init_of(void)
{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
- const char *compat = dss_compat_conv_list[i];
- struct device_node *node = NULL;
-
- while ((node = of_find_compatible_node(node, NULL, compat))) {
- if (!of_device_is_available(node))
- continue;
- omapdss_omapify_node(node, compat);
- }
- }
}
struct device_node * __init omapdss_find_dss_of_node(void)
diff --git a/drivers/video/fbdev/omap2/Makefile b/drivers/video/fbdev/omap2/Makefile
index bf8127df8c71..f8745ec369cc 100644
--- a/drivers/video/fbdev/omap2/Makefile
+++ b/drivers/video/fbdev/omap2/Makefile
@@ -1,5 +1,5 @@
obj-$(CONFIG_OMAP2_VRFB) += vrfb.o
-obj-$(CONFIG_OMAP2_DSS) += dss/
+obj-y += dss/
obj-y += displays-new/
obj-$(CONFIG_FB_OMAP2) += omapfb/
diff --git a/drivers/video/fbdev/omap2/dss/Kconfig b/drivers/video/fbdev/omap2/dss/Kconfig
index dde4281663b1..a28f3a39ab1b 100644
--- a/drivers/video/fbdev/omap2/dss/Kconfig
+++ b/drivers/video/fbdev/omap2/dss/Kconfig
@@ -1,6 +1,10 @@
+config OMAP2_DSS_INIT
+ bool
+
menuconfig OMAP2_DSS
tristate "OMAP2+ Display Subsystem support"
select VIDEOMODE_HELPERS
+ select OMAP2_DSS_INIT
help
OMAP2+ Display Subsystem support.
diff --git a/drivers/video/fbdev/omap2/dss/Makefile b/drivers/video/fbdev/omap2/dss/Makefile
index 8aec8bda27cc..3b79ad74f2e1 100644
--- a/drivers/video/fbdev/omap2/dss/Makefile
+++ b/drivers/video/fbdev/omap2/dss/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
obj-$(CONFIG_OMAP2_DSS) += omapdss.o
# Core DSS files
omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
new file mode 100644
index 000000000000..468f6eff370c
--- /dev/null
+++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
@@ -0,0 +1,112 @@
+/*
+ * Copyright (C) 2014 Texas Instruments
+ * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+/* list of 'compatible' nodes to convert to omapdss specific */
+static const char * const dss_compat_conv_list[] __initconst = {
+ "composite-connector",
+ "dvi-connector",
+ "hdmi-connector",
+ "panel-dpi",
+ "panel-dsi-cm",
+ "sony,acx565akm",
+ "svideo-connector",
+ "ti,tfp410",
+ "ti,tpd12s015",
+};
+
+/* prepend compatible string with "omapdss," */
+static __init void omapdss_omapify_node(struct device_node *node,
+ const char *compat)
+{
+ char *new_compat;
+ struct property *prop;
+
+ new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
+
+ prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+
+ if (!prop) {
+ pr_err("omapdss_omapify_node: kzalloc failed\n");
+ return;
+ }
+
+ prop->name = "compatible";
+ prop->value = new_compat;
+ prop->length = strlen(new_compat) + 1;
+
+ of_update_property(node, prop);
+}
+
+static struct device_node * __init omapdss_find_dss_of_node(void)
+{
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap2-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap3-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap4-dss");
+ if (node)
+ return node;
+
+ node = of_find_compatible_node(NULL, NULL, "ti,omap5-dss");
+ if (node)
+ return node;
+
+ return NULL;
+}
+
+/*
+ * As omapdss panel drivers are omapdss specific, but we want to define the
+ * DT-data in generic manner, we convert the compatible strings of the panel
+ * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
+ * correct DT data and omapdss specific drivers.
+ *
+ * When we get generic panel drivers to the kernel, this will be removed.
+ */
+static int __init omap_dss_boot_init(void)
+{
+ int i;
+
+ /* do we have omapdss device? */
+ if (omapdss_find_dss_of_node() = NULL)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
+ const char *compat = dss_compat_conv_list[i];
+ struct device_node *node = NULL;
+
+ while ((node = of_find_compatible_node(node, NULL, compat))) {
+ if (!of_device_is_available(node))
+ continue;
+
+ omapdss_omapify_node(node, compat);
+ }
+ }
+
+ return 0;
+}
+
+subsys_initcall(omap_dss_boot_init);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513212639.GA18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On 14/05/14 00:26, Tony Lindgren wrote:
> + /* lcd MO */
> + ddata->mo_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 1, "mode");
> + if (PTR_ERR(ddata->mo_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + if (!IS_ERR(ddata->mo_gpio))
> + if (gpiod_get_raw_value_cansleep(ddata->mo_gpio))
> + ddata->flags |= SHARP_LS_QVGA;
Shouldn't there be an explicit flag in the DT data for this? If the
panel's MO pin is hardwired to, say, pull up, then the mode-gpios won't
have MO gpio, right? So something like:
mode-gpios = <0 /* high, lcd MO */
&gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
&gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
vga-mode; /* MO hardwired high */
Btw, the gpio.txt has each gpio inside <>:
chipsel-gpios = <&gpio1 12 0>,
<&gpio1 13 0>,
<0>, /* holes are permitted, means no GPIO 2 */
<&gpio2 2>;
Is that equivalent to having all gpios inside <>?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] ARM: dts: Add LCD panel sharp ls037v7dw01 support for omap3-evm and ldp
From: Tomi Valkeinen @ 2014-05-15 8:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513213205.GB18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]
On 14/05/14 00:32, Tony Lindgren wrote:
> +&lcd0 {
> + enable-gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>; /* gpio152, lcd INI */
> + reset-gpios = <&gpio5 27 GPIO_ACTIVE_HIGH>; /* gpio155, lcd RESB */
> + /*
> + * The LCD is sideways, so we want the VGA mode instead of
> + * QVGA mode. Probably also want to have omapfb.rotate=3
> + * in the kernel cmdline until there's a binding for that.
> + */
> + mode-gpios = <&gpio5 26 GPIO_ACTIVE_LOW /* gpio154, lcd MO */
> + &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
> + &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
I don't think that is correct. The panel bindings should define what the
first mode-gpio means. Looking at the panel spec, I think the definition
should be "enable QVGA mode". And in the board's dts above, the
GPIO_ACTIVE_x should tell which is one is correct polarity for QVGA
mode. Which is GPIO_ACTIVE_HIGH here.
If we want to tell the panel driver to use QVGA mode, we should do that
explicitly with a flag, not by hacking the GPIO polarities. It's the
panel driver's job to set the GPIO.
So in the previous mail I suggested the 'vga-mode' flag, but I think we
need actually two flags for each GPIO: one that's used to tell the
driver which mode we want, which is used if the panel driver has control
for the GPIO, and the other that tells which is the hardwired setting.
Then again, the two cases are exclusive, so maybe a single flag per mode
pin is ok. So, for the MO pin, we could have 'qvga-mode' flag in the
.dts, which means:
"If there is MO gpio, set MO high. If there's no MO gpio, presume MO pin
is pulled up"
Of course, one could argue that, in case MO is controlled with GPIO, the
'qvga-mode' flag is about SW level configuration, not hardware...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140514160216.GD18463@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 5274 bytes --]
On 14/05/14 19:02, Tony Lindgren wrote:
>> The video paths of the panels and encoders are connected using the v4l2
>> style ports/endpoints. We can use those to see what display controller a
>> panel is connected to, but only after the panel driver has already
>> probed. We don't have control for the actual probing, as that happens
>> with whatever the control bus is for the display component.
>
> OK. So with generic panels, you can just let the panels probe with
> the right compatible flag then and let the ports/endpoints registration
> to figure out if the panel is usable for the display controller in
> question.
I'm not sure what you mean here.
Do you mean with the future common display framework? There's no need to
figure out anything there, as supposedly the .dts has been written
correctly and the panel and the display controller work together.
If you mean with the current kernel, there's still nothing to figure
out. We can have only single driver with a particular compatible string.
And as our current drivers are omap specific, it makes sense to have
their compatible string be something omap-ish. And if the .dts file
connects the display controller and the panel, then they must be usable
together.
>>> Well it seems at least the reset and enable pin standard from that
>>> binding can be kept.
>>
>> Only enable gpio there. But even that's vague. Do you turn on the power
>> before or after setting the enable gpio? How long delay should be
>> between the power and the gpio? Different panels have different rules
>> for the power-up.
>
> Sure, it's a complex problem. But for the enable gpio..
>
> Maybe the enable GPIO should be treated as a regulator? That would allow
> specifying first the source regulator startup delay, and then the
> panel has it's own startup delay.
Well... I don't know. Sounds rather hacky to me. We have the option to
have a specific driver for this panel, and that driver can handle all
the delays and power-up sequences just right in a clean manner.
>>>>> But I'm not really familiar with using extending current bindings, and
>>>>> making new bindings compatible with old ones. Can you explain why it'd
>>>>> be good to have the sharp panel bindings compatible with simple-panel?
>>>>> In what kind of scenario would it be used?
>>>
>>> Ideally the panel binding just describes the panel and it should not
>>> matter which display controller it is a child of.
>>
>> Yes, but that means the panel bindings need to have enough information
>> so that all display controllers can use it. Simple-panel bindings do not
>> have enough information. The simple-panel bindings do not have
>> information about the video bus input, and it doesn't even have
>> information about the resolution or bitdepth of the panel.
>
> Some of that you can hide into the panel driver based on the compatible
> flag. So why not already do something like this in the .dts files
> instead of the remapping:
>
> compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01";
>
> And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c
> would only claim to be compatible with "sharp,ls037v7dw01-omap-dss".
>
> Then when the common panel framework is available, you can stop
> parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need
> to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss"
> in the .dts files.
Hmm, I don't see how this relates to the simple-panel bindings.
But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an
alternative for the compatible-string conversion we do now. I guess it's
a matter of taste, but I rather hide it inside the kernel, in an
internal omapdss file, than pollute the .dts files with those compatible
strings.
>> So I'm still asking, if we create sharp bindings that use the same
>> properties as the simple-panel bindings, and define that sharp panel is
>> compatible with simple-panel, what kind of scenario in practice would it
>> be used in?
>
> Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss"
> until some other SoC notices it can use the GPIO parts of the panel
> code at least :)
>
>> Would the scenario be some other OS, that doesn't have a driver for the
>> sharp panel, but has a driver for the simple-panel? That would only work
>> if the sharp panel hardware is setup so that only the enable gpio is
>> needed, so that quite a narrow definition of "compatible".
>
> That's where we can use the compatible flags and just avoid parsing
> the generic compatible flag unless some common framework is available.
Hmm, sorry, I don't follow. My question was about the simple-panel
bindings, not common display framework.
You were saying that the sharp bindings should be compatible with
simple-panel bindings. I'm still trying to understand what benefit does
that give us.
As I see it, the sharp panel could be used with the simple-panel
bindings only in certain special case, where all the mode pins and the
reset are hardwired in the board hardware, and they are hardwired in a
certain state (all hardwired low, probably), which matches what the
simple-panel driver expects.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 0/3] OMAPDSS: DT support for panel-lgphilips-lb035q02
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
Hi,
This adds DT support to panel-lgphilips-lb035q02. Compile tested only.
This panel is used on some Overo boards (alto35, if I'm not mistaken).
Tomi
Tomi Valkeinen (3):
OMAPDSS: panel-lgphilips-lb035q02: use gpiod for enable gpio
OMAPDSS: panel-lgphilips-lb035q02: Add DT support
Doc/DT: Add binding doc for lgphilips,lb035q02.txt
.../bindings/video/lgphilips,lb035q02.txt | 33 ++++++++++
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 76 +++++++++++++++++-----
2 files changed, 94 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
--
1.9.1
^ permalink raw reply
* [PATCH 1/3] OMAPDSS: panel-lgphilips-lb035q02: use gpiod for enable gpio
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
The new gpiod API supports automatic handling of active-high/active-low
with DT. To make it possible to use that when booting with DT, change
the driver's handling of the enable GPIO to use gpiod.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 32 ++++++++++++----------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
index 2e6b513222d9..6e6acd696a70 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
@@ -50,9 +50,10 @@ struct panel_drv_data {
struct omap_video_timings videomode;
- int reset_gpio;
+ /* used for non-DT boot, to be removed */
int backlight_gpio;
- int enable_gpio;
+
+ struct gpio_desc *enable_gpio;
};
#define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
@@ -165,8 +166,8 @@ static int lb035q02_enable(struct omap_dss_device *dssdev)
if (r)
return r;
- if (gpio_is_valid(ddata->enable_gpio))
- gpio_set_value_cansleep(ddata->enable_gpio, 1);
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 1);
if (gpio_is_valid(ddata->backlight_gpio))
gpio_set_value_cansleep(ddata->backlight_gpio, 1);
@@ -184,8 +185,8 @@ static void lb035q02_disable(struct omap_dss_device *dssdev)
if (!omapdss_device_is_enabled(dssdev))
return;
- if (gpio_is_valid(ddata->enable_gpio))
- gpio_set_value_cansleep(ddata->enable_gpio, 0);
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 0);
if (gpio_is_valid(ddata->backlight_gpio))
gpio_set_value_cansleep(ddata->backlight_gpio, 0);
@@ -243,6 +244,7 @@ static int lb035q02_probe_pdata(struct spi_device *spi)
const struct panel_lb035q02_platform_data *pdata;
struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
struct omap_dss_device *dssdev, *in;
+ int r;
pdata = dev_get_platdata(&spi->dev);
@@ -260,10 +262,19 @@ static int lb035q02_probe_pdata(struct spi_device *spi)
dssdev = &ddata->dssdev;
dssdev->name = pdata->name;
- ddata->enable_gpio = pdata->enable_gpio;
+ r = devm_gpio_request_one(&spi->dev, pdata->enable_gpio,
+ GPIOF_OUT_INIT_LOW, "panel enable");
+ if (r)
+ goto err_gpio;
+
+ ddata->enable_gpio = gpio_to_desc(pdata->enable_gpio);
+
ddata->backlight_gpio = pdata->backlight_gpio;
return 0;
+err_gpio:
+ omap_dss_put_device(ddata->in);
+ return r;
}
static int lb035q02_panel_spi_probe(struct spi_device *spi)
@@ -288,13 +299,6 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
return -ENODEV;
}
- if (gpio_is_valid(ddata->enable_gpio)) {
- r = devm_gpio_request_one(&spi->dev, ddata->enable_gpio,
- GPIOF_OUT_INIT_LOW, "panel enable");
- if (r)
- goto err_gpio;
- }
-
if (gpio_is_valid(ddata->backlight_gpio)) {
r = devm_gpio_request_one(&spi->dev, ddata->backlight_gpio,
GPIOF_OUT_INIT_LOW, "panel backlight");
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] OMAPDSS: panel-lgphilips-lb035q02: Add DT support
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap; +Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
Add DT support for panel-lgphilips-lb035q02.
We disable the use of the backlight_gpio as it should be handled via
backlight framework with DT boots.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
.../omap2/displays-new/panel-lgphilips-lb035q02.c | 44 +++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
index 6e6acd696a70..76c4fdc51c31 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-lgphilips-lb035q02.c
@@ -159,7 +159,8 @@ static int lb035q02_enable(struct omap_dss_device *dssdev)
if (omapdss_device_is_enabled(dssdev))
return 0;
- in->ops.dpi->set_data_lines(in, ddata->data_lines);
+ if (ddata->data_lines)
+ in->ops.dpi->set_data_lines(in, ddata->data_lines);
in->ops.dpi->set_timings(in, &ddata->videomode);
r = in->ops.dpi->enable(in);
@@ -277,6 +278,35 @@ err_gpio:
return r;
}
+static int lb035q02_probe_of(struct spi_device *spi)
+{
+ struct device_node *node = spi->dev.of_node;
+ struct panel_drv_data *ddata = dev_get_drvdata(&spi->dev);
+ struct omap_dss_device *in;
+ struct gpio_desc *gpio;
+
+ gpio = devm_gpiod_get(&spi->dev, "enable");
+ if (IS_ERR(gpio)) {
+ dev_err(&spi->dev, "failed to parse enable gpio\n");
+ return PTR_ERR(gpio);
+ } else {
+ gpiod_direction_output(gpio, 0);
+ ddata->enable_gpio = gpio;
+ }
+
+ ddata->backlight_gpio = -ENOENT;
+
+ in = omapdss_of_find_source_for_first_ep(node);
+ if (IS_ERR(in)) {
+ dev_err(&spi->dev, "failed to find video source\n");
+ return PTR_ERR(in);
+ }
+
+ ddata->in = in;
+
+ return 0;
+}
+
static int lb035q02_panel_spi_probe(struct spi_device *spi)
{
struct panel_drv_data *ddata;
@@ -295,6 +325,10 @@ static int lb035q02_panel_spi_probe(struct spi_device *spi)
r = lb035q02_probe_pdata(spi);
if (r)
return r;
+ } else if (spi->dev.of_node) {
+ r = lb035q02_probe_of(spi);
+ if (r)
+ return r;
} else {
return -ENODEV;
}
@@ -346,6 +380,13 @@ static int lb035q02_panel_spi_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id lb035q02_of_match[] = {
+ { .compatible = "omapdss,lgphilips,lb035q02", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, lb035q02_of_match);
+
static struct spi_driver lb035q02_spi_driver = {
.probe = lb035q02_panel_spi_probe,
.remove = lb035q02_panel_spi_remove,
@@ -357,6 +398,7 @@ static struct spi_driver lb035q02_spi_driver = {
module_spi_driver(lb035q02_spi_driver);
+MODULE_ALIAS("spi:lgphilips,lb035q02");
MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@ti.com>");
MODULE_DESCRIPTION("LG.Philips LB035Q02 LCD Panel driver");
MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [PATCH 3/3] Doc/DT: Add binding doc for lgphilips,lb035q02.txt
From: Tomi Valkeinen @ 2014-05-15 10:10 UTC (permalink / raw)
To: linux-fbdev, linux-omap
Cc: Archit Taneja, Florian Vaussard, Tomi Valkeinen, devicetree
In-Reply-To: <1400148637-17726-1-git-send-email-tomi.valkeinen@ti.com>
Add DT bindings documentation for LG.Philips LB035Q02 LCD panel.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: devicetree@vger.kernel.org
---
.../bindings/video/lgphilips,lb035q02.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
diff --git a/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt b/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
new file mode 100644
index 000000000000..1a1e653e5407
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
@@ -0,0 +1,33 @@
+LG.Philips LB035Q02 Panel
+============+
+Required properties:
+- compatible: "lgphilips,lb035q02"
+- enable-gpios: panel enable gpio
+
+Optional properties:
+- label: a symbolic name for the panel
+
+Required nodes:
+- Video port for DPI input
+
+Example
+-------
+
+lcd-panel: panel@0 {
+ compatible = "lgphilips,lb035q02";
+ reg = <0>;
+ spi-max-frequency = <100000>;
+ spi-cpol;
+ spi-cpha;
+
+ label = "lcd";
+
+ enable-gpios = <&gpio7 7 0>;
+
+ port {
+ lcd_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] video: of: display_timing: remove two unsafe error messages
From: Tomi Valkeinen @ 2014-05-15 10:47 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1399985901-30756-1-git-send-email-l.stach@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 463 bytes --]
On 13/05/14 15:58, Lucas Stach wrote:
> The error message would try to dereference the pointer that
> just has been tested to be NULL. As those messages don't
> really add any value without the info that the np could
> provide, just remove them.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/video/of_display_timing.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
Thanks, queued for 3.16.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] matroxfb: perform a dummy read of M_STATUS
From: Mikulas Patocka @ 2014-05-15 10:58 UTC (permalink / raw)
To: linux-fbdev
I had occasional screen corruption with the matrox framebuffer driver and
I found out that the reason for the corruption is that the hardware
blitter accesses the videoram while it is being written to.
The matrox driver has a macro WaitTillIdle() that should wait until the
blitter is idle, but it sometimes doesn't work. I added a dummy read
mga_inl(M_STATUS) to WaitTillIdle() to fix the problem. The dummy read
will flush the write buffer in the PCI chipset, and the next read of
M_STATUS will return the hardware status.
Since applying this patch, I had no screen corruption at all.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/video/matrox/matroxfb_base.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-3.14/drivers/video/matrox/matroxfb_base.h
=================================--- linux-3.14.orig/drivers/video/matrox/matroxfb_base.h 2014-04-08 00:35:19.460051657 +0200
+++ linux-3.14/drivers/video/matrox/matroxfb_base.h 2014-05-03 18:38:18.882396833 +0200
@@ -698,7 +698,7 @@ void matroxfb_unregister_driver(struct m
#define mga_fifo(n) do {} while ((mga_inl(M_FIFOSTATUS) & 0xFF) < (n))
-#define WaitTillIdle() do {} while (mga_inl(M_STATUS) & 0x10000)
+#define WaitTillIdle() do { mga_inl(M_STATUS); do {} while (mga_inl(M_STATUS) & 0x10000); } while (0)
/* code speedup */
#ifdef CONFIG_FB_MATROX_MILLENIUM
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-15 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140513212639.GA18001@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 963 bytes --]
On 14/05/14 00:26, Tony Lindgren wrote:
> +static int sharp_ls_probe_of(struct platform_device *pdev)
> +{
> + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct device_node *node = pdev->dev.of_node;
> + struct omap_dss_device *in;
> +
> + ddata->vcc = devm_regulator_get(&pdev->dev, "envdd");
> + if (IS_ERR(ddata->vcc)) {
> + dev_err(&pdev->dev, "failed to get regulator\n");
> + return PTR_ERR(ddata->vcc);
> + }
> +
> + /* lcd INI */
> + ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
> + if (PTR_ERR(ddata->ini_gpio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
Hmm, the GPIOs are optional, but shouldn't we react somehow to real
errors? I guess we should do something like:
ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
if (IS_ERR(ddata->ini_gpio) {
int err = PTR_ERR(ddata->ini_gpio);
if (err == -EPROBE_DEFER || err != -ENOENT)
return err;
}
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] OMAPDSS: move 'compatible' converter to omapdss driver
From: Tony Lindgren @ 2014-05-15 18:13 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <1400141136-25358-1-git-send-email-tomi.valkeinen@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 01:06]:
> Move the panel/encoder driver compatible-string converter from
> arch/arm/mach-omap2/display.c to omapdss driver. That is a more logical
> place for it, as it's really an omapdss internal hack.
Thanks, maybe also consider just using the compatible flags
instead as discussed in the other thread. Anyways for moving
this out of mach-omap2:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/display.c | 56 -----------
> drivers/video/fbdev/omap2/Makefile | 2 +-
> drivers/video/fbdev/omap2/dss/Kconfig | 4 +
> drivers/video/fbdev/omap2/dss/Makefile | 1 +
> drivers/video/fbdev/omap2/dss/omapdss-boot-init.c | 112 ++++++++++++++++++++++
> 5 files changed, 118 insertions(+), 57 deletions(-)
> create mode 100644 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
>
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 16d33d831287..519a20fc0432 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -555,65 +555,9 @@ int omap_dss_reset(struct omap_hwmod *oh)
> return r;
> }
>
> -/* list of 'compatible' nodes to convert to omapdss specific */
> -static const char * const dss_compat_conv_list[] __initconst = {
> - "composite-connector",
> - "dvi-connector",
> - "hdmi-connector",
> - "panel-dpi",
> - "panel-dsi-cm",
> - "sony,acx565akm",
> - "svideo-connector",
> - "ti,tfp410",
> - "ti,tpd12s015",
> -};
> -
> -/* prepend compatible string with "omapdss," */
> -static __init void omapdss_omapify_node(struct device_node *node,
> - const char *compat)
> -{
> - char *new_compat;
> - struct property *prop;
> -
> - new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
> -
> - prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> -
> - if (!prop) {
> - pr_err("omapdss_omapify_node: kzalloc failed\n");
> - return;
> - }
> -
> - prop->name = "compatible";
> - prop->value = new_compat;
> - prop->length = strlen(new_compat) + 1;
> -
> - of_update_property(node, prop);
> -}
> -
> -/*
> - * As omapdss panel drivers are omapdss specific, but we want to define the
> - * DT-data in generic manner, we convert the compatible strings of the panel
> - * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
> - * correct DT data and omapdss specific drivers.
> - *
> - * When we get generic panel drivers to the kernel, this will be removed.
> - */
> void __init omapdss_early_init_of(void)
> {
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
> - const char *compat = dss_compat_conv_list[i];
> - struct device_node *node = NULL;
> -
> - while ((node = of_find_compatible_node(node, NULL, compat))) {
> - if (!of_device_is_available(node))
> - continue;
>
> - omapdss_omapify_node(node, compat);
> - }
> - }
> }
>
> struct device_node * __init omapdss_find_dss_of_node(void)
> diff --git a/drivers/video/fbdev/omap2/Makefile b/drivers/video/fbdev/omap2/Makefile
> index bf8127df8c71..f8745ec369cc 100644
> --- a/drivers/video/fbdev/omap2/Makefile
> +++ b/drivers/video/fbdev/omap2/Makefile
> @@ -1,5 +1,5 @@
> obj-$(CONFIG_OMAP2_VRFB) += vrfb.o
>
> -obj-$(CONFIG_OMAP2_DSS) += dss/
> +obj-y += dss/
> obj-y += displays-new/
> obj-$(CONFIG_FB_OMAP2) += omapfb/
> diff --git a/drivers/video/fbdev/omap2/dss/Kconfig b/drivers/video/fbdev/omap2/dss/Kconfig
> index dde4281663b1..a28f3a39ab1b 100644
> --- a/drivers/video/fbdev/omap2/dss/Kconfig
> +++ b/drivers/video/fbdev/omap2/dss/Kconfig
> @@ -1,6 +1,10 @@
> +config OMAP2_DSS_INIT
> + bool
> +
> menuconfig OMAP2_DSS
> tristate "OMAP2+ Display Subsystem support"
> select VIDEOMODE_HELPERS
> + select OMAP2_DSS_INIT
> help
> OMAP2+ Display Subsystem support.
>
> diff --git a/drivers/video/fbdev/omap2/dss/Makefile b/drivers/video/fbdev/omap2/dss/Makefile
> index 8aec8bda27cc..3b79ad74f2e1 100644
> --- a/drivers/video/fbdev/omap2/dss/Makefile
> +++ b/drivers/video/fbdev/omap2/dss/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_OMAP2_DSS_INIT) += omapdss-boot-init.o
> obj-$(CONFIG_OMAP2_DSS) += omapdss.o
> # Core DSS files
> omapdss-y := core.o dss.o dss_features.o dispc.o dispc_coefs.o display.o \
> diff --git a/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
> new file mode 100644
> index 000000000000..468f6eff370c
> --- /dev/null
> +++ b/drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright (C) 2014 Texas Instruments
> + * Author: Tomi Valkeinen <tomi.valkeinen@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +/* list of 'compatible' nodes to convert to omapdss specific */
> +static const char * const dss_compat_conv_list[] __initconst = {
> + "composite-connector",
> + "dvi-connector",
> + "hdmi-connector",
> + "panel-dpi",
> + "panel-dsi-cm",
> + "sony,acx565akm",
> + "svideo-connector",
> + "ti,tfp410",
> + "ti,tpd12s015",
> +};
> +
> +/* prepend compatible string with "omapdss," */
> +static __init void omapdss_omapify_node(struct device_node *node,
> + const char *compat)
> +{
> + char *new_compat;
> + struct property *prop;
> +
> + new_compat = kasprintf(GFP_KERNEL, "omapdss,%s", compat);
> +
> + prop = kzalloc(sizeof(*prop), GFP_KERNEL);
> +
> + if (!prop) {
> + pr_err("omapdss_omapify_node: kzalloc failed\n");
> + return;
> + }
> +
> + prop->name = "compatible";
> + prop->value = new_compat;
> + prop->length = strlen(new_compat) + 1;
> +
> + of_update_property(node, prop);
> +}
> +
> +static struct device_node * __init omapdss_find_dss_of_node(void)
> +{
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap2-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap3-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap4-dss");
> + if (node)
> + return node;
> +
> + node = of_find_compatible_node(NULL, NULL, "ti,omap5-dss");
> + if (node)
> + return node;
> +
> + return NULL;
> +}
> +
> +/*
> + * As omapdss panel drivers are omapdss specific, but we want to define the
> + * DT-data in generic manner, we convert the compatible strings of the panel
> + * nodes from "panel-foo" to "omapdss,panel-foo". This way we can have both
> + * correct DT data and omapdss specific drivers.
> + *
> + * When we get generic panel drivers to the kernel, this will be removed.
> + */
> +static int __init omap_dss_boot_init(void)
> +{
> + int i;
> +
> + /* do we have omapdss device? */
> + if (omapdss_find_dss_of_node() = NULL)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(dss_compat_conv_list); ++i) {
> + const char *compat = dss_compat_conv_list[i];
> + struct device_node *node = NULL;
> +
> + while ((node = of_find_compatible_node(node, NULL, compat))) {
> + if (!of_device_is_available(node))
> + continue;
> +
> + omapdss_omapify_node(node, compat);
> + }
> + }
> +
> + return 0;
> +}
> +
> +subsys_initcall(omap_dss_boot_init);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tony Lindgren @ 2014-05-15 18:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <537487AE.3060906@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 02:24]:
> On 14/05/14 19:02, Tony Lindgren wrote:
>
> >> The video paths of the panels and encoders are connected using the v4l2
> >> style ports/endpoints. We can use those to see what display controller a
> >> panel is connected to, but only after the panel driver has already
> >> probed. We don't have control for the actual probing, as that happens
> >> with whatever the control bus is for the display component.
> >
> > OK. So with generic panels, you can just let the panels probe with
> > the right compatible flag then and let the ports/endpoints registration
> > to figure out if the panel is usable for the display controller in
> > question.
>
> I'm not sure what you mean here.
>
> Do you mean with the future common display framework? There's no need to
> figure out anything there, as supposedly the .dts has been written
> correctly and the panel and the display controller work together.
OK. Yes I meant for future use.
> If you mean with the current kernel, there's still nothing to figure
> out. We can have only single driver with a particular compatible string.
> And as our current drivers are omap specific, it makes sense to have
> their compatible string be something omap-ish. And if the .dts file
> connects the display controller and the panel, then they must be usable
> together.
Yup.
> >>> Well it seems at least the reset and enable pin standard from that
> >>> binding can be kept.
> >>
> >> Only enable gpio there. But even that's vague. Do you turn on the power
> >> before or after setting the enable gpio? How long delay should be
> >> between the power and the gpio? Different panels have different rules
> >> for the power-up.
> >
> > Sure, it's a complex problem. But for the enable gpio..
> >
> > Maybe the enable GPIO should be treated as a regulator? That would allow
> > specifying first the source regulator startup delay, and then the
> > panel has it's own startup delay.
>
> Well... I don't know. Sounds rather hacky to me. We have the option to
> have a specific driver for this panel, and that driver can handle all
> the delays and power-up sequences just right in a clean manner.
I guess we could have a generic startup-latency property for the GPIOs.
> >>>>> But I'm not really familiar with using extending current bindings, and
> >>>>> making new bindings compatible with old ones. Can you explain why it'd
> >>>>> be good to have the sharp panel bindings compatible with simple-panel?
> >>>>> In what kind of scenario would it be used?
> >>>
> >>> Ideally the panel binding just describes the panel and it should not
> >>> matter which display controller it is a child of.
> >>
> >> Yes, but that means the panel bindings need to have enough information
> >> so that all display controllers can use it. Simple-panel bindings do not
> >> have enough information. The simple-panel bindings do not have
> >> information about the video bus input, and it doesn't even have
> >> information about the resolution or bitdepth of the panel.
> >
> > Some of that you can hide into the panel driver based on the compatible
> > flag. So why not already do something like this in the .dts files
> > instead of the remapping:
> >
> > compatible = "sharp,ls037v7dw01-omap-dss", "sharp,ls037v7dw01";
> >
> > And drivers/video/fbdev/omap2/displays-new/panel-sharp-ls037v7dw01.c
> > would only claim to be compatible with "sharp,ls037v7dw01-omap-dss".
> >
> > Then when the common panel framework is available, you can stop
> > parsing sharp,ls037v7dw01-omap-dss but the .dts files don't need
> > to be changed and it's fine to keep "sharp,ls037v7dw01-omap-dss"
> > in the .dts files.
>
> Hmm, I don't see how this relates to the simple-panel bindings.
>
> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an
> alternative for the compatible-string conversion we do now. I guess it's
> a matter of taste, but I rather hide it inside the kernel, in an
> internal omapdss file, than pollute the .dts files with those compatible
> strings.
Well it avoid you parsing through all the nodes during booting
and leaves out the function to do remapping. And removes the need
for maintaining a custom display mapping table. I'd say that's a
pretty good list of advantages right there :)
> >> So I'm still asking, if we create sharp bindings that use the same
> >> properties as the simple-panel bindings, and define that sharp panel is
> >> compatible with simple-panel, what kind of scenario in practice would it
> >> be used in?
> >
> > Well with the above example, just by dss with "sharp,ls037v7dw01-omap-dss"
> > until some other SoC notices it can use the GPIO parts of the panel
> > code at least :)
> >
> >> Would the scenario be some other OS, that doesn't have a driver for the
> >> sharp panel, but has a driver for the simple-panel? That would only work
> >> if the sharp panel hardware is setup so that only the enable gpio is
> >> needed, so that quite a narrow definition of "compatible".
> >
> > That's where we can use the compatible flags and just avoid parsing
> > the generic compatible flag unless some common framework is available.
>
> Hmm, sorry, I don't follow. My question was about the simple-panel
> bindings, not common display framework.
>
> You were saying that the sharp bindings should be compatible with
> simple-panel bindings. I'm still trying to understand what benefit does
> that give us.
Oh sorry, for that part I don't really have an opinion. If you think
the simple-panel binding is not going to be usable in the long
run, then I'm fine with whatever binding you display guys come up
with and prefer to use.
> As I see it, the sharp panel could be used with the simple-panel
> bindings only in certain special case, where all the mode pins and the
> reset are hardwired in the board hardware, and they are hardwired in a
> certain state (all hardwired low, probably), which matches what the
> simple-panel driver expects.
OK. Maybe take a stab at updating the binding for this as I don't
know what you want to do with it?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tony Lindgren @ 2014-05-15 18:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53747DD5.2030406@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 01:42]:
> On 14/05/14 00:26, Tony Lindgren wrote:
>
> > + /* lcd MO */
> > + ddata->mo_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 1, "mode");
> > + if (PTR_ERR(ddata->mo_gpio) = -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > +
> > + if (!IS_ERR(ddata->mo_gpio))
> > + if (gpiod_get_raw_value_cansleep(ddata->mo_gpio))
> > + ddata->flags |= SHARP_LS_QVGA;
>
> Shouldn't there be an explicit flag in the DT data for this? If the
> panel's MO pin is hardwired to, say, pull up, then the mode-gpios won't
> have MO gpio, right? So something like:
>
>
> mode-gpios = <0 /* high, lcd MO */
> &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
> &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
>
> vga-mode; /* MO hardwired high */
Yeah holes there are just fine. I figured let's keep the custom
vga-mode property out of the way until we actually run into a panel
that's not using a GPIO for mode.
So far it seems that mode GPIO is there for the panels I've seen,
just the scan direction pins seem to be hard wired on LDP. But
then again, maybe I'm still having trouble locating all the
GPIOs in the LDP schematics.
> Btw, the gpio.txt has each gpio inside <>:
>
> chipsel-gpios = <&gpio1 12 0>,
> <&gpio1 13 0>,
> <0>, /* holes are permitted, means no GPIO 2 */
> <&gpio2 2>;
>
> Is that equivalent to having all gpios inside <>?
Yeah, just less <> braces. The number of elements for each
entry is what matters and that's known by the GPIO parsing
code.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tony Lindgren @ 2014-05-15 18:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5374BC01.7030802@ti.com>
* Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 06:08]:
> On 14/05/14 00:26, Tony Lindgren wrote:
>
> > +static int sharp_ls_probe_of(struct platform_device *pdev)
> > +{
> > + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> > + struct device_node *node = pdev->dev.of_node;
> > + struct omap_dss_device *in;
> > +
> > + ddata->vcc = devm_regulator_get(&pdev->dev, "envdd");
> > + if (IS_ERR(ddata->vcc)) {
> > + dev_err(&pdev->dev, "failed to get regulator\n");
> > + return PTR_ERR(ddata->vcc);
> > + }
> > +
> > + /* lcd INI */
> > + ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
> > + if (PTR_ERR(ddata->ini_gpio) = -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
>
> Hmm, the GPIOs are optional, but shouldn't we react somehow to real
> errors? I guess we should do something like:
>
> ddata->ini_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 0, "enable");
> if (IS_ERR(ddata->ini_gpio) {
> int err = PTR_ERR(ddata->ini_gpio);
> if (err = -EPROBE_DEFER || err != -ENOENT)
> return err;
> }
Yeah that makes sense to me.
Regards,
Tony
^ permalink raw reply
* [PATCH 0/4] Add backlight registration notification and use it in acpi-video
From: Hans de Goede @ 2014-05-15 18:34 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Jingoo Han, Bryan Wu, Lee Jones,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Ben Skeggs,
David Airlie
Cc: Zhang Rui, Len Brown, linux-acpi, linux-fbdev, dri-devel
Hi all,
Here is a patch series to make video.use_native_backlight work on laptops
with nv gfx, at least on those were nv gfx have a raw backlight interface.
I've tried this on my own somewhat old nv equipped laptop, but that does
not have a raw backlight interface.
I'm currently doing a scratch build of the Fedora-20 kernel with this patches
for the user with an affected system to test.
Regards,
Hans
^ permalink raw reply
* [PATCH 1/4] nouveau: Don't check acpi_video_backlight_support() before registering backlight
From: Hans de Goede @ 2014-05-15 18:34 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Jingoo Han, Bryan Wu, Lee Jones,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Ben Skeggs,
David Airlie
Cc: Zhang Rui, Len Brown, linux-acpi, linux-fbdev, dri-devel,
Hans de Goede
In-Reply-To: <1400178866-3659-1-git-send-email-hdegoede@redhat.com>
acpi_video_backlight_support() is supposed to be called by other (vendor
specific) firmware backlight controls, not by native / raw backlight controls
like nv_backlight.
Userspace will normally prefer firmware interfaces over raw interfaces, so
if acpi_video backlight support is present it will use that even if
nv_backlight is registered as well.
Except when video.use_native_backlight is present on the kernel cmdline
(or enabled through a dmi based quirk). As the name indicates the goal here
is to make only the raw interface available to userspace so that it will use
that (it only does this when it sees a win8 compliant bios).
This is done by:
1) Not registering any acpi_video# backlight devices; and
2) Making acpi_video_backlight_support() return true so that other firmware
drivers, ie acer_wmi, thinkpad_acpi, dell_laptop, etc. Don't register their
own vender specific interfaces.
Currently nouveau breaks this setup, as when acpi_video_backlight_support()
returns true, it does not register itself, resulting in no backlight control
at all.
This is esp. going to be a problem with 3.16 which will default to
video.use_native_backlight=1, and thus nouveau based laptops with a win8 bios
will get no backlight control at all.
This also likely explains why the previous attempt to make
video.use_native_backlight=1 the default was not a success, as without this
patch having a default of video.use_native_backlight=1 will cause regressions.
Note this effectively reverts commit 5bead799
Also see: https://bugzilla.redhat.com/show_bug.cgi?id\x1093171
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/gpu/drm/nouveau/nouveau_backlight.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
index 630f6e8..2c1e4aa 100644
--- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
+++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
@@ -31,7 +31,6 @@
*/
#include <linux/backlight.h>
-#include <linux/acpi.h>
#include "nouveau_drm.h"
#include "nouveau_reg.h"
@@ -222,14 +221,6 @@ nouveau_backlight_init(struct drm_device *dev)
struct nouveau_device *device = nv_device(drm->device);
struct drm_connector *connector;
-#ifdef CONFIG_ACPI
- if (acpi_video_backlight_support()) {
- NV_INFO(drm, "ACPI backlight interface available, "
- "not registering our own\n");
- return 0;
- }
-#endif
-
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
connector->connector_type != DRM_MODE_CONNECTOR_eDP)
--
1.9.0
^ permalink raw reply related
* [PATCH 2/4] backlight: Add backlight device (un)registration notification
From: Hans de Goede @ 2014-05-15 18:34 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Jingoo Han, Bryan Wu, Lee Jones,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Ben Skeggs,
David Airlie
Cc: Zhang Rui, Len Brown, linux-acpi, linux-fbdev, dri-devel,
Hans de Goede
In-Reply-To: <1400178866-3659-1-git-send-email-hdegoede@redhat.com>
Some firmware drivers, ie acpi-video want to get themselves out of the
way (in some cases) when their also is a raw backlight device available.
Due to module loading ordering being unknown, acpi-video cannot be certain
that the backlight_device_registered(BACKLIGHT_RAW) it does for this is
the final verdict wrt there being a BACKLIGHT_RAW device.
By adding notification acpi-video can listen for backlight devices showing
up after it has loaded, and unregister its backlight device if desired.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/video/backlight/backlight.c | 40 +++++++++++++++++++++++++++++++++++++
include/linux/backlight.h | 7 +++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index bd2172c..4280890 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -23,6 +23,7 @@
static struct list_head backlight_dev_list;
static struct mutex backlight_dev_list_mutex;
+static struct blocking_notifier_head backlight_notifier;
static const char *const backlight_types[] = {
[BACKLIGHT_RAW] = "raw",
@@ -370,6 +371,9 @@ struct backlight_device *backlight_device_register(const char *name,
list_add(&new_bd->entry, &backlight_dev_list);
mutex_unlock(&backlight_dev_list_mutex);
+ blocking_notifier_call_chain(&backlight_notifier,
+ BACKLIGHT_REGISTERED, new_bd);
+
return new_bd;
}
EXPORT_SYMBOL(backlight_device_register);
@@ -413,6 +417,10 @@ void backlight_device_unregister(struct backlight_device *bd)
pmac_backlight = NULL;
mutex_unlock(&pmac_backlight_mutex);
#endif
+
+ blocking_notifier_call_chain(&backlight_notifier,
+ BACKLIGHT_UNREGISTERED, bd);
+
mutex_lock(&bd->ops_lock);
bd->ops = NULL;
mutex_unlock(&bd->ops_lock);
@@ -438,6 +446,36 @@ static int devm_backlight_device_match(struct device *dev, void *res,
}
/**
+ * backlight_register_notifier - get notified of backlight (un)registration
+ * @nb: notifier block with the notifier to call on backlight (un)registration
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_register_notifier);
+
+/**
+ * backlight_unregister_notifier - unregister a backlight notifier
+ * @nb: notifier block to unregister
+ *
+ * @return 0 on success, otherwise a negative error code
+ *
+ * Register a notifier to get notified when backlight devices get registered
+ * or unregistered.
+ */
+int backlight_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&backlight_notifier, nb);
+}
+EXPORT_SYMBOL(backlight_unregister_notifier);
+
+/**
* devm_backlight_device_register - resource managed backlight_device_register()
* @dev: the device to register
* @name: the name of the device
@@ -544,6 +582,8 @@ static int __init backlight_class_init(void)
backlight_class->pm = &backlight_class_dev_pm_ops;
INIT_LIST_HEAD(&backlight_dev_list);
mutex_init(&backlight_dev_list_mutex);
+ BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
+
return 0;
}
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 7264742..adb14a8 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -40,6 +40,11 @@ enum backlight_type {
BACKLIGHT_TYPE_MAX,
};
+enum backlight_notification {
+ BACKLIGHT_REGISTERED,
+ BACKLIGHT_UNREGISTERED,
+};
+
struct backlight_device;
struct fb_info;
@@ -133,6 +138,8 @@ extern void devm_backlight_device_unregister(struct device *dev,
extern void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason);
extern bool backlight_device_registered(enum backlight_type type);
+extern int backlight_register_notifier(struct notifier_block *nb);
+extern int backlight_unregister_notifier(struct notifier_block *nb);
#define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
--
1.9.0
^ permalink raw reply related
* [PATCH 3/4] acpi-video: Unregister the backlight device if a raw one shows up later
From: Hans de Goede @ 2014-05-15 18:34 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Jingoo Han, Bryan Wu, Lee Jones,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Ben Skeggs,
David Airlie
Cc: Zhang Rui, Len Brown, linux-acpi, linux-fbdev, dri-devel,
Hans de Goede
In-Reply-To: <1400178866-3659-1-git-send-email-hdegoede@redhat.com>
When video.use_native_backlight=1 and non intel gfx are in use, the raw
backlight device of the gfx driver will show up after acpi-video has done its
acpi_video_verify_backlight_support() check.
This causes video.use_native_backlight=1 to not have the desired result.
This patch fixes this by adding a backlight notifier and when a raw
backlight is registered or unregistered re-doing the
acpi_video_verify_backlight_support() check.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/video.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index aee85c4..123f919 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -151,6 +151,7 @@ struct acpi_video_enumerated_device {
struct acpi_video_bus {
struct acpi_device *device;
bool backlight_registered;
+ bool backlight_notifier_registered;
u8 dos_setting;
struct acpi_video_enumerated_device *attached_array;
u8 attached_count;
@@ -162,6 +163,7 @@ struct acpi_video_bus {
struct input_dev *input;
char phys[32]; /* for input device */
struct notifier_block pm_nb;
+ struct notifier_block backlight_nb;
};
struct acpi_video_device_flags {
@@ -1836,6 +1838,9 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
{
struct acpi_video_device *dev;
+ if (video->backlight_registered)
+ return 0;
+
if (!acpi_video_verify_backlight_support())
return 0;
@@ -1980,6 +1985,56 @@ static void acpi_video_bus_remove_notify_handler(struct acpi_video_bus *video)
video->input = NULL;
}
+static int acpi_video_backlight_notify(struct notifier_block *nb,
+ unsigned long val, void *bd)
+{
+ struct backlight_device *backlight = bd;
+ struct acpi_video_bus *video;
+
+ /* acpi_video_verify_backlight_support only cares about raw devices */
+ if (backlight->props.type != BACKLIGHT_RAW)
+ return NOTIFY_DONE;
+
+ video = container_of(nb, struct acpi_video_bus, backlight_nb);
+
+ switch (val) {
+ case BACKLIGHT_REGISTERED:
+ if (!acpi_video_verify_backlight_support())
+ acpi_video_bus_unregister_backlight(video);
+ break;
+ case BACKLIGHT_UNREGISTERED:
+ acpi_video_bus_register_backlight(video);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static int acpi_video_bus_add_backlight_notify_handler(
+ struct acpi_video_bus *video)
+{
+ int error;
+
+ video->backlight_nb.notifier_call = acpi_video_backlight_notify;
+ video->backlight_nb.priority = 0;
+ error = backlight_register_notifier(&video->backlight_nb);
+ if (error = 0)
+ video->backlight_notifier_registered = true;
+
+ return error;
+}
+
+static int acpi_video_bus_remove_backlight_notify_handler(
+ struct acpi_video_bus *video)
+{
+ if (!video->backlight_notifier_registered)
+ return 0;
+
+ video->backlight_notifier_registered = false;
+
+ return backlight_unregister_notifier(&video->backlight_nb);
+}
+
static int acpi_video_bus_put_devices(struct acpi_video_bus *video)
{
struct acpi_video_device *dev, *next;
@@ -2061,6 +2116,7 @@ static int acpi_video_bus_add(struct acpi_device *device)
acpi_video_bus_register_backlight(video);
acpi_video_bus_add_notify_handler(video);
+ acpi_video_bus_add_backlight_notify_handler(video);
return 0;
@@ -2084,6 +2140,7 @@ static int acpi_video_bus_remove(struct acpi_device *device)
video = acpi_driver_data(device);
+ acpi_video_bus_remove_backlight_notify_handler(video);
acpi_video_bus_remove_notify_handler(video);
acpi_video_bus_unregister_backlight(video);
acpi_video_bus_put_devices(video);
--
1.9.0
^ permalink raw reply related
* [PATCH 4/4] acpi-video: Add use native backlight quirk for the ThinkPad W530
From: Hans de Goede @ 2014-05-15 18:34 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu, Jingoo Han, Bryan Wu, Lee Jones,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Ben Skeggs,
David Airlie
Cc: Zhang Rui, Len Brown, linux-acpi, linux-fbdev, dri-devel,
Hans de Goede, stable
In-Reply-To: <1400178866-3659-1-git-send-email-hdegoede@redhat.com>
Like all of the other *30 ThinkPad models, the W530 has a broken acpi-video
backlight control. Note in order for this to actually fix things on the
ThinkPad W530 the commit titled:
"nouveau: Don't check acpi_video_backlight_support() before registering backlight"
is also needed.
https://bugzilla.redhat.com/show_bug.cgi?id\x1093171
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/video.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 123f919..0fec70d 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -523,6 +523,14 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
},
},
{
+ .callback = video_set_use_native_backlight,
+ .ident = "ThinkPad W530",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad W530"),
+ },
+ },
+ {
.callback = video_set_use_native_backlight,
.ident = "ThinkPad X1 Carbon",
.matches = {
--
1.9.0
^ permalink raw reply related
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-16 5:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140515182548.GD23659@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]
On 15/05/14 21:25, Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@ti.com> [140515 01:42]:
>> On 14/05/14 00:26, Tony Lindgren wrote:
>>
>>> + /* lcd MO */
>>> + ddata->mo_gpio = sharp_ls_get_gpio_of(&pdev->dev, 0, 1, "mode");
>>> + if (PTR_ERR(ddata->mo_gpio) == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> +
>>> + if (!IS_ERR(ddata->mo_gpio))
>>> + if (gpiod_get_raw_value_cansleep(ddata->mo_gpio))
>>> + ddata->flags |= SHARP_LS_QVGA;
>>
>> Shouldn't there be an explicit flag in the DT data for this? If the
>> panel's MO pin is hardwired to, say, pull up, then the mode-gpios won't
>> have MO gpio, right? So something like:
>>
>>
>> mode-gpios = <0 /* high, lcd MO */
>> &gpio1 2 GPIO_ACTIVE_HIGH /* gpio2, lcd LR */
>> &gpio1 3 GPIO_ACTIVE_HIGH>; /* gpio3, lcd UD */
>>
>> vga-mode; /* MO hardwired high */
>
> Yeah holes there are just fine. I figured let's keep the custom
> vga-mode property out of the way until we actually run into a panel
> that's not using a GPIO for mode.
Ok, I'm fine with that, but in that case I think we have to use all the
panels in the same mode, i.e. the driver either sets the MO gpio always
low and uses VGA mode, or sets the MO always high and uses QVGA mode.
In your omap3-evm.dts patch, you set the MO gpio ACTIVE_LOW in order to
change the mode, even if it really should be ACTIVE_HIGH. But if you do
that, and we later add the support to the panel driver to manage the MO
gpio dynamically (i.e. the user can select VGA/QVGA at runtime), then
the panel won't work as the driver uses wrong polarity for the pin.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] OMAPDSS: panel-sharp-ls037v7dw01: add device tree support
From: Tomi Valkeinen @ 2014-05-16 5:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140515182133.GC23659@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]
On 15/05/14 21:21, Tony Lindgren wrote:
>> But you're right, having "sharp,ls037v7dw01-omap-dss" in the .dts is an
>> alternative for the compatible-string conversion we do now. I guess it's
>> a matter of taste, but I rather hide it inside the kernel, in an
>> internal omapdss file, than pollute the .dts files with those compatible
>> strings.
>
> Well it avoid you parsing through all the nodes during booting
> and leaves out the function to do remapping. And removes the need
> for maintaining a custom display mapping table. I'd say that's a
> pretty good list of advantages right there :)
Yep... I don't know. Maybe I'm being too careful about doing wrong
things with .dts. I just like it more if any hacks are in kernel code,
which I can remove without anyone noticing.
Anyway, we already have board.dts files using the non-omapified
compatible strings in the mainline, so if I would now add the omapified
compatible strings to .dts files, those old board.dts files would break.
So I guess the choice has already been made.
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