Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Changing which device has VTs?
From: Andy Lutomirski @ 2013-08-14 18:47 UTC (permalink / raw)
  To: linux-fbdev

I have a box with a real graphics card (radeon) and an onboard
graphics card (mgag200).  I want to use the radeon and ignore the
mgag200.  For reasons of extreme firmware suckage, I have two choices:

1. Leave both devices enabled in BIOS and set the mgag200 device
primary.  This causes efifb and later mgag200 to both point at the
mgag200.  All of my text consoles end up on the mgag200.  This is
unfortunate, especially because if, even if I set xorg to ignore the
mgag200 device, hitting ctrl-alt-f2 causes X to suspend itself and my
text console to show up on a device with no monitor attached.

2. Set the radeon device as primary in BIOS.  System doesn't boot.

I'd like to leave mgag200 primary but tell Linux to put all the text
consoles on the radeon device once it shows up.  I could compile out
both efifb and mgag200, but that seems like a waste.  Is there any way
to do this with a command-line parameter?  I tried fbcon=map:1, and I
get no fbcon at all.

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: Changing which device has VTs?
From: Dave Airlie @ 2013-08-15  5:49 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CALCETrX5_CAREEPSWkwE6+bY+w+c_qJcYBVM7Nx16_URZ9Gn5A@mail.gmail.com>

(resend, gmail went html again).

On Thu, Aug 15, 2013 at 4:47 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> I have a box with a real graphics card (radeon) and an onboard
> graphics card (mgag200).  I want to use the radeon and ignore the
> mgag200.  For reasons of extreme firmware suckage, I have two choices:
>
> 1. Leave both devices enabled in BIOS and set the mgag200 device
> primary.  This causes efifb and later mgag200 to both point at the
> mgag200.  All of my text consoles end up on the mgag200.  This is
> unfortunate, especially because if, even if I set xorg to ignore the
> mgag200 device, hitting ctrl-alt-f2 causes X to suspend itself and my
> text console to show up on a device with no monitor attached.
>
> 2. Set the radeon device as primary in BIOS.  System doesn't boot.

get vendor to fix BIOS :-)

>
> I'd like to leave mgag200 primary but tell Linux to put all the text
> consoles on the radeon device once it shows up.  I could compile out
> both efifb and mgag200, but that seems like a waste.  Is there any way
> to do this with a command-line parameter?  I tried fbcon=map:1, and I
> get no fbcon at all.

the mapping stuff should work, alternative is to write some code to
call the fbcon
event, FB_EVENT_REMAP_ALL_CONSOLE, which is what the GPU switcher
code uses to remap fbcon between switchable GPU systems.

Dave.

^ permalink raw reply

* Re: Changing which device has VTs?
From: Andy Lutomirski @ 2013-08-15  6:02 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CALCETrX5_CAREEPSWkwE6+bY+w+c_qJcYBVM7Nx16_URZ9Gn5A@mail.gmail.com>

On Wed, Aug 14, 2013 at 10:49 PM, Dave Airlie <airlied@gmail.com> wrote:
> (resend, gmail went html again).

And gmail broke pgdn, too :(

>
> On Thu, Aug 15, 2013 at 4:47 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> I have a box with a real graphics card (radeon) and an onboard
>> graphics card (mgag200).  I want to use the radeon and ignore the
>> mgag200.  For reasons of extreme firmware suckage, I have two choices:
>>
>> 1. Leave both devices enabled in BIOS and set the mgag200 device
>> primary.  This causes efifb and later mgag200 to both point at the
>> mgag200.  All of my text consoles end up on the mgag200.  This is
>> unfortunate, especially because if, even if I set xorg to ignore the
>> mgag200 device, hitting ctrl-alt-f2 causes X to suspend itself and my
>> text console to show up on a device with no monitor attached.
>>
>> 2. Set the radeon device as primary in BIOS.  System doesn't boot.
>
> get vendor to fix BIOS :-)

Done :)  (I'm now running some mysterious unreleased BIOS -- it's
vastly superior.)

>
>>
>> I'd like to leave mgag200 primary but tell Linux to put all the text
>> consoles on the radeon device once it shows up.  I could compile out
>> both efifb and mgag200, but that seems like a waste.  Is there any way
>> to do this with a command-line parameter?  I tried fbcon=map:1, and I
>> get no fbcon at all.
>
> the mapping stuff should work, alternative is to write some code to
> call the fbcon
> event, FB_EVENT_REMAP_ALL_CONSOLE, which is what the GPU switcher
> code uses to remap fbcon between switchable GPU systems.

Blech.  Fortunately, my new BIOS works and I can stop worrying.
(Plymouth is still broken -- separate bug filed.)

--Andy

>
> Dave.



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* [PATCH v3 0/5] ARM: vf610: Add DCU framebuffer driver for Vybrid VF610 platform
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel

This series contain DCU framebuffer driver for Freescale Vybrid VF610 platform.

The Display Controller Unit (DCU) module is a system master that
fetches graphics stored in internal or external memory and displays
them on a TFT LCD panel. A wide range of panel sizes is supported
and the timing of the interface signals is highly configurable.
Graphics are read directly from memory and then blended in real-time,
which allows for dynamic content creation with minimal CPU intervention.

The features:
(1) Full RGB888 output to TFT LCD panel.
(2) For the current LCD panel, WQVGA "480x272" is tested.
(3) Blending of each pixel using up to 4 source layers dependent on size of panel.
(4) Each graphic layer can be placed with one pixel resolution in either axis.
(5) Each graphic layer support RGB565 and RGB888 direct colors without alpha channel
and BGRA8888 direct colors with an alpha channel.
(6) Each graphic layer support alpha blending with 8-bit resolution.

Changes in v3:
- Correct DCU_MODE_BLEND_ITER macro definition.
- Remove hardcode panel description in the driver. Use the videomode helpers to get the relevant data from devicetree.
- Correct the wrong indentation.
- Use dev_* for printing messages in drivers.
- Change calc_div_ratio() to fsl_dcu_calc_div(), and rewrite this function.
- Use devm_request_irq() instead of request_irq().
- Drop useless code.
- Increase the layers number to the maximum 6.
- Use dma_alloc_writecombine() instead of dma_alloc_coherent().
- Use runtime PM.

Changes in v2:
- Add a document for DCU framebuffer driver under Documentation/devicetree/bindings/fb/.

----------------------------------------------------------------
Alison Wang (5):
      ARM: dts: vf610: Add DCU and TCON nodes
      ARM: dts: vf610-twr: Enable DCU and TCON devices
      ARM: clk: vf610: Add DCU and TCON clock support
      fb: Add DCU framebuffer driver for Vybrid VF610 platform
      Documentation: DT: Add DCU framebuffer driver

 Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt |   67 +++++
 arch/arm/boot/dts/vf610-twr.dts                     |   32 +++
 arch/arm/boot/dts/vf610.dtsi                        |   19 +-
 arch/arm/mach-imx/clk-vf610.c                       |    5 +
 drivers/video/Kconfig                               |   10 +
 drivers/video/Makefile                              |    1 +
 drivers/video/fsl-dcu-fb.c                          | 1095 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/vf610-clock.h             |    3 +-
 8 files changed, 1230 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt
 create mode 100644 drivers/video/fsl-dcu-fb.c



^ permalink raw reply

* [PATCH v3 1/5] ARM: dts: vf610: Add DCU and TCON nodes
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>

This patch adds DCU and TCON nodes in SoC level DTS for Freescale Vybrid VF610.
It also removes useless pin for DCU0 pinctrl.

Signed-off-by: Alison Wang <b18965@freescale.com>
---
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/vf610.dtsi | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 67d929c..145e1f4 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -140,6 +140,14 @@
 				clock-names = "pit";
 			};
 
+			tcon0: tcon@4003d000 {
+				compatible = "fsl,vf610-tcon";
+				reg = <0x4003d000 0x1000>;
+				clocks = <&clks VF610_CLK_TCON0>;
+				clock-names = "tcon";
+				status = "disabled";
+			};
+
 			wdog@4003e000 {
 				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
@@ -169,7 +177,6 @@
 				dcu0 {
 					pinctrl_dcu0_1: dcu0grp_1 {
 						fsl,pins = <
-						VF610_PAD_PTB8__GPIO_30		0x42
 						VF610_PAD_PTE0__DCU0_HSYNC	0x42
 						VF610_PAD_PTE1__DCU0_VSYNC	0x42
 						VF610_PAD_PTE2__DCU0_PCLK	0x42
@@ -395,6 +402,16 @@
 				reg = <0x40050000 0x1000>;
 			};
 
+			dcu0: dcu@40058000 {
+				compatible = "fsl,vf610-dcu";
+				reg = <0x40058000 0x1200>;
+				interrupts = <0 30 0x04>;
+				clocks = <&clks VF610_CLK_DCU0>;
+				clock-names = "dcu";
+				tcon-controller = <&tcon0>;
+				status = "disabled";
+			};
+
 			i2c0: i2c@40066000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-- 
1.8.0



^ permalink raw reply related

* [PATCH v3 2/5] ARM: dts: vf610-twr: Enable DCU and TCON devices
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>

This patch enables DCU and TCON devices for Vybrid VF610 TOWER board.

Signed-off-by: Alison Wang <b18965@freescale.com>
---
Changes in v3: Add panel description in devicetree.
Changes in v2: None

 arch/arm/boot/dts/vf610-twr.dts | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index b3905f5..82c03fe 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -36,6 +36,34 @@
 
 };
 
+&dcu0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_dcu0_1>;
+	display = <&display>;
+	status = "okay";
+
+	display: display@0 {
+		bits-per-pixel = <24>;
+
+		display-timings {
+			native-mode = <&timing0>;
+			timing0: nl4827hc19 {
+				clock-frequency = <10870000>;
+				hactive = <480>;
+				vactive = <272>;
+				hback-porch = <2>;
+				hfront-porch = <2>;
+				vback-porch = <1>;
+				vfront-porch = <1>;
+				hsync-len = <41>;
+				vsync-len = <2>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+			};
+		};
+	};
+};
+
 &fec0 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
@@ -50,6 +78,10 @@
 	status = "okay";
 };
 
+&tcon0 {
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_1>;
-- 
1.8.0



^ permalink raw reply related

* [PATCH v3 3/5] ARM: clk: vf610: Add DCU and TCON clock support
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>

This patch adds DCU and TCON clock support for Vybrid VF610 platform.

Signed-off-by: Alison Wang <b18965@freescale.com>
---
Changes in v3: None
Changes in v2: None

 arch/arm/mach-imx/clk-vf610.c           | 5 +++++
 include/dt-bindings/clock/vf610-clock.h | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/clk-vf610.c b/arch/arm/mach-imx/clk-vf610.c
index b169a39..689d3da 100644
--- a/arch/arm/mach-imx/clk-vf610.c
+++ b/arch/arm/mach-imx/clk-vf610.c
@@ -247,6 +247,8 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 	clk[VF610_CLK_DCU1_DIV] = imx_clk_divider("dcu1_div", "dcu1_en", CCM_CSCDR3, 20, 3);
 	clk[VF610_CLK_DCU1] = imx_clk_gate2("dcu1", "dcu1_div", CCM_CCGR9, CCM_CCGRx_CGn(8));
 
+	clk[VF610_CLK_TCON0] = imx_clk_gate2("tcon0", "platform_bus", CCM_CCGR1, CCM_CCGRx_CGn(13));
+
 	clk[VF610_CLK_ESAI_SEL] = imx_clk_mux("esai_sel", CCM_CSCMR1, 20, 2, esai_sels, 4);
 	clk[VF610_CLK_ESAI_EN] = imx_clk_gate("esai_en", "esai_sel", CCM_CSCDR2, 30);
 	clk[VF610_CLK_ESAI_DIV] = imx_clk_divider("esai_div", "esai_en", CCM_CSCDR2, 24, 4);
@@ -313,6 +315,9 @@ static void __init vf610_clocks_init(struct device_node *ccm_node)
 	clk_set_parent(clk[VF610_CLK_SAI2_SEL], clk[VF610_CLK_AUDIO_EXT]);
 	clk_set_parent(clk[VF610_CLK_SAI3_SEL], clk[VF610_CLK_AUDIO_EXT]);
 
+	clk_set_parent(clk[VF610_CLK_DCU0_SEL], clk[VF610_CLK_PLL1_PFD2]);
+	clk_set_rate(clk[VF610_CLK_DCU0_DIV], 113200000);
+
 	/* Add the clocks to provider list */
 	clk_data.clks = clk;
 	clk_data.clk_num = ARRAY_SIZE(clk);
diff --git a/include/dt-bindings/clock/vf610-clock.h b/include/dt-bindings/clock/vf610-clock.h
index 4aa2b48..4ccf563 100644
--- a/include/dt-bindings/clock/vf610-clock.h
+++ b/include/dt-bindings/clock/vf610-clock.h
@@ -160,6 +160,7 @@
 #define VF610_CLK_GPU2D			147
 #define VF610_CLK_ENET0			148
 #define VF610_CLK_ENET1			149
-#define VF610_CLK_END			150
+#define VF610_CLK_TCON0			150
+#define VF610_CLK_END			151
 
 #endif /* __DT_BINDINGS_CLOCK_VF610_H */
-- 
1.8.0



^ permalink raw reply related

* [PATCH v3 4/5] fb: Add DCU framebuffer driver for Vybrid VF610 platform
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>

The Display Controller Unit (DCU) module is a system master that
fetches graphics stored in internal or external memory and displays
them on a TFT LCD panel. A wide range of panel sizes is supported
and the timing of the interface signals is highly configurable.
Graphics are read directly from memory and then blended in real-time,
which allows for dynamic content creation with minimal CPU intervention.

The features:

(1) Full RGB888 output to TFT LCD panel.
(2) For the current LCD panel, WQVGA "480x272" is supported.
(3) Blending of each pixel using up to 4 source layers dependent on size of panel.
(4) Each graphic layer can be placed with one pixel resolution in either axis.
(5) Each graphic layer support RGB565 and RGB888 direct colors without alpha channel
and BGRA8888 direct colors with an alpha channel.
(6) Each graphic layer support alpha blending with 8-bit resolution.

This driver has been tested on Vybrid VF610TWR board.

Signed-off-by: Alison Wang <b18965@freescale.com>
---
Changes in v3:
- Correct DCU_MODE_BLEND_ITER macro definition.
- Remove hardcode panel description in the driver. Use the videomode helpers to get the relevant data from devicetree.
- Correct the wrong indentation.
- Use dev_* for printing messages in drivers.
- Change calc_div_ratio() to fsl_dcu_calc_div(), and rewrite this function.
- Use devm_request_irq() instead of request_irq().
- Drop useless code.
- Increase the layers number to the maximum 6.
- Use dma_alloc_writecombine() instead of dma_alloc_coherent().
- Use runtime PM.
Changes in v2: None

 drivers/video/Kconfig      |   10 +
 drivers/video/Makefile     |    1 +
 drivers/video/fsl-dcu-fb.c | 1095 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1106 insertions(+)
 create mode 100644 drivers/video/fsl-dcu-fb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index c4d5b44..59a6b86 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1978,6 +1978,16 @@ config FB_FSL_DIU
 	---help---
 	  Framebuffer driver for the Freescale SoC DIU
 
+config FB_FSL_DCU
+	tristate "Freescale DCU framebuffer support"
+	depends on FB
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS
+	---help---
+	  Framebuffer driver for the Freescale SoC DCU
+
 config FB_W100
 	tristate "W100 frame buffer support"
 	depends on FB && ARCH_PXA
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e8bae8d..3707a7d 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -129,6 +129,7 @@ obj-$(CONFIG_FB_IMX)              += imxfb.o
 obj-$(CONFIG_FB_S3C)		  += s3c-fb.o
 obj-$(CONFIG_FB_S3C2410)	  += s3c2410fb.o
 obj-$(CONFIG_FB_FSL_DIU)	  += fsl-diu-fb.o
+obj-$(CONFIG_FB_FSL_DCU)	  += fsl-dcu-fb.o
 obj-$(CONFIG_FB_COBALT)           += cobalt_lcdfb.o
 obj-$(CONFIG_FB_IBM_GXT4500)	  += gxt4500.o
 obj-$(CONFIG_FB_PS3)		  += ps3fb.o
diff --git a/drivers/video/fsl-dcu-fb.c b/drivers/video/fsl-dcu-fb.c
new file mode 100644
index 0000000..6b45357
--- /dev/null
+++ b/drivers/video/fsl-dcu-fb.c
@@ -0,0 +1,1095 @@
+/*
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
+ *
+ * Freescale DCU framebuffer device driver
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/fb.h>
+#include <linux/clk.h>
+#include <linux/of_platform.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
+#include <linux/pm_runtime.h>
+
+#define DRIVER_NAME			"fsl-dcu-fb"
+
+#define DCU_DCU_MODE			0x0010
+#define DCU_MODE_BLEND_ITER(x)		((x) << 20)
+#define DCU_MODE_RASTER_EN		(1 << 14)
+#define DCU_MODE_DCU_MODE(x)		(x)
+#define DCU_MODE_DCU_MODE_MASK		0x03
+#define DCU_MODE_OFF			0
+#define DCU_MODE_NORMAL			1
+#define DCU_MODE_TEST			2
+#define DCU_MODE_COLORBAR		3
+
+#define DCU_BGND			0x0014
+#define DCU_BGND_R(x)			((x) << 16)
+#define DCU_BGND_G(x)			((x) << 8)
+#define DCU_BGND_B(x)			(x)
+
+#define DCU_DISP_SIZE			0x0018
+#define DCU_DISP_SIZE_DELTA_Y(x)	((x) << 16)
+#define DCU_DISP_SIZE_DELTA_X(x)	(x)
+
+#define DCU_HSYN_PARA			0x001c
+#define DCU_HSYN_PARA_BP(x)		((x) << 22)
+#define DCU_HSYN_PARA_PW(x)		((x) << 11)
+#define DCU_HSYN_PARA_FP(x)		(x)
+
+#define DCU_VSYN_PARA			0x0020
+#define DCU_VSYN_PARA_BP(x)		((x) << 22)
+#define DCU_VSYN_PARA_PW(x)		((x) << 11)
+#define DCU_VSYN_PARA_FP(x)		(x)
+
+#define DCU_SYN_POL			0x0024
+#define DCU_SYN_POL_INV_PXCK_FALL	(0 << 6)
+#define DCU_SYN_POL_NEG_REMAIN		(0 << 5)
+#define DCU_SYN_POL_INV_VS_LOW		(1 << 1)
+#define DCU_SYN_POL_INV_HS_LOW		(1)
+
+#define DCU_THRESHOLD			0x0028
+#define DCU_THRESHOLD_LS_BF_VS(x)	((x) << 16)
+#define DCU_THRESHOLD_OUT_BUF_HIGH(x)	((x) << 8)
+#define DCU_THRESHOLD_OUT_BUF_LOW(x)	(x)
+
+#define DCU_INT_STATUS			0x002C
+#define DCU_INT_STATUS_UNDRUN		(1 << 1)
+
+#define DCU_INT_MASK			0x0030
+#define DCU_INT_MASK_UNDRUN		(1 << 1)
+
+#define DCU_DIV_RATIO			0x0054
+
+#define DCU_UPDATE_MODE			0x00cc
+#define DCU_UPDATE_MODE_MODE		(1 << 31)
+#define DCU_UPDATE_MODE_READREG		(1 << 30)
+
+#define DCU_CTRLDESCLN_1(x)		(0x200 + (x) * 0x40)
+#define DCU_CTRLDESCLN_1_HEIGHT(x)	((x) << 16)
+#define DCU_CTRLDESCLN_1_WIDTH(x)	(x)
+
+#define DCU_CTRLDESCLN_2(x)		(0x204 + (x) * 0x40)
+#define DCU_CTRLDESCLN_2_POSY(x)	((x) << 16)
+#define DCU_CTRLDESCLN_2_POSX(x)	(x)
+
+#define DCU_CTRLDESCLN_3(x)		(0x208 + (x) * 0x40)
+
+#define DCU_CTRLDESCLN_4(x)		(0x20c + (x) * 0x40)
+#define DCU_CTRLDESCLN_4_EN		(1 << 31)
+#define DCU_CTRLDESCLN_4_TILE_EN	(1 << 30)
+#define DCU_CTRLDESCLN_4_DATA_SEL_CLUT	(1 << 29)
+#define DCU_CTRLDESCLN_4_SAFETY_EN	(1 << 28)
+#define DCU_CTRLDESCLN_4_TRANS(x)	((x) << 20)
+#define DCU_CTRLDESCLN_4_BPP(x)		((x) << 16)
+#define DCU_CTRLDESCLN_4_RLE_EN		(1 << 15)
+#define DCU_CTRLDESCLN_4_LUOFFS(x)	((x) << 4)
+#define DCU_CTRLDESCLN_4_BB_ON		(1 << 2)
+#define DCU_CTRLDESCLN_4_AB(x)		(x)
+
+#define DCU_CTRLDESCLN_5(x)		(0x210 + (x) * 0x40)
+#define DCU_CTRLDESCLN_5_CKMAX_R(x)	((x) << 16)
+#define DCU_CTRLDESCLN_5_CKMAX_G(x)	((x) << 8)
+#define DCU_CTRLDESCLN_5_CKMAX_B(x)	(x)
+
+#define DCU_CTRLDESCLN_6(x)		(0x214 + (x) * 0x40)
+#define DCU_CTRLDESCLN_6_CKMIN_R(x)	((x) << 16)
+#define DCU_CTRLDESCLN_6_CKMIN_G(x)	((x) << 8)
+#define DCU_CTRLDESCLN_6_CKMIN_B(x)	(x)
+
+#define DCU_CTRLDESCLN_7(x)		(0x218 + (x) * 0x40)
+#define DCU_CTRLDESCLN_7_TILE_VER(x)	((x) << 16)
+#define DCU_CTRLDESCLN_7_TILE_HOR(x)	(x)
+
+#define DCU_CTRLDESCLN_8(x)		(0x21c + (x) * 0x40)
+#define DCU_CTRLDESCLN_8_FG_FCOLOR(x)	(x)
+
+#define DCU_CTRLDESCLN_9(x)		(0x220 + (x) * 0x40)
+#define DCU_CTRLDESCLN_9_BG_BCOLOR(x)	(x)
+
+#define DCU_TOTAL_LAYER_NUM		64
+#define DCU_LAYER_NUM_MAX		6
+
+#define BPP_16_RGB565			4
+#define BPP_24_RGB888			5
+#define BPP_32_ARGB8888			6
+
+#define TCON_CTRL1			0x0000
+#define TCON_BYPASS_ENABLE		(1 << 29)
+
+#define MFB_SET_ALPHA		_IOW('M', 0, __u8)
+#define MFB_GET_ALPHA		_IOR('M', 0, __u8)
+#define MFB_SET_LAYER		_IOW('M', 4, struct layer_display_offset)
+#define MFB_GET_LAYER		_IOR('M', 4, struct layer_display_offset)
+
+struct dcu_fb_data {
+	struct fb_info *fsl_dcu_info[DCU_LAYER_NUM_MAX];
+	struct device *dev;
+	void __iomem *reg_base;
+	unsigned int irq;
+	struct clk *clk;
+};
+
+struct layer_display_offset {
+	int x_layer_d;
+	int y_layer_d;
+};
+
+struct mfb_info {
+	int index;
+	char *id;
+	unsigned long pseudo_palette[16];
+	unsigned char alpha;
+	unsigned char blend;
+	unsigned int count;
+	int x_layer_d;	/* layer display x offset to physical screen */
+	int y_layer_d;	/* layer display y offset to physical screen */
+	struct dcu_fb_data *parent;
+};
+
+enum mfb_index {
+	LAYER0 = 0,
+	LAYER1,
+	LAYER2,
+	LAYER3,
+	LAYER4,
+	LAYER5,
+};
+
+static struct mfb_info mfb_template[] = {
+	{
+		.index = LAYER0,
+		.id = "Layer0",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 0,
+		.y_layer_d = 0,
+	},
+	{
+		.index = LAYER1,
+		.id = "Layer1",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 50,
+		.y_layer_d = 50,
+	},
+	{
+		.index = LAYER2,
+		.id = "Layer2",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 100,
+		.y_layer_d = 100,
+	},
+	{
+		.index = LAYER3,
+		.id = "Layer3",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 150,
+		.y_layer_d = 150,
+	},
+	{
+		.index = LAYER4,
+		.id = "Layer4",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 200,
+		.y_layer_d = 200,
+	},
+	{
+		.index = LAYER5,
+		.id = "Layer5",
+		.alpha = 0xff,
+		.blend = 0,
+		.count = 0,
+		.x_layer_d = 250,
+		.y_layer_d = 250,
+	},
+};
+
+static int enable_panel(struct fb_info *info)
+{
+	struct fb_var_screeninfo *var = &info->var;
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	unsigned int bpp;
+
+	writel(DCU_CTRLDESCLN_1_HEIGHT(var->yres) |
+		DCU_CTRLDESCLN_1_WIDTH(var->xres),
+		dcufb->reg_base + DCU_CTRLDESCLN_1(mfbi->index));
+	writel(DCU_CTRLDESCLN_2_POSY(mfbi->y_layer_d) |
+		DCU_CTRLDESCLN_2_POSX(mfbi->x_layer_d),
+		dcufb->reg_base + DCU_CTRLDESCLN_2(mfbi->index));
+
+	writel(info->fix.smem_start,
+		dcufb->reg_base + DCU_CTRLDESCLN_3(mfbi->index));
+
+	switch (var->bits_per_pixel) {
+	case 16:
+		bpp = BPP_16_RGB565;
+		break;
+	case 24:
+		bpp = BPP_24_RGB888;
+		break;
+	case 32:
+		bpp = BPP_32_ARGB8888;
+		break;
+	default:
+		dev_err(dcufb->dev, "unsupported color depth: %u\n",
+			var->bits_per_pixel);
+		return -EINVAL;
+	}
+
+	writel(DCU_CTRLDESCLN_4_EN |
+		DCU_CTRLDESCLN_4_TRANS(mfbi->alpha) |
+		DCU_CTRLDESCLN_4_BPP(bpp) |
+		DCU_CTRLDESCLN_4_AB(mfbi->blend),
+		dcufb->reg_base + DCU_CTRLDESCLN_4(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_5_CKMAX_R(0xff) |
+		DCU_CTRLDESCLN_5_CKMAX_G(0xff) |
+		DCU_CTRLDESCLN_5_CKMAX_B(0xff),
+		dcufb->reg_base + DCU_CTRLDESCLN_5(mfbi->index));
+	writel(DCU_CTRLDESCLN_6_CKMIN_R(0) |
+		DCU_CTRLDESCLN_6_CKMIN_G(0) |
+		DCU_CTRLDESCLN_6_CKMIN_B(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_6(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_7_TILE_VER(0) | DCU_CTRLDESCLN_7_TILE_HOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_7(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_8_FG_FCOLOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_8(mfbi->index));
+	writel(DCU_CTRLDESCLN_9_BG_BCOLOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_9(mfbi->index));
+
+	writel(DCU_UPDATE_MODE_READREG, dcufb->reg_base + DCU_UPDATE_MODE);
+	return 0;
+}
+
+static int disable_panel(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+
+	writel(DCU_CTRLDESCLN_1_HEIGHT(0) |
+		DCU_CTRLDESCLN_1_WIDTH(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_1(mfbi->index));
+	writel(DCU_CTRLDESCLN_2_POSY(0) | DCU_CTRLDESCLN_2_POSX(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_2(mfbi->index));
+
+	writel(0, dcufb->reg_base + DCU_CTRLDESCLN_3(mfbi->index));
+	writel(0, dcufb->reg_base + DCU_CTRLDESCLN_4(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_5_CKMAX_R(0) |
+		DCU_CTRLDESCLN_5_CKMAX_G(0) |
+		DCU_CTRLDESCLN_5_CKMAX_B(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_5(mfbi->index));
+	writel(DCU_CTRLDESCLN_6_CKMIN_R(0) |
+		DCU_CTRLDESCLN_6_CKMIN_G(0) |
+		DCU_CTRLDESCLN_6_CKMIN_B(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_6(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_7_TILE_VER(0) | DCU_CTRLDESCLN_7_TILE_HOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_7(mfbi->index));
+
+	writel(DCU_CTRLDESCLN_8_FG_FCOLOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_8(mfbi->index));
+	writel(DCU_CTRLDESCLN_9_BG_BCOLOR(0),
+		dcufb->reg_base + DCU_CTRLDESCLN_9(mfbi->index));
+
+	writel(DCU_UPDATE_MODE_READREG, dcufb->reg_base + DCU_UPDATE_MODE);
+	return 0;
+}
+
+static void enable_controller(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	unsigned int dcu_mode;
+
+	dcu_mode = readl(dcufb->reg_base + DCU_DCU_MODE);
+	writel(dcu_mode | DCU_MODE_DCU_MODE(DCU_MODE_NORMAL),
+		dcufb->reg_base + DCU_DCU_MODE);
+}
+
+static void disable_controller(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+
+	writel(DCU_MODE_DCU_MODE(DCU_MODE_OFF),
+		dcufb->reg_base + DCU_DCU_MODE);
+}
+
+static int fsl_dcu_check_var(struct fb_var_screeninfo *var,
+		struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+
+	if (var->xres_virtual < var->xres)
+		var->xres_virtual = var->xres;
+	if (var->yres_virtual < var->yres)
+		var->yres_virtual = var->yres;
+
+	if (var->xoffset + info->var.xres > info->var.xres_virtual)
+		var->xoffset = info->var.xres_virtual - info->var.xres;
+
+	if (var->yoffset + info->var.yres > info->var.yres_virtual)
+		var->yoffset = info->var.yres_virtual - info->var.yres;
+
+	switch (var->bits_per_pixel) {
+	case 16:
+		var->red.length = 5;
+		var->red.offset = 11;
+		var->red.msb_right = 0;
+
+		var->green.length = 6;
+		var->green.offset = 5;
+		var->green.msb_right = 0;
+
+		var->blue.length = 5;
+		var->blue.offset = 0;
+		var->blue.msb_right = 0;
+
+		var->transp.length = 0;
+		var->transp.offset = 0;
+		var->transp.msb_right = 0;
+		break;
+	case 24:
+		var->red.length = 8;
+		var->red.offset = 16;
+		var->red.msb_right = 0;
+
+		var->green.length = 8;
+		var->green.offset = 8;
+		var->green.msb_right = 0;
+
+		var->blue.length = 8;
+		var->blue.offset = 0;
+		var->blue.msb_right = 0;
+
+		var->transp.length = 0;
+		var->transp.offset = 0;
+		var->transp.msb_right = 0;
+		break;
+	case 32:
+		var->red.length = 8;
+		var->red.offset = 16;
+		var->red.msb_right = 0;
+
+		var->green.length = 8;
+		var->green.offset = 8;
+		var->green.msb_right = 0;
+
+		var->blue.length = 8;
+		var->blue.offset = 0;
+		var->blue.msb_right = 0;
+
+		var->transp.length = 8;
+		var->transp.offset = 24;
+		var->transp.msb_right = 0;
+		break;
+	default:
+		dev_err(dcufb->dev, "unsupported color depth: %u\n",
+			var->bits_per_pixel);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int fsl_dcu_calc_div(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	unsigned long long div;
+
+	div = (unsigned long long)(clk_get_rate(dcufb->clk) / 1000);
+	div *= info->var.pixclock;
+	do_div(div, 1000000000);
+
+	return div;
+}
+
+static void update_controller(struct fb_info *info)
+{
+	struct fb_var_screeninfo *var = &info->var;
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	unsigned int div;
+
+	div = fsl_dcu_calc_div(info);
+	writel((div - 1), dcufb->reg_base + DCU_DIV_RATIO);
+
+	writel(DCU_DISP_SIZE_DELTA_Y(var->yres) |
+		DCU_DISP_SIZE_DELTA_X(var->xres / 16),
+		dcufb->reg_base + DCU_DISP_SIZE);
+
+	/* Horizontal and vertical sync parameters */
+	writel(DCU_HSYN_PARA_BP(var->left_margin) |
+		DCU_HSYN_PARA_PW(var->hsync_len) |
+		DCU_HSYN_PARA_FP(var->right_margin),
+		dcufb->reg_base + DCU_HSYN_PARA);
+
+	writel(DCU_VSYN_PARA_BP(var->upper_margin) |
+		DCU_VSYN_PARA_PW(var->vsync_len) |
+		DCU_VSYN_PARA_FP(var->lower_margin),
+		dcufb->reg_base + DCU_VSYN_PARA);
+
+	writel(DCU_SYN_POL_INV_PXCK_FALL | DCU_SYN_POL_NEG_REMAIN |
+		DCU_SYN_POL_INV_VS_LOW | DCU_SYN_POL_INV_HS_LOW,
+		dcufb->reg_base + DCU_SYN_POL);
+
+	writel(DCU_BGND_R(0) | DCU_BGND_G(0) | DCU_BGND_B(0),
+		dcufb->reg_base + DCU_BGND);
+
+	writel(DCU_MODE_BLEND_ITER(DCU_LAYER_NUM_MAX) | DCU_MODE_RASTER_EN,
+			dcufb->reg_base + DCU_DCU_MODE);
+
+	writel(DCU_THRESHOLD_LS_BF_VS(0x3) | DCU_THRESHOLD_OUT_BUF_HIGH(0x78) |
+		DCU_THRESHOLD_OUT_BUF_LOW(0), dcufb->reg_base + DCU_THRESHOLD);
+}
+
+static int map_video_memory(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	u32 smem_len = info->fix.line_length * info->var.yres_virtual;
+
+	info->fix.smem_len = smem_len;
+
+	info->screen_base = dma_alloc_writecombine(info->device,
+		info->fix.smem_len, (dma_addr_t *)&info->fix.smem_start,
+		GFP_KERNEL);
+	if (!info->screen_base) {
+		dev_err(dcufb->dev, "unable to allocate fb memory\n");
+		return -ENOMEM;
+	}
+
+	memset(info->screen_base, 0, info->fix.smem_len);
+
+	return 0;
+}
+
+static void unmap_video_memory(struct fb_info *info)
+{
+	if (!info->screen_base)
+		return;
+
+	dma_free_writecombine(info->device, info->fix.smem_len,
+		info->screen_base, info->fix.smem_start);
+
+	info->screen_base = NULL;
+	info->fix.smem_start = 0;
+	info->fix.smem_len = 0;
+}
+
+static int fsl_dcu_set_layer(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct fb_var_screeninfo *var = &info->var;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	int pixel_offset;
+	unsigned long addr;
+
+	pixel_offset = (var->yoffset * var->xres_virtual) + var->xoffset;
+	addr = info->fix.smem_start +
+		(pixel_offset * (var->bits_per_pixel >> 3));
+
+	writel(addr, dcufb->reg_base + DCU_CTRLDESCLN_3(mfbi->index));
+	writel(DCU_UPDATE_MODE_READREG, dcufb->reg_base + DCU_UPDATE_MODE);
+
+	return 0;
+}
+
+static int fsl_dcu_set_par(struct fb_info *info)
+{
+	unsigned long len;
+	struct fb_var_screeninfo *var = &info->var;
+	struct fb_fix_screeninfo *fix = &info->fix;
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+
+	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
+	fix->type = FB_TYPE_PACKED_PIXELS;
+	fix->accel = FB_ACCEL_NONE;
+	fix->visual = FB_VISUAL_TRUECOLOR;
+	fix->xpanstep = 1;
+	fix->ypanstep = 1;
+
+	len = info->var.yres_virtual * info->fix.line_length;
+	if (len != info->fix.smem_len) {
+		if (info->fix.smem_start)
+			unmap_video_memory(info);
+
+		if (map_video_memory(info)) {
+			dev_err(dcufb->dev, "unable to allocate fb memory\n");
+			return -ENOMEM;
+		}
+	}
+
+	/* Only layer 0 could update LCD controller */
+	if (mfbi->index = LAYER0) {
+		update_controller(info);
+		enable_controller(info);
+	}
+
+	enable_panel(info);
+	return 0;
+}
+
+static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
+{
+	return ((val<<width) + 0x7FFF - val) >> 16;
+}
+
+static int fsl_dcu_setcolreg(unsigned regno, unsigned red, unsigned green,
+			   unsigned blue, unsigned transp, struct fb_info *info)
+{
+	unsigned int val;
+	int ret = -EINVAL;
+
+	/*
+	 * If greyscale is true, then we convert the RGB value
+	 * to greyscale no matter what visual we are using.
+	 */
+	if (info->var.grayscale)
+		red = green = blue = (19595 * red + 38470 * green +
+				      7471 * blue) >> 16;
+	switch (info->fix.visual) {
+	case FB_VISUAL_TRUECOLOR:
+		/*
+		 * 16-bit True Colour.  We encode the RGB value
+		 * according to the RGB bitfield information.
+		 */
+		if (regno < 16) {
+			u32 *pal = info->pseudo_palette;
+
+			red = CNVT_TOHW(red, info->var.red.length);
+			green = CNVT_TOHW(green, info->var.green.length);
+			blue = CNVT_TOHW(blue, info->var.blue.length);
+			transp = CNVT_TOHW(transp, info->var.transp.length);
+
+			val = (red << info->var.red.offset) |
+			    (green << info->var.green.offset) |
+			    (blue << info->var.blue.offset) |
+			    (transp << info->var.transp.offset);
+
+			pal[regno] = val;
+			ret = 0;
+		}
+		break;
+	case FB_VISUAL_STATIC_PSEUDOCOLOR:
+	case FB_VISUAL_PSEUDOCOLOR:
+		break;
+	}
+
+	return ret;
+}
+
+static int fsl_dcu_pan_display(struct fb_var_screeninfo *var,
+			     struct fb_info *info)
+{
+	if ((info->var.xoffset = var->xoffset) &&
+	    (info->var.yoffset = var->yoffset))
+		return 0;
+
+	if ((var->xoffset + info->var.xres) > info->var.xres_virtual
+	    || (var->yoffset + info->var.yres) > info->var.yres_virtual)
+		return -EINVAL;
+
+	info->var.xoffset = var->xoffset;
+	info->var.yoffset = var->yoffset;
+
+	if (var->vmode & FB_VMODE_YWRAP)
+		info->var.vmode |= FB_VMODE_YWRAP;
+	else
+		info->var.vmode &= ~FB_VMODE_YWRAP;
+
+	fsl_dcu_set_layer(info);
+
+	return 0;
+}
+
+static int fsl_dcu_blank(int blank_mode, struct fb_info *info)
+{
+	switch (blank_mode) {
+	case FB_BLANK_VSYNC_SUSPEND:
+	case FB_BLANK_HSYNC_SUSPEND:
+	case FB_BLANK_NORMAL:
+		disable_panel(info);
+		break;
+	case FB_BLANK_POWERDOWN:
+		disable_controller(info);
+		break;
+	case FB_BLANK_UNBLANK:
+		enable_panel(info);
+		break;
+	}
+
+	return 0;
+}
+
+static int fsl_dcu_ioctl(struct fb_info *info, unsigned int cmd,
+		unsigned long arg)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	struct layer_display_offset layer_d;
+	void __user *buf = (void __user *)arg;
+	unsigned char alpha;
+
+	switch (cmd) {
+	case MFB_SET_LAYER:
+		if (copy_from_user(&layer_d, buf, sizeof(layer_d)))
+			return -EFAULT;
+		mfbi->x_layer_d = layer_d.x_layer_d;
+		mfbi->y_layer_d = layer_d.y_layer_d;
+		fsl_dcu_set_par(info);
+		break;
+	case MFB_GET_LAYER:
+		layer_d.x_layer_d = mfbi->x_layer_d;
+		layer_d.y_layer_d = mfbi->y_layer_d;
+		if (copy_to_user(buf, &layer_d, sizeof(layer_d)))
+			return -EFAULT;
+		break;
+	case MFB_GET_ALPHA:
+		alpha = mfbi->alpha;
+		if (copy_to_user(buf, &alpha, sizeof(alpha)))
+			return -EFAULT;
+		break;
+	case MFB_SET_ALPHA:
+		if (copy_from_user(&alpha, buf, sizeof(alpha)))
+			return -EFAULT;
+		mfbi->blend = 1;
+		mfbi->alpha = alpha;
+		fsl_dcu_set_par(info);
+		break;
+	default:
+		dev_err(dcufb->dev, "unknown ioctl command (0x%08X)\n", cmd);
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
+static void reset_total_layers(struct dcu_fb_data *dcufb)
+{
+	int i;
+
+	for (i = 1; i < DCU_TOTAL_LAYER_NUM; i++) {
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_1(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_2(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_3(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_4(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_5(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_6(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_7(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_8(i));
+		writel(0, dcufb->reg_base + DCU_CTRLDESCLN_9(i));
+	}
+	writel(DCU_UPDATE_MODE_READREG, dcufb->reg_base + DCU_UPDATE_MODE);
+}
+
+static int fsl_dcu_open(struct fb_info *info, int user)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	u32 int_mask = readl(dcufb->reg_base + DCU_INT_MASK);
+	int ret = 0;
+
+	mfbi->index = info->node;
+
+	mfbi->count++;
+	if (mfbi->count = 1) {
+		fsl_dcu_check_var(&info->var, info);
+		ret = fsl_dcu_set_par(info);
+		if (ret < 0)
+			mfbi->count--;
+		else
+			writel(int_mask & ~DCU_INT_MASK_UNDRUN,
+				dcufb->reg_base + DCU_INT_MASK);
+	}
+
+	return ret;
+}
+
+static int fsl_dcu_release(struct fb_info *info, int user)
+{
+	struct mfb_info *mfbi = info->par;
+	int ret = 0;
+
+	mfbi->count--;
+	if (mfbi->count = 0)
+		ret = disable_panel(info);
+
+	return ret;
+}
+
+static struct fb_ops fsl_dcu_ops = {
+	.owner = THIS_MODULE,
+	.fb_check_var = fsl_dcu_check_var,
+	.fb_set_par = fsl_dcu_set_par,
+	.fb_setcolreg = fsl_dcu_setcolreg,
+	.fb_blank = fsl_dcu_blank,
+	.fb_pan_display = fsl_dcu_pan_display,
+	.fb_fillrect = cfb_fillrect,
+	.fb_copyarea = cfb_copyarea,
+	.fb_imageblit = cfb_imageblit,
+	.fb_ioctl = fsl_dcu_ioctl,
+	.fb_open = fsl_dcu_open,
+	.fb_release = fsl_dcu_release,
+};
+
+static int fsl_dcu_init_fbinfo(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct fb_var_screeninfo *var = &info->var;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	struct device_node *np = dcufb->dev->of_node;
+	struct device_node *display_np;
+	struct device_node *timings_np;
+	struct display_timings *timings;
+	int i;
+	int ret = 0;
+
+	display_np = of_parse_phandle(np, "display", 0);
+	if (!display_np) {
+		dev_err(dcufb->dev, "failed to find display phandle\n");
+		return -ENOENT;
+	}
+
+	ret = of_property_read_u32(display_np, "bits-per-pixel",
+				   &var->bits_per_pixel);
+	if (ret < 0) {
+		dev_err(dcufb->dev, "failed to get property bits-per-pixel\n");
+		goto put_display_node;
+	}
+
+	timings = of_get_display_timings(display_np);
+	if (!timings) {
+		dev_err(dcufb->dev, "failed to get display timings\n");
+		ret = -ENOENT;
+		goto put_display_node;
+	}
+
+	timings_np = of_find_node_by_name(display_np,
+					  "display-timings");
+	if (!timings_np) {
+		dev_err(dcufb->dev, "failed to find display-timings node\n");
+		ret = -ENOENT;
+		goto put_display_node;
+	}
+
+	for (i = 0; i < of_get_child_count(timings_np); i++) {
+		struct videomode vm;
+		struct fb_videomode fb_vm;
+
+		ret = videomode_from_timings(timings, &vm, i);
+		if (ret < 0)
+			goto put_timings_node;
+
+		ret = fb_videomode_from_videomode(&vm, &fb_vm);
+		if (ret < 0)
+			goto put_timings_node;
+
+		fb_add_videomode(&fb_vm, &info->modelist);
+	}
+
+	return 0;
+put_timings_node:
+	of_node_put(timings_np);
+put_display_node:
+	of_node_put(display_np);
+	return ret;
+}
+
+static int install_framebuffer(struct fb_info *info)
+{
+	struct mfb_info *mfbi = info->par;
+	struct dcu_fb_data *dcufb = mfbi->parent;
+	struct fb_modelist *modelist;
+	int ret;
+
+	info->var.activate = FB_ACTIVATE_NOW;
+	info->fbops = &fsl_dcu_ops;
+	info->flags = FBINFO_FLAG_DEFAULT;
+	info->pseudo_palette = &mfbi->pseudo_palette;
+
+	fb_alloc_cmap(&info->cmap, 16, 0);
+
+	INIT_LIST_HEAD(&info->modelist);
+
+	ret = fsl_dcu_init_fbinfo(info);
+	if (ret)
+		return ret;
+
+	modelist = list_first_entry(&info->modelist,
+			struct fb_modelist, list);
+	fb_videomode_to_var(&info->var, &modelist->mode);
+
+	fsl_dcu_check_var(&info->var, info);
+	ret = register_framebuffer(info);
+	if (ret < 0) {
+		dev_err(dcufb->dev, "failed to register framebuffer device\n");
+		return ret;
+	}
+
+	printk(KERN_INFO "fb%d: fb device registered successfully.\n",
+			info->node);
+	return 0;
+}
+
+static void uninstall_framebuffer(struct fb_info *info)
+{
+	unregister_framebuffer(info);
+	unmap_video_memory(info);
+
+	if (&info->cmap)
+		fb_dealloc_cmap(&info->cmap);
+}
+
+static irqreturn_t fsl_dcu_irq(int irq, void *dev_id)
+{
+	struct dcu_fb_data *dcufb = dev_id;
+	unsigned int status = readl(dcufb->reg_base + DCU_INT_STATUS);
+	u32 dcu_mode;
+
+	if (status & DCU_INT_STATUS_UNDRUN) {
+		dcu_mode = readl(dcufb->reg_base + DCU_DCU_MODE);
+		dcu_mode &= ~DCU_MODE_DCU_MODE_MASK;
+		writel(dcu_mode | DCU_MODE_DCU_MODE(DCU_MODE_OFF),
+			dcufb->reg_base + DCU_DCU_MODE);
+		udelay(1);
+		writel(dcu_mode | DCU_MODE_DCU_MODE(DCU_MODE_NORMAL),
+			dcufb->reg_base + DCU_DCU_MODE);
+	}
+
+	writel(status, dcufb->reg_base + DCU_INT_STATUS);
+	return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM_RUNTIME
+static int fsl_dcu_runtime_suspend(struct device *dev)
+{
+	struct dcu_fb_data *dcufb = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(dcufb->clk);
+
+	return 0;
+}
+
+static int fsl_dcu_runtime_resume(struct device *dev)
+{
+	struct dcu_fb_data *dcufb = dev_get_drvdata(dev);
+
+	clk_prepare_enable(dcufb->clk);
+
+	return 0;
+}
+#endif
+
+static int bypass_tcon(struct device_node *np)
+{
+	struct device_node *tcon_np;
+	struct platform_device *tcon_pdev;
+	struct clk *tcon_clk;
+	struct resource *res;
+	void __iomem *tcon_reg;
+
+	tcon_np = of_parse_phandle(np, "tcon-controller", 0);
+	if (!tcon_np)
+		return -EINVAL;
+
+	tcon_pdev = of_find_device_by_node(tcon_np);
+	if (!tcon_pdev)
+		return -EINVAL;
+
+	tcon_clk = devm_clk_get(&tcon_pdev->dev, "tcon");
+	if (IS_ERR(tcon_clk))
+		return PTR_ERR(tcon_clk);
+	clk_prepare_enable(tcon_clk);
+
+	res = platform_get_resource(tcon_pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	tcon_reg = devm_ioremap_resource(&tcon_pdev->dev, res);
+	if (IS_ERR(tcon_reg))
+		return PTR_ERR(tcon_reg);
+
+	writel(TCON_BYPASS_ENABLE, tcon_reg + TCON_CTRL1);
+	return 0;
+}
+
+static int fsl_dcu_probe(struct platform_device *pdev)
+{
+	struct dcu_fb_data *dcufb;
+	struct mfb_info *mfbi;
+	struct resource *res;
+	int ret = 0;
+	int i;
+
+	dcufb = devm_kzalloc(&pdev->dev,
+		sizeof(struct dcu_fb_data), GFP_KERNEL);
+	if (!dcufb)
+		return -ENOMEM;
+
+	dcufb->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, dcufb);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "could not get memory IO resource\n");
+		return -ENODEV;
+	}
+
+	dcufb->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dcufb->reg_base)) {
+		dev_err(&pdev->dev, "could not ioremap resource\n");
+		return PTR_ERR(dcufb->reg_base);
+	}
+
+	dcufb->irq = platform_get_irq(pdev, 0);
+	if (!dcufb->irq) {
+		dev_err(&pdev->dev, "could not get irq\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(&pdev->dev, dcufb->irq, fsl_dcu_irq,
+			0, DRIVER_NAME, dcufb);
+	if (ret) {
+		dev_err(&pdev->dev, "could not request irq\n");
+		goto failed_ioremap;
+	}
+
+	/* Put TCON in bypass mode, so the input signals from DCU are passed
+	 * through TCON unchanged */
+	ret = bypass_tcon(pdev->dev.of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "could not bypass TCON\n");
+		goto failed_bypasstcon;
+	}
+
+	dcufb->clk = devm_clk_get(&pdev->dev, "dcu");
+	if (IS_ERR(dcufb->clk)) {
+		ret = PTR_ERR(dcufb->clk);
+		dev_err(&pdev->dev, "could not get clock\n");
+		goto failed_getclock;
+	}
+	clk_prepare_enable(dcufb->clk);
+
+	pm_runtime_enable(dcufb->dev);
+	pm_runtime_get_sync(dcufb->dev);
+
+	for (i = 0; i < ARRAY_SIZE(dcufb->fsl_dcu_info); i++) {
+		dcufb->fsl_dcu_info[i] +			framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
+		if (!dcufb->fsl_dcu_info[i]) {
+			ret = -ENOMEM;
+			goto failed_alloc_framebuffer;
+		}
+
+		dcufb->fsl_dcu_info[i]->fix.smem_start = 0;
+
+		mfbi = dcufb->fsl_dcu_info[i]->par;
+		memcpy(mfbi, &mfb_template[i], sizeof(struct mfb_info));
+		mfbi->parent = dcufb;
+
+		ret = install_framebuffer(dcufb->fsl_dcu_info[i]);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"could not register framebuffer %d\n", i);
+			goto failed_register_framebuffer;
+		}
+	}
+
+	reset_total_layers(mfbi->parent);
+	return 0;
+
+failed_register_framebuffer:
+	for (i = 0; i < ARRAY_SIZE(dcufb->fsl_dcu_info); i++) {
+		if (dcufb->fsl_dcu_info[i])
+			framebuffer_release(dcufb->fsl_dcu_info[i]);
+	}
+failed_alloc_framebuffer:
+	pm_runtime_put_sync(dcufb->dev);
+	pm_runtime_disable(dcufb->dev);
+failed_getclock:
+failed_bypasstcon:
+	free_irq(dcufb->irq, dcufb);
+failed_ioremap:
+	return ret;
+}
+
+static int fsl_dcu_remove(struct platform_device *pdev)
+{
+	struct dcu_fb_data *dcufb = dev_get_drvdata(&pdev->dev);
+	int i;
+
+	pm_runtime_get_sync(dcufb->dev);
+
+	disable_controller(dcufb->fsl_dcu_info[0]);
+
+	clk_disable_unprepare(dcufb->clk);
+	free_irq(dcufb->irq, dcufb);
+
+	for (i = 0; i < ARRAY_SIZE(dcufb->fsl_dcu_info); i++) {
+		uninstall_framebuffer(dcufb->fsl_dcu_info[i]);
+		framebuffer_release(dcufb->fsl_dcu_info[i]);
+	}
+
+	pm_runtime_put_sync(dcufb->dev);
+	pm_runtime_disable(dcufb->dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops fsl_dcu_pm_ops = {
+	SET_RUNTIME_PM_OPS(fsl_dcu_runtime_suspend,
+			fsl_dcu_runtime_resume, NULL)
+};
+
+static struct of_device_id fsl_dcu_dt_ids[] = {
+	{
+		.compatible = "fsl,vf610-dcu",
+	},
+	{}
+};
+
+static struct platform_driver fsl_dcu_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = fsl_dcu_dt_ids,
+		.pm = &fsl_dcu_pm_ops,
+	},
+	.probe = fsl_dcu_probe,
+	.remove = fsl_dcu_remove,
+};
+
+module_platform_driver(fsl_dcu_driver);
+
+MODULE_AUTHOR("Alison Wang");
+MODULE_DESCRIPTION("Freescale DCU framebuffer driver");
+MODULE_LICENSE("GPL v2");
-- 
1.8.0



^ permalink raw reply related

* [PATCH v3 5/5] Documentation: DT: Add DCU framebuffer driver
From: Alison Wang @ 2013-08-15  8:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1376553714-25922-1-git-send-email-b18965@freescale.com>

This patch adds the document for DCU framebuffer driver under
Documentation/devicetree/bindings/fb/.

Signed-off-by: Alison Wang <b18965@freescale.com>
---
Changes in v3: Add display node description
Changes in v2: New

 .../devicetree/bindings/fb/fsl-dcu-fb.txt          | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt

diff --git a/Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt b/Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt
new file mode 100644
index 0000000..d0d50dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/fb/fsl-dcu-fb.txt
@@ -0,0 +1,67 @@
+* Freescale Display Control Unit (DCU)
+
+Required properties:
+- compatible: Should be "fsl,vf610-dcu". Supported chips include
+  Vybrid VF610.
+- reg: Address and length of the register set for DCU.
+- interrupts: Should contain DCU interrupts.
+- clocks: From common clock binding: handle to DCU clock.
+- clock-names: From common clock binding: Shall be "dcu".
+- tcon-controller: The phandle of TCON controller.
+- display: The phandle to display node.
+
+For the DCU initialization, we read data from TCON node.
+Required properties for TCON:
+- compatible: Should be "fsl,vf610-tcon". Supported chips include
+  Vybrid VF610.
+- reg: Address and length of the register set for TCON.
+- clocks: From common clock binding: handle to TCON clock.
+- clock-names: From common clock binding: Shall be "tcon".
+
+* display node
+
+Required properties:
+- bits-per-pixel: <24> for RGB888.
+
+Required sub-node:
+- display-timings: Refer to binding doc display-timing.txt for details.
+
+Examples:
+
+dcu0: dcu@40058000 {
+	compatible = "fsl,vf610-dcu";
+	reg = <0x40058000 0x1200>;
+	interrupts = <0 30 0x04>;
+	clocks = <&clks VF610_CLK_DCU0>;
+	clock-names = "dcu";
+	tcon-controller = <&tcon0>;
+	display = <&display>;
+
+	display: display@0 {
+		bits-per-pixel = <24>;
+
+		display-timings {
+			native-mode = <&timing0>;
+			timing0: nl4827hc19 {
+				clock-frequency = <10870000>;
+				hactive = <480>;
+				vactive = <272>;
+				hback-porch = <2>;
+				hfront-porch = <2>;
+				vback-porch = <1>;
+				vfront-porch = <1>;
+				hsync-len = <41>;
+				vsync-len = <2>;
+				hsync-active = <1>;
+				vsync-active = <1>;
+			};
+		};
+	};
+};
+
+tcon0: tcon@4003d000 {
+	compatible = "fsl,vf610-tcon";
+	reg = <0x4003d000 0x1000>;
+	clocks = <&clks VF610_CLK_TCON0>;
+	clock-names = "tcon";
+};
-- 
1.8.0



^ permalink raw reply related

* [PATCH] fbmem: move EXPORT_SYMBOL annotation next to symbol declarations
From: Daniel Mack @ 2013-08-15 14:12 UTC (permalink / raw)
  To: linux-fbdev

Just a cosmetic thing to bring that file in line with others in the
tree.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/video/fbmem.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 36e1fe2..dacaf74 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -43,8 +43,12 @@
 #define FBPIXMAPSIZE	(1024 * 8)
 
 static DEFINE_MUTEX(registration_lock);
+
 struct fb_info *registered_fb[FB_MAX] __read_mostly;
+EXPORT_SYMBOL(registered_fb);
+
 int num_registered_fb __read_mostly;
+EXPORT_SYMBOL(num_registered_fb);
 
 static struct fb_info *get_fb_info(unsigned int idx)
 {
@@ -182,6 +186,7 @@ char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size
 
 	return addr;
 }
+EXPORT_SYMBOL(fb_get_buffer_offset);
 
 #ifdef CONFIG_LOGO
 
@@ -669,6 +674,7 @@ int fb_show_logo(struct fb_info *info, int rotate)
 int fb_prepare_logo(struct fb_info *info, int rotate) { return 0; }
 int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
 #endif /* CONFIG_LOGO */
+EXPORT_SYMBOL(fb_show_logo);
 
 static void *fb_seq_start(struct seq_file *m, loff_t *pos)
 {
@@ -909,6 +915,7 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
 		info->var.vmode &= ~FB_VMODE_YWRAP;
 	return 0;
 }
+EXPORT_SYMBOL(fb_pan_display);
 
 static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 			 u32 activate)
@@ -1042,6 +1049,7 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
  done:
 	return ret;
 }
+EXPORT_SYMBOL(fb_set_var);
 
 int
 fb_blank(struct fb_info *info, int blank)
@@ -1073,6 +1081,7 @@ fb_blank(struct fb_info *info, int blank)
 
  	return ret;
 }
+EXPORT_SYMBOL(fb_blank);
 
 static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			unsigned long arg)
@@ -1745,6 +1754,7 @@ register_framebuffer(struct fb_info *fb_info)
 
 	return ret;
 }
+EXPORT_SYMBOL(register_framebuffer);
 
 /**
  *	unregister_framebuffer - releases a frame buffer device
@@ -1773,6 +1783,7 @@ unregister_framebuffer(struct fb_info *fb_info)
 
 	return ret;
 }
+EXPORT_SYMBOL(unregister_framebuffer);
 
 /**
  *	fb_set_suspend - low level driver signals suspend
@@ -1796,6 +1807,7 @@ void fb_set_suspend(struct fb_info *info, int state)
 		fb_notifier_call_chain(FB_EVENT_RESUME, &event);
 	}
 }
+EXPORT_SYMBOL(fb_set_suspend);
 
 /**
  *	fbmem_init - init frame buffer subsystem
@@ -1912,6 +1924,7 @@ int fb_get_options(const char *name, char **option)
 
 	return retval;
 }
+EXPORT_SYMBOL(fb_get_options);
 
 #ifndef MODULE
 /**
@@ -1959,20 +1972,4 @@ static int __init video_setup(char *options)
 __setup("video=", video_setup);
 #endif
 
-    /*
-     *  Visible symbols for modules
-     */
-
-EXPORT_SYMBOL(register_framebuffer);
-EXPORT_SYMBOL(unregister_framebuffer);
-EXPORT_SYMBOL(num_registered_fb);
-EXPORT_SYMBOL(registered_fb);
-EXPORT_SYMBOL(fb_show_logo);
-EXPORT_SYMBOL(fb_set_var);
-EXPORT_SYMBOL(fb_blank);
-EXPORT_SYMBOL(fb_pan_display);
-EXPORT_SYMBOL(fb_get_buffer_offset);
-EXPORT_SYMBOL(fb_set_suspend);
-EXPORT_SYMBOL(fb_get_options);
-
 MODULE_LICENSE("GPL");
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] efifb: prevent null dereferences by removing unused array indices from dmi_list
From: David Herrmann @ 2013-08-16 13:37 UTC (permalink / raw)
  To: James Bates
  Cc: Peter Jones, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev, linux-kernel, H. Peter Anvin
In-Reply-To: <C9C6655E-4974-4C04-8CEE-CA16563F4CC5@gmail.com>

Hi

On Wed, Aug 7, 2013 at 1:15 AM, James Bates <james.h.bates@gmail.com> wrote:
> Hi all,
>
> The dmi_list array is initialized using gnu designated initializers, and therefore
> contains fewer explicitly defined entries as there are elements in it. This
> is because the enum above with M_blabla constants contains more items than the designated
> initializer. Those elements not explicitly initialized are implicitly set to 0.
>
> Now efifb_setup(), L.322 & L.323, loops through all these array elements, and performs
> a strcmp o a field (optname) in each item. For non explicitly initialized elements this
> will be a null pointer:
>
>                         for (i = 0; i < M_UNKNOWN; i++) {
>                                 if (!strcmp(this_opt, dmi_list[i].optname) &&
>
> On my macbook6,1 the predefined values are for some reason incorrect, and most parameters
> are preset correctly by my efi bootloader (elilo). but stride/line_length is not detected
> correctly and so I wish to set it explicitly using a "videoïifb:stride:2048" command-line
> argument. Because of the above null dereference, an exception (presumably) occurs before
> the parsing code (L.333) is ever reached. I say presumably since the mac hangs on boot
> without a console, and I can therefore not see any output.
>
> By removing the unused values from the enum, and thus preventing implicitly initialized items
> in the dmi_list array, the null dereference does not occur, my customer command-line arg is
> parsed correctly, and my console displays correctly.
>
> Signed-off-by: James Bates <james.h.bates@gmail.com>

(CC'ing hpa)

Yepp, this patch looks good:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

However, I'd like to see some code to prevent this from happening
again. It isn't really obvious that removing an entry will result in a
NULL-deref. I am not the maintainer of this code, but I'd really like
to see a "(dmi_list[i].optname && !strcmp())" check in efifb_setup().
Otherwise we _will_ run into this again.

Side note: this code got moved to arch/x86/kernel/sysfb_efi.c in the
x86 tree. I am adding hpa here so he will remember this once Linus
gets a merge conflict (iff the sysfb changes get merged through the
x86 tree).

Thanks
David

> ---
>  drivers/video/efifb.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 50fe668..52d1d88 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -50,12 +50,9 @@ enum {
>         M_MINI_3_1,     /* Mac Mini, 3,1th gen */
>         M_MINI_4_1,     /* Mac Mini, 4,1th gen */
>         M_MB,           /* MacBook */
> -       M_MB_2,         /* MacBook, 2nd rev. */
> -       M_MB_3,         /* MacBook, 3rd rev. */
>         M_MB_5_1,       /* MacBook, 5th rev. */
>         M_MB_6_1,       /* MacBook, 6th rev. */
>         M_MB_7_1,       /* MacBook, 7th rev. */
> -       M_MB_SR,        /* MacBook, 2nd gen, (Santa Rosa) */
>         M_MBA,          /* MacBook Air */
>         M_MBA_3,        /* Macbook Air, 3rd rev */
>         M_MBP,          /* MacBook Pro */
> --
> 1.7.12.4 (Apple Git-37)
>

^ permalink raw reply

* Re: source code file for the generic vga driver?
From: David Herrmann @ 2013-08-16 13:45 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <7c86dd7e9e26494785ec5607b9e79440@DFM-DB3MBX15-06.exchange.corp.microsoft.com>

Hi

On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
> Hi folks,
>
> I'm working on an issue of HyperV synthetic frame buffer driver, which seems to have
> a conflict with the generic vga driver (not the vesa driver). I hope to read and trace into
> the source code for the generic vga driver...
>
> Can anyone point me to the source code file for the generic vga driver in the kernel tree?

Everything lives in ./drivers/video/. The drivers you're probably
interested in are "vesafb.c" or "vga16fb.c". There is also the
"vgacon" driver in ./drivers/video/console/vgacon.c. I am not sure
which one you are talking about.

You might also want to have a look at the x86 sysfb infrastructure
which isn't merged, yet:
http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
It provides proper platform-devices so drivers no longer conflict on
the vga/vesa/efi.. framebuffer resources. It's x86 only as all the
relevant drivers only work on x86.

If you give some more information on what you are trying to do, I can
point you to the relevant resources. My guess is that you want to have
a look at remove_conflicting_framebuffers() in
./drivers/video/fbmem.c.

Regards
David

^ permalink raw reply

* Re: [PATCH] Release efifb's colormap in efifb_destroy()
From: David Herrmann @ 2013-08-16 13:51 UTC (permalink / raw)
  To: Peter Jones
  Cc: Catalin Marinas, Alexandra N. Kossovsky,
	linux-fbdev@vger.kernel.org, linux-kernel, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <1374767291-2874-1-git-send-email-pjones@redhat.com>

Hi

On Thu, Jul 25, 2013 at 5:48 PM, Peter Jones <pjones@redhat.com> wrote:
> This was found by Alexandra Kossovsky, who noted this traceback from
> kmemleak:
>
>> unreferenced object 0xffff880216fcfe00 (size 512):
>>   comm "swapper/0", pid 1, jiffies 4294895429 (age 1415.320s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa  ................
>>     55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff  UUUUUUUU........
>>   backtrace:
>>     [<ffffffff813e415c>] kmemleak_alloc+0x21/0x3e
>>     [<ffffffff8111c17f>]
>>     kmemleak_alloc_recursive.constprop.57+0x16/0x18
>>     [<ffffffff8111e63b>] __kmalloc+0xf9/0x144
>>     [<ffffffff8123d9cf>] fb_alloc_cmap_gfp+0x47/0xe1
>>     [<ffffffff8123da77>] fb_alloc_cmap+0xe/0x10
>>     [<ffffffff81aff40a>] efifb_probe+0x3e9/0x48f
>>     [<ffffffff812c566f>] platform_drv_probe+0x34/0x5e
>>     [<ffffffff812c3e6d>] driver_probe_device+0x98/0x1b4
>>     [<ffffffff812c3fd7>] __driver_attach+0x4e/0x6f
>>     [<ffffffff812c25bf>] bus_for_each_dev+0x57/0x8a
>>     [<ffffffff812c3984>] driver_attach+0x19/0x1b
>>     [<ffffffff812c362b>] bus_add_driver+0xde/0x201
>>     [<ffffffff812c453f>] driver_register+0x8c/0x110
>>     [<ffffffff812c510d>] platform_driver_register+0x41/0x43
>>     [<ffffffff812c5127>] platform_driver_probe+0x18/0x8a
>>     [<ffffffff81aff002>] efifb_init+0x276/0x295

(CC'ing fbdev maintainers)

Your signed-off-by is missing. Apart from that:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Regards
David

> ---
>  drivers/video/efifb.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 390b61b..1f3eab3 100644
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -289,6 +289,7 @@ static void efifb_destroy(struct fb_info *info)
>         if (request_mem_succeeded)
>                 release_mem_region(info->apertures->ranges[0].base,
>                                    info->apertures->ranges[0].size);
> +       fb_dealloc_cmap(&info->cmap);
>         framebuffer_release(info);
>  }
>
> --
> 1.8.3.1
>
> --
> 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] drivers: video: fbcmap: add signed type cast for comparation
From: David Herrmann @ 2013-08-16 13:56 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <51ECECA2.1020006@asianux.com>

Hi

On Mon, Jul 22, 2013 at 10:26 AM, Chen Gang <gang.chen@asianux.com> wrote:
> According to fb_set_cmap() in this file, knows about it is necessary to
> check 'start' whether less than zero or not, so need add related type
> cast for its comparing.
>
> The related warning:
>
>   drivers/video/fbcmap.c:288:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  drivers/video/fbcmap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
> index 5c3960d..44c88e3 100644
> --- a/drivers/video/fbcmap.c
> +++ b/drivers/video/fbcmap.c
> @@ -285,7 +285,7 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
>                 rc = -ENODEV;
>                 goto out;
>         }
> -       if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
> +       if ((int)cmap->start < 0 || (!info->fbops->fb_setcolreg &&
>                                 !info->fbops->fb_setcmap)) {

What's the reason to do this, anyway? fb_set_cmap() does the same but
actually casts to a signed integer. So I think we can remove this
check here.

Regards
David

>                 rc = -EINVAL;
>                 goto out1;
> --
> 1.7.7.6
> --
> 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: conflict of hyperv_fb and the generic video driver?
From: Haiyang Zhang @ 2013-08-16 18:45 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard



> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Friday, August 16, 2013 9:46 AM
> To: Haiyang Zhang
> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> foundation.org; KY Srinivasan
> Subject: Re: source code file for the generic vga driver?
> 
> Hi
> 
> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang <haiyangz@microsoft.com>
> wrote:
> > Hi folks,
> >
> > I'm working on an issue of HyperV synthetic frame buffer driver, which
> > seems to have a conflict with the generic vga driver (not the vesa
> > driver). I hope to read and trace into the source code for the generic vga
> driver...
> >
> > Can anyone point me to the source code file for the generic vga driver in
> the kernel tree?
> 
> Everything lives in ./drivers/video/. The drivers you're probably interested in
> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
> in ./drivers/video/console/vgacon.c. I am not sure which one you are talking
> about.
> 
> You might also want to have a look at the x86 sysfb infrastructure which isn't
> merged, yet:
> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
> It provides proper platform-devices so drivers no longer conflict on the
> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant drivers
> only work on x86.
> 
> If you give some more information on what you are trying to do, I can point
> you to the relevant resources. My guess is that you want to have a look at
> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.

Thank you for the detailed reply!

I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which seems
to have conflict with some kind of generic video driver. I'm not sure which driver
is this.

On Suse, the vesafb is removed automatically by remove_conflicting_framebuffers()
when hyperv_fb is loaded. We don't have any problem here.

On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver seems not
to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver seems not be
removed when hyperv_fb is loaded. This generic video driver is not vesafb or vga16fb or
vgacon, because it supports x-window GUI, and it's still here after I un-configured vesafb
and vga16fb.

Could point out what is the generic video driver used by RHEL, Ubuntu by default? And,
how to let it exit automatically when our FB driver (hyperv_fb) is loaded?

Thanks,
 - Haiyang


^ permalink raw reply

* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 19:11 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <511c70f647a8481d90e8660773512efc@DFM-DB3MBX15-06.exchange.corp.microsoft.com>

Hi

(hint: your mail header seems to drop Reference-to/Reply-to headers
and breaks thread-info)

On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> Sent: Friday, August 16, 2013 9:46 AM
>> To: Haiyang Zhang
>> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> foundation.org; KY Srinivasan
>> Subject: Re: source code file for the generic vga driver?
>>
>> Hi
>>
>> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang <haiyangz@microsoft.com>
>> wrote:
>> > Hi folks,
>> >
>> > I'm working on an issue of HyperV synthetic frame buffer driver, which
>> > seems to have a conflict with the generic vga driver (not the vesa
>> > driver). I hope to read and trace into the source code for the generic vga
>> driver...
>> >
>> > Can anyone point me to the source code file for the generic vga driver in
>> the kernel tree?
>>
>> Everything lives in ./drivers/video/. The drivers you're probably interested in
>> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
>> in ./drivers/video/console/vgacon.c. I am not sure which one you are talking
>> about.
>>
>> You might also want to have a look at the x86 sysfb infrastructure which isn't
>> merged, yet:
>> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
>> It provides proper platform-devices so drivers no longer conflict on the
>> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant drivers
>> only work on x86.
>>
>> If you give some more information on what you are trying to do, I can point
>> you to the relevant resources. My guess is that you want to have a look at
>> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
>
> Thank you for the detailed reply!
>
> I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which seems
> to have conflict with some kind of generic video driver. I'm not sure which driver
> is this.
>
> On Suse, the vesafb is removed automatically by remove_conflicting_framebuffers()
> when hyperv_fb is loaded. We don't have any problem here.
>
> On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver seems not
> to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver seems not be
> removed when hyperv_fb is loaded. This generic video driver is not vesafb or vga16fb or
> vgacon, because it supports x-window GUI, and it's still here after I un-configured vesafb
> and vga16fb.
>
> Could point out what is the generic video driver used by RHEL, Ubuntu by default? And,
> how to let it exit automatically when our FB driver (hyperv_fb) is loaded?

I have no idea what RHEL or Ubuntu use, sorry. But your description
sounds odd. The kernel has 3 different places that could use VGA
resources:
 - fbdev drivers (/dev/fbX or /sys/class/graphics)
 - DRM drivers (/dev/dri/cardX or /sys/class/drm)
 - vgacon
Could you check whether these directories are empty?
(/sys/class/graphics/ and /sys/class/drm/ on a running machine)

If these are empty, this doesn't look like a kernel thing. Are you
sure it's a kernel driver that accesses your VGA resources? The
xserver used to have some pretty huge vga/vesa/.. user-space drivers.

Could you tell me whether the linux-console actually works? That is,
do you get some console output if xserver is not running?

Cheers
David

^ permalink raw reply

* RE: [RFC 1/1] video: da8xx-fb: adding dt support
From: Etheridge, Darren @ 2013-08-16 20:11 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: devicetree@vger.kernel.org, Valkeinen, Tomi,
	plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
	Mohammed, Afzal
In-Reply-To: <CA+V-a8uSd7cypG=zTPyfkdq_xSdxHeGpoRu497U8C-3ZUsfu5w@mail.gmail.com>

From: Prabhakar Lad [mailto:prabhakar.csengg@gmail.com]
> Sent: Thursday, August 08, 2013 11:07 PM
> Subject: Re: [RFC 1/1] video: da8xx-fb: adding dt support
> 
> Hi Darren,
> 
> Thanks for the patch, below are few nits!
> 
Prabhakar,

Thanks for reviewing, somehow I missed your reply so sorry about late response.

> On Fri, Aug 9, 2013 at 1:45 AM, Darren Etheridge <detheridge@ti.com>
> wrote:
> > Enhancing driver to enable probe triggered by a corresponding dt entry.
> >
<snip>
> > +
> > +       cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode),
> GFP_KERNEL);
> > +       if (!cfg) {
> > +               dev_err(&dev->dev, "memory allocation failed\n");
> 
> devm_* api will print the message if allocation has failed, please remove the
> above err statement and also similarly below.

Will fix.

<snip>

> > +               lcdc_info = devm_kzalloc(&dev->dev,
> > +                                        sizeof(struct fb_videomode),
> > +                                        GFP_KERNEL);
> > +               if (!lcdc_info) {
> > +                       dev_err(&dev->dev, "memory allocation
> > + failed\n");
> ditto

Will fix

<snip>

> > @@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device
> *device)
> >         par->dev = &device->dev;
> >         par->lcdc_clk = tmp_lcdc_clk;
> >         par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
> > -       if (fb_pdata->panel_power_ctrl) {
> > +       if (fb_pdata && fb_pdata->panel_power_ctrl) {
> >                 par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
> >                 par->panel_power_ctrl(1);
> 
> What happens to this data in DT case ?
>

Good find,  right now we don't do any power control in the DT case - this is something that needs to be addressed.

>
 > >         }
> > @@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device
> > *dev)  #define fb_resume NULL  #endif
> >
> > +static const struct of_device_id da8xx_fb_of_match[] = {
> > +       /*
> > +        * this driver supports version 1 and version 2 of the
> > +        * Texas Instruments lcd controller (lcdc) hardware block
> > +        */
> > +       {.compatible = "ti,da830-lcdc", },
> > +       {.compatible = "ti,am3352-lcdc", },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
> > +
> you can bound this structure and the macro in IS_ENABLED(CONFIG_OF)

Ok will look at that.

Thanks,
Darren

^ permalink raw reply

* RE: conflict of hyperv_fb and the generic video driver?
From: Haiyang Zhang @ 2013-08-16 20:27 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4SsnQfRNpgcNW12t26MeQvVVS9p136dGTirPaJZKxXo_Q@mail.gmail.com>



> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Friday, August 16, 2013 3:11 PM
> To: Haiyang Zhang
> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> foundation.org; KY Srinivasan
> Subject: Re: conflict of hyperv_fb and the generic video driver?
> 
> Hi
> 
> (hint: your mail header seems to drop Reference-to/Reply-to headers
> and breaks thread-info)
> 
> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> Sent: Friday, August 16, 2013 9:46 AM
> >> To: Haiyang Zhang
> >> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> foundation.org; KY Srinivasan
> >> Subject: Re: source code file for the generic vga driver?
> >>
> >> Hi
> >>
> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
> <haiyangz@microsoft.com>
> >> wrote:
> >> > Hi folks,
> >> >
> >> > I'm working on an issue of HyperV synthetic frame buffer driver, which
> >> > seems to have a conflict with the generic vga driver (not the vesa
> >> > driver). I hope to read and trace into the source code for the generic vga
> >> driver...
> >> >
> >> > Can anyone point me to the source code file for the generic vga driver in
> >> the kernel tree?
> >>
> >> Everything lives in ./drivers/video/. The drivers you're probably interested
> in
> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
> >> in ./drivers/video/console/vgacon.c. I am not sure which one you are
> talking
> >> about.
> >>
> >> You might also want to have a look at the x86 sysfb infrastructure which
> isn't
> >> merged, yet:
> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
> >> It provides proper platform-devices so drivers no longer conflict on the
> >> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant
> drivers
> >> only work on x86.
> >>
> >> If you give some more information on what you are trying to do, I can
> point
> >> you to the relevant resources. My guess is that you want to have a look at
> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
> >
> > Thank you for the detailed reply!
> >
> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which
> seems
> > to have conflict with some kind of generic video driver. I'm not sure which
> driver
> > is this.
> >
> > On Suse, the vesafb is removed automatically by
> remove_conflicting_framebuffers()
> > when hyperv_fb is loaded. We don't have any problem here.
> >
> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver
> seems not
> > to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver
> seems not be
> > removed when hyperv_fb is loaded. This generic video driver is not vesafb
> or vga16fb or
> > vgacon, because it supports x-window GUI, and it's still here after I un-
> configured vesafb
> > and vga16fb.
> >
> > Could point out what is the generic video driver used by RHEL, Ubuntu by
> default? And,
> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
> 
> I have no idea what RHEL or Ubuntu use, sorry. But your description
> sounds odd. The kernel has 3 different places that could use VGA
> resources:
>  - fbdev drivers (/dev/fbX or /sys/class/graphics)
>  - DRM drivers (/dev/dri/cardX or /sys/class/drm)
>  - vgacon
> Could you check whether these directories are empty?
> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
> 
> If these are empty, this doesn't look like a kernel thing. Are you
> sure it's a kernel driver that accesses your VGA resources? The
> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
> 
> Could you tell me whether the linux-console actually works? That is,
> do you get some console output if xserver is not running?

To find out the default driver, I manually removed my hyperv_fb driver.
The vesafb is unconfigured:
# CONFIG_FB_BOOT_VESA_SUPPORT is not set
# CONFIG_FB_UVESA is not set
# CONFIG_FB_VESA is not set

But I saw VESA VBE in the x log. Seems it's the default driver:
"/var/log/Xorg.0.log":
[    12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
[    12.341] (II) VESA(0): VESA BIOS detected
[    12.341] (II) VESA(0): VESA VBE Version 2.0
[    12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
[    12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993 International Business Machines
[    12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
[    12.365] (II) VESA(0): Creating default Display subsection in Screen section
        "Default Screen Section" for depth/fbbpp 24/32
[    12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
[    12.365] (=) VESA(0): RGB weight 888
[    12.365] (=) VESA(0): Default visual is TrueColor
[    12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)

There is no  /dev/fb*,  /dev/dri/, /sys/class/drm
I see /sys/class/graphics/fbcon is here.  But console output is not working.

Seems that the VESA VBE is causing conflict with my driver... Is there
any way to disable VESA VBE driver?

Thanks,
- Haiyang


^ permalink raw reply

* Re: [PATCH v2] efifb: prevent null dereferences by removing unused array indices from dmi_list
From: David Herrmann @ 2013-08-16 20:36 UTC (permalink / raw)
  To: James Bates
  Cc: Peter Jones, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev@vger.kernel.org, linux-kernel, H. Peter Anvin
In-Reply-To: <1376684395.6529.5.camel@hermes>

Hi

On Fri, Aug 16, 2013 at 10:19 PM, James Bates <james.h.bates@gmail.com> wrote:
> On Fri, 2013-08-16 at 15:37 +0200, David Herrmann wrote:
>
> Hi
> (CC'ing hpa)
>
> Yepp, this patch looks good:
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>
> However, I'd like to see some code to prevent this from happening
> again. It isn't really obvious that removing an entry will result in a
> NULL-deref. I am not the maintainer of this code, but I'd really like
> to see a "(dmi_list[i].optname && !strcmp())" check in efifb_setup().
> Otherwise we _will_ run into this again.
>
> Side note: this code got moved to arch/x86/kernel/sysfb_efi.c in the
> x86 tree. I am adding hpa here so he will remember this once Linus
> gets a merge conflict (iff the sysfb changes get merged through the
> x86 tree).
>
> Thanks
> David
>
> Sure thing, here is the patch again, with the extra check in efifb_setup()
> (it turns out one can simply
> switch around the order of the checks: implicitly initialized dmi_list items
> will have base = 0):

You can put such comments below the "---" in your patch so they will
not show up in the commit-message (between commit message and the
diffstat).

> Full patch v2:
>
> the dmi_list array is initialized using gnu designated initializers, and
> therefore
> contains fewer explicitly defined entries as there are elements in it. This
> is because the enum above with M_blabla constants contains more items than
> the designated
> initializer. Those elements not explicitly initialized are implicitly set to
> 0.
>
> Now efifb_setup(), L.322 & L.323, loops through all these array elements,
> and performs
> a strcmp o a field (optname) in each item. For non explicitly initialized
> elements this
> will be a null pointer:
>
>                         for (i = 0; i < M_UNKNOWN; i++) {
>                                 if (!strcmp(this_opt, dmi_list[i].optname)
> &&
>
> On my macbook6,1 the predefined values are for some reason incorrect, and
> most parameters
> are preset correctly by my efi bootloader (elilo). but stride/line_length is
> not detected
> correctly and so I wish to set it explicitly using a
> "videoïifb:stride:2048" command-line
> argument. Because of the above null dereference, an exception (presumably)
> occurs before
> the parsing code (L.333) is ever reached. I say presumably since the mac
> hangs on boot
> without a console, and I can therefore not see any output.
>
> By removing the unused values from the enum, and thus preventing implicitly
> initialized items
> in the dmi_list array, the null dereference does not occur, my customer
> command-line arg is
> parsed correctly, and my console displays correctly.
>
> This patch removes the unused enum values, and also guards against any
> future implicit
> initializing by inverting the check order in the if statement, and checking
> first whether
> dmi_list[i].base is null.
>
> Signed-off-by: James Bates <james.h.bates@yahoo.com>
> ---
> drivers/video/efifb.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 50fe668..161757b 100644
>
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -50,12 +50,9 @@ enum {
> M_MINI_3_1, /* Mac Mini, 3,1th gen */
> M_MINI_4_1, /* Mac Mini, 4,1th gen */
> M_MB, /* MacBook */
> - M_MB_2, /* MacBook, 2nd rev. */
> - M_MB_3, /* MacBook, 3rd rev. */
> M_MB_5_1, /* MacBook, 5th rev. */
> M_MB_6_1, /* MacBook, 6th rev. */
> M_MB_7_1, /* MacBook, 7th rev. */
> - M_MB_SR, /* MacBook, 2nd gen, (Santa Rosa) */
> M_MBA, /* MacBook Air */
> M_MBA_3, /* Macbook Air, 3rd rev */
> M_MBP, /* MacBook Pro */
> @@ -323,8 +320,8 @@ static int __init efifb_setup(char *options)
> if (!*this_opt) continue;
>
>
> for (i = 0; i < M_UNKNOWN; i++) {
> - if (!strcmp(this_opt, dmi_list[i].optname) &&
> -     dmi_list[i].base != 0) {
> + if (dmi_list[i].base != 0 &&

From a quick glance, M_MBA_3 has "base = 0" but is a valid entry. Hm,
I think we can be sure that there will never be a framebuffer at phys
0, so yeah, I am fine with this.

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Regards
David

> +     !strcmp(this_opt, dmi_list[i].optname)) {
> screen_info.lfb_base = dmi_list[i].base;
> screen_info.lfb_linelength = dmi_list[i].stride;
> screen_info.lfb_width = dmi_list[i].width;
> --
>

^ permalink raw reply

* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 20:40 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <c4a33a56e2844e5b8bda5bcfc2535aa4@DFM-DB3MBX15-06.exchange.corp.microsoft.com>

Hi

On Fri, Aug 16, 2013 at 10:27 PM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> Sent: Friday, August 16, 2013 3:11 PM
>> To: Haiyang Zhang
>> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> foundation.org; KY Srinivasan
>> Subject: Re: conflict of hyperv_fb and the generic video driver?
>>
>> Hi
>>
>> (hint: your mail header seems to drop Reference-to/Reply-to headers
>> and breaks thread-info)
>>
>> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> >> Sent: Friday, August 16, 2013 9:46 AM
>> >> To: Haiyang Zhang
>> >> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> >> foundation.org; KY Srinivasan
>> >> Subject: Re: source code file for the generic vga driver?
>> >>
>> >> Hi
>> >>
>> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
>> <haiyangz@microsoft.com>
>> >> wrote:
>> >> > Hi folks,
>> >> >
>> >> > I'm working on an issue of HyperV synthetic frame buffer driver, which
>> >> > seems to have a conflict with the generic vga driver (not the vesa
>> >> > driver). I hope to read and trace into the source code for the generic vga
>> >> driver...
>> >> >
>> >> > Can anyone point me to the source code file for the generic vga driver in
>> >> the kernel tree?
>> >>
>> >> Everything lives in ./drivers/video/. The drivers you're probably interested
>> in
>> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
>> >> in ./drivers/video/console/vgacon.c. I am not sure which one you are
>> talking
>> >> about.
>> >>
>> >> You might also want to have a look at the x86 sysfb infrastructure which
>> isn't
>> >> merged, yet:
>> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
>> >> It provides proper platform-devices so drivers no longer conflict on the
>> >> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant
>> drivers
>> >> only work on x86.
>> >>
>> >> If you give some more information on what you are trying to do, I can
>> point
>> >> you to the relevant resources. My guess is that you want to have a look at
>> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
>> >
>> > Thank you for the detailed reply!
>> >
>> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which
>> seems
>> > to have conflict with some kind of generic video driver. I'm not sure which
>> driver
>> > is this.
>> >
>> > On Suse, the vesafb is removed automatically by
>> remove_conflicting_framebuffers()
>> > when hyperv_fb is loaded. We don't have any problem here.
>> >
>> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver
>> seems not
>> > to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver
>> seems not be
>> > removed when hyperv_fb is loaded. This generic video driver is not vesafb
>> or vga16fb or
>> > vgacon, because it supports x-window GUI, and it's still here after I un-
>> configured vesafb
>> > and vga16fb.
>> >
>> > Could point out what is the generic video driver used by RHEL, Ubuntu by
>> default? And,
>> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
>>
>> I have no idea what RHEL or Ubuntu use, sorry. But your description
>> sounds odd. The kernel has 3 different places that could use VGA
>> resources:
>>  - fbdev drivers (/dev/fbX or /sys/class/graphics)
>>  - DRM drivers (/dev/dri/cardX or /sys/class/drm)
>>  - vgacon
>> Could you check whether these directories are empty?
>> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
>>
>> If these are empty, this doesn't look like a kernel thing. Are you
>> sure it's a kernel driver that accesses your VGA resources? The
>> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
>>
>> Could you tell me whether the linux-console actually works? That is,
>> do you get some console output if xserver is not running?
>
> To find out the default driver, I manually removed my hyperv_fb driver.
> The vesafb is unconfigured:
> # CONFIG_FB_BOOT_VESA_SUPPORT is not set
> # CONFIG_FB_UVESA is not set
> # CONFIG_FB_VESA is not set
>
> But I saw VESA VBE in the x log. Seems it's the default driver:
> "/var/log/Xorg.0.log":
> [    12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
> [    12.341] (II) VESA(0): VESA BIOS detected
> [    12.341] (II) VESA(0): VESA VBE Version 2.0
> [    12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
> [    12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993 International Business Machines
> [    12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
> [    12.365] (II) VESA(0): Creating default Display subsection in Screen section
>         "Default Screen Section" for depth/fbbpp 24/32
> [    12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
> [    12.365] (=) VESA(0): RGB weight 888
> [    12.365] (=) VESA(0): Default visual is TrueColor
> [    12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
>
> There is no  /dev/fb*,  /dev/dri/, /sys/class/drm
> I see /sys/class/graphics/fbcon is here.  But console output is not working.
>
> Seems that the VESA VBE is causing conflict with my driver... Is there
> any way to disable VESA VBE driver?

This is the Xorg vesa driver. There is no way to detect that from the
kernel (at least no sane way). Just uninstall the vesa driver, no one
uses that these days. At least I see no reason why you would use it.
Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver
is loaded before xorg starts and then load xf86-video-fbdev over
xf86-video-vesa.

Regards
David

^ permalink raw reply

* RE: conflict of hyperv_fb and the generic video driver?
From: Haiyang Zhang @ 2013-08-16 23:04 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4S9kwDj_J0tu8pgXOVJhvokEzCFNvs2VTQfxA21GHgfGQ@mail.gmail.com>



> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Friday, August 16, 2013 4:40 PM
> To: Haiyang Zhang
> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> foundation.org; KY Srinivasan
> Subject: Re: conflict of hyperv_fb and the generic video driver?
> 
> Hi
> 
> On Fri, Aug 16, 2013 at 10:27 PM, Haiyang Zhang <haiyangz@microsoft.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> Sent: Friday, August 16, 2013 3:11 PM
> >> To: Haiyang Zhang
> >> Cc: linux-fbdev@vger.kernel.org;
> >> driverdev-devel@linuxdriverproject.org;
> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> foundation.org; KY Srinivasan
> >> Subject: Re: conflict of hyperv_fb and the generic video driver?
> >>
> >> Hi
> >>
> >> (hint: your mail header seems to drop Reference-to/Reply-to headers
> >> and breaks thread-info)
> >>
> >> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang
> >> <haiyangz@microsoft.com>
> >> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> >> Sent: Friday, August 16, 2013 9:46 AM
> >> >> To: Haiyang Zhang
> >> >> Cc: linux-fbdev@vger.kernel.org;
> >> >> driverdev-devel@linuxdriverproject.org;
> >> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> >> foundation.org; KY Srinivasan
> >> >> Subject: Re: source code file for the generic vga driver?
> >> >>
> >> >> Hi
> >> >>
> >> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
> >> <haiyangz@microsoft.com>
> >> >> wrote:
> >> >> > Hi folks,
> >> >> >
> >> >> > I'm working on an issue of HyperV synthetic frame buffer driver,
> >> >> > which seems to have a conflict with the generic vga driver (not
> >> >> > the vesa driver). I hope to read and trace into the source code
> >> >> > for the generic vga
> >> >> driver...
> >> >> >
> >> >> > Can anyone point me to the source code file for the generic vga
> >> >> > driver in
> >> >> the kernel tree?
> >> >>
> >> >> Everything lives in ./drivers/video/. The drivers you're probably
> >> >> interested
> >> in
> >> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
> >> >> in ./drivers/video/console/vgacon.c. I am not sure which one you
> >> >> are
> >> talking
> >> >> about.
> >> >>
> >> >> You might also want to have a look at the x86 sysfb infrastructure
> >> >> which
> >> isn't
> >> >> merged, yet:
> >> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86
> >> >> /fb It provides proper platform-devices so drivers no longer
> >> >> conflict on the vga/vesa/efi.. framebuffer resources. It's x86
> >> >> only as all the relevant
> >> drivers
> >> >> only work on x86.
> >> >>
> >> >> If you give some more information on what you are trying to do, I
> >> >> can
> >> point
> >> >> you to the relevant resources. My guess is that you want to have a
> >> >> look at
> >> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
> >> >
> >> > Thank you for the detailed reply!
> >> >
> >> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb)
> >> > which
> >> seems
> >> > to have conflict with some kind of generic video driver. I'm not
> >> > sure which
> >> driver
> >> > is this.
> >> >
> >> > On Suse, the vesafb is removed automatically by
> >> remove_conflicting_framebuffers()
> >> > when hyperv_fb is loaded. We don't have any problem here.
> >> >
> >> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic
> >> > driver
> >> seems not
> >> > to be vesafb -- I can't see any /dev/fb* there. And, the generic
> >> > video driver
> >> seems not be
> >> > removed when hyperv_fb is loaded. This generic video driver is not
> >> > vesafb
> >> or vga16fb or
> >> > vgacon, because it supports x-window GUI, and it's still here after
> >> > I un-
> >> configured vesafb
> >> > and vga16fb.
> >> >
> >> > Could point out what is the generic video driver used by RHEL,
> >> > Ubuntu by
> >> default? And,
> >> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
> >>
> >> I have no idea what RHEL or Ubuntu use, sorry. But your description
> >> sounds odd. The kernel has 3 different places that could use VGA
> >> resources:
> >>  - fbdev drivers (/dev/fbX or /sys/class/graphics)
> >>  - DRM drivers (/dev/dri/cardX or /sys/class/drm)
> >>  - vgacon
> >> Could you check whether these directories are empty?
> >> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
> >>
> >> If these are empty, this doesn't look like a kernel thing. Are you
> >> sure it's a kernel driver that accesses your VGA resources? The
> >> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
> >>
> >> Could you tell me whether the linux-console actually works? That is,
> >> do you get some console output if xserver is not running?
> >
> > To find out the default driver, I manually removed my hyperv_fb driver.
> > The vesafb is unconfigured:
> > # CONFIG_FB_BOOT_VESA_SUPPORT is not set # CONFIG_FB_UVESA is
> not set
> > # CONFIG_FB_VESA is not set
> >
> > But I saw VESA VBE in the x log. Seems it's the default driver:
> > "/var/log/Xorg.0.log":
> > [    12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
> > [    12.341] (II) VESA(0): VESA BIOS detected
> > [    12.341] (II) VESA(0): VESA VBE Version 2.0
> > [    12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
> > [    12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993
> International Business Machines
> > [    12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
> > [    12.365] (II) VESA(0): Creating default Display subsection in Screen
> section
> >         "Default Screen Section" for depth/fbbpp 24/32
> > [    12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
> > [    12.365] (=) VESA(0): RGB weight 888
> > [    12.365] (=) VESA(0): Default visual is TrueColor
> > [    12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
> >
> > There is no  /dev/fb*,  /dev/dri/, /sys/class/drm I see
> > /sys/class/graphics/fbcon is here.  But console output is not working.
> >
> > Seems that the VESA VBE is causing conflict with my driver... Is there
> > any way to disable VESA VBE driver?
> 
> This is the Xorg vesa driver. There is no way to detect that from the kernel (at
> least no sane way). Just uninstall the vesa driver, no one uses that these days.
> At least I see no reason why you would use it.
> Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver is
> loaded before xorg starts and then load xf86-video-fbdev over xf86-video-
> vesa.

Removing the xorg vesa driver has solved the conflict. 

Also, I found adding a xorg.conf which specifies fbdev can solve the problem too:
[root@localhost ~]# cat /etc/X11/xorg.conf
Section "Device"
    Identifier  "HYPER-V Framebuffer"
    Driver      "fbdev"
EndSection

Thank you SO MUCH for the help!!

- Haiyang


^ permalink raw reply

* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 23:11 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-fbdev@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
	akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <34df20beb0de41f4b52ec4a2177e8f3e@DFM-DB3MBX15-06.exchange.corp.microsoft.com>

Hi

On Sat, Aug 17, 2013 at 1:04 AM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>> > But I saw VESA VBE in the x log. Seems it's the default driver:
>> > "/var/log/Xorg.0.log":
>> > [    12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
>> > [    12.341] (II) VESA(0): VESA BIOS detected
>> > [    12.341] (II) VESA(0): VESA VBE Version 2.0
>> > [    12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
>> > [    12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993
>> International Business Machines
>> > [    12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
>> > [    12.365] (II) VESA(0): Creating default Display subsection in Screen
>> section
>> >         "Default Screen Section" for depth/fbbpp 24/32
>> > [    12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
>> > [    12.365] (=) VESA(0): RGB weight 888
>> > [    12.365] (=) VESA(0): Default visual is TrueColor
>> > [    12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
>> >
>> > There is no  /dev/fb*,  /dev/dri/, /sys/class/drm I see
>> > /sys/class/graphics/fbcon is here.  But console output is not working.
>> >
>> > Seems that the VESA VBE is causing conflict with my driver... Is there
>> > any way to disable VESA VBE driver?
>>
>> This is the Xorg vesa driver. There is no way to detect that from the kernel (at
>> least no sane way). Just uninstall the vesa driver, no one uses that these days.
>> At least I see no reason why you would use it.
>> Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver is
>> loaded before xorg starts and then load xf86-video-fbdev over xf86-video-
>> vesa.
>
> Removing the xorg vesa driver has solved the conflict.
>
> Also, I found adding a xorg.conf which specifies fbdev can solve the problem too:
> [root@localhost ~]# cat /etc/X11/xorg.conf
> Section "Device"
>     Identifier  "HYPER-V Framebuffer"
>     Driver      "fbdev"
> EndSection
>
> Thank you SO MUCH for the help!!

You're welcome. Good to hear it's solved now.

Cheers
David

^ permalink raw reply

* efifb: patch to allow userspace to unbind efi framebuffer driver from VGA device
From: Jonathan Campbell @ 2013-08-19  4:22 UTC (permalink / raw)
  To: linux-fbdev

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

This patch allows userspace to direct efifb to let go of the VGA device 
and unregister it's framebuffer. As far as I can tell, the Linux kernel 
framebuffer console knows to let go when efifb unregisters it's 
framebuffer. The problem I'm trying to solve is that I need efifb so the 
kernel can show it's status on-screen during boot, but then I need efifb 
to step aside and let a better driver load and take the VGA device later 
during boot up.

The custom Linux distribution I've made for myself uses a userspace 
program to "boot" secondary VGA devices and load both fbdev and drm/kms 
drivers to bring video online. When I wrote this, I needed the ability 
for efifb to let go so that it could load bochsfb to better use the VGA 
output of VirtualBox and bochs. Without it, my console is stuck at 
whatever video mode the UEFI BIOS happened to leave it at and the more 
specialized driver cannot acquire the resources it needs to do it's work.

The patch adds a sysfs device file "bind_fb" to the 
/sys/bus/platform/devices/efifb.0 filesystem tree. Writing "1" causes it 
to re-register the framebuffer, "0" to unregister. Checks are in place 
to not register the framebuffer if already registered, etc.

I realize it's not perfect and I wanted to know what I could do to 
improve it and eventually make it to mainline.

On a related issue, when will efifb make use of the Graphics Output 
Protocol through UEFI to offer modesetting? Is that possible? If no 
specialized drivers are available it'd be nice to at least have basic 
modesetting as needed.


[-- Attachment #2: efifb-bind-ctl.patch --]
[-- Type: text/plain, Size: 5950 bytes --]

--- a/drivers/video/efifb.c	2013-08-15 05:59:42.000000000 +0000
+++ b/drivers/video/efifb.c	2013-08-17 23:56:56.929260269 +0000
@@ -18,7 +18,8 @@
 
 static bool request_mem_succeeded = false;
 
-static struct pci_dev *default_vga;
+static struct pci_dev *default_vga = NULL;
+static struct fb_info *efifb_info = NULL;
 
 static struct fb_var_screeninfo efifb_defined = {
 	.activate		= FB_ACTIVATE_NOW,
@@ -86,7 +87,7 @@
 	int width;
 	int height;
 	int flags;
-} dmi_list[] __initdata = {
+} dmi_list[] = {
 	[M_I17] = { "i17", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
 	[M_I20] = { "i20", 0x80010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE }, /* guess */
 	[M_I20_SR] = { "imac7", 0x40010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE },
@@ -127,7 +128,7 @@
 		DMI_MATCH(DMI_PRODUCT_NAME, name) },		\
 	  &dmi_list[enumid] }
 
-static const struct dmi_system_id dmi_system_table[] __initconst = {
+static const struct dmi_system_id dmi_system_table[] = {
 	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac4,1", M_I17),
 	/* At least one of these two will be right; maybe both? */
 	EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac5,1", M_I20),
@@ -288,6 +289,7 @@
 	if (request_mem_succeeded)
 		release_mem_region(info->apertures->ranges[0].base,
 				   info->apertures->ranges[0].size);
+	fb_dealloc_cmap(&info->cmap);
 	framebuffer_release(info);
 }
 
@@ -312,7 +314,7 @@
 	default_vga = pdev;
 }
 
-static int __init efifb_setup(char *options)
+static int efifb_setup(char *options)
 {
 	char *this_opt;
 	int i;
@@ -369,9 +371,8 @@
 	return 0;
 }
 
-static int __init efifb_probe(struct platform_device *dev)
+static int efifb_probe(struct platform_device *dev)
 {
-	struct fb_info *info;
 	int err;
 	unsigned int size_vmode;
 	unsigned int size_remap;
@@ -438,25 +439,25 @@
 			efifb_fix.smem_start);
 	}
 
-	info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
-	if (!info) {
+	efifb_info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
+	if (!efifb_info) {
 		printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
 		err = -ENOMEM;
 		goto err_release_mem;
 	}
-	info->pseudo_palette = info->par;
-	info->par = NULL;
+	efifb_info->pseudo_palette = efifb_info->par;
+	efifb_info->par = NULL;
 
-	info->apertures = alloc_apertures(1);
-	if (!info->apertures) {
+	efifb_info->apertures = alloc_apertures(1);
+	if (!efifb_info->apertures) {
 		err = -ENOMEM;
 		goto err_release_fb;
 	}
-	info->apertures->ranges[0].base = efifb_fix.smem_start;
-	info->apertures->ranges[0].size = size_remap;
+	efifb_info->apertures->ranges[0].base = efifb_fix.smem_start;
+	efifb_info->apertures->ranges[0].size = size_remap;
 
-	info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
-	if (!info->screen_base) {
+	efifb_info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
+	if (!efifb_info->screen_base) {
 		printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
 				"0x%x @ 0x%lx\n",
 			efifb_fix.smem_len, efifb_fix.smem_start);
@@ -466,7 +467,7 @@
 
 	printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
 	       "using %dk, total %dk\n",
-	       efifb_fix.smem_start, info->screen_base,
+	       efifb_fix.smem_start, efifb_info->screen_base,
 	       size_remap/1024, size_total/1024);
 	printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
 	       efifb_defined.xres, efifb_defined.yres,
@@ -509,29 +510,29 @@
 	efifb_fix.ypanstep  = 0;
 	efifb_fix.ywrapstep = 0;
 
-	info->fbops = &efifb_ops;
-	info->var = efifb_defined;
-	info->fix = efifb_fix;
-	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
+	efifb_info->fbops = &efifb_ops;
+	efifb_info->var = efifb_defined;
+	efifb_info->fix = efifb_fix;
+	efifb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
 
-	if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
+	if ((err = fb_alloc_cmap(&efifb_info->cmap, 256, 0)) < 0) {
 		printk(KERN_ERR "efifb: cannot allocate colormap\n");
 		goto err_unmap;
 	}
-	if ((err = register_framebuffer(info)) < 0) {
+	if ((err = register_framebuffer(efifb_info)) < 0) {
 		printk(KERN_ERR "efifb: cannot register framebuffer\n");
 		goto err_fb_dealoc;
 	}
 	printk(KERN_INFO "fb%d: %s frame buffer device\n",
-		info->node, info->fix.id);
+		efifb_info->node, efifb_info->fix.id);
 	return 0;
 
 err_fb_dealoc:
-	fb_dealloc_cmap(&info->cmap);
+	fb_dealloc_cmap(&efifb_info->cmap);
 err_unmap:
-	iounmap(info->screen_base);
+	iounmap(efifb_info->screen_base);
 err_release_fb:
-	framebuffer_release(info);
+	framebuffer_release(efifb_info);
 err_release_mem:
 	if (request_mem_succeeded)
 		release_mem_region(efifb_fix.smem_start, size_total);
@@ -548,6 +549,38 @@
 	.name	= "efifb",
 };
 
+/* use a sysfs file "trace_running" to start/stop tracing */
+static ssize_t efifb_bind_fb_attr_show(struct device *dev,struct device_attribute *attr,char *buf)
+{
+	return sprintf(buf, "%u\n", efifb_info != NULL ? 1 : 0);
+}
+
+static ssize_t efifb_bind_fb_attr_store(struct device *dev,struct device_attribute *attr,const char *buf,size_t n)
+{
+	unsigned int value;
+	int ret = 0;
+
+	if (sscanf(buf, "%u", &value) != 1)
+		return -EINVAL;
+	if (value > 1)
+		return -EINVAL;
+
+	if ((efifb_info != NULL ? 1 : 0) != value) {
+		if (value)
+			ret = efifb_probe(&efifb_device);
+		else {
+			ret = unregister_framebuffer(efifb_info);
+			if (ret >= 0) efifb_info = NULL;
+		}
+	}
+
+	return (ret < 0 ? ret : n);
+}
+
+static const struct device_attribute efifb_bind_fb_attr =
+	__ATTR(bind_fb, 0644, efifb_bind_fb_attr_show, efifb_bind_fb_attr_store);
+
+
 static int __init efifb_init(void)
 {
 	int ret;
@@ -575,6 +608,11 @@
 	if (ret)
 		return ret;
 
+	/* create "bind_fb" sysfs file */
+	if (device_create_file(&efifb_device.dev,&efifb_bind_fb_attr))
+		printk(KERN_WARNING
+		       "efifb: cannot create bind_fb attribute\n");
+
 	/*
 	 * This is not just an optimization.  We will interfere
 	 * with a real driver if we get reprobed, so don't allow

^ permalink raw reply

* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-19  5:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <520B9C9E.8010002@ti.com>

Felipe,

ping..

On Wednesday 14 August 2013 08:35 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 14 August 2013 04:34 AM, Tomasz Figa wrote:
>> On Wednesday 14 of August 2013 00:19:28 Sylwester Nawrocki wrote:
>>> W dniu 2013-08-13 14:05, Kishon Vijay Abraham I pisze:
>>>> On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:
>>>>> On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:
>>>>>> On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
>>>>>>> On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I 
>> wrote:
>>>>>>>>>>>>> IMHO we need a lookup method for PHYs, just like for clocks,
>>>>>>>>>>>>> regulators, PWMs or even i2c busses because there are complex
>>>>>>>>>>>>> cases
>>>>>>>>>>>>> when passing just a name using platform data will not work. I
>>>>>>>>>>>>> would
>>>>>>>>>>>>> second what Stephen said [1] and define a structure doing
>>>>>>>>>>>>> things
>>>>>>>>>>>>> in a
>>>>>>>>>>>>> DT-like way.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Example;
>>>>>>>>>>>>>
>>>>>>>>>>>>> [platform code]
>>>>>>>>>>>>>
>>>>>>>>>>>>> static const struct phy_lookup my_phy_lookup[] = {
>>>>>>>>>>>>>
>>>>>>>>>>>>> 	PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1",
>>>>>>>>>>>>> 	"phy.2"),
>>>>>>>>>>>>
>>>>>>>>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used
>>>>>>>>>>>> while
>>>>>>>>>>>> creating the device, the ids in the device name would change
>>>>>>>>>>>> and
>>>>>>>>>>>> PHY_LOOKUP wont be useful.
>>>>>>>>>>>
>>>>>>>>>>> I don't think this is a problem. All the existing lookup
>>>>>>>>>>> methods
>>>>>>>>>>> already
>>>>>>>>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c,
>>>>>>>>>>> ...). You
>>>>>>>>>>> can simply add a requirement that the ID must be assigned
>>>>>>>>>>> manually,
>>>>>>>>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
>>>>>>>>>>
>>>>>>>>>> And I'm saying that this idea, of using a specific name and id,
>>>>>>>>>> is
>>>>>>>>>> frought with fragility and will break in the future in various
>>>>>>>>>> ways
>>>>>>>>>> when
>>>>>>>>>> devices get added to systems, making these strings constantly
>>>>>>>>>> have
>>>>>>>>>> to be
>>>>>>>>>> kept up to date with different board configurations.
>>>>>>>>>>
>>>>>>>>>> People, NEVER, hardcode something like an id.  The fact that
>>>>>>>>>> this
>>>>>>>>>> happens today with the clock code, doesn't make it right, it
>>>>>>>>>> makes
>>>>>>>>>> the
>>>>>>>>>> clock code wrong.  Others have already said that this is wrong
>>>>>>>>>> there
>>>>>>>>>> as
>>>>>>>>>> well, as systems change and dynamic ids get used more and more.
>>>>>>>>>>
>>>>>>>>>> Let's not repeat the same mistakes of the past just because we
>>>>>>>>>> refuse to
>>>>>>>>>> learn from them...
>>>>>>>>>>
>>>>>>>>>> So again, the "find a phy by a string" functions should be
>>>>>>>>>> removed,
>>>>>>>>>> the
>>>>>>>>>> device id should be automatically created by the phy core just
>>>>>>>>>> to
>>>>>>>>>> make
>>>>>>>>>> things unique in sysfs, and no driver code should _ever_ be
>>>>>>>>>> reliant
>>>>>>>>>> on
>>>>>>>>>> the number that is being created, and the pointer to the phy
>>>>>>>>>> structure
>>>>>>>>>> should be used everywhere instead.
>>>>>>>>>>
>>>>>>>>>> With those types of changes, I will consider merging this
>>>>>>>>>> subsystem,
>>>>>>>>>> but
>>>>>>>>>> without them, sorry, I will not.
>>>>>>>>>
>>>>>>>>> I'll agree with Greg here, the very fact that we see people
>>>>>>>>> trying to
>>>>>>>>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already
>>>>>>>>> points
>>>>>>>>> to a big problem in the framework.
>>>>>>>>>
>>>>>>>>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will
>>>>>>>>> end up
>>>>>>>>> adding similar infrastructure to the driver themselves to make
>>>>>>>>> sure
>>>>>>>>> we
>>>>>>>>> don't end up with duplicate names in sysfs in case we have
>>>>>>>>> multiple
>>>>>>>>> instances of the same IP in the SoC (or several of the same PCIe
>>>>>>>>> card).
>>>>>>>>> I really don't want to go back to that.
>>>>>>>>
>>>>>>>> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we
>>>>>>>> can
>>>>>>>> give the correct binding information to the PHY framework. I think
>>>>>>>> we
>>>>>>>> can drop having this non-dt support in PHY framework? I see only
>>>>>>>> one
>>>>>>>> platform (OMAP3) going to be needing this non-dt support and we
>>>>>>>> can
>>>>>>>> use the USB PHY library for it.>
>>>>>>>
>>>>>>> you shouldn't drop support for non-DT platform, in any case we
>>>>>>> lived
>>>>>>> without DT (and still do) for years. Gotta find a better way ;-)
>>>>>>
>>>>>> hmm..
>>>>>>
>>>>>> how about passing the device names of PHY in platform data of the
>>>>>> controller? It should be deterministic as the PHY framework assigns
>>>>>> its
>>>>>> own id and we *don't* want to add any requirement that the ID must
>>>>>> be
>>>>>> assigned manually without using PLATFORM_DEVID_AUTO. We can get rid
>>>>>> of
>>>>>> *phy_init_data* in the v10 patch series.
>>>
>>> OK, so the PHY device name would have a fixed part, passed as
>>> platform data of the controller and a variable part appended
>>> by the PHY core, depending on the number of registered PHYs ?
>>>
>>> Then same PHY names would be passed as the PHY provider driver's
>>> platform data ?
>>>
>>> Then if there are 2 instances of the above (same names in platform
>>> data) how would be determined which PHY controller is linked to
>>> which PHY supplier ?
>>>
>>> I guess you want each device instance to have different PHY device
>>> names already in platform data ? That might work. We probably will
>>> be focused mostly on DT anyway. It seem without DT we are trying
>>> to find some layer that would allow us to couple relevant devices
>>> and overcome driver core inconvenience that it provides to means
>>> to identify specific devices in advance. :) Your proposal sounds
>>> reasonable, however I might be missing some details or corner cases.
>>>
>>>>> What about slightly altering the concept of v9 to pass a pointer to
>>>>> struct device instead of device name inside phy_init_data?
>>>
>>> As Felipe said, we don't want to pass pointers in platform_data
>>> to/from random subsystems. We pass data, passing pointers would
>>> be a total mess IMHO.
>>
>> Well, this is a total mess anyway... I don't really get the point of using 
>> PLATFORM_DEVID_AUTO. The only thing that comes to my mind is that you can 
>> use it if you don't care about the ID and so it can be assigned 
>> automatically.
>>
>> However my understanding of the device ID is that it was supposed to 
>> provide a way to identify multiple instances of identical devices in a 
>> reliable way, to solve problems like the one we are trying to solve 
>> here...
>>
>> So maybe let's stop solving an already solved problem and just state that 
>> you need to explicitly assign device ID to use this framework?
> 
> Felipe,
> Can we have it the way I had in my v10 patch series till we find a better way?
> I think this *non-dt* stuff shouldn't be blocking as most of the users are dt only?
> 
> Thanks
> Kishon
> 


^ permalink raw reply

* Re: efifb: patch to allow userspace to unbind efi framebuffer driver from VGA device
From: Bruno Prémont @ 2013-08-19  5:51 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <52119D6D.5070209@castus.tv>

Hi Jonathan,

On Sun, 18 Aug 2013 21:22:05 -0700 Jonathan Campbell wrote:
> This patch allows userspace to direct efifb to let go of the VGA device 
> and unregister it's framebuffer. As far as I can tell, the Linux kernel 
> framebuffer console knows to let go when efifb unregisters it's 
> framebuffer. The problem I'm trying to solve is that I need efifb so the 
> kernel can show it's status on-screen during boot, but then I need efifb 
> to step aside and let a better driver load and take the VGA device later 
> during boot up.
> 
> The custom Linux distribution I've made for myself uses a userspace 
> program to "boot" secondary VGA devices and load both fbdev and drm/kms 
> drivers to bring video online. When I wrote this, I needed the ability 
> for efifb to let go so that it could load bochsfb to better use the VGA 
> output of VirtualBox and bochs. Without it, my console is stuck at 
> whatever video mode the UEFI BIOS happened to leave it at and the more 
> specialized driver cannot acquire the resources it needs to do it's work.
> 
> The patch adds a sysfs device file "bind_fb" to the 
> /sys/bus/platform/devices/efifb.0 filesystem tree. Writing "1" causes it 
> to re-register the framebuffer, "0" to unregister. Checks are in place 
> to not register the framebuffer if already registered, etc.

That is the wrong approach.

On fbdev subsystem there is infrastructure for replacing generic
firmware drivers with specialized drivers (as is called by KMS
drivers).

Have a look at remove_conflicting_framebuffers() and its use by KMS
drivers (i915, radeon, nouveau, mgag200, cirrus).

Unloading efifb should be able to happen automatically when
loading/probing new driver, without userspace help (so it can work when
both are built-in).

Bruno


> I realize it's not perfect and I wanted to know what I could do to 
> improve it and eventually make it to mainline.
> 
> On a related issue, when will efifb make use of the Graphics Output 
> Protocol through UEFI to offer modesetting? Is that possible? If no 
> specialized drivers are available it'd be nice to at least have basic 
> modesetting as needed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox