[PATCH] tdfxfb: mtrr support

[PATCH] tdfxfb: mtrr support

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

This patch adds mtrr support to the tdfxfb driver.

It also kills one redundant include and initialization value.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

---

This patch requires all previous tdfxfb patches sent to this list.

--- linux-2.6.22.old/drivers/video/tdfxfb.c	2007-07-28 23:37:00.000000000 +0200
+++ linux-2.6.22/drivers/video/tdfxfb.c	2007-07-28 23:35:49.000000000 +0200
@@ -67,7 +67,9 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <asm/io.h>
-#include <linux/spinlock.h>
+#ifdef CONFIG_MTRR
+#include <asm/mtrr.h>
+#endif
 
 #include <video/tdfx.h>
 
@@ -150,7 +152,11 @@ MODULE_DEVICE_TABLE(pci, tdfxfb_id_table
 static int nopan;
 static int nowrap = 1;      /* not implemented (yet) */
 static int hwcursor = 1;
-static char *mode_option __devinitdata = NULL;
+static char *mode_option __devinitdata;
+/* mtrr option */
+#ifdef CONFIG_MTRR
+static int nomtrr __devinitdata;
+#endif
 
 /* -------------------------------------------------------------------------
  *                      Hardware-specific funcions
@@ -1227,6 +1233,14 @@ static int __devinit tdfxfb_probe(struct
 
 	printk("fb: %s memory = %dK\n", tdfx_fix.id, tdfx_fix.smem_len >> 10);
 
+#ifdef CONFIG_MTRR
+	default_par->mtrr_handle = -1;
+	if (!nomtrr)
+		default_par->mtrr_handle =
+			mtrr_add(tdfx_fix.smem_start, tdfx_fix.smem_len,
+				 MTRR_TYPE_WRCOMB, 1);
+#endif
+
 	tdfx_fix.ypanstep	= nopan ? 0 : 1;
 	tdfx_fix.ywrapstep	= nowrap ? 0 : 1;
 
@@ -1276,6 +1290,11 @@ static int __devinit tdfxfb_probe(struct
 	return 0;
 
 out_err_iobase:
+#ifdef CONFIG_MTRR
+	if (default_par->mtrr_handle >= 0)
+		mtrr_del(default_par->mtrr_handle, info->fix.smem_start,
+			 info->fix.smem_len);
+#endif
 	release_mem_region(pci_resource_start(pdev, 2),
 			   pci_resource_len(pdev, 2));
 out_err_screenbase:
@@ -1311,6 +1330,10 @@ static void tdfxfb_setup(char *options)
 			nowrap = 1;
 		} else if (!strcmp(this_opt, "hwcursor")) {
 			hwcursor = simple_strtoul(opt + 9, NULL, 0);
+#ifdef CONFIG_MTRR
+		} else if (!strncmp(this_opt, "nomtrr", 6)) {
+			nomtrr = 1;
+#endif
 		} else {
 			mode_option = this_opt;
 		}
@@ -1333,6 +1356,11 @@ static void __devexit tdfxfb_remove(stru
 	struct tdfx_par *par = info->par;
 
 	unregister_framebuffer(info);
+#ifdef CONFIG_MTRR
+	if (par->mtrr_handle >= 0)
+		mtrr_del(par->mtrr_handle, info->fix.smem_start,
+			 info->fix.smem_len);
+#endif
 	iounmap(par->regbase_virt);
 	iounmap(info->screen_base);
 
@@ -1372,6 +1400,10 @@ MODULE_LICENSE("GPL");
 module_param(hwcursor, int, 0644);
 MODULE_PARM_DESC(hwcursor, "Enable hardware cursor "
 			"(1=enable, 0=disable, default=1)");
+#ifdef CONFIG_MTRR
+module_param(nomtrr, bool, 0);
+MODULE_PARM_DESC(nomtrr, "Disable MTRR support (0 or 1=disabled) (default=0)");
+#endif
 
 module_init(tdfxfb_init);
 module_exit(tdfxfb_exit);
--- linux-2.6.22.old/include/video/tdfx.h	2007-07-28 23:37:00.000000000 +0200
+++ linux-2.6.22/include/video/tdfx.h	2007-07-28 23:13:19.000000000 +0200
@@ -175,6 +175,7 @@ struct tdfx_par {
 	u32 palette[16];
 	void __iomem *regbase_virt;
 	unsigned long iobase;
+	int mtrr_handle;
 };
 
 #endif	/* __KERNEL__ */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] tdfxfb: mtrr support

[Antonino A. Daplas]

On Sun, 2007-07-29 at 23:06 +0200, Krzysztof Helt wrote:
> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> This patch adds mtrr support to the tdfxfb driver.
> 
> It also kills one redundant include and initialization value.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> ---
> 
> This patch requires all previous tdfxfb patches sent to this list.
> 
> --- linux-2.6.22.old/drivers/video/tdfxfb.c	2007-07-28 23:37:00.000000000 +0200
> +++ linux-2.6.22/drivers/video/tdfxfb.c	2007-07-28 23:35:49.000000000 +0200
> @@ -67,7 +67,9 @@
>  #include <linux/init.h>
>  #include <linux/pci.h>
>  #include <asm/io.h>
> -#include <linux/spinlock.h>
> +#ifdef CONFIG_MTRR
> +#include <asm/mtrr.h>
> +#endif
>  
>  #include <video/tdfx.h>
>  
> @@ -150,7 +152,11 @@ MODULE_DEVICE_TABLE(pci, tdfxfb_id_table
>  static int nopan;
>  static int nowrap = 1;      /* not implemented (yet) */
>  static int hwcursor = 1;
> -static char *mode_option __devinitdata = NULL;
> +static char *mode_option __devinitdata;
> +/* mtrr option */
> +#ifdef CONFIG_MTRR
> +static int nomtrr __devinitdata;
> +#endif
>  
>  /* -------------------------------------------------------------------------
>   *                      Hardware-specific funcions
> @@ -1227,6 +1233,14 @@ static int __devinit tdfxfb_probe(struct
>  
>  	printk("fb: %s memory = %dK\n", tdfx_fix.id, tdfx_fix.smem_len >> 10);
>  
> +#ifdef CONFIG_MTRR
> +	default_par->mtrr_handle = -1;
> +	if (!nomtrr)
> +		default_par->mtrr_handle =
> +			mtrr_add(tdfx_fix.smem_start, tdfx_fix.smem_len,
> +				 MTRR_TYPE_WRCOMB, 1);
> +#endif
> +

Since akpm already commented against using #ifdef's within functions,
might as well modify the patch to something like this:

#ifdef CONFIG_MTRR
static inline void tdfxfb_mtrr_add(struct fb_info *info)
{
	struct tdfxfb_par *par = info->par;

	par->mtrr_handle =  mtrr_add(info->fix.smem_start,
				 info->fix.smem_len, 
				 MTRR_TYPE_WRCOMB, 1);
}
#else
#define tdfxfb_mtrr_add(...) do {} while (0)
#endif

...tdfxfb_probe(...)
{
	...
	tdfxfb_mtrr_add();
	...
} 

Tony


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] tdfxfb: mtrr support

[Krzysztof Helt]

On Mon, 30 Jul 2007 07:16:31 +0800
"Antonino A. Daplas" <adaplas@gmail.com> wrote:
> Since akpm already commented against using #ifdef's within functions,
> might as well modify the patch to something like this:
> 
> #ifdef CONFIG_MTRR
> static inline void tdfxfb_mtrr_add(struct fb_info *info)
> {
> 	struct tdfxfb_par *par = info->par;
> 
> 	par->mtrr_handle =  mtrr_add(info->fix.smem_start,
> 				 info->fix.smem_len, 
> 				 MTRR_TYPE_WRCOMB, 1);
> }
> #else
> #define tdfxfb_mtrr_add(...) do {} while (0)
> #endif

It doesn't really solve all mtrr #ifdefs problems.

The ultimate solution is to move mtrr.h to include/asm-generic or include/linux
directory.
It has redefined all functions as dummies in case CONFIG_MTRR is not
defined.

Regards,
Krzysztof

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH] tdfxfb: mtrr support (2nd revision)

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>
 
This patch adds mtrr support to the tdfxfb driver.
It also kills one redundant include and initialization value.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>

---

This patch requires all previous tdfxfb patches sent to this list.
It reduces ifdefs to only removing unused "nomtrr" parameter from the code.

--- linux-2.6.22/drivers/video/tdfxfb.c	2007-07-31 11:49:16.921420742 +0200
+++ linux-2.6.23/drivers/video/tdfxfb.c	2007-07-31 11:49:46.147086218 +0200
@@ -67,7 +67,6 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <asm/io.h>
-#include <linux/spinlock.h>
 
 #include <video/tdfx.h>
 
@@ -78,6 +77,24 @@
 #define DPRINTK(a,b...)
 #endif
 
+#ifdef CONFIG_MTRR
+#include <asm/mtrr.h>
+#else
+/* duplicate asm/mtrr.h defines to work on archs without mtrr */
+#define MTRR_TYPE_WRCOMB     1
+
+static __inline__ int mtrr_add (unsigned long base, unsigned long size,
+				unsigned int type, char increment)
+{
+    return -ENODEV;
+}
+static __inline__ int mtrr_del (int reg, unsigned long base,
+				unsigned long size)
+{
+    return -ENODEV;
+}
+#endif
+
 #define BANSHEE_MAX_PIXCLOCK 270000
 #define VOODOO3_MAX_PIXCLOCK 300000
 #define VOODOO5_MAX_PIXCLOCK 350000
@@ -150,7 +167,9 @@ MODULE_DEVICE_TABLE(pci, tdfxfb_id_table
 static int nopan;
 static int nowrap = 1;      /* not implemented (yet) */
 static int hwcursor = 1;
-static char *mode_option __devinitdata = NULL;
+static char *mode_option __devinitdata;
+/* mtrr option */
+static int nomtrr __devinitdata;
 
 /* -------------------------------------------------------------------------
  *                      Hardware-specific funcions
@@ -1227,6 +1246,12 @@ static int __devinit tdfxfb_probe(struct
 
 	printk("fb: %s memory = %dK\n", tdfx_fix.id, tdfx_fix.smem_len >> 10);
 
+	default_par->mtrr_handle = -1;
+	if (!nomtrr)
+		default_par->mtrr_handle =
+			mtrr_add(tdfx_fix.smem_start, tdfx_fix.smem_len,
+				 MTRR_TYPE_WRCOMB, 1);
+
 	tdfx_fix.ypanstep	= nopan ? 0 : 1;
 	tdfx_fix.ywrapstep	= nowrap ? 0 : 1;
 
@@ -1276,6 +1301,9 @@ static int __devinit tdfxfb_probe(struct
 	return 0;
 
 out_err_iobase:
+	if (default_par->mtrr_handle >= 0)
+		mtrr_del(default_par->mtrr_handle, info->fix.smem_start,
+			 info->fix.smem_len);
 	release_mem_region(pci_resource_start(pdev, 2),
 			   pci_resource_len(pdev, 2));
 out_err_screenbase:
@@ -1311,6 +1339,10 @@ static void tdfxfb_setup(char *options)
 			nowrap = 1;
 		} else if (!strcmp(this_opt, "hwcursor")) {
 			hwcursor = simple_strtoul(opt + 9, NULL, 0);
+#ifdef CONFIG_MTRR
+		} else if (!strncmp(this_opt, "nomtrr", 6)) {
+			nomtrr = 1;
+#endif
 		} else {
 			mode_option = this_opt;
 		}
@@ -1333,6 +1365,9 @@ static void __devexit tdfxfb_remove(stru
 	struct tdfx_par *par = info->par;
 
 	unregister_framebuffer(info);
+	if (par->mtrr_handle >= 0)
+		mtrr_del(par->mtrr_handle, info->fix.smem_start,
+			 info->fix.smem_len);
 	iounmap(par->regbase_virt);
 	iounmap(info->screen_base);
 
@@ -1372,6 +1407,10 @@ MODULE_LICENSE("GPL");
 module_param(hwcursor, int, 0644);
 MODULE_PARM_DESC(hwcursor, "Enable hardware cursor "
 			"(1=enable, 0=disable, default=1)");
+#ifdef CONFIG_MTRR
+module_param(nomtrr, bool, 0);
+MODULE_PARM_DESC(nomtrr, "Disable MTRR support (0 or 1=disabled) (default=0)");
+#endif
 
 module_init(tdfxfb_init);
 module_exit(tdfxfb_exit);
--- linux-2.6.22.old/include/video/tdfx.h	2007-07-28 23:37:00.000000000 +0200
+++ linux-2.6.22/include/video/tdfx.h	2007-07-28 23:13:19.000000000 +0200
@@ -175,6 +175,7 @@ struct tdfx_par {
 	u32 palette[16];
 	void __iomem *regbase_virt;
 	unsigned long iobase;
+	int mtrr_handle;
 };
 
 #endif	/* __KERNEL__ */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] tdfxfb: mtrr support (2nd revision)

[Geert Uytterhoeven]

On Tue, 31 Jul 2007, Krzysztof Helt wrote:
> This patch adds mtrr support to the tdfxfb driver.
> It also kills one redundant include and initialization value.
> 
> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> ---
> 
> This patch requires all previous tdfxfb patches sent to this list.
> It reduces ifdefs to only removing unused "nomtrr" parameter from the code.
> 
> --- linux-2.6.22/drivers/video/tdfxfb.c	2007-07-31 11:49:16.921420742 +0200
> +++ linux-2.6.23/drivers/video/tdfxfb.c	2007-07-31 11:49:46.147086218 +0200
> @@ -78,6 +77,24 @@
>  #define DPRINTK(a,b...)
>  #endif
>  
> +#ifdef CONFIG_MTRR
> +#include <asm/mtrr.h>
> +#else
> +/* duplicate asm/mtrr.h defines to work on archs without mtrr */
> +#define MTRR_TYPE_WRCOMB     1
> +
> +static __inline__ int mtrr_add (unsigned long base, unsigned long size,
> +				unsigned int type, char increment)
> +{
> +    return -ENODEV;
> +}
> +static __inline__ int mtrr_del (int reg, unsigned long base,
> +				unsigned long size)
> +{
> +    return -ENODEV;
> +}
> +#endif

While I like your suggestion to move mtrr.h to the include/asm-generic or
include/linux directory, and redefine all functions as dummies in case
CONFIG_MTRR is not defined, the above are not really dummies, as they
return an error code.

> @@ -1227,6 +1246,12 @@ static int __devinit tdfxfb_probe(struct
>  
>  	printk("fb: %s memory = %dK\n", tdfx_fix.id, tdfx_fix.smem_len >> 10);
>  
> +	default_par->mtrr_handle = -1;
> +	if (!nomtrr)
> +		default_par->mtrr_handle =
> +			mtrr_add(tdfx_fix.smem_start, tdfx_fix.smem_len,
> +				 MTRR_TYPE_WRCOMB, 1);
> +

... but everything works nicely, because no callers of mttr_{add,del}()
check for actual errors ;-)

In this case it's a bit tricky to have dummies _and_ check for
failures...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] tdfxfb: mtrr support (2nd revision)

[Krzysztof Helt]

Dnia 2-08-2007 o godz. 19:56 Geert Uytterhoeven napisa³(a):
> On Tue, 31 Jul 2007, Krzysztof Helt wrote:
> >  
> > +	default_par->mtrr_handle = -1;
> > +	if (!nomtrr)
> > +		default_par->mtrr_handle =
> > +			mtrr_add(tdfx_fix.smem_start, tdfx_fix.smem_len,
> > +				 MTRR_TYPE_WRCOMB, 1);
> > +
> 
> ... but everything works nicely, because no callers of 
mttr_{add,del}()
> check for actual errors ;-)
> 
>

Actually, there is an indirect check. The mttr handle is freed 
only if mtrr_add returned no error value. That's why the mtrr_add 
"dummy" must return an error.

I agree, that I do not print any message that inform about 
failure of mtrr_add, but any other fb driver does not, either. I 
think the message does not make any sense (as the CPU may be not 
able to handle this - early x86 - or the mttr is disable in 
config - require more ifdefs for messages).

Regards,
Krzysztof

----------------------------------------------------
06.10.07. Sensation White, Hala Ludowa, Wroc³aw 
Wyst±pi±: Michael Burian, Angelo Mike & John Hetmond,
Sebastian Ingrosso, David Guetta, Markus Schulz, Sander Van Doorn.
Ostatnia pula biletów: http://klik.wp.pl/?adr=http%3A%2F%2Fadv.reklama.wp.pl%2Fas%2Fwhites.html&sid=1245



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/