Skip to contentSkip to conversation

Export a func to find the corresponding PCI bus:seg.func of an ACPI device

PATCH In review Thomas Renninger 4 participants 7 messages Oct 15, 2007 – Oct 16, 2007 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 Export a func to find the corresponding PCI bus:seg.func of an ACPI device

Hi,

Li: Could you double check and comment on that one please. It works fine here on a Lenovo ThinkPad...

These two should avoid the registration of a dummy ACPI device for graphics card on recent Lenovo ThinkPads.

Christian: This is why you saw two devices in /proc/acpi/video It would be great if you could check if it's working for you.

Thomas

------------------

Export a func to find the corresponding PCI bus:seg.func of an ACPI device

---
 drivers/acpi/pci_root.c |  155 ++++++++++++++++++++++++++++++++++++++----------
 include/linux/acpi.h    |    2 
 2 files changed, 127 insertions(+), 30 deletions(-)
drivers/acpi/pci_root.c +125 -30
Index: lenb/drivers/acpi/pci_root.c
===================================================================
@@ -32,6 +32,7 @@
 #include <linux/pm.h>
 #include <linux/pci.h>
 #include <linux/acpi.h>
+#include <linux/mod_devicetable.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
@@ -184,31 +185,11 @@ static void acpi_pci_bridge_scan(struct 
 		}
 }
 
-static int acpi_pci_root_add(struct acpi_device *device)
+static int acpi_pci_root_get_seg_bus(struct acpi_device *device, u16 *seg, u16 *bus)
 {
-	int result = 0;
-	struct acpi_pci_root *root = NULL;
-	struct acpi_pci_root *tmp;
+
 	acpi_status status = AE_OK;
 	unsigned long value = 0;
-	acpi_handle handle = NULL;
-	struct acpi_device *child;
-
-
-	if (!device)
-		return -EINVAL;
-
-	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
-	if (!root)
-		return -ENOMEM;
-	INIT_LIST_HEAD(&root->node);
-
-	root->device = device;
-	strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME);
-	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
-	acpi_driver_data(device) = root;
-
-	device->ops.bind = acpi_pci_bind;
 
 	/* 
 	 * Segment
@@ -219,17 +200,16 @@ static int acpi_pci_root_add(struct acpi
 				       &value);
 	switch (status) {
 	case AE_OK:
-		root->id.segment = (u16) value;
+		*seg = (u16) value;
 		break;
 	case AE_NOT_FOUND:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Assuming segment 0 (no _SEG)\n"));
-		root->id.segment = 0;
+		*seg = 0;
 		break;
 	default:
 		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _SEG"));
-		result = -ENODEV;
-		goto end;
+		return -ENODEV;
 	}
 
 	/* 
@@ -241,17 +221,48 @@ static int acpi_pci_root_add(struct acpi
 				       &value);
 	switch (status) {
 	case AE_OK:
-		root->id.bus = (u16) value;
+		*bus = (u16) value;
 		break;
 	case AE_NOT_FOUND:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Assuming bus 0 (no _BBN)\n"));
-		root->id.bus = 0;
+		*bus = 0;
 		break;
 	default:
 		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BBN"));
-		result = -ENODEV;
-		goto end;
+		return -ENODEV;
 	}
+	return 0;
+}
+
+static int acpi_pci_root_add(struct acpi_device *device)
+{
+	int result = 0;
+	struct acpi_pci_root *root = NULL;
+	struct acpi_pci_root *tmp;
+	acpi_status status = AE_OK;
+	acpi_handle handle = NULL;
+	struct acpi_device *child;
+
+
+	if (!device)
+		return -EINVAL;
+
+	root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL);
+	if (!root)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&root->node);
+
+	root->device = device;
+	strcpy(acpi_device_name(device), ACPI_PCI_ROOT_DEVICE_NAME);
+	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
+	acpi_driver_data(device) = root;
+
+	device->ops.bind = acpi_pci_bind;
+
+	result = acpi_pci_root_get_seg_bus(device, &root->id.bus,
+					   &root->id.segment);
+	if (result < 0)
+		goto end;
 
 	/* Some systems have wrong _BBN */
 	list_for_each_entry(tmp, &acpi_pci_roots, node) {
@@ -392,3 +403,87 @@ static int __init acpi_pci_root_init(voi
 }
 
 subsys_initcall(acpi_pci_root_init);
+
+/*
+ * Find the struct pci_dev* structure associated to the given ACPI device
+ *
+ * The device might be deeper in the device tree, the function goes until the
+ * PCI root bridge and returns the registered PCI device in the PCI sublayer,
+ * e.g.:
+ *
+ * \_SB
+ *     PCI0
+ *         SATA          S-ATA controller
+ *             _ADR      PCI Address of the SATA controller
+ *             PRT0      Port 0 device
+ *                 _ADR  Physical port and multiplier topology
+ *             PRTn      Port n device
+ *
+ * Whether PRT0 or SATA is passed it will always return the pci_dev
+ * structure of the SATA device.
+ * Be aware that the PCI device could be a PCI root bridge (e.g. in AGP
+ * case). If you want to have the real PCI struct of it, you need PCI sublayer
+ * functions to map the PCI root bridge device to the PCI bus the device has
+ * been set up to.
+ *
+ * Returns:
+ *   - zero on success
+ *   - -EINVAL on a real error -> buggy BIOS...
+ *   - -ENODEV is there is no physicaly PCI device associated/registered
+ */
+
+int acpi_find_pci_device(struct acpi_device *device, struct pci_dev *pci)
+{
+	struct acpi_device *parent, *child;
+	unsigned int devfn;
+	u16 bus, seg;
+	int result;
+	const struct acpi_device_id root_bridge_ids[] = {
+		{PCI_ROOT_HID_STRING, 0},
+		{PCI_EXPRESS_ROOT_HID_STRING, 0},
+		{"", 0},
+	};
+	pci = NULL;
+
+	if (!device || !device->parent) {
+		return -EINVAL;
+	}
+	child = device;
+	parent = device->parent;
+
+	while (acpi_match_device_ids(parent, root_bridge_ids)) {
+		/* Passed device was not under a PCI root bus */
+		if (!parent)
+			return -EINVAL;
+
+		child = parent;
+		parent = parent->parent;
+
+	}
+
+	if (!child->flags.bus_address) {
+		printk (KERN_DEBUG PREFIX "Child device of PCI root bus has no"
+			" _ADR func\n");
+		return -EINVAL;
+	}
+
+	/* _ADR  is lowest 16 bit -> func, highest 16 bit -> slot
+	 * devfn is lowest  3 bit -> func, rest is        -> slot
+	 */
+	devfn = (acpi_device_adr(child) >> 13) |
+		(acpi_device_adr(child) & 0xFFFF);
+
+	result = acpi_pci_root_get_seg_bus(parent, &bus, &seg);
+	if (result < 0)
+		return -EINVAL;
+
+	pci = pci_get_bus_and_slot(bus, devfn);
+
+	/* There is no physical PCI device registered for the ACPI device */
+	if (!pci)
+		return -ENODEV;
+	pci_dev_put(pci);
+
+	return 0;
+}
+EXPORT_SYMBOL(acpi_find_pci_device);
include/linux/acpi.h +2 -0
Index: lenb/include/linux/acpi.h
===================================================================
@@ -170,6 +170,8 @@ struct acpi_prt_list {
 
 struct pci_dev;
 
+int acpi_find_pci_device(struct acpi_device *device, struct pci_dev *pci);
+
 int acpi_pci_irq_enable (struct pci_dev *dev);
 void acpi_penalize_isa_irq(int irq, int active);
 
#1 lore raw

Why not just call acpi_get_pyhsical_device() on the appropriate handle and look at the struct device you get back? Remember to put_device() it once you're done.

Signature · 1 line
Matthew Garrett | [email protected]
#2 lore raw
Quoted 4 lines · Matthew Garrett

Why not just call acpi_get_pyhsical_device() on the appropriate handle and look at the struct device you get back? Remember to put_device() it
once you're done.

Yes, for PCI bus, we already had a method to bind native device to ACPI device. Other bus not.

Thanks,
Shaohua

#3 lore raw

Re: [PATCH 1/1] Recent Lenovo ThinkPads define a dummy grahpics device, find it and ignore it

In reply to Matthew Garrett
Quoted 3 lines · Matthew Garrett

Why not just call acpi_get_pyhsical_device() on the appropriate handle and look at the struct device you get back? Remember to put_device() it once you're done.

Yep, works great. I should have asked earlier for such a thing...

Len can you apply this one, pls.

Thanks,

Thomas

Recent Lenovo ThinkPads define a dummy grahpics device, ignore it.

---
 drivers/acpi/video.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/acpi/video.c +14 -0
Index: lenb/drivers/acpi/video.c
===================================================================
@@ -34,6 +34,7 @@
 #include <linux/input.h>
 #include <linux/backlight.h>
 #include <linux/video_output.h>
+#include <linux/acpi.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
@@ -1901,11 +1902,24 @@ static int acpi_video_bus_add(struct acp
 	acpi_status status = 0;
 	struct acpi_video_bus *video = NULL;
 	struct input_dev *input;
+	struct device *dev;
 
 
 	if (!device)
 		return -EINVAL;
 
+	/*
+	 *	Check whether we have really  a graphics device physically
+	 *      in the slot and registered at the system.
+	 */
+	dev = acpi_get_physical_device(device->handle);
+	if (!dev) {
+		printk (KERN_DEBUG PREFIX "Video device %s.%s not physically"
+			" connected, ignoring\n", acpi_device_bid(device),
+			device->parent ? acpi_device_bid(device->parent) : "");
+		return -ENODEV;
+	}
+
 	video = kzalloc(sizeof(struct acpi_video_bus), GFP_KERNEL);
 	if (!video)
 		return -ENOMEM;
#4 lore raw

Re: [PATCH 1/1] Recent Lenovo ThinkPads define a dummy grahpics device, find it and ignore it

Quoted 5 lines · Thomas Renninger
Quoted 2 lines · Matthew Garrett

Why not just call acpi_get_pyhsical_device() on the appropriate handle and look at the struct device you get back?

Remember to put_device() it once you're done.

First read, then post..., forgot the put_device... This one is better.

Quoted 1 line

Len can you apply this one, pls.

Thomas

Recent Lenovo ThinkPads define a dummy grahpics device, find it and ignore it

---
 drivers/acpi/video.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)
drivers/acpi/video.c +15 -0
Index: lenb/drivers/acpi/video.c
===================================================================
@@ -34,6 +34,7 @@
 #include <linux/input.h>
 #include <linux/backlight.h>
 #include <linux/video_output.h>
+#include <linux/acpi.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
@@ -1901,11 +1902,25 @@ static int acpi_video_bus_add(struct acp
 	acpi_status status = 0;
 	struct acpi_video_bus *video = NULL;
 	struct input_dev *input;
+	struct device *dev;
 
 
 	if (!device)
 		return -EINVAL;
 
+	/*
+	 *	Check whether we have really  a graphics device physically
+	 *      in the slot and registered at the system.
+	 */
+	dev = acpi_get_physical_device(device->handle);
+	if (!dev) {
+		printk (KERN_DEBUG PREFIX "Video device %s.%s not physically"
+			" connected, ignoring\n", acpi_device_bid(device),
+			device->parent ? acpi_device_bid(device->parent) : "");
+		return -ENODEV;
+	}
+	put_device(dev);
+
 	video = kzalloc(sizeof(struct acpi_video_bus), GFP_KERNEL);
 	if (!video)
 		return -ENOMEM;
#5 lore raw

Re: [PATCH 1/1] Recent Lenovo ThinkPads define a dummy grahpics device, find it and ignore it

In reply to Thomas Renninger

drivers/acpi/video.c static int acpi_video_bus_add(struct acp · line 1922

Quoted 12 lines · Thomas Renninger
drivers/acpi/video.c +12 -0
+	/*
+	 *	Check whether we have really  a graphics device physically
+	 *      in the slot and registered at the system.
+	 */
+	dev = acpi_get_physical_device(device->handle);
+	if (!dev) {
+		printk (KERN_DEBUG PREFIX "Video device %s.%s not physically"
+			" connected, ignoring\n", acpi_device_bid(device),
+			device->parent ? acpi_device_bid(device->parent) : "");
+		return -ENODEV;
+	}
+

I suspect this will break other machines. Not all video extension implementations are directly associated with the PCI ID. The Toshiba M200 (for example) has

            Device (PCI1)
            {
                Name (_ADR, 0x00010000)
                Device (VGA)
                {
                    Name (_ADR, 0x00)

which will result in VGA not having a physical device. You might be able to get away with walking the parents until you find a pci ID and then checking whether it matches the graphics adaptor, but I'm not certain of that.

To make things more entertaining, Dell tend to implement a video extension for both the 00:02.0 and 00:02.1 devices on Intel systems. We need to be smarter about this, but I don't think simply looking for a physical device is the solution.

Signature · 1 line
Matthew Garrett | [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

Re: [PATCH 1/1] Recent Lenovo ThinkPads define a dummy grahpics device, find it and ignore it

Quoted 26 lines · Matthew Garrett

drivers/acpi/video.c static int acpi_video_bus_add(struct acp · line 1922

Quoted 12 lines · Thomas Renninger
drivers/acpi/video.c +12 -0
+	/*
+	 *	Check whether we have really  a graphics device physically
+	 *      in the slot and registered at the system.
+	 */
+	dev = acpi_get_physical_device(device->handle);
+	if (!dev) {
+		printk (KERN_DEBUG PREFIX "Video device %s.%s not physically"
+			" connected, ignoring\n", acpi_device_bid(device),
+			device->parent ? acpi_device_bid(device->parent) : "");
+		return -ENODEV;
+	}
+

I suspect this will break other machines. Not all video extension implementations are directly associated with the PCI ID. The Toshiba M200 (for example) has

            Device (PCI1)
            {
                Name (_ADR, 0x00010000)
                Device (VGA)
                {
                    Name (_ADR, 0x00)
Quoted 4 lines

which will result in VGA not having a physical device. You might be able to get away with walking the parents until you find a pci ID and then checking whether it matches the graphics adaptor, but I'm not certain of that.

You mean like I did in my previous approach?

Quoted 4 lines

To make things more entertaining, Dell tend to implement a video extension for both the 00:02.0 and 00:02.1 devices on Intel systems. We need to be smarter about this, but I don't think simply looking for a physical device is the solution.

Li, could you help here, pls.
I don't know enough about different possible PCI setups...

Thanks,

Thomas

-------------------------------------------------------------------------
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/