| Message ID | 1363471581-10132-1-git-send-email-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
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>
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
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 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.
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
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
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
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
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
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
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
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 */
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.