Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v6 4/4] video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Rob Clark,
	Laurent Pinchart, Jingoo Han, Lad, Prabhakar, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
	Dave Airlie, Antonino Daplas, Jean-Christophe Plagniol-Villard
In-Reply-To: <1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver uses the same area for MTRR as for the ioremap().
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Rob Clark <robdclark@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/vt8623fb.c | 31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index ea7f056..60f24828 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -26,13 +26,9 @@
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
 struct vt8623fb_info {
 	char __iomem *mmio_base;
-	int mtrr_reg;
+	int wc_cookie;
 	struct vgastate state;
 	struct mutex open_lock;
 	unsigned int ref_count;
@@ -99,10 +95,7 @@ static struct svga_timing_regs vt8623_timing_regs     = {
 /* Module parameters */
 
 static char *mode_option = "640x480-8@60";
-
-#ifdef CONFIG_MTRR
 static int mtrr = 1;
-#endif
 
 MODULE_AUTHOR("(c) 2006 Ondrej Zajicek <santiago@crfreenet.org>");
 MODULE_LICENSE("GPL");
@@ -112,11 +105,8 @@ module_param(mode_option, charp, 0644);
 MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
 module_param_named(mode, mode_option, charp, 0);
 MODULE_PARM_DESC(mode, "Default video mode e.g. '648x480-8@60' (deprecated)");
-
-#ifdef CONFIG_MTRR
 module_param(mtrr, int, 0444);
 MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
-#endif
 
 
 /* ------------------------------------------------------------------------- */
@@ -710,7 +700,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	info->fix.mmio_len = pci_resource_len(dev, 1);
 
 	/* Map physical IO memory address into kernel space */
-	info->screen_base = pci_iomap(dev, 0, 0);
+	info->screen_base = pci_iomap_wc(dev, 0, 0);
 	if (! info->screen_base) {
 		rc = -ENOMEM;
 		dev_err(info->device, "iomap for framebuffer failed\n");
@@ -781,12 +771,9 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* Record a reference to the driver data */
 	pci_set_drvdata(dev, 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
+	if (mtrr)
+		par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+						  info->fix.smem_len);
 
 	return 0;
 
@@ -816,13 +803,7 @@ static void vt8623_pci_remove(struct pci_dev *dev)
 	if (info) {
 		struct vt8623fb_info *par = info->par;
 
-#ifdef CONFIG_MTRR
-		if (par->mtrr_reg >= 0) {
-			mtrr_del(par->mtrr_reg, 0, 0);
-			par->mtrr_reg = -1;
-		}
-#endif
-
+		arch_phys_wc_del(par->wc_cookie);
 		unregister_framebuffer(info);
 		fb_dealloc_cmap(&info->cmap);
 
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v6 3/4] video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez,
	Jean-Christophe Plagniol-Villard, Jingoo Han, Geert Uytterhoeven,
	Daniel Vetter, Lad, Prabhakar, Rickard Strandqvist, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Dave Airlie,
	Antonino Daplas
In-Reply-To: <1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This driver uses the same area for MTRR as for the ioremap().
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/s3fb.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index f0ae61a..13b1090 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -28,13 +28,9 @@
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
 struct s3fb_info {
 	int chip, rev, mclk_freq;
-	int mtrr_reg;
+	int wc_cookie;
 	struct vgastate state;
 	struct mutex open_lock;
 	unsigned int ref_count;
@@ -154,11 +150,7 @@ static const struct svga_timing_regs s3_timing_regs     = {
 
 
 static char *mode_option;
-
-#ifdef CONFIG_MTRR
 static int mtrr = 1;
-#endif
-
 static int fasttext = 1;
 
 
@@ -170,11 +162,8 @@ module_param(mode_option, charp, 0444);
 MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
 module_param_named(mode, mode_option, charp, 0444);
 MODULE_PARM_DESC(mode, "Default video mode ('640x480-8@60', etc) (deprecated)");
-
-#ifdef CONFIG_MTRR
 module_param(mtrr, int, 0444);
 MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
-#endif
 
 module_param(fasttext, int, 0644);
 MODULE_PARM_DESC(fasttext, "Enable S3 fast text mode (1=enable, 0=disable, default=1)");
@@ -1168,7 +1157,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	info->fix.smem_len = pci_resource_len(dev, 0);
 
 	/* Map physical IO memory address into kernel space */
-	info->screen_base = pci_iomap(dev, 0, 0);
+	info->screen_base = pci_iomap_wc(dev, 0, 0);
 	if (! info->screen_base) {
 		rc = -ENOMEM;
 		dev_err(info->device, "iomap for framebuffer failed\n");
@@ -1365,12 +1354,9 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* Record a reference to the driver data */
 	pci_set_drvdata(dev, 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
+	if (mtrr)
+		par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+						  info->fix.smem_len);
 
 	return 0;
 
@@ -1405,14 +1391,7 @@ static void s3_pci_remove(struct pci_dev *dev)
 
 	if (info) {
 		par = info->par;
-
-#ifdef CONFIG_MTRR
-		if (par->mtrr_reg >= 0) {
-			mtrr_del(par->mtrr_reg, 0, 0);
-			par->mtrr_reg = -1;
-		}
-#endif
-
+		arch_phys_wc_del(par->wc_cookie);
 		unregister_framebuffer(info);
 		fb_dealloc_cmap(&info->cmap);
 
@@ -1551,10 +1530,8 @@ static int  __init s3fb_setup(char *options)
 
 		if (!*opt)
 			continue;
-#ifdef CONFIG_MTRR
 		else if (!strncmp(opt, "mtrr:", 5))
 			mtrr = simple_strtoul(opt + 5, NULL, 0);
-#endif
 		else if (!strncmp(opt, "fasttext:", 9))
 			fasttext = simple_strtoul(opt + 9, NULL, 0);
 		else
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v6 2/4] video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Laurent Pinchart,
	Geert Uytterhoeven, Lad, Prabhakar, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Antonino Daplas, Jean-Christophe Plagniol-Villard
In-Reply-To: <1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.

There are a few motivations for this:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.

@ mtrr_found @
expression index, base, size;
@@

-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);

@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@

-mtrr_del(index, base, size);
+arch_phys_wc_del(index);

@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@

-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);

@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@

-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);

@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);

@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@

-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);

Generated-by: Coccinelle SmPL
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 drivers/video/fbdev/arkfb.c | 36 +++++-------------------------------
 1 file changed, 5 insertions(+), 31 deletions(-)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index b305a1e..6a317de 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -26,13 +26,9 @@
 #include <linux/console.h> /* Why should fb driver call console functions? because console_lock() */
 #include <video/vga.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
 struct arkfb_info {
 	int mclk_freq;
-	int mtrr_reg;
+	int wc_cookie;
 
 	struct dac_info *dac;
 	struct vgastate state;
@@ -102,10 +98,6 @@ static const struct svga_timing_regs ark_timing_regs     = {
 
 static char *mode_option = "640x480-8@60";
 
-#ifdef CONFIG_MTRR
-static int mtrr = 1;
-#endif
-
 MODULE_AUTHOR("(c) 2007 Ondrej Zajicek <santiago@crfreenet.org>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("fbdev driver for ARK 2000PV");
@@ -115,11 +107,6 @@ MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
 module_param_named(mode, mode_option, charp, 0444);
 MODULE_PARM_DESC(mode, "Default video mode ('640x480-8@60', etc) (deprecated)");
 
-#ifdef CONFIG_MTRR
-module_param(mtrr, int, 0444);
-MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
-#endif
-
 static int threshold = 4;
 
 module_param(threshold, int, 0644);
@@ -1002,7 +989,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	info->fix.smem_len = pci_resource_len(dev, 0);
 
 	/* Map physical IO memory address into kernel space */
-	info->screen_base = pci_iomap(dev, 0, 0);
+	info->screen_base = pci_iomap_wc(dev, 0, 0);
 	if (! info->screen_base) {
 		rc = -ENOMEM;
 		dev_err(info->device, "iomap for framebuffer failed\n");
@@ -1057,14 +1044,8 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	/* Record a reference to the driver data */
 	pci_set_drvdata(dev, 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
-
+	par->wc_cookie = arch_phys_wc_add(info->fix.smem_start,
+					  info->fix.smem_len);
 	return 0;
 
 	/* Error handling */
@@ -1092,14 +1073,7 @@ static void ark_pci_remove(struct pci_dev *dev)
 
 	if (info) {
 		struct arkfb_info *par = info->par;
-
-#ifdef CONFIG_MTRR
-		if (par->mtrr_reg >= 0) {
-			mtrr_del(par->mtrr_reg, 0, 0);
-			par->mtrr_reg = -1;
-		}
-#endif
-
+		arch_phys_wc_del(par->wc_cookie);
 		dac_release(par->dac);
 		unregister_framebuffer(info);
 		fb_dealloc_cmap(&info->cmap);
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v6 1/4] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez, Toshi Kani,
	Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
	Daniel Vetter, Dave Airlie, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Dave Hansen, Arnd Bergmann,
	Michael S. Tsirkin, venkatesh.pallipadi, Stefan Bader,
	Ville Syrjälä, Mel Gorman, Vlastimil Babka,
	Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
	david.vrabel, jbeulich, Roger Pau Monné
In-Reply-To: <1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This allows drivers to take advantage of write-combining
when possible. The PCI specification does not allow for us
to automatically identify a memory region which needs
write-combining so drivers have to identify these areas
on their own. There is IORESOURCE_PREFETCH but as clarified
by Michael and confirmed later by Bjorn, PCI prefetch bit
merely means bridges can combine writes and prefetch reads.
Prefetch does not affect ordering rules and does not allow
writes to be collapsed [0]. WC is stronger, it allows collapsing
and changes ordering rules. WC can also hurt latency as small
writes are buffered. Because of all this drivers needs to
know what they are doing, we can't set a write-combining
preference flag in the pci core automatically for drivers.

Lastly although there is also arch_phys_wc_add() this makes
use of architecture specific write-combining *hacks* and
the only one currently defined and used is MTRR for x86.
MTRRs are legacy, limited in number, have restrictive size
constraints, and are known to interact pooly with the BIOS.
MTRRs should only really be considered on old video framebuffer
drivers. If we made ioremap_wc() and similar calls start
automatically adding MTRRs, then performance will vary wildly
with the order of driver loading because we'll run out of MTRRs
part-way through bootup.

There are a few motivations for phasing out of MTRR and
helping driver change over to use write-combining with PAT:

a) Take advantage of PAT when available

b) Help bury MTRR code away, MTRR is architecture specific and on
   x86 its replaced by PAT

c) Help with the goal of eventually using _PAGE_CACHE_UC over
   _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
   de33c442e titled "x86 PAT: fix performance drop for glx,
   use UC minus for ioremap(), ioremap_nocache() and
   pci_mmap_page_range()")

[0] https://lkml.org/lkml/2015/4/21/714

Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 include/asm-generic/pci_iomap.h | 14 ++++++++++
 lib/pci_iomap.c                 | 61 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 7389c87..b1e17fc 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,9 +15,13 @@ struct pci_dev;
 #ifdef CONFIG_PCI
 /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
 extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max);
 extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
 				     unsigned long offset,
 				     unsigned long maxlen);
+extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+					unsigned long offset,
+					unsigned long maxlen);
 /* Create a virtual mapping cookie for a port on a given PCI device.
  * Do not call this directly, it exists to make it easier for architectures
  * to override */
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
 	return NULL;
 }
 
+static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max)
+{
+	return NULL;
+}
 static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
 					    unsigned long offset,
 					    unsigned long maxlen)
 {
 	return NULL;
 }
+static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+					       unsigned long offset,
+					       unsigned long maxlen)
+{
+	return NULL;
+}
 #endif
 
 #endif /* __ASM_GENERIC_IO_H */
diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
index bcce5f1..9604dcb 100644
--- a/lib/pci_iomap.c
+++ b/lib/pci_iomap.c
@@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev,
 EXPORT_SYMBOL(pci_iomap_range);
 
 /**
+ * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @offset: map memory at the given offset in BAR
+ * @maxlen: max length of the memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using ioread*() and iowrite*(). These functions hide
+ * the details if this is a MMIO or PIO address space and will just do what
+ * you expect from them in the correct way. When possible write combining
+ * is used.
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR from offset to the end, pass %0 here.
+ * */
+void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
+				 int bar,
+				 unsigned long offset,
+				 unsigned long maxlen)
+{
+	resource_size_t start = pci_resource_start(dev, bar);
+	resource_size_t len = pci_resource_len(dev, bar);
+	unsigned long flags = pci_resource_flags(dev, bar);
+
+	if (len <= offset || !start)
+		return NULL;
+	len -= offset;
+	start += offset;
+	if (maxlen && len > maxlen)
+		len = maxlen;
+	if (flags & IORESOURCE_IO)
+		return NULL;
+	if (flags & IORESOURCE_MEM)
+		return ioremap_wc(start, len);
+	/* What? */
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
+
+/**
  * pci_iomap - create a virtual mapping cookie for a PCI BAR
  * @dev: PCI device that owns the BAR
  * @bar: BAR number
@@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 	return pci_iomap_range(dev, bar, 0, maxlen);
 }
 EXPORT_SYMBOL(pci_iomap);
+
+/**
+ * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @maxlen: length of the memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using ioread*() and iowrite*(). These functions hide
+ * the details if this is a MMIO or PIO address space and will just do what
+ * you expect from them in the correct way. When possible write combining
+ * is used.
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR without checking for its length first, pass %0 here.
+ * */
+void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	return pci_iomap_wc_range(dev, bar, 0, maxlen);
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc);
 #endif /* CONFIG_PCI */
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* [PATCH v6 0/4] pci: add and use pci_iomap_wc()
From: Luis R. Rodriguez @ 2015-05-20 23:08 UTC (permalink / raw)
  To: bhelgaas
  Cc: tomi.valkeinen, airlied, linux-fbdev, luto, linux-kernel,
	linux-pci, xen-devel, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Bjorn, this v6 respins the introduction of pci_iomap_wc() and
its users onto Bjorn's pci tree [0] on the next branch. It goes
with your own Acked-by on the PCI part as well as the framebuffer
subsystem maintainer's own Acked-bys for the framebuffer driver
specific changes.

The driver changes are being submitted through the PCI tree
to make the caller have users as soon as its merged otherwise
we'd have to wait 2 release before full integration.

Bjorn, if you rather pull instead of apply these patches one
by one I've put these changes up on a branch on a tree based
on yours to pull from.

The following changes since commit 9b08ae0dde00c20a75cf15636a08e0901c6fe482:

  Merge branch 'pci/msi' into next (2015-05-08 15:05:10 -0500)

are available in the git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-pci.git pci_iomap_wc-v6-20150520

for you to fetch changes up to 480d550fa62bdad1c8f25b16d27d2035f36a283b:

  video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc() (2015-05-20 15:12:04 -0700)

[0] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/

Luis R. Rodriguez (4):
  pci: add pci_iomap_wc() variants
  video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
  video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
  video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()

 drivers/video/fbdev/arkfb.c     | 36 ++++--------------------
 drivers/video/fbdev/s3fb.c      | 35 ++++-------------------
 drivers/video/fbdev/vt8623fb.c  | 31 ++++-----------------
 include/asm-generic/pci_iomap.h | 14 ++++++++++
 lib/pci_iomap.c                 | 61 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 92 insertions(+), 85 deletions(-)

-- 
2.3.2.209.gd67f9d5.dirty

^ permalink raw reply

* Re: [PATCH v3 1/4] pci: add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-05-20 21:06 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Michael S. Tsirkin, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, linux-fbdev, Andy Lutomirski,
	cocci@systeme.lip6.fr, Toshi Kani, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Antonino Daplas, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
	linux-kernel@vger.kernel.org
In-Reply-To: <20150520210234.GD32152@google.com>

On Wed, May 20, 2015 at 2:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Apr 21, 2015 at 01:20:31PM -0700, Luis R. Rodriguez wrote:
>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>
>> This lets drivers take advantage of PAT when available. This
>> should help with the transition of converting video drivers over
>> to ioremap_wc() to help with the goal of eventually using
>> _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
>> ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
>> drop for glx, use UC minus for ioremap(), ioremap_nocache() and
>> pci_mmap_page_range()")
>>
>> Cc: Toshi Kani <toshi.kani@hp.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Suresh Siddha <sbsiddha@gmail.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Antonino Daplas <adaplas@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Ville Syrjälä <syrjala@sci.fi>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Davidlohr Bueso <dbueso@suse.de>
>> Cc: linux-fbdev@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>
> I don't have any objection to this, and I can apply the follow-on patches
> as well, given the appropriate acks and prerequisite patches (ioremap_wc()
> implementation).

OK great, ioremap_wc() has been upstream for a long time. So I'll wait
for Tomi's review of the fbdev changes. Once I get those I can resend
in a series bundled together with the other pending pci / fbdev
changes we just discussed to make things clear.

 Luis

^ permalink raw reply

* Re: [PATCH v3 1/4] pci: add pci_ioremap_wc_bar()
From: Bjorn Helgaas @ 2015-05-20 21:02 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: mst, plagnioj, tomi.valkeinen, linux-fbdev, luto, cocci,
	Luis R. Rodriguez, Toshi Kani, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Antonino Daplas, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso, linux-kernel
In-Reply-To: <1429647634-17169-2-git-send-email-mcgrof@do-not-panic.com>

On Tue, Apr 21, 2015 at 01:20:31PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This lets drivers take advantage of PAT when available. This
> should help with the transition of converting video drivers over
> to ioremap_wc() to help with the goal of eventually using
> _PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
> ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
> drop for glx, use UC minus for ioremap(), ioremap_nocache() and
> pci_mmap_page_range()")
> 
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Ville Syrjälä <syrjala@sci.fi>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>

I don't have any objection to this, and I can apply the follow-on patches
as well, given the appropriate acks and prerequisite patches (ioremap_wc()
implementation).

> ---
>  drivers/pci/pci.c   | 14 ++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 687af72..fd14f10 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -138,6 +138,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>  	return ioremap_nocache(res->start, resource_size(res));
>  }
>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
> +
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
> +{
> +	/*
> +	 * Make sure the BAR is actually a memory resource, not an IO resource
> +	 */
> +	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> +		WARN_ON(1);
> +		return NULL;
> +	}
> +	return ioremap_wc(pci_resource_start(pdev, bar),
> +			  pci_resource_len(pdev, bar));
> +}
> +EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
>  #endif
>  
>  #define PCI_FIND_CAP_TTL	48
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index bc50bb0..490ca41 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1667,6 +1667,7 @@ static inline void pci_mmcfg_late_init(void) { }
>  int pci_ext_cfg_avail(void);
>  
>  void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
> +void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
>  
>  #ifdef CONFIG_PCI_IOV
>  int pci_iov_virtfn_bus(struct pci_dev *dev, int id);
> -- 
> 2.3.2.209.gd67f9d5.dirty
> 

^ permalink raw reply

* Re: [PATCH v4 6/6] video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-20 20:57 UTC (permalink / raw)
  To: Luis R. Rodriguez, Borislav Petkov, Tomi Valkeinen, Bjorn Helgaas
  Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Jean-Christophe Plagniol-Villard, Daniel Vetter, Dave Airlie,
	Doug Ledford, Andy Walls, Ville Syrjälä,
	Andy Lutomirski, Michael S. Tsirkin, cocci@systeme.lip6.fr,
	linux-kernel@vger.kernel.org, Toshi Kani, Suresh Siddha,
	Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
	Rob Clark, Mathias Krause, Andrzej Hajda, Mel Gorman,
	Vlastimil Babka, Davidlohr Bueso, linux-fbdev
In-Reply-To: <20150520195333.GZ23057@wotan.suse.de>

On Wed, May 20, 2015 at 12:53 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> Tomi,
>
> the new required ioremap_uc() which was added in the initial patch set here is
> now merged on linux-next but I just noticed a small issue with this atyfb
> specific patch, I'll fix that and respin and send to you as v5.

Actually since this series depends on ioremap_uc() and since I need to
respin the last patch in this series for atyfb provided I get an
Acked-by from you for the fbdev changes do you mind if this goes
through the x86 tree as that already has the ioremap_uc() required
call? Otherwise we will have one kernel release with that call
available but not used, and we'll have to wait for 2 releases before
these changes get merged.

Thoughts?

 Luis

^ permalink raw reply

* Re: [Cocci] [PATCH v3 0/4] pci: add and use pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-05-20 20:54 UTC (permalink / raw)
  To: cocci
In-Reply-To: <20150519175310.GH23057@wotan.suse.de>

On Tue, May 19, 2015 at 10:53 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Wed, Apr 29, 2015 at 02:08:47PM -0700, Luis R. Rodriguez wrote:
>> On Tue, Apr 21, 2015 at 1:20 PM, Luis R. Rodriguez
>> <mcgrof@do-not-panic.com> wrote:
>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> >
>> > This series adds pci_ioremap_wc_bar() and makes use of it
>> > on a few framebuffer device drivers.
>> >
>> > Luis R. Rodriguez (4):
>> >   pci: add pci_ioremap_wc_bar()
>> >   video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
>> >   video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
>> >   video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
>>
>> Bjorn,
>>
>> any feedback on this series?
>
> Hey Bjorn, any feedback on this series ?

I meant this series. Also Tomi, do I get your Acks for these as well?
Since they depend on pci parts do you mind if we route them through
the PCI tree? I'll poke you soon about another similar type of patch
which has dependencies on another development tree.

 Luis

^ permalink raw reply

* Re: [PATCH v5 1/5] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-05-20 20:52 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: David Airlie, Tomi Valkeinen, Michael S. Tsirkin,
	Jean-Christophe Plagniol-Villard, Dave Airlie, Daniel Vetter,
	linux-fbdev, Andy Lutomirski, cocci@systeme.lip6.fr,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
	Juergen Gross, Daniel Vetter, Antonino Daplas, Dave Hansen,
	Arnd Bergmann, Stefan Bader, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
	Konrad Rzeszutek Wilk, Ville Syrjälä, David Vrabel,
	Jan Beulich, Roger Pau Monné, xen-devel@lists.xensource.com
In-Reply-To: <20150520203936.GC32152@google.com>

On Wed, May 20, 2015 at 1:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, May 19, 2015 at 04:45:30PM -0700, Luis R. Rodriguez wrote:
>> On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote:
>> >
>> >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> >> > [-cc Venkatesh (bouncing)
>> >> >
>> >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez
>> >> > <mcgrof@do-not-panic.com> wrote:
>> >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com>
>> >> >> wrote:
>> >> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> >> >>
>> >> >> Thanks! Who's tree should this go through?
>> >> >
>> >> > I don't know.  This is the only patch that went to linux-pci, so I
>> >> > haven't seen the rest.
>> >>
>> >> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback
>> >> asking for changes.
>> >>
>> >> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you
>> >> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not
>> >> yet used. I replied. This patch can then be ignored but again, I'd
>> >> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL()
>> >> symbol of this.
>
> I'm not really a fan of adding code before it's needed.

OK I'll skip this patch.

> But even when we have a user and that objection is resolved, I'd
> like to have a little more guidance on the difference between
> EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), e.g., a patch to clarify
> what's in Documentation/DocBook/kernel-hacking.tmpl.  I can't
> evaluate that issue based on "current trends and reality"; I need
> something a little more formal.
>
> If we want to allow non-GPL modules and we want to give them a
> consistent kernel interface, even though it might be lacking some
> implementation-specific things, I can follow that guideline.
>
> That's how I read the current kernel-hacking.tmpl text
> ("[EXPORT_SYMBOL_GPL()] implies that the function is considered
> an internal implementation issue, and not really an interface"),
> and that would lead me to suggest EXPORT_SYMBOL() in this case.
>
> If we don't care about consistency, and EXPORT_SYMBOL_GPL()
> should be used at the developer's preference, I can follow that
> guideline instead, but it would be easier if it were made more
> explicit.

I think that's fair for a maintainer to ask, provided that some
maintainers may not be aware of some history, I'll send a patch to
update the documentation to reflect reality. Note that this is not
just about allowing or not proprietary drivers, some folks also take
code from the kernel and use it or sell it on proprietary systems. The
rabbit hole is pretty big.

>> >> Patches v4 3-5 remain intact, I had addressed it to you, but failed to
>> >> Cc linux-pci, I'll go ahead and bounce those now.
>> >>
>> >> Just today Dave Arlie provided a Reviewed-by to some simple
>> >> framebuffer device driver changes. I wonder if these changes should go
>> >> through the framebuffer tree provided you already gave the Acked-by
>> >> for the PCI parts, or if the PCI parts should go in first and only
>> >> later (I guess we'd have to wait) then intake the driver changes that
>> >> use the symbol.
>> >>
>> >> What we decide should likely also apply to the series that adds
>> >> pci_ioremap_wc_bar() and makes use of it on drivers.
>> >>
>> >> Dave, Tomi, any preference?
>> >>
>> >
>> > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi.
>> >
>> > Seems like it could be the simplest path forward.
>>
>> Works with me, Bjorn, are you OK with that?
>
> Can you incorporate the acks and just post a complete v6?  I don't like
> trying to assemble patches from different versions of a series; it's too
> much administrative hassle and too easy for me to screw up.

Yeah sure, will do. While at it, can you also review the patch that
adds pci_ioremap_wc_bar() as well? I'll bundle that series up too.

 Luis

^ permalink raw reply

* Re: [PATCH v5 1/5] pci: add pci_iomap_wc() variants
From: Bjorn Helgaas @ 2015-05-20 20:39 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: David Airlie, Tomi Valkeinen, Michael S. Tsirkin,
	Jean-Christophe Plagniol-Villard, Dave Airlie, Daniel Vetter,
	linux-fbdev, Andy Lutomirski, cocci@systeme.lip6.fr,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
	Juergen Gross, Daniel Vetter, Antonino Daplas, Dave Hansen,
	Arnd Bergmann, Stefan Bader, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
	Konrad Rzeszutek Wilk, Ville Syrjälä, David Vrabel,
	Jan Beulich, Roger Pau Monné, xen-devel@lists.xensource.com
In-Reply-To: <CAB=NE6VkAT7JidB4ty0C1tAWTc__-tgTvzHa5OdPazrDzCv0-g@mail.gmail.com>

On Tue, May 19, 2015 at 04:45:30PM -0700, Luis R. Rodriguez wrote:
> On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote:
> >
> >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> > [-cc Venkatesh (bouncing)
> >> >
> >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez
> >> > <mcgrof@do-not-panic.com> wrote:
> >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com>
> >> >> wrote:
> >> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> >> >>
> >> >> Thanks! Who's tree should this go through?
> >> >
> >> > I don't know.  This is the only patch that went to linux-pci, so I
> >> > haven't seen the rest.
> >>
> >> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback
> >> asking for changes.
> >>
> >> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you
> >> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not
> >> yet used. I replied. This patch can then be ignored but again, I'd
> >> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL()
> >> symbol of this.

I'm not really a fan of adding code before it's needed.

But even when we have a user and that objection is resolved, I'd
like to have a little more guidance on the difference between
EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), e.g., a patch to clarify
what's in Documentation/DocBook/kernel-hacking.tmpl.  I can't
evaluate that issue based on "current trends and reality"; I need
something a little more formal.

If we want to allow non-GPL modules and we want to give them a
consistent kernel interface, even though it might be lacking some
implementation-specific things, I can follow that guideline.

That's how I read the current kernel-hacking.tmpl text
("[EXPORT_SYMBOL_GPL()] implies that the function is considered
an internal implementation issue, and not really an interface"),
and that would lead me to suggest EXPORT_SYMBOL() in this case.

If we don't care about consistency, and EXPORT_SYMBOL_GPL()
should be used at the developer's preference, I can follow that
guideline instead, but it would be easier if it were made more
explicit.

> >> Patches v4 3-5 remain intact, I had addressed it to you, but failed to
> >> Cc linux-pci, I'll go ahead and bounce those now.
> >>
> >> Just today Dave Arlie provided a Reviewed-by to some simple
> >> framebuffer device driver changes. I wonder if these changes should go
> >> through the framebuffer tree provided you already gave the Acked-by
> >> for the PCI parts, or if the PCI parts should go in first and only
> >> later (I guess we'd have to wait) then intake the driver changes that
> >> use the symbol.
> >>
> >> What we decide should likely also apply to the series that adds
> >> pci_ioremap_wc_bar() and makes use of it on drivers.
> >>
> >> Dave, Tomi, any preference?
> >>
> >
> > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi.
> >
> > Seems like it could be the simplest path forward.
> 
> Works with me, Bjorn, are you OK with that?

Can you incorporate the acks and just post a complete v6?  I don't like
trying to assemble patches from different versions of a series; it's too
much administrative hassle and too easy for me to screw up.

Bjorn

^ permalink raw reply

* Re: [PATCH v4 6/6] video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-20 19:53 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: mingo, tglx, hpa, bp, plagnioj, tomi.valkeinen, daniel.vetter,
	airlied, dledford, awalls, syrjala, luto, mst, cocci,
	linux-kernel, Toshi Kani, Suresh Siddha, Juergen Gross,
	Daniel Vetter, Dave Airlie, Antonino Daplas, Rob Clark,
	Mathias Krause, Andrzej Hajda, Mel Gorman, Vlastimil Babka,
	Davidlohr Bueso, linux-fbdev
In-Reply-To: <1430343851-967-7-git-send-email-mcgrof@do-not-panic.com>

Tomi,

the new required ioremap_uc() which was added in the initial patch set here is
now merged on linux-next but I just noticed a small issue with this atyfb
specific patch, I'll fix that and respin and send to you as v5.

 Luis

^ permalink raw reply

* Re: [PATCH v3 2/3] video: fbdev: vesafb: add missing mtrr_del() for added MTRR
From: Luis R. Rodriguez @ 2015-05-20 19:46 UTC (permalink / raw)
  To: cocci
In-Reply-To: <555C594E.1010607@ti.com>

On Wed, May 20, 2015 at 12:52:14PM +0300, Tomi Valkeinen wrote:
> Hi Luis,
> 
> On 21/04/15 23:40, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > The MTRR added was never being deleted.
> > 
> > Cc: Toshi Kani <toshi.kani@hp.com>
> > Cc: Suresh Siddha <sbsiddha@gmail.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Jingoo Han <jg1.han@samsung.com>
> > Cc: Wolfram Sang <wsa@the-dreams.de>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > ---
> >  drivers/video/fbdev/vesafb.c | 30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
> > index 191156b..a2261d0 100644
> > --- a/drivers/video/fbdev/vesafb.c
> > +++ b/drivers/video/fbdev/vesafb.c
> > @@ -29,6 +29,10 @@
> >  
> >  /* --------------------------------------------------------------------- */
> >  
> > +struct vesafb_par {
> > +	int wc_cookie;
> > +};
> > +
> >  static struct fb_var_screeninfo vesafb_defined = {
> >  	.activate	= FB_ACTIVATE_NOW,
> >  	.height		= -1,
> > @@ -175,7 +179,16 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
> >  
> >  static void vesafb_destroy(struct fb_info *info)
> >  {
> > +#ifdef CONFIG_MTRR
> > +	struct vesafb_par *par = info->par;
> > +#endif
> > +
> >  	fb_dealloc_cmap(&info->cmap);
> > +
> > +#ifdef CONFIG_MTRR
> > +	if (par->wc_cookie >= 0)
> > +		mtrr_del(par->wc_cookie, 0, 0);
> > +#endif
> >  	if (info->screen_base)
> >  		iounmap(info->screen_base);
> >  	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
> > @@ -228,6 +241,7 @@ static int vesafb_setup(char *options)
> >  static int vesafb_probe(struct platform_device *dev)
> >  {
> >  	struct fb_info *info;
> > +	struct vesafb_par *par;
> >  	int i, err;
> >  	unsigned int size_vmode;
> >  	unsigned int size_remap;
> > @@ -297,8 +311,8 @@ static int vesafb_probe(struct platform_device *dev)
> >  		return -ENOMEM;
> >  	}
> >  	platform_set_drvdata(dev, info);
> > -	info->pseudo_palette = info->par;
> > -	info->par = NULL;
> > +	info->pseudo_palette = NULL;
> > +	par = info->par;
> >  
> >  	/* set vesafb aperture size for generic probing */
> >  	info->apertures = alloc_apertures(1);
> > @@ -407,17 +421,17 @@ static int vesafb_probe(struct platform_device *dev)
> >  	if (mtrr = 3) {
> >  #ifdef CONFIG_MTRR
> >  		unsigned int temp_size = size_total;
> > -		int rc;
> >  
> >  		/* Find the largest power-of-two */
> >  		temp_size = roundup_pow_of_two(temp_size);
> >  
> >  		/* Try and find a power of two to add */
> >  		do {
> > -			rc = mtrr_add(vesafb_fix.smem_start, temp_size,
> > -				      MTRR_TYPE_WRCOMB, 1);
> > +			par->wc_cookie = mtrr_add(vesafb_fix.smem_start,
> > +						  temp_size,
> > +						  MTRR_TYPE_WRCOMB, 1);
> >  			temp_size >>= 1;
> > -		} while (temp_size >= PAGE_SIZE && rc = -EINVAL);
> > +		} while (temp_size >= PAGE_SIZE && par->wc_cookie = -EINVAL);
> >  #endif
> >  		info->screen_base = ioremap_wc(vesafb_fix.smem_start, vesafb_fix.smem_len);
> >  	} else {
> > @@ -462,6 +476,10 @@ static int vesafb_probe(struct platform_device *dev)
> >  	fb_info(info, "%s frame buffer device\n", info->fix.id);
> >  	return 0;
> >  err:
> > +#ifdef CONFIG_MTRR
> > +	if (par->wc_cookie >= 0)
> > +		mtrr_del(par->wc_cookie, 0, 0);
> > +#endif
> >  	if (info->screen_base)
> >  		iounmap(info->screen_base);
> >  	framebuffer_release(info);
> 
> Hmm, this looks a bit odd... You're removing the pseudo_palette, and
> using its memory for mtrr cookie?

You are totally right, sorry, will spin a v3.

  Luis

^ permalink raw reply

* Re: [PATCH v3 00/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-05-20 18:58 UTC (permalink / raw)
  To: cocci
In-Reply-To: <555C4ADE.6030400@ti.com>

On Wed, May 20, 2015 at 11:50:38AM +0300, Tomi Valkeinen wrote:
> 
> 
> On 14/05/15 17:26, Luis R. Rodriguez wrote:
> 
> > Tomi,
> > 
> > Any chance you can consider this series again? I realize you say you are
> > not familiar with MTRR but given its prevalence use on framebuffer device
> > drivers and since you are a framebuffer subsystem maintainer I think its
> > perhaps fair to ask one of you become familiar with this stuff, or ask
> > questions if there are any. If its of any help this patch which adds
> > ioremap_uc() already got merged and the logic to phase MTRR is described
> > there too:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?idäb6be33c28923d8cde53023e0888b1c5d1a9027
> > 
> > That patch won't even be needed on this series, this series happens to be
> > straight forward: if you just ioremap_nocache() once and use mtrr_add()
> > then conver over to ioremap_wc() and use arch_phys_wc_add(). If it already
> > used ioremap_wc() just convert over from mtrr_add() to arch_phys_wc_add()
> > 
> > Not sure if you will get an Acked-by by x86 folks on this series as they
> > are busy, but I'm adding bp and hpa just in case.
> 
> These are now queued for 4.2. Sorry it took so long. I've been away and
> my bandwidth for the fbdev maintainership has been rather limited lately.

Great thanks for the heads up!

  Luis

^ permalink raw reply

* Re: distorted text
From: Simon Vincent @ 2015-05-20 16:03 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <555C660B.2080808@xsilon.com>

Sorry slightly incorrect I upgraded to linux v4.1-rc4.

I have done some testing and the display works correctly with clear text 
on Linux v4.0 but is distorted/corrupted on v4.1-rc1. (These are tagged 
releases from https://github.com/torvalds/linux).

I have had a look at the changes but I can't find anything that might 
cause the problem although I don't know much about this part of Linux.
Any ideas?

Simon

On 20/05/15 11:46, Simon Vincent wrote:
> Hi,
>
> I have previously been using Linux 3.16 with a OLED 128x32 display 
> setup as a fbconsole. This was working well until I upgraded to linux 
> 4.0. Now the console text is displayed on the OLED but the text is no 
> longer clear, it looks distorted.
> I have uploaded a photo of the problem here. 
> http://s3.postimg.org/66a7lrp2b/IMG_20150520_114135_2.jpg
>
> Is this a known bug?
>
> Simon
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Thierry Reding @ 2015-05-20 12:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kevin Hilman, Scot Doyle, Tomi Valkeinen, Michael Kerrisk,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Pavel Machek,
	Geert Uytterhoeven, lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Tyler Baker, Olof Johansson,
	Daniel Stone, Arnd Bergmann
In-Reply-To: <20150519234112.GA25218-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 3049 bytes --]

On Tue, May 19, 2015 at 04:41:12PM -0700, Greg Kroah-Hartman wrote:
> On Tue, May 19, 2015 at 11:52:29PM +0200, Thierry Reding wrote:
> > On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > > <thierry.reding@gmail.com> wrote:
> > > > On Tue, May 19, 2015 at 02:15:41PM -0700, Kevin Hilman wrote:
> > > >> On Thu, Mar 26, 2015 at 6:56 AM, Scot Doyle <lkml14@scotdoyle.com> wrote:
> > > >> > vt now provides a cursor blink interval via vc_data. Use this
> > > >> > interval instead of the currently hardcoded 200 msecs. Store it in
> > > >> > fbcon_ops to avoid locking the console in cursor_timer_handler().
> > > >> >
> > > >> > Signed-off-by: Scot Doyle <lkml14@scotdoyle.com>
> > > >> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > > >>
> > > >> This patch hit next-20150519 in the form of commit 27a4c827c34a
> > > >> (fbcon: use the cursor blink interval provided by vt) and has caused
> > > >> boot failure on a handful of ARM platforms when booting a MMC root
> > > >> filesystem.  This error was spotted by the kernelci.org bot on
> > > >> exynos5800-peach-pi[1] and Thierry and Daniel (Cc'd) have seen it on
> > > >> some tegra platforms too.
> > > >>
> > > >> Thierry spotted this commit as a potential cause, and both Daniel and
> > > >> I have reverted and boot tested on exynos5 and tegra respectively and
> > > >> the boot panics disappear.
> > > >
> > > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > > back to normal as well:
> > > >
> > > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > > index 05b1d1a71ef9..658c34bb9076 100644
> > > > --- a/drivers/video/console/fbcon.c
> > > > +++ b/drivers/video/console/fbcon.c
> > > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > > >                 return;
> > > >
> > > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > > -       fbcon_del_cursor_timer(info);
> > > > -       if (!(vc->vc_cursor_type & 0x10))
> > > > +       if (vc->vc_cursor_type & 0x10)
> > > > +               fbcon_del_cursor_timer(info);
> > > > +       else
> > > >                 fbcon_add_cursor_timer(info);
> > > >
> > > >         ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
> > > 
> > > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > > 
> > > Tested-by: Kevin Hilman <khilman@linaro.org>
> > 
> > Excellent. Greg, Scot, any opinions on whether or not this is the right
> > thing to do? It restores a bit that looks suspiciously like it snuck in
> > in the original (at least it isn't documented in the commit message).
> > 
> > Greg, feel free to squash this in if everybody agrees this is good to
> > go. If you prefer a patch on top let me know and I'll come up with a
> > proper commit message.
> 
> Please send a real patch and I'll apply it on top, as I can't rebase my
> public tree.

Attached.

Thierry

[-- Attachment #1.2: 0001-fbcon-Avoid-deleting-a-timer-in-IRQ-context.patch --]
[-- Type: text/x-diff, Size: 3279 bytes --]

From 4f2f70dbbe9de54c0da9b03a1f384e1464755eab Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Wed, 20 May 2015 13:41:52 +0200
Subject: [PATCH] fbcon: Avoid deleting a timer in IRQ context

Commit 27a4c827c34a ("fbcon: use the cursor blink interval provided by
vt") unconditionally removes the cursor blink timer. Unfortunately that
wreaks havoc under some circumstances. An easily reproducible way is to
use both the framebuffer console and a debug serial port as the console
output for kernel messages (e.g. "console=ttyS0 console=tty1" on the
kernel command-line. Upon boot this triggers a warning from within the
del_timer_sync() function because it is called from IRQ context:

	[    5.070096] ------------[ cut here ]------------
	[    5.070110] WARNING: CPU: 0 PID: 0 at ../kernel/time/timer.c:1098 del_timer_sync+0x4c/0x54()
	[    5.070115] Modules linked in:
	[    5.070120] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150519 #1
	[    5.070123] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
	[    5.070142] [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
	[    5.070156] [] (show_stack) from [] (dump_stack+0x70/0xbc)
	[    5.070164] [] (dump_stack) from [] (warn_slowpath_common+0x74/0xb0)
	[    5.070169] [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24)
	[    5.070174] [] (warn_slowpath_null) from [] (del_timer_sync+0x4c/0x54)
	[    5.070183] [] (del_timer_sync) from [] (fbcon_del_cursor_timer+0x2c/0x40)
	[    5.070190] [] (fbcon_del_cursor_timer) from [] (fbcon_cursor+0x9c/0x180)
	[    5.070198] [] (fbcon_cursor) from [] (hide_cursor+0x30/0x98)
	[    5.070204] [] (hide_cursor) from [] (vt_console_print+0x2a8/0x340)
	[    5.070212] [] (vt_console_print) from [] (call_console_drivers.constprop.23+0xc8/0xec)
	[    5.070218] [] (call_console_drivers.constprop.23) from [] (console_unlock+0x498/0x4f0)
	[    5.070223] [] (console_unlock) from [] (vprintk_emit+0x1f0/0x508)
	[    5.070228] [] (vprintk_emit) from [] (vprintk_default+0x24/0x2c)
	[    5.070234] [] (vprintk_default) from [] (printk+0x70/0x88)

After which the system starts spewing all kinds of weird and seemingly
unrelated error messages.

This commit fixes this by restoring the condition under which the call
to fbcon_del_cursor_timer() happens.

Reported-by: Daniel Stone <daniel@fooishbar.org>
Reported-by: Kevin Hilman <khilman@kernel.org>
Tested-by: Kevin Hilman <khilman@linaro.org>
Tested-by: Scot Doyle <lkml14@scotdoyle.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/video/console/fbcon.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 05b1d1a71ef9..658c34bb9076 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 		return;
 
 	ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
-	fbcon_del_cursor_timer(info);
-	if (!(vc->vc_cursor_type & 0x10))
+	if (vc->vc_cursor_type & 0x10)
+		fbcon_del_cursor_timer(info);
+	else
 		fbcon_add_cursor_timer(info);
 
 	ops->cursor_flash = (mode == CM_ERASE) ? 0 : 1;
-- 
2.4.1


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

^ permalink raw reply related

* Re: [PATCH v3 06/17] video: lcd: add LoCoMo LCD driver
From: Jingoo Han @ 2015-05-20 12:26 UTC (permalink / raw)
  To: 'Dmitry Eremin-Solenikov', 'Lee Jones'
  Cc: 'Russell King', 'Daniel Mack',
	'Robert Jarzmik', 'Linus Walleij',
	'Alexandre Courbot', 'Wolfram Sang',
	'Dmitry Torokhov', 'Bryan Wu',
	'Richard Purdie', 'Samuel Ortiz',
	'Mark Brown', 'Jean-Christophe PLAGNIOL-VILLARD',
	'Tomi Valkeinen', 'Liam Girdwood',
	'Andrea Adami', linux-arm-kernel, linux-gpiol,
	linux-input, linux-leds, linux-spi, linux-fbdev, alsa-devel
In-Reply-To: <1431880077-26321-7-git-send-email-dbaryshkov@gmail.com>

> LoCoMo has some special handling for TFT screens attached to Collie and
> Poodle. Implement that as a separate driver.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> ---
>  drivers/video/backlight/Kconfig      |  10 ++
>  drivers/video/backlight/Makefile     |   1 +
>  drivers/video/backlight/locomo_lcd.c | 285 +++++++++++++++++++++++++++++++++++
>  3 files changed, 296 insertions(+)
>  create mode 100644 drivers/video/backlight/locomo_lcd.c



^ permalink raw reply

* distorted text
From: Simon Vincent @ 2015-05-20 10:46 UTC (permalink / raw)
  To: linux-fbdev

Hi,

I have previously been using Linux 3.16 with a OLED 128x32 display setup 
as a fbconsole. This was working well until I upgraded to linux 4.0. Now 
the console text is displayed on the OLED but the text is no longer 
clear, it looks distorted.
I have uploaded a photo of the problem here. 
http://s3.postimg.org/66a7lrp2b/IMG_20150520_114135_2.jpg

Is this a known bug?

Simon

^ permalink raw reply

* Re: [PATCH v3 2/3] video: fbdev: vesafb: add missing mtrr_del() for added MTRR
From: Tomi Valkeinen @ 2015-05-20  9:52 UTC (permalink / raw)
  To: Luis R. Rodriguez, plagnioj, hpa
  Cc: linux-fbdev, luto, cocci, Luis R. Rodriguez, Toshi Kani,
	Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
	Daniel Vetter, Dave Airlie, Antonino Daplas, Rob Clark,
	Jingoo Han, Wolfram Sang, linux-kernel
In-Reply-To: <1429648850-17902-3-git-send-email-mcgrof@do-not-panic.com>

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

Hi Luis,

On 21/04/15 23:40, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> The MTRR added was never being deleted.
> 
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  drivers/video/fbdev/vesafb.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
> index 191156b..a2261d0 100644
> --- a/drivers/video/fbdev/vesafb.c
> +++ b/drivers/video/fbdev/vesafb.c
> @@ -29,6 +29,10 @@
>  
>  /* --------------------------------------------------------------------- */
>  
> +struct vesafb_par {
> +	int wc_cookie;
> +};
> +
>  static struct fb_var_screeninfo vesafb_defined = {
>  	.activate	= FB_ACTIVATE_NOW,
>  	.height		= -1,
> @@ -175,7 +179,16 @@ static int vesafb_setcolreg(unsigned regno, unsigned red, unsigned green,
>  
>  static void vesafb_destroy(struct fb_info *info)
>  {
> +#ifdef CONFIG_MTRR
> +	struct vesafb_par *par = info->par;
> +#endif
> +
>  	fb_dealloc_cmap(&info->cmap);
> +
> +#ifdef CONFIG_MTRR
> +	if (par->wc_cookie >= 0)
> +		mtrr_del(par->wc_cookie, 0, 0);
> +#endif
>  	if (info->screen_base)
>  		iounmap(info->screen_base);
>  	release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size);
> @@ -228,6 +241,7 @@ static int vesafb_setup(char *options)
>  static int vesafb_probe(struct platform_device *dev)
>  {
>  	struct fb_info *info;
> +	struct vesafb_par *par;
>  	int i, err;
>  	unsigned int size_vmode;
>  	unsigned int size_remap;
> @@ -297,8 +311,8 @@ static int vesafb_probe(struct platform_device *dev)
>  		return -ENOMEM;
>  	}
>  	platform_set_drvdata(dev, info);
> -	info->pseudo_palette = info->par;
> -	info->par = NULL;
> +	info->pseudo_palette = NULL;
> +	par = info->par;
>  
>  	/* set vesafb aperture size for generic probing */
>  	info->apertures = alloc_apertures(1);
> @@ -407,17 +421,17 @@ static int vesafb_probe(struct platform_device *dev)
>  	if (mtrr == 3) {
>  #ifdef CONFIG_MTRR
>  		unsigned int temp_size = size_total;
> -		int rc;
>  
>  		/* Find the largest power-of-two */
>  		temp_size = roundup_pow_of_two(temp_size);
>  
>  		/* Try and find a power of two to add */
>  		do {
> -			rc = mtrr_add(vesafb_fix.smem_start, temp_size,
> -				      MTRR_TYPE_WRCOMB, 1);
> +			par->wc_cookie = mtrr_add(vesafb_fix.smem_start,
> +						  temp_size,
> +						  MTRR_TYPE_WRCOMB, 1);
>  			temp_size >>= 1;
> -		} while (temp_size >= PAGE_SIZE && rc == -EINVAL);
> +		} while (temp_size >= PAGE_SIZE && par->wc_cookie == -EINVAL);
>  #endif
>  		info->screen_base = ioremap_wc(vesafb_fix.smem_start, vesafb_fix.smem_len);
>  	} else {
> @@ -462,6 +476,10 @@ static int vesafb_probe(struct platform_device *dev)
>  	fb_info(info, "%s frame buffer device\n", info->fix.id);
>  	return 0;
>  err:
> +#ifdef CONFIG_MTRR
> +	if (par->wc_cookie >= 0)
> +		mtrr_del(par->wc_cookie, 0, 0);
> +#endif
>  	if (info->screen_base)
>  		iounmap(info->screen_base);
>  	framebuffer_release(info);

Hmm, this looks a bit odd... You're removing the pseudo_palette, and
using its memory for mtrr cookie?

 Tomi


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

^ permalink raw reply

* Re: [PATCH v5 1/5] pci: add pci_iomap_wc() variants
From: Tomi Valkeinen @ 2015-05-20  9:04 UTC (permalink / raw)
  To: Luis R. Rodriguez, David Airlie
  Cc: Bjorn Helgaas, Michael S. Tsirkin,
	Jean-Christophe Plagniol-Villard, Dave Airlie, Daniel Vetter,
	linux-fbdev, Andy Lutomirski, cocci@systeme.lip6.fr,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
	Juergen Gross, Daniel Vetter, Antonino Daplas, Dave Hansen,
	Arnd Bergmann, Stefan Bader, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
	Konrad Rzeszutek Wilk, Ville Syrjälä, David Vrabel,
	Jan Beulich, Roger Pau Monné, xen-devel@lists.xensource.com
In-Reply-To: <CAB=NE6VkAT7JidB4ty0C1tAWTc__-tgTvzHa5OdPazrDzCv0-g@mail.gmail.com>

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



On 20/05/15 02:45, Luis R. Rodriguez wrote:
> On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote:
>>
>>> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>> [-cc Venkatesh (bouncing)
>>>>
>>>> On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez
>>>> <mcgrof@do-not-panic.com> wrote:
>>>>> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com>
>>>>> wrote:
>>>>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>>>>>
>>>>> Thanks! Who's tree should this go through?
>>>>
>>>> I don't know.  This is the only patch that went to linux-pci, so I
>>>> haven't seen the rest.
>>>
>>> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback
>>> asking for changes.
>>>
>>> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you
>>> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not
>>> yet used. I replied. This patch can then be ignored but again, I'd
>>> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL()
>>> symbol of this.
>>>
>>> Patches v4 3-5 remain intact, I had addressed it to you, but failed to
>>> Cc linux-pci, I'll go ahead and bounce those now.
>>>
>>> Just today Dave Arlie provided a Reviewed-by to some simple
>>> framebuffer device driver changes. I wonder if these changes should go
>>> through the framebuffer tree provided you already gave the Acked-by
>>> for the PCI parts, or if the PCI parts should go in first and only
>>> later (I guess we'd have to wait) then intake the driver changes that
>>> use the symbol.
>>>
>>> What we decide should likely also apply to the series that adds
>>> pci_ioremap_wc_bar() and makes use of it on drivers.
>>>
>>> Dave, Tomi, any preference?
>>>
>>
>> Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi.
>>
>> Seems like it could be the simplest path forward.
> 
> Works with me, Bjorn, are you OK with that?
> 
> Dave, Tomi, I still need your Reviewed-by and Acked-by's Tomi on these
> two PCI series. Please let me know if I should resend those first, but
> I think I had Cc'd you folks already on them.

For the fb parts:

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

I don't have anything in my fbdev-next branch touching the fbdev drivers
changed in this series, so merging via some other tree might go fine
without conflicts.

 Tomi


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

^ permalink raw reply

* Re: [PATCH v3 00/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Tomi Valkeinen @ 2015-05-20  8:50 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1429647398-16983-1-git-send-email-mcgrof@do-not-panic.com>

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



On 14/05/15 17:26, Luis R. Rodriguez wrote:

> Tomi,
> 
> Any chance you can consider this series again? I realize you say you are
> not familiar with MTRR but given its prevalence use on framebuffer device
> drivers and since you are a framebuffer subsystem maintainer I think its
> perhaps fair to ask one of you become familiar with this stuff, or ask
> questions if there are any. If its of any help this patch which adds
> ioremap_uc() already got merged and the logic to phase MTRR is described
> there too:
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=e4b6be33c28923d8cde53023e0888b1c5d1a9027
> 
> That patch won't even be needed on this series, this series happens to be
> straight forward: if you just ioremap_nocache() once and use mtrr_add()
> then conver over to ioremap_wc() and use arch_phys_wc_add(). If it already
> used ioremap_wc() just convert over from mtrr_add() to arch_phys_wc_add()
> 
> Not sure if you will get an Acked-by by x86 folks on this series as they
> are busy, but I'm adding bp and hpa just in case.

These are now queued for 4.2. Sorry it took so long. I've been away and
my bandwidth for the fbdev maintainership has been rather limited lately.

 Tomi


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

^ permalink raw reply

* Re: [Cocci] [PATCH v3 00/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Tomi Valkeinen @ 2015-05-20  8:45 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150519175802.GJ23057@wotan.suse.de>

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

Hi,

On 20/05/15 01:17, Dave Airlie wrote:
> On 20 May 2015 at 03:58, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> On Thu, May 14, 2015 at 04:26:45PM +0200, Luis R. Rodriguez wrote:
>>> On Tue, May 05, 2015 at 06:47:31AM -0700, Andy Lutomirski wrote:
>>>> On Mon, May 4, 2015 at 3:33 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>>> On 29/04/15 22:18, Luis R. Rodriguez wrote:
>>>>>> On Tue, Apr 21, 2015 at 1:16 PM, Luis R. Rodriguez
>>>>>> <mcgrof@do-not-panic.com> wrote:
>>>>>>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>>>>>>
>>>>>>> This series addresses simple changes to framebuffer drivers to use
>>>>>>> arch_phys_wc_add() and ioremap_wc() as well as fixing gbefb to add
>>>>>>> missing mtrr_del() calls. These changes are pretty straight forward.
>>>>>>>
>>>>>>> Luis R. Rodriguez (17):
>>>>>>>   video: fbdev: radeonfb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: gbefb: add missing mtrr_del() calls
>>>>>>>   video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
>>>>>>>   video: fbdev: intelfb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: neofb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: nvidia: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: savagefb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: sisfb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: aty: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: i810: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: pm2fb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: pm3fb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: rivafb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: tdfxfb: use arch_phys_wc_add() and ioremap_wc()
>>>>>>>   video: fbdev: atmel_lcdfb: use ioremap_wc() for framebuffer
>>>>>>>   video: fbdev: geode gxfb: use ioremap_wc() for framebuffer
>>>>>>
> 
> These 17 are all
> 
> Reviewed-by: Dave Airlie <airlied@redhat.com>
> 
> Tomi, can you please take them via the fbdev tree?

Thanks. Yep, I'll queue them for 4.2.

 Tomi


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

^ permalink raw reply

* Re: [PATCH] video: exynos: fix modular build
From: Paul Bolle @ 2015-05-20  7:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2708563.ADEZz3igC5@wuerfel>

On Tue, 2015-05-19 at 14:17 +0200, Arnd Bergmann wrote:
> Finally, the EXYNOS_VIDEO option is turned into tristate as well
> for good measure, as all framebuffer drivers should be configurable
> as modules.

EXYNOS_MIPI_DSI and EXYNOS_LCD_S6E8AX0 are made tristate too. And that
is, I think, what counts. See below.

For anyone wondering how these two drivers can be made modular with only
a few tweaks to the build system: that is because exynos_mipi_dsi.c and
s6e8ax0.c already contain module specific boilerplate. They probably
already contained that boilerplate when they were added three years ago.
(Perhaps the changelog should mention this.)

> --- a/drivers/video/fbdev/exynos/Kconfig
> +++ b/drivers/video/fbdev/exynos/Kconfig

>  menuconfig EXYNOS_VIDEO
> -	bool "Exynos Video driver support"
> +	tristate "Exynos Video driver support"
>  	depends on ARCH_S5PV210 || ARCH_EXYNOS
>  	help
>  	  This enables support for EXYNOS Video device.

I have tested this only lightly but I do think this hunk is not needed.
Because basically all that this entry does is making EXYNOS_MIPI_DSI and
EXYNOS_LCD_S6E8AX0 available. And that works just as well if it's a
bool. Correct?

>  config EXYNOS_MIPI_DSI
> -	bool "EXYNOS MIPI DSI driver support."
> +	tristate "EXYNOS MIPI DSI driver support."
>  	select GENERIC_PHY
>  	help
>  	  This enables support for MIPI-DSI device.
>  
>  config EXYNOS_LCD_S6E8AX0
> -	bool "S6E8AX0 MIPI AMOLED LCD Driver"
> +	tristate "S6E8AX0 MIPI AMOLED LCD Driver"
>  	depends on EXYNOS_MIPI_DSI && BACKLIGHT_CLASS_DEVICE
>  	depends on (LCD_CLASS_DEVICE = y)
>  	default n

> --- a/drivers/video/fbdev/exynos/Makefile
> +++ b/drivers/video/fbdev/exynos/Makefile

> -obj-$(CONFIG_EXYNOS_MIPI_DSI)		+= exynos_mipi_dsi.o exynos_mipi_dsi_common.o \
> -				     	exynos_mipi_dsi_lowlevel.o
> +obj-$(CONFIG_EXYNOS_MIPI_DSI)		+= exynos-mipi-dsi-mod.o
> +
> +exynos-mipi-dsi-mod-objs		+= exynos_mipi_dsi.o exynos_mipi_dsi_common.o \
> +					   exynos_mipi_dsi_lowlevel.o

I don't speak Makefilese fluently, so I have to ask. Is the -mod
extension needed because a module built from multiple files can't have a
name that matches the name of one of its .c files (minus the .c
extension, of course)?

>  obj-$(CONFIG_EXYNOS_LCD_S6E8AX0)	+= s6e8ax0.o

Thanks,


Paul Bolle


^ permalink raw reply

* Re: [PATCH] video: exynos: fix modular build
From: Krzysztof Kozlowski @ 2015-05-20  1:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2708563.ADEZz3igC5@wuerfel>

On 19.05.2015 21:17, Arnd Bergmann wrote:
> The s6e8ax0 driver has a dependency on BACKLIGHT_CLASS_DEVICE,
> which can be configured as a loadable module, so we have to
> make the driver a tristate symbol as well, to avoid this error:
> 
> drivers/built-in.o: In function `s6e8ax0_probe':
> :(.text+0x23a48): undefined reference to `devm_backlight_device_register'
> 
> This also means we get another error from a missing export, which
> this fixes as well:
> 
> ERROR: "exynos_mipi_dsi_register_lcd_driver" [drivers/video/fbdev/exynos/s6e8ax0.ko] undefined!
> 
> Finally, the EXYNOS_VIDEO option is turned into tristate as well
> for good measure, as all framebuffer drivers should be configurable
> as modules.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Looks good:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH v2 2/3] fbcon: use the cursor blink interval provided by vt
From: Scot Doyle @ 2015-05-20  0:36 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kevin Hilman, Scot Doyle, Greg Kroah-Hartman, Tomi Valkeinen,
	Michael Kerrisk, Jiri Slaby, Jean-Christophe Plagniol-Villard,
	Pavel Machek, Geert Uytterhoeven, lkml, linux-fbdev, linux-man,
	linux-api, Tyler Baker, Olof Johansson, Daniel Stone,
	Arnd Bergmann
In-Reply-To: <20150519215228.GA27163@ulmo.nvidia.com>

On Tue, 19 May 2015, Thierry Reding wrote:
> On Tue, May 19, 2015 at 02:45:19PM -0700, Kevin Hilman wrote:
> > On Tue, May 19, 2015 at 2:40 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:

...

> > >
> > > FWIW, if I apply the below on top of next-20150519 things seem to be
> > > back to normal as well:
> > >
> > > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > > index 05b1d1a71ef9..658c34bb9076 100644
> > > --- a/drivers/video/console/fbcon.c
> > > +++ b/drivers/video/console/fbcon.c
> > > @@ -1310,8 +1310,9 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
> > >                 return;
> > >
> > >         ops->cur_blink_jiffies = msecs_to_jiffies(vc->vc_cur_blink_ms);
> > > -       fbcon_del_cursor_timer(info);
> > > -       if (!(vc->vc_cursor_type & 0x10))
> > > +       if (vc->vc_cursor_type & 0x10)
> > > +               fbcon_del_cursor_timer(info);
> > > +       else
> > >                 fbcon_add_cursor_timer(info);
> > >
> > >         ops->cursor_flash = (mode = CM_ERASE) ? 0 : 1;
> > 
> > Applying this on next-20150519 makes my exynos board happily boot again as well.
> > 
> > Tested-by: Kevin Hilman <khilman@linaro.org>
> 
> Excellent. Greg, Scot, any opinions on whether or not this is the right
> thing to do? It restores a bit that looks suspiciously like it snuck in
> in the original (at least it isn't documented in the commit message).
> 
> Greg, feel free to squash this in if everybody agrees this is good to
> go. If you prefer a patch on top let me know and I'll come up with a
> proper commit message.
> 
> Thierry

Hi all, sorry for the trouble.

The timer delete was to prevent blink stutter when updating the interval. 
Since the stutter isn't so noticable when changing from the default 200ms, 
and since most people seem to prefer leaving the fbcon code alone if 
possible, I agree with Thierry's approach.

Tested-by: Scot Doyle <lkml14@scotdoyle.com>


^ 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