Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753334Ab3ERTvj (ORCPT ); Sat, 18 May 2013 15:51:39 -0400 Received: from server.prisktech.co.nz ([115.188.14.127]:50280 "EHLO server.prisktech.co.nz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752983Ab3ERTvi (ORCPT ); Sat, 18 May 2013 15:51:38 -0400 Message-ID: <5197DBCA.8050708@prisktech.co.nz> Date: Sun, 19 May 2013 07:51:38 +1200 From: Tony Prisk User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: Alexey Charkov CC: VT8500/WM8505 Linux Kernel , Florian Tobias Schandinat , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , tomi.valkeinen@ti.com, "linux-fbdev@vger.kernel.org" Subject: Re: [PATCH 4/4] fb: vt8500: Add VGA output support to wm8505fb driver. References: <1368868514-18975-1-git-send-email-linux@prisktech.co.nz> <1368868514-18975-5-git-send-email-linux@prisktech.co.nz> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5361 Lines: 122 On 19/05/13 01:28, Alexey Charkov wrote: > 2013/5/18 Tony Prisk : >> The APC8750 does not support an LCD panel, but provides a VGA connector. >> This patch adds support for the VGA interface, and defines an optional >> devicetree property to specify the output interface. The default if not >> specified is LCD for backward compatibility. >> >> Signed-off-by: Tony Prisk >> --- >> .../devicetree/bindings/video/wm,wm8505-fb.txt | 5 ++++ >> drivers/video/wm8505fb.c | 31 ++++++++++++++++++-- >> 2 files changed, 34 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt >> index 601416c..9f1d648 100644 >> --- a/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt >> +++ b/Documentation/devicetree/bindings/video/wm,wm8505-fb.txt >> @@ -7,6 +7,10 @@ Required properties: >> - bits-per-pixel : bit depth of framebuffer (16 or 32) >> - clocks : phandle to DVO clock >> >> +Optional properties: >> +- output-interface : the interface the fb should output on. Valid values are >> + "lcd" or "vga". If not specified, the default is "lcd". >> + >> Required subnodes: >> - display-timings: see display-timing.txt for information >> >> @@ -17,6 +21,7 @@ Example: >> reg = <0xd8051700 0x200>; >> bits-per-pixel = <16>; >> clocks = <&clkdvo>; >> + output-interface = "vga"; >> >> display-timings { >> native-mode = <&timing0>; >> diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c >> index f8bffc2..d1f7f33 100644 >> --- a/drivers/video/wm8505fb.c >> +++ b/drivers/video/wm8505fb.c >> @@ -130,12 +130,17 @@ >> >> #define to_wm8505fb_info(__info) container_of(__info, \ >> struct wm8505fb_info, fb) >> + >> +#define INTERFACE_LCD 1 >> +#define INTERFACE_VGA 2 >> + >> struct wm8505fb_info { >> struct fb_info fb; >> void __iomem *regbase; >> unsigned int contrast; >> struct device *dev; >> struct clk *clk_dvo; >> + int interface; >> }; >> >> >> @@ -158,7 +163,11 @@ static int wm8505fb_init_hw(struct fb_info *info) >> * 0x31C sets the correct color mode (RGB565) for WM8650 >> * Bit 8+9 (0x300) are ignored on WM8505 as reserved >> */ >> - writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB); >> + if (fbi->interface == INTERFACE_VGA) >> + writel(0x338, fbi->regbase + REG_GOVRH_YUVRGB); >> + else >> + writel(0x31c, fbi->regbase + REG_GOVRH_YUVRGB); >> + >> writel(1, fbi->regbase + REG_GOVRH_DVO_PIX); > Tony, > > Would it be possible to also define known bit offsets for those > registers, while you are at this? It would probably reduce the black > magic quite a bit :) On my list of things to do :) >> /* Virtual buffer size */ >> @@ -167,7 +176,12 @@ static int wm8505fb_init_hw(struct fb_info *info) >> >> /* black magic ;) */ >> writel(0xf, fbi->regbase + REG_GOVRH_FHI); >> - writel(4, fbi->regbase + REG_GOVRH_DVO_SET); >> + >> + if (fbi->interface == INTERFACE_VGA) >> + writel(0xe, fbi->regbase + REG_GOVRH_DVO_SET); >> + else >> + writel(4, fbi->regbase + REG_GOVRH_DVO_SET); > I don't remember if HDMI is yet another option for this register or > not... If it is, it would probably warrant defining fbi->interface as > an enum and changing this if-else into a switch statement to let the > compiler add its checks/warnings. This register defines the h/v syncpolarity and enable/disable for DVO. > >> writel(1, fbi->regbase + REG_GOVRH_MIF); >> writel(1, fbi->regbase + REG_GOVRH_REG_STS); >> >> @@ -194,11 +208,15 @@ static int wm8505fb_set_timing(struct fb_info *info) >> writel(h_end, fbi->regbase + REG_GOVRH_ACTPX_END); >> writel(h_all, fbi->regbase + REG_GOVRH_H_ALLPXL); >> writel(h_sync, fbi->regbase + REG_GOVRH_HDMI_HSYNW); >> + if (fbi->interface == INTERFACE_VGA) >> + writel(h_sync, fbi->regbase + REG_GOVRH_VGA_HSYNW); > Will it misbehave on LCD if you write to the VGA register unconditionally? Don't know - wouldn't imagine so. I will test it and see. > >> writel(v_start, fbi->regbase + REG_GOVRH_ACTLN_BG); >> writel(v_end, fbi->regbase + REG_GOVRH_ACTLN_END); >> writel(v_all, fbi->regbase + REG_GOVRH_V_ALLLN); >> writel(v_sync, fbi->regbase + REG_GOVRH_HDMI_VBISW); >> + if (fbi->interface == INTERFACE_VGA) >> + writel(info->var.pixclock, fbi->regbase + REG_GOVRH_VGA_VSYNW); > Same here. I would assume that setting the pixclock should not hurt > LCD, which would then simplify the code a little. > > Thanks, > Alexey Regards Tony Prisk -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/