Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH 2/2] MAINTAINERS: update for sm750fb driver
From: Sudip Mukherjee @ 2015-03-03 10:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Sudip Mukherjee, linux-kernel, devel, linux-fbdev
In-Reply-To: <1425379867-2176-1-git-send-email-sudipm.mukherjee@gmail.com>

add myself and Teddy Wang as the Maintainer of the sm750
frame buffer driver.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ee5466..1a51043 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9340,6 +9340,14 @@ L:	linux-fbdev@vger.kernel.org
 S:	Maintained
 F:	drivers/staging/sm7xxfb/
 
+STAGING - SILICON MOTION SM750 FRAME BUFFER DRIVER
+M:	Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+M:	Teddy Wang <teddy.wang@siliconmotion.com>
+M:	Sudip Mukherjee <sudip@vectorindia.org>
+L:	linux-fbdev@vger.kernel.org
+S:	Maintained
+F:	drivers/staging/sm750fb/
+
 STAGING - SLICOSS
 M:	Lior Dotan <liodot@gmail.com>
 M:	Christopher Harrer <charrer@alacritech.com>
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH,RESEND] framebuffer: don't link fb_devio into kernel image unconditionally
From: Harald Geyer @ 2015-03-03 13:09 UTC (permalink / raw)
  To: linux-fbdev

CONFIG_FB_DEFERRED_IO is defined as bool while CONFIG_FB is defined as
tristate. Currently fb_defio.o is linked into the kernel image even if
CONFIG_FB=m. 

I fix this by updating the Makefile to link fb_defio.o into fb.o and thus
go into one place with the other core framebuffer code.

This has been tested on arm/sunxi and arm/mxs.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
Resending this patch as I didn't get any reply for over two weeks.

 drivers/video/fbdev/core/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
index 67f28e2..23d86a8 100644
--- a/drivers/video/fbdev/core/Makefile
+++ b/drivers/video/fbdev/core/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_FB_CMDLINE)          += fb_cmdline.o
 obj-$(CONFIG_FB)                  += fb.o
 fb-y                              := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
                                      modedb.o fbcvt.o
+fb-$(CONFIG_FB_DEFERRED_IO)       += fb_defio.o
 fb-objs                           := $(fb-y)
 
 obj-$(CONFIG_FB_CFB_FILLRECT)  += cfbfillrect.o
@@ -14,4 +15,3 @@ obj-$(CONFIG_FB_SYS_IMAGEBLIT) += sysimgblt.o
 obj-$(CONFIG_FB_SYS_FOPS)      += fb_sys_fops.o
 obj-$(CONFIG_FB_SVGALIB)       += svgalib.o
 obj-$(CONFIG_FB_DDC)           += fb_ddc.o
-obj-$(CONFIG_FB_DEFERRED_IO)   += fb_defio.o
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCHv2 02/10] fbdev: ssd1307fb: Use vmalloc to allocate video memory.
From: Thomas Niederprüm @ 2015-03-03 19:04 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: plagnioj, tomi.valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <20150303085255.GB4911@lukather>

Am Tue, 3 Mar 2015 09:52:55 +0100
schrieb Maxime Ripard <maxime.ripard@free-electrons.com>:

> Hi,
> 
> On Sun, Mar 01, 2015 at 11:27:55PM +0100, Thomas Niederprüm wrote:
> > It makes sense to use vmalloc to allocate the video buffer since it
> > has to be page aligned memory for using it with mmap. Also deffered
> > io seems buggy in combination with kmalloc'ed memory (crash on
> > unloading the module).
> > 
> > Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> > ---
> >  drivers/video/fbdev/ssd1307fb.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/video/fbdev/ssd1307fb.c
> > b/drivers/video/fbdev/ssd1307fb.c index 61e0ce8..25dd08d 100644
> > --- a/drivers/video/fbdev/ssd1307fb.c
> > +++ b/drivers/video/fbdev/ssd1307fb.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/fb.h>
> >  #include <linux/uaccess.h>
> > +#include <linux/vmalloc.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_gpio.h>
> >  #include <linux/pwm.h>
> > @@ -489,7 +490,7 @@ static int ssd1307fb_probe(struct i2c_client
> > *client, 
> >  	vmem_size = par->width * par->height / 8;
> >  
> > -	vmem = devm_kzalloc(&client->dev, vmem_size, GFP_KERNEL);
> > +	vmem = vzalloc(vmem_size);
> >  	if (!vmem) {
> >  		dev_err(&client->dev, "Couldn't allocate graphical
> > memory.\n"); ret = -ENOMEM;
> > @@ -559,6 +560,7 @@ panel_init_error:
> >  		par->ops->remove(par);
> >  reset_oled_error:
> >  	fb_deferred_io_cleanup(info);
> > +	vfree(vmem);
> >  fb_alloc_error:
> >  	framebuffer_release(info);
> >  	return ret;
> 
> Don't you need the vfree in the remove too?

Yes, of course. Memory will be freed in v3

> 
> Maxime
> 


^ permalink raw reply

* [PATCH 1/4] [resend] tridentfb: fix hang on Blade3D with CONFIG_CC_OPTIMIZE_FOR_SIZE
From: Ondrej Zary @ 2015-03-03 20:56 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list

When the kernel is compiled with -Os (CONFIG_CC_OPTIMIZE_FOR_SIZE), tridentfb
hangs the machine upon load with Blade3D cards unless acceleration is disabled.

This is caused by memcpy() which copies data byte-by-byte (rep movsb) when
compiled with -Os. The card does not like that - it requires 32-bit access.

Use iowrite_32() instead.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
 drivers/video/fbdev/tridentfb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7ed9a22..7429713 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -226,7 +226,7 @@ static void blade_image_blit(struct tridentfb_par *par, const char *data,
 	writemmr(par, DST1, point(x, y));
 	writemmr(par, DST2, point(x + w - 1, y + h - 1));
 
-	memcpy(par->io_virt + 0x10000, data, 4 * size);
+	iowrite32_rep(par->io_virt + 0x10000, data, size);
 }
 
 static void blade_copy_rect(struct tridentfb_par *par,
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 2/4] [resend] tridentfb: Fix set_lwidth on TGUI9440 and CYBER9320
From: Ondrej Zary @ 2015-03-03 20:56 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416163-10700-1-git-send-email-linux@rainbow-software.org>

According to X.Org driver, chips older than TGUI9660 have only 1 width bit
in AddColReg. Touching the 2nd one causes I2C/DDC to fail on TGUI9440.

Set only 1 bit of width in AddColReg on TGUI9440 and CYBER9320.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/tridentfb.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 7429713..01b43e9 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -673,8 +673,14 @@ static int get_nativex(struct tridentfb_par *par)
 static inline void set_lwidth(struct tridentfb_par *par, int width)
 {
 	write3X4(par, VGA_CRTC_OFFSET, width & 0xFF);
-	write3X4(par, AddColReg,
-		 (read3X4(par, AddColReg) & 0xCF) | ((width & 0x300) >> 4));
+	/* chips older than TGUI9660 have only 1 width bit in AddColReg */
+	/* touching the other one breaks I2C/DDC */
+	if (par->chip_id = TGUI9440 || par->chip_id = CYBER9320)
+		write3X4(par, AddColReg,
+		     (read3X4(par, AddColReg) & 0xEF) | ((width & 0x100) >> 4));
+	else
+		write3X4(par, AddColReg,
+		     (read3X4(par, AddColReg) & 0xCF) | ((width & 0x300) >> 4));
 }
 
 /* For resolutions smaller than FP resolution stretch */
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 3/4] [resend] fb_ddc: Allow I2C adapters without SCL read capability
From: Ondrej Zary @ 2015-03-03 20:56 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416163-10700-1-git-send-email-linux@rainbow-software.org>

i2c-algo-bit allows I2C adapters without SCL read capability to work but
fb_ddc_read fails to work on them.

Fix fb_ddc_read to work with I2C adapters not capable of reading SCL.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
Acked-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
 drivers/video/fbdev/core/fb_ddc.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/core/fb_ddc.c b/drivers/video/fbdev/core/fb_ddc.c
index 94322cc..22c694a 100644
--- a/drivers/video/fbdev/core/fb_ddc.c
+++ b/drivers/video/fbdev/core/fb_ddc.c
@@ -69,10 +69,11 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
 		algo_data->setscl(algo_data->data, 1);
 		for (j = 0; j < 5; j++) {
 			msleep(10);
-			if (algo_data->getscl(algo_data->data))
+			if (algo_data->getscl &&
+			    algo_data->getscl(algo_data->data))
 				break;
 		}
-		if (j = 5)
+		if (algo_data->getscl && j = 5)
 			continue;
 
 		algo_data->setsda(algo_data->data, 0);
@@ -91,7 +92,8 @@ unsigned char *fb_ddc_read(struct i2c_adapter *adapter)
 		algo_data->setscl(algo_data->data, 1);
 		for (j = 0; j < 10; j++) {
 			msleep(10);
-			if (algo_data->getscl(algo_data->data))
+			if (algo_data->getscl &&
+			    algo_data->getscl(algo_data->data))
 				break;
 		}
 
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 4/4] [resend] tridentfb: Add DDC support
From: Ondrej Zary @ 2015-03-03 20:56 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416163-10700-1-git-send-email-linux@rainbow-software.org>

Add DDC support for Trident cards.

Tested on TGUI9440, TGUI9680, 3DImage 9750, Blade3D 9880 and Blade XP.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/Kconfig     |    9 ++
 drivers/video/fbdev/tridentfb.c |  192 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 196 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 4916c97..08a7a04 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1682,6 +1682,15 @@ config FB_TRIDENT
 	  To compile this driver as a module, choose M here: the
 	  module will be called tridentfb.
 
+config FB_TRIDENT_DDC
+	bool "DDC for Trident support"
+	depends on FB_TRIDENT
+	select FB_DDC
+	select FB_MODE_HELPERS
+	default y
+	help
+	  Say Y here if you want DDC support for your Trident graphics card.
+
 config FB_ARK
 	tristate "ARK 2000PV support"
 	depends on FB && PCI
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 01b43e9..59471a0 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -25,6 +25,9 @@
 #include <video/vga.h>
 #include <video/trident.h>
 
+#include <linux/i2c.h>
+#include <linux/i2c-algo-bit.h>
+
 struct tridentfb_par {
 	void __iomem *io_virt;	/* iospace virtual memory address */
 	u32 pseudo_pal[16];
@@ -40,6 +43,11 @@ struct tridentfb_par {
 		(struct tridentfb_par *par, const char*,
 		 u32, u32, u32, u32, u32, u32);
 	unsigned char eng_oper;	/* engine operation... */
+#ifdef CONFIG_FB_TRIDENT_DDC
+	bool ddc_registered;
+	struct i2c_adapter ddc_adapter;
+	struct i2c_algo_bit_data ddc_algo;
+#endif
 };
 
 static struct fb_fix_screeninfo tridentfb_fix = {
@@ -53,7 +61,7 @@ static struct fb_fix_screeninfo tridentfb_fix = {
 /* defaults which are normally overriden by user values */
 
 /* video mode */
-static char *mode_option = "640x480-8@60";
+static char *mode_option;
 static int bpp = 8;
 
 static int noaccel;
@@ -174,6 +182,124 @@ static inline u32 readmmr(struct tridentfb_par *par, u16 r)
 	return fb_readl(par->io_virt + r);
 }
 
+#ifdef CONFIG_FB_TRIDENT_DDC
+
+#define DDC_SDA_TGUI		BIT(0)
+#define DDC_SCL_TGUI		BIT(1)
+#define DDC_SCL_DRIVE_TGUI	BIT(2)
+#define DDC_SDA_DRIVE_TGUI	BIT(3)
+#define DDC_MASK_TGUI		(DDC_SCL_DRIVE_TGUI | DDC_SDA_DRIVE_TGUI)
+
+static void tridentfb_ddc_setscl_tgui(void *data, int val)
+{
+	struct tridentfb_par *par = data;
+	u8 reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK_TGUI;
+
+	if (val)
+		reg &= ~DDC_SCL_DRIVE_TGUI; /* disable drive - don't drive hi */
+	else
+		reg |= DDC_SCL_DRIVE_TGUI; /* drive low */
+
+	vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static void tridentfb_ddc_setsda_tgui(void *data, int val)
+{
+	struct tridentfb_par *par = data;
+	u8 reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK_TGUI;
+
+	if (val)
+		reg &= ~DDC_SDA_DRIVE_TGUI; /* disable drive - don't drive hi */
+	else
+		reg |= DDC_SDA_DRIVE_TGUI; /* drive low */
+
+	vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static int tridentfb_ddc_getsda_tgui(void *data)
+{
+	struct tridentfb_par *par = data;
+
+	return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SDA_TGUI);
+}
+
+#define DDC_SDA_IN	BIT(0)
+#define DDC_SCL_OUT	BIT(1)
+#define DDC_SDA_OUT	BIT(3)
+#define DDC_SCL_IN	BIT(6)
+#define DDC_MASK	(DDC_SCL_OUT | DDC_SDA_OUT)
+
+static void tridentfb_ddc_setscl(void *data, int val)
+{
+	struct tridentfb_par *par = data;
+	unsigned char reg;
+
+	reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK;
+	if (val)
+		reg |= DDC_SCL_OUT;
+	else
+		reg &= ~DDC_SCL_OUT;
+	vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static void tridentfb_ddc_setsda(void *data, int val)
+{
+	struct tridentfb_par *par = data;
+	unsigned char reg;
+
+	reg = vga_mm_rcrt(par->io_virt, I2C) & DDC_MASK;
+	if (!val)
+		reg |= DDC_SDA_OUT;
+	else
+		reg &= ~DDC_SDA_OUT;
+	vga_mm_wcrt(par->io_virt, I2C, reg);
+}
+
+static int tridentfb_ddc_getscl(void *data)
+{
+	struct tridentfb_par *par = data;
+
+	return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SCL_IN);
+}
+
+static int tridentfb_ddc_getsda(void *data)
+{
+	struct tridentfb_par *par = data;
+
+	return !!(vga_mm_rcrt(par->io_virt, I2C) & DDC_SDA_IN);
+}
+
+static int tridentfb_setup_ddc_bus(struct fb_info *info)
+{
+	struct tridentfb_par *par = info->par;
+
+	strlcpy(par->ddc_adapter.name, info->fix.id,
+		sizeof(par->ddc_adapter.name));
+	par->ddc_adapter.owner		= THIS_MODULE;
+	par->ddc_adapter.class		= I2C_CLASS_DDC;
+	par->ddc_adapter.algo_data	= &par->ddc_algo;
+	par->ddc_adapter.dev.parent	= info->device;
+	if (is_oldclock(par->chip_id)) { /* not sure if this check is OK */
+		par->ddc_algo.setsda	= tridentfb_ddc_setsda_tgui;
+		par->ddc_algo.setscl	= tridentfb_ddc_setscl_tgui;
+		par->ddc_algo.getsda	= tridentfb_ddc_getsda_tgui;
+		/* no getscl */
+	} else {
+		par->ddc_algo.setsda	= tridentfb_ddc_setsda;
+		par->ddc_algo.setscl	= tridentfb_ddc_setscl;
+		par->ddc_algo.getsda	= tridentfb_ddc_getsda;
+		par->ddc_algo.getscl	= tridentfb_ddc_getscl;
+	}
+	par->ddc_algo.udelay		= 10;
+	par->ddc_algo.timeout		= 20;
+	par->ddc_algo.data		= par;
+
+	i2c_set_adapdata(&par->ddc_adapter, par);
+
+	return i2c_bit_add_bus(&par->ddc_adapter);
+}
+#endif /* CONFIG_FB_TRIDENT_DDC */
+
 /*
  * Blade specific acceleration.
  */
@@ -1346,6 +1472,7 @@ static int trident_pci_probe(struct pci_dev *dev,
 	struct tridentfb_par *default_par;
 	int chip3D;
 	int chip_id;
+	bool found = false;
 
 	err = pci_enable_device(dev);
 	if (err)
@@ -1499,6 +1626,7 @@ static int trident_pci_probe(struct pci_dev *dev,
 	info->pixmap.scan_align = 1;
 	info->pixmap.access_align = 32;
 	info->pixmap.flags = FB_PIXMAP_SYSTEM;
+	info->var.bits_per_pixel = 8;
 
 	if (default_par->image_blit) {
 		info->flags |= FBINFO_HWACCEL_IMAGEBLIT;
@@ -1511,11 +1639,57 @@ static int trident_pci_probe(struct pci_dev *dev,
 		info->pixmap.scan_align = 1;
 	}
 
-	if (!fb_find_mode(&info->var, info,
-			  mode_option, NULL, 0, NULL, bpp)) {
-		err = -EINVAL;
-		goto out_unmap2;
+#ifdef CONFIG_FB_TRIDENT_DDC
+	if (tridentfb_setup_ddc_bus(info) = 0) {
+		u8 *edid = fb_ddc_read(&default_par->ddc_adapter);
+
+		default_par->ddc_registered = true;
+		if (edid) {
+			fb_edid_to_monspecs(edid, &info->monspecs);
+			kfree(edid);
+			if (!info->monspecs.modedb)
+				dev_err(info->device, "error getting mode database\n");
+			else {
+				const struct fb_videomode *m;
+
+				fb_videomode_to_modelist(info->monspecs.modedb,
+						 info->monspecs.modedb_len,
+						 &info->modelist);
+				m = fb_find_best_display(&info->monspecs,
+							 &info->modelist);
+				if (m) {
+					fb_videomode_to_var(&info->var, m);
+					/* fill all other info->var's fields */
+					if (tridentfb_check_var(&info->var,
+								info) = 0)
+						found = true;
+				}
+			}
+		}
 	}
+#endif
+	if (!mode_option && !found)
+		mode_option = "640x480-8@60";
+
+	/* Prepare startup mode */
+	if (mode_option) {
+		err = fb_find_mode(&info->var, info, mode_option,
+				   info->monspecs.modedb,
+				   info->monspecs.modedb_len,
+				   NULL, info->var.bits_per_pixel);
+		if (!err || err = 4) {
+			err = -EINVAL;
+			dev_err(info->device, "mode %s not found\n",
+								mode_option);
+			fb_destroy_modedb(info->monspecs.modedb);
+			info->monspecs.modedb = NULL;
+			goto out_unmap2;
+		}
+	}
+
+	fb_destroy_modedb(info->monspecs.modedb);
+	info->monspecs.modedb = NULL;
+
 	err = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (err < 0)
 		goto out_unmap2;
@@ -1536,6 +1710,10 @@ static int trident_pci_probe(struct pci_dev *dev,
 	return 0;
 
 out_unmap2:
+#ifdef CONFIG_FB_TRIDENT_DDC
+	if (default_par->ddc_registered)
+		i2c_del_adapter(&default_par->ddc_adapter);
+#endif
 	kfree(info->pixmap.addr);
 	if (info->screen_base)
 		iounmap(info->screen_base);
@@ -1555,6 +1733,10 @@ static void trident_pci_remove(struct pci_dev *dev)
 	struct tridentfb_par *par = info->par;
 
 	unregister_framebuffer(info);
+#ifdef CONFIG_FB_TRIDENT_DDC
+	if (par->ddc_registered)
+		i2c_del_adapter(&par->ddc_adapter);
+#endif
 	iounmap(par->io_virt);
 	iounmap(info->screen_base);
 	release_mem_region(tridentfb_fix.smem_start, tridentfb_fix.smem_len);
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 1/5] [resend] gxt4500: enable on non-PPC architectures
From: Ondrej Zary @ 2015-03-03 21:00 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list

These chips can be present at least on x86 too - Fire GL2 AGP has GXT6000P but
this driver is currently limited to PPC.
Enable it for all architectures and add chip configuration for little-endian.

Tested on x86 with Fire GL2 AGP.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/Kconfig   |    5 +++--
 drivers/video/fbdev/gxt4500.c |    7 +++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 08a7a04..cb15734 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2143,7 +2143,7 @@ config FB_UDL
 
 config FB_IBM_GXT4500
 	tristate "Framebuffer support for IBM GXT4000P/4500P/6000P/6500P adaptors"
-	depends on FB && PPC
+	depends on FB
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
@@ -2151,7 +2151,8 @@ config FB_IBM_GXT4500
 	  Say Y here to enable support for the IBM GXT4000P/6000P and
 	  GXT4500P/6500P display adaptor based on Raster Engine RC1000,
 	  found on some IBM System P (pSeries) machines. This driver
-	  doesn't use Geometry Engine GT1000.
+	  doesn't use Geometry Engine GT1000. This driver also supports
+	  AGP Fire GL2/3/4 cards on x86.
 
 config FB_PS3
 	tristate "PS3 GPU framebuffer driver"
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a..1bf9894 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -670,8 +670,15 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, info);
 
+#ifdef __BIG_ENDIAN
 	/* Set byte-swapping for DFA aperture for all pixel sizes */
 	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
+#else /* __LITTLE_ENDIAN */
+	/* not sure what this means but fgl23 driver does that */
+	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x2300);
+/*	pci_write_config_dword(pdev, CFG_ENDIAN0 + 4, 0x400000);*/
+	pci_write_config_dword(pdev, CFG_ENDIAN0 + 8, 0x98530000);
+#endif
 
 	info->fbops = &gxt4500_ops;
 	info->flags = FBINFO_FLAG_DEFAULT;
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 2/5] [resend] gxt4500: fix 16bpp 565 mode
From: Ondrej Zary @ 2015-03-03 21:00 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416460-10773-1-git-send-email-linux@rainbow-software.org>

Fix wrong colors in 16bpp 565 mode.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/gxt4500.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1bf9894..1f2fd5b 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -525,7 +525,7 @@ static int gxt4500_setcolreg(unsigned int reg, unsigned int red,
 		u32 val = reg;
 		switch (par->pixfmt) {
 		case DFA_PIX_16BIT_565:
-			val |= (reg << 11) | (reg << 6);
+			val |= (reg << 11) | (reg << 5);
 			break;
 		case DFA_PIX_16BIT_1555:
 			val |= (reg << 10) | (reg << 5);
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 3/5] [resend] gxt4500: fix color order
From: Ondrej Zary @ 2015-03-03 21:00 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416460-10773-1-git-send-email-linux@rainbow-software.org>

The color order in truecolor modes is wrong. This does not affect console but
is visible e.g. in X11 which has wrong colors.

Swap blue and red colors to fix the problem.
Fixes https://forums.gentoo.org/viewtopic-t-692740-start-0.html

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/gxt4500.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 1f2fd5b..442b07c 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -347,11 +347,12 @@ static void gxt4500_unpack_pixfmt(struct fb_var_screeninfo *var,
 		break;
 	}
 	if (pixfmt != DFA_PIX_8BIT) {
-		var->green.offset = var->red.length;
-		var->blue.offset = var->green.offset + var->green.length;
+		var->blue.offset = 0;
+		var->green.offset = var->blue.length;
+		var->red.offset = var->green.offset + var->green.length;
 		if (var->transp.length)
 			var->transp.offset -				var->blue.offset + var->blue.length;
+				var->red.offset + var->red.length;
 	}
 }
 
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 4/5] [resend] gxt4500: Use write-combining for framebuffer (PAT and MTRR)
From: Ondrej Zary @ 2015-03-03 21:00 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416460-10773-1-git-send-email-linux@rainbow-software.org>

Use write-combining for framebuffer to greatly improve performance on x86.
Add both ioremap_wc (for systems with PAT) and MTRR setup for non-PAT systems.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/gxt4500.c |   32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 442b07c..f278c82 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -13,6 +13,9 @@
 #include <linux/pci_ids.h>
 #include <linux/delay.h>
 #include <linux/string.h>
+#ifdef CONFIG_MTRR
+#include <asm/mtrr.h>
+#endif
 
 #define PCI_DEVICE_ID_IBM_GXT4500P	0x21c
 #define PCI_DEVICE_ID_IBM_GXT6500P	0x21b
@@ -142,7 +145,9 @@ static const unsigned char watfmt[] = {
 
 struct gxt4500_par {
 	void __iomem *regs;
-
+#ifdef CONFIG_MTRR
+	int mtrr_reg;
+#endif
 	int pixfmt;		/* pixel format, see DFA_PIX_* values */
 
 	/* PLL parameters */
@@ -158,6 +163,10 @@ struct gxt4500_par {
 /* mode requested by user */
 static char *mode_option;
 
+#ifdef CONFIG_MTRR
+static int mtrr = 1;
+#endif
+
 /* default mode: 1280x1024 @ 60 Hz, 8 bpp */
 static const struct fb_videomode defaultmode = {
 	.refresh = 60,
@@ -663,7 +672,7 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	info->fix.smem_start = fb_phys;
 	info->fix.smem_len = pci_resource_len(pdev, 1);
-	info->screen_base = pci_ioremap_bar(pdev, 1);
+	info->screen_base = ioremap_wc(fb_phys, info->fix.smem_len);
 	if (!info->screen_base) {
 		dev_err(&pdev->dev, "gxt4500: cannot map framebuffer\n");
 		goto err_unmap_regs;
@@ -671,6 +680,14 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, info);
 
+#ifdef CONFIG_MTRR
+	if (mtrr) {
+		par->mtrr_reg = -1;
+		par->mtrr_reg = mtrr_add(info->fix.smem_start,
+				info->fix.smem_len, MTRR_TYPE_WRCOMB, 1);
+	}
+#endif
+
 #ifdef __BIG_ENDIAN
 	/* Set byte-swapping for DFA aperture for all pixel sizes */
 	pci_write_config_dword(pdev, CFG_ENDIAN0, 0x333300);
@@ -735,6 +752,12 @@ static void gxt4500_remove(struct pci_dev *pdev)
 		return;
 	par = info->par;
 	unregister_framebuffer(info);
+#ifdef CONFIG_MTRR
+	if (par->mtrr_reg >= 0) {
+		mtrr_del(par->mtrr_reg, 0, 0);
+		par->mtrr_reg = -1;
+	}
+#endif
 	fb_dealloc_cmap(&info->cmap);
 	iounmap(par->regs);
 	iounmap(info->screen_base);
@@ -789,3 +812,8 @@ MODULE_DESCRIPTION("FBDev driver for IBM GXT4500P/6500P and GXT4000P/6000P");
 MODULE_LICENSE("GPL");
 module_param(mode_option, charp, 0);
 MODULE_PARM_DESC(mode_option, "Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\"");
+
+#ifdef CONFIG_MTRR
+module_param(mtrr, int, 0444);
+MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
+#endif
-- 
Ondrej Zary


^ permalink raw reply related

* [PATCH 5/5] [resend] gxt4500: enable panning
From: Ondrej Zary @ 2015-03-03 21:01 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <1425416460-10773-1-git-send-email-linux@rainbow-software.org>

The driver implements pan_display but the corresponding flags are not set.

Add FBINFO_HWACCEL_XPAN and FBINFO_HWACCEL_YPAN to flags to allow HW
accelerated panning (for fast scrolling).

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/video/fbdev/gxt4500.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index f278c82..7f97c89 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -699,7 +699,8 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 #endif
 
 	info->fbops = &gxt4500_ops;
-	info->flags = FBINFO_FLAG_DEFAULT;
+	info->flags = FBINFO_FLAG_DEFAULT | FBINFO_HWACCEL_XPAN |
+					    FBINFO_HWACCEL_YPAN;
 
 	err = fb_alloc_cmap(&info->cmap, 256, 0);
 	if (err) {
-- 
Ondrej Zary


^ permalink raw reply related

* Re: [PATCH] video: ARM CLCD: Added support for FBIOPAN_DISPLAY and virtual y resolution
From: Arun Ramamurthy @ 2015-03-04  0:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Russell King - ARM Linux, Pawel Moll, Rob Herring, Mark Rutland,
	Ian Campbell, Kumar Gala, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Dmitry Torokhov, Anatol Pomazau, Jonathan Richardson,
	Scott Branden, Ray Jui,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org
In-Reply-To: <CAL_JsqKMP4JxQ-Q5V1skcaKPdhC4v-joOrNyn++tMfat8arMKA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 15-03-02 03:22 PM, Rob Herring wrote:
> On Mon, Mar 2, 2015 at 1:09 PM, Arun Ramamurthy
> <arun.ramamurthy@broadcom.com> wrote:
>>
>>
>> On 15-03-02 08:11 AM, Russell King - ARM Linux wrote:
>>>
>>> On Mon, Mar 02, 2015 at 04:08:29PM +0000, Pawel Moll wrote:
>>>>
>>>> I'm not sure about this... The word "virtual" never works well with
>>>> device tree nodes defined as "hardware description".
>>>>
>>>> I understand what you're doing, but adding this property to the display
>>>> controller's node doesn't sound right. How does this describe hardware?
>>>> If anywhere, it's more like a job for the panel node?
>>>
>>>
>> I see what you are saying Pawel, I can follow Russell's recommendation of
>> adding a RAM size node called max-memory-available or something similar
>
> We've already got a binding for reserved memory regions for this
> purpose. And there is also simplefb binding.
>
Rob, I am not sure what binding you are referring to, could you be more 
specific? Also how does the simplefb bindings apply to me considering 
that is a separate driver? Thanks
> Rob
>

^ permalink raw reply

* Re: [PATCH] video: ARM CLCD: Added support for FBIO_WAITFORVSYNC
From: Arun Ramamurthy @ 2015-03-04  0:31 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll
  Cc: Rob Herring, Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fbdev@vger.kernel.org, Dmitry Torokhov, Anatol Pomazau,
	Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list@broadcom.com
In-Reply-To: <CAL_Jsq+XKESfwU+UfkbDYW+-=3F723OjuqEtoBhbQZprWszvvA@mail.gmail.com>



On 15-03-02 03:27 PM, Rob Herring wrote:
> On Mon, Mar 2, 2015 at 10:00 AM, Pawel Moll <pawel.moll@arm.com> wrote:
>> On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote:
>>> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC
>>> Also corrected documentation to make interrupts and interrupt-names
>>> optional as they are not required properties.
>>
>> You may not be aware of this fact, but its the "documentation" what
>> defines what properties are required...
>
> Except when docs are wrong. Then dts files win.
>
>>> Reviewed-by: Ray Jui <rjui@broadcom.com>
>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0
>>> ---
>>>   .../devicetree/bindings/video/arm,pl11x.txt        | 11 +--
>>>   drivers/video/fbdev/amba-clcd.c                    | 82 ++++++++++++++++++++++
>>>   include/linux/amba/clcd.h                          |  4 ++
>>>   3 files changed, 89 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt
>>> index 2262cdb..7d19024 100644
>>> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt
>>> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt
>>> @@ -10,14 +10,6 @@ Required properties:
>>>
>>>   - reg: base address and size of the control registers block
>>>
>>> -- interrupt-names: either the single entry "combined" representing a
>>> -     combined interrupt output (CLCDINTR), or the four entries
>>> -     "mbe", "vcomp", "lnbu", "fuf" representing the individual
>>> -     CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts
>>> -
>>> -- interrupts: contains an interrupt specifier for each entry in
>>> -     interrupt-names
>>> -
>>>   - clock-names: should contain "clcdclk" and "apb_pclk"
>>>
>>>   - clocks: contains phandle and clock specifier pairs for the entries
>>
>> So no, you can't do that.
>
> You can't do the other way around (making optional ones required), but
> I think this is okay if the h/w interrupt lines are not physically
> connected. However, if it is simply because the driver doesn't use
> them, then I agree this should not be changed.
>
Agreed, I will undo this change in V2
> Rob
>

^ permalink raw reply

* Re: [PATCH] video: ARM CLCD: Added support for FBIO_WAITFORVSYNC
From: Arun Ramamurthy @ 2015-03-04  0:33 UTC (permalink / raw)
  To: Rob Herring
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Russell King, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Dmitry Torokhov, Anatol Pomazau, Jonathan Richardson,
	Scott Branden, Ray Jui,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
In-Reply-To: <CAL_JsqJfA3Pxvdux-Um9nFoaZpRh30S9d1TQid_TchDH_qo7Ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 15-03-02 03:29 PM, Rob Herring wrote:
> On Wed, Feb 25, 2015 at 3:01 PM, Arun Ramamurthy
> <arun.ramamurthy@broadcom.com> wrote:
>> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC
>> Also corrected documentation to make interrupts and interrupt-names
>> optional as they are not required properties.
>>
>> Reviewed-by: Ray Jui <rjui@broadcom.com>
>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>0
>> ---
>>   .../devicetree/bindings/video/arm,pl11x.txt        | 11 +--
>
> Please split bindings to separate patches.
Rob, I looked at the history of patches to this driver and all them 
included changes to the documentation in the same patch so I followed a 
similar model. I assumed that a separate patch was only required when 
the documentation was being added from scratch and not when modifying a 
few lines. Is this not the case?
>
>>   drivers/video/fbdev/amba-clcd.c                    | 82 ++++++++++++++++++++++
>>   include/linux/amba/clcd.h                          |  4 ++
>>   3 files changed, 89 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/arm,pl11x.txt b/Documentation/devicetree/bindings/video/arm,pl11x.txt
>> index 2262cdb..7d19024 100644
>> --- a/Documentation/devicetree/bindings/video/arm,pl11x.txt
>> +++ b/Documentation/devicetree/bindings/video/arm,pl11x.txt
>> @@ -10,14 +10,6 @@ Required properties:
>>
>>   - reg: base address and size of the control registers block
>>
>> -- interrupt-names: either the single entry "combined" representing a
>> -       combined interrupt output (CLCDINTR), or the four entries
>> -       "mbe", "vcomp", "lnbu", "fuf" representing the individual
>> -       CLCDMBEINTR, CLCDVCOMPINTR, CLCDLNBUINTR, CLCDFUFINTR interrupts
>> -
>> -- interrupts: contains an interrupt specifier for each entry in
>> -       interrupt-names
>> -
>>   - clock-names: should contain "clcdclk" and "apb_pclk"
>>
>>   - clocks: contains phandle and clock specifier pairs for the entries
>> @@ -54,6 +46,9 @@ Optional properties:
>>          It can be used to configure a virtual y resolution. It
>>          must be a value larger than the actual y resolution.
>>
>> +- interrupts: contains an interrupt specifier for the clcd vcomp interrupt
>> +        This is required for the driver to handle FBIO_WAITFORVSYNC ioctls.
>> +
>
> What happened to interrupt-names?
As mentioned before, these changes to the documentation  will be undone 
in V2. Thanks
>
> Rob
>

^ permalink raw reply

* Re: [PATCH] video: ARM CLCD: Added support for FBIO_WAITFORVSYNC
From: Arun Ramamurthy @ 2015-03-04  0:35 UTC (permalink / raw)
  To: Pawel Moll
  Cc: Rob Herring, Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fbdev@vger.kernel.org, Dmitry Torokhov, Anatol Pomazau,
	Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list@broadcom.com
In-Reply-To: <1425376886.3092.25.camel@arm.com>



On 15-03-03 02:01 AM, Pawel Moll wrote:
> On Mon, 2015-03-02 at 19:09 +0000, Arun Ramamurthy wrote:
>> On 15-03-02 08:00 AM, Pawel Moll wrote:
>>> On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote:
>>>> Added ioctl and interrupt handler functions to support FBIO_WAITFORVSYNC
>>>> Also corrected documentation to make interrupts and interrupt-names
>>>> optional as they are not required properties.
>>>
>>> You may not be aware of this fact, but its the "documentation" what
>>> defines what properties are required...
>>>
>> Pawel, I was not aware of that. Since the driver code did not require
>> the interrupts or interrupt-names bindings to load properly, I moved it
>> out of the required properties. I can remove that change.
>
> Cool. Drivers don't have to use all available properties :-) The
> interrupt names are required because CLCD can be wired up with a single,
> combined interrupt or with 4 separate interrupt lines (see "A.4. On-chip
> signals", in the TRM) and the binding has to provide ways of describing
> this. Yes, one could say "1 number = combined, 4 numbers = split", but
> I personally prefer to be explicit than implicit.
>
> Just request the interrupt by name and you'll be fine.
>
Ok got it, will make the necessary changes
>> Any other comments on this change? Thanks
>
> I have no experience with the vsync ioctl, so can't really comment on
> it. One minor thing I did spot is your use of curly brackets in one of
> the switch cases:
>
> On Wed, 2015-02-25 at 21:01 +0000, Arun Ramamurthy wrote:
> @@ -466,6 +468,73 @@ static int clcdfb_pan_display(struct fb_var_screeninfo *var,
>>   	return 0;
>>   }
>>
>> +static int clcdfb_ioctl(struct fb_info *info,
>> +			unsigned int cmd, unsigned long args)
>> +{
>> +	struct clcd_fb *fb = to_clcd(info);
>> +	int retval = 0;
>> +	u32 val, ienb_val;
>> +
>> +	switch (cmd) {
>> +	case FBIO_WAITFORVSYNC:{
>
> In the line above...
>
>> +		if (fb->lcd_irq <= 0) {
>> +			retval = -EINVAL;
>> +			break;
>> +		}
>> +		/* disable Vcomp interrupts */
>> +		ienb_val = readl(fb->regs + fb->off_ienb);
>> +		ienb_val &= ~CLCD_PL111_IENB_VCOMP;
>> +		writel(ienb_val, fb->regs + fb->off_ienb);
>> +
>> +		/* clear Vcomp interrupt */
>> +		writel(CLCD_PL111_IENB_VCOMP, fb->regs + CLCD_PL111_ICR);
>> +
>> +		/* Generate Interrupt at the start of Vsync */
>> +		reinit_completion(&fb->wait);
>> +		val = readl(fb->regs +  fb->off_cntl);
>> +		val &= ~(CNTL_LCDVCOMP(3));
>> +		writel(val, fb->regs + fb->off_cntl);
>> +
>> +		/* enable Vcomp interrupt */
>> +		ienb_val = readl(fb->regs + fb->off_ienb);
>> +		ienb_val |= CLCD_PL111_IENB_VCOMP;
>> +		writel(ienb_val, fb->regs + fb->off_ienb);
>> +		if (!wait_for_completion_interruptible_timeout
>> +			(&fb->wait, HZ/10))
>> +			retval = -ETIMEDOUT;
>> +		break;
>> +	}
>
> ... and here.
>
> Not sure it's needed?
>
No its not needed, I can remove it.
> Pawel
>
>

^ permalink raw reply

* Re: [PATCH] video: ARM CLCD: Added dt support to set tim2 register
From: Arun Ramamurthy @ 2015-03-04  0:37 UTC (permalink / raw)
  To: Pawel Moll
  Cc: Rob Herring, Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Dmitry Torokhov, Anatol Pomazau, Jonathan Richardson,
	Scott Branden, Ray Jui,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org
In-Reply-To: <1425378127.3092.38.camel-5wv7dgnIgG8@public.gmane.org>



On 15-03-03 02:22 AM, Pawel Moll wrote:
> On Tue, 2015-03-03 at 10:02 +0000, Pawel Moll wrote:
>> On Mon, 2015-03-02 at 19:09 +0000, Arun Ramamurthy wrote:
>>>> The existing bindings intentionally avoided quoting internal registers -
>>>> they are supposed to describe how the hardware is wired up...
>>>>
>>>> So how about something like "arm,pl11x,tft-invert-clac"? Then the driver
>>>> sets the bit or not, depending on the property existance?
>>>>
>>> Sure, I can change it to two properties called arm,pl11x,tft-invert-clac
>>> and arm,pl11x,tft-clksel. Would that be acceptable?
>>
>> That would be fine by me :-)
>
> Or (after having a look at the TRM) I should rather say: the invert-clac
> is fine by me :-) but the tft-clksel doesn't work, I afraid.
>
> If I'm not mistaken, there are two problems with it.
>
> Number one: it's not TFT-specific, is it? So it certainly should not
> have the "tft-" bit.
>
> Number two: setting this bit says "do not use CLCDCLK for the logic; use
> HCLK instead", correct? If so, have a look at the clock properties. They
> say:
>
> - clock-names: should contain "clcdclk" and "apb_pclk"
>
> - clocks: contains phandle and clock specifier pairs for the entries
>          in the clock-names property. See
>
> So if your hardware has the reference clock wired to HCLK, and you
> defining the clocks as "clcdclk", you are (no offence meant ;-)
> lying :-)
>
No offense taken :)
> So how about solving the problem by extending the clock-names definition
> like this (feel free to use own wording):
>
> - clock-names: should contain two clocks, either "clcdclk" or "hclk"
>                 (depending on which input is to be used as a reference
>                 clock by the controller logic) and "apb_pclk"
>
> That way you're precisely describing the way the hardware is wired up.
> And the driver simply tries to get clcdclk first, if it's defined -
> cool, set clksel to 1, if not - try hclk and set clksel to 0. If neither
> of them is present - bail out.
>
> Does this make any sense?
>
This makes sense to me, thank you for the suggestions. I will fix it all 
up in V2
> Pawel
>

^ permalink raw reply

* [PATCH] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Liu Ying @ 2015-03-04  5:58 UTC (permalink / raw)
  To: linux-fbdev
  Cc: Peter Chen, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Fabio Estevam, linux-kernel, stable

The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
as the engine's system clock.  The clock should be enabled when accessing
LCDIF registers, otherwise the kernel would hang up.  We should also keep
the clock being enabled when the engine is being active to scan out frames
from memory.  This patch makes sure the axi clock is enabled when accessing
registers so that the kernel hang up issue can be fixed.

Reported-by: Peter Chen <peter.chen@freescale.com>
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index f8ac4a4..a8cf3b2 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -316,6 +316,18 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
 	return 0;
 }
 
+static inline void mxsfb_enable_axi_clk(struct mxsfb_info *host)
+{
+	if (host->clk_axi)
+		clk_prepare_enable(host->clk_axi);
+}
+
+static inline void mxsfb_disable_axi_clk(struct mxsfb_info *host)
+{
+	if (host->clk_axi)
+		clk_disable_unprepare(host->clk_axi);
+}
+
 static void mxsfb_enable_controller(struct fb_info *fb_info)
 {
 	struct mxsfb_info *host = to_imxfb_host(fb_info);
@@ -333,14 +345,13 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
 		}
 	}
 
-	if (host->clk_axi)
-		clk_prepare_enable(host->clk_axi);
-
 	if (host->clk_disp_axi)
 		clk_prepare_enable(host->clk_disp_axi);
 	clk_prepare_enable(host->clk);
 	clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
 
+	mxsfb_enable_axi_clk(host);
+
 	/* if it was disabled, re-enable the mode again */
 	writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
 
@@ -380,11 +391,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 	reg = readl(host->base + LCDC_VDCTRL4);
 	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
+	mxsfb_disable_axi_clk(host);
+
 	clk_disable_unprepare(host->clk);
 	if (host->clk_disp_axi)
 		clk_disable_unprepare(host->clk_disp_axi);
-	if (host->clk_axi)
-		clk_disable_unprepare(host->clk_axi);
 
 	host->enabled = 0;
 
@@ -421,6 +432,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		mxsfb_disable_controller(fb_info);
 	}
 
+	mxsfb_enable_axi_clk(host);
+
 	/* clear the FIFOs */
 	writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
 
@@ -438,6 +451,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		ctrl |= CTRL_SET_WORD_LENGTH(3);
 		switch (host->ld_intf_width) {
 		case STMLCDIF_8BIT:
+			mxsfb_disable_axi_clk(host);
 			dev_err(&host->pdev->dev,
 					"Unsupported LCD bus width mapping\n");
 			return -EINVAL;
@@ -451,6 +465,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
 		break;
 	default:
+		mxsfb_disable_axi_clk(host);
 		dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
 				fb_info->var.bits_per_pixel);
 		return -EINVAL;
@@ -504,6 +519,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 			fb_info->fix.line_length * fb_info->var.yoffset,
 			host->base + host->devdata->next_buf);
 
+	mxsfb_disable_axi_clk(host);
+
 	if (reenable)
 		mxsfb_enable_controller(fb_info);
 
@@ -582,10 +599,16 @@ static int mxsfb_pan_display(struct fb_var_screeninfo *var,
 
 	offset = fb_info->fix.line_length * var->yoffset;
 
+	if (!host->enabled)
+		mxsfb_enable_axi_clk(host);
+
 	/* update on next VSYNC */
 	writel(fb_info->fix.smem_start + offset,
 			host->base + host->devdata->next_buf);
 
+	if (!host->enabled)
+		mxsfb_disable_axi_clk(host);
+
 	return 0;
 }
 
@@ -608,13 +631,17 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 	unsigned line_count;
 	unsigned period;
 	unsigned long pa, fbsize;
-	int bits_per_pixel, ofs;
+	int bits_per_pixel, ofs, ret = 0;
 	u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
 
+	mxsfb_enable_axi_clk(host);
+
 	/* Only restore the mode when the controller is running */
 	ctrl = readl(host->base + LCDC_CTRL);
-	if (!(ctrl & CTRL_RUN))
-		return -EINVAL;
+	if (!(ctrl & CTRL_RUN)) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	vdctrl0 = readl(host->base + LCDC_VDCTRL0);
 	vdctrl2 = readl(host->base + LCDC_VDCTRL2);
@@ -635,7 +662,8 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 		break;
 	case 1:
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	fb_info->var.bits_per_pixel = bits_per_pixel;
@@ -673,10 +701,14 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 
 	pa = readl(host->base + host->devdata->cur_buf);
 	fbsize = fb_info->fix.line_length * vmode->yres;
-	if (pa < fb_info->fix.smem_start)
-		return -EINVAL;
-	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
-		return -EINVAL;
+	if (pa < fb_info->fix.smem_start) {
+		ret = -EINVAL;
+		goto err;
+	}
+	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len) {
+		ret = -EINVAL;
+		goto err;
+	}
 	ofs = pa - fb_info->fix.smem_start;
 	if (ofs) {
 		memmove(fb_info->screen_base, fb_info->screen_base + ofs, fbsize);
@@ -689,7 +721,11 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 	clk_prepare_enable(host->clk);
 	host->enabled = 1;
 
-	return 0;
+err:
+	if (ret)
+		mxsfb_disable_axi_clk(host);
+
+	return ret;
 }
 
 static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
@@ -915,7 +951,9 @@ static int mxsfb_probe(struct platform_device *pdev)
 	}
 
 	if (!host->enabled) {
+		mxsfb_enable_axi_clk(host);
 		writel(0, host->base + LCDC_CTRL);
+		mxsfb_disable_axi_clk(host);
 		mxsfb_set_par(fb_info);
 		mxsfb_enable_controller(fb_info);
 	}
@@ -954,11 +992,15 @@ static void mxsfb_shutdown(struct platform_device *pdev)
 	struct fb_info *fb_info = platform_get_drvdata(pdev);
 	struct mxsfb_info *host = to_imxfb_host(fb_info);
 
+	mxsfb_enable_axi_clk(host);
+
 	/*
 	 * Force stop the LCD controller as keeping it running during reboot
 	 * might interfere with the BootROM's boot mode pads sampling.
 	 */
 	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
+
+	mxsfb_disable_axi_clk(host);
 }
 
 static struct platform_driver mxsfb_driver = {
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Greg KH @ 2015-03-04  6:08 UTC (permalink / raw)
  To: Liu Ying
  Cc: linux-fbdev, Peter Chen, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Fabio Estevam, linux-kernel, stable
In-Reply-To: <1425448735-21490-1-git-send-email-Ying.Liu@freescale.com>

On Wed, Mar 04, 2015 at 01:58:55PM +0800, Liu Ying wrote:
> The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
> as the engine's system clock.  The clock should be enabled when accessing
> LCDIF registers, otherwise the kernel would hang up.  We should also keep
> the clock being enabled when the engine is being active to scan out frames
> from memory.  This patch makes sure the axi clock is enabled when accessing
> registers so that the kernel hang up issue can be fixed.
> 
> Reported-by: Peter Chen <peter.chen@freescale.com>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 56 insertions(+), 14 deletions(-)

<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>

^ permalink raw reply

* Re: [PATCH] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Peter Chen @ 2015-03-04  6:36 UTC (permalink / raw)
  To: Liu Ying
  Cc: linux-fbdev, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Fabio Estevam, linux-kernel, stable
In-Reply-To: <1425448735-21490-1-git-send-email-Ying.Liu@freescale.com>

On Wed, Mar 04, 2015 at 01:58:55PM +0800, Liu Ying wrote:
> The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
> as the engine's system clock.  The clock should be enabled when accessing
> LCDIF registers, otherwise the kernel would hang up.  We should also keep
> the clock being enabled when the engine is being active to scan out frames
> from memory.  This patch makes sure the axi clock is enabled when accessing
> registers so that the kernel hang up issue can be fixed.
> 
> Reported-by: Peter Chen <peter.chen@freescale.com>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
>  drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
> index f8ac4a4..a8cf3b2 100644
> --- a/drivers/video/fbdev/mxsfb.c
> +++ b/drivers/video/fbdev/mxsfb.c
> @@ -316,6 +316,18 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
>  	return 0;
>  }
>  
> +static inline void mxsfb_enable_axi_clk(struct mxsfb_info *host)
> +{
> +	if (host->clk_axi)
> +		clk_prepare_enable(host->clk_axi);
> +}
> +
> +static inline void mxsfb_disable_axi_clk(struct mxsfb_info *host)
> +{
> +	if (host->clk_axi)
> +		clk_disable_unprepare(host->clk_axi);
> +}
> +
>  static void mxsfb_enable_controller(struct fb_info *fb_info)
>  {
>  	struct mxsfb_info *host = to_imxfb_host(fb_info);
> @@ -333,14 +345,13 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
>  		}
>  	}
>  
> -	if (host->clk_axi)
> -		clk_prepare_enable(host->clk_axi);
> -
>  	if (host->clk_disp_axi)
>  		clk_prepare_enable(host->clk_disp_axi);
>  	clk_prepare_enable(host->clk);
>  	clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
>  
> +	mxsfb_enable_axi_clk(host);
> +
>  	/* if it was disabled, re-enable the mode again */
>  	writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
>  
> @@ -380,11 +391,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
>  	reg = readl(host->base + LCDC_VDCTRL4);
>  	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
>  
> +	mxsfb_disable_axi_clk(host);
> +
>  	clk_disable_unprepare(host->clk);
>  	if (host->clk_disp_axi)
>  		clk_disable_unprepare(host->clk_disp_axi);
> -	if (host->clk_axi)
> -		clk_disable_unprepare(host->clk_axi);
>  
>  	host->enabled = 0;
>  
> @@ -421,6 +432,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  		mxsfb_disable_controller(fb_info);
>  	}
>  
> +	mxsfb_enable_axi_clk(host);
> +
>  	/* clear the FIFOs */
>  	writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
>  
> @@ -438,6 +451,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  		ctrl |= CTRL_SET_WORD_LENGTH(3);
>  		switch (host->ld_intf_width) {
>  		case STMLCDIF_8BIT:
> +			mxsfb_disable_axi_clk(host);
>  			dev_err(&host->pdev->dev,
>  					"Unsupported LCD bus width mapping\n");
>  			return -EINVAL;
> @@ -451,6 +465,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
>  		break;
>  	default:
> +		mxsfb_disable_axi_clk(host);
>  		dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
>  				fb_info->var.bits_per_pixel);
>  		return -EINVAL;
> @@ -504,6 +519,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
>  			fb_info->fix.line_length * fb_info->var.yoffset,
>  			host->base + host->devdata->next_buf);
>  
> +	mxsfb_disable_axi_clk(host);
> +
>  	if (reenable)
>  		mxsfb_enable_controller(fb_info);
>  
> @@ -582,10 +599,16 @@ static int mxsfb_pan_display(struct fb_var_screeninfo *var,
>  
>  	offset = fb_info->fix.line_length * var->yoffset;
>  
> +	if (!host->enabled)
> +		mxsfb_enable_axi_clk(host);
> +
>  	/* update on next VSYNC */
>  	writel(fb_info->fix.smem_start + offset,
>  			host->base + host->devdata->next_buf);
>  
> +	if (!host->enabled)
> +		mxsfb_disable_axi_clk(host);
> +
>  	return 0;
>  }
>  
> @@ -608,13 +631,17 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
>  	unsigned line_count;
>  	unsigned period;
>  	unsigned long pa, fbsize;
> -	int bits_per_pixel, ofs;
> +	int bits_per_pixel, ofs, ret = 0;
>  	u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
>  
> +	mxsfb_enable_axi_clk(host);
> +
>  	/* Only restore the mode when the controller is running */
>  	ctrl = readl(host->base + LCDC_CTRL);
> -	if (!(ctrl & CTRL_RUN))
> -		return -EINVAL;
> +	if (!(ctrl & CTRL_RUN)) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
>  
>  	vdctrl0 = readl(host->base + LCDC_VDCTRL0);
>  	vdctrl2 = readl(host->base + LCDC_VDCTRL2);
> @@ -635,7 +662,8 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
>  		break;
>  	case 1:
>  	default:
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err;
>  	}
>  
>  	fb_info->var.bits_per_pixel = bits_per_pixel;
> @@ -673,10 +701,14 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
>  
>  	pa = readl(host->base + host->devdata->cur_buf);
>  	fbsize = fb_info->fix.line_length * vmode->yres;
> -	if (pa < fb_info->fix.smem_start)
> -		return -EINVAL;
> -	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
> -		return -EINVAL;
> +	if (pa < fb_info->fix.smem_start) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
>  	ofs = pa - fb_info->fix.smem_start;
>  	if (ofs) {
>  		memmove(fb_info->screen_base, fb_info->screen_base + ofs, fbsize);
> @@ -689,7 +721,11 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
>  	clk_prepare_enable(host->clk);
>  	host->enabled = 1;
>  
> -	return 0;
> +err:
> +	if (ret)
> +		mxsfb_disable_axi_clk(host);
> +
> +	return ret;
>  }
>  
>  static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
> @@ -915,7 +951,9 @@ static int mxsfb_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!host->enabled) {
> +		mxsfb_enable_axi_clk(host);
>  		writel(0, host->base + LCDC_CTRL);
> +		mxsfb_disable_axi_clk(host);
>  		mxsfb_set_par(fb_info);
>  		mxsfb_enable_controller(fb_info);
>  	}
> @@ -954,11 +992,15 @@ static void mxsfb_shutdown(struct platform_device *pdev)
>  	struct fb_info *fb_info = platform_get_drvdata(pdev);
>  	struct mxsfb_info *host = to_imxfb_host(fb_info);
>  
> +	mxsfb_enable_axi_clk(host);
> +
>  	/*
>  	 * Force stop the LCD controller as keeping it running during reboot
>  	 * might interfere with the BootROM's boot mode pads sampling.
>  	 */
>  	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
> +
> +	mxsfb_disable_axi_clk(host);
>  }
>  
>  static struct platform_driver mxsfb_driver = {
> -- 
> 2.1.0
> 

It fixed the system hang issue at yocto rootfs at imx6sx sdb
and imx6sl sdb boards.

Tested-by: Peter Chen <peter.chen@freescale.com>

-- 

Best Regards,
Peter Chen

^ permalink raw reply

* Re: [PATCH] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Peter Chen @ 2015-03-04  6:57 UTC (permalink / raw)
  To: Greg KH
  Cc: Liu Ying, linux-fbdev, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Fabio Estevam, linux-kernel, stable
In-Reply-To: <20150304060817.GA12241@kroah.com>

On Tue, Mar 03, 2015 at 10:08:17PM -0800, Greg KH wrote:
> On Wed, Mar 04, 2015 at 01:58:55PM +0800, Liu Ying wrote:
> > The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
> > as the engine's system clock.  The clock should be enabled when accessing
> > LCDIF registers, otherwise the kernel would hang up.  We should also keep
> > the clock being enabled when the engine is being active to scan out frames
> > from memory.  This patch makes sure the axi clock is enabled when accessing
> > registers so that the kernel hang up issue can be fixed.
> > 
> > Reported-by: Peter Chen <peter.chen@freescale.com>
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> > ---
> >  drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 56 insertions(+), 14 deletions(-)
> 
> <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>

Hi Greg,

I find this system hang issue at v4.0, and it should exist at v3.19 too.
Do you mean Ying forget to add tag "Cc: stable@vger.kernel.org # 3.19"
at commit log or you think this fix is too large?

-- 

Best Regards,
Peter Chen

^ permalink raw reply

* [PATCH v2] video: mxsfb: Make sure axi clock is enabled when accessing registers
From: Liu Ying @ 2015-03-04  7:06 UTC (permalink / raw)
  To: linux-fbdev
  Cc: Peter Chen, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Fabio Estevam, Greg Kroah-Hartman, linux-kernel, stable

The LCDIF engines embedded in i.MX6sl and i.MX6sx SoCs need the axi clock
as the engine's system clock.  The clock should be enabled when accessing
LCDIF registers, otherwise the kernel would hang up.  We should also keep
the clock being enabled when the engine is being active to scan out frames
from memory.  This patch makes sure the axi clock is enabled when accessing
registers so that the kernel hang up issue can be fixed.

Reported-by: Peter Chen <peter.chen@freescale.com>
Tested-by: Peter Chen <peter.chen@freescale.com>
Cc: <stable@vger.kernel.org> # 3.19+
Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
v1->v2:
* Add 'Tested-by: Peter Chen <peter.chen@freescale.com>' tag.
* Add 'Cc: <stable@vger.kernel.org> # 3.19+' tag.

 drivers/video/fbdev/mxsfb.c | 70 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index f8ac4a4..a8cf3b2 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -316,6 +316,18 @@ static int mxsfb_check_var(struct fb_var_screeninfo *var,
 	return 0;
 }
 
+static inline void mxsfb_enable_axi_clk(struct mxsfb_info *host)
+{
+	if (host->clk_axi)
+		clk_prepare_enable(host->clk_axi);
+}
+
+static inline void mxsfb_disable_axi_clk(struct mxsfb_info *host)
+{
+	if (host->clk_axi)
+		clk_disable_unprepare(host->clk_axi);
+}
+
 static void mxsfb_enable_controller(struct fb_info *fb_info)
 {
 	struct mxsfb_info *host = to_imxfb_host(fb_info);
@@ -333,14 +345,13 @@ static void mxsfb_enable_controller(struct fb_info *fb_info)
 		}
 	}
 
-	if (host->clk_axi)
-		clk_prepare_enable(host->clk_axi);
-
 	if (host->clk_disp_axi)
 		clk_prepare_enable(host->clk_disp_axi);
 	clk_prepare_enable(host->clk);
 	clk_set_rate(host->clk, PICOS2KHZ(fb_info->var.pixclock) * 1000U);
 
+	mxsfb_enable_axi_clk(host);
+
 	/* if it was disabled, re-enable the mode again */
 	writel(CTRL_DOTCLK_MODE, host->base + LCDC_CTRL + REG_SET);
 
@@ -380,11 +391,11 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
 	reg = readl(host->base + LCDC_VDCTRL4);
 	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
+	mxsfb_disable_axi_clk(host);
+
 	clk_disable_unprepare(host->clk);
 	if (host->clk_disp_axi)
 		clk_disable_unprepare(host->clk_disp_axi);
-	if (host->clk_axi)
-		clk_disable_unprepare(host->clk_axi);
 
 	host->enabled = 0;
 
@@ -421,6 +432,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		mxsfb_disable_controller(fb_info);
 	}
 
+	mxsfb_enable_axi_clk(host);
+
 	/* clear the FIFOs */
 	writel(CTRL1_FIFO_CLEAR, host->base + LCDC_CTRL1 + REG_SET);
 
@@ -438,6 +451,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		ctrl |= CTRL_SET_WORD_LENGTH(3);
 		switch (host->ld_intf_width) {
 		case STMLCDIF_8BIT:
+			mxsfb_disable_axi_clk(host);
 			dev_err(&host->pdev->dev,
 					"Unsupported LCD bus width mapping\n");
 			return -EINVAL;
@@ -451,6 +465,7 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 		writel(CTRL1_SET_BYTE_PACKAGING(0x7), host->base + LCDC_CTRL1);
 		break;
 	default:
+		mxsfb_disable_axi_clk(host);
 		dev_err(&host->pdev->dev, "Unhandled color depth of %u\n",
 				fb_info->var.bits_per_pixel);
 		return -EINVAL;
@@ -504,6 +519,8 @@ static int mxsfb_set_par(struct fb_info *fb_info)
 			fb_info->fix.line_length * fb_info->var.yoffset,
 			host->base + host->devdata->next_buf);
 
+	mxsfb_disable_axi_clk(host);
+
 	if (reenable)
 		mxsfb_enable_controller(fb_info);
 
@@ -582,10 +599,16 @@ static int mxsfb_pan_display(struct fb_var_screeninfo *var,
 
 	offset = fb_info->fix.line_length * var->yoffset;
 
+	if (!host->enabled)
+		mxsfb_enable_axi_clk(host);
+
 	/* update on next VSYNC */
 	writel(fb_info->fix.smem_start + offset,
 			host->base + host->devdata->next_buf);
 
+	if (!host->enabled)
+		mxsfb_disable_axi_clk(host);
+
 	return 0;
 }
 
@@ -608,13 +631,17 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 	unsigned line_count;
 	unsigned period;
 	unsigned long pa, fbsize;
-	int bits_per_pixel, ofs;
+	int bits_per_pixel, ofs, ret = 0;
 	u32 transfer_count, vdctrl0, vdctrl2, vdctrl3, vdctrl4, ctrl;
 
+	mxsfb_enable_axi_clk(host);
+
 	/* Only restore the mode when the controller is running */
 	ctrl = readl(host->base + LCDC_CTRL);
-	if (!(ctrl & CTRL_RUN))
-		return -EINVAL;
+	if (!(ctrl & CTRL_RUN)) {
+		ret = -EINVAL;
+		goto err;
+	}
 
 	vdctrl0 = readl(host->base + LCDC_VDCTRL0);
 	vdctrl2 = readl(host->base + LCDC_VDCTRL2);
@@ -635,7 +662,8 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 		break;
 	case 1:
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	fb_info->var.bits_per_pixel = bits_per_pixel;
@@ -673,10 +701,14 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 
 	pa = readl(host->base + host->devdata->cur_buf);
 	fbsize = fb_info->fix.line_length * vmode->yres;
-	if (pa < fb_info->fix.smem_start)
-		return -EINVAL;
-	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len)
-		return -EINVAL;
+	if (pa < fb_info->fix.smem_start) {
+		ret = -EINVAL;
+		goto err;
+	}
+	if (pa + fbsize > fb_info->fix.smem_start + fb_info->fix.smem_len) {
+		ret = -EINVAL;
+		goto err;
+	}
 	ofs = pa - fb_info->fix.smem_start;
 	if (ofs) {
 		memmove(fb_info->screen_base, fb_info->screen_base + ofs, fbsize);
@@ -689,7 +721,11 @@ static int mxsfb_restore_mode(struct mxsfb_info *host,
 	clk_prepare_enable(host->clk);
 	host->enabled = 1;
 
-	return 0;
+err:
+	if (ret)
+		mxsfb_disable_axi_clk(host);
+
+	return ret;
 }
 
 static int mxsfb_init_fbinfo_dt(struct mxsfb_info *host,
@@ -915,7 +951,9 @@ static int mxsfb_probe(struct platform_device *pdev)
 	}
 
 	if (!host->enabled) {
+		mxsfb_enable_axi_clk(host);
 		writel(0, host->base + LCDC_CTRL);
+		mxsfb_disable_axi_clk(host);
 		mxsfb_set_par(fb_info);
 		mxsfb_enable_controller(fb_info);
 	}
@@ -954,11 +992,15 @@ static void mxsfb_shutdown(struct platform_device *pdev)
 	struct fb_info *fb_info = platform_get_drvdata(pdev);
 	struct mxsfb_info *host = to_imxfb_host(fb_info);
 
+	mxsfb_enable_axi_clk(host);
+
 	/*
 	 * Force stop the LCD controller as keeping it running during reboot
 	 * might interfere with the BootROM's boot mode pads sampling.
 	 */
 	writel(CTRL_RUN, host->base + LCDC_CTRL + REG_CLR);
+
+	mxsfb_disable_axi_clk(host);
 }
 
 static struct platform_driver mxsfb_driver = {
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH] video: ARM CLCD: Added dt support to set tim2 register
From: Pawel Moll @ 2015-03-05 10:59 UTC (permalink / raw)
  To: Arun Ramamurthy
  Cc: Rob Herring, Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-fbdev@vger.kernel.org, Dmitry Torokhov, Anatol Pomazau,
	Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list@broadcom.com
In-Reply-To: <54F653B9.4010507@broadcom.com>

On Wed, 2015-03-04 at 00:37 +0000, Arun Ramamurthy wrote:
> > That way you're precisely describing the way the hardware is wired up.
> > And the driver simply tries to get clcdclk first, if it's defined -
> > cool, set clksel to 1, if not - try hclk and set clksel to 0. If neither
> > of them is present - bail out.
> >
> > Does this make any sense?
> >
> This makes sense to me, thank you for the suggestions. I will fix it all 
> up in V2

Cool. Just a word of comment to my own words ;-) The "bail out" case was
a bad idea - the non-DT use cases more likely than not will have no
clock name defined. So, to maintain backward compatibility, the driver
will still have to work when no named clock is available. I think the
simplest way to do that is to check if "hclk" is available, and use it
if it is (setting the clksel accordingly). Otherwise - proceed as it was
the case previously.

Pawel


^ permalink raw reply

* Re: [PATCHv2 04/10] fbdev: ssd1307fb: Unify init code and obtain hw specific bits from DT
From: Maxime Ripard @ 2015-03-05 21:49 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: plagnioj, tomi.valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1425248883-25367-5-git-send-email-niederp@physik.uni-kl.de>

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

On Sun, Mar 01, 2015 at 11:27:57PM +0100, Thomas Niederprüm wrote:
> The SSD130X controllers are very similar from the configuration point of view.
> The configuration registers for the SSD1305/6/7 are bit identical (except the
> the VHCOM register and the the default values for clock setup register). This
> patch unifies the init code of the controller and adds hardware specific
> properties to DT that are needed to correctly initialize the device.
> 
> The SSD130X can be wired to the OLED panel in various ways. Even for the
> same controller this wiring can differ from one display module to another
> and can not be probed by software. The added DT properties reflect these
> hardware decisions of the display module manufacturer.
> The 'com-sequential', 'com-lrremap' and 'com-invdir' values define different
> possibilities for the COM signals pin configuration and readout direction
> of the video memory. The 'segment-remap' allows the inversion of the memory-
> to-pin mapping ultimately inverting the order of the controllers output pins.
> The 'prechargepX' values need to be adapted according the capacitance of the
> OLEDs pixel cells.
> 
> So far these hardware specific bits are hard coded in the init code, making
> the driver usable only for one certain wiring of the controller. This patch
> makes the driver usable with all possible hardware setups, given a valid hw
> description in DT. If the values are not set in DT the default values
> according to the controllers datasheet are assumed.

Unfortunately, this is not a reasonable thing to do, even if you fix
the existing user, there's still the case where you have an older DT
with a newer kernel.

Keeping (and documenting) the previous defaults is the only easy way
to support this.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv2 07/10] fbdev: ssd1307fb: Add module parameter to set refresh rate of the display
From: Maxime Ripard @ 2015-03-05 22:12 UTC (permalink / raw)
  To: Thomas Niederprüm
  Cc: plagnioj, tomi.valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1425248883-25367-8-git-send-email-niederp@physik.uni-kl.de>

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

On Sun, Mar 01, 2015 at 11:28:00PM +0100, Thomas Niederprüm wrote:
> This patch adds the module parameter "refreshrate" to set delay for the
> deferred io. The refresh rate is given in units of Hertz. The default
> refresh rate is 1 Hz.
> 
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
>  drivers/video/fbdev/ssd1307fb.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index c7ed287..ea58e87 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -43,6 +43,11 @@
>  #define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
>  #define	SSD1307FB_SET_VCOMH		0xdb
>  
> +#define REFRASHRATE 1
> +
> +static u_int refreshrate = REFRASHRATE;
> +module_param(refreshrate, uint, 0);
> +
>  static u_int contrast = 128;
>  module_param(contrast, uint, S_IRUGO);
>  
> @@ -270,11 +275,6 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
>  	ssd1307fb_update_display(info->par);
>  }
>  
> -static struct fb_deferred_io ssd1307fb_defio = {
> -	.delay		= HZ,
> -	.deferred_io	= ssd1307fb_deferred_io,
> -};
> -
>  static int ssd1307fb_init(struct ssd1307fb_par *par)
>  {
>  	int ret;
> @@ -475,6 +475,7 @@ static int ssd1307fb_probe(struct i2c_client *client,
>  {
>  	struct fb_info *info;
>  	struct device_node *node = client->dev.of_node;
> +	struct fb_deferred_io *ssd1307fb_defio;
>  	u32 vmem_size;
>  	struct ssd1307fb_par *par;
>  	u8 *vmem;
> @@ -544,10 +545,20 @@ static int ssd1307fb_probe(struct i2c_client *client,
>  		goto fb_alloc_error;
>  	}
>  
> +	ssd1307fb_defio = devm_kzalloc(&client->dev, sizeof(struct fb_deferred_io), GFP_KERNEL);
> +	if (!ssd1307fb_defio) {
> +		dev_err(&client->dev, "Couldn't allocate deferred io.\n");
> +		ret = -ENOMEM;
> +		goto fb_alloc_error;
> +	}
> +
> +	ssd1307fb_defio->delay = HZ/refreshrate;

a space around the operator.

I'm not so sure this is a good solution, since you might perfectly
want to have an SSD1305 with a refreshrate of 1Hz, and an SSD1306 with
a refreshrate of 20Hz.

Unfortunately, beside sysfs, I don't really have a better suggestion.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ 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