| Message ID | 1420401940-15136-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Campbell |
| Headers | show |
On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: > Hitachi tx18d42vm LCD panels have an onboard controller which needs some > initialization via spi for the panel to become functional as a regular LVDS > panel. This doesn't sound terribly sunxi specific, shouldn't sunxi_lcd_panel_hitachi_tx18d42vm_init be generic but making use of some sort of arch-provided interface for actually talking SPI to the device (which may or may not use the generic SPI infra, as your code comment talked about)? Ian.
Hi, On 07-01-15 19:59, Ian Campbell wrote: > On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: >> Hitachi tx18d42vm LCD panels have an onboard controller which needs some >> initialization via spi for the panel to become functional as a regular LVDS >> panel. > > This doesn't sound terribly sunxi specific, shouldn't > sunxi_lcd_panel_hitachi_tx18d42vm_init be generic but making use of some > sort of arch-provided interface for actually talking SPI to the device > (which may or may not use the generic SPI infra, as your code comment > talked about)? You're right it is not sunxi specific, but for now sunxi is the only user (I've checked) so this seemed simplest, with the idea that it is always much easier to make something properly generic when there is more then one user. I guess I should have put something about this in the commit message :) Regards, Hans
On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: > Hitachi tx18d42vm LCD panels have an onboard controller which needs some > initialization via spi for the panel to become functional as a regular LVDS > panel. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > board/sunxi/Kconfig | 3 ++ > drivers/video/Makefile | 2 +- > drivers/video/sunxi_display.c | 9 ++++++ > drivers/video/sunxi_lcd_panel.c | 68 +++++++++++++++++++++++++++++++++++++++++ > drivers/video/sunxi_lcd_panel.h | 9 ++++++ > 5 files changed, 90 insertions(+), 1 deletion(-) > create mode 100644 drivers/video/sunxi_lcd_panel.c > create mode 100644 drivers/video/sunxi_lcd_panel.h > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > index fdb18a4..e9f62b9 100644 > --- a/board/sunxi/Kconfig > +++ b/board/sunxi/Kconfig > @@ -357,6 +357,9 @@ config VIDEO_LCD_PANEL_PARALLEL > config VIDEO_LCD_PANEL_LVDS > bool "Generic lvds interface LCD panel" > > +config VIDEO_LCD_PANEL_HITACHI_TX18D42VM > + bool "Hitachi tx18d42vm LCD panel" > + > endchoice > > config USB_KEYBOARD > diff --git a/drivers/video/Makefile b/drivers/video/Makefile > index 42b1eaa..d4fe1aa 100644 > --- a/drivers/video/Makefile > +++ b/drivers/video/Makefile > @@ -39,7 +39,7 @@ obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o > obj-$(CONFIG_VIDEO_SED13806) += sed13806.o > obj-$(CONFIG_VIDEO_SM501) += sm501.o > obj-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o videomodes.o > -obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o videomodes.o > +obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o sunxi_lcd_panel.o videomodes.o > obj-$(CONFIG_VIDEO_TEGRA) += tegra.o > obj-$(CONFIG_VIDEO_VCXK) += bus_vcxk.o > obj-$(CONFIG_VIDEO_X86) += x86_fb.o > diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c > index 4b63b01..f087c2c 100644 > --- a/drivers/video/sunxi_display.c > +++ b/drivers/video/sunxi_display.c > @@ -19,8 +19,13 @@ > #include <fdtdec.h> > #include <fdt_support.h> > #include <video_fb.h> > +#include "sunxi_lcd_panel.h" > #include "videomodes.h" > > +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM > +#define CONFIG_VIDEO_LCD_PANEL_LVDS > +#endif Your reply to 1/3 made me notice this, which I somehow missed before, I don't think this is the right way to go about this. It seems to me that LCD_PANEL_HITACHI does not belong under the PARALLEL vs LVDS Kconfig choice, rather it should be an independent bool option which depends on PANEL_LVDS. Ian.
On Wed, 2015-01-07 at 21:04 +0100, Hans de Goede wrote: > Hi, > > On 07-01-15 19:59, Ian Campbell wrote: > > On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: > >> Hitachi tx18d42vm LCD panels have an onboard controller which needs some > >> initialization via spi for the panel to become functional as a regular LVDS > >> panel. > > > > This doesn't sound terribly sunxi specific, shouldn't > > sunxi_lcd_panel_hitachi_tx18d42vm_init be generic but making use of some > > sort of arch-provided interface for actually talking SPI to the device > > (which may or may not use the generic SPI infra, as your code comment > > talked about)? > > You're right it is not sunxi specific, but for now sunxi is the only user > (I've checked) so this seemed simplest, with the idea that it is always > much easier to make something properly generic when there is more then one > user. I think we are more likely to simply end up with N versions of this code because no one notices that it already exists under some other name. If Anatolij is OK with it then I have no objections. > > I guess I should have put something about this in the commit message :) > > Regards, > > Hans >
Hi, On 08-01-15 09:32, Ian Campbell wrote: > On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: >> Hitachi tx18d42vm LCD panels have an onboard controller which needs some >> initialization via spi for the panel to become functional as a regular LVDS >> panel. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> board/sunxi/Kconfig | 3 ++ >> drivers/video/Makefile | 2 +- >> drivers/video/sunxi_display.c | 9 ++++++ >> drivers/video/sunxi_lcd_panel.c | 68 +++++++++++++++++++++++++++++++++++++++++ >> drivers/video/sunxi_lcd_panel.h | 9 ++++++ >> 5 files changed, 90 insertions(+), 1 deletion(-) >> create mode 100644 drivers/video/sunxi_lcd_panel.c >> create mode 100644 drivers/video/sunxi_lcd_panel.h >> >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig >> index fdb18a4..e9f62b9 100644 >> --- a/board/sunxi/Kconfig >> +++ b/board/sunxi/Kconfig >> @@ -357,6 +357,9 @@ config VIDEO_LCD_PANEL_PARALLEL >> config VIDEO_LCD_PANEL_LVDS >> bool "Generic lvds interface LCD panel" >> >> +config VIDEO_LCD_PANEL_HITACHI_TX18D42VM >> + bool "Hitachi tx18d42vm LCD panel" >> + >> endchoice >> >> config USB_KEYBOARD >> diff --git a/drivers/video/Makefile b/drivers/video/Makefile >> index 42b1eaa..d4fe1aa 100644 >> --- a/drivers/video/Makefile >> +++ b/drivers/video/Makefile >> @@ -39,7 +39,7 @@ obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o >> obj-$(CONFIG_VIDEO_SED13806) += sed13806.o >> obj-$(CONFIG_VIDEO_SM501) += sm501.o >> obj-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o videomodes.o >> -obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o videomodes.o >> +obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o sunxi_lcd_panel.o videomodes.o >> obj-$(CONFIG_VIDEO_TEGRA) += tegra.o >> obj-$(CONFIG_VIDEO_VCXK) += bus_vcxk.o >> obj-$(CONFIG_VIDEO_X86) += x86_fb.o >> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c >> index 4b63b01..f087c2c 100644 >> --- a/drivers/video/sunxi_display.c >> +++ b/drivers/video/sunxi_display.c >> @@ -19,8 +19,13 @@ >> #include <fdtdec.h> >> #include <fdt_support.h> >> #include <video_fb.h> >> +#include "sunxi_lcd_panel.h" >> #include "videomodes.h" >> >> +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM >> +#define CONFIG_VIDEO_LCD_PANEL_LVDS >> +#endif > > Your reply to 1/3 made me notice this, which I somehow missed before, I > don't think this is the right way to go about this. > > It seems to me that LCD_PANEL_HITACHI does not belong under the PARALLEL > vs LVDS Kconfig choice, rather it should be an independent bool option > which depends on PANEL_LVDS. Technically you are correct, but from a user pov it seems easier to tell the user to select which panel he has, and then the user can chose between a bunch of generic ones (generic parallel, generic lvds) and a bunch of specific panels which need some extra setup. If you insist I can live with doing things your way though, either way let me know how you want to handle this. Regards, Hans
On Thu, 2015-01-08 at 09:39 +0100, Hans de Goede wrote: > Hi, > > On 08-01-15 09:32, Ian Campbell wrote: > > On Sun, 2015-01-04 at 21:05 +0100, Hans de Goede wrote: > >> Hitachi tx18d42vm LCD panels have an onboard controller which needs some > >> initialization via spi for the panel to become functional as a regular LVDS > >> panel. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> board/sunxi/Kconfig | 3 ++ > >> drivers/video/Makefile | 2 +- > >> drivers/video/sunxi_display.c | 9 ++++++ > >> drivers/video/sunxi_lcd_panel.c | 68 +++++++++++++++++++++++++++++++++++++++++ > >> drivers/video/sunxi_lcd_panel.h | 9 ++++++ > >> 5 files changed, 90 insertions(+), 1 deletion(-) > >> create mode 100644 drivers/video/sunxi_lcd_panel.c > >> create mode 100644 drivers/video/sunxi_lcd_panel.h > >> > >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > >> index fdb18a4..e9f62b9 100644 > >> --- a/board/sunxi/Kconfig > >> +++ b/board/sunxi/Kconfig > >> @@ -357,6 +357,9 @@ config VIDEO_LCD_PANEL_PARALLEL > >> config VIDEO_LCD_PANEL_LVDS > >> bool "Generic lvds interface LCD panel" > >> > >> +config VIDEO_LCD_PANEL_HITACHI_TX18D42VM > >> + bool "Hitachi tx18d42vm LCD panel" > >> + > >> endchoice > >> > >> config USB_KEYBOARD > >> diff --git a/drivers/video/Makefile b/drivers/video/Makefile > >> index 42b1eaa..d4fe1aa 100644 > >> --- a/drivers/video/Makefile > >> +++ b/drivers/video/Makefile > >> @@ -39,7 +39,7 @@ obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o > >> obj-$(CONFIG_VIDEO_SED13806) += sed13806.o > >> obj-$(CONFIG_VIDEO_SM501) += sm501.o > >> obj-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o videomodes.o > >> -obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o videomodes.o > >> +obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o sunxi_lcd_panel.o videomodes.o > >> obj-$(CONFIG_VIDEO_TEGRA) += tegra.o > >> obj-$(CONFIG_VIDEO_VCXK) += bus_vcxk.o > >> obj-$(CONFIG_VIDEO_X86) += x86_fb.o > >> diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c > >> index 4b63b01..f087c2c 100644 > >> --- a/drivers/video/sunxi_display.c > >> +++ b/drivers/video/sunxi_display.c > >> @@ -19,8 +19,13 @@ > >> #include <fdtdec.h> > >> #include <fdt_support.h> > >> #include <video_fb.h> > >> +#include "sunxi_lcd_panel.h" > >> #include "videomodes.h" > >> > >> +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM > >> +#define CONFIG_VIDEO_LCD_PANEL_LVDS > >> +#endif > > > > Your reply to 1/3 made me notice this, which I somehow missed before, I > > don't think this is the right way to go about this. > > > > It seems to me that LCD_PANEL_HITACHI does not belong under the PARALLEL > > vs LVDS Kconfig choice, rather it should be an independent bool option > > which depends on PANEL_LVDS. > > Technically you are correct, but from a user pov it seems easier to tell > the user to select which panel he has, and then the user can chose between a > bunch of generic ones (generic parallel, generic lvds) and a bunch of specific > panels which need some extra setup. Hrm, true. > If you insist I can live with doing things your way though, either way let > me know how you want to handle this. Perhaps have hidden Kconfig options for the baseline LVDS vs PARALLEL support and then a choice of GENERIC //, GENERIC LVDS, HITACHI TX.., OTHER MAGIC PANEL with appropriate selects on the hidden options? Then the generic code would use the hidden options, no need for a weird #ifdef/#define pair for each non-standard panel (which depending on how many panels are normal vs. needing magic could get unwieldy). Ian.
diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig index fdb18a4..e9f62b9 100644 --- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -357,6 +357,9 @@ config VIDEO_LCD_PANEL_PARALLEL config VIDEO_LCD_PANEL_LVDS bool "Generic lvds interface LCD panel" +config VIDEO_LCD_PANEL_HITACHI_TX18D42VM + bool "Hitachi tx18d42vm LCD panel" + endchoice config USB_KEYBOARD diff --git a/drivers/video/Makefile b/drivers/video/Makefile index 42b1eaa..d4fe1aa 100644 --- a/drivers/video/Makefile +++ b/drivers/video/Makefile @@ -39,7 +39,7 @@ obj-$(CONFIG_VIDEO_SANDBOX_SDL) += sandbox_sdl.o obj-$(CONFIG_VIDEO_SED13806) += sed13806.o obj-$(CONFIG_VIDEO_SM501) += sm501.o obj-$(CONFIG_VIDEO_SMI_LYNXEM) += smiLynxEM.o videomodes.o -obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o videomodes.o +obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o sunxi_lcd_panel.o videomodes.o obj-$(CONFIG_VIDEO_TEGRA) += tegra.o obj-$(CONFIG_VIDEO_VCXK) += bus_vcxk.o obj-$(CONFIG_VIDEO_X86) += x86_fb.o diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c index 4b63b01..f087c2c 100644 --- a/drivers/video/sunxi_display.c +++ b/drivers/video/sunxi_display.c @@ -19,8 +19,13 @@ #include <fdtdec.h> #include <fdt_support.h> #include <video_fb.h> +#include "sunxi_lcd_panel.h" #include "videomodes.h" +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM +#define CONFIG_VIDEO_LCD_PANEL_LVDS +#endif + DECLARE_GLOBAL_DATA_PTR; enum sunxi_monitor { @@ -486,6 +491,10 @@ static void sunxi_lcdc_panel_enable(void) gpio_request(pin, "lcd_power"); gpio_direction_output(pin, 1); } + +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM + sunxi_lcd_panel_hitachi_tx18d42vm_init(); +#endif } static void sunxi_lcdc_backlight_enable(void) diff --git a/drivers/video/sunxi_lcd_panel.c b/drivers/video/sunxi_lcd_panel.c new file mode 100644 index 0000000..9ebaff2 --- /dev/null +++ b/drivers/video/sunxi_lcd_panel.c @@ -0,0 +1,68 @@ +/* + * LCD panel driver for Allwinner SoCs. + * + * (C) Copyright 2015 Hans de Goede <hdegoede@redhat.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> + +#include <asm/arch/gpio.h> +#include <asm/gpio.h> +#include <asm/io.h> + +#ifdef CONFIG_VIDEO_LCD_PANEL_HITACHI_TX18D42VM + +#define SPI_CS SUNXI_GPA(0) +#define SPI_CLK SUNXI_GPA(1) +#define SPI_MOSI SUNXI_GPA(2) + +/* + * Very simple write only SPI support, this does not use the generic SPI infra + * because that assumes R/W SPI, requiring a MISO pin. Also the necessary glue + * code alone would be larger then this minimal version. + */ + +static void sunxi_lcd_panel_spi_write(unsigned int data, int bits) +{ + int i, offset; + + gpio_direction_output(SPI_CS, 0); + for (i = 0; i < bits; i++) { + gpio_direction_output(SPI_CLK, 0); + offset = (bits - 1) - i; + gpio_direction_output(SPI_MOSI, (data >> offset) & 1); + udelay(2); + gpio_direction_output(SPI_CLK, 1); + udelay(2); + } + gpio_direction_output(SPI_CS, 1); + udelay(2); +} + +void sunxi_lcd_panel_hitachi_tx18d42vm_init(void) +{ + const u16 init_data[] = { + 0x0029, /* reset */ + 0x0025, /* standby */ + 0x0840, /* enable normally black */ + 0x0430, /* enable FRC/dither */ + 0x385f, /* enter test mode(1) */ + 0x3ca4, /* enter test mode(2) */ + 0x3409, /* enable SDRRS, enlarge OE width */ + 0x4041, /* adopt 2 line / 1 dot */ + }; + int i; + + mdelay(50); /* Wait for lcd controller power on */ + + for (i = 0; i < ARRAY_SIZE(init_data); i++) + sunxi_lcd_panel_spi_write(init_data[i], 16); + + mdelay(50); /* All the tx18d42vm drivers have a delay here ? */ + + sunxi_lcd_panel_spi_write(0x00ad, 16); /* display on */ +} + +#endif diff --git a/drivers/video/sunxi_lcd_panel.h b/drivers/video/sunxi_lcd_panel.h new file mode 100644 index 0000000..1fb9f1e --- /dev/null +++ b/drivers/video/sunxi_lcd_panel.h @@ -0,0 +1,9 @@ +/* + * LCD panel driver for Allwinner SoCs. + * + * (C) Copyright 2015 Hans de Goede <hdegoede@redhat.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +void sunxi_lcd_panel_hitachi_tx18d42vm_init(void);
Hitachi tx18d42vm LCD panels have an onboard controller which needs some initialization via spi for the panel to become functional as a regular LVDS panel. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- board/sunxi/Kconfig | 3 ++ drivers/video/Makefile | 2 +- drivers/video/sunxi_display.c | 9 ++++++ drivers/video/sunxi_lcd_panel.c | 68 +++++++++++++++++++++++++++++++++++++++++ drivers/video/sunxi_lcd_panel.h | 9 ++++++ 5 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 drivers/video/sunxi_lcd_panel.c create mode 100644 drivers/video/sunxi_lcd_panel.h