[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Fix agp_backend usage in drm_agp_init (was: 2.6.11-mm3 - DRM/i915
From:       Brice Goglin <Brice.Goglin () ens-lyon ! org>
Date:       2005-03-19 2:45:53
Message-ID: 423B9261.9040108 () ens-lyon ! org
[Download RAW message or body]

Dave Airlie wrote:
>>DRM/i915 does not work on my Dell Dimension 3000 (i865 chipset).
> 
> This is more than likely caused by the multi-bridge AGP stuff in -bk3

Yes, that's it! The bug appeared in -mm when the multi-bridge AGP stuff
was merged (2.6.10-mm3). It is still here in 2.6.12-rc1.

Here's the scenario I think I'm seeing:

agpioc_acquire_wrap is called, it increments the agp_in_use. Then (before
agpioc_release_wrap happens), drm_agp_init is called (I don't know how).
drm_agp_init uses agp_backend_acquire which fails because agp_in_use is
non-null (hold by agpioc_acquire_wrap).

The multi-bridge AGP patch actually changed drm_agp_init by adding
agp_backend_acquire/release around agp_copy_info.
It is why drm_agp_init fails now while it worked before.

I don't think we need to "acquire" it during agp_copy_info.
Why don't we just get a pointer to the bridge instead ?
(is there any chance this bridge gets deleted during drm_agp_init ?)
That's what the attached patch implements on top of 2.6.12-rc1.

I chose to add a new agp_backend_find() function, but we might also
directly call agp_find_bridge() from drm_agp_init(). I don't know what's
the best.

I'm not familiar enough with DRM/AGP code to understand everything here.
I might be missing something...

Regards,
Brice


Signed-off-by: Brice Goglin <Brice.Goglin@ens-lyon.org>


--- linux-rc/include/linux/agp_backend.h.old	2005-03-19 03:26:36.000000000 +0100
+++ linux-rc/include/linux/agp_backend.h	2005-03-19 03:35:43.000000000 +0100
@@ -102,6 +102,7 @@ extern int agp_copy_info(struct agp_brid
  extern int agp_bind_memory(struct agp_memory *, off_t);
  extern int agp_unbind_memory(struct agp_memory *);
  extern void agp_enable(struct agp_bridge_data *, u32);
+extern struct agp_bridge_data *agp_backend_find(struct pci_dev *);
  extern struct agp_bridge_data *agp_backend_acquire(struct pci_dev *);
  extern void agp_backend_release(struct agp_bridge_data *);

--- linux-rc/drivers/char/agp/backend.c.old	2005-03-19 03:30:32.000000000 +0100
+++ linux-rc/drivers/char/agp/backend.c	2005-03-19 03:35:24.000000000 +0100
@@ -58,6 +58,12 @@ LIST_HEAD(agp_bridges);
  EXPORT_SYMBOL(agp_bridge);
  EXPORT_SYMBOL(agp_bridges);

+struct agp_bridge_data *agp_backend_find(struct pci_dev *pdev)
+{
+	return agp_find_bridge(pdev);
+}
+EXPORT_SYMBOL(agp_backend_find);
+
  /**
   *	agp_backend_acquire  -  attempt to acquire an agp backend.
   *
@@ -66,7 +72,7 @@ struct agp_bridge_data *agp_backend_acqu
  {
  	struct agp_bridge_data *bridge;

-	bridge = agp_find_bridge(pdev);
+	bridge = agp_backend_find(pdev);

  	if (!bridge)
  		return NULL;
--- linux-rc/drivers/char/drm/drm_agpsupport.c.old	2005-03-19 03:29:50.000000000 +0100
+++ linux-rc/drivers/char/drm/drm_agpsupport.c	2005-03-19 03:34:28.000000000 +0100
@@ -387,12 +387,11 @@ drm_agp_head_t *drm_agp_init(drm_device_
  	if (!(head = drm_alloc(sizeof(*head), DRM_MEM_AGPLISTS)))
  		return NULL;
  	memset((void *)head, 0, sizeof(*head));
-	if (!(head->bridge = agp_backend_acquire(dev->pdev))) {
+	if (!(head->bridge = agp_backend_find(dev->pdev))) {
  		drm_free(head, sizeof(*head), DRM_MEM_AGPLISTS);
  		return NULL;
  	}
  	agp_copy_info(head->bridge, &head->agp_info);
-	agp_backend_release(head->bridge);
  	if (head->agp_info.chipset == NOT_SUPPORTED) {
  		drm_free(head, sizeof(*head), DRM_MEM_AGPLISTS);
  		return NULL;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic