Shifted framebuffer after X11 starts on mx28

Shifted framebuffer after X11 starts on mx28

[Fabio Estevam]

Hi,

I am running 3.8.2 kernel on a mx28evk and when I start X11 I get a
"shifted" framebuffer.

The framebuffer works well if X11 is not called.

There is no issue with X11 when running with Freescale  2.6.35 kernel.

Looking at drivers/video/mxsfb.c there is the following comment:

	/*
	 * It seems, you can't re-program the controller if it is still running.
	 * This may lead into shifted pictures (FIFO issue?).
	 * So, first stop the controller and drain its FIFOs
	 */

So, it looks like that I am still facing this issue when X11 starts.

If anyone has any suggestions, please let me know.

Thanks,

Fabio Estevam

Re: Shifted framebuffer after X11 starts on mx28

[Marek Vasut]

Dear Fabio Estevam,

> Hi,
> 
> I am running 3.8.2 kernel on a mx28evk and when I start X11 I get a
> "shifted" framebuffer.
> 
> The framebuffer works well if X11 is not called.
> 
> There is no issue with X11 when running with Freescale  2.6.35 kernel.
> 
> Looking at drivers/video/mxsfb.c there is the following comment:
> 
> 	/*
> 	 * It seems, you can't re-program the controller if it is still running.
> 	 * This may lead into shifted pictures (FIFO issue?).
> 	 * So, first stop the controller and drain its FIFOs
> 	 */
> 
> So, it looks like that I am still facing this issue when X11 starts.
> 
> If anyone has any suggestions, please let me know.

This is not it, I almost hunted the bug down. Hang on.

Basically check mxsfb_set_par() and observe fb_info->var.sync . It's set to 
"something" during regular framebuffer operation and it's set to "something 
else" during Xorg operation. And that's what causes it to get broken. I forced 
fb_info->var.sync to the "something" value by hardcoding it in kernel just now 
for a quick test and X works for me.

Best regards,
Marek Vasut

Re: Shifted framebuffer after X11 starts on mx28

[Marek Vasut]

Dear Marek Vasut,

> Dear Fabio Estevam,
> 
> > Hi,
> > 
> > I am running 3.8.2 kernel on a mx28evk and when I start X11 I get a
> > "shifted" framebuffer.
> > 
> > The framebuffer works well if X11 is not called.
> > 
> > There is no issue with X11 when running with Freescale  2.6.35 kernel.
> > 
> > Looking at drivers/video/mxsfb.c there is the following comment:
> > 	/*
> > 	
> > 	 * It seems, you can't re-program the controller if it is still running.
> > 	 * This may lead into shifted pictures (FIFO issue?).
> > 	 * So, first stop the controller and drain its FIFOs
> > 	 */
> > 
> > So, it looks like that I am still facing this issue when X11 starts.
> > 
> > If anyone has any suggestions, please let me know.
> 
> This is not it, I almost hunted the bug down. Hang on.
> 
> Basically check mxsfb_set_par() and observe fb_info->var.sync . It's set to
> "something" during regular framebuffer operation and it's set to "something
> else" during Xorg operation. And that's what causes it to get broken. I
> forced fb_info->var.sync to the "something" value by hardcoding it in
> kernel just now for a quick test and X works for me.

Uh, being in hacking-mode and replying to email doesn't work well together :-)

In my case, fb_info->var.sync is set to FB_SYNC_DATA_ENABLE_HIGH_ACT during 
regular operation (framebuffer console), but if X11 starts, this fb_info-
>var.sync is forced to value 0 which causes the malfunction. Now back into 
hacking-mode ;-)

Best regards,
Marek Vasut

[PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

The issue fixed by this patch manifests only then using X11
with mxsfb driver. The X11 will display either shifted image
or otherwise distorted image on the LCD.

The problem is that the X11 tries to reconfigure the framebuffer
and along the way call fb_ops.fb_set_par() with it's configuration
values. The field of particular interest is fb_info->var.sync which
contains non-standard values if configured by kernel. These are
FB_SYNC_DATA_ENABLE_HIGH_ACT and FB_SYNC_DOTCLK_FAILING_ACT defined
in include/linux/mxsfb.h . The driver interprets those and configures
the LCD controller accordingly. Yet X11 only has access to standard
values for this field defined in include/uapi/linux/fb.h and thus
omits these special values. This results in distorted image on the
LCD.

This patch moves these non-standard values into new field of the
mxsfb_platform_data structure so the driver can in turn check this
field instead of the video mode field for these specific portions.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estavem@freescale.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
Cc: Lothar Waßmann <LW@karo-electronics.de>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/mach-mxs/mach-mxs.c |   22 +++++++++++-----------
 drivers/video/mxsfb.c        |    7 +++++--
 include/linux/mxsfb.h        |    7 +++++--
 3 files changed, 21 insertions(+), 15 deletions(-)

NOTE: We ought to get rid of this platform data goo, but we can't do
      it in here. This is a bugfix.

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index c66129b..ebf592c 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -41,8 +41,6 @@ static struct fb_videomode mx23evk_video_modes[] = {
 		.lower_margin	= 4,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -59,8 +57,6 @@ static struct fb_videomode mx28evk_video_modes[] = {
 		.lower_margin	= 10,
 		.hsync_len	= 10,
 		.vsync_len	= 10,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -77,7 +73,6 @@ static struct fb_videomode m28evk_video_modes[] = {
 		.lower_margin	= 45,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT,
 	},
 };
 
@@ -94,9 +89,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
 		.lower_margin	= 13,
 		.hsync_len	= 48,
 		.vsync_len	= 3,
-		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -113,9 +106,7 @@ static struct fb_videomode apf28dev_video_modes[] = {
 		.lower_margin = 0x15,
 		.hsync_len = 64,
 		.vsync_len = 4,
-		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -250,6 +241,8 @@ static void __init imx23_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx23evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static inline void enable_clk_enet_out(void)
@@ -269,6 +262,8 @@ static void __init imx28_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx28evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 
 	mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
 }
@@ -288,6 +283,7 @@ static void __init m28evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
 }
 
 static void __init sc_sps1_init(void)
@@ -313,6 +309,8 @@ static void __init apx4devkit_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apx4devkit_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 #define ENET0_MDC__GPIO_4_0	MXS_GPIO_NR(4, 0)
@@ -403,6 +401,8 @@ static void __init apf28_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apf28dev_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_16BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static void __init mxs_machine_init(void)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 755556c..45169cb 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -169,6 +169,7 @@ struct mxsfb_info {
 	unsigned dotclk_delay;
 	const struct mxsfb_devdata *devdata;
 	int mapped;
+	u32 sync;
 };
 
 #define mxsfb_is_v3(host) (host->devdata->ipversion = 3)
@@ -456,9 +457,9 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
 	if (fb_info->var.sync & FB_SYNC_VERT_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
+	if (host->sync & MXSFB_SYNC_DATA_ENABLE_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
+	if (host->sync & MXSFB_SYNC_DOTCLK_FAILING_ACT)
 		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
 
 	writel(vdctrl0, host->base + LCDC_VDCTRL0);
@@ -861,6 +862,8 @@ static int mxsfb_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&fb_info->modelist);
 
+	host->sync = pdata->sync;
+
 	ret = mxsfb_init_fbinfo(host);
 	if (ret != 0)
 		goto error_init_fb;
diff --git a/include/linux/mxsfb.h b/include/linux/mxsfb.h
index f14943d..f80af86 100644
--- a/include/linux/mxsfb.h
+++ b/include/linux/mxsfb.h
@@ -24,8 +24,8 @@
 #define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
 #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
 
-#define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
+#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
+#define MXSFB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
 
 struct mxsfb_platform_data {
 	struct fb_videomode *mode_list;
@@ -44,6 +44,9 @@ struct mxsfb_platform_data {
 				 * allocated. If specified,fb_size must also be specified.
 				 * fb_phys must be unused by Linux.
 				 */
+	u32 sync;		/* sync mask, contains MXSFB specifics not
+				 * carried in fb_info->var.sync
+				 */
 };
 
 #endif /* __LINUX_MXSFB_H */
-- 
1.7.10.4

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Fabio Estevam]

On Sat, Mar 16, 2013 at 7:06 PM, Marek Vasut <marex@denx.de> wrote:
> The issue fixed by this patch manifests only then using X11
> with mxsfb driver. The X11 will display either shifted image
> or otherwise distorted image on the LCD.
>
> The problem is that the X11 tries to reconfigure the framebuffer
> and along the way call fb_ops.fb_set_par() with it's configuration
> values. The field of particular interest is fb_info->var.sync which
> contains non-standard values if configured by kernel. These are
> FB_SYNC_DATA_ENABLE_HIGH_ACT and FB_SYNC_DOTCLK_FAILING_ACT defined
> in include/linux/mxsfb.h . The driver interprets those and configures
> the LCD controller accordingly. Yet X11 only has access to standard
> values for this field defined in include/uapi/linux/fb.h and thus
> omits these special values. This results in distorted image on the
> LCD.
>
> This patch moves these non-standard values into new field of the
> mxsfb_platform_data structure so the driver can in turn check this
> field instead of the video mode field for these specific portions.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estavem@freescale.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Lothar Waßmann <LW@karo-electronics.de>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>

This fixes the X11 offset issue on my mx28evk, thanks!

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Sat, Mar 16, 2013 at 11:06:21PM +0100, Marek Vasut wrote:
> The issue fixed by this patch manifests only then using X11
> with mxsfb driver. The X11 will display either shifted image
> or otherwise distorted image on the LCD.
> 
> The problem is that the X11 tries to reconfigure the framebuffer
> and along the way call fb_ops.fb_set_par() with it's configuration
> values. The field of particular interest is fb_info->var.sync which
> contains non-standard values if configured by kernel. These are
> FB_SYNC_DATA_ENABLE_HIGH_ACT and FB_SYNC_DOTCLK_FAILING_ACT defined
> in include/linux/mxsfb.h . The driver interprets those and configures
> the LCD controller accordingly. Yet X11 only has access to standard
> values for this field defined in include/uapi/linux/fb.h and thus
> omits these special values. This results in distorted image on the
> LCD.
> 
> This patch moves these non-standard values into new field of the
> mxsfb_platform_data structure so the driver can in turn check this
> field instead of the video mode field for these specific portions.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estavem@freescale.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Lothar Waßmann <LW@karo-electronics.de>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> ---
>  arch/arm/mach-mxs/mach-mxs.c |   22 +++++++++++-----------
>  drivers/video/mxsfb.c        |    7 +++++--
>  include/linux/mxsfb.h        |    7 +++++--
>  3 files changed, 21 insertions(+), 15 deletions(-)

Since it touches mach-mxs as well, let me send it through arm-soc tree
as fix for 3.9.

Shawn

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Fabio Estevam]

Shawn Guo wrote:

> Since it touches mach-mxs as well, let me send it through arm-soc tree
> as fix for 3.9.

Please CC stable as well, so that we can have X11 functional with 3.8.

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Mon, Mar 18, 2013 at 10:11:47AM -0300, Fabio Estevam wrote:
> Shawn Guo wrote:
> 
> > Since it touches mach-mxs as well, let me send it through arm-soc tree
> > as fix for 3.9.
> 
> Please CC stable as well, so that we can have X11 functional with 3.8.

Ok, done.

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Dear Shawn Guo,

> On Mon, Mar 18, 2013 at 10:11:47AM -0300, Fabio Estevam wrote:
> > Shawn Guo wrote:
> > > Since it touches mach-mxs as well, let me send it through arm-soc tree
> > > as fix for 3.9.
> > 
> > Please CC stable as well, so that we can have X11 functional with 3.8.
> 
> Ok, done.

Shawn, I think this patch applies to 3.9-rc but there's one more platform in 
mach-mxs.c in 3.9-rc thus this will have compile issue. Just renaming those two 
FB_SYNC_ values to MXSFB_SYNC_ fixes the compile issue on 3.9-rc. Also the 
commit log doesn't look all that cool .... I probably should have marked the 
patch as RFC.

Shall I send you one for 3.9-rc and one for stable maybe ?

Best regards,
Marek Vasut

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Mon, Mar 18, 2013 at 02:58:17PM +0100, Marek Vasut wrote:
> Dear Shawn Guo,
> 
> > On Mon, Mar 18, 2013 at 10:11:47AM -0300, Fabio Estevam wrote:
> > > Shawn Guo wrote:
> > > > Since it touches mach-mxs as well, let me send it through arm-soc tree
> > > > as fix for 3.9.
> > > 
> > > Please CC stable as well, so that we can have X11 functional with 3.8.
> > 
> > Ok, done.
> 
> Shawn, I think this patch applies to 3.9-rc but there's one more platform in 
> mach-mxs.c in 3.9-rc thus this will have compile issue. Just renaming those two 
> FB_SYNC_ values to MXSFB_SYNC_ fixes the compile issue on 3.9-rc.

Yes, I noticed that and fixed it when applying the patch.

> Also the 
> commit log doesn't look all that cool .... I probably should have marked the 
> patch as RFC.
> 
> Shall I send you one for 3.9-rc and one for stable maybe ?

Yeah, improved patch is welcomed.  So I'm dropping the current one and
waiting for the new one.

Shawn

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Dear Shawn Guo,

> On Mon, Mar 18, 2013 at 02:58:17PM +0100, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> > > On Mon, Mar 18, 2013 at 10:11:47AM -0300, Fabio Estevam wrote:
> > > > Shawn Guo wrote:
> > > > > Since it touches mach-mxs as well, let me send it through arm-soc
> > > > > tree as fix for 3.9.
> > > > 
> > > > Please CC stable as well, so that we can have X11 functional with
> > > > 3.8.
> > > 
> > > Ok, done.
> > 
> > Shawn, I think this patch applies to 3.9-rc but there's one more platform
> > in mach-mxs.c in 3.9-rc thus this will have compile issue. Just renaming
> > those two FB_SYNC_ values to MXSFB_SYNC_ fixes the compile issue on
> > 3.9-rc.
> 
> Yes, I noticed that and fixed it when applying the patch.
> 
> > Also the
> > commit log doesn't look all that cool .... I probably should have marked
> > the patch as RFC.
> > 
> > Shall I send you one for 3.9-rc and one for stable maybe ?
> 
> Yeah, improved patch is welcomed.  So I'm dropping the current one and
> waiting for the new one.

Perfect, latest tomorrow then.

Did you manage to test it please?

Best regards,
Marek Vasut

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> Did you manage to test it please?

I do not have X11 environment handy to test, but framework console
seems still working.

Shawn

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Dear Shawn Guo,

> On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> > Did you manage to test it please?
> 
> I do not have X11 environment handy to test, but framework console
> seems still working.

Even after me poking in it, lol :-)

You can grab [1], extract it onto card or NFS and just use it as root 
filesystem, the X11 will come up automagically.

[1] ftp://ftp.denx.de/pub/eldk/5.3/targets/armv5te/core-image-sato-sdk-generic-
armv5te.tar.gz

Best regards,
Marek Vasut

[PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

The issue fixed by this patch manifests only then using X11
with mxsfb driver. The X11 will display either shifted image
or otherwise distorted image on the LCD.

The problem is that the X11 tries to reconfigure the framebuffer
and along the way calls fb_ops.fb_set_par() with X11's desired
configuration values. The field of particular interest is
fb_info->var.sync which contains non-standard values if
configured by kernel. These are either FB_SYNC_DATA_ENABLE_HIGH_ACT,
FB_SYNC_DOTCLK_FAILING_ACT or both, depending on the platform
configuration. Both of these values are defined in the
include/linux/mxsfb.h file.

The driver interprets these values and configures the LCD controller
accordingly. Yet X11 only has access to the standard values for this
field defined in include/uapi/linux/fb.h and thus, unlike kernel,
omits these special values. This results in distorted image on the
LCD.

This patch moves these non-standard values into new field of the
mxsfb_platform_data structure so the driver can in turn check this
field instead of the video mode field for these specific portions.

Moreover, this patch prefixes these values with MXSFB_SYNC_ prefix
instead of FB_SYNC_ prefix to prevent confusion of subsequent users.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
Cc: Lothar Waßmann <LW@karo-electronics.de>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/mach-mxs/mach-mxs.c |   22 +++++++++++-----------
 drivers/video/mxsfb.c        |    7 +++++--
 include/linux/mxsfb.h        |    7 +++++--
 3 files changed, 21 insertions(+), 15 deletions(-)

NOTE: This is the version for stable v3.8.3, so I'm sending it to -stable.
      I will send adjusted version for mainline 3.9-rc , since there is
      one more board in mainline and therefore the versions of the patch
      must differ.

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index c66129b..ebf592c 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -41,8 +41,6 @@ static struct fb_videomode mx23evk_video_modes[] = {
 		.lower_margin	= 4,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -59,8 +57,6 @@ static struct fb_videomode mx28evk_video_modes[] = {
 		.lower_margin	= 10,
 		.hsync_len	= 10,
 		.vsync_len	= 10,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -77,7 +73,6 @@ static struct fb_videomode m28evk_video_modes[] = {
 		.lower_margin	= 45,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT,
 	},
 };
 
@@ -94,9 +89,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
 		.lower_margin	= 13,
 		.hsync_len	= 48,
 		.vsync_len	= 3,
-		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -113,9 +106,7 @@ static struct fb_videomode apf28dev_video_modes[] = {
 		.lower_margin = 0x15,
 		.hsync_len = 64,
 		.vsync_len = 4,
-		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -250,6 +241,8 @@ static void __init imx23_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx23evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static inline void enable_clk_enet_out(void)
@@ -269,6 +262,8 @@ static void __init imx28_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx28evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 
 	mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
 }
@@ -288,6 +283,7 @@ static void __init m28evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
 }
 
 static void __init sc_sps1_init(void)
@@ -313,6 +309,8 @@ static void __init apx4devkit_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apx4devkit_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 #define ENET0_MDC__GPIO_4_0	MXS_GPIO_NR(4, 0)
@@ -403,6 +401,8 @@ static void __init apf28_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apf28dev_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_16BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static void __init mxs_machine_init(void)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 755556c..45169cb 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -169,6 +169,7 @@ struct mxsfb_info {
 	unsigned dotclk_delay;
 	const struct mxsfb_devdata *devdata;
 	int mapped;
+	u32 sync;
 };
 
 #define mxsfb_is_v3(host) (host->devdata->ipversion = 3)
@@ -456,9 +457,9 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
 	if (fb_info->var.sync & FB_SYNC_VERT_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
+	if (host->sync & MXSFB_SYNC_DATA_ENABLE_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
+	if (host->sync & MXSFB_SYNC_DOTCLK_FAILING_ACT)
 		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
 
 	writel(vdctrl0, host->base + LCDC_VDCTRL0);
@@ -861,6 +862,8 @@ static int mxsfb_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&fb_info->modelist);
 
+	host->sync = pdata->sync;
+
 	ret = mxsfb_init_fbinfo(host);
 	if (ret != 0)
 		goto error_init_fb;
diff --git a/include/linux/mxsfb.h b/include/linux/mxsfb.h
index f14943d..f80af86 100644
--- a/include/linux/mxsfb.h
+++ b/include/linux/mxsfb.h
@@ -24,8 +24,8 @@
 #define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
 #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
 
-#define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
+#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
+#define MXSFB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
 
 struct mxsfb_platform_data {
 	struct fb_videomode *mode_list;
@@ -44,6 +44,9 @@ struct mxsfb_platform_data {
 				 * allocated. If specified,fb_size must also be specified.
 				 * fb_phys must be unused by Linux.
 				 */
+	u32 sync;		/* sync mask, contains MXSFB specifics not
+				 * carried in fb_info->var.sync
+				 */
 };
 
 #endif /* __LINUX_MXSFB_H */
-- 
1.7.10.4

[PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

The issue fixed by this patch manifests only then using X11
with mxsfb driver. The X11 will display either shifted image
or otherwise distorted image on the LCD.

The problem is that the X11 tries to reconfigure the framebuffer
and along the way calls fb_ops.fb_set_par() with X11's desired
configuration values. The field of particular interest is
fb_info->var.sync which contains non-standard values if
configured by kernel. These are either FB_SYNC_DATA_ENABLE_HIGH_ACT,
FB_SYNC_DOTCLK_FAILING_ACT or both, depending on the platform
configuration. Both of these values are defined in the
include/linux/mxsfb.h file.

The driver interprets these values and configures the LCD controller
accordingly. Yet X11 only has access to the standard values for this
field defined in include/uapi/linux/fb.h and thus, unlike kernel,
omits these special values. This results in distorted image on the
LCD.

This patch moves these non-standard values into new field of the
mxsfb_platform_data structure so the driver can in turn check this
field instead of the video mode field for these specific portions.

Moreover, this patch prefixes these values with MXSFB_SYNC_ prefix
instead of FB_SYNC_ prefix to prevent confusion of subsequent users.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
Cc: Lothar Waßmann <LW@karo-electronics.de>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/mach-mxs/mach-mxs.c |   25 +++++++++++++------------
 drivers/video/mxsfb.c        |    7 +++++--
 include/linux/mxsfb.h        |    7 +++++--
 3 files changed, 23 insertions(+), 16 deletions(-)

NOTE: This is the version for mainline, I'm not CCing stable.

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index 3218f1f..cc8d704 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -41,8 +41,6 @@ static struct fb_videomode mx23evk_video_modes[] = {
 		.lower_margin	= 4,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -59,8 +57,6 @@ static struct fb_videomode mx28evk_video_modes[] = {
 		.lower_margin	= 10,
 		.hsync_len	= 10,
 		.vsync_len	= 10,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
 	},
 };
 
@@ -77,7 +73,6 @@ static struct fb_videomode m28evk_video_modes[] = {
 		.lower_margin	= 45,
 		.hsync_len	= 1,
 		.vsync_len	= 1,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT,
 	},
 };
 
@@ -94,9 +89,7 @@ static struct fb_videomode apx4devkit_video_modes[] = {
 		.lower_margin	= 13,
 		.hsync_len	= 48,
 		.vsync_len	= 3,
-		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				  FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				  FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -113,9 +106,7 @@ static struct fb_videomode apf28dev_video_modes[] = {
 		.lower_margin = 0x15,
 		.hsync_len = 64,
 		.vsync_len = 4,
-		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT |
-				FB_SYNC_DATA_ENABLE_HIGH_ACT |
-				FB_SYNC_DOTCLK_FAILING_ACT,
+		.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 	},
 };
 
@@ -132,7 +123,6 @@ static struct fb_videomode cfa10049_video_modes[] = {
 		.lower_margin	= 2,
 		.hsync_len	= 15,
 		.vsync_len	= 15,
-		.sync		= FB_SYNC_DATA_ENABLE_HIGH_ACT
 	},
 };
 
@@ -259,6 +249,8 @@ static void __init imx23_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx23evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static inline void enable_clk_enet_out(void)
@@ -278,6 +270,8 @@ static void __init imx28_evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(mx28evk_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 
 	mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
 }
@@ -297,6 +291,7 @@ static void __init m28evk_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(m28evk_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
 }
 
 static void __init sc_sps1_init(void)
@@ -322,6 +317,8 @@ static void __init apx4devkit_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apx4devkit_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_24BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 #define ENET0_MDC__GPIO_4_0	MXS_GPIO_NR(4, 0)
@@ -407,6 +404,8 @@ static void __init cfa10049_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(cfa10049_video_modes);
 	mxsfb_pdata.default_bpp = 32;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
+
 }
 
 static void __init cfa10037_init(void)
@@ -423,6 +422,8 @@ static void __init apf28_init(void)
 	mxsfb_pdata.mode_count = ARRAY_SIZE(apf28dev_video_modes);
 	mxsfb_pdata.default_bpp = 16;
 	mxsfb_pdata.ld_intf_width = STMLCDIF_16BIT;
+	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT |
+				MXSFB_SYNC_DOTCLK_FAILING_ACT;
 }
 
 static void __init mxs_machine_init(void)
diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 755556c..45169cb 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -169,6 +169,7 @@ struct mxsfb_info {
 	unsigned dotclk_delay;
 	const struct mxsfb_devdata *devdata;
 	int mapped;
+	u32 sync;
 };
 
 #define mxsfb_is_v3(host) (host->devdata->ipversion = 3)
@@ -456,9 +457,9 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		vdctrl0 |= VDCTRL0_HSYNC_ACT_HIGH;
 	if (fb_info->var.sync & FB_SYNC_VERT_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_VSYNC_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DATA_ENABLE_HIGH_ACT)
+	if (host->sync & MXSFB_SYNC_DATA_ENABLE_HIGH_ACT)
 		vdctrl0 |= VDCTRL0_ENABLE_ACT_HIGH;
-	if (fb_info->var.sync & FB_SYNC_DOTCLK_FAILING_ACT)
+	if (host->sync & MXSFB_SYNC_DOTCLK_FAILING_ACT)
 		vdctrl0 |= VDCTRL0_DOTCLK_ACT_FAILING;
 
 	writel(vdctrl0, host->base + LCDC_VDCTRL0);
@@ -861,6 +862,8 @@ static int mxsfb_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&fb_info->modelist);
 
+	host->sync = pdata->sync;
+
 	ret = mxsfb_init_fbinfo(host);
 	if (ret != 0)
 		goto error_init_fb;
diff --git a/include/linux/mxsfb.h b/include/linux/mxsfb.h
index f14943d..f80af86 100644
--- a/include/linux/mxsfb.h
+++ b/include/linux/mxsfb.h
@@ -24,8 +24,8 @@
 #define STMLCDIF_18BIT 2 /** pixel data bus to the display is of 18 bit width */
 #define STMLCDIF_24BIT 3 /** pixel data bus to the display is of 24 bit width */
 
-#define FB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
-#define FB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
+#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT	(1 << 6)
+#define MXSFB_SYNC_DOTCLK_FAILING_ACT	(1 << 7) /* failing/negtive edge sampling */
 
 struct mxsfb_platform_data {
 	struct fb_videomode *mode_list;
@@ -44,6 +44,9 @@ struct mxsfb_platform_data {
 				 * allocated. If specified,fb_size must also be specified.
 				 * fb_phys must be unused by Linux.
 				 */
+	u32 sync;		/* sync mask, contains MXSFB specifics not
+				 * carried in fb_info->var.sync
+				 */
 };
 
 #endif /* __LINUX_MXSFB_H */
-- 
1.7.10.4

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Greg KH]

On Mon, Mar 18, 2013 at 07:23:17PM +0100, Marek Vasut wrote:
> The issue fixed by this patch manifests only then using X11
> with mxsfb driver. The X11 will display either shifted image
> or otherwise distorted image on the LCD.
> 
> The problem is that the X11 tries to reconfigure the framebuffer
> and along the way calls fb_ops.fb_set_par() with X11's desired
> configuration values. The field of particular interest is
> fb_info->var.sync which contains non-standard values if
> configured by kernel. These are either FB_SYNC_DATA_ENABLE_HIGH_ACT,
> FB_SYNC_DOTCLK_FAILING_ACT or both, depending on the platform
> configuration. Both of these values are defined in the
> include/linux/mxsfb.h file.
> 
> The driver interprets these values and configures the LCD controller
> accordingly. Yet X11 only has access to the standard values for this
> field defined in include/uapi/linux/fb.h and thus, unlike kernel,
> omits these special values. This results in distorted image on the
> LCD.
> 
> This patch moves these non-standard values into new field of the
> mxsfb_platform_data structure so the driver can in turn check this
> field instead of the video mode field for these specific portions.
> 
> Moreover, this patch prefixes these values with MXSFB_SYNC_ prefix
> instead of FB_SYNC_ prefix to prevent confusion of subsequent users.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Lothar Waßmann <LW@karo-electronics.de>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/mach-mxs/mach-mxs.c |   22 +++++++++++-----------
>  drivers/video/mxsfb.c        |    7 +++++--
>  include/linux/mxsfb.h        |    7 +++++--
>  3 files changed, 21 insertions(+), 15 deletions(-)
> 
> NOTE: This is the version for stable v3.8.3, so I'm sending it to -stable.
>       I will send adjusted version for mainline 3.9-rc , since there is
>       one more board in mainline and therefore the versions of the patch
>       must differ.


<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Hi Greg,

[...]

> > NOTE: This is the version for stable v3.8.3, so I'm sending it to
> > -stable.
> > 
> >       I will send adjusted version for mainline 3.9-rc , since there is
> >       one more board in mainline and therefore the versions of the patch
> >       must differ.
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.

I'm somewhat sure I violated a few (see below), I won't argue there as you 
surely are much more experienced, but let me dissect the rules so I can learn 
which one to be careful about. Please help me if I did not understand something 
properly.

 - It must be obviously correct and tested.
YES, Fabio tested it on MX28EVK, me on M28EVK (two different devices)

 - It cannot be bigger than 100 lines, with context.
NO, but I see no way to compress it under 100 lines.

 - It must fix only one thing.
YES, it fixes broken X11 on MX28

 - It must fix a real bug that bothers people (not a, "This could be a
   problem..." type thing).
YES, it fixes broken X11 on MX28

 - It must fix a problem that causes a build error (but not for things
   marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   security issue, or some "oh, that's not good" issue.  In short, something
   critical.
YES, it fixes broken X11 on MX28

 - Serious issues as reported by a user of a distribution kernel may also
   be considered if they fix a notable performance or interactivity issue.
   As these fixes are not as obvious and have a higher risk of a subtle
   regression they should only be submitted by a distribution kernel
   maintainer and include an addendum linking to a bugzilla entry if it 
   exists and additional information on the user-visible impact.
This doesn't apply I guess?

 - New device IDs and quirks are also accepted.
This doesn't apply for sure.

 - No "theoretical race condition" issues, unless an explanation of how the
   race can be exploited is also provided.
This doesn't apply for sure.

 - It cannot contain any "trivial" fixes in it (spelling changes,
   whitespace cleanups, etc).
This doesn't apply for sure.

 - It must follow the Documentation/SubmittingPatches rules.
I believe it does. It has one checkpatch warning, but to keep the amount of 
changes to bare minimum, I left it in there.

 - It or an equivalent fix must already exist in Linus' tree (upstream).
This can not happen, the fix in Linus' tree will have to be different since in 
v3.9 there is one more MX28 platform which also needs fixing. I suppose I will 
now wait for Shawn to merge the patch for 3.9-rc, get it mainline and then this 
one can be merged?

Thank you!

Best regards,
Marek Vasut

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Jonathan Nieder]

Marek Vasut wrote:

>  - It or an equivalent fix must already exist in Linus' tree (upstream).
> This can not happen, the fix in Linus' tree will have to be different since in 
> v3.9 there is one more MX28 platform which also needs fixing. I suppose I will 
> now wait for Shawn to merge the patch for 3.9-rc, get it mainline and then this 
> one can be merged?

Yes, please write again with a patch referencing the upstream commit
id when this has been fixed upstream.

Thanks and hope that helps,
Jonathan

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Hi Jonathan,

> Marek Vasut wrote:
> >  - It or an equivalent fix must already exist in Linus' tree (upstream).
> > 
> > This can not happen, the fix in Linus' tree will have to be different
> > since in v3.9 there is one more MX28 platform which also needs fixing. I
> > suppose I will now wait for Shawn to merge the patch for 3.9-rc, get it
> > mainline and then this one can be merged?
> 
> Yes, please write again with a patch referencing the upstream commit
> id when this has been fixed upstream.

Roger that, will do. Is that all that needs repairing in the patch?

Thank you!

Best regards,
Marek Vasut

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Mon, Mar 18, 2013 at 04:19:52PM +0100, Marek Vasut wrote:
> Dear Shawn Guo,
> 
> > On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> > > Did you manage to test it please?
> > 
> > I do not have X11 environment handy to test, but framework console
> > seems still working.
> 
> Even after me poking in it, lol :-)
> 
> You can grab [1], extract it onto card or NFS and just use it as root 
> filesystem, the X11 will come up automagically.
> 
Thanks, Marek.  Yeah, now I can see the problem and your patch does fix
it.

Shawn

> [1] ftp://ftp.denx.de/pub/eldk/5.3/targets/armv5te/core-image-sato-sdk-generic-
> armv5te.tar.gz

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Shawn Guo]

On Mon, Mar 18, 2013 at 07:24:02PM +0100, Marek Vasut wrote:
> The issue fixed by this patch manifests only then using X11
> with mxsfb driver. The X11 will display either shifted image
> or otherwise distorted image on the LCD.
> 
> The problem is that the X11 tries to reconfigure the framebuffer
> and along the way calls fb_ops.fb_set_par() with X11's desired
> configuration values. The field of particular interest is
> fb_info->var.sync which contains non-standard values if
> configured by kernel. These are either FB_SYNC_DATA_ENABLE_HIGH_ACT,
> FB_SYNC_DOTCLK_FAILING_ACT or both, depending on the platform
> configuration. Both of these values are defined in the
> include/linux/mxsfb.h file.
> 
> The driver interprets these values and configures the LCD controller
> accordingly. Yet X11 only has access to the standard values for this
> field defined in include/uapi/linux/fb.h and thus, unlike kernel,
> omits these special values. This results in distorted image on the
> LCD.
> 
> This patch moves these non-standard values into new field of the
> mxsfb_platform_data structure so the driver can in turn check this
> field instead of the video mode field for these specific portions.
> 
> Moreover, this patch prefixes these values with MXSFB_SYNC_ prefix
> instead of FB_SYNC_ prefix to prevent confusion of subsequent users.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
> Cc: Linux FBDEV <linux-fbdev@vger.kernel.org>
> Cc: Lothar Waßmann <LW@karo-electronics.de>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied with one trivial change below.

...

> @@ -407,6 +404,8 @@ static void __init cfa10049_init(void)
>  	mxsfb_pdata.mode_count = ARRAY_SIZE(cfa10049_video_modes);
>  	mxsfb_pdata.default_bpp = 32;
>  	mxsfb_pdata.ld_intf_width = STMLCDIF_18BIT;
> +	mxsfb_pdata.sync = MXSFB_SYNC_DATA_ENABLE_HIGH_ACT;
> +

I removed this unnecessary new line.

Shawn

>  }

Re: [PATCH] ARM: video: mxs: Fix mxsfb misconfiguring VDCTRL0

[Marek Vasut]

Dear Shawn Guo,

> On Mon, Mar 18, 2013 at 04:19:52PM +0100, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> > > On Mon, Mar 18, 2013 at 03:18:13PM +0100, Marek Vasut wrote:
> > > > Did you manage to test it please?
> > > 
> > > I do not have X11 environment handy to test, but framework console
> > > seems still working.
> > 
> > Even after me poking in it, lol :-)
> > 
> > You can grab [1], extract it onto card or NFS and just use it as root
> > filesystem, the X11 will come up automagically.
> 
> Thanks, Marek.  Yeah, now I can see the problem and your patch does fix
> it.

Whew, cool. Can you please apply it and push mainline so I can re-send the one 
for -stable then?

Thank you!

Best regards,
Marek Vasut