* [PATCH 3/5] atmel_lcdfb: remove unsupported 15-bpp mode
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Since commit 787f9fd23283 ("atmel_lcdfb: support 16bit BGR:565 mode,
remove unsupported 15bit modes") atmel_lcdfb_check_var does not accept
15-bpp mode so remove it from atmel_lcdfb_set_par as well.
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/atmel_lcdfb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 025428e..93910e3 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -567,7 +567,6 @@ static int atmel_lcdfb_set_par(struct fb_info *info)
case 2: value |= ATMEL_LCDC_PIXELSIZE_2; break;
case 4: value |= ATMEL_LCDC_PIXELSIZE_4; break;
case 8: value |= ATMEL_LCDC_PIXELSIZE_8; break;
- case 15: /* fall through */
case 16: value |= ATMEL_LCDC_PIXELSIZE_16; break;
case 24: value |= ATMEL_LCDC_PIXELSIZE_24; break;
case 32: value |= ATMEL_LCDC_PIXELSIZE_32; break;
--
1.8.1.1
^ permalink raw reply related
* [PATCH 2/5] ARM: at91/neocore926: fix LCD-wiring mode
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Fix regression introduced by commit 787f9fd23283 ("atmel_lcdfb: support
16bit BGR:565 mode, remove unsupported 15bit modes") which broke 16-bpp
modes for older SOCs which use IBGR:555 (msb is intensity) rather than
BGR:565.
The above commit also removed the RGB:555-wiring hack without fixing the
neocore926 board which used it. Fix by specifying RGB-wiring and let the
driver handle the final SOC-dependant layout.
Remove the no longer used ATMEL_LCDC_WIRING_RGB555 define.
Compile-only tested.
Cc: <stable@vger.kernel.org>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
arch/arm/mach-at91/board-neocore926.c | 2 +-
include/video/atmel_lcdc.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/mach-at91/board-neocore926.c b/arch/arm/mach-at91/board-neocore926.c
index bc7a1c4..4726297 100644
--- a/arch/arm/mach-at91/board-neocore926.c
+++ b/arch/arm/mach-at91/board-neocore926.c
@@ -266,7 +266,7 @@ static struct atmel_lcdfb_info __initdata neocore926_lcdc_data = {
.default_monspecs = &at91fb_default_monspecs,
.atmel_lcdfb_power_control = at91_lcdc_power_control,
.guard_time = 1,
- .lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB555,
+ .lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB,
};
#else
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 5f0e234..8deb226 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -30,7 +30,6 @@
*/
#define ATMEL_LCDC_WIRING_BGR 0
#define ATMEL_LCDC_WIRING_RGB 1
-#define ATMEL_LCDC_WIRING_RGB555 2
/* LCD Controller info data structure, stored in device platform_data */
--
1.8.1.1
^ permalink raw reply related
* [PATCH 1/5] atmel_lcdfb: fix 16-bpp modes on older SOCs
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1360071315-4032-1-git-send-email-jhovold@gmail.com>
Fix regression introduced by commit 787f9fd23283 ("atmel_lcdfb: support
16bit BGR:565 mode, remove unsupported 15bit modes") which broke 16-bpp
modes for older SOCs which use IBGR:555 (msb is intensity) rather
than BGR:565.
Use SOC-type to determine the pixel layout.
Tested on at91sam9263 and at91sam9g45.
Cc: <stable@vger.kernel.org>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
drivers/video/atmel_lcdfb.c | 22 +++++++++++++++-------
include/video/atmel_lcdc.h | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 12cf5f3..025428e 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -422,17 +422,22 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
= var->bits_per_pixel;
break;
case 16:
+ /* Older SOCs use IBGR:555 rather than BGR:565. */
+ if (sinfo->have_intensity_bit)
+ var->green.length = 5;
+ else
+ var->green.length = 6;
+
if (sinfo->lcd_wiring_mode = ATMEL_LCDC_WIRING_RGB) {
- /* RGB:565 mode */
- var->red.offset = 11;
+ /* RGB:5X5 mode */
+ var->red.offset = var->green.length + 5;
var->blue.offset = 0;
} else {
- /* BGR:565 mode */
+ /* BGR:5X5 mode */
var->red.offset = 0;
- var->blue.offset = 11;
+ var->blue.offset = var->green.length + 5;
}
var->green.offset = 5;
- var->green.length = 6;
var->red.length = var->blue.length = 5;
break;
case 32:
@@ -679,8 +684,7 @@ static int atmel_lcdfb_setcolreg(unsigned int regno, unsigned int red,
case FB_VISUAL_PSEUDOCOLOR:
if (regno < 256) {
- if (cpu_is_at91sam9261() || cpu_is_at91sam9263()
- || cpu_is_at91sam9rl()) {
+ if (sinfo->have_intensity_bit) {
/* old style I+BGR:555 */
val = ((red >> 11) & 0x001f);
val |= ((green >> 6) & 0x03e0);
@@ -870,6 +874,10 @@ static int __init atmel_lcdfb_probe(struct platform_device *pdev)
}
sinfo->info = info;
sinfo->pdev = pdev;
+ if (cpu_is_at91sam9261() || cpu_is_at91sam9263() ||
+ cpu_is_at91sam9rl()) {
+ sinfo->have_intensity_bit = true;
+ }
strcpy(info->fix.id, sinfo->pdev->name);
info->flags = ATMEL_LCDFB_FBINFO_DEFAULT;
diff --git a/include/video/atmel_lcdc.h b/include/video/atmel_lcdc.h
index 28447f1..5f0e234 100644
--- a/include/video/atmel_lcdc.h
+++ b/include/video/atmel_lcdc.h
@@ -62,6 +62,7 @@ struct atmel_lcdfb_info {
void (*atmel_lcdfb_power_control)(int on);
struct fb_monspecs *default_monspecs;
u32 pseudo_palette[16];
+ bool have_intensity_bit;
};
#define ATMEL_LCDC_DMABADDR1 0x00
--
1.8.1.1
^ permalink raw reply related
* [PATCH 0/5] atmel_lcdfb: regression fixes and cpu_is removal
From: Johan Hovold @ 2013-02-05 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130129135435.GN7360@game.jcrosoft.org>
The first three patches are resends of two regression fixes and one clean up
posted in December (with previous acked-bys included). The regression fixes are
kept minimal and are tagged for stable.
The last two patches replace all uses of cpu_is macros in atmel_lcdfb with a
platform-device-id table and static configurations.
Thanks,
Johan
Johan Hovold (5):
atmel_lcdfb: fix 16-bpp modes on older SOCs
ARM: at91/neocore926: fix LCD-wiring mode
atmel_lcdfb: remove unsupported 15-bpp mode
atmel_lcdfb: move lcdcon2 register access to compute_hozval
ARM: at91/avr32/atmel_lcdfb: replace cpu_is macros with device-id
table
arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
arch/arm/mach-at91/at91sam9263_devices.c | 2 +-
arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
arch/arm/mach-at91/at91sam9rl_devices.c | 2 +-
arch/arm/mach-at91/board-neocore926.c | 2 +-
arch/avr32/mach-at32ap/at32ap700x.c | 2 +
drivers/video/atmel_lcdfb.c | 115 ++++++++++++++++++++++++++-----
include/video/atmel_lcdc.h | 4 +-
8 files changed, 116 insertions(+), 23 deletions(-)
--
1.8.1.1
^ permalink raw reply
* Re: Revert "console: implement lockdep support for console_lock"
From: Sedat Dilek @ 2013-02-05 10:50 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Linus Torvalds, linux-fbdev, LKML, linux-next,
Andrew Morton, Stephen Rothwell
In-Reply-To: <CA+icZUU4JMfws_jFpoCaXnSNJeigONSEik_oGSa_ZQ=uJ_GJOA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]
On Tue, Feb 5, 2013 at 11:47 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Fri, Feb 1, 2013 at 9:42 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Fri, Feb 1, 2013 at 9:21 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>> people having the fbcon-locking-fixes [1] in their local GIT tree can
>>> revert this change?
>>
>> Yeah, if you have all the fixes reverting this is fine and appreciated
>> to increase testing. Dave will probably push the revert himself to
>> drm-next soon.
>
> When will that happen: drm-next inclusion?
>
> Just FYI:
> Pulling fbcon-locking-fixes into Linux-Next (next-20130205) shows some
> ugly UTF-8 mismatch, means "fb: rework locking to fix lock ordering on
> takeover" is not the same patch as in -next.
> Just wanna let you know.
>
> It would be good to inform Andrew and Stephen when this happened, so
> Andrew can drop the two fb-fixes from his mmotm-tree.
>
Attached the UTF-8 "mismatch" in vt.c.
- Sedat -
> Thanks!
>
> - Sedat -
>
>> -Daniel
>>
>>>
>>> commit ff0d05bf73620eb7dc8aee7423e992ef87870bdf
>>> Refs: v3.8-rc5-194-gff0d05b
>>> Author: Dave Airlie <airlied@gmail.com>
>>> AuthorDate: Thu Jan 31 14:27:03 2013 +1100
>>> Commit: Dave Airlie <airlied@gmail.com>
>>> CommitDate: Thu Jan 31 15:46:56 2013 +1100
>>>
>>> Revert "console: implement lockdep support for console_lock"
>>>
>>> This reverts commit daee779718a319ff9f83e1ba3339334ac650bb22.
>>>
>>> I'll requeue this after the console locking fixes, so lockdep
>>> is useful again for people until fbcon is fixed.
>>>
>>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>>>
>>> Thanks!
>>>
>>> Regards,
>>> - Sedat
>>>
>>> [1] http://cgit.freedesktop.org/~airlied/linux/log/?h=fbcon-locking-fixes
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[-- Attachment #2: VT_C_UTF-8_CRAP.diff --]
[-- Type: application/octet-stream, Size: 1773 bytes --]
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 56dd69c..86b76f4 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -3656,7 +3656,7 @@ EXPORT_SYMBOL_GPL(do_unregister_con_driver);
* when a driver wants to take over some existing consoles
* and become default driver for newly opened ones.
*
- * take_over_console is basically a register followed by unbind
+ * take_over_console is basically a register followed by unbind
*/
int do_take_over_console(const struct consw *csw, int first, int last, int deflt)
{
@@ -3666,7 +3666,7 @@ int do_take_over_console(const struct consw *csw, int first, int last, int deflt
/*
* If we get an busy error we still want to bind the console driver
* and return success, as we may have unbound the console driver
- * but not unregistered it.
+ * but not unregistered it.
*/
if (err == -EBUSY)
err = 0;
@@ -3682,7 +3682,7 @@ EXPORT_SYMBOL_GPL(do_take_over_console);
* when a driver wants to take over some existing consoles
* and become default driver for newly opened ones.
*
- * take_over_console is basically a register followed by unbind
+ * take_over_console is basically a register followed by unbind
*/
int take_over_console(const struct consw *csw, int first, int last, int deflt)
{
@@ -3692,7 +3692,7 @@ int take_over_console(const struct consw *csw, int first, int last, int deflt)
/*
* If we get an busy error we still want to bind the console driver
* and return success, as we may have unbound the console driver
- * but not unregistered it.
+ * but not unregistered it.
*/
if (err == -EBUSY)
err = 0;
^ permalink raw reply related
* Re: Revert "console: implement lockdep support for console_lock"
From: Sedat Dilek @ 2013-02-05 10:47 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Linus Torvalds, linux-fbdev, LKML, linux-next,
Andrew Morton, Stephen Rothwell
In-Reply-To: <CAKMK7uEFCm5JPD9bg4uO9K_q-w0=9GWzRLoB9H-_=ZR7NxA9=A@mail.gmail.com>
On Fri, Feb 1, 2013 at 9:42 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> On Fri, Feb 1, 2013 at 9:21 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> people having the fbcon-locking-fixes [1] in their local GIT tree can
>> revert this change?
>
> Yeah, if you have all the fixes reverting this is fine and appreciated
> to increase testing. Dave will probably push the revert himself to
> drm-next soon.
When will that happen: drm-next inclusion?
Just FYI:
Pulling fbcon-locking-fixes into Linux-Next (next-20130205) shows some
ugly UTF-8 mismatch, means "fb: rework locking to fix lock ordering on
takeover" is not the same patch as in -next.
Just wanna let you know.
It would be good to inform Andrew and Stephen when this happened, so
Andrew can drop the two fb-fixes from his mmotm-tree.
Thanks!
- Sedat -
> -Daniel
>
>>
>> commit ff0d05bf73620eb7dc8aee7423e992ef87870bdf
>> Refs: v3.8-rc5-194-gff0d05b
>> Author: Dave Airlie <airlied@gmail.com>
>> AuthorDate: Thu Jan 31 14:27:03 2013 +1100
>> Commit: Dave Airlie <airlied@gmail.com>
>> CommitDate: Thu Jan 31 15:46:56 2013 +1100
>>
>> Revert "console: implement lockdep support for console_lock"
>>
>> This reverts commit daee779718a319ff9f83e1ba3339334ac650bb22.
>>
>> I'll requeue this after the console locking fixes, so lockdep
>> is useful again for people until fbcon is fixed.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>>
>> Thanks!
>>
>> Regards,
>> - Sedat
>>
>> [1] http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver
From: Guennadi Liakhovetski @ 2013-02-05 9:08 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh,
'Magnus Damm', 'Richard Purdie'
In-Reply-To: <00fe01cdfcf1$dcfdc970$96f95c50$%han@samsung.com>
Hi Richard
Could you tell us your opinion on this:
On Mon, 28 Jan 2013, Jingoo Han wrote:
> On Friday, January 25, 2013 4:49 PM, Guennadi Liakhovetski wrote
> >
> > Hi Jingoo Han
> >
> > On Fri, 25 Jan 2013, Jingoo Han wrote:
> >
> > > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote
> > > >
> > > > This is an initial commit of a backlight driver, using step-up DCDC power
> > > > supplies on AS3711 PMIC. Only one mode has actually been tested, several
> > > > further modes have been implemented "dry," but disabled to avoid accidental
> > > > hardware damage. Anyone wishing to use any of those modes will have to
> > > > modify the driver.
> > > >
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > >
> > > CC'ed Andrew Morton.
> > >
> > > Hi Guennadi Liakhovetski,
> > >
> > > I have reviewed this patch with AS3711 datasheet.
> > > I cannot find any problems. It looks good.
> >
> > Thanks for the review.
> >
> > > However, some hardcoded numbers need to be changed
> > > to the bit definitions.
> >
> > Which specific hardcoded values do you mean? A while ago in a discussion
> > on one of kernel mailing lists a conclusion has been made, that macros do
> > not always improve code readability. E.g. in a statement like this
> >
> > + case AS3711_SU2_CURR_AUTO:
> > + if (pdata->su2_auto_curr1)
> > + ctl = 2;
> > + if (pdata->su2_auto_curr2)
> > + ctl |= 8;
> > + if (pdata->su2_auto_curr3)
> > + ctl |= 0x20;
> >
> > making it
> >
> > + case AS3711_SU2_CURR_AUTO:
> > + if (pdata->su2_auto_curr1)
> > + ctl = SU2_AUTO_CURR1;
> >
> > would hardly make it more readable. IMHO it is already pretty clear, that
> > bit 1 enables the current-1 sink. As long as these fields are only used at
> > one location in the driver (and they are), using a macro and defining it
> > elsewhere only makes it harder to see actual values. Speaking of this, a
> > comment like
>
> I don't think so. Some people feel that it is not clear a bit.
> Of course, I know what you mean.
> Also, your comment is reasonable.
> However, personally, I prefer the latter.
> Because, I think that the meaning of bits is more important than
> actual bits. In the latter case, we can notice the meaning of bits
> more easily.
Do you also find preferable to use symbolic names for every single bit,
occurring in a driver, or you agree, that excessive use of such macros can
only needlessly clutter the code?
Thanks
Guennadi
> Best regards,
> Jingoo Han
>
> >
> > /*
> > * Select, which current sinks shall be used for automatic
> > * feedback selection
> > */
> >
> > would help much more than any macro names. But as it stands, I think the
> > current version is also possible to understand :-) If desired, however,
> > comments can be added in an incremental patch
>
> >
> > Thanks
> > Guennadi
> >
> > > Acked-by: Jingoo Han <jg1.han@samsung.com>
> > >
> > >
> > > Best regards,
> > > Jingoo Han
> > >
> > > > ---
> > > >
> > > > Tested on sh73a0-based kzm9g board. As the commit message says, only one
> > > > mode has been tested and is enabled. That mode copies the sample code from
> > > > the manufacturer. Deviations from that code proved to be fatal for the
> > > > hardware...
> > > >
> > > > drivers/video/backlight/Kconfig | 7 +
> > > > drivers/video/backlight/Makefile | 1 +
> > > > drivers/video/backlight/as3711_bl.c | 379 +++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 387 insertions(+), 0 deletions(-)
> > > > create mode 100644 drivers/video/backlight/as3711_bl.c
> > > >
> > > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > > > index 765a945..6ef0ede 100644
> > > > --- a/drivers/video/backlight/Kconfig
> > > > +++ b/drivers/video/backlight/Kconfig
> > > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217
> > > > If you have a Texas Instruments TPS65217 say Y to enable the
> > > > backlight driver.
> > > >
> > > > +config BACKLIGHT_AS3711
> > > > + tristate "AS3711 Backlight"
> > > > + depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711
> > > > + help
> > > > + If you have an Austrian Microsystems AS3711 say Y to enable the
> > > > + backlight driver.
> > > > +
> > > > endif # BACKLIGHT_CLASS_DEVICE
> > > >
> > > > endif # BACKLIGHT_LCD_SUPPORT
> > > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > > > index e7ce729..d3e188f 100644
> > > > --- a/drivers/video/backlight/Makefile
> > > > +++ b/drivers/video/backlight/Makefile
> > > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
> > > > obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o
> > > > obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
> > > > obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> > > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> > > > new file mode 100644
> > > > index 0000000..c6bc65d
> > > > --- /dev/null
> > > > +++ b/drivers/video/backlight/as3711_bl.c
> > > > @@ -0,0 +1,379 @@
> > > > +/*
> > > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters
> > > > + *
> > > > + * Copyright (C) 2012 Renesas Electronics Corporation
> > > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the version 2 of the GNU General Public License as
> > > > + * published by the Free Software Foundation
> > > > + */
> > > > +
> > > > +#include <linux/backlight.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/err.h>
> > > > +#include <linux/fb.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/mfd/as3711.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/regmap.h>
> > > > +#include <linux/slab.h>
> > > > +
> > > > +enum as3711_bl_type {
> > > > + AS3711_BL_SU1,
> > > > + AS3711_BL_SU2,
> > > > +};
> > > > +
> > > > +struct as3711_bl_data {
> > > > + bool powered;
> > > > + const char *fb_name;
> > > > + struct device *fb_dev;
> > > > + enum as3711_bl_type type;
> > > > + int brightness;
> > > > + struct backlight_device *bl;
> > > > +};
> > > > +
> > > > +struct as3711_bl_supply {
> > > > + struct as3711_bl_data su1;
> > > > + struct as3711_bl_data su2;
> > > > + const struct as3711_bl_pdata *pdata;
> > > > + struct as3711 *as3711;
> > > > +};
> > > > +
> > > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su)
> > > > +{
> > > > + switch (su->type) {
> > > > + case AS3711_BL_SU1:
> > > > + return container_of(su, struct as3711_bl_supply, su1);
> > > > + case AS3711_BL_SU2:
> > > > + return container_of(su, struct as3711_bl_supply, su2);
> > > > + }
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data,
> > > > + unsigned int brightness)
> > > > +{
> > > > + struct as3711_bl_supply *supply = to_supply(data);
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > + int ret = 0;
> > > > +
> > > > + /* Only all equal current values are supported */
> > > > + if (pdata->su2_auto_curr1)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > > + brightness);
> > > > + if (!ret && pdata->su2_auto_curr2)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > > + brightness);
> > > > + if (!ret && pdata->su2_auto_curr3)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > > + brightness);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_set_brightness_v(struct as3711 *as3711,
> > > > + unsigned int brightness,
> > > > + unsigned int reg)
> > > > +{
> > > > + if (brightness > 31)
> > > > + return -EINVAL;
> > > > +
> > > > + return regmap_update_bits(as3711->regmap, reg, 0xf0,
> > > > + brightness << 4);
> > > > +}
> > > > +
> > > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply)
> > > > +{
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5,
> > > > + 3, supply->pdata->su2_fbprot);
> > > > + if (!ret)
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 0);
> > > > + if (!ret)
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 1);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Someone with less fragile or less expensive hardware could try to simplify
> > > > + * the brightness adjustment procedure.
> > > > + */
> > > > +static int as3711_bl_update_status(struct backlight_device *bl)
> > > > +{
> > > > + struct as3711_bl_data *data = bl_get_data(bl);
> > > > + struct as3711_bl_supply *supply = to_supply(data);
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + int brightness = bl->props.brightness;
> > > > + int ret = 0;
> > > > +
> > > > + dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n",
> > > > + __func__, bl->props.brightness, bl->props.power,
> > > > + bl->props.fb_blank, bl->props.state);
> > > > +
> > > > + if (bl->props.power != FB_BLANK_UNBLANK ||
> > > > + bl->props.fb_blank != FB_BLANK_UNBLANK ||
> > > > + bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > > > + brightness = 0;
> > > > +
> > > > + if (data->type = AS3711_BL_SU1) {
> > > > + ret = as3711_set_brightness_v(as3711, brightness,
> > > > + AS3711_STEPUP_CONTROL_1);
> > > > + } else {
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + ret = as3711_set_brightness_v(as3711, brightness,
> > > > + AS3711_STEPUP_CONTROL_2);
> > > > + break;
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + ret = as3711_set_brightness_auto_i(data, brightness / 4);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + if (brightness) {
> > > > + ret = as3711_bl_su2_reset(supply);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + udelay(500);
> > > > + ret = as3711_set_brightness_auto_i(data, brightness);
> > > > + } else {
> > > > + ret = regmap_update_bits(as3711->regmap,
> > > > + AS3711_STEPUP_CONTROL_2, 1, 0);
> > > > + }
> > > > + break;
> > > > + /* Manual one current feedback pin below */
> > > > + case AS3711_SU2_CURR1:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + case AS3711_SU2_CURR2:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + case AS3711_SU2_CURR3:
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > > + brightness);
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + }
> > > > + }
> > > > + if (!ret)
> > > > + data->brightness = brightness;
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_bl_get_brightness(struct backlight_device *bl)
> > > > +{
> > > > + struct as3711_bl_data *data = bl_get_data(bl);
> > > > +
> > > > + return data->brightness;
> > > > +}
> > > > +
> > > > +static const struct backlight_ops as3711_bl_ops = {
> > > > + .update_status = as3711_bl_update_status,
> > > > + .get_brightness = as3711_bl_get_brightness,
> > > > +};
> > > > +
> > > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply)
> > > > +{
> > > > + struct as3711 *as3711 = supply->as3711;
> > > > + const struct as3711_bl_pdata *pdata = supply->pdata;
> > > > + u8 ctl = 0;
> > > > + int ret;
> > > > +
> > > > + dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback);
> > > > +
> > > > + /* Turn SU2 off */
> > > > + ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0);
> > > > + break;
> > > > + case AS3711_SU2_CURR1:
> > > > + ctl = 1;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1);
> > > > + break;
> > > > + case AS3711_SU2_CURR2:
> > > > + ctl = 4;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2);
> > > > + break;
> > > > + case AS3711_SU2_CURR3:
> > > > + ctl = 0x10;
> > > > + ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3);
> > > > + break;
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + if (pdata->su2_auto_curr1)
> > > > + ctl = 2;
> > > > + if (pdata->su2_auto_curr2)
> > > > + ctl |= 8;
> > > > + if (pdata->su2_auto_curr3)
> > > > + ctl |= 0x20;
> > > > + ret = 0;
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (!ret)
> > > > + ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_bl_register(struct platform_device *pdev,
> > > > + unsigned int max_brightness, struct as3711_bl_data *su)
> > > > +{
> > > > + struct backlight_properties props = {.type = BACKLIGHT_RAW,};
> > > > + struct backlight_device *bl;
> > > > +
> > > > + /* max tuning I = 31uA for voltage- and 38250uA for current-feedback */
> > > > + props.max_brightness = max_brightness;
> > > > +
> > > > + bl = backlight_device_register(su->type = AS3711_BL_SU1 ?
> > > > + "as3711-su1" : "as3711-su2",
> > > > + &pdev->dev, su,
> > > > + &as3711_bl_ops, &props);
> > > > + if (IS_ERR(bl)) {
> > > > + dev_err(&pdev->dev, "failed to register backlight\n");
> > > > + return PTR_ERR(bl);
> > > > + }
> > > > +
> > > > + bl->props.brightness = props.max_brightness;
> > > > +
> > > > + backlight_update_status(bl);
> > > > +
> > > > + su->bl = bl;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int as3711_backlight_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> > > > + struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent);
> > > > + struct as3711_bl_supply *supply;
> > > > + struct as3711_bl_data *su;
> > > > + unsigned int max_brightness;
> > > > + int ret;
> > > > +
> > > > + if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> > > > + dev_err(&pdev->dev, "No platform data, exiting...\n");
> > > > + return -ENODEV;
> > > > + }
> > > > +
> > > > + /*
> > > > + * Due to possible hardware damage I chose to block all modes,
> > > > + * unsupported on my hardware. Anyone, wishing to use any of those modes
> > > > + * will have to first review the code, then activate and test it.
> > > > + */
> > > > + if (pdata->su1_fb ||
> > > > + pdata->su2_fbprot != AS3711_SU2_GPIO4 ||
> > > > + pdata->su2_feedback != AS3711_SU2_CURR_AUTO) {
> > > > + dev_warn(&pdev->dev,
> > > > + "Attention! An untested mode has been chosen!\n"
> > > > + "Please, review the code, enable, test, and report success:-)\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL);
> > > > + if (!supply)
> > > > + return -ENOMEM;
> > > > +
> > > > + supply->as3711 = as3711;
> > > > + supply->pdata = pdata;
> > > > +
> > > > + if (pdata->su1_fb) {
> > > > + su = &supply->su1;
> > > > + su->fb_name = pdata->su1_fb;
> > > > + su->type = AS3711_BL_SU1;
> > > > +
> > > > + max_brightness = min(pdata->su1_max_uA, 31);
> > > > + ret = as3711_bl_register(pdev, max_brightness, su);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + if (pdata->su2_fb) {
> > > > + su = &supply->su2;
> > > > + su->fb_name = pdata->su2_fb;
> > > > + su->type = AS3711_BL_SU2;
> > > > +
> > > > + switch (pdata->su2_fbprot) {
> > > > + case AS3711_SU2_GPIO2:
> > > > + case AS3711_SU2_GPIO3:
> > > > + case AS3711_SU2_GPIO4:
> > > > + case AS3711_SU2_LX_SD4:
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + switch (pdata->su2_feedback) {
> > > > + case AS3711_SU2_VOLTAGE:
> > > > + max_brightness = min(pdata->su2_max_uA, 31);
> > > > + break;
> > > > + case AS3711_SU2_CURR1:
> > > > + case AS3711_SU2_CURR2:
> > > > + case AS3711_SU2_CURR3:
> > > > + case AS3711_SU2_CURR_AUTO:
> > > > + max_brightness = min(pdata->su2_max_uA / 150, 255);
> > > > + break;
> > > > + default:
> > > > + ret = -EINVAL;
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + ret = as3711_bl_init_su2(supply);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + ret = as3711_bl_register(pdev, max_brightness, su);
> > > > + if (ret < 0)
> > > > + goto esu2;
> > > > + }
> > > > +
> > > > + platform_set_drvdata(pdev, supply);
> > > > +
> > > > + return 0;
> > > > +
> > > > +esu2:
> > > > + backlight_device_unregister(supply->su1.bl);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int as3711_backlight_remove(struct platform_device *pdev)
> > > > +{
> > > > + struct as3711_bl_supply *supply = platform_get_drvdata(pdev);
> > > > +
> > > > + backlight_device_unregister(supply->su1.bl);
> > > > + backlight_device_unregister(supply->su2.bl);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static struct platform_driver as3711_backlight_driver = {
> > > > + .driver = {
> > > > + .name = "as3711-backlight",
> > > > + .owner = THIS_MODULE,
> > > > + },
> > > > + .probe = as3711_backlight_probe,
> > > > + .remove = as3711_backlight_remove,
> > > > +};
> > > > +
> > > > +module_platform_driver(as3711_backlight_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs");
> > > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de");
> > > > +MODULE_LICENSE("GPL");
> > > > +MODULE_ALIAS("platform:as3711-backlight");
> > > > --
> > > > 1.7.2.5
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> >
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-05 7:08 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <20130204192514.GA32318@kroah.com>
Am 04.02.2013 20:25, schrieb Greg KH:
> On Mon, Feb 04, 2013 at 08:17:04PM +0100, Alexander Holler wrote:
>> Am 04.02.2013 13:05, schrieb Alexander Holler:
>>> Am 04.02.2013 02:14, schrieb Greg KH:
>>>
>>>> So you are right in that your driver will wait for forever for a
>>>> disconnect() to happen, as it will never be called. I don't understand
>>>> the problem that this is causing when it happens. What's wrong with
>>>> udlfb that having the cpu suddently reset as the powerdown happened
>>>> without it knowing about it?
>>>
>>> There is nothing wrong with that. I've just explained why a problem
>>> doesn't occur on shutdown but on disconnect (of the device).
>>
>> Maybe my explanation before was just to long and I try to explain it
>> a bit shorter:
>>
>> If a device gets disconnected, the disconnect in udlfb might wait
>> forever in down_interruptible() (because it waits for an urb it
>> never receives). This even prevents a shutdown afterwards, because
>> that down_interruptible() never receives a signal (at shutdown,
>> because kernel threads don't get such).
>
> Where was that urb when the disconnect happened? The USB core should
> call your urb callback for any outstanding urbs at that point in time,
> with the proper error flag being set, are you handling that properly?
I don't know where that urb is as I don't handle it. I just know that
_interruptible doesn't make any sense and _timeout is necessary here.
But as nobody else seems to have a problem, nobody else see seems to see
the problem there and I seem to be unable to explain it, just ignore
that patch.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Greg KH @ 2013-02-04 19:25 UTC (permalink / raw)
To: Alexander Holler
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <51100930.6080405@ahsoftware.de>
On Mon, Feb 04, 2013 at 08:17:04PM +0100, Alexander Holler wrote:
> Am 04.02.2013 13:05, schrieb Alexander Holler:
> >Am 04.02.2013 02:14, schrieb Greg KH:
> >
> >>So you are right in that your driver will wait for forever for a
> >>disconnect() to happen, as it will never be called. I don't understand
> >>the problem that this is causing when it happens. What's wrong with
> >>udlfb that having the cpu suddently reset as the powerdown happened
> >>without it knowing about it?
> >
> >There is nothing wrong with that. I've just explained why a problem
> >doesn't occur on shutdown but on disconnect (of the device).
>
> Maybe my explanation before was just to long and I try to explain it
> a bit shorter:
>
> If a device gets disconnected, the disconnect in udlfb might wait
> forever in down_interruptible() (because it waits for an urb it
> never receives). This even prevents a shutdown afterwards, because
> that down_interruptible() never receives a signal (at shutdown,
> because kernel threads don't get such).
Where was that urb when the disconnect happened? The USB core should
call your urb callback for any outstanding urbs at that point in time,
with the proper error flag being set, are you handling that properly?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-04 19:17 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <510FA409.2080201@ahsoftware.de>
Am 04.02.2013 13:05, schrieb Alexander Holler:
> Am 04.02.2013 02:14, schrieb Greg KH:
>
>> So you are right in that your driver will wait for forever for a
>> disconnect() to happen, as it will never be called. I don't understand
>> the problem that this is causing when it happens. What's wrong with
>> udlfb that having the cpu suddently reset as the powerdown happened
>> without it knowing about it?
>
> There is nothing wrong with that. I've just explained why a problem
> doesn't occur on shutdown but on disconnect (of the device).
Maybe my explanation before was just to long and I try to explain it a
bit shorter:
If a device gets disconnected, the disconnect in udlfb might wait
forever in down_interruptible() (because it waits for an urb it never
receives). This even prevents a shutdown afterwards, because that
down_interruptible() never receives a signal (at shutdown, because
kernel threads don't get such).
So the change from down_timeout() to down_interruptible() in
dlfb_free_urb_list() with commit
33077b8d3042e01da61924973e372abe589ba297 only results in that the
following code (thus the break there) will never be reached if an urb
got missed (because of a disconnect).
And the accompanying comment (... at shutdown) is misleading, because on
shutdown, the kernel thread which calls dlfb_free_urb_list() never gets
a signal, so the interruption just never happens.
As I've experienced the "missing urb on disconnect" problem quiet often,
I've changed that down_interruptible() to down_timeout() (in v1 and in
v2 to down_timeout_interruptible, because I wasn't aware that no signal
arrives on shutdown).
Hmm, ok, that explanation isn't much shorter. ;)
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-02-04 12:05 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <20130204011413.GA6413@kroah.com>
Am 04.02.2013 02:14, schrieb Greg KH:
> So you are right in that your driver will wait for forever for a
> disconnect() to happen, as it will never be called. I don't understand
> the problem that this is causing when it happens. What's wrong with
> udlfb that having the cpu suddently reset as the powerdown happened
> without it knowing about it?
There is nothing wrong with that. I've just explained why a problem
doesn't occur on shutdown but on disconnect (of the device).
Regards,
Alexander
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Marcus Lorentzon @ 2013-02-04 10:05 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tomasz Figa, Thomas Petazzoni, linux-fbdev@vger.kernel.org,
Philipp Zabel, Tom Gall, Ragesh Radhakrishnan,
dri-devel@lists.freedesktop.org, Rob Clark, Kyungmin Park,
Tomi Valkeinen, sunil joshi, Benjamin Gaignard, Bryan Wu,
Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
linux-media@vger.kernel.org
In-Reply-To: <3057999.UZLp2j2DkQ@avalon>
On 02/02/2013 12:35 AM, Laurent Pinchart wrote:
> Hi Marcus,
>
> On Tuesday 08 January 2013 18:08:19 Marcus Lorentzon wrote:
>> On 01/08/2013 05:36 PM, Tomasz Figa wrote:
>>> On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote:
[...]
>>>> But it is not perfect. After a couple of products we realized that most
>>>> panel drivers want an easy way to send a bunch of init commands in one
>>>> go. So I think it should be an op for sending an array of commands at
>>>> once. Something like
>>>>
>>>> struct dsi_cmd {
>>>> enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */
>>>> u8 cmd;
>>>> int dataLen;
>>>> u8 *data;
>>>> }
>>>>
>>>> struct dsi_ops {
>>>> int dsi_write(source, int num_cmds, struct dsi_cmd *cmds);
>>>> ...
>>>> }
> Do you have DSI IP(s) that can handle a list of commands ? Or would all DSI
> transmitter drivers need to iterate over the commands manually ? In the later
> case a lower-level API might be easier to implement in DSI transmitter
> drivers. Helper functions could provide the higher-level API you proposed.
The HW has a FIFO, so it can handle a few. Currently we use the low
level type of call with one call per command. But we have found DSI
command mode panels that don't accept any commands during the "update"
(write start+continues). And so we must use a mutex/state machine to
exclude any async calls to send DSI commands during update. But if you
need to send more than one command per frame this will be hard (like
CABC and backlight commands). It will be a ping pong between update and
command calls. One option is to expose the mutex to the caller so it can
make many calls before the next update grabs the mutex again.
So maybe we could create a helper that handle the op for list of
commands and another op for single command that you actually have to
implement.
>>> Yes, this should be flexible enough to cover most of (or even whole) DSI
>>> specification.
>>>
>>> However I'm not sure whether the dsi_write name would be appropriate,
>>> since DSI packet types include also read and special transactions. So,
>>> according to DSI terminology, maybe dsi_transaction would be better?
> Or dsi_transfer or dsi_xfer ? Does the DSI bus have a concept of transactions
> ?
No transactions. And I don't want to mix reads and writes. It should be
similar to I2C and other stream control busses. So one read and one
write should be fine. And then a bunch of helpers on top for callers to
use, like one per major DSI packet type.
>> I think read should still be separate. At least on my HW read and write
>> are quite different. But all "transactions" are very much the same in HW
>> setup. The ... was dsi_write etc ;) Like set_max_packet_size should
>> maybe be an ops. Since only the implementer of the "video source" will
>> know what the max read return packet size for that DSI IP is. The panels
>> don't know that. Maybe another ops to retrieve some info about the caps
>> of the video source would help that. Then a helper could call that and
>> then the dsi_write one.
> If panels (or helper functions) need information about the DSI transmitter
> capabilities, sure, we can add an op.
Yes, a "video source" op for getting caps would be ok too. But so far
the only limits I have found is the read/write sizes. But if anyone else
has other limits, please list them so we could add them to this struct
dsi_host_caps.
>>>> And I think I still prefer the dsi_bus in favor of the abstract video
>>>> source. It just looks like a home made bus with bus-ops ... can't you do
>>>> something similar using the normal driver framework? enable/disable
>>>> looks like suspend/resume, register/unregister_vid_src is like
>>>> bus_(un)register_device, ... the video source anyway seems unattached
>>>> to the panel stuff with the find_video_source call.
> The Linux driver framework is based on control busses. DSI usually handles
> both control and video transfer, but the control and data busses can also be
> separate (think DPI + SPI/I2C for instance). In that case the panel will be a
> child of its control bus master, and will need a separate interface to access
> video data operations. As a separate video interface is thus needed, I think
> we should use it for DSI as well.
>
> My initial proposal included a DBI bus (as I don't have any DSI hardware - DBI
> and DSI can be used interchangeably in this discussions, they both share the
> caracteristic of having a common control + data bus), and panels were children
> of the DBI bus master. The DBI bus API was only used for control, not for data
> transfers. Tomi then removed the DBI bus and moved the control operations to
> the video source, turning the DBI panels into platform devices. I still favor
> my initial approach, but I can agree to drop the DBI bus if there's a
> consensus on that. Video bus operations will be separate in any case.
As discussed at FOSDEM I will give DSI bus with full feature set a try.
BTW. Who got the action to ask Greg about devices with multiple
parents/buses?
>>> Also, as discussed in previous posts, some panels might use DSI only for
>>> video data and another interface (I2C, SPI) for control data. In such case
>>> it would be impossible to represent such device in a reasonable way using
>>> current driver model.
>> I understand that you need to get hold of both the control and data bus
>> device in the driver. (Toshiba DSI-LVDS bridge is a good example and
>> commonly used "encoder" that can use both DSI and I2C control interface.)
>> But the control bus you get from device probe, and I guess you could call
>> bus_find_device_by_name(dsi_bus, "mydev") and return the "datadev" which
>> will have access to dsi bus ops just as you call
>> find_video_source("mysource") to access the "databus" ops directly with
>> a logical device (display entity).
>> I'm not saying I would refuse to use video sources. I just think the two
>> models are so similar so it would be worth exploring how a device model
>> style API would look like and to compare against.
> I don't think we should use the Linux device model for data busses. It hasn't
> been designed for that use case, and definitely doesn't support devices that
> would be children of two separate masters (control and data). For shared bus
> devices such as DSI, having a DSI bus was my preference to start with, as
> mentioned above :-) However, even in that case, I think it would still make
> sense to use video sources to control the video operations. As usual the devil
> is in the details, so there will probably be some tricky problems we'll need
> to solve, but that will require coding the proposed solution.
>
I will give it a try after asking Greg for guidelines.
/BR
/Marcus
^ permalink raw reply
* Re: [PATCH] video: add ili922x lcd driver
From: Jingoo Han @ 2013-02-04 5:45 UTC (permalink / raw)
To: 'Anatolij Gustschin'
Cc: 'Andrew Morton', 'LKML', linux-fbdev,
'Stefano Babic', 'Richard Purdie',
'Florian Tobias Schandinat', 'Jingoo Han'
In-Reply-To: <1359729703-24915-1-git-send-email-agust@denx.de>
On Friday, February 01, 2013 11:42 PM, Anatolij Gustschin wrote
CC'ed Andrew Morton
>
> From: Stefano Babic <sbabic@denx.de>
>
> Add LCD driver for Ilitek ILI9221/ILI9222 controller.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> ---
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/ili922x.c | 586 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 594 insertions(+), 0 deletions(-)
> create mode 100644 drivers/video/backlight/ili922x.c
>
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 765a945..97b4672 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -59,6 +59,13 @@ config LCD_LTV350QV
>
> The LTV350QV panel is present on all ATSTK1000 boards.
>
> +config LCD_ILI922X
> + tristate "ILI Technology ILI9221/ILI9222 support"
> + depends on SPI
> + help
> + If you have a panel based on the ILI9221/9222 controller
> + chip then say y to include a driver for it.
> +
> config LCD_ILI9320
> tristate "ILI Technology ILI9320 controller support"
> depends on SPI
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index e7ce729..3cfd901 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_LCD_HP700) += jornada720_lcd.o
> obj-$(CONFIG_LCD_L4F00242T03) += l4f00242t03.o
> obj-$(CONFIG_LCD_LMS283GF05) += lms283gf05.o
> obj-$(CONFIG_LCD_LTV350QV) += ltv350qv.o
> +obj-$(CONFIG_LCD_ILI922X) += ili922x.o
> obj-$(CONFIG_LCD_ILI9320) += ili9320.o
> obj-$(CONFIG_LCD_PLATFORM) += platform_lcd.o
> obj-$(CONFIG_LCD_VGG2432A4) += vgg2432a4.o
> diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
> new file mode 100644
> index 0000000..18c33df
> --- /dev/null
> +++ b/drivers/video/backlight/ili922x.c
> @@ -0,0 +1,586 @@
> +/*
> + * (C) Copyright 2008
> + * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
Please remove this comment. It is hard to keep track of the address of
Free Software Foundation.
Also, above mentioned address is not the same with the current address.
> + *
> + * This driver implements a lcd device for the ILITEK 922x display
> + * controller. The interface to the display is SPI and the display's
> + * memory is cyclically updated
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/fb.h>
> +#include <linux/init.h>
> +#include <linux/lcd.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
Would you order inclusions of <linux/xxx.h> according to alphabetical
ordering, for readability?
> +
> +/* Register offset, see manual section 8.2 */
> +#define REG_START_OSCILLATION 0x00
> +#define REG_DRIVER_CODE_READ 0x00
> +#define REG_DRIVER_OUTPUT_CONTROL 0x01
> +#define REG_LCD_AC_DRIVEING_CONTROL 0x02
> +#define REG_ENTRY_MODE 0x03
> +#define REG_COMPARE_1 0x04
> +#define REG_COMPARE_2 0x05
> +#define REG_DISPLAY_CONTROL_1 0x07
> +#define REG_DISPLAY_CONTROL_2 0x08
> +#define REG_DISPLAY_CONTROL_3 0x09
> +#define REG_FRAME_CYCLE_CONTROL 0x0B
> +#define REG_EXT_INTF_CONTROL 0x0C
> +#define REG_POWER_CONTROL_1 0x10
> +#define REG_POWER_CONTROL_2 0x11
> +#define REG_POWER_CONTROL_3 0x12
> +#define REG_POWER_CONTROL_4 0x13
> +#define REG_RAM_ADDRESS_SET 0x21
> +#define REG_WRITE_DATA_TO_GRAM 0x22
> +#define REG_RAM_WRITE_MASK1 0x23
> +#define REG_RAM_WRITE_MASK2 0x24
> +#define REG_GAMMA_CONTROL_1 0x30
> +#define REG_GAMMA_CONTROL_2 0x31
> +#define REG_GAMMA_CONTROL_3 0x32
> +#define REG_GAMMA_CONTROL_4 0x33
> +#define REG_GAMMA_CONTROL_5 0x34
> +#define REG_GAMMA_CONTROL_6 0x35
> +#define REG_GAMMA_CONTROL_7 0x36
> +#define REG_GAMMA_CONTROL_8 0x37
> +#define REG_GAMMA_CONTROL_9 0x38
> +#define REG_GAMMA_CONTROL_10 0x39
> +#define REG_GATE_SCAN_CONTROL 0x40
> +#define REG_VERT_SCROLL_CONTROL 0x41
> +#define REG_FIRST_SCREEN_DRIVE_POS 0x42
> +#define REG_SECOND_SCREEN_DRIVE_POS 0x43
> +#define REG_RAM_ADDR_POS_H 0x44
> +#define REG_RAM_ADDR_POS_V 0x45
> +#define REG_OSCILLATOR_CONTROL 0x4F
> +#define REG_GPIO 0x60
> +#define REG_OTP_VCM_PROGRAMMING 0x61
> +#define REG_OTP_VCM_STATUS_ENABLE 0x62
> +#define REG_OTP_PROGRAMMING_ID_KEY 0x65
> +
> +/*
> + * maximum frequency for register access
> + * (not for the GRAM access)
> + */
> +#define ILITEK_MAX_FREQ_REG 4000000
> +
> +/*
> + * Device ID as found in the datasheet (supports 9221 and 9222)
> + */
> +#define ILITEK_DEVICE_ID 0x9220
> +#define ILITEK_DEVICE_ID_MASK 0xFFF0
> +
> +/* Last two bits in the START BYTE */
> +#define START_RS_INDEX 0
> +#define START_RS_REG 1
> +#define START_RW_WRITE 0
> +#define START_RW_READ 1
> +
> +/**
> + * START_BYTE(id, rs, rw)
> + *
> + * Set the start byte according to the required operation.
> + * The start byte is defined as:
> + * ----------------------------------
> + * | 0 | 1 | 1 | 1 | 0 | ID | RS | RW |
> + * ----------------------------------
> + * @id: display's id as set by the manufacturer
> + * @rs: operation type bit, one of:
> + * - START_RS_INDEX set the index register
> + * - START_RS_REG write/read registers/GRAM
> + * @rw: read/write operation
> + * - START_RW_WRITE write
> + * - START_RW_READ read
> + */
> +#define START_BYTE(id, rs, rw) \
> + (0x70 | (((id) & 0x01) << 2) | (((rs) & 0x01) << 1) | ((rw) & 0x01))
> +
> +/**
> + * CHECK_FREQ_REG(spi_device s, spi_transfer x) - Check the frequency
> + * for the SPI transfer. According to the datasheet, the controller
> + * accept higher frequency for the GRAM transfer, but it requires
> + * lower frequency when the registers are read/written.
> + * The macro sets the frequency in the spi_transfer structure if
> + * the frequency exceeds the maximum value.
> + */
> +#define CHECK_FREQ_REG(s, x) \
> + do { \
> + if (s->max_speed_hz > ILITEK_MAX_FREQ_REG) \
> + ((struct spi_transfer *)x)->speed_hz = \
> + ILITEK_MAX_FREQ_REG; \
> + } while (0)
> +
> +#define CMD_BUFSIZE 16
> +
> +#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
> +
> +#define set_tx_byte(b) (tx_invert ? ~(b) : b)
> +
> +/**
> + * ili922x_id - id as set by manufacturer
> + */
> +static int ili922x_id = 1;
> +module_param(ili922x_id, int, 0);
> +
> +static int tx_invert;
> +module_param(tx_invert, int, 0);
> +
> +/**
> + * driver's private structure
> + */
> +struct ili922x {
> + struct spi_device *spi;
> + struct lcd_device *ld;
> + int power;
> +};
> +
> +#define NUM_DUMMY_BYTES 1
> +static void send_dummy(struct spi_device *spi)
> +{
> + struct spi_message m;
> + struct spi_transfer xfer;
> + unsigned char tbuf[CMD_BUFSIZE];
> + unsigned char rbuf[CMD_BUFSIZE];
> + int ret = 0, i;
> +
> + return;
> +
> + memset(&xfer, 0, sizeof(struct spi_transfer));
> + spi_message_init(&m);
> + xfer.tx_buf = tbuf;
> + xfer.rx_buf = rbuf;
> + xfer.cs_change = 1;
> + CHECK_FREQ_REG(spi, &xfer);
> +
> + for (i = 0; i < NUM_DUMMY_BYTES; i++)
> + tbuf[i] = set_tx_byte(0xFF);
> +
> + xfer.bits_per_word = 8;
> + xfer.len = NUM_DUMMY_BYTES;
> + spi_message_add_tail(&xfer, &m);
> + ret = spi_sync(spi, &m);
> +
> + udelay(10);
How about replacing udelay() with usleep_range()?
> +}
> +
> +/**
> + * read_status - read status register from display
> + * @spi: spi device
> + */
> +static u16 read_status(struct spi_device *spi)
> +{
> + struct spi_message m;
Would you replace 'm' with 'msg' or 'message' for the readability?
It's too short, even though 'm' is used in include/linux/spi/spi.h.
struct spi_message msg;
> + struct spi_transfer xfer;
> + unsigned char tbuf[CMD_BUFSIZE];
> + unsigned char rbuf[CMD_BUFSIZE];
> + int ret = 0, i;
> +
> + send_dummy(spi);
> +
> + memset(&xfer, 0, sizeof(struct spi_transfer));
> + spi_message_init(&m);
> + xfer.tx_buf = tbuf;
> + xfer.rx_buf = rbuf;
> + xfer.cs_change = 1;
> + CHECK_FREQ_REG(spi, &xfer);
> +
> + tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
> + START_RW_READ));
> + for (i = 1; i < 4; i++)
> + tbuf[i] = set_tx_byte(0); /* dummy */
> +
> + xfer.bits_per_word = 8;
> + xfer.len = 4;
> + spi_message_add_tail(&xfer, &m);
> + ret = spi_sync(spi, &m);
> +
> + return (rbuf[2] << 8) + rbuf[3];
> +}
> +
> +/**
> + * read_reg - read register from display
> + * @spi: spi device
> + * @reg: offset of the register to be read
> + * @rx: output value
> + */
> +static int read_reg(struct spi_device *spi, u8 reg, u16 *rx)
> +{
> + struct spi_message m;
> + struct spi_transfer xfer_regindex, xfer_regvalue;
> + unsigned char tbuf[CMD_BUFSIZE];
> + unsigned char rbuf[CMD_BUFSIZE];
> + int ret = 0, len = 0, i, send_bytes;
> +
> + send_dummy(spi);
> +
> + memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
> + memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
> + spi_message_init(&m);
> + xfer_regindex.tx_buf = tbuf;
> + xfer_regindex.rx_buf = rbuf;
> + xfer_regindex.cs_change = 1;
> + CHECK_FREQ_REG(spi, &xfer_regindex);
> +
> + tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
> + START_RW_WRITE));
> + tbuf[1] = set_tx_byte(0);
> + tbuf[2] = set_tx_byte(reg);
> + xfer_regindex.bits_per_word = 8;
> + len = xfer_regindex.len = 3;
> + spi_message_add_tail(&xfer_regindex, &m);
> +
> + send_bytes = len;
> +
> + tbuf[len++] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
> + START_RW_READ));
> + for (i = len; i < CMD_BUFSIZE; i++)
> + tbuf[i] = set_tx_byte(0); /* dummy */
> +
> + xfer_regvalue.cs_change = 1;
> + xfer_regvalue.len = 4;
I don't understand why length 4 is necessary.
In my opinion, length 3 seems to be enough.
- tbuf[4] is used for sending 'START_BYTE(ili922x_id, START_RS_REG, START_RW_READ)'.
- rbuf[5] and rbuf[6] are used for receiving value as below.
*rx = (rbuf[1 + send_bytes] << 8) + rbuf[2 + send_bytes];
However, tbuf[7] or rbuf[7] seems to be unnecessary.
If I'm wrong, please let me know kindly.
> + xfer_regvalue.tx_buf = &tbuf[send_bytes];
> + xfer_regvalue.rx_buf = &rbuf[send_bytes];
> + CHECK_FREQ_REG(spi, &xfer_regvalue);
> +
> + spi_message_add_tail(&xfer_regvalue, &m);
> + ret = spi_sync(spi, &m);
> + if (ret < 0) {
> + dev_dbg(&spi->dev, "Error sending SPI message 0x%x", ret);
> + return ret;
> + }
> +
> + *rx = (rbuf[1 + send_bytes] << 8) + rbuf[2 + send_bytes];
> + return 0;
> +}
> +
> +/**
> + * write_reg - write a controller register
> + * @spi: struct spi_device *
> + * @reg: offset of the register to be written
> + * @value: value to be written
> + */
> +static int write_reg(struct spi_device *spi, u8 reg, u16 value)
> +{
> + struct spi_message m;
> + struct spi_transfer xfer_regindex, xfer_regvalue;
> + unsigned char tbuf[CMD_BUFSIZE];
> + unsigned char rbuf[CMD_BUFSIZE];
> + int ret = 0, len = 0;
> +
> + send_dummy(spi);
> +
> + memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
> + memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
> +
> + spi_message_init(&m);
> + xfer_regindex.tx_buf = tbuf;
> + xfer_regindex.rx_buf = rbuf;
> + xfer_regindex.cs_change = 1;
> + CHECK_FREQ_REG(spi, &xfer_regindex);
> +
> + tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
> + START_RW_WRITE));
> + tbuf[1] = set_tx_byte(0);
> + tbuf[2] = set_tx_byte(reg);
> + xfer_regindex.bits_per_word = 8;
> + xfer_regindex.len = 3;
> + spi_message_add_tail(&xfer_regindex, &m);
> +
> + ret = spi_sync(spi, &m);
> +
> + send_dummy(spi);
> +
> + spi_message_init(&m);
> + len = 0;
> + tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
> + START_RW_WRITE));
> + tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);
> + tbuf[2] = set_tx_byte(value & 0x00FF);
> +
> + xfer_regvalue.cs_change = 1;
> + xfer_regvalue.len = 3;
> + xfer_regvalue.tx_buf = tbuf;
> + xfer_regvalue.rx_buf = rbuf;
> + CHECK_FREQ_REG(spi, &xfer_regvalue);
> +
> + spi_message_add_tail(&xfer_regvalue, &m);
> +
> + ret = spi_sync(spi, &m);
> + if (ret < 0) {
> + dev_err(&spi->dev, "Error sending SPI message 0x%x", ret);
> + return ret;
> + }
> + return 0;
> +}
> +
> +#ifdef DEBUG
> +/**
> + * ili922x_reg_dump - dump all registers
> + */
> +static void ili922x_reg_dump(struct spi_device *spi)
> +{
> + u8 reg;
> + u16 rx;
> +
> + pr_info("ILI922x configuration registers:\n");
Please replace pr_info() with dev_info() as below.
dev_err(&spi->dev, "ILI922x configuration registers:\n");
> + for (reg = REG_START_OSCILLATION;
> + reg <= REG_OTP_PROGRAMMING_ID_KEY; reg++) {
> + read_reg(spi, reg, &rx);
> + pr_info("reg @ 0x%02X: 0x%04X\n", reg, rx);
Same as above.
> + }
> +}
> +#else
> +static inline void ili922x_reg_dump(struct spi_device *spi) {}
> +#endif
> +
> +/**
> + * set_write_to_gram_reg - initialize the display to write the GRAM
> + * @spi: spi device
> + */
> +static void set_write_to_gram_reg(struct spi_device *spi)
> +{
> + struct spi_message m;
> + struct spi_transfer xfer;
> + unsigned char tbuf[CMD_BUFSIZE];
> +
> + memset(&xfer, 0, sizeof(struct spi_transfer));
> +
> + spi_message_init(&m);
> + xfer.tx_buf = tbuf;
> + xfer.rx_buf = NULL;
> + xfer.cs_change = 1;
> +
> + tbuf[0] = START_BYTE(ili922x_id, START_RS_INDEX, START_RW_WRITE);
> + tbuf[1] = 0;
> + tbuf[2] = REG_WRITE_DATA_TO_GRAM;
> +
> + xfer.bits_per_word = 8;
> + xfer.len = 3;
> + spi_message_add_tail(&xfer, &m);
> + spi_sync(spi, &m);
> +}
> +
> +/**
> + * ili922x_poweron - turn the display on
> + * @spi: spi device
> + *
> + * The sequence to turn on the display is taken from
> + * the datasheet and/or the example code provided by the
> + * manufacturer.
> + */
> +static int ili922x_poweron(struct spi_device *spi)
> +{
> + int ret = 0;
Initialization is not necessary.
Just declare it as below:
int ret;
> +
> + /* Power on */
> + ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
> + mdelay(10);
> + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
> + mdelay(40);
> + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
> + mdelay(40);
> + ret += write_reg(spi, 0x56, 0x080F);
Would you replace this hard-coded address with the bit definition?
> + ret += write_reg(spi, REG_POWER_CONTROL_1, 0x4240);
> + mdelay(10);
> + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0014);
> + mdelay(40);
> + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x1319);
> + mdelay(40);
How about replacing mdelay() with msleep()?
> +
> + return ret;
> +}
> +
> +/**
> + * ili922x_poweroff - turn the display off
> + * @spi: spi device
> + */
> +static int ili922x_poweroff(struct spi_device *spi)
> +{
> + int ret = 0;
Initialization is not necessary.
Just declare it as below:
int ret;
> +
> + /* Power off */
> + ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
> + mdelay(10);
> + ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
> + ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
> + mdelay(40);
> + ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
> + mdelay(40);
Same as above.
> +
> + return ret;
> +}
> +
> +/**
> + * ili922x_display_init - initialize the display by setting
> + * the configuration registers
> + * @spi: spi device
> + */
> +static void ili922x_display_init(struct spi_device *spi)
> +{
> + write_reg(spi, REG_START_OSCILLATION, 1);
> + mdelay(10);
Same as above.
> + write_reg(spi, REG_DRIVER_OUTPUT_CONTROL, 0x691B);
> + write_reg(spi, REG_LCD_AC_DRIVEING_CONTROL, 0x0700);
> + write_reg(spi, REG_ENTRY_MODE, 0x1030);
> + write_reg(spi, REG_COMPARE_1, 0x0000);
> + write_reg(spi, REG_COMPARE_2, 0x0000);
> + write_reg(spi, REG_DISPLAY_CONTROL_1, 0x0037);
> + write_reg(spi, REG_DISPLAY_CONTROL_2, 0x0202);
> + write_reg(spi, REG_DISPLAY_CONTROL_3, 0x0000);
> + write_reg(spi, REG_FRAME_CYCLE_CONTROL, 0x0000);
> +
> + /* Set RGB interface */
> + write_reg(spi, REG_EXT_INTF_CONTROL, 0x0110);
> +
> + ili922x_poweron(spi);
> +
> + write_reg(spi, REG_GAMMA_CONTROL_1, 0x0302);
> + write_reg(spi, REG_GAMMA_CONTROL_2, 0x0407);
> + write_reg(spi, REG_GAMMA_CONTROL_3, 0x0304);
> + write_reg(spi, REG_GAMMA_CONTROL_4, 0x0203);
> + write_reg(spi, REG_GAMMA_CONTROL_5, 0x0706);
> + write_reg(spi, REG_GAMMA_CONTROL_6, 0x0407);
> + write_reg(spi, REG_GAMMA_CONTROL_7, 0x0706);
> + write_reg(spi, REG_GAMMA_CONTROL_8, 0x0000);
> + write_reg(spi, REG_GAMMA_CONTROL_9, 0x0C06);
> + write_reg(spi, REG_GAMMA_CONTROL_10, 0x0F00);
> + write_reg(spi, REG_RAM_ADDRESS_SET, 0x0000);
> + write_reg(spi, REG_GATE_SCAN_CONTROL, 0x0000);
> + write_reg(spi, REG_VERT_SCROLL_CONTROL, 0x0000);
> + write_reg(spi, REG_FIRST_SCREEN_DRIVE_POS, 0xDB00);
> + write_reg(spi, REG_SECOND_SCREEN_DRIVE_POS, 0xDB00);
> + write_reg(spi, REG_RAM_ADDR_POS_H, 0xAF00);
> + write_reg(spi, REG_RAM_ADDR_POS_V, 0xDB00);
> + ili922x_reg_dump(spi);
> + set_write_to_gram_reg(spi);
> +}
> +
> +static int ili922x_lcd_power(struct ili922x *lcd, int power)
> +{
> + int ret = 0;
> +
> + if (POWER_IS_ON(power) && !POWER_IS_ON(lcd->power))
> + ret = ili922x_poweron(lcd->spi);
> + else if (!POWER_IS_ON(power) && POWER_IS_ON(lcd->power))
> + ret = ili922x_poweroff(lcd->spi);
> +
> + if (!ret)
> + lcd->power = power;
> +
> + return ret;
> +}
> +
> +static int ili922x_set_power(struct lcd_device *ld, int power)
> +{
> + struct ili922x *ili = lcd_get_data(ld);
> +
> + return ili922x_lcd_power(ili, power);
> +}
> +
> +static int ili922x_get_power(struct lcd_device *ld)
> +{
> + struct ili922x *ili = lcd_get_data(ld);
> +
> + return ili->power;
> +}
> +
> +static struct lcd_ops ili922x_ops = {
> + .get_power = ili922x_get_power,
> + .set_power = ili922x_set_power,
> +};
> +
> +static int ili922x_probe(struct spi_device *spi)
> +{
> + struct ili922x *ili;
> + struct lcd_device *lcd;
> + int ret;
> + u16 reg = 0;
> +
> + ili = devm_kzalloc(&spi->dev, sizeof(*ili), GFP_KERNEL);
> + if (!ili) {
> + dev_err(&spi->dev, "cannot alloc priv data\n");
> + return -ENOMEM;
> + }
> +
> + ili->spi = spi;
> + dev_set_drvdata(&spi->dev, ili);
> +
> + /* check if the device is connected */
> + ret = read_reg(spi, REG_DRIVER_CODE_READ, ®);
> + if (ret || ((reg & ILITEK_DEVICE_ID_MASK) != ILITEK_DEVICE_ID)) {
> + dev_err(&spi->dev,
> + "no LCD found: Chip ID 0x%x, ret %d\n",
> + reg, ret);
> + return -ENODEV;
> + } else {
> + dev_info(&spi->dev, "ILI%x found, SPI freq %d, mode %d\n",
> + reg, spi->max_speed_hz, spi->mode);
> + }
> +
> + dev_dbg(&spi->dev, "status: 0x%x\n", read_status(spi));
> +
> + ili922x_display_init(spi);
> +
> + ili->power = FB_BLANK_POWERDOWN;
> +
> + lcd = lcd_device_register("ili922xlcd", &spi->dev, ili,
> + &ili922x_ops);
> + if (IS_ERR(lcd)) {
> + dev_err(&spi->dev, "cannot register LCD\n");
> + return PTR_ERR(lcd);
> + }
> +
> + ili->ld = lcd;
> + spi_set_drvdata(spi, ili);
> +
> + ili922x_lcd_power(ili, FB_BLANK_UNBLANK);
> +
> + return 0;
> +}
> +
> +static int ili922x_remove(struct spi_device *spi)
> +{
> + struct ili922x *ili = spi_get_drvdata(spi);
> +
> + ili922x_poweroff(spi);
> + lcd_device_unregister(ili->ld);
> + return 0;
> +}
> +
> +static struct spi_driver ili922x_driver = {
> + .driver = {
> + .name = "ili922x",
> + .owner = THIS_MODULE,
> + },
> + .probe = ili922x_probe,
> + .remove = ili922x_remove,
> +};
> +
> +module_spi_driver(ili922x_driver);
> +
> +MODULE_AUTHOR("Stefano Babic <sbabic@denx.de>");
> +MODULE_DESCRIPTION("ILI9221/9222 LCD driver");
> +MODULE_LICENSE("GPL");
> +MODULE_PARM_DESC(ili922x_id, "set controller identifier (default=1)");
> +MODULE_PARM_DESC(tx_invert, "invert bytes before sending");
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Greg KH @ 2013-02-04 1:14 UTC (permalink / raw)
To: Alexander Holler
Cc: Andrew Morton, linux-kernel, linux-fbdev,
Florian Tobias Schandinat, Bernie Thompson, Steve Glendinning,
stable
In-Reply-To: <5108329E.2050802@ahsoftware.de>
On Tue, Jan 29, 2013 at 09:35:42PM +0100, Alexander Holler wrote:
> Am 29.01.2013 16:51, schrieb Alexander Holler:
> >Am 29.01.2013 12:11, schrieb Alexander Holler:
> >
> >>
> >>To explain the problem on shutdown a bit further, I think the following
> >>happens (usb and driver are statically linked and started by the kernel):
> >>
> >>shutdown -> kill signal -> usb stack shuts down -> udlfb waits (forever)
> >>for a kill or an urb which it doesn't get.
> >
> >Having a second look at what I've written above, I'm not even sure if
> >the kernel sends one or more fatal signals on shutdown at all. I've just
> >assumed it because otherwise down_interruptible() wouldn't have worked
> >before (it would have stalled on shutdown too (if an urb got missed),
> >not only on disconnect).
> >
> >Sounds like an interesting question I should read about (if/when fatal
> >signals are issued by the kernel). ;)
> >
> >>Maybe the sequence is different if the usb-stack and udlfb are used as a
> >>module and/or udlfb is used only for X/fb. I'm not sure what actually
> >>does shut down the usb-stack in such a case, but maybe more than one
> >>kill signal might be thrown around.
>
> If anyone still follows my monologue: The question was interesting
> enough that I couldn't resist for long. ;)
>
> (all pasted => format broken)
>
> In drivers/tty/sysrq.c there is
>
> ------
> static void send_sig_all(int sig)
> {
> struct task_struct *p;
>
> read_lock(&tasklist_lock);
> for_each_process(p) {
> if (p->flags & PF_KTHREAD)
> continue;
> if (is_global_init(p))
> continue;
>
> do_send_sig_info(sig, SEND_SIG_FORCED, p, true);
> }
> read_unlock(&tasklist_lock);
> }
>
> static void sysrq_handle_term(int key)
> {
> send_sig_all(SIGTERM);
> console_loglevel = 8;
> }
>
> (...)
>
> static void sysrq_handle_kill(int key)
> {
> send_sig_all(SIGKILL);
> console_loglevel = 8;
> }
> ------
>
> Now I've done some learning by doing (kernel 3.7.5 + some patches):
>
> ------
> diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
> index df249f3..db8a86c 100644
> --- a/drivers/video/udlfb.c
> +++ b/drivers/video/udlfb.c
> @@ -1876,14 +1876,18 @@ static void dlfb_free_urb_list(struct dlfb_data
> *dev)
> unsigned long flags;
>
> pr_notice("Freeing all render urbs\n");
> + if (current->flags & PF_KTHREAD)
> + pr_info("AHO: I'm a kernel thread\n");
>
> /* keep waiting and freeing, until we've got 'em all */
> while (count--) {
>
> /* Timeout likely occurs at disconnect (resulting in a
> leak) */
> ret = down_timeout_killable(&dev->urbs.limit_sem,
> FREE_URB_TIMEOUT);
> - if (ret)
> + if (ret) {
> + pr_info("AHO: ret %d\n", ret);
> break;
> + }
>
> spin_lock_irqsave(&dev->urbs.lock, flags);
> ------
>
> Now I've disconnected the display. And, as send_sig_all() already
> suggests, the result was (besides discovering an oops in
> call_timer_fn.isra (once)):
>
> ------
> [ 120.963010] udlfb: AHO: I'm a kernel thread
> [ 122.957024] udlfb: AHO: ret -62
> ------
> (-62 is -ETIME)
>
> So, if the above down_timeout_killable() is only
> down_interruptible(), as in kernel 3.7.5, the box would not
> shutdown afterwards, because on shutdown no signal would be send to
> that kernel-thread which called dlfb_free_urb_list().
>
> A last note: dlfb_usb_disconnect() (thus dlfb_free_urb_list()) isn't
> called on shutdown if the device would still be connected. So the
> problem only might happen, if the screen will be disconnected before
> shutdown (and an urb gets missed). So the subject of my patch is
> correct. ;)
Yes, we don't disconnect all devices from the USB bus on shutdown
because, I think, we didn't tear down all of the PCI devices originally,
so your USB bus never knew it was going to be shutdown.
This is how things have always worked, and shutting down PCI devices in
the past have been known to cause problems. I think. I vaguely
remember some issues when I tried to do this 10+ years or so ago in the
2.5 kernel days, but I could be totally wrong given that I can't
remember what I was working on last month at times...
So you are right in that your driver will wait for forever for a
disconnect() to happen, as it will never be called. I don't understand
the problem that this is causing when it happens. What's wrong with
udlfb that having the cpu suddently reset as the powerdown happened
without it knowing about it?
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Tomasz Figa @ 2013-02-03 19:17 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Tomasz Figa, dri-devel, linux-fbdev, linux-samsung-soc,
kyungmin.park, m.szyprowski, Daniel Vetter, Marcus Lorentzon, rob,
tomi.valkeinen, Vikas Sajjan, inki.dae, dh09.lee, ville.syrjala,
s.nawrocki
In-Reply-To: <4292748.rtElrP8BXd@avalon>
Hi Laurent,
On Saturday 02 of February 2013 11:39:59 Laurent Pinchart wrote:
> Hi Tomasz,
>
> Thank you for your RFC.
>
> On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote:
> > Hi,
> >
> > After pretty long time of trying to adapt Exynos-specific DSI display
> > support to Common Display Framework I'm ready to show some preliminary
> > RFC patches. This series shows some ideas for CDF that came to my
> > mind during my work, some changes based on comments received by
> > Tomi's edition of CDF and also preliminarys version of Exynos DSI
> > (video source part only, still with some FIXMEs) and Samsung S6E8AX0
> > DSI panel drivers.
> >
> > It is heavily based on Tomi's work which can be found here:
> > http://thread.gmane.org/gmane.comp.video.dri.devel/78227
> >
> > The code might be a bit hacky in places, as I wanted to get it to a
> > kind of complete and working state first. However I hope that some of
> > the ideas might useful for further works.
> >
> > So, here comes the TF edition of Common Clock Framework.
> > --------------------------------------------------------
> >
> > Changes done to Tomi's edition of CDF:
> > - Replaced set_operation_mode, set_pixel_format and set_size video
> > source>
> > operations with get_params display entity operation, as it was
> > originally in Laurent's version.
> >
> > We have discussed this matter on the mailing list and decided that
> > it
> > would be better to have the source ask the sink for its parameter
> > structure and do whatever appropriate with it.
>
> Could you briefly outline the rationale behind this ?
Display interfaces may be described by an arbitrary number of parameters.
Some will require just video timings, others like DSI will require a
significant number of additional bus-specific ones.
Separating all the parameters (all of which are usually set at the same
time) into a lot of ops setting single parameter will just add unnecessary
complexity.
> I'm wondering whether get_params could limit us if a device needs to
> modify parameters at runtime. For instance a panel might need to change
> clock frequencies or number of used data lane depending on various
> runtime conditions. I don't have a real use case here, so it might just
> be a bit of over-engineering.
Hmm, isn't it in the opposite direction (the user requests the display
interface to change, for example, video mode, which in turn reconfigures
the link and requests the panel to update its settings)?
However it might be reasonable to split the parameters into constant and
variable ones. In case of DSI bus, there is a lot of parameters that are
considered just at link initialization time (the whole dsi params struct I
defined). Video mode, however, is a variable parameter that can be changed
on some displays.
>
> > - Defined a preliminary set of DSI bus parameters.
> >
> > Following parameters are defined:
> >
> > 1. Pixel format used for video data transmission.
> >
> > 2. Mode flags (several bit flags ORed together):
> > a) DSI_MODE_VIDEO - entity uses video mode (as opposed to command
> >
> > mode), following DSI_MODE_VIDEO_* flags are relevant only if
> > this
> > flag is set.
> > b) DSI_MODE_VIDEO_BURST - entity uses burst transfer for video
> > data
> > c) DSI_MODE_VIDEO_SYNC_PULSE - entity uses sync pulses (as
> > opposed
> >
> > to sync events)
> >
> > d) DSI_MODE_VIDEO_AUTO_VERT - entity uses automatic video mode
> >
> > detection
> >
> > e) DSI_MODE_VIDEO_HSE - entity needs horizontal sync end
> > packets
> > f) DSI_MODE_VIDEO_HFP - entity needs horizontal front porch
> > area
> > g) DSI_MODE_VIDEO_HBP - entity needs horizontal back porch
> > area
> > h) DSI_MODE_VIDEO_HSA - entity needs horizontal sync active
> > area
> >
> > i) DSI_MODE_VSYNC_FLUSH - vertical sync pulse flushes video data
> > j) DSI_MODE_EOT_PACKET - entity needs EoT packets
> >
> > 3. Bit (serial) clock frequency in HS mode.
> > 4. Escape mode clock frequency.
> > 5. Mask of used data lanes (each bit represents single lane).
>
> From my experience with MIPI CSI (Camera Serial Interface, similar to
> DSI) some transceivers can reroute data lanes internally. Is that true
> for DSI as well ? In that case we would need a list of data lanes, not
> just a mask.
Hmm, I don't remember at the moment, will have to look to the
specification. Exynos DSI master doesn't support such feature.
> > 6. Command allow area in video mode - amount of lines after
> > transmitting>
> > video data when generic commands are accepted.
> >
> > Feel free to suggest anything missing or wrong, as the list of
> > parameters is based merely on what was used in original Exynos MIPI
> > DSIM driver.
> >
> > - Redesigned source-entity matching.
> >
> > Now each source has name string and integer id and each entity has
> > source name and source id. In addition, they can have of_node
> > specified, which is used for Device Tree-based matching.
> >
> > The matching procedure is as follows:
> >
> > 1. Matching takes place when display entity registers.
> >
> > a) If there is of_node specified for the entity then
> > "video-source"
> >
> > phandle of this node is being parsed and list of registered
> > sources
> > is traversed in search of a source with of_node received from
> > parsing the phandle.
> >
> > b) Otherwise the matching key is a pair of source name and id.
> >
> > 2. If no matching source is found, display_entity_register returns
> >
> > -EPROBE_DEFER error which causes the entity driver to defer its
> > probe until the source registers.
> >
> > 3. Otherwise an optional bind operation of video source is called,
> >
> > sink field of source and source field of entity are set and the
> > matching ends successfully.
> >
> > - Some initial concept of source-entity cross-locking.
> >
> > Only video source is protected at the moment, as I still have to
> > think
> > a bit about locking the entity in a way where removing it by user
> > request is still possible.
>
> One of the main locking issues here are cross-references. The display
> entity holds a reference to the video source (for video operations),
> and the display controller driver holds a reference to the display
> entity (for control operations), resulting in a cross-references lock
> situation. One possible solution would be to first unbind the display
> entity device from its driver to break the cycle.
Yes, this is a problem that requires a bit more thought.
I've been discussing this issue with Sylwester and we came to a conclusion
that for sure a display entity can normally get a reference to its video
source (e.g. module_get, to prevent unloading the module), as there should
be no possibility of video source going away, leaving display entity
present and so there is no need for unregistering a bound video source.
This is what I implemented in my code.
Now it shall be possible to unregister a bound display entity, for example
to have the ability of unplugging a hotpluggable display (HDMI, Display
Port), but this will need some non-trivial synchronization. I'm still
thinking how to do it properly.
Best regards,
Tomasz
> > - Dropped any panels and chips that I can't test.
> >
> > They are irrelevant for this series, so there is no point in
> > including
> > them.
> >
> > - Added Exynos DSI video source driver.
> >
> > This is a new driver for the DSI controller found in Exynos SoCs.
> > It
> > still needs some work, but in current state can be considered an
> > example of DSI video source implementation for my version of CDF.
> >
> > - Added Samsung S6E8AX0 DSI panel driver.
> >
> > This is the old Exynos-specific driver, just migrated to CDF and
> > with
> > some hacks to provide control over entity state to userspace using
> > lcd device. However it can be used to demonstrate video source ops
> > in
> > use.
> >
> > Feel free to comment as much as you can.
> >
> > Tomasz Figa (4):
> > video: add display-core
> > video: add makefile & kconfig
> > video: display: Add exynos-dsi video source driver
> > video: display: Add Samsung s6e8ax0 display panel driver
> >
> > drivers/video/Kconfig | 1 +
> > drivers/video/Makefile | 1 +
> > drivers/video/display/Kconfig | 16 +
> > drivers/video/display/Makefile | 3 +
> > drivers/video/display/display-core.c | 295 +++++++
> > drivers/video/display/panel-s6e8ax0.c | 1027
> > ++++++++++++++++++++++
> > drivers/video/display/source-exynos_dsi.c | 1313
> > ++++++++++++++++++++++++++ include/video/display.h
> > | 230 +++++
> > include/video/exynos_dsi.h | 41 +
> > include/video/panel-s6e8ax0.h | 41 +
> > 10 files changed, 2968 insertions(+)
> > create mode 100644 drivers/video/display/Kconfig
> > create mode 100644 drivers/video/display/Makefile
> > create mode 100644 drivers/video/display/display-core.c
> > create mode 100644 drivers/video/display/panel-s6e8ax0.c
> > create mode 100644 drivers/video/display/source-exynos_dsi.c
> > create mode 100644 include/video/display.h
> > create mode 100644 include/video/exynos_dsi.h
> > create mode 100644 include/video/panel-s6e8ax0.h
^ permalink raw reply
* Re: [RFC PATCH 0/4] Common Display Framework-TF
From: Laurent Pinchart @ 2013-02-02 10:39 UTC (permalink / raw)
To: Tomasz Figa
Cc: dri-devel, linux-fbdev, linux-samsung-soc, kyungmin.park,
m.szyprowski, tomasz.figa, Daniel Vetter, Marcus Lorentzon, rob,
tomi.valkeinen, Vikas Sajjan, inki.dae, dh09.lee, ville.syrjala,
s.nawrocki
In-Reply-To: <1359560343-31636-1-git-send-email-t.figa@samsung.com>
Hi Tomasz,
Thank you for your RFC.
On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote:
> Hi,
>
> After pretty long time of trying to adapt Exynos-specific DSI display
> support to Common Display Framework I'm ready to show some preliminary RFC
> patches. This series shows some ideas for CDF that came to my mind during
> my work, some changes based on comments received by Tomi's edition of CDF
> and also preliminarys version of Exynos DSI (video source part only, still
> with some FIXMEs) and Samsung S6E8AX0 DSI panel drivers.
>
> It is heavily based on Tomi's work which can be found here:
> http://thread.gmane.org/gmane.comp.video.dri.devel/78227
>
> The code might be a bit hacky in places, as I wanted to get it to a kind
> of complete and working state first. However I hope that some of the ideas
> might useful for further works.
>
> So, here comes the TF edition of Common Clock Framework.
> --------------------------------------------------------
>
> Changes done to Tomi's edition of CDF:
>
> - Replaced set_operation_mode, set_pixel_format and set_size video source
> operations with get_params display entity operation, as it was originally
> in Laurent's version.
>
> We have discussed this matter on the mailing list and decided that it
> would be better to have the source ask the sink for its parameter
> structure and do whatever appropriate with it.
Could you briefly outline the rationale behind this ?
I'm wondering whether get_params could limit us if a device needs to modify
parameters at runtime. For instance a panel might need to change clock
frequencies or number of used data lane depending on various runtime
conditions. I don't have a real use case here, so it might just be a bit of
over-engineering.
> - Defined a preliminary set of DSI bus parameters.
>
> Following parameters are defined:
>
> 1. Pixel format used for video data transmission.
> 2. Mode flags (several bit flags ORed together):
> a) DSI_MODE_VIDEO - entity uses video mode (as opposed to command
> mode), following DSI_MODE_VIDEO_* flags are relevant only if this
> flag is set.
> b) DSI_MODE_VIDEO_BURST - entity uses burst transfer for video data
> c) DSI_MODE_VIDEO_SYNC_PULSE - entity uses sync pulses (as opposed
> to sync events)
> d) DSI_MODE_VIDEO_AUTO_VERT - entity uses automatic video mode
> detection
> e) DSI_MODE_VIDEO_HSE - entity needs horizontal sync end packets
> f) DSI_MODE_VIDEO_HFP - entity needs horizontal front porch area
> g) DSI_MODE_VIDEO_HBP - entity needs horizontal back porch area
> h) DSI_MODE_VIDEO_HSA - entity needs horizontal sync active area
> i) DSI_MODE_VSYNC_FLUSH - vertical sync pulse flushes video data
> j) DSI_MODE_EOT_PACKET - entity needs EoT packets
> 3. Bit (serial) clock frequency in HS mode.
> 4. Escape mode clock frequency.
> 5. Mask of used data lanes (each bit represents single lane).
From my experience with MIPI CSI (Camera Serial Interface, similar to DSI)
some transceivers can reroute data lanes internally. Is that true for DSI as
well ? In that case we would need a list of data lanes, not just a mask.
> 6. Command allow area in video mode - amount of lines after transmitting
> video data when generic commands are accepted.
>
> Feel free to suggest anything missing or wrong, as the list of
> parameters is based merely on what was used in original Exynos MIPI
> DSIM driver.
>
> - Redesigned source-entity matching.
>
> Now each source has name string and integer id and each entity has
> source name and source id. In addition, they can have of_node specified,
> which is used for Device Tree-based matching.
>
> The matching procedure is as follows:
>
> 1. Matching takes place when display entity registers.
> a) If there is of_node specified for the entity then "video-source"
> phandle of this node is being parsed and list of registered sources
> is traversed in search of a source with of_node received from
> parsing the phandle.
> b) Otherwise the matching key is a pair of source name and id.
> 2. If no matching source is found, display_entity_register returns
> -EPROBE_DEFER error which causes the entity driver to defer its
> probe until the source registers.
> 3. Otherwise an optional bind operation of video source is called,
> sink field of source and source field of entity are set and the
> matching ends successfully.
>
> - Some initial concept of source-entity cross-locking.
>
> Only video source is protected at the moment, as I still have to think
> a bit about locking the entity in a way where removing it by user request
> is still possible.
One of the main locking issues here are cross-references. The display entity
holds a reference to the video source (for video operations), and the display
controller driver holds a reference to the display entity (for control
operations), resulting in a cross-references lock situation. One possible
solution would be to first unbind the display entity device from its driver to
break the cycle.
> - Dropped any panels and chips that I can't test.
>
> They are irrelevant for this series, so there is no point in including
> them.
>
> - Added Exynos DSI video source driver.
>
> This is a new driver for the DSI controller found in Exynos SoCs. It
> still needs some work, but in current state can be considered an example
> of DSI video source implementation for my version of CDF.
>
> - Added Samsung S6E8AX0 DSI panel driver.
>
> This is the old Exynos-specific driver, just migrated to CDF and with
> some hacks to provide control over entity state to userspace using
> lcd device. However it can be used to demonstrate video source ops in
> use.
>
> Feel free to comment as much as you can.
>
> Tomasz Figa (4):
> video: add display-core
> video: add makefile & kconfig
> video: display: Add exynos-dsi video source driver
> video: display: Add Samsung s6e8ax0 display panel driver
>
> drivers/video/Kconfig | 1 +
> drivers/video/Makefile | 1 +
> drivers/video/display/Kconfig | 16 +
> drivers/video/display/Makefile | 3 +
> drivers/video/display/display-core.c | 295 +++++++
> drivers/video/display/panel-s6e8ax0.c | 1027 ++++++++++++++++++++++
> drivers/video/display/source-exynos_dsi.c | 1313 ++++++++++++++++++++++++++
> include/video/display.h | 230 +++++
> include/video/exynos_dsi.h | 41 +
> include/video/panel-s6e8ax0.h | 41 +
> 10 files changed, 2968 insertions(+)
> create mode 100644 drivers/video/display/Kconfig
> create mode 100644 drivers/video/display/Makefile
> create mode 100644 drivers/video/display/display-core.c
> create mode 100644 drivers/video/display/panel-s6e8ax0.c
> create mode 100644 drivers/video/display/source-exynos_dsi.c
> create mode 100644 include/video/display.h
> create mode 100644 include/video/exynos_dsi.h
> create mode 100644 include/video/panel-s6e8ax0.h
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Rob Clark @ 2013-02-02 10:08 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Rahul Sharma, Thomas Petazzoni, Linux Fbdev development list,
Benjamin Gaignard, Tom Gall, Kyungmin Park,
dri-devel@lists.freedesktop.org, Ragesh Radhakrishnan,
Tomi Valkeinen, Philipp Zabel, Maxime Ripard, Vikas Sajjan,
Sumit Semwal, Sebastien Guiriec, linux-media@vger.kernel.org
In-Reply-To: <1699935.hiBjgCN6cp@avalon>
On Fri, Feb 1, 2013 at 5:42 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rahul,
>
> On Wednesday 09 January 2013 13:53:30 Rahul Sharma wrote:
>> Hi Laurent,
>>
>> CDF will also be helpful in supporting Panels with integrated audio
>> (HDMI/DP) if we can add audio related control operations to
>> display_entity_control_ops. Video controls will be called by crtc in DRM/V4L
>> and audio controls from Alsa.
>
> I knew that would come up at some point :-) I agree with you that adding audio
> support would be a very nice improvement, and I'm totally open to that, but I
> will concentrate on video, at least to start with. The first reason is that
> I'm not familiar enough with ALSA, and the second that there's only 24h per
> day :-)
>
> Please feel free, of course, to submit a proposal for audio support.
>
>> Secondly, if I need to support get_modes operation in hdmi/dp panel, I need
>> to implement edid parser inside the panel driver. It will be meaningful to
>> add get_edid control operation for hdmi/dp.
>
> Even if EDID data is parsed in the panel driver, raw EDID will still need to
> be exported, so a get_edid control operation (or something similar) is
> definitely needed. There's no disagreement on this, I just haven't included
> that operation yet because my test hardware is purely panel-based.
one of (probably many) places that just keeping CDF (CDH? common
display helpers..) inside DRM makes life easier :-P
BR,
-R
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Laurent Pinchart @ 2013-02-01 23:42 UTC (permalink / raw)
To: Rahul Sharma
Cc: Rob Clark, Thomas Petazzoni, Linux Fbdev development list,
Benjamin Gaignard, Tom Gall, Kyungmin Park,
dri-devel@lists.freedesktop.org, Ragesh Radhakrishnan,
Tomi Valkeinen, Philipp Zabel, Maxime Ripard, Vikas Sajjan,
Sumit Semwal, Sebastien Guiriec, linux-media@vger.kernel.org
In-Reply-To: <CAPdUM4P6riQVJ4m4Sdkh1O8xmpKF4YnhGq69p7DkxAdr165KYA@mail.gmail.com>
Hi Rahul,
On Wednesday 09 January 2013 13:53:30 Rahul Sharma wrote:
> Hi Laurent,
>
> CDF will also be helpful in supporting Panels with integrated audio
> (HDMI/DP) if we can add audio related control operations to
> display_entity_control_ops. Video controls will be called by crtc in DRM/V4L
> and audio controls from Alsa.
I knew that would come up at some point :-) I agree with you that adding audio
support would be a very nice improvement, and I'm totally open to that, but I
will concentrate on video, at least to start with. The first reason is that
I'm not familiar enough with ALSA, and the second that there's only 24h per
day :-)
Please feel free, of course, to submit a proposal for audio support.
> Secondly, if I need to support get_modes operation in hdmi/dp panel, I need
> to implement edid parser inside the panel driver. It will be meaningful to
> add get_edid control operation for hdmi/dp.
Even if EDID data is parsed in the panel driver, raw EDID will still need to
be exported, so a get_edid control operation (or something similar) is
definitely needed. There's no disagreement on this, I just haven't included
that operation yet because my test hardware is purely panel-based.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Laurent Pinchart @ 2013-02-01 23:37 UTC (permalink / raw)
To: Marcus Lorentzon
Cc: Tomasz Figa, Thomas Petazzoni, linux-fbdev@vger.kernel.org,
Philipp Zabel, Tom Gall, Ragesh Radhakrishnan,
dri-devel@lists.freedesktop.org, Rob Clark, Kyungmin Park,
Tomi Valkeinen, sunil joshi, Benjamin Gaignard, Bryan Wu,
Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
linux-media@vger.kernel.org
In-Reply-To: <50EBF10A.7080906@stericsson.com>
Hi Marcus,
On Tuesday 08 January 2013 11:12:26 Marcus Lorentzon wrote:
[snip]
> I also looked at the video source in Tomi's git tree
> (http://gitorious.org/linux-omap-dss2/linux/blobs/work/dss-dev-model-cdf/inc
> lude/video/display.h). I think I would prefer a single "setup" op taking a
> "struct dsi_config" as argument. Then each DSI formatter/encoder driver
> could decide best way to set that up. We have something similar at
> http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f=inc
> lude/video/mcde.h;hI9ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD#l118
A single setup function indeed seems easier, but I don't have enough
experience with DSI to have a strong opinion on that. We'll have to compare
implementations if there's a disagreement on this.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC v2 0/5] Common Display Framework
From: Laurent Pinchart @ 2013-02-01 23:35 UTC (permalink / raw)
To: Marcus Lorentzon
Cc: Tomasz Figa, Thomas Petazzoni, linux-fbdev@vger.kernel.org,
Philipp Zabel, Tom Gall, Ragesh Radhakrishnan,
dri-devel@lists.freedesktop.org, Rob Clark, Kyungmin Park,
Tomi Valkeinen, sunil joshi, Benjamin Gaignard, Bryan Wu,
Maxime Ripard, Vikas Sajjan, Sumit Semwal, Sebastien Guiriec,
linux-media@vger.kernel.org
In-Reply-To: <50EC5283.80006@stericsson.com>
Hi Marcus,
On Tuesday 08 January 2013 18:08:19 Marcus Lorentzon wrote:
> On 01/08/2013 05:36 PM, Tomasz Figa wrote:
> > On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote:
[snip]
> >> FYI,
> >> here is STE "DSI API":
> >> http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f
> >> =include/video/mcde.h;hI9ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD
> >> #l361
Thank you.
> >> But it is not perfect. After a couple of products we realized that most
> >> panel drivers want an easy way to send a bunch of init commands in one
> >> go. So I think it should be an op for sending an array of commands at
> >> once. Something like
> >>
> >> struct dsi_cmd {
> >> enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */
> >> u8 cmd;
> >> int dataLen;
> >> u8 *data;
> >> }
> >>
> >> struct dsi_ops {
> >> int dsi_write(source, int num_cmds, struct dsi_cmd *cmds);
> >> ...
> >> }
Do you have DSI IP(s) that can handle a list of commands ? Or would all DSI
transmitter drivers need to iterate over the commands manually ? In the later
case a lower-level API might be easier to implement in DSI transmitter
drivers. Helper functions could provide the higher-level API you proposed.
> > Yes, this should be flexible enough to cover most of (or even whole) DSI
> > specification.
> >
> > However I'm not sure whether the dsi_write name would be appropriate,
> > since DSI packet types include also read and special transactions. So,
> > according to DSI terminology, maybe dsi_transaction would be better?
Or dsi_transfer or dsi_xfer ? Does the DSI bus have a concept of transactions
?
> I think read should still be separate. At least on my HW read and write
> are quite different. But all "transactions" are very much the same in HW
> setup. The ... was dsi_write etc ;) Like set_max_packet_size should
> maybe be an ops. Since only the implementer of the "video source" will
> know what the max read return packet size for that DSI IP is. The panels
> don't know that. Maybe another ops to retrieve some info about the caps
> of the video source would help that. Then a helper could call that and
> then the dsi_write one.
If panels (or helper functions) need information about the DSI transmitter
capabilities, sure, we can add an op.
> >> And I think I still prefer the dsi_bus in favor of the abstract video
> >> source. It just looks like a home made bus with bus-ops ... can't you do
> >> something similar using the normal driver framework? enable/disable
> >> looks like suspend/resume, register/unregister_vid_src is like
> >> bus_(un)register_device, ... the video source anyway seems unattached
> >> to the panel stuff with the find_video_source call.
The Linux driver framework is based on control busses. DSI usually handles
both control and video transfer, but the control and data busses can also be
separate (think DPI + SPI/I2C for instance). In that case the panel will be a
child of its control bus master, and will need a separate interface to access
video data operations. As a separate video interface is thus needed, I think
we should use it for DSI as well.
My initial proposal included a DBI bus (as I don't have any DSI hardware - DBI
and DSI can be used interchangeably in this discussions, they both share the
caracteristic of having a common control + data bus), and panels were children
of the DBI bus master. The DBI bus API was only used for control, not for data
transfers. Tomi then removed the DBI bus and moved the control operations to
the video source, turning the DBI panels into platform devices. I still favor
my initial approach, but I can agree to drop the DBI bus if there's a
consensus on that. Video bus operations will be separate in any case.
> > DSI needs specific power management. It's necessary to power up the panel
> > first to make it wait for Tinit event and then enable DSI master to
> > trigger such event. Only then rest of panel initialization can be
> > completed.
>
> I know, we have a very complex sequence for our HDMI encoder which uses
> sort of continuous DSI cmmand mode. And power/clock on sequences are
> tricky to get right in our current "CDF" API (mcde_display). But I fail
> to see how the current video source API is different from just using the
> bus/device APIs.
As mentioned above, the video source API handles video transfers, while the
bus/device API handles control transfers. Operations such as "start the video
stream" will thus be video source APIs. Operations such as "enable the DSI
master", used to trigger the Tinit event (whatever that is :-)) at power up
time would probably be DSI bus operations.
> > Also, as discussed in previous posts, some panels might use DSI only for
> > video data and another interface (I2C, SPI) for control data. In such case
> > it would be impossible to represent such device in a reasonable way using
> > current driver model.
>
> I understand that you need to get hold of both the control and data bus
> device in the driver. (Toshiba DSI-LVDS bridge is a good example and
> commonly used "encoder" that can use both DSI and I2C control interface.)
> But the control bus you get from device probe, and I guess you could call
> bus_find_device_by_name(dsi_bus, "mydev") and return the "datadev" which
> will have access to dsi bus ops just as you call
> find_video_source("mysource") to access the "databus" ops directly with
> a logical device (display entity).
> I'm not saying I would refuse to use video sources. I just think the two
> models are so similar so it would be worth exploring how a device model
> style API would look like and to compare against.
I don't think we should use the Linux device model for data busses. It hasn't
been designed for that use case, and definitely doesn't support devices that
would be children of two separate masters (control and data). For shared bus
devices such as DSI, having a DSI bus was my preference to start with, as
mentioned above :-) However, even in that case, I think it would still make
sense to use video sources to control the video operations. As usual the devil
is in the details, so there will probably be some tricky problems we'll need
to solve, but that will require coding the proposed solution.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] video: add ili922x lcd driver
From: Anatolij Gustschin @ 2013-02-01 14:41 UTC (permalink / raw)
To: linux-fbdev
From: Stefano Babic <sbabic@denx.de>
Add LCD driver for Ilitek ILI9221/ILI9222 controller.
Signed-off-by: Stefano Babic <sbabic@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Richard Purdie <rpurdie@rpsys.net>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
---
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/ili922x.c | 586 +++++++++++++++++++++++++++++++++++++
3 files changed, 594 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/backlight/ili922x.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 765a945..97b4672 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -59,6 +59,13 @@ config LCD_LTV350QV
The LTV350QV panel is present on all ATSTK1000 boards.
+config LCD_ILI922X
+ tristate "ILI Technology ILI9221/ILI9222 support"
+ depends on SPI
+ help
+ If you have a panel based on the ILI9221/9222 controller
+ chip then say y to include a driver for it.
+
config LCD_ILI9320
tristate "ILI Technology ILI9320 controller support"
depends on SPI
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index e7ce729..3cfd901 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_LCD_HP700) += jornada720_lcd.o
obj-$(CONFIG_LCD_L4F00242T03) += l4f00242t03.o
obj-$(CONFIG_LCD_LMS283GF05) += lms283gf05.o
obj-$(CONFIG_LCD_LTV350QV) += ltv350qv.o
+obj-$(CONFIG_LCD_ILI922X) += ili922x.o
obj-$(CONFIG_LCD_ILI9320) += ili9320.o
obj-$(CONFIG_LCD_PLATFORM) += platform_lcd.o
obj-$(CONFIG_LCD_VGG2432A4) += vgg2432a4.o
diff --git a/drivers/video/backlight/ili922x.c b/drivers/video/backlight/ili922x.c
new file mode 100644
index 0000000..18c33df
--- /dev/null
+++ b/drivers/video/backlight/ili922x.c
@@ -0,0 +1,586 @@
+/*
+ * (C) Copyright 2008
+ * Stefano Babic, DENX Software Engineering, sbabic@denx.de.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * This driver implements a lcd device for the ILITEK 922x display
+ * controller. The interface to the display is SPI and the display's
+ * memory is cyclically updated
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/fb.h>
+#include <linux/init.h>
+#include <linux/lcd.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+/* Register offset, see manual section 8.2 */
+#define REG_START_OSCILLATION 0x00
+#define REG_DRIVER_CODE_READ 0x00
+#define REG_DRIVER_OUTPUT_CONTROL 0x01
+#define REG_LCD_AC_DRIVEING_CONTROL 0x02
+#define REG_ENTRY_MODE 0x03
+#define REG_COMPARE_1 0x04
+#define REG_COMPARE_2 0x05
+#define REG_DISPLAY_CONTROL_1 0x07
+#define REG_DISPLAY_CONTROL_2 0x08
+#define REG_DISPLAY_CONTROL_3 0x09
+#define REG_FRAME_CYCLE_CONTROL 0x0B
+#define REG_EXT_INTF_CONTROL 0x0C
+#define REG_POWER_CONTROL_1 0x10
+#define REG_POWER_CONTROL_2 0x11
+#define REG_POWER_CONTROL_3 0x12
+#define REG_POWER_CONTROL_4 0x13
+#define REG_RAM_ADDRESS_SET 0x21
+#define REG_WRITE_DATA_TO_GRAM 0x22
+#define REG_RAM_WRITE_MASK1 0x23
+#define REG_RAM_WRITE_MASK2 0x24
+#define REG_GAMMA_CONTROL_1 0x30
+#define REG_GAMMA_CONTROL_2 0x31
+#define REG_GAMMA_CONTROL_3 0x32
+#define REG_GAMMA_CONTROL_4 0x33
+#define REG_GAMMA_CONTROL_5 0x34
+#define REG_GAMMA_CONTROL_6 0x35
+#define REG_GAMMA_CONTROL_7 0x36
+#define REG_GAMMA_CONTROL_8 0x37
+#define REG_GAMMA_CONTROL_9 0x38
+#define REG_GAMMA_CONTROL_10 0x39
+#define REG_GATE_SCAN_CONTROL 0x40
+#define REG_VERT_SCROLL_CONTROL 0x41
+#define REG_FIRST_SCREEN_DRIVE_POS 0x42
+#define REG_SECOND_SCREEN_DRIVE_POS 0x43
+#define REG_RAM_ADDR_POS_H 0x44
+#define REG_RAM_ADDR_POS_V 0x45
+#define REG_OSCILLATOR_CONTROL 0x4F
+#define REG_GPIO 0x60
+#define REG_OTP_VCM_PROGRAMMING 0x61
+#define REG_OTP_VCM_STATUS_ENABLE 0x62
+#define REG_OTP_PROGRAMMING_ID_KEY 0x65
+
+/*
+ * maximum frequency for register access
+ * (not for the GRAM access)
+ */
+#define ILITEK_MAX_FREQ_REG 4000000
+
+/*
+ * Device ID as found in the datasheet (supports 9221 and 9222)
+ */
+#define ILITEK_DEVICE_ID 0x9220
+#define ILITEK_DEVICE_ID_MASK 0xFFF0
+
+/* Last two bits in the START BYTE */
+#define START_RS_INDEX 0
+#define START_RS_REG 1
+#define START_RW_WRITE 0
+#define START_RW_READ 1
+
+/**
+ * START_BYTE(id, rs, rw)
+ *
+ * Set the start byte according to the required operation.
+ * The start byte is defined as:
+ * ----------------------------------
+ * | 0 | 1 | 1 | 1 | 0 | ID | RS | RW |
+ * ----------------------------------
+ * @id: display's id as set by the manufacturer
+ * @rs: operation type bit, one of:
+ * - START_RS_INDEX set the index register
+ * - START_RS_REG write/read registers/GRAM
+ * @rw: read/write operation
+ * - START_RW_WRITE write
+ * - START_RW_READ read
+ */
+#define START_BYTE(id, rs, rw) \
+ (0x70 | (((id) & 0x01) << 2) | (((rs) & 0x01) << 1) | ((rw) & 0x01))
+
+/**
+ * CHECK_FREQ_REG(spi_device s, spi_transfer x) - Check the frequency
+ * for the SPI transfer. According to the datasheet, the controller
+ * accept higher frequency for the GRAM transfer, but it requires
+ * lower frequency when the registers are read/written.
+ * The macro sets the frequency in the spi_transfer structure if
+ * the frequency exceeds the maximum value.
+ */
+#define CHECK_FREQ_REG(s, x) \
+ do { \
+ if (s->max_speed_hz > ILITEK_MAX_FREQ_REG) \
+ ((struct spi_transfer *)x)->speed_hz = \
+ ILITEK_MAX_FREQ_REG; \
+ } while (0)
+
+#define CMD_BUFSIZE 16
+
+#define POWER_IS_ON(pwr) ((pwr) <= FB_BLANK_NORMAL)
+
+#define set_tx_byte(b) (tx_invert ? ~(b) : b)
+
+/**
+ * ili922x_id - id as set by manufacturer
+ */
+static int ili922x_id = 1;
+module_param(ili922x_id, int, 0);
+
+static int tx_invert;
+module_param(tx_invert, int, 0);
+
+/**
+ * driver's private structure
+ */
+struct ili922x {
+ struct spi_device *spi;
+ struct lcd_device *ld;
+ int power;
+};
+
+#define NUM_DUMMY_BYTES 1
+static void send_dummy(struct spi_device *spi)
+{
+ struct spi_message m;
+ struct spi_transfer xfer;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret = 0, i;
+
+ return;
+
+ memset(&xfer, 0, sizeof(struct spi_transfer));
+ spi_message_init(&m);
+ xfer.tx_buf = tbuf;
+ xfer.rx_buf = rbuf;
+ xfer.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer);
+
+ for (i = 0; i < NUM_DUMMY_BYTES; i++)
+ tbuf[i] = set_tx_byte(0xFF);
+
+ xfer.bits_per_word = 8;
+ xfer.len = NUM_DUMMY_BYTES;
+ spi_message_add_tail(&xfer, &m);
+ ret = spi_sync(spi, &m);
+
+ udelay(10);
+}
+
+/**
+ * read_status - read status register from display
+ * @spi: spi device
+ */
+static u16 read_status(struct spi_device *spi)
+{
+ struct spi_message m;
+ struct spi_transfer xfer;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret = 0, i;
+
+ send_dummy(spi);
+
+ memset(&xfer, 0, sizeof(struct spi_transfer));
+ spi_message_init(&m);
+ xfer.tx_buf = tbuf;
+ xfer.rx_buf = rbuf;
+ xfer.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_READ));
+ for (i = 1; i < 4; i++)
+ tbuf[i] = set_tx_byte(0); /* dummy */
+
+ xfer.bits_per_word = 8;
+ xfer.len = 4;
+ spi_message_add_tail(&xfer, &m);
+ ret = spi_sync(spi, &m);
+
+ return (rbuf[2] << 8) + rbuf[3];
+}
+
+/**
+ * read_reg - read register from display
+ * @spi: spi device
+ * @reg: offset of the register to be read
+ * @rx: output value
+ */
+static int read_reg(struct spi_device *spi, u8 reg, u16 *rx)
+{
+ struct spi_message m;
+ struct spi_transfer xfer_regindex, xfer_regvalue;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret = 0, len = 0, i, send_bytes;
+
+ send_dummy(spi);
+
+ memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
+ memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
+ spi_message_init(&m);
+ xfer_regindex.tx_buf = tbuf;
+ xfer_regindex.rx_buf = rbuf;
+ xfer_regindex.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer_regindex);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte(0);
+ tbuf[2] = set_tx_byte(reg);
+ xfer_regindex.bits_per_word = 8;
+ len = xfer_regindex.len = 3;
+ spi_message_add_tail(&xfer_regindex, &m);
+
+ send_bytes = len;
+
+ tbuf[len++] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
+ START_RW_READ));
+ for (i = len; i < CMD_BUFSIZE; i++)
+ tbuf[i] = set_tx_byte(0); /* dummy */
+
+ xfer_regvalue.cs_change = 1;
+ xfer_regvalue.len = 4;
+ xfer_regvalue.tx_buf = &tbuf[send_bytes];
+ xfer_regvalue.rx_buf = &rbuf[send_bytes];
+ CHECK_FREQ_REG(spi, &xfer_regvalue);
+
+ spi_message_add_tail(&xfer_regvalue, &m);
+ ret = spi_sync(spi, &m);
+ if (ret < 0) {
+ dev_dbg(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
+
+ *rx = (rbuf[1 + send_bytes] << 8) + rbuf[2 + send_bytes];
+ return 0;
+}
+
+/**
+ * write_reg - write a controller register
+ * @spi: struct spi_device *
+ * @reg: offset of the register to be written
+ * @value: value to be written
+ */
+static int write_reg(struct spi_device *spi, u8 reg, u16 value)
+{
+ struct spi_message m;
+ struct spi_transfer xfer_regindex, xfer_regvalue;
+ unsigned char tbuf[CMD_BUFSIZE];
+ unsigned char rbuf[CMD_BUFSIZE];
+ int ret = 0, len = 0;
+
+ send_dummy(spi);
+
+ memset(&xfer_regindex, 0, sizeof(struct spi_transfer));
+ memset(&xfer_regvalue, 0, sizeof(struct spi_transfer));
+
+ spi_message_init(&m);
+ xfer_regindex.tx_buf = tbuf;
+ xfer_regindex.rx_buf = rbuf;
+ xfer_regindex.cs_change = 1;
+ CHECK_FREQ_REG(spi, &xfer_regindex);
+
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_INDEX,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte(0);
+ tbuf[2] = set_tx_byte(reg);
+ xfer_regindex.bits_per_word = 8;
+ xfer_regindex.len = 3;
+ spi_message_add_tail(&xfer_regindex, &m);
+
+ ret = spi_sync(spi, &m);
+
+ send_dummy(spi);
+
+ spi_message_init(&m);
+ len = 0;
+ tbuf[0] = set_tx_byte(START_BYTE(ili922x_id, START_RS_REG,
+ START_RW_WRITE));
+ tbuf[1] = set_tx_byte((value & 0xFF00) >> 8);
+ tbuf[2] = set_tx_byte(value & 0x00FF);
+
+ xfer_regvalue.cs_change = 1;
+ xfer_regvalue.len = 3;
+ xfer_regvalue.tx_buf = tbuf;
+ xfer_regvalue.rx_buf = rbuf;
+ CHECK_FREQ_REG(spi, &xfer_regvalue);
+
+ spi_message_add_tail(&xfer_regvalue, &m);
+
+ ret = spi_sync(spi, &m);
+ if (ret < 0) {
+ dev_err(&spi->dev, "Error sending SPI message 0x%x", ret);
+ return ret;
+ }
+ return 0;
+}
+
+#ifdef DEBUG
+/**
+ * ili922x_reg_dump - dump all registers
+ */
+static void ili922x_reg_dump(struct spi_device *spi)
+{
+ u8 reg;
+ u16 rx;
+
+ pr_info("ILI922x configuration registers:\n");
+ for (reg = REG_START_OSCILLATION;
+ reg <= REG_OTP_PROGRAMMING_ID_KEY; reg++) {
+ read_reg(spi, reg, &rx);
+ pr_info("reg @ 0x%02X: 0x%04X\n", reg, rx);
+ }
+}
+#else
+static inline void ili922x_reg_dump(struct spi_device *spi) {}
+#endif
+
+/**
+ * set_write_to_gram_reg - initialize the display to write the GRAM
+ * @spi: spi device
+ */
+static void set_write_to_gram_reg(struct spi_device *spi)
+{
+ struct spi_message m;
+ struct spi_transfer xfer;
+ unsigned char tbuf[CMD_BUFSIZE];
+
+ memset(&xfer, 0, sizeof(struct spi_transfer));
+
+ spi_message_init(&m);
+ xfer.tx_buf = tbuf;
+ xfer.rx_buf = NULL;
+ xfer.cs_change = 1;
+
+ tbuf[0] = START_BYTE(ili922x_id, START_RS_INDEX, START_RW_WRITE);
+ tbuf[1] = 0;
+ tbuf[2] = REG_WRITE_DATA_TO_GRAM;
+
+ xfer.bits_per_word = 8;
+ xfer.len = 3;
+ spi_message_add_tail(&xfer, &m);
+ spi_sync(spi, &m);
+}
+
+/**
+ * ili922x_poweron - turn the display on
+ * @spi: spi device
+ *
+ * The sequence to turn on the display is taken from
+ * the datasheet and/or the example code provided by the
+ * manufacturer.
+ */
+static int ili922x_poweron(struct spi_device *spi)
+{
+ int ret = 0;
+
+ /* Power on */
+ ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
+ mdelay(10);
+ ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
+ mdelay(40);
+ ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
+ mdelay(40);
+ ret += write_reg(spi, 0x56, 0x080F);
+ ret += write_reg(spi, REG_POWER_CONTROL_1, 0x4240);
+ mdelay(10);
+ ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0014);
+ mdelay(40);
+ ret += write_reg(spi, REG_POWER_CONTROL_4, 0x1319);
+ mdelay(40);
+
+ return ret;
+}
+
+/**
+ * ili922x_poweroff - turn the display off
+ * @spi: spi device
+ */
+static int ili922x_poweroff(struct spi_device *spi)
+{
+ int ret = 0;
+
+ /* Power off */
+ ret = write_reg(spi, REG_POWER_CONTROL_1, 0x0000);
+ mdelay(10);
+ ret += write_reg(spi, REG_POWER_CONTROL_2, 0x0000);
+ ret += write_reg(spi, REG_POWER_CONTROL_3, 0x0000);
+ mdelay(40);
+ ret += write_reg(spi, REG_POWER_CONTROL_4, 0x0000);
+ mdelay(40);
+
+ return ret;
+}
+
+/**
+ * ili922x_display_init - initialize the display by setting
+ * the configuration registers
+ * @spi: spi device
+ */
+static void ili922x_display_init(struct spi_device *spi)
+{
+ write_reg(spi, REG_START_OSCILLATION, 1);
+ mdelay(10);
+ write_reg(spi, REG_DRIVER_OUTPUT_CONTROL, 0x691B);
+ write_reg(spi, REG_LCD_AC_DRIVEING_CONTROL, 0x0700);
+ write_reg(spi, REG_ENTRY_MODE, 0x1030);
+ write_reg(spi, REG_COMPARE_1, 0x0000);
+ write_reg(spi, REG_COMPARE_2, 0x0000);
+ write_reg(spi, REG_DISPLAY_CONTROL_1, 0x0037);
+ write_reg(spi, REG_DISPLAY_CONTROL_2, 0x0202);
+ write_reg(spi, REG_DISPLAY_CONTROL_3, 0x0000);
+ write_reg(spi, REG_FRAME_CYCLE_CONTROL, 0x0000);
+
+ /* Set RGB interface */
+ write_reg(spi, REG_EXT_INTF_CONTROL, 0x0110);
+
+ ili922x_poweron(spi);
+
+ write_reg(spi, REG_GAMMA_CONTROL_1, 0x0302);
+ write_reg(spi, REG_GAMMA_CONTROL_2, 0x0407);
+ write_reg(spi, REG_GAMMA_CONTROL_3, 0x0304);
+ write_reg(spi, REG_GAMMA_CONTROL_4, 0x0203);
+ write_reg(spi, REG_GAMMA_CONTROL_5, 0x0706);
+ write_reg(spi, REG_GAMMA_CONTROL_6, 0x0407);
+ write_reg(spi, REG_GAMMA_CONTROL_7, 0x0706);
+ write_reg(spi, REG_GAMMA_CONTROL_8, 0x0000);
+ write_reg(spi, REG_GAMMA_CONTROL_9, 0x0C06);
+ write_reg(spi, REG_GAMMA_CONTROL_10, 0x0F00);
+ write_reg(spi, REG_RAM_ADDRESS_SET, 0x0000);
+ write_reg(spi, REG_GATE_SCAN_CONTROL, 0x0000);
+ write_reg(spi, REG_VERT_SCROLL_CONTROL, 0x0000);
+ write_reg(spi, REG_FIRST_SCREEN_DRIVE_POS, 0xDB00);
+ write_reg(spi, REG_SECOND_SCREEN_DRIVE_POS, 0xDB00);
+ write_reg(spi, REG_RAM_ADDR_POS_H, 0xAF00);
+ write_reg(spi, REG_RAM_ADDR_POS_V, 0xDB00);
+ ili922x_reg_dump(spi);
+ set_write_to_gram_reg(spi);
+}
+
+static int ili922x_lcd_power(struct ili922x *lcd, int power)
+{
+ int ret = 0;
+
+ if (POWER_IS_ON(power) && !POWER_IS_ON(lcd->power))
+ ret = ili922x_poweron(lcd->spi);
+ else if (!POWER_IS_ON(power) && POWER_IS_ON(lcd->power))
+ ret = ili922x_poweroff(lcd->spi);
+
+ if (!ret)
+ lcd->power = power;
+
+ return ret;
+}
+
+static int ili922x_set_power(struct lcd_device *ld, int power)
+{
+ struct ili922x *ili = lcd_get_data(ld);
+
+ return ili922x_lcd_power(ili, power);
+}
+
+static int ili922x_get_power(struct lcd_device *ld)
+{
+ struct ili922x *ili = lcd_get_data(ld);
+
+ return ili->power;
+}
+
+static struct lcd_ops ili922x_ops = {
+ .get_power = ili922x_get_power,
+ .set_power = ili922x_set_power,
+};
+
+static int ili922x_probe(struct spi_device *spi)
+{
+ struct ili922x *ili;
+ struct lcd_device *lcd;
+ int ret;
+ u16 reg = 0;
+
+ ili = devm_kzalloc(&spi->dev, sizeof(*ili), GFP_KERNEL);
+ if (!ili) {
+ dev_err(&spi->dev, "cannot alloc priv data\n");
+ return -ENOMEM;
+ }
+
+ ili->spi = spi;
+ dev_set_drvdata(&spi->dev, ili);
+
+ /* check if the device is connected */
+ ret = read_reg(spi, REG_DRIVER_CODE_READ, ®);
+ if (ret || ((reg & ILITEK_DEVICE_ID_MASK) != ILITEK_DEVICE_ID)) {
+ dev_err(&spi->dev,
+ "no LCD found: Chip ID 0x%x, ret %d\n",
+ reg, ret);
+ return -ENODEV;
+ } else {
+ dev_info(&spi->dev, "ILI%x found, SPI freq %d, mode %d\n",
+ reg, spi->max_speed_hz, spi->mode);
+ }
+
+ dev_dbg(&spi->dev, "status: 0x%x\n", read_status(spi));
+
+ ili922x_display_init(spi);
+
+ ili->power = FB_BLANK_POWERDOWN;
+
+ lcd = lcd_device_register("ili922xlcd", &spi->dev, ili,
+ &ili922x_ops);
+ if (IS_ERR(lcd)) {
+ dev_err(&spi->dev, "cannot register LCD\n");
+ return PTR_ERR(lcd);
+ }
+
+ ili->ld = lcd;
+ spi_set_drvdata(spi, ili);
+
+ ili922x_lcd_power(ili, FB_BLANK_UNBLANK);
+
+ return 0;
+}
+
+static int ili922x_remove(struct spi_device *spi)
+{
+ struct ili922x *ili = spi_get_drvdata(spi);
+
+ ili922x_poweroff(spi);
+ lcd_device_unregister(ili->ld);
+ return 0;
+}
+
+static struct spi_driver ili922x_driver = {
+ .driver = {
+ .name = "ili922x",
+ .owner = THIS_MODULE,
+ },
+ .probe = ili922x_probe,
+ .remove = ili922x_remove,
+};
+
+module_spi_driver(ili922x_driver);
+
+MODULE_AUTHOR("Stefano Babic <sbabic@denx.de>");
+MODULE_DESCRIPTION("ILI9221/9222 LCD driver");
+MODULE_LICENSE("GPL");
+MODULE_PARM_DESC(ili922x_id, "set controller identifier (default=1)");
+MODULE_PARM_DESC(tx_invert, "invert bytes before sending");
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH v17 4/7] fbmon: add videomode helpers
From: Jingoo Han @ 2013-02-01 9:33 UTC (permalink / raw)
To: Steffen Trumtrar
Cc: Mohammed, Afzal, Jingoo Han, Florian Tobias Schandinat,
Dave Airlie,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Tomi Valkeinen, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
Guennady Liakhovetski,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1359104515-8907-5-git-send-email-s.trumtrar@pengutronix.de>
T24gRnJpZGF5LCBKYW51YXJ5IDI1LCAyMDEzIDY6MDIgUE0sIFN0ZWZmZW4gVHJ1bXRyYXIgd3Jv
dGUNCj4gDQo+IEFkZCBhIGZ1bmN0aW9uIHRvIGNvbnZlcnQgZnJvbSB0aGUgZ2VuZXJpYyB2aWRl
b21vZGUgdG8gYSBmYl92aWRlb21vZGUuDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBTdGVmZmVuIFRy
dW10cmFyIDxzLnRydW10cmFyQHBlbmd1dHJvbml4LmRlPg0KPiBSZXZpZXdlZC1ieTogVGhpZXJy
eSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRlPg0KPiBBY2tlZC1ieTog
VGhpZXJyeSBSZWRpbmcgPHRoaWVycnkucmVkaW5nQGF2aW9uaWMtZGVzaWduLmRlPg0KPiBUZXN0
ZWQtYnk6IFRoaWVycnkgUmVkaW5nIDx0aGllcnJ5LnJlZGluZ0BhdmlvbmljLWRlc2lnbi5kZT4N
Cj4gVGVzdGVkLWJ5OiBQaGlsaXBwIFphYmVsIDxwLnphYmVsQHBlbmd1dHJvbml4LmRlPg0KPiBS
ZXZpZXdlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9h
cmQuY29tPg0KPiBBY2tlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBp
ZGVhc29uYm9hcmQuY29tPg0KPiBUZXN0ZWQtYnk6IEFmemFsIE1vaGFtbWVkIDxBZnphbEB0aS5j
b20+DQo+IFRlc3RlZC1ieTogUm9iIENsYXJrIDxyb2JjbGFya0BnbWFpbC5jb20+DQo+IFRlc3Rl
ZC1ieTogTGVlbGEgS3Jpc2huYSBBbXVkYWxhIDxsZWVsYWtyaXNobmEuYUBnbWFpbC5jb20+DQo+
IC0tLQ0KPiAgZHJpdmVycy92aWRlby9mYm1vbi5jIHwgICA1MiArKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrDQo+ICBpbmNsdWRlL2xpbnV4L2ZiLmggICAg
fCAgICA0ICsrKysNCj4gIDIgZmlsZXMgY2hhbmdlZCwgNTYgaW5zZXJ0aW9ucygrKQ0KPiANCj4g
ZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmlkZW8vZmJtb24uYyBiL2RyaXZlcnMvdmlkZW8vZmJtb24u
Yw0KPiBpbmRleCBjZWY2NTU3Li4xN2NlMTM1IDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL3ZpZGVv
L2ZibW9uLmMNCj4gKysrIGIvZHJpdmVycy92aWRlby9mYm1vbi5jDQo+IEBAIC0zMSw2ICszMSw3
IEBADQo+ICAjaW5jbHVkZSA8bGludXgvcGNpLmg+DQo+ICAjaW5jbHVkZSA8bGludXgvc2xhYi5o
Pg0KPiAgI2luY2x1ZGUgPHZpZGVvL2VkaWQuaD4NCj4gKyNpbmNsdWRlIDx2aWRlby92aWRlb21v
ZGUuaD4NCj4gICNpZmRlZiBDT05GSUdfUFBDX09GDQo+ICAjaW5jbHVkZSA8YXNtL3Byb20uaD4N
Cj4gICNpbmNsdWRlIDxhc20vcGNpLWJyaWRnZS5oPg0KPiBAQCAtMTM3Myw2ICsxMzc0LDU3IEBA
IGludCBmYl9nZXRfbW9kZShpbnQgZmxhZ3MsIHUzMiB2YWwsIHN0cnVjdCBmYl92YXJfc2NyZWVu
aW5mbyAqdmFyLCBzdHJ1Y3QgZmJfaW5mDQo+ICAJa2ZyZWUodGltaW5ncyk7DQo+ICAJcmV0dXJu
IGVycjsNCj4gIH0NCj4gKw0KPiArI2lmIElTX0VOQUJMRUQoQ09ORklHX1ZJREVPTU9ERSkNCj4g
K2ludCBmYl92aWRlb21vZGVfZnJvbV92aWRlb21vZGUoY29uc3Qgc3RydWN0IHZpZGVvbW9kZSAq
dm0sDQo+ICsJCQkJc3RydWN0IGZiX3ZpZGVvbW9kZSAqZmJtb2RlKQ0KPiArew0KPiArCXVuc2ln
bmVkIGludCBodG90YWwsIHZ0b3RhbDsNCj4gKw0KPiArCWZibW9kZS0+eHJlcyA9IHZtLT5oYWN0
aXZlOw0KPiArCWZibW9kZS0+bGVmdF9tYXJnaW4gPSB2bS0+aGJhY2tfcG9yY2g7DQo+ICsJZmJt
b2RlLT5yaWdodF9tYXJnaW4gPSB2bS0+aGZyb250X3BvcmNoOw0KPiArCWZibW9kZS0+aHN5bmNf
bGVuID0gdm0tPmhzeW5jX2xlbjsNCj4gKw0KPiArCWZibW9kZS0+eXJlcyA9IHZtLT52YWN0aXZl
Ow0KPiArCWZibW9kZS0+dXBwZXJfbWFyZ2luID0gdm0tPnZiYWNrX3BvcmNoOw0KPiArCWZibW9k
ZS0+bG93ZXJfbWFyZ2luID0gdm0tPnZmcm9udF9wb3JjaDsNCj4gKwlmYm1vZGUtPnZzeW5jX2xl
biA9IHZtLT52c3luY19sZW47DQo+ICsNCj4gKwkvKiBwcmV2ZW50IGRpdmlzaW9uIGJ5IHplcm8g
aW4gS0haMlBJQ09TIG1hY3JvICovDQo+ICsJZmJtb2RlLT5waXhjbG9jayA9IHZtLT5waXhlbGNs
b2NrID8NCj4gKwkJCUtIWjJQSUNPUyh2bS0+cGl4ZWxjbG9jayAvIDEwMDApIDogMDsNCj4gKw0K
PiArCWZibW9kZS0+c3luYyA9IDA7DQo+ICsJZmJtb2RlLT52bW9kZSA9IDA7DQo+ICsJaWYgKHZt
LT5kbXRfZmxhZ3MgJiBWRVNBX0RNVF9IU1lOQ19ISUdIKQ0KPiArCQlmYm1vZGUtPnN5bmMgfD0g
RkJfU1lOQ19IT1JfSElHSF9BQ1Q7DQo+ICsJaWYgKHZtLT5kbXRfZmxhZ3MgJiBWRVNBX0RNVF9I
U1lOQ19ISUdIKQ0KDQpIaSBTdGVmZmVuIFRydW10cmFyLA0KDQpVbSwgaXQgc2VlbXMgdG8gYmUg
YSB0eXBlLiAnSCdTWU5DIC0+ICdWJ1NZTkMNClRodXMsIGl0IHdvdWxkIGJlIGNoYW5nZWQgYXMg
YmVsb3c6DQoNCiAgICBWRVNBX0RNVF9IU1lOQ19ISUdIIC0+IFZFU0FfRE1UX1ZTWU5DX0hJR0gN
Cg0KDQpCZXN0IHJlZ2FyZHMsDQpKaW5nb28gSGFuDQoNCj4gKwkJZmJtb2RlLT5zeW5jIHw9IEZC
X1NZTkNfVkVSVF9ISUdIX0FDVDsNCj4gKwlpZiAodm0tPmRhdGFfZmxhZ3MgJiBESVNQTEFZX0ZM
QUdTX0lOVEVSTEFDRUQpDQo+ICsJCWZibW9kZS0+dm1vZGUgfD0gRkJfVk1PREVfSU5URVJMQUNF
RDsNCj4gKwlpZiAodm0tPmRhdGFfZmxhZ3MgJiBESVNQTEFZX0ZMQUdTX0RPVUJMRVNDQU4pDQo+
ICsJCWZibW9kZS0+dm1vZGUgfD0gRkJfVk1PREVfRE9VQkxFOw0KPiArCWZibW9kZS0+ZmxhZyA9
IDA7DQo+ICsNCj4gKwlodG90YWwgPSB2bS0+aGFjdGl2ZSArIHZtLT5oZnJvbnRfcG9yY2ggKyB2
bS0+aGJhY2tfcG9yY2ggKw0KPiArCQkgdm0tPmhzeW5jX2xlbjsNCj4gKwl2dG90YWwgPSB2bS0+
dmFjdGl2ZSArIHZtLT52ZnJvbnRfcG9yY2ggKyB2bS0+dmJhY2tfcG9yY2ggKw0KPiArCQkgdm0t
PnZzeW5jX2xlbjsNCj4gKwkvKiBwcmV2ZW50IGRpdmlzaW9uIGJ5IHplcm8gKi8NCj4gKwlpZiAo
aHRvdGFsICYmIHZ0b3RhbCkgew0KPiArCQlmYm1vZGUtPnJlZnJlc2ggPSB2bS0+cGl4ZWxjbG9j
ayAvIChodG90YWwgKiB2dG90YWwpOw0KPiArCS8qIGEgbW9kZSBtdXN0IGhhdmUgaHRvdGFsIGFu
ZCB2dG90YWwgIT0gMCBvciBpdCBpcyBpbnZhbGlkICovDQo+ICsJfSBlbHNlIHsNCj4gKwkJZmJt
b2RlLT5yZWZyZXNoID0gMDsNCj4gKwkJcmV0dXJuIC1FSU5WQUw7DQo+ICsJfQ0KPiArDQo+ICsJ
cmV0dXJuIDA7DQo+ICt9DQo+ICtFWFBPUlRfU1lNQk9MX0dQTChmYl92aWRlb21vZGVfZnJvbV92
aWRlb21vZGUpOw0KPiArI2VuZGlmDQo+ICsNCj4gICNlbHNlDQo+ICBpbnQgZmJfcGFyc2VfZWRp
ZCh1bnNpZ25lZCBjaGFyICplZGlkLCBzdHJ1Y3QgZmJfdmFyX3NjcmVlbmluZm8gKnZhcikNCj4g
IHsNCj4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvZmIuaCBiL2luY2x1ZGUvbGludXgvZmIu
aA0KPiBpbmRleCBjN2E5NTcxLi4xMDBhMTc2IDEwMDY0NA0KPiAtLS0gYS9pbmNsdWRlL2xpbnV4
L2ZiLmgNCj4gKysrIGIvaW5jbHVkZS9saW51eC9mYi5oDQo+IEBAIC0xOSw2ICsxOSw3IEBAIHN0
cnVjdCB2bV9hcmVhX3N0cnVjdDsNCj4gIHN0cnVjdCBmYl9pbmZvOw0KPiAgc3RydWN0IGRldmlj
ZTsNCj4gIHN0cnVjdCBmaWxlOw0KPiArc3RydWN0IHZpZGVvbW9kZTsNCj4gDQo+ICAvKiBEZWZp
bml0aW9ucyBiZWxvdyBhcmUgdXNlZCBpbiB0aGUgcGFyc2VkIG1vbml0b3Igc3BlY3MgKi8NCj4g
ICNkZWZpbmUgRkJfRFBNU19BQ1RJVkVfT0ZGCTENCj4gQEAgLTcxNCw2ICs3MTUsOSBAQCBleHRl
cm4gdm9pZCBmYl9kZXN0cm95X21vZGVkYihzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2RlZGIpOw0K
PiAgZXh0ZXJuIGludCBmYl9maW5kX21vZGVfY3Z0KHN0cnVjdCBmYl92aWRlb21vZGUgKm1vZGUs
IGludCBtYXJnaW5zLCBpbnQgcmIpOw0KPiAgZXh0ZXJuIHVuc2lnbmVkIGNoYXIgKmZiX2RkY19y
ZWFkKHN0cnVjdCBpMmNfYWRhcHRlciAqYWRhcHRlcik7DQo+IA0KPiArZXh0ZXJuIGludCBmYl92
aWRlb21vZGVfZnJvbV92aWRlb21vZGUoY29uc3Qgc3RydWN0IHZpZGVvbW9kZSAqdm0sDQo+ICsJ
CQkJICAgICAgIHN0cnVjdCBmYl92aWRlb21vZGUgKmZibW9kZSk7DQo+ICsNCj4gIC8qIGRyaXZl
cnMvdmlkZW8vbW9kZWRiLmMgKi8NCj4gICNkZWZpbmUgVkVTQV9NT0RFREJfU0laRSAzNA0KPiAg
ZXh0ZXJuIHZvaWQgZmJfdmFyX3RvX3ZpZGVvbW9kZShzdHJ1Y3QgZmJfdmlkZW9tb2RlICptb2Rl
LA0KPiAtLQ0KPiAxLjcuMTAuNA0KPiANCj4gX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX18NCj4gZHJpLWRldmVsIG1haWxpbmcgbGlzdA0KPiBkcmktZGV2ZWxA
bGlzdHMuZnJlZWRlc2t0b3Aub3JnDQo+IGh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp
bG1hbi9saXN0aW5mby9kcmktZGV2ZWwNCg=
^ permalink raw reply
* RE: [PATCH v17 4/7] fbmon: add videomode helpers
From: Jingoo Han @ 2013-02-01 9:29 UTC (permalink / raw)
To: 'Steffen Trumtrar'
Cc: 'Mohammed, Afzal', 'Florian Tobias Schandinat',
devicetree-discuss, 'Stephen Warren', linux-fbdev,
dri-devel, 'Tomi Valkeinen', 'Rob Herring',
'Laurent Pinchart', kernel,
'Guennady Liakhovetski', linux-media
In-Reply-To: <1359104515-8907-5-git-send-email-s.trumtrar@pengutronix.de>
On Friday, January 25, 2013 6:02 PM, Steffen Trumtrar wrote
>
> Add a function to convert from the generic videomode to a fb_videomode.
>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Afzal Mohammed <Afzal@ti.com>
> Tested-by: Rob Clark <robclark@gmail.com>
> Tested-by: Leela Krishna Amudala <leelakrishna.a@gmail.com>
> ---
> drivers/video/fbmon.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/fb.h | 4 ++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/video/fbmon.c b/drivers/video/fbmon.c
> index cef6557..17ce135 100644
> --- a/drivers/video/fbmon.c
> +++ b/drivers/video/fbmon.c
> @@ -31,6 +31,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <video/edid.h>
> +#include <video/videomode.h>
> #ifdef CONFIG_PPC_OF
> #include <asm/prom.h>
> #include <asm/pci-bridge.h>
> @@ -1373,6 +1374,57 @@ int fb_get_mode(int flags, u32 val, struct fb_var_screeninfo *var, struct fb_inf
> kfree(timings);
> return err;
> }
> +
> +#if IS_ENABLED(CONFIG_VIDEOMODE)
> +int fb_videomode_from_videomode(const struct videomode *vm,
> + struct fb_videomode *fbmode)
> +{
> + unsigned int htotal, vtotal;
> +
> + fbmode->xres = vm->hactive;
> + fbmode->left_margin = vm->hback_porch;
> + fbmode->right_margin = vm->hfront_porch;
> + fbmode->hsync_len = vm->hsync_len;
> +
> + fbmode->yres = vm->vactive;
> + fbmode->upper_margin = vm->vback_porch;
> + fbmode->lower_margin = vm->vfront_porch;
> + fbmode->vsync_len = vm->vsync_len;
> +
> + /* prevent division by zero in KHZ2PICOS macro */
> + fbmode->pixclock = vm->pixelclock ?
> + KHZ2PICOS(vm->pixelclock / 1000) : 0;
> +
> + fbmode->sync = 0;
> + fbmode->vmode = 0;
> + if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
> + fbmode->sync |= FB_SYNC_HOR_HIGH_ACT;
> + if (vm->dmt_flags & VESA_DMT_HSYNC_HIGH)
Hi Steffen Trumtrar,
Um, it seems to be a type. 'H'SYNC -> 'V'SYNC
Thus, it would be changed as below:
VESA_DMT_HSYNC_HIGH -> VESA_DMT_VSYNC_HIGH
Best regards,
Jingoo Han
> + fbmode->sync |= FB_SYNC_VERT_HIGH_ACT;
> + if (vm->data_flags & DISPLAY_FLAGS_INTERLACED)
> + fbmode->vmode |= FB_VMODE_INTERLACED;
> + if (vm->data_flags & DISPLAY_FLAGS_DOUBLESCAN)
> + fbmode->vmode |= FB_VMODE_DOUBLE;
> + fbmode->flag = 0;
> +
> + htotal = vm->hactive + vm->hfront_porch + vm->hback_porch +
> + vm->hsync_len;
> + vtotal = vm->vactive + vm->vfront_porch + vm->vback_porch +
> + vm->vsync_len;
> + /* prevent division by zero */
> + if (htotal && vtotal) {
> + fbmode->refresh = vm->pixelclock / (htotal * vtotal);
> + /* a mode must have htotal and vtotal != 0 or it is invalid */
> + } else {
> + fbmode->refresh = 0;
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(fb_videomode_from_videomode);
> +#endif
> +
> #else
> int fb_parse_edid(unsigned char *edid, struct fb_var_screeninfo *var)
> {
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index c7a9571..100a176 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -19,6 +19,7 @@ struct vm_area_struct;
> struct fb_info;
> struct device;
> struct file;
> +struct videomode;
>
> /* Definitions below are used in the parsed monitor specs */
> #define FB_DPMS_ACTIVE_OFF 1
> @@ -714,6 +715,9 @@ extern void fb_destroy_modedb(struct fb_videomode *modedb);
> extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int rb);
> extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
>
> +extern int fb_videomode_from_videomode(const struct videomode *vm,
> + struct fb_videomode *fbmode);
> +
> /* drivers/video/modedb.c */
> #define VESA_MODEDB_SIZE 34
> extern void fb_var_to_videomode(struct fb_videomode *mode,
> --
> 1.7.10.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v6 8/8] ARM: mmp: add display and fb support in pxa910 defconfig
From: Zhou Zhu @ 2013-02-01 9:21 UTC (permalink / raw)
To: linux-fbdev
added display and fb support in pxa910 defconfig.
added tpohvga panel, spi support.
added logo support.
Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
arch/arm/configs/pxa910_defconfig | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/configs/pxa910_defconfig b/arch/arm/configs/pxa910_defconfig
index 191118c..3bb7771 100644
--- a/arch/arm/configs/pxa910_defconfig
+++ b/arch/arm/configs/pxa910_defconfig
@@ -42,6 +42,14 @@ CONFIG_SMC91X=y
# CONFIG_SERIO is not set
CONFIG_SERIAL_PXA=y
CONFIG_SERIAL_PXA_CONSOLE=y
+CONFIG_SPI=y
+CONFIG_FB=y
+CONFIG_MMP_DISP=y
+CONFIG_MMP_DISP_CONTROLLER=y
+CONFIG_MMP_SPI=y
+CONFIG_MMP_PANEL_TPOHVGA=y
+CONFIG_MMP_FB=y
+CONFIG_LOGO=y
# CONFIG_LEGACY_PTYS is not set
# CONFIG_HW_RANDOM is not set
# CONFIG_HWMON is not set
--
1.7.9.5
^ permalink raw reply related
* [PATCH v6 7/8] ARM: mmp: enable display in ttc_dkb
From: Zhou Zhu @ 2013-02-01 9:20 UTC (permalink / raw)
To: linux-fbdev
enable display in ttc_dkb
Signed-off-by: Zhou Zhu <zzhu3@marvell.com>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
arch/arm/mach-mmp/ttc_dkb.c | 92 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/arch/arm/mach-mmp/ttc_dkb.c b/arch/arm/mach-mmp/ttc_dkb.c
index ce55fd8..49b3cf9 100644
--- a/arch/arm/mach-mmp/ttc_dkb.c
+++ b/arch/arm/mach-mmp/ttc_dkb.c
@@ -19,6 +19,8 @@
#include <linux/gpio.h>
#include <linux/mfd/88pm860x.h>
#include <linux/platform_data/mv_usb.h>
+#include <linux/spi/spi.h>
+#include <linux/delay.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -184,6 +186,92 @@ static struct pxa3xx_nand_platform_data dkb_nand_info = {
};
#endif
+#ifdef CONFIG_MMP_DISP
+/* path config */
+#define CFG_IOPADMODE(iopad) (iopad) /* 0x0 ~ 0xd */
+#define SCLK_SOURCE_SELECT(x) (x << 30) /* 0x0 ~ 0x3 */
+/* link config */
+#define CFG_DUMBMODE(mode) (mode << 28) /* 0x0 ~ 0x6*/
+#define CFG_GRA_SWAPRB(x) (x << 0) /* 1: rbswap enabled */
+static struct mmp_mach_path_config dkb_disp_config[] = {
+ [0] = {
+ .name = "mmp-parallel",
+ .overlay_num = 2,
+ .output_type = PATH_OUT_PARALLEL,
+ .path_config = CFG_IOPADMODE(0x1)
+ | SCLK_SOURCE_SELECT(0x1),
+ .link_config = CFG_DUMBMODE(0x2)
+ | CFG_GRA_SWAPRB(0x1),
+ },
+};
+
+static struct mmp_mach_plat_info dkb_disp_info = {
+ .name = "mmp-disp",
+ .clk_name = "disp0",
+ .path_num = 1,
+ .paths = dkb_disp_config,
+};
+
+static struct mmp_buffer_driver_mach_info dkb_fb_info = {
+ .name = "mmp-fb",
+ .path_name = "mmp-parallel",
+ .overlay_id = 0,
+ .dmafetch_id = 1,
+ .default_pixfmt = PIXFMT_RGB565,
+};
+
+static void dkb_tpo_panel_power(int on)
+{
+ int err;
+ u32 spi_reset = mfp_to_gpio(MFP_PIN_GPIO106);
+
+ if (on) {
+ err = gpio_request(spi_reset, "TPO_LCD_SPI_RESET");
+ if (err) {
+ pr_err("failed to request GPIO for TPO LCD RESET\n");
+ return;
+ }
+ gpio_direction_output(spi_reset, 0);
+ udelay(100);
+ gpio_set_value(spi_reset, 1);
+ gpio_free(spi_reset);
+ } else {
+ err = gpio_request(spi_reset, "TPO_LCD_SPI_RESET");
+ if (err) {
+ pr_err("failed to request LCD RESET gpio\n");
+ return;
+ }
+ gpio_set_value(spi_reset, 0);
+ gpio_free(spi_reset);
+ }
+}
+
+static struct mmp_mach_panel_info dkb_tpo_panel_info = {
+ .name = "tpo-hvga",
+ .plat_path_name = "mmp-parallel",
+ .plat_set_onoff = dkb_tpo_panel_power,
+};
+
+static struct spi_board_info spi_board_info[] __initdata = {
+ {
+ .modalias = "tpo-hvga",
+ .platform_data = &dkb_tpo_panel_info,
+ .bus_num = 5,
+ }
+};
+
+static void __init add_disp(void)
+{
+ pxa_register_device(&pxa910_device_disp,
+ &dkb_disp_info, sizeof(dkb_disp_info));
+ spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info));
+ pxa_register_device(&pxa910_device_fb,
+ &dkb_fb_info, sizeof(dkb_fb_info));
+ pxa_register_device(&pxa910_device_panel,
+ &dkb_tpo_panel_info, sizeof(dkb_tpo_panel_info));
+}
+#endif
+
static void __init ttc_dkb_init(void)
{
mfp_config(ARRAY_AND_SIZE(ttc_dkb_pin_config));
@@ -212,6 +300,10 @@ static void __init ttc_dkb_init(void)
pxa168_device_u2ootg.dev.platform_data = &ttc_usb_pdata;
platform_device_register(&pxa168_device_u2ootg);
#endif
+
+#ifdef CONFIG_MMP_DISP
+ add_disp();
+#endif
}
MACHINE_START(TTC_DKB, "PXA910-based TTC_DKB Development Platform")
--
1.7.9.5
^ permalink raw reply related
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