Skip to contentSkip to conversation

agpgart splitup and cleanup for 2.5.25

PATCH In review Greg KH 7 participants 15 messages Jul 11, 2002 – Jul 28, 2002

Subsystems (2)
Char and misc drivers Agpgart driver
lore
Share & tools
lore

Greg KH

author
#0 lore raw

PATCH 1/1 agpgart splitup and cleanup for 2.5.25

So there I was, trying to remove pci_find_device() and pci_find_class() from the 2.5 kernel (it's an old depreciated api, so don't start complaining...) when I ran across the agpgart code which uses pci_find_class(). I realized I'd have to fix this driver to use the "new" pci api if I had a chance of getting rid of pci_find_class(). Dave Jones had previously split up this driver into some very nice logical parts, and it has been living successfully in his tree for a while now.

So I took that patch, and cleaned it up a bit more (removed some unneeded typedefs, whitespace changes, moved the Config.in and Configure.help stuff to the drivers/char/agp directory) and ported the driver to the pci_register_module() api. It seems to work for me, so I would appreciate it if other people tested it out before I sent it on to Linus for inclusion in the main 2.5 tree.

It can be found at:

	http://www.kernel.org/pub/linux/kernel/people/gregkh/misc/agpgart-2.5.25.patch.bz2

Here's the diffstat output of it:

 drivers/char/Config.help             |   87 
 drivers/char/Config.in               |   15 
 drivers/char/agp/Config.help         |   88 
 drivers/char/agp/Config.in           |   14 
 drivers/char/agp/Makefile            |   13 
 drivers/char/agp/agp.h               |  348 +-
 drivers/char/agp/agpgart_be-ali.c    |  265 ++
 drivers/char/agp/agpgart_be-amd.c    |  408 +++
 drivers/char/agp/agpgart_be-hp.c     |  394 +++
 drivers/char/agp/agpgart_be-i460.c   |  595 ++++
 drivers/char/agp/agpgart_be-i810.c   |  594 ++++
 drivers/char/agp/agpgart_be-i8x0.c   |  720 +++++
 drivers/char/agp/agpgart_be-sis.c    |  142 +
 drivers/char/agp/agpgart_be-sworks.c |  626 +++++
 drivers/char/agp/agpgart_be-via.c    |  151 +
 drivers/char/agp/agpgart_be.c        | 4368 +++--------------------------------
 drivers/char/agp/agpgart_fe.c        |   15 
 include/linux/agp_backend.h          |    6 
 include/linux/agpgart.h              |   10 
 19 files changed, 4682 insertions(+), 4177 deletions(-)

thanks,

greg k-h

author
#1 lore raw

And here's the version against the 2.5.25-dj1 tree. Dave, will you please add it to your tree?

thanks,

greg k-h

drivers/char/Config.help +0 -87
diff -Nru a/drivers/char/Config.help b/drivers/char/Config.help
@@ -91,93 +91,6 @@
   The module will be called mwave.o. If you want to compile it as
   a module, say M here and read Documentation/modules.txt.
 
-CONFIG_AGP
-  AGP (Accelerated Graphics Port) is a bus system mainly used to
-  connect graphics cards to the rest of the system.
-
-  If you have an AGP system and you say Y here, it will be possible to
-  use the AGP features of your 3D rendering video card. This code acts
-  as a sort of "AGP driver" for the motherboard's chipset.
-
-  If you need more texture memory than you can get with the AGP GART
-  (theoretically up to 256 MB, but in practice usually 64 or 128 MB
-  due to kernel allocation issues), you could use PCI accesses
-  and have up to a couple gigs of texture space.
-
-  Note that this is the only means to have XFree4/GLX use
-  write-combining with MTRR support on the AGP bus. Without it, OpenGL
-  direct rendering will be a lot slower but still faster than PIO.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-  This driver is available as a module.  If you want to compile it as
-  a module, say M here and read <file:Documentation/modules.txt>.  The
-  module will be called agpgart.o.
-
-CONFIG_AGP_INTEL
-  This option gives you AGP support for the GLX component of the
-  XFree86 4.x on Intel 440LX/BX/GX, 815, 820, 830, 840, 845, 850 and 860 chipsets.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-CONFIG_AGP_I810
-  This option gives you AGP support for the Xserver on the Intel 810
-  815 and 830m chipset boards for their on-board integrated graphics. This
-  is required to do any useful video modes with these boards.
-
-CONFIG_AGP_I460
-  This option gives you AGP GART support for the Intel 460GX chipset
-  for IA64 processors.
-
-CONFIG_AGP_VIA
-  This option gives you AGP support for the GLX component of the
-  XFree86 4.x on VIA MPV3/Apollo Pro chipsets.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-CONFIG_AGP_AMD
-  This option gives you AGP support for the GLX component of the
-  XFree86 4.x on AMD Irongate, 761, and 762 chipsets.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-CONFIG_AGP_SIS
-  This option gives you AGP support for the GLX component of the "soon
-  to be released" XFree86 4.x on Silicon Integrated Systems [SiS]
-  chipsets.
-
-  Note that 5591/5592 AGP chipsets are NOT supported.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-CONFIG_AGP_SWORKS
-  Say Y here to support the Serverworks AGP card.  See 
-  <http://www.serverworks.com/> for product descriptions and images.
-
-CONFIG_AGP_ALI
-  This option gives you AGP support for the GLX component of the
-  XFree86 4.x on the following ALi chipsets.  The supported chipsets
-  include M1541, M1621, M1631, M1632, M1641,M1647,and M1651.
-  For the ALi-chipset question, ALi suggests you refer to
-  <http://www.ali.com.tw/eng/support/index.shtml>.
-
-  The M1541 chipset can do AGP 1x and 2x, but note that there is an
-  acknowledged incompatibility with Matrox G200 cards. Due to
-  timing issues, this chipset cannot do AGP 2x with the G200.
-  This is a hardware limitation. AGP 1x seems to be fine, though.
-
-  You should say Y here if you use XFree86 3.3.6 or 4.x and want to
-  use GLX or DRI.  If unsure, say N.
-
-CONFIG_AGP_HP_ZX1
-  This option gives you AGP GART support for the HP ZX1 chipset
-  for IA64 processors.
-
 CONFIG_I810_TCO
   Hardware driver for the TCO timer built into the Intel i810 and i815
   chipset family.  The TCO (Total Cost of Ownership) timer is a
drivers/char/Config.in +1 -14
diff -Nru a/drivers/char/Config.in b/drivers/char/Config.in
@@ -210,20 +210,7 @@
 fi
 endmenu
 
-dep_tristate '/dev/agpgart (AGP Support)' CONFIG_AGP $CONFIG_DRM_AGP
-if [ "$CONFIG_AGP" != "n" ]; then
-   bool '  Intel 440LX/BX/GX and I815/I820/I830M/I830MP/I840/I845/I850/I860 support' CONFIG_AGP_INTEL
-   bool '  Intel I810/I815/I830M (on-board) support' CONFIG_AGP_I810
-   bool '  VIA chipset support' CONFIG_AGP_VIA
-   bool '  AMD Irongate, 761, and 762 support' CONFIG_AGP_AMD
-   bool '  Generic SiS support' CONFIG_AGP_SIS
-   bool '  ALI chipset support' CONFIG_AGP_ALI
-   bool '  Serverworks LE/HE support' CONFIG_AGP_SWORKS
-   if [ "$CONFIG_IA64" = "y" ]; then
-     bool '  Intel 460GX support' CONFIG_AGP_I460
-     bool '  HP ZX1 AGP support' CONFIG_AGP_HP_ZX1
-   fi
-fi
+source drivers/char/agp/Config.in
 
 source drivers/char/drm/Config.in
drivers/char/agp/Config.help new file +88 -0

88 lines not on this page · raw message

drivers/char/agp/Config.in new file +14 -0
diff -Nru a/drivers/char/agp/Config.in b/drivers/char/agp/Config.in
@@ -0,0 +1,14 @@
+dep_tristate '/dev/agpgart (AGP Support)' CONFIG_AGP $CONFIG_DRM_AGP
+if [ "$CONFIG_AGP" != "n" ]; then
+   bool '  Intel 440LX/BX/GX and I815/I820/I830M/I830MP/I840/I845/I850/I860 support' CONFIG_AGP_INTEL
+   bool '  Intel I810/I815/I830M (on-board) support' CONFIG_AGP_I810
+   bool '  VIA chipset support' CONFIG_AGP_VIA
+   bool '  AMD Irongate, 761, and 762 support' CONFIG_AGP_AMD
+   bool '  Generic SiS support' CONFIG_AGP_SIS
+   bool '  ALI chipset support' CONFIG_AGP_ALI
+   bool '  Serverworks LE/HE support' CONFIG_AGP_SWORKS
+   if [ "$CONFIG_IA64" = "y" ]; then
+     bool '  Intel 460GX support' CONFIG_AGP_I460
+     bool '  HP ZX1 AGP support' CONFIG_AGP_HP_ZX1
+   fi
+fi
drivers/char/agp/Makefile +7 -7
diff -Nru a/drivers/char/agp/Makefile b/drivers/char/agp/Makefile
@@ -7,14 +7,14 @@
 
 agpgart-y := agpgart_fe.o agpgart_be.o
 
-agpgart-$(CONFIG_AGP_INTEL)		+=	agpgart_be-i8x0.o
-agpgart-$(CONFIG_AGP_I810)		+=	agpgart_be-i810.o
-agpgart-$(CONFIG_AGP_VIA)		+=	agpgart_be-via.o
-agpgart-$(CONFIG_AGP_AMD)		+=	agpgart_be-amd.o
-agpgart-$(CONFIG_AGP_SIS)		+=	agpgart_be-sis.o
-agpgart-$(CONFIG_AGP_ALI)		+=	agpgart_be-ali.o
+agpgart-$(CONFIG_AGP_INTEL)	+=	agpgart_be-i8x0.o
+agpgart-$(CONFIG_AGP_I810)	+=	agpgart_be-i810.o
+agpgart-$(CONFIG_AGP_VIA)	+=	agpgart_be-via.o
+agpgart-$(CONFIG_AGP_AMD)	+=	agpgart_be-amd.o
+agpgart-$(CONFIG_AGP_SIS)	+=	agpgart_be-sis.o
+agpgart-$(CONFIG_AGP_ALI)	+=	agpgart_be-ali.o
 agpgart-$(CONFIG_AGP_SWORKS)	+=	agpgart_be-sworks.o
-agpgart-$(CONFIG_AGP_I460)		+=	agpgart_be-i460.o
+agpgart-$(CONFIG_AGP_I460)	+=	agpgart_be-i460.o
 agpgart-$(CONFIG_AGP_HP_ZX1)	+=	agpgart_be-hp.o
 agpgart-objs := $(agpgart-y)
drivers/char/agp/agp.h +146 -146

466 lines not on this page · raw message

drivers/char/agp/agpgart_be-ali.c +6 -23
diff -Nru a/drivers/char/agp/agpgart_be-ali.c b/drivers/char/agp/agpgart_be-ali.c
@@ -26,27 +26,10 @@
  * - Allocate more than order 0 pages to avoid too much linear map splitting.
  */
 #include <linux/config.h>
-#include <linux/version.h>
-#include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
-#include <linux/jiffies.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/pagemap.h>
-#include <linux/miscdevice.h>
-#include <linux/pm.h>
-#include <asm/system.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
-#include <asm/page.h>
-#include <asm/agp.h>
-
 #include <linux/agp_backend.h>
 #include "agp.h"
 

@@ -54,7 +37,7 @@
 {
 	int i;
 	u32 temp;
-	aper_size_info_32 *values;
+	struct aper_size_info_32 *values;
 
 	pci_read_config_dword(agp_bridge.dev, ALI_ATTBASE, &temp);
 	temp &= ~(0xfffffff0);

@@ -84,7 +67,7 @@
 
 static void ali_cleanup(void)
 {
-	aper_size_info_32 *previous_size;
+	struct aper_size_info_32 *previous_size;
 	u32 temp;
 
 	previous_size = A_SIZE_32(agp_bridge.previous_size);

@@ -102,7 +85,7 @@
 static int ali_configure(void)
 {
 	u32 temp;
-	aper_size_info_32 *current_size;
+	struct aper_size_info_32 *current_size;
 
 	current_size = A_SIZE_32(agp_bridge.current_size);
 

@@ -231,12 +214,12 @@
 }
 
 /* Setup function */
-static gatt_mask ali_generic_masks[] =
+static struct gatt_mask ali_generic_masks[] =
 {
-	{0x00000000, 0}
+	{mask: 0x00000000, type: 0}
 };
 
-static aper_size_info_32 ali_generic_sizes[7] =
+static struct aper_size_info_32 ali_generic_sizes[7] =
 {
 	{256, 65536, 6, 10},
 	{128, 32768, 5, 9},
drivers/char/agp/agpgart_be-amd.c +23 -41

157 lines not on this page · raw message

drivers/char/agp/agpgart_be-hp.c +4 -22
diff -Nru a/drivers/char/agp/agpgart_be-hp.c b/drivers/char/agp/agpgart_be-hp.c
@@ -25,28 +25,10 @@
  * TODO: 
  * - Allocate more than order 0 pages to avoid too much linear map splitting.
  */
-#include <linux/config.h>
-#include <linux/version.h>
+
 #include <linux/module.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/jiffies.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/pagemap.h>
-#include <linux/miscdevice.h>
-#include <linux/pm.h>
-#include <asm/system.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
-#include <asm/page.h>
-#include <asm/agp.h>
-
 #include <linux/agp_backend.h>
 #include "agp.h"
 

@@ -64,14 +46,14 @@
 #define HP_ZX1_IOVA_TO_PDIR(va)	((va - hp_private.iova_base) >> \
 					hp_private.io_tlb_shift)
 
-static aper_size_info_fixed hp_zx1_sizes[] =
+static struct aper_size_info_fixed hp_zx1_sizes[] =
 {
 	{0, 0, 0},		/* filled in by hp_zx1_fetch_size() */
 };
 
-static gatt_mask hp_zx1_masks[] =
+static struct gatt_mask hp_zx1_masks[] =
 {
-	{HP_ZX1_PDIR_VALID_BIT, 0}
+	{mask: HP_ZX1_PDIR_VALID_BIT, type: 0}
 };
 
 static struct _hp_private {
drivers/char/agp/agpgart_be-i460.c +8 -26
diff -Nru a/drivers/char/agp/agpgart_be-i460.c b/drivers/char/agp/agpgart_be-i460.c
@@ -25,28 +25,10 @@
  * TODO: 
  * - Allocate more than order 0 pages to avoid too much linear map splitting.
  */
-#include <linux/config.h>
-#include <linux/version.h>
+
 #include <linux/module.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/jiffies.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/pagemap.h>
-#include <linux/miscdevice.h>
-#include <linux/pm.h>
-#include <asm/system.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
-#include <asm/page.h>
-#include <asm/agp.h>
-
 #include <linux/agp_backend.h>
 #include "agp.h"
 

@@ -87,7 +69,7 @@
 {
 	int i;
 	u8 temp;
-	aper_size_info_8 *values;
+	struct aper_size_info_8 *values;
 
 	/* Determine the GART page size */
 	pci_read_config_byte(agp_bridge.dev, INTEL_I460_GXBCTL, &temp);

@@ -160,7 +142,7 @@
 
 static void intel_i460_cleanup(void)
 {
-	aper_size_info_8 *previous_size;
+	struct aper_size_info_8 *previous_size;
 
 	previous_size = A_SIZE_8(agp_bridge.previous_size);
 	intel_i460_write_agpsiz(previous_size->size_value);

@@ -185,7 +167,7 @@
 	u8 scratch;
 	int i;
 
-	aper_size_info_8 *current_size;
+	struct aper_size_info_8 *current_size;
 
 	temp.large = 0;
 

@@ -548,11 +530,11 @@
 		agp_generic_destroy_page(page);
 }
 
-static gatt_mask intel_i460_masks[] =
+static struct gatt_mask intel_i460_masks[] =
 {
 	{
-	  INTEL_I460_GATT_VALID | INTEL_I460_GATT_COHERENT,
-	  0
+	  mask: INTEL_I460_GATT_VALID | INTEL_I460_GATT_COHERENT,
+	  type: 0
 	}
 };
 

@@ -569,7 +551,7 @@
 	return ((addr & 0xffffff) << 12);
 }
 
-static aper_size_info_8 intel_i460_sizes[3] =
+static struct aper_size_info_8 intel_i460_sizes[3] =
 {
 	/*
 	 * The 32GB aperture is only available with a 4M GART page size.
drivers/char/agp/agpgart_be-i810.c +12 -30

97 lines not on this page · raw message

drivers/char/agp/agpgart_be-i8x0.c +20 -39

174 lines not on this page · raw message

drivers/char/agp/agpgart_be-serverworks.c deleted +0 -644

644 lines not on this page · raw message

drivers/char/agp/agpgart_be-sis.c +7 -25
diff -Nru a/drivers/char/agp/agpgart_be-sis.c b/drivers/char/agp/agpgart_be-sis.c
@@ -25,28 +25,10 @@
  * TODO: 
  * - Allocate more than order 0 pages to avoid too much linear map splitting.
  */
-#include <linux/config.h>
-#include <linux/version.h>
+
 #include <linux/module.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/jiffies.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/pagemap.h>
-#include <linux/miscdevice.h>
-#include <linux/pm.h>
-#include <asm/system.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
-#include <asm/page.h>
-#include <asm/agp.h>
-
 #include <linux/agp_backend.h>
 #include "agp.h"
 

@@ -54,7 +36,7 @@
 {
 	u8 temp_size;
 	int i;
-	aper_size_info_8 *values;
+	struct aper_size_info_8 *values;
 
 	pci_read_config_byte(agp_bridge.dev, SIS_APSIZE, &temp_size);
 	values = A_SIZE_8(agp_bridge.aperture_sizes);

@@ -82,7 +64,7 @@
 static int sis_configure(void)
 {
 	u32 temp;
-	aper_size_info_8 *current_size;
+	struct aper_size_info_8 *current_size;
 
 	current_size = A_SIZE_8(agp_bridge.current_size);
 	pci_write_config_byte(agp_bridge.dev, SIS_TLBCNTRL, 0x05);

@@ -97,7 +79,7 @@
 
 static void sis_cleanup(void)
 {
-	aper_size_info_8 *previous_size;
+	struct aper_size_info_8 *previous_size;
 
 	previous_size = A_SIZE_8(agp_bridge.previous_size);
 	pci_write_config_byte(agp_bridge.dev, SIS_APSIZE,

@@ -111,7 +93,7 @@
 	return addr | agp_bridge.masks[0].mask;
 }
 
-static aper_size_info_8 sis_generic_sizes[7] =
+static struct aper_size_info_8 sis_generic_sizes[7] =
 {
 	{256, 65536, 6, 99},
 	{128, 32768, 5, 83},

@@ -122,9 +104,9 @@
 	{4, 1024, 0, 3}
 };
 
-static gatt_mask sis_generic_masks[] =
+static struct gatt_mask sis_generic_masks[] =
 {
-	{0x00000000, 0}
+	{mask: 0x00000000, type: 0}
 };
 
 int __init sis_generic_setup (struct pci_dev *pdev)
drivers/char/agp/agpgart_be-sworks.c +23 -41

151 lines not on this page · raw message

drivers/char/agp/agpgart_be-via.c +6 -23
diff -Nru a/drivers/char/agp/agpgart_be-via.c b/drivers/char/agp/agpgart_be-via.c
@@ -26,27 +26,10 @@
  * - Allocate more than order 0 pages to avoid too much linear map splitting.
  */
 #include <linux/config.h>
-#include <linux/version.h>
-#include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
-#include <linux/jiffies.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/slab.h>
-#include <linux/vmalloc.h>
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/pagemap.h>
-#include <linux/miscdevice.h>
-#include <linux/pm.h>
-#include <asm/system.h>
-#include <asm/uaccess.h>
-#include <asm/io.h>
-#include <asm/page.h>
-#include <asm/agp.h>
-
 #include <linux/agp_backend.h>
 #include "agp.h"
 

@@ -55,7 +38,7 @@
 {
 	int i;
 	u8 temp;
-	aper_size_info_8 *values;
+	struct aper_size_info_8 *values;
 
 	values = A_SIZE_8(agp_bridge.aperture_sizes);
 	pci_read_config_byte(agp_bridge.dev, VIA_APSIZE, &temp);

@@ -74,7 +57,7 @@
 static int via_configure(void)
 {
 	u32 temp;
-	aper_size_info_8 *current_size;
+	struct aper_size_info_8 *current_size;
 
 	current_size = A_SIZE_8(agp_bridge.current_size);
 	/* aperture size */

@@ -95,7 +78,7 @@
 
 static void via_cleanup(void)
 {
-	aper_size_info_8 *previous_size;
+	struct aper_size_info_8 *previous_size;
 
 	previous_size = A_SIZE_8(agp_bridge.previous_size);
 	pci_write_config_byte(agp_bridge.dev, VIA_APSIZE,

@@ -118,7 +101,7 @@
 	return addr | agp_bridge.masks[0].mask;
 }
 
-static aper_size_info_8 via_generic_sizes[7] =
+static struct aper_size_info_8 via_generic_sizes[7] =
 {
 	{256, 65536, 6, 0},
 	{128, 32768, 5, 128},

@@ -129,9 +112,9 @@
 	{4, 1024, 0, 252}
 };
 
-static gatt_mask via_generic_masks[] =
+static struct gatt_mask via_generic_masks[] =
 {
-	{0x00000000, 0}
+	{mask: 0x00000000, type: 0}
 };
 
 int __init via_generic_setup (struct pci_dev *pdev)
drivers/char/agp/agpgart_be.c +444 -337

745 lines not on this page · raw message

drivers/char/agp/agpgart_fe.c +1 -14
diff -Nru a/drivers/char/agp/agpgart_fe.c b/drivers/char/agp/agpgart_fe.c

Lines of this file are not shown. View the raw message.

include/linux/agp_backend.h +3 -3
diff -Nru a/include/linux/agp_backend.h b/include/linux/agp_backend.h

Lines of this file are not shown. View the raw message.

include/linux/agpgart.h +5 -5
diff -Nru a/include/linux/agpgart.h b/include/linux/agpgart.h

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Part of the diff is not shown here. View the raw message.

#2 lore raw
Quoted 2 lines · Greg KH

And here's the version against the 2.5.25-dj1 tree. Dave, will you please add it to your tree?

Done. Along with some small CodingStyle changes (to existing code, not anything you added).

Thanks for doing this, it's a big improvement.

Dave

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs
author
#3 lore raw
In reply to Greg KH

Dave Jones found a bug in this patch that causes the driver to try to bind to multiple busses :(

This patch seems to fix this problem.

thanks,

greg k-h

drivers/char/agp/agpgart_be.c +6 -1
diff -Nru a/drivers/char/agp/agpgart_be.c b/drivers/char/agp/agpgart_be.c
@@ -47,7 +47,7 @@
 EXPORT_SYMBOL(agp_backend_acquire);
 EXPORT_SYMBOL(agp_backend_release);
 
-struct agp_bridge_data agp_bridge;
+struct agp_bridge_data agp_bridge = { type: NOT_SUPPORTED };
 static int agp_try_unsupported __initdata = 0;
 
 int agp_backend_acquire(void)

@@ -1593,6 +1593,11 @@
 static int agp_probe (struct pci_dev *dev, const struct pci_device_id *ent)
 {
 	int ret_val;
+
+	if (agp_bridge.type != NOT_SUPPORTED) {
+		printk (KERN_DEBUG "Oops, don't init a 2nd agpgard device.\n");
+		return -ENODEV;
+	}
 
 	ret_val = agp_backend_initialize(dev);
 	if (ret_val) {
#4 lore raw
Quoted 23 lines · Greg KH

Dave Jones found a bug in this patch that causes the driver to try to bind to multiple busses :(

This patch seems to fix this problem.

thanks,

greg k-h

diff -Nru a/drivers/char/agp/agpgart_be.c b/drivers/char/agp/agpgart_be.c

drivers/char/agp/agpgart_be.c +1 -1
@@ -47,7 +47,7 @@
 EXPORT_SYMBOL(agp_backend_acquire);
 EXPORT_SYMBOL(agp_backend_release);
 
-struct agp_bridge_data agp_bridge;
+struct agp_bridge_data agp_bridge = { type: NOT_SUPPORTED };
 static int agp_try_unsupported __initdata = 0;
 
 int agp_backend_acquire(void)

drivers/char/agp/agpgart_be.c line 1598

Quoted 6 lines
drivers/char/agp/agpgart_be.c +3 -0
@@ -1593,6 +1593,11 @@
 static int agp_probe (struct pci_dev *dev, const struct pci_device_id *ent)
 {
 	int ret_val;
+
+	if (agp_bridge.type != NOT_SUPPORTED) {
+		printk (KERN_DEBUG "Oops, don't init a 2nd agpgard device.\n");

                                                           ^^^^^^^
							   agpgart?

drivers/char/agp/agpgart_be.c line 1603

Quoted 5 lines
drivers/char/agp/agpgart_be.c +2 -0
+		return -ENODEV;
+	}
 
 	ret_val = agp_backend_initialize(dev);
 	if (ret_val) {

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

author
#5 lore raw
Quoted 3 lines · Geert Uytterhoeven

drivers/char/agp/agpgart_be.c line 1598

Quoted 1 line
drivers/char/agp/agpgart_be.c +1 -0
+		printk (KERN_DEBUG "Oops, don't init a 2nd agpgard device.\n");

                                                           ^^^^^^^
							   agpgart?

Fixed in the latest version, thanks.

greg k-h

#6 lore raw
In reply to Greg KH
Quoted 17 lines · Greg KH

Here's the diffstat output of it:

drivers/char/Config.help             |   87 
drivers/char/Config.in               |   15 
drivers/char/agp/Config.help         |   88 
drivers/char/agp/Config.in           |   14 
drivers/char/agp/Makefile            |   13 
drivers/char/agp/agp.h               |  348 +-
drivers/char/agp/agpgart_be-ali.c    |  265 ++
drivers/char/agp/agpgart_be-amd.c    |  408 +++
drivers/char/agp/agpgart_be-hp.c     |  394 +++
drivers/char/agp/agpgart_be-i460.c   |  595 ++++
drivers/char/agp/agpgart_be-i810.c   |  594 ++++
drivers/char/agp/agpgart_be-i8x0.c   |  720 +++++
drivers/char/agp/agpgart_be-sis.c    |  142 +
drivers/char/agp/agpgart_be-sworks.c |  626 +++++
drivers/char/agp/agpgart_be-via.c    |  151 +

Ok, so is there any real _reason_ to have filenames quite this ugly?

I'd love to have agp split up sanely, but "sanely" does imply that a via driver is called "via.c", since the "be" part ("backend") is completely silly (of _course_ a chipset driver is a "backend" driver, it sure as hell aint frontend), and that "agpgart_" prefix is kind of silly, since the whole subdirectory is clearly about AGP.

Having filenames that repeat is redundant, excessive, exorbitant, and superfluous.

If you want the redundancy, duplication and profusion, please keep it shorter. And put it at the end, so that at least filename completion works well: "via-agp.c".

Ok?

Linus

#7 lore raw
Quoted 2 lines · Linus Torvalds
Quoted 1 line

drivers/char/agp/agpgart_be-via.c | 151 +

Ok, so is there any real _reason_ to have filenames quite this ugly?

of course not. I'll fix it up later, and send an updated patch with 1-2 other small changes that happened since Greg's work (one of the Intel backends had a subtle thinko brought forward
from 2.4.19rc1)

Quoted 5 lines

If you want the redundancy, duplication and profusion, please keep it shorter. And put it at the end, so that at least filename completion works well: "via-agp.c".

Ok?

Ok, I think we can even do away with the -agp, as we're in the agp/ dircetory, which again seems to be pointless duplication.

Dave

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs
#8 lore raw
Quoted 9 lines · Dave Jones
Quoted 2 lines · Linus Torvalds
Quoted 1 line

drivers/char/agp/agpgart_be-via.c | 151 +

Ok, so is there any real _reason_ to have filenames quite this ugly?

of course not. I'll fix it up later, and send an updated patch with 1-2 other small changes that happened since Greg's work (one of the Intel backends had a subtle thinko brought forward
from 2.4.19rc1)

Quoted 8 lines
Quoted 5 lines

If you want the redundancy, duplication and profusion, please keep it shorter. And put it at the end, so that at least filename completion works well: "via-agp.c".

Ok?

Ok, I think we can even do away with the -agp, as we're in the agp/ dircetory, which again seems to be pointless duplication.

modprobe is still a flat namespace :/ So it's not necessarily pointless duplication. That's why I named the via82cxxx audio driver with the _audio suffix, to eliminate conflicts with drivers/ide/via82cxxx.c when/if it becomes directly modprobe-able.

Jeff

#9 lore raw
Quoted 7 lines · Jeff Garzik
Quoted 2 lines

Ok, I think we can even do away with the -agp, as we're in the agp/ dircetory, which again seems to be pointless duplication.

modprobe is still a flat namespace :/ So it's not necessarily pointless duplication. That's why I named the via82cxxx audio driver with the _audio suffix, to eliminate conflicts with drivers/ide/via82cxxx.c when/if it becomes directly modprobe-able.

the .o files get linked into agpgart.o
Still just one module. Christoph Hellwig proposed making each back end a module too, which is dependant upon agpgart.o, but that's more pain than I feel like enduring right now.. Maybe later.

Linus ?

Dave

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs
#10 lore raw
Quoted 7 lines · Dave Jones

the .o files get linked into agpgart.o
Still just one module. Christoph Hellwig proposed making each back end a module too, which is dependant upon agpgart.o, but that's more pain than I feel like enduring right now.. Maybe later.

Linus ?

I'm perfectly happy with something like "via-agp.c", if it heads off potential future trouble. I just don't like the "agpgart_be" thing, it's unreadable in the first place, too long in the second, and having a prefix (as opposed to a postfix) makes filename completion suck in the third place.

Something like "via-agp.c" doesn't have any of those problems, and while the "agp" is slightly redundant in the directory structure, it's at least not ugly.

Linus

#11 lore raw
In reply to Greg KH

Hi,

I just tried to compile 2.5.29, haven't tried dev kernels since 2.5.24 and it seems that although the nvidia kernel module builds ok when I try to start X I get a freeze or a reboot. Any chance it has something to do with this?

Quoted 8 lines

So there I was, trying to remove pci_find_device() and pci_find_class() from the 2.5 kernel (it's an old depreciated api, so don't start complaining...) when I ran across the agpgart code which uses pci_find_class(). I realized I'd have to fix this driver to use the "new" pci api if I had a chance of getting rid of pci_find_class(). Dave Jones had previously split up this driver into some very nice logical parts, and it has been living successfully in his tree for a while now.

author
#12 lore raw
Quoted 6 lines · Tommy Faasen

Hi,

I just tried to compile 2.5.29, haven't tried dev kernels since 2.5.24 and it seems that although the nvidia kernel module builds ok when I try to start X I get a freeze or a reboot. Any chance it has something to do with this?

No, I do not. Without the nvidia kernel module, does everything work just fine?

thanks,

greg k-h

#13 lore raw
Quoted 7 lines · Greg KH
Quoted 4 lines

I just tried to compile 2.5.29, haven't tried dev kernels since 2.5.24 and it seems that although the nvidia kernel module builds ok when I try to start X I get a freeze or a reboot. Any chance it has something to do with this?

No, I do not. Without the nvidia kernel module, does everything work just fine?

Given the amount of internal changes since .24 -> .29, I'm not in the least surprised that some binary only junk has stopped working. As usual, the reply AFAICS is "bug nvidia".

Dave

Signature · 2 lines
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs
#14 lore raw
In reply to Greg KH
Quoted 11 lines · Greg KH
Quoted 6 lines · Tommy Faasen

Hi,

I just tried to compile 2.5.29, haven't tried dev kernels since 2.5.24 and it seems that although the nvidia kernel module builds ok when I try to start X I get a freeze or a reboot. Any chance it has something to do with this?

No, I do not. Without the nvidia kernel module, does everything work just fine?

Console wise yes, I'll give the xfree86 module a go...

i'll let you know.

Tommy

Quoted 9 lines

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/