[PATCH] drm/i915: Recognise non-VGA display devices

[PATCH] drm/i915: Recognise non-VGA display devices

[Chris Wilson]

Starting with SandyBridge (though possible with earlier hacked BIOSes),
the BIOS may initialise the IGFX as secondary to a discrete GPU. Prior,
it would simply disable the integrated GPU. In such a configuration, the
IGFX is not a primary VGA controller and so should not take part in VGA
arbitration.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |   34 +++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 76f2df7..46ef1fb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1183,6 +1183,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	unsigned long prealloc_size, gtt_size, mappable_size;
 	int ret = 0;
 
+	dev_priv->is_vga = dev->pdev->class == PCI_CLASS_DISPLAY_VGA;
+
 	prealloc_size = dev_priv->mm.gtt->stolen_size;
 	gtt_size = dev_priv->mm.gtt->gtt_total_entries << PAGE_SHIFT;
 	mappable_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
@@ -1228,19 +1230,23 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		DRM_INFO("failed to find VBIOS tables\n");
 
-	/* if we have > 1 VGA cards, then disable the radeon VGA resources */
-	ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
-	if (ret)
-		goto cleanup_ringbuffer;
+	if (dev_priv->is_vga) {
+		/* If we have > 1 VGA cards with a single output,
+		 * then disable the radeon VGA resources.
+		 */
+		ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
+		if (ret)
+			goto cleanup_ringbuffer;
 
-	intel_register_dsm_handler();
+		intel_register_dsm_handler();
 
-	ret = vga_switcheroo_register_client(dev->pdev,
-					     i915_switcheroo_set_state,
-					     NULL,
-					     i915_switcheroo_can_switch);
-	if (ret)
-		goto cleanup_vga_client;
+		ret = vga_switcheroo_register_client(dev->pdev,
+						     i915_switcheroo_set_state,
+						     NULL,
+						     i915_switcheroo_can_switch);
+		if (ret)
+			goto cleanup_vga_client;
+	}
 
 	/* IIR "flip pending" bit means done if this bit is set */
 	if (IS_GEN3(dev) && (I915_READ(ECOSKPD) & ECO_FLIP_DONE))
@@ -2106,8 +2112,10 @@ int i915_driver_unload(struct drm_device *dev)
 			dev_priv->child_dev_num = 0;
 		}
 
-		vga_switcheroo_unregister_client(dev->pdev);
-		vga_client_register(dev->pdev, NULL, NULL, NULL);
+		if (dev_priv->is_vga) {
+			vga_switcheroo_unregister_client(dev->pdev);
+			vga_client_register(dev->pdev, NULL, NULL, NULL);
+		}
 	}
 
 	/* Free error state after interrupts are fully disabled. */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2d31f5f..7fbb132 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -60,7 +60,7 @@ extern int intel_agp_enabled;
 
 #define INTEL_VGA_DEVICE(id, info) {		\
 	.class = PCI_CLASS_DISPLAY_VGA << 8,	\
-	.class_mask = 0xffff00,			\
+	.class_mask = 0xff0000,			\
 	.vendor = 0x8086,			\
 	.device = id,				\
 	.subvendor = PCI_ANY_ID,		\
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 52ceae5..2b274f4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -262,6 +262,7 @@ typedef struct drm_i915_private {
 
 	int has_gem;
 	int relative_constants_mode;
+	int is_vga;
 
 	void __iomem *regs;
 
-- 
1.7.2.3

Re: [PATCH] drm/i915: Recognise non-VGA display devices

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 292 bytes --]

On Thu, 20 Jan 2011 15:07:07 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:

> +	if (dev_priv->is_vga) {
> +		/* If we have > 1 VGA cards with a single output,
> +		 * then disable the radeon VGA resources.
> +		 */

Why does this say 'radeon'?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Recognise non-VGA display devices

[Chris Wilson]

On Thu, 20 Jan 2011 09:04:16 -0800, Keith Packard <keithp@keithp.com> wrote:
> On Thu, 20 Jan 2011 15:07:07 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > +	if (dev_priv->is_vga) {
> > +		/* If we have > 1 VGA cards with a single output,
> > +		 * then disable the radeon VGA resources.
> > +		 */
> 
> Why does this say 'radeon'?

Presumably that is what Dave had in his box when adding the comment. You
mean I have to fix comments as well... ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Recognise non-VGA display devices

[Dave Airlie]

On Fri, Jan 21, 2011 at 1:07 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Starting with SandyBridge (though possible with earlier hacked BIOSes),
> the BIOS may initialise the IGFX as secondary to a discrete GPU. Prior,
> it would simply disable the integrated GPU. In such a configuration, the
> IGFX is not a primary VGA controller and so should not take part in VGA
> arbitration.

VGA arb shouldn't happen for any devices not VGA class, it shouldn't place the
devices into the arb list at all in this case.

Dave.

Re: [PATCH] drm/i915: Recognise non-VGA display devices

[Chris Wilson]

On Fri, 21 Jan 2011 08:49:59 +1000, Dave Airlie <airlied@gmail.com> wrote:
> On Fri, Jan 21, 2011 at 1:07 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Starting with SandyBridge (though possible with earlier hacked BIOSes),
> > the BIOS may initialise the IGFX as secondary to a discrete GPU. Prior,
> > it would simply disable the integrated GPU. In such a configuration, the
> > IGFX is not a primary VGA controller and so should not take part in VGA
> > arbitration.
> 
> VGA arb shouldn't happen for any devices not VGA class, it shouldn't place the
> devices into the arb list at all in this case.

Right, it doesn't happen, but i915 also need not to throw an error when
vgaarb rejects the device as being !DISPLAY_VGA.

I'll try and update the comments to explain why I've chosen to handle it
this way.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Recognise non-VGA display devices

[Dave Airlie]

On Fri, Jan 21, 2011 at 10:25 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, 21 Jan 2011 08:49:59 +1000, Dave Airlie <airlied@gmail.com> wrote:
>> On Fri, Jan 21, 2011 at 1:07 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > Starting with SandyBridge (though possible with earlier hacked BIOSes),
>> > the BIOS may initialise the IGFX as secondary to a discrete GPU. Prior,
>> > it would simply disable the integrated GPU. In such a configuration, the
>> > IGFX is not a primary VGA controller and so should not take part in VGA
>> > arbitration.
>>
>> VGA arb shouldn't happen for any devices not VGA class, it shouldn't place the
>> devices into the arb list at all in this case.
>
> Right, it doesn't happen, but i915 also need not to throw an error when
> vgaarb rejects the device as being !DISPLAY_VGA.
>
> I'll try and update the comments to explain why I've chosen to handle it
> this way.

You could just ignore the error return, also vga arb and vga
switcheroo are very different
things, so they don't mean the same thing at all. So not sure why you
are touching
the switcheroo code here.

Or we could just fix the arbiter to not return anything on failure.

Dave.

[PATCH] drm/i915: Recognise non-VGA display devices

[Chris Wilson]

Starting with SandyBridge (though possible with earlier hacked BIOSes),
the BIOS may initialise the IGFX as secondary to a discrete GPU. Prior,
it would simply disable the integrated GPU. So we adjust the our PCI
class mask to match any DISPLAY_CLASS device.

In such a configuration, the IGFX is not a primary VGA controller and
so should not take part in VGA arbitration, and the error return from
vga_client_register() is expected.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++--
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 drivers/gpu/vga/vgaarb.c        |    2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 6658981..17bd766 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1215,9 +1215,15 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	if (ret)
 		DRM_INFO("failed to find VBIOS tables\n");
 
-	/* if we have > 1 VGA cards, then disable the radeon VGA resources */
+	/* If we have > 1 VGA cards, then we need to arbitrate access
+	 * to the common VGA resources.
+	 *
+	 * If we are a secondary display controller (!PCI_DISPLAY_CLASS_VGA),
+	 * then we do not take part in VGA arbitration and the
+	 * vga_client_register() fails with -ENODEV.
+	 */
 	ret = vga_client_register(dev->pdev, dev, NULL, i915_vga_set_decode);
-	if (ret)
+	if (ret && ret != -ENODEV)
 		goto cleanup_ringbuffer;
 
 	intel_register_dsm_handler();
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 72fea2b..59eb19b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -60,7 +60,7 @@ extern int intel_agp_enabled;
 
 #define INTEL_VGA_DEVICE(id, info) {		\
 	.class = PCI_CLASS_DISPLAY_VGA << 8,	\
-	.class_mask = 0xffff00,			\
+	.class_mask = 0xff0000,			\
 	.vendor = 0x8086,			\
 	.device = id,				\
 	.subvendor = PCI_ANY_ID,		\
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index c380c65..ace2b16 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -636,7 +636,7 @@ int vga_client_register(struct pci_dev *pdev, void *cookie,
 			void (*irq_set_state)(void *cookie, bool state),
 			unsigned int (*set_vga_decode)(void *cookie, bool decode))
 {
-	int ret = -1;
+	int ret = -ENODEV;
 	struct vga_device *vgadev;
 	unsigned long flags;
 
-- 
1.7.2.3