From: Krzysztof H. <krz...@wp...> - 2007-07-29 21:03:32
|
From: Krzysztof Helt <krz...@wp...> This patch adds mtrr support to the tdfxfb driver. It also kills one redundant include and initialization value. Signed-off-by: Krzysztof Helt <krz...@wp...> --- 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__ */ |
From: Antonino A. D. <ad...@gm...> - 2007-07-29 23:16:52
|
On Sun, 2007-07-29 at 23:06 +0200, Krzysztof Helt wrote: > From: Krzysztof Helt <krz...@wp...> > > This patch adds mtrr support to the tdfxfb driver. > > It also kills one redundant include and initialization value. > > Signed-off-by: Krzysztof Helt <krz...@wp...> > > --- > > 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 |
From: Krzysztof H. <krz...@wp...> - 2007-07-30 11:55:15
|
On Mon, 30 Jul 2007 07:16:31 +0800 "Antonino A. Daplas" <ad...@gm...> 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 |
From: Krzysztof H. <krz...@wp...> - 2007-07-31 09:52:32
|
From: Krzysztof Helt <krz...@wp...> This patch adds mtrr support to the tdfxfb driver. It also kills one redundant include and initialization value. Signed-off-by: Krzysztof Helt <krz...@wp...> --- 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__ */ |
From: Geert U. <ge...@li...> - 2007-08-02 17:56:25
|
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 <krz...@wp...> > > --- > > 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 -- ge...@li... 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 |
From: Krzysztof H. <krz...@wp...> - 2007-08-03 03:24:59
|
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 |
From: Antonino A. D. <ad...@gm...> - 2007-08-01 08:17:03
|
From: Krzysztof Helt <krz...@wp...> This patch adds mtrr support to the tdfxfb driver. It also kills one redundant include and initialization value. Signed-off-by: Krzysztof Helt <krz...@wp...> Signed-off-by: Antonino Daplas <ad...@gm...> --- drivers/video/tdfxfb.c | 43 +++++++++++++++++++++++++++++++++++++++++-- include/video/tdfx.h | 1 + 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/drivers/video/tdfxfb.c b/drivers/video/tdfxfb.c index c5ba647..9bf4081 100644 --- a/drivers/video/tdfxfb.c +++ b/drivers/video/tdfxfb.c @@ -67,7 +67,6 @@ #include <linux/fb.h> #include <linux/init.h> #include <linux/pci.h> #include <asm/io.h> -#include <linux/spinlock.h> #include <video/tdfx.h> @@ -78,6 +77,24 @@ #else #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 @@ #endif 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); diff --git a/include/video/tdfx.h b/include/video/tdfx.h index 8a2bb91..05b63c2 100644 --- a/include/video/tdfx.h +++ b/include/video/tdfx.h @@ -175,6 +175,7 @@ struct tdfx_par { u32 palette[16]; void __iomem *regbase_virt; unsigned long iobase; + int mtrr_handle; }; #endif /* __KERNEL__ */ |