- video/tdfxfb.c warning fix.

PATCH In review Luiz Fernando N. Capitulino 3 participants 8 messages Apr 30, 2004 – May 3, 2004 Framebuffer layer

lore

Share & tools

lore

Luiz Fernando N. Capitulino

author

2004-04-30 18:07 UTC #0 lore raw

PATCH 1/1 - video/tdfxfb.c warning fix.

Hi Andrew,

This patch fixes this warning:

drivers/video/tdfxfb.c:1005: warning: `tdfxfb_cursor' defined but not used

and make the acceleration function selectable (like hgafb
and tridentfb):

 drivers/video/Kconfig  |    8 ++++++++
 drivers/video/tdfxfb.c |    8 +++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

drivers/video/Kconfig +8 -0

diff -X dontdiff -Nparu a/drivers/video/Kconfig a~/drivers/video/Kconfig
@@ -751,6 +751,14 @@ config FB_3DFX
 	  To compile this driver as a module, choose M here: the
 	  module will be called tdfxfb.
 
+config FB_3DFX_ACCEL
+	bool "3Dfx Banshee/Voodoo3 Acceleration functions (EXPERIMENTAL)"
+	depends on FB_3DFX && EXPERIMENTAL
+	---help---
+	This will compile the 3Dfx Banshee/Voodoo3 frame buffer device
+	with acceleration functions.
+
+
 config FB_VOODOO1
 	tristate "3Dfx Voodoo Graphics (sst1) support"
 	depends on FB && PCI

drivers/video/tdfxfb.c +7 -1

diff -X dontdiff -Nparu a/drivers/video/tdfxfb.c a~/drivers/video/tdfxfb.c
@@ -166,7 +166,11 @@ static int tdfxfb_pan_display(struct fb_
 static void tdfxfb_fillrect(struct fb_info *info, const struct fb_fillrect *rect);
 static void tdfxfb_copyarea(struct fb_info *info, const struct fb_copyarea *area);  
 static void tdfxfb_imageblit(struct fb_info *info, const struct fb_image *image); 
+#ifdef CONFIG_FB_3DFX_ACCEL
 static int tdfxfb_cursor(struct fb_info *info, struct fb_cursor *cursor);
+#else /* !CONFIG_FB_3DFX_ACCEL */
+#define tdfxfb_cursor soft_cursor
+#endif /* CONFIG_FB_3DFX_ACCEL */
 static int banshee_wait_idle(struct fb_info *info);
 
 static struct fb_ops tdfxfb_ops = {

@@ -180,7 +184,7 @@ static struct fb_ops tdfxfb_ops = {
 	.fb_copyarea	= tdfxfb_copyarea,
 	.fb_imageblit	= tdfxfb_imageblit,
 	.fb_sync	= banshee_wait_idle,
-	.fb_cursor	= soft_cursor,
+	.fb_cursor	= tdfxfb_cursor,
 };
 
 /*

@@ -1001,6 +1005,7 @@ static void tdfxfb_imageblit(struct fb_i
 	banshee_wait_idle(info);
 }
 
+#ifdef CONFIG_FB_3DFX_ACCEL
 static int tdfxfb_cursor(struct fb_info *info, struct fb_cursor *cursor)
 {
 	struct tdfx_par *par = (struct tdfx_par *) info->par;

@@ -1137,6 +1142,7 @@ static int tdfxfb_cursor(struct fb_info 
 	spin_unlock_irqrestore(&par->DAClock, flags);
 	return 0;
 }
+#endif /* CONFIG_FB_3DFX_ACCEL */
 
 /**
  *      tdfxfb_probe - Device Initializiation

Signature · 9 lines

Luiz Fernando N. Capitulino
<http://www.telecentros.sp.gov.br>


-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Andrew Morton

2004-05-01 04:35 UTC #1 lore raw

Quoted 9 lines · Luiz Fernando N. Capitulino

This patch fixes this warning:
drivers/video/tdfxfb.c:1005: warning: `tdfxfb_cursor' defined but not used
and make the acceleration function selectable (like hgafb
and tridentfb):

You've taken a bunch of previously-working 3d acceleration functions and made them configurable, dependent upon CONFIG_EXPERIMENTAL.

What on earth for? Will this not gratuitously break people's previously-working 3d setups?

-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. Take an Oracle 10g class now, and we'll give you the exam FREE.
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click

Geert Uytterhoeven

2004-05-01 18:47 UTC #2 lore raw

Quoted 6 lines · Andrew Morton

Quoted 5 lines · Luiz Fernando N. Capitulino
This patch fixes this warning:
drivers/video/tdfxfb.c:1005: warning: `tdfxfb_cursor' defined but not used

Quoted 8 lines

Quoted 2 lines
and make the acceleration function selectable (like hgafb
and tridentfb):
You've taken a bunch of previously-working 3d acceleration functions and made them configurable, dependent upon CONFIG_EXPERIMENTAL.
What on earth for? Will this not gratuitously break people's previously-working 3d setups?

No, tdfxfb_cursor() was not used before, causing a compiler warning. tdfxfb_cursor() may work, but we don't know, so we didn't dare to enable it by default. Now the user (he who has the hardware) can enable it, and tell us whether it works or not.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Andrew Morton

2004-05-01 19:09 UTC #3 lore raw

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines
You've taken a bunch of previously-working 3d acceleration functions and made them configurable, dependent upon CONFIG_EXPERIMENTAL.
What on earth for? Will this not gratuitously break people's previously-working 3d setups?
No, tdfxfb_cursor() was not used before, causing a compiler warning. tdfxfb_cursor() may work, but we don't know, so we didn't dare to enable it by default. Now the user (he who has the hardware) can enable it, and tell us whether it works or not.

OK, thanks. As you can see, it really helps if people explain this sort of thing in their initial patch (sigh).

Does the same argument apply to this patch?

From: "Luiz Fernando N. Capitulino" <[email protected]>

Speaking with frame buffer people, we agree with this patch to fix the warning:

drivers/video/tridentfb.c:455: warning: tridentfb_fillrect' defined but not used drivers/video/tridentfb.c:473: warning: tridentfb_copyarea' defined but not used

---

 25-akpm/drivers/video/Kconfig     |    8 ++++++++
 25-akpm/drivers/video/tridentfb.c |   13 ++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

drivers/video/Kconfig renamed from drivers/video/Kconfig~tridentfbc-warning-fix +8 -0

diff -puN drivers/video/Kconfig~tridentfbc-warning-fix drivers/video/Kconfig
@@ -787,6 +787,14 @@ config FB_TRIDENT
 	  To compile this driver as a module, choose M here: the
 	  module will be called tridentfb.
 
+config FB_TRIDENT_ACCEL
+	bool "Trident Acceleration functions (EXPERIMENTAL)"
+	depends on FB_TRIDENT && EXPERIMENTAL
+	---help---
+	This will compile the Trident frame buffer device with
+	acceleration functions.
+
+
 config FB_PM3
 	tristate "Permedia3 support"
 	depends on FB && PCI && BROKEN

drivers/video/tridentfb.c renamed from drivers/video/tridentfb.c~tridentfbc-warning-fix +8 -5

diff -puN drivers/video/tridentfb.c~tridentfbc-warning-fix drivers/video/tridentfb.c
@@ -450,7 +450,7 @@ static struct accel_switch accel_image =
 /*
  * Accel functions called by the upper layers
  */
-
+#ifdef CONFIG_FB_TRIDENT_ACCEL
 static void tridentfb_fillrect(struct fb_info * info, const struct fb_fillrect *fr)
 {
 	int bpp = info->var.bits_per_pixel;

@@ -474,6 +474,11 @@ static void tridentfb_copyarea(struct fb
 	acc->copy_rect(ca->sx,ca->sy,ca->dx,ca->dy,ca->width,ca->height);
 	acc->wait_engine();
 }
+#else /* !CONFIG_FB_TRIDENT_ACCEL */
+#define tridentfb_fillrect cfb_fillrect
+#define tridentfb_copyarea cfb_copyarea
+#endif /* CONFIG_FB_TRIDENT_ACCEL */
+
 
 /*
  * Hardware access functions

@@ -1265,10 +1270,8 @@ static struct fb_ops tridentfb_ops = {
 	.fb_blank = tridentfb_blank,
 	.fb_check_var = tridentfb_check_var,
 	.fb_set_par = tridentfb_set_par,
-//	.fb_fillrect = tridentfb_fillrect,
-//	.fb_copyarea= tridentfb_copyarea,
-	.fb_fillrect = cfb_fillrect,
-	.fb_copyarea= cfb_copyarea,
+	.fb_fillrect = tridentfb_fillrect,
+	.fb_copyarea= tridentfb_copyarea,
 	.fb_imageblit = cfb_imageblit,
 	.fb_cursor = soft_cursor,
 };

Andrew Morton

2004-05-01 19:10 UTC #4 lore raw

Quoted 3 lines · Andrew Morton

Does the same argument apply to this patch?

And to this one?

From: "Luiz Fernando N. Capitulino" <[email protected]>

Make HGA acceleration functions selectable in kernel config, fix these warnings:

drivers/video/hgafb.c:452: warning: hgafb_fillrect' defined but not used drivers/video/hgafb.c:472: warning: hgafb_copyarea' defined but not used drivers/video/hgafb.c:502: warning: `hgafb_imageblit' defined but not used

---

 25-akpm/drivers/video/Kconfig |    8 ++++++++
 25-akpm/drivers/video/hgafb.c |   15 ++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

drivers/video/hgafb.c renamed from drivers/video/hgafb.c~hgafbc-warning-fix +12 -3

diff -puN drivers/video/hgafb.c~hgafbc-warning-fix drivers/video/hgafb.c
@@ -448,6 +448,10 @@ static int hgafb_blank(int blank_mode, s
 	return 0;
 }
 
+/*
+ * Accel functions
+ */
+#ifdef CONFIG_FB_HGA_ACCEL
 static void hgafb_fillrect(struct fb_info *info, const struct fb_fillrect *rect)
 {
 	u_int rows, y;

@@ -510,6 +514,11 @@ static void hgafb_imageblit(struct fb_in
 		*dest = d;
 	}
 }
+#else /* !CONFIG_FB_HGA_ACCEL */
+#define hgafb_fillrect cfb_fillrect
+#define hgafb_copyarea cfb_copyarea
+#define hgafb_imageblit cfb_imageblit
+#endif /* CONFIG_FB_HGA_ACCEL */
 
 
 static struct fb_ops hgafb_ops = {

@@ -519,9 +528,9 @@ static struct fb_ops hgafb_ops = {
 	.fb_setcolreg	= hgafb_setcolreg,
 	.fb_pan_display	= hgafb_pan_display,
 	.fb_blank	= hgafb_blank,
-	.fb_fillrect	= cfb_fillrect, //hgafb_fillrect,
-	.fb_copyarea	= cfb_copyarea,	//hgafb_copyarea,
-	.fb_imageblit	= cfb_imageblit,//hgafb_imageblit,
+	.fb_fillrect	= hgafb_fillrect,
+	.fb_copyarea	= hgafb_copyarea,
+	.fb_imageblit	= hgafb_imageblit,
 };
 		
 /* ------------------------------------------------------------------------- *

drivers/video/Kconfig renamed from drivers/video/Kconfig~hgafbc-warning-fix +8 -0

diff -puN drivers/video/Kconfig~hgafbc-warning-fix drivers/video/Kconfig
@@ -308,6 +308,14 @@ config FB_HGA
 	  As this card technology is 15 years old, most people will answer N
 	  here.
 
+config FB_HGA_ACCEL
+	bool "Hercules mono Acceleration functions (EXPERIMENTAL)"
+	depends on FB_HGA && EXPERIMENTAL
+	---help---
+	This will compile the Hercules mono graphics with
+	acceleration functions.
+
+
 config VIDEO_SELECT
 	bool
 	depends on FB && X86

Geert Uytterhoeven

2004-05-01 19:17 UTC #5 lore raw

Quoted 11 lines · Andrew Morton

Quoted 2 lines · Andrew Morton
Does the same argument apply to this patch?
And to this one?
From: "Luiz Fernando N. Capitulino" <[email protected]>
Make HGA acceleration functions selectable in kernel config, fix these warnings:

Yep.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Geert Uytterhoeven

2004-05-01 19:16 UTC #6 lore raw

In reply to Andrew Morton

Quoted 12 lines · Andrew Morton

Quoted 10 lines · Geert Uytterhoeven
Quoted 5 lines
You've taken a bunch of previously-working 3d acceleration functions and made them configurable, dependent upon CONFIG_EXPERIMENTAL.
What on earth for? Will this not gratuitously break people's previously-working 3d setups?
No, tdfxfb_cursor() was not used before, causing a compiler warning. tdfxfb_cursor() may work, but we don't know, so we didn't dare to enable it by default. Now the user (he who has the hardware) can enable it, and tell us whether it works or not.

Quoted 4 lines

OK, thanks. As you can see, it really helps if people explain this sort of thing in their initial patch (sigh).
Does the same argument apply to this patch?

Yes. All these patches should enable the currently unused acceleration routines if the corresponding config option is set, and kill the compiler warnings if its not set.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Luiz Fernando N. Capitulino

author

2004-05-03 16:10 UTC #7 lore raw

Em Sat, May 01, 2004 at 09:16:29PM +0200, Geert Uytterhoeven escreveu:

| On Sat, 1 May 2004, Andrew Morton wrote:
| > Geert Uytterhoeven <[email protected]> wrote:
| > >  > You've taken a bunch of previously-working 3d acceleration functions and
| > >  > made them configurable, dependent upon CONFIG_EXPERIMENTAL.
| > >  >
| > >  > What on earth for?  Will this not gratuitously break people's
| > >  > previously-working 3d setups?
| > >
| > >  No, tdfxfb_cursor() was not used before, causing a compiler warning.
| > >  tdfxfb_cursor() may work, but we don't know, so we didn't dare to enable it by
| > >  default. Now the user (he who has the hardware) can enable it, and tell us
| > >  whether it works or not.
| >
| > OK, thanks.  As you can see, it really helps if people explain this sort of
| > thing in their initial patch (sigh).
| >
| > Does the same argument apply to this patch?
| 
| Yes. All these patches should enable the currently unused acceleration routines
| if the corresponding config option is set, and kill the compiler warnings if
| its not set.

Humm, my fault. I thought my comments was enough, will try to make
better in next time.

PS: Andrew, I always need to think about "be very clear" vs "Andrew's time to read e-mails". It is not a easy task. :-)

Signature · 9 lines

Luiz Fernando N. Capitulino
<http://www.telecentros.sp.gov.br>


-------------------------------------------------------
This SF.Net email is sponsored by: Oracle 10g
Get certified on the hottest thing ever to hit the market... Oracle 10g. 
Take an Oracle 10g class now, and we'll give you the exam FREE. 
http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click