Re: [RFC][PATCH] gpu:drm:i915:intel_detect_pch: back to check devfn instead of check class type

From: Chen, Tiejun
Date: Sun Jun 22 2014 - 04:00:48 EST


On 2014/6/20 20:32, Daniel Vetter wrote:
Well I have no clue about forwarding the intel gpu to virtualized
hosts and also no idea who could review this really. There's been a
bit a discussion around the iommu mapping forwarding and similar

No, this doesn't affect IOMMU mapping.

topics though. So I really wonder how well our driver works in this
use case ...

In native case the truth is intel_detect_pch() needs to probe the 00:15.0 to determine what type current pch is, then the i915 driver can be initialized correctly. Actually the 00:15.0 is just a ISA bridge.

In virtualized case we thought this ISA bridge mayn't be represented with 00:15.0 originally by qemu-xen-traditional. So instead of checking devfn, the i915 driver check the class type to get this ISA bridge.

But with my investigation,qemu-xen-traditional always set 00:15.0 explicitly to represent this ISA bridge. And especially, we wouldn't provide that ISA bridge with an explicit class type in qemu-upstream, so we need to the i915 driver to probe pch by checking devfn.

This should work both on the native case and the virtualized case.

Thanks
Tiejun

-Daniel

On Fri, Jun 20, 2014 at 11:40 AM, Chen, Tiejun <tiejun.chen@xxxxxxxxx> wrote:
Just ping, any comments?

Thanks
Tiejun


On 2014/6/19 17:53, Tiejun Chen wrote:

Originally the reason to probe ISA bridge instead of Dev31:Fun0
is to make graphics device passthrough work easy for VMM, that
only need to expose ISA bridge to let driver know the real
hardware underneath. This is a requirement from virtualization
team. Especially in that virtualized environments, XEN, there
is irrelevant ISA bridge in the system with that legacy qemu
version specific to xen, qemu-xen-traditional. So to work
reliably, we should scan through all the ISA bridge devices
and check for the first match, instead of only checking the
first one.

But actually, qemu-xen-traditional, is always enumerated with
Dev31:Fun0, 00:1f.0 as follows:

hw/pt-graphics.c:

intel_pch_init()
|
+ pci_isa_bridge_init(bus, PCI_DEVFN(0x1f, 0), ...);

so this mean that isa bridge is still represented with Dev31:Func0
like the native OS. Furthermore, currently we're pushing VGA
passthrough support into qemu upstream, and with some discussion,
we wouldn't set the bridge class type and just expose this devfn.

So we just go back to check devfn to make life normal.

Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 651e65e..cb2526e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -417,18 +417,8 @@ void intel_detect_pch(struct drm_device *dev)
return;
}

- /*
- * The reason to probe ISA bridge instead of Dev31:Fun0 is to
- * make graphics device passthrough work easy for VMM, that only
- * need to expose ISA bridge to let driver know the real hardware
- * underneath. This is a requirement from virtualization team.
- *
- * In some virtualized environments (e.g. XEN), there is
irrelevant
- * ISA bridge in the system. To work reliably, we should scan
trhough
- * all the ISA bridge devices and check for the first match,
instead
- * of only checking the first one.
- */
- while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch))) {
+ pch = pci_get_bus_and_slot(0, PCI_DEVFN(0x1f, 0));
+ if (pch) {
if (pch->vendor == PCI_VENDOR_ID_INTEL) {
unsigned short id = pch->device &
INTEL_PCH_DEVICE_ID_MASK;
dev_priv->pch_id = id;
@@ -462,10 +452,7 @@ void intel_detect_pch(struct drm_device *dev)
DRM_DEBUG_KMS("Found LynxPoint LP PCH\n");
WARN_ON(!IS_HASWELL(dev));
WARN_ON(!IS_ULT(dev));
- } else
- continue;
-
- break;
+ }
}
}
if (!pch)





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/