From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ijc@hellion.org.uk>
Date: Thu, 08 Jan 2015 09:04:17 +0000
Subject: [U-Boot] [PATCH 2/3] sunxi: video: Add support for Hitachi
 tx18d42vm LCD panels
In-Reply-To: <54AE4247.40003@redhat.com>
References: <1420401940-15136-1-git-send-email-hdegoede@redhat.com>
	<1420401940-15136-2-git-send-email-hdegoede@redhat.com>
	<1420705965.11796.46.camel@hellion.org.uk> <54AE4247.40003@redhat.com>
Message-ID: <1420707857.11796.63.camel@hellion.org.uk>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.