[RFC PATCH 0/5] i915 changes for hybrid graphics support on Macbooks

[RFC PATCH 0/5] i915 changes for hybrid graphics support on Macbooks

[Seth Forshee]

The following patches are part of a larger series I've been working on
to get vga_switcheroo working on hybrid graphics Macbooks. Some of these
machines are not providing a VBT, and since the LVDS panel is connected
to the discrete GPU at boot we can't get a mode for the panel during
initialization. As a result the LVDS connector is not registered with
DRM, and graphics switching is not possible.

These patches fix this issue by registering the connector even if we
can't get a mode for the panel. If we don't have an EDID we check again
from the vga_switcheroo reprobe callback.

This is working, except for the framebuffer console which isn't
displaying when switched to the integrated GPU, which I still need to
debug. I'm not entirely sure whether or not this is the correct approach
though, so I wanted to go ahead and get some feedback on the patches now
to make sure I'm on the right track.

Thanks,
Seth


Andreas Heider (1):
  drm/i915: Add support for vga_switcheroo reprobe

Seth Forshee (4):
  drm/i915: separate out code to get EDID from LVDS panel
  drm/i915: register LVDS connector even if we can't get a panel mode
  drm/i915: make intel_lvds_get_edid() more robust
  drm/i915: check LVDS for EDID on GPU switches

 drivers/gpu/drm/i915/i915_dma.c   |    9 ++-
 drivers/gpu/drm/i915/intel_drv.h  |    1 +
 drivers/gpu/drm/i915/intel_lvds.c |  156 +++++++++++++++++++++----------------
 3 files changed, 97 insertions(+), 69 deletions(-)

[RFC PATCH 1/5] drm/i915: Add support for vga_switcheroo reprobe

[Seth Forshee]

From: Andreas Heider <andreas@meetr.de>

Signed-off-by: Andreas Heider <andreas@meetr.de>
---
 drivers/gpu/drm/i915/i915_dma.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9cf7dfe..5b5176d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1263,6 +1263,12 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_
 	}
 }
 
+static void i915_switcheroo_reprobe(struct pci_dev *pdev)
+{
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	intel_fb_output_poll_changed(dev);
+}
+
 static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
@@ -1276,7 +1282,7 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 
 static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.set_gpu_state = i915_switcheroo_set_state,
-	.reprobe = NULL,
+	.reprobe = i915_switcheroo_reprobe,
 	.can_switch = i915_switcheroo_can_switch,
 };
 
-- 
1.7.9.5

[RFC PATCH 2/5] drm/i915: separate out code to get EDID from LVDS panel

[Seth Forshee]

This code will be reused to support hybrid graphics on some Apple
machines that can't get a mode for the LVDS panel at boot, so move it
into a new function named intel_lvds_get_edid().

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   95 +++++++++++++++++++++----------------
 1 file changed, 55 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index e05c0d3..5069137 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -46,6 +46,7 @@ struct intel_lvds {
 
 	struct edid *edid;
 
+	u8 i2c_pin;
 	int fitting_mode;
 	u32 pfit_control;
 	u32 pfit_pgm_ratios;
@@ -897,6 +898,54 @@ static bool intel_lvds_supported(struct drm_device *dev)
 	return IS_MOBILE(dev) && !IS_I830(dev);
 }
 
+static bool intel_lvds_get_edid(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_connector *connector = dev_priv->int_lvds_connector;
+	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
+	struct drm_display_mode *scan; /* *modes, *bios_mode; */
+
+	/*
+	 * Attempt to get the fixed panel mode from DDC.  Assume that the
+	 * preferred mode is the right one.
+	 */
+	intel_lvds->edid = drm_get_edid(connector,
+					intel_gmbus_get_adapter(dev_priv,
+								intel_lvds->i2c_pin));
+	if (intel_lvds->edid) {
+		if (drm_add_edid_modes(connector,
+				       intel_lvds->edid)) {
+			drm_mode_connector_update_edid_property(connector,
+								intel_lvds->edid);
+		} else {
+			kfree(intel_lvds->edid);
+			intel_lvds->edid = NULL;
+		}
+	}
+	if (!intel_lvds->edid) {
+		/* Didn't get an EDID, so
+		 * Set wide sync ranges so we get all modes
+		 * handed to valid_mode for checking
+		 */
+		connector->display_info.min_vfreq = 0;
+		connector->display_info.max_vfreq = 200;
+		connector->display_info.min_hfreq = 0;
+		connector->display_info.max_hfreq = 200;
+	}
+
+	list_for_each_entry(scan, &connector->probed_modes, head) {
+		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
+			intel_lvds->fixed_mode =
+				drm_mode_duplicate(dev, scan);
+			intel_find_lvds_downclock(dev,
+						  intel_lvds->fixed_mode,
+						  connector);
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * intel_lvds_init - setup LVDS connectors on this device
  * @dev: drm device
@@ -912,7 +961,6 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_display_mode *scan; /* *modes, *bios_mode; */
 	struct drm_crtc *crtc;
 	u32 lvds;
 	int pipe;
@@ -955,9 +1003,11 @@ bool intel_lvds_init(struct drm_device *dev)
 		intel_lvds->pfit_control = I915_READ(PFIT_CONTROL);
 	}
 
+	intel_lvds->i2c_pin = pin;
 	intel_encoder = &intel_lvds->base;
 	encoder = &intel_encoder->base;
 	connector = &intel_connector->base;
+	dev_priv->int_lvds_connector = connector;
 	drm_connector_init(dev, &intel_connector->base, &intel_lvds_connector_funcs,
 			   DRM_MODE_CONNECTOR_LVDS);
 
@@ -991,6 +1041,7 @@ bool intel_lvds_init(struct drm_device *dev)
 				      dev->mode_config.scaling_mode_property,
 				      DRM_MODE_SCALE_ASPECT);
 	intel_lvds->fitting_mode = DRM_MODE_SCALE_ASPECT;
+
 	/*
 	 * LVDS discovery:
 	 * 1) check for EDID on DDC
@@ -1001,44 +1052,8 @@ bool intel_lvds_init(struct drm_device *dev)
 	 *    if closed, act like it's not there for now
 	 */
 
-	/*
-	 * Attempt to get the fixed panel mode from DDC.  Assume that the
-	 * preferred mode is the right one.
-	 */
-	intel_lvds->edid = drm_get_edid(connector,
-					intel_gmbus_get_adapter(dev_priv,
-								pin));
-	if (intel_lvds->edid) {
-		if (drm_add_edid_modes(connector,
-				       intel_lvds->edid)) {
-			drm_mode_connector_update_edid_property(connector,
-								intel_lvds->edid);
-		} else {
-			kfree(intel_lvds->edid);
-			intel_lvds->edid = NULL;
-		}
-	}
-	if (!intel_lvds->edid) {
-		/* Didn't get an EDID, so
-		 * Set wide sync ranges so we get all modes
-		 * handed to valid_mode for checking
-		 */
-		connector->display_info.min_vfreq = 0;
-		connector->display_info.max_vfreq = 200;
-		connector->display_info.min_hfreq = 0;
-		connector->display_info.max_hfreq = 200;
-	}
-
-	list_for_each_entry(scan, &connector->probed_modes, head) {
-		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
-			intel_lvds->fixed_mode =
-				drm_mode_duplicate(dev, scan);
-			intel_find_lvds_downclock(dev,
-						  intel_lvds->fixed_mode,
-						  connector);
-			goto out;
-		}
-	}
+	if (intel_lvds_get_edid(dev))
+		goto out;
 
 	/* Failed to get EDID, what about VBT? */
 	if (dev_priv->lfp_lvds_vbt_mode) {
@@ -1096,7 +1111,6 @@ out:
 		dev_priv->lid_notifier.notifier_call = NULL;
 	}
 	/* keep the LVDS connector */
-	dev_priv->int_lvds_connector = connector;
 	drm_sysfs_connector_add(connector);
 
 	intel_panel_setup_backlight(dev);
@@ -1105,6 +1119,7 @@ out:
 
 failed:
 	DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
+	dev_priv->int_lvds_connector = NULL;
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
 	kfree(intel_lvds);
-- 
1.7.9.5

[RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

Some Apple hybrid graphics machines do not have the LVDS panel connected
to the integrated GPU at boot and also do not supply a VBT. The LVDS
connector is not registered as a result, making it impossible to support
graphics switching.

This patch changes intel_lvds_init() to register the connector even if
we can't find any panel modes. This makes it necessary to always check
intel_lvds->fixed_mode before use, as it could now be NULL.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   48 +++++++++++++++----------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 5069137..c1ab632 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -161,6 +161,8 @@ static int intel_lvds_mode_valid(struct drm_connector *connector,
 	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
 	struct drm_display_mode *fixed_mode = intel_lvds->fixed_mode;
 
+	if (!fixed_mode)
+		return MODE_PANEL;
 	if (mode->hdisplay > fixed_mode->hdisplay)
 		return MODE_PANEL;
 	if (mode->vdisplay > fixed_mode->vdisplay)
@@ -262,7 +264,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder,
 	 * with the panel scaling set up to source from the H/VDisplay
 	 * of the original mode.
 	 */
-	intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
+	if (intel_lvds->fixed_mode)
+		intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode);
 
 	if (HAS_PCH_SPLIT(dev)) {
 		intel_pch_panel_fitting(dev, intel_lvds->fitting_mode,
@@ -461,12 +464,13 @@ static int intel_lvds_get_modes(struct drm_connector *connector)
 {
 	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
 	struct drm_device *dev = connector->dev;
-	struct drm_display_mode *mode;
+	struct drm_display_mode *mode = NULL;
 
 	if (intel_lvds->edid)
 		return drm_add_edid_modes(connector, intel_lvds->edid);
 
-	mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
+	if (intel_lvds->fixed_mode)
+		mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode);
 	if (mode == NULL)
 		return 0;
 
@@ -1073,26 +1077,21 @@ bool intel_lvds_init(struct drm_device *dev)
 	 */
 
 	/* Ironlake: FIXME if still fail, not try pipe mode now */
-	if (HAS_PCH_SPLIT(dev))
-		goto failed;
-
-	lvds = I915_READ(LVDS);
-	pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
-	crtc = intel_get_crtc_for_pipe(dev, pipe);
-
-	if (crtc && (lvds & LVDS_PORT_EN)) {
-		intel_lvds->fixed_mode = intel_crtc_mode_get(dev, crtc);
-		if (intel_lvds->fixed_mode) {
-			intel_lvds->fixed_mode->type |=
-				DRM_MODE_TYPE_PREFERRED;
-			goto out;
+	if (!HAS_PCH_SPLIT(dev)) {
+		lvds = I915_READ(LVDS);
+		pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
+		crtc = intel_get_crtc_for_pipe(dev, pipe);
+
+		if (crtc && (lvds & LVDS_PORT_EN)) {
+			intel_lvds->fixed_mode = intel_crtc_mode_get(dev, crtc);
+			if (intel_lvds->fixed_mode) {
+				intel_lvds->fixed_mode->type |=
+					DRM_MODE_TYPE_PREFERRED;
+				goto out;
+			}
 		}
 	}
 
-	/* If we still don't have a mode after all that, give up. */
-	if (!intel_lvds->fixed_mode)
-		goto failed;
-
 out:
 	/*
 	 * Unlock registers and just
@@ -1116,13 +1115,4 @@ out:
 	intel_panel_setup_backlight(dev);
 
 	return true;
-
-failed:
-	DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
-	dev_priv->int_lvds_connector = NULL;
-	drm_connector_cleanup(connector);
-	drm_encoder_cleanup(encoder);
-	kfree(intel_lvds);
-	kfree(intel_connector);
-	return false;
 }
-- 
1.7.9.5

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Matthew Garrett]

On Fri, Aug 03, 2012 at 11:02:19AM -0500, Seth Forshee wrote:
> Some Apple hybrid graphics machines do not have the LVDS panel connected
> to the integrated GPU at boot and also do not supply a VBT. The LVDS
> connector is not registered as a result, making it impossible to support
> graphics switching.
> 
> This patch changes intel_lvds_init() to register the connector even if
> we can't find any panel modes. This makes it necessary to always check
> intel_lvds->fixed_mode before use, as it could now be NULL.

This one kind of sucks. I think adding a quirk for this situation would 
be justifiable, rather than doing it for all devices.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Fri, Aug 03, 2012 at 05:14:16PM +0100, Matthew Garrett wrote:
> On Fri, Aug 03, 2012 at 11:02:19AM -0500, Seth Forshee wrote:
> > Some Apple hybrid graphics machines do not have the LVDS panel connected
> > to the integrated GPU at boot and also do not supply a VBT. The LVDS
> > connector is not registered as a result, making it impossible to support
> > graphics switching.
> > 
> > This patch changes intel_lvds_init() to register the connector even if
> > we can't find any panel modes. This makes it necessary to always check
> > intel_lvds->fixed_mode before use, as it could now be NULL.
> 
> This one kind of sucks. I think adding a quirk for this situation would 
> be justifiable, rather than doing it for all devices.

This is one of the things I wasn't so sure about. There are various
checks in intel_lvds_init() that can cause it to bail out before we try
to get the EDID, and I don't fully understand all of them. If non-laptop
machines are expected to bail out before the EDID check then the
approach I've taken seems reasonable. Otherwise adding a quirk probably
is a good idea.

Seth

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Matthew Garrett]

On Fri, Aug 03, 2012 at 11:24:51AM -0500, Seth Forshee wrote:

> This is one of the things I wasn't so sure about. There are various
> checks in intel_lvds_init() that can cause it to bail out before we try
> to get the EDID, and I don't fully understand all of them. If non-laptop
> machines are expected to bail out before the EDID check then the
> approach I've taken seems reasonable. Otherwise adding a quirk probably
> is a good idea.

I know we've previously had problems with machines with phantom LVDS 
hardware, but I'm not sure what the current state of affairs is.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Fri, Aug 03, 2012 at 05:27:02PM +0100, Matthew Garrett wrote:
> On Fri, Aug 03, 2012 at 11:24:51AM -0500, Seth Forshee wrote:
> 
> > This is one of the things I wasn't so sure about. There are various
> > checks in intel_lvds_init() that can cause it to bail out before we try
> > to get the EDID, and I don't fully understand all of them. If non-laptop
> > machines are expected to bail out before the EDID check then the
> > approach I've taken seems reasonable. Otherwise adding a quirk probably
> > is a good idea.
> 
> I know we've previously had problems with machines with phantom LVDS 
> hardware, but I'm not sure what the current state of affairs is.

It turns out that the framebuffer console issue is due to not having a
mode when initializing LVDS. As a result 1024x768 is getting used for
the framebuffer.

So quirking is going to fix this situation. In fact, with the changes
below switcheroo seems to work perfectly, without any of these patches
at all.


diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 627fe35..d83e5bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -503,6 +503,7 @@ typedef struct drm_i915_private {
 	enum intel_pch pch_type;
 
 	unsigned long quirks;
+	struct drm_display_mode *lvds_quirk_mode;
 
 	/* Register state */
 	bool modeset_on_lid;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f615976..c810177 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7069,7 +7069,7 @@ static void intel_init_display(struct drm_device *dev)
  * resume, or other times.  This quirk makes sure that's the case for
  * affected systems.
  */
-static void quirk_pipea_force(struct drm_device *dev)
+static void quirk_pipea_force(struct drm_device *dev, void *data)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -7080,7 +7080,7 @@ static void quirk_pipea_force(struct drm_device *dev)
 /*
  * Some machines (Lenovo U160) do not work with SSC on LVDS for some reason
  */
-static void quirk_ssc_force_disable(struct drm_device *dev)
+static void quirk_ssc_force_disable(struct drm_device *dev, void *data)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
@@ -7091,48 +7091,74 @@ static void quirk_ssc_force_disable(struct drm_device *dev)
  * A machine (e.g. Acer Aspire 5734Z) may need to invert the panel backlight
  * brightness value
  */
-static void quirk_invert_brightness(struct drm_device *dev)
+static void quirk_invert_brightness(struct drm_device *dev, void *data)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	dev_priv->quirks |= QUIRK_INVERT_BRIGHTNESS;
 	DRM_INFO("applying inverted panel brightness quirk\n");
 }
 
+/*
+ * Some machines (e.g. certain Macbooks) may not be able to determine the
+ * LVDS mode during driver initialization
+ */
+static void quirk_lvds_panel_mode(struct drm_device *dev, void *data)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	dev_priv->lvds_quirk_mode = data;
+	DRM_INFO("applying LVDS panel mode quirk: %p\n", data);
+}
+
+/* LVDS panel mode for Macbook Pro 8,2 */
+struct drm_display_mode mbp82_panel_mode = {
+	DRM_MODE("1440x900", DRM_MODE_TYPE_DRIVER, 88750, 1440, 1488, 1520,
+		 1600, 0, 900, 903, 909, 926, 0,
+		 DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC)
+};
+
 struct intel_quirk {
 	int device;
 	int subsystem_vendor;
 	int subsystem_device;
-	void (*hook)(struct drm_device *dev);
+	void (*hook)(struct drm_device *dev, void *data);
+	void *hook_data;
 };
 
 static struct intel_quirk intel_quirks[] = {
 	/* HP Mini needs pipe A force quirk (LP: #322104) */
-	{ 0x27ae, 0x103c, 0x361a, quirk_pipea_force },
+	{ 0x27ae, 0x103c, 0x361a, quirk_pipea_force, NULL },
 
 	/* Thinkpad R31 needs pipe A force quirk */
-	{ 0x3577, 0x1014, 0x0505, quirk_pipea_force },
+	{ 0x3577, 0x1014, 0x0505, quirk_pipea_force, NULL },
 	/* Toshiba Protege R-205, S-209 needs pipe A force quirk */
-	{ 0x2592, 0x1179, 0x0001, quirk_pipea_force },
+	{ 0x2592, 0x1179, 0x0001, quirk_pipea_force, NULL },
 
 	/* ThinkPad X30 needs pipe A force quirk (LP: #304614) */
-	{ 0x3577,  0x1014, 0x0513, quirk_pipea_force },
+	{ 0x3577,  0x1014, 0x0513, quirk_pipea_force, NULL },
 	/* ThinkPad X40 needs pipe A force quirk */
 
 	/* ThinkPad T60 needs pipe A force quirk (bug #16494) */
-	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force },
+	{ 0x2782, 0x17aa, 0x201a, quirk_pipea_force, NULL },
 
 	/* 855 & before need to leave pipe A & dpll A up */
-	{ 0x3582, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
-	{ 0x2562, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force },
+	{ 0x3582, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force, NULL },
+	{ 0x2562, PCI_ANY_ID, PCI_ANY_ID, quirk_pipea_force, NULL },
 
 	/* Lenovo U160 cannot use SSC on LVDS */
-	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable },
+	{ 0x0046, 0x17aa, 0x3920, quirk_ssc_force_disable, NULL },
 
 	/* Sony Vaio Y cannot use SSC on LVDS */
-	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
+	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable, NULL },
 
 	/* Acer Aspire 5734Z must invert backlight brightness */
-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
+	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness, NULL },
+
+	/*
+	 * Some versions of the Macbook Pro 8,2 cannot use SSC and
+	 * cannot get the panel mode on LVDS
+	 */
+	{ 0x0116, 0x106b, 0x00dc, quirk_ssc_force_disable, NULL },
+	{ 0x0116, 0x106b, 0x00dc, quirk_lvds_panel_mode, &mbp82_panel_mode },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
@@ -7148,7 +7174,7 @@ static void intel_init_quirks(struct drm_device *dev)
 		     q->subsystem_vendor == PCI_ANY_ID) &&
 		    (d->subsystem_device == q->subsystem_device ||
 		     q->subsystem_device == PCI_ANY_ID))
-			q->hook(dev);
+			q->hook(dev, q->hook_data);
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index e05c0d3..303068d 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -996,6 +996,7 @@ bool intel_lvds_init(struct drm_device *dev)
 	 * 1) check for EDID on DDC
 	 * 2) check for VBT data
 	 * 3) check to see if LVDS is already on
+	 * 4) check for LVDS panel mode quirk
 	 *    if none of the above, no panel
 	 * 4) make sure lid is open
 	 *    if closed, act like it's not there for now
@@ -1058,15 +1059,25 @@ bool intel_lvds_init(struct drm_device *dev)
 	 */
 
 	/* Ironlake: FIXME if still fail, not try pipe mode now */
-	if (HAS_PCH_SPLIT(dev))
-		goto failed;
-
-	lvds = I915_READ(LVDS);
-	pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
-	crtc = intel_get_crtc_for_pipe(dev, pipe);
+	if (!HAS_PCH_SPLIT(dev)) {
+		lvds = I915_READ(LVDS);
+		pipe = (lvds & LVDS_PIPEB_SELECT) ? 1 : 0;
+		crtc = intel_get_crtc_for_pipe(dev, pipe);
+
+		if (crtc && (lvds & LVDS_PORT_EN)) {
+			intel_lvds->fixed_mode = intel_crtc_mode_get(dev, crtc);
+			if (intel_lvds->fixed_mode) {
+				intel_lvds->fixed_mode->type |=
+					DRM_MODE_TYPE_PREFERRED;
+				goto out;
+			}
+		}
+	}
 
-	if (crtc && (lvds & LVDS_PORT_EN)) {
-		intel_lvds->fixed_mode = intel_crtc_mode_get(dev, crtc);
+	/* Check for panel mode quirk as a last resort */
+	if (dev_priv->lvds_quirk_mode) {
+		intel_lvds->fixed_mode =
+			drm_mode_duplicate(dev, dev_priv->lvds_quirk_mode);
 		if (intel_lvds->fixed_mode) {
 			intel_lvds->fixed_mode->type |=
 				DRM_MODE_TYPE_PREFERRED;

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Daniel Vetter]

On Sat, Aug 04, 2012 at 11:57:27AM -0500, Seth Forshee wrote:
> On Fri, Aug 03, 2012 at 05:27:02PM +0100, Matthew Garrett wrote:
> > On Fri, Aug 03, 2012 at 11:24:51AM -0500, Seth Forshee wrote:
> > 
> > > This is one of the things I wasn't so sure about. There are various
> > > checks in intel_lvds_init() that can cause it to bail out before we try
> > > to get the EDID, and I don't fully understand all of them. If non-laptop
> > > machines are expected to bail out before the EDID check then the
> > > approach I've taken seems reasonable. Otherwise adding a quirk probably
> > > is a good idea.
> > 
> > I know we've previously had problems with machines with phantom LVDS 
> > hardware, but I'm not sure what the current state of affairs is.
> 
> It turns out that the framebuffer console issue is due to not having a
> mode when initializing LVDS. As a result 1024x768 is getting used for
> the framebuffer.
> 
> So quirking is going to fix this situation. In fact, with the changes
> below switcheroo seems to work perfectly, without any of these patches
> at all.

I like this approach more - the only other solution I see is to ask the
currently active driver (i.e. radeon) at bootime for the right mode. Which
sounds much more hellish and fragile ...
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Matthew Garrett]

On Sun, Aug 05, 2012 at 11:14:12PM +0200, Daniel Vetter wrote:

> I like this approach more - the only other solution I see is to ask the
> currently active driver (i.e. radeon) at bootime for the right mode. Which
> sounds much more hellish and fragile ...

The "correct" approach is clearly to just have the drm core change the 
i2c mux before requesting edid, but that's made difficult because of the 
absence of ordering guarantees in initialisation. I don't like quirking 
this, since we're then back to the situation of potentially having to 
add every new piece of related hardware to the quirk list.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Daniel Vetter]

On Sun, Aug 05, 2012 at 10:18:38PM +0100, Matthew Garrett wrote:
> On Sun, Aug 05, 2012 at 11:14:12PM +0200, Daniel Vetter wrote:
> 
> > I like this approach more - the only other solution I see is to ask the
> > currently active driver (i.e. radeon) at bootime for the right mode. Which
> > sounds much more hellish and fragile ...
> 
> The "correct" approach is clearly to just have the drm core change the 
> i2c mux before requesting edid, but that's made difficult because of the 
> absence of ordering guarantees in initialisation. I don't like quirking 
> this, since we're then back to the situation of potentially having to 
> add every new piece of related hardware to the quirk list.

The "correct" approach of switching the mux before we fetch the edid is
actualy the one I fear will result in fragile code: Only run on few
machines, and as you say with tons of funky interactions with the init
sequence ordering. And I guess people will bitch&moan about the flickering
this will cause ;-)

As long as it's only apple shipping multi-gpu machines with
broken/non-existing vbt, I'll happily stomach the quirk list entries.
They're bad, but imo the lesser evil.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Dave Airlie]

On Mon, Aug 6, 2012 at 7:40 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sun, Aug 05, 2012 at 10:18:38PM +0100, Matthew Garrett wrote:
>> On Sun, Aug 05, 2012 at 11:14:12PM +0200, Daniel Vetter wrote:
>>
>> > I like this approach more - the only other solution I see is to ask the
>> > currently active driver (i.e. radeon) at bootime for the right mode. Which
>> > sounds much more hellish and fragile ...
>>
>> The "correct" approach is clearly to just have the drm core change the
>> i2c mux before requesting edid, but that's made difficult because of the
>> absence of ordering guarantees in initialisation. I don't like quirking
>> this, since we're then back to the situation of potentially having to
>> add every new piece of related hardware to the quirk list.
>
> The "correct" approach of switching the mux before we fetch the edid is
> actualy the one I fear will result in fragile code: Only run on few
> machines, and as you say with tons of funky interactions with the init
> sequence ordering. And I guess people will bitch&moan about the flickering
> this will cause ;-)
>
> As long as it's only apple shipping multi-gpu machines with
> broken/non-existing vbt, I'll happily stomach the quirk list entries.
> They're bad, but imo the lesser evil.

Well in theory you can switch the ddc lines without switching the other lines,
so we could do a mutex protected mux switch around edid retrival,

Of course someone would have to code it up first then we could see how
ugly it would be.

Dave.
> -Daniel
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
> --
> 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/

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Alex Deucher]

On Sun, Aug 5, 2012 at 5:44 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Mon, Aug 6, 2012 at 7:40 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Sun, Aug 05, 2012 at 10:18:38PM +0100, Matthew Garrett wrote:
>>> On Sun, Aug 05, 2012 at 11:14:12PM +0200, Daniel Vetter wrote:
>>>
>>> > I like this approach more - the only other solution I see is to ask the
>>> > currently active driver (i.e. radeon) at bootime for the right mode. Which
>>> > sounds much more hellish and fragile ...
>>>
>>> The "correct" approach is clearly to just have the drm core change the
>>> i2c mux before requesting edid, but that's made difficult because of the
>>> absence of ordering guarantees in initialisation. I don't like quirking
>>> this, since we're then back to the situation of potentially having to
>>> add every new piece of related hardware to the quirk list.
>>
>> The "correct" approach of switching the mux before we fetch the edid is
>> actualy the one I fear will result in fragile code: Only run on few
>> machines, and as you say with tons of funky interactions with the init
>> sequence ordering. And I guess people will bitch&moan about the flickering
>> this will cause ;-)
>>
>> As long as it's only apple shipping multi-gpu machines with
>> broken/non-existing vbt, I'll happily stomach the quirk list entries.
>> They're bad, but imo the lesser evil.
>
> Well in theory you can switch the ddc lines without switching the other lines,
> so we could do a mutex protected mux switch around edid retrival,
>

Depends on the system.  On non-Macs, some systems have a single mux,
others have a separate mux for i2c and display as specified in the
ATPX ACPI methods.  Not sure how the Macs do it.  I've started
cleaning up the PX radeon code along with a bunch of other radeon
ralated ACPI fixes:
http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches

Alex

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Sun, Aug 05, 2012 at 07:20:31PM -0400, Alex Deucher wrote:
> On Sun, Aug 5, 2012 at 5:44 PM, Dave Airlie <airlied@gmail.com> wrote:
> > On Mon, Aug 6, 2012 at 7:40 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> On Sun, Aug 05, 2012 at 10:18:38PM +0100, Matthew Garrett wrote:
> >>> On Sun, Aug 05, 2012 at 11:14:12PM +0200, Daniel Vetter wrote:
> >>>
> >>> > I like this approach more - the only other solution I see is to ask the
> >>> > currently active driver (i.e. radeon) at bootime for the right mode. Which
> >>> > sounds much more hellish and fragile ...
> >>>
> >>> The "correct" approach is clearly to just have the drm core change the
> >>> i2c mux before requesting edid, but that's made difficult because of the
> >>> absence of ordering guarantees in initialisation. I don't like quirking
> >>> this, since we're then back to the situation of potentially having to
> >>> add every new piece of related hardware to the quirk list.
> >>
> >> The "correct" approach of switching the mux before we fetch the edid is
> >> actualy the one I fear will result in fragile code: Only run on few
> >> machines, and as you say with tons of funky interactions with the init
> >> sequence ordering. And I guess people will bitch&moan about the flickering
> >> this will cause ;-)
> >>
> >> As long as it's only apple shipping multi-gpu machines with
> >> broken/non-existing vbt, I'll happily stomach the quirk list entries.
> >> They're bad, but imo the lesser evil.
> >
> > Well in theory you can switch the ddc lines without switching the other lines,
> > so we could do a mutex protected mux switch around edid retrival,
> >
> 
> Depends on the system.  On non-Macs, some systems have a single mux,
> others have a separate mux for i2c and display as specified in the
> ATPX ACPI methods.  Not sure how the Macs do it.  I've started
> cleaning up the PX radeon code along with a bunch of other radeon
> ralated ACPI fixes:
> http://cgit.freedesktop.org/~agd5f/linux/log/?h=acpi_patches

The Macs mux the i2c and display separately. However they don't support
the vendor ACPI interfaces for the mux. The driver that provides the
vga_switcheroo handler is separate from the graphics drivers, and the
same whether the discrete graphics are Radeon or nVidia.

Really to support this in any generic sort of way vga_switcheroo needs
to support muxing the DDC separately from the display, but as Matthew
pointed out the ordering of initialization could be a problem. Even if
we protect the DDC with a mutex how can we guarantee that the switcheroo
handler is registered to switch the DDC before i915 is ready to check
for an EDID?

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Fri, Aug 10, 2012 at 05:19:48PM -0500, Seth Forshee wrote:
> First, I don't have a solution for the ordering of initialization. It
> just happens to work out for me right now.

Okay, I've got a proof-of-concept implementation of delaying secondary
GPU initialization until the i2c can be muxed over to that card. It's
not exactly pretty, but it is working. I'd really like to get some
feedback on the concept and implementation before spending more time on
it. Patches follow.

One problem I'm aware of is if the switcheroo handler is in the driver
for the secondary GPU. I think this was the case for a machine I used to
have with Optimus graphics. In that scenario the secondary graphics
device is never initialized because the switcheroo handler is registered
during initialization of the secondary device. The driver load method
would need to be split up to cope with this.

Thanks,
Seth

[RFC PATCH 1/7] vga_switcheroo: Add support for switching only the DDC

[Seth Forshee]

During graphics driver initialization its useful to be able to mux only
the DDC to the inactive client in order to read the EDID. Add a
switch_ddc callback to allow capable handlers to provide this
functionality, and add vga_switcheroo_switch_ddc() to allow DRM to mux
only the DDC.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/vga/vga_switcheroo.c |   39 +++++++++++++++++++++++++++++++++++++-
 include/linux/vga_switcheroo.h   |    4 ++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index e25cf31..ea6bcc2 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -252,6 +252,29 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
 }
 EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int id;
+
+	mutex_lock(&vgasr_mutex);
+
+	if (!vgasr_priv.handler) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (vgasr_priv.handler->switch_ddc) {
+		id = vgasr_priv.handler->get_client_id(pdev);
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+
+out:
+	mutex_unlock(&vgasr_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(vga_switcheroo_switch_ddc);
+
 static int vga_switcheroo_show(struct seq_file *m, void *v)
 {
 	struct vga_switcheroo_client *client;
@@ -342,9 +365,15 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
 	}
 
+	if (vgasr_priv.handler->switch_ddc) {
+		ret = vgasr_priv.handler->switch_ddc(new_client->id);
+		if (ret)
+			return ret;
+	}
+
 	ret = vgasr_priv.handler->switchto(new_client->id);
 	if (ret)
-		return ret;
+		goto restore_ddc;
 
 	if (new_client->ops->reprobe)
 		new_client->ops->reprobe(new_client->pdev);
@@ -356,6 +385,14 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 
 	new_client->active = true;
 	return 0;
+
+restore_ddc:
+	if (vgasr_priv.handler->switch_ddc) {
+		int id = (new_client->id == VGA_SWITCHEROO_IGD) ?
+				VGA_SWITCHEROO_DIS : VGA_SWITCHEROO_IGD;
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+	return ret;
 }
 
 static bool check_can_switch(void)
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index ddb419c..b0d0839 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -29,6 +29,7 @@ enum vga_switcheroo_client_id {
 };
 
 struct vga_switcheroo_handler {
+	int (*switch_ddc)(enum vga_switcheroo_client_id id);
 	int (*switchto)(enum vga_switcheroo_client_id id);
 	int (*power_state)(enum vga_switcheroo_client_id id,
 			   enum vga_switcheroo_state state);
@@ -53,6 +54,8 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev);
+
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler);
 void vga_switcheroo_unregister_handler(void);
 
@@ -66,6 +69,7 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
+static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; }
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,
-- 
1.7.9.5

[RFC PATCH 2/7] vga_switcheroo: Add helper function to get the active client

[Seth Forshee]

Add vga_switcheroo_get_active_client() to allow drivers to get the
active video client. This will be used by drivers wishing to temporarily
mux only the DDC to the inactive client.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/vga/vga_switcheroo.c |   14 ++++++++++++++
 include/linux/vga_switcheroo.h   |    2 ++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index ea6bcc2..e53f67d 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -205,6 +205,20 @@ find_active_client(struct list_head *head)
 	return NULL;
 }
 
+struct pci_dev *vga_switcheroo_get_active_client(void)
+{
+	struct vga_switcheroo_client *client;
+	struct pci_dev *pdev = NULL;
+
+	mutex_lock(&vgasr_mutex);
+	client = find_active_client(&vgasr_priv.clients);
+	if (client)
+		pdev = client->pdev;
+	mutex_unlock(&vgasr_mutex);
+	return pdev;
+}
+EXPORT_SYMBOL(vga_switcheroo_get_active_client);
+
 int vga_switcheroo_get_client_state(struct pci_dev *pdev)
 {
 	struct vga_switcheroo_client *client;
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index b0d0839..e361858 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -61,6 +61,7 @@ void vga_switcheroo_unregister_handler(void);
 
 int vga_switcheroo_process_delayed_switch(void);
 
+struct pci_dev *vga_switcheroo_get_active_client(void);
 int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 #else
@@ -76,6 +77,7 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	int id, bool active) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
+static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; }
 static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
 
 
-- 
1.7.9.5

[RFC PATCH 3/7] vga_switcheroo: Add notifier call chain for switcheroo events

[Seth Forshee]

DRM needs to be notified of client and handler registration in order to
defer initialization of the secondary GPU until the EDID can be read
from the LVDS panel. To support this add a notifier call chain to
vga_switcheroo for subscribing to switcheroo events. Events are
generated for registration and unregistration of handlers and clients.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/vga/vga_switcheroo.c |   34 ++++++++++++++++++++++++++++++++++
 include/linux/vga_switcheroo.h   |   14 ++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index e53f67d..d5cd274 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -24,6 +24,7 @@
 #include <linux/fs.h>
 #include <linux/debugfs.h>
 #include <linux/fb.h>
+#include <linux/notifier.h>
 
 #include <linux/pci.h>
 #include <linux/vga_switcheroo.h>
@@ -70,6 +71,28 @@ static struct vgasr_priv vgasr_priv = {
 	.clients = LIST_HEAD_INIT(vgasr_priv.clients),
 };
 
+static BLOCKING_NOTIFIER_HEAD(vga_switcheroo_notifier_list);
+
+int vga_switcheroo_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&vga_switcheroo_notifier_list,
+						nb);
+}
+EXPORT_SYMBOL(vga_switcheroo_register_notifier);
+
+int vga_switcheroo_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&vga_switcheroo_notifier_list,
+						  nb);
+}
+EXPORT_SYMBOL(vga_switcheroo_unregister_notifier);
+
+static int vga_switcheroo_notifier_call_chain(enum vga_switcheroo_event event)
+{
+	return blocking_notifier_call_chain(&vga_switcheroo_notifier_list,
+					    event, NULL);
+}
+
 static bool vga_switcheroo_ready(void)
 {
 	/* we're ready if we get two clients + handler */
@@ -113,10 +136,18 @@ int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
 		vga_switcheroo_enable();
 	}
 	mutex_unlock(&vgasr_mutex);
+
+	vga_switcheroo_notifier_call_chain(VGA_SWITCHEROO_HANDLER_REGISTERED);
 	return 0;
 }
 EXPORT_SYMBOL(vga_switcheroo_register_handler);
 
+bool vga_switcheroo_handler_registered(void)
+{
+	return !!vgasr_priv.handler;
+}
+EXPORT_SYMBOL(vga_switcheroo_handler_registered);
+
 void vga_switcheroo_unregister_handler(void)
 {
 	mutex_lock(&vgasr_mutex);
@@ -127,6 +158,7 @@ void vga_switcheroo_unregister_handler(void)
 		vgasr_priv.active = false;
 	}
 	mutex_unlock(&vgasr_mutex);
+	vga_switcheroo_notifier_call_chain(VGA_SWITCHEROO_HANDLER_UNREGISTERED);
 }
 EXPORT_SYMBOL(vga_switcheroo_unregister_handler);
 
@@ -156,6 +188,7 @@ static int register_client(struct pci_dev *pdev,
 		vga_switcheroo_enable();
 	}
 	mutex_unlock(&vgasr_mutex);
+	vga_switcheroo_notifier_call_chain(VGA_SWITCHEROO_CLIENT_REGISTERED);
 	return 0;
 }
 
@@ -250,6 +283,7 @@ void vga_switcheroo_unregister_client(struct pci_dev *pdev)
 		vgasr_priv.active = false;
 	}
 	mutex_unlock(&vgasr_mutex);
+	vga_switcheroo_notifier_call_chain(VGA_SWITCHEROO_CLIENT_UNREGISTERED);
 }
 EXPORT_SYMBOL(vga_switcheroo_unregister_client);
 
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index e361858..c3d7c6f 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -11,6 +11,7 @@
 #define _LINUX_VGA_SWITCHEROO_H_
 
 #include <linux/fb.h>
+#include <linux/notifier.h>
 
 struct pci_dev;
 
@@ -28,6 +29,13 @@ enum vga_switcheroo_client_id {
 	VGA_SWITCHEROO_MAX_CLIENTS,
 };
 
+enum vga_switcheroo_event {
+	VGA_SWITCHEROO_CLIENT_REGISTERED,
+	VGA_SWITCHEROO_CLIENT_UNREGISTERED,
+	VGA_SWITCHEROO_HANDLER_REGISTERED,
+	VGA_SWITCHEROO_HANDLER_UNREGISTERED,
+};
+
 struct vga_switcheroo_handler {
 	int (*switch_ddc)(enum vga_switcheroo_client_id id);
 	int (*switchto)(enum vga_switcheroo_client_id id);
@@ -44,6 +52,9 @@ struct vga_switcheroo_client_ops {
 };
 
 #if defined(CONFIG_VGA_SWITCHEROO)
+int vga_switcheroo_register_notifier(struct notifier_block *nb);
+int vga_switcheroo_unregister_notifier(struct notifier_block *nb);
+bool vga_switcheroo_handler_registered(void);
 void vga_switcheroo_unregister_client(struct pci_dev *dev);
 int vga_switcheroo_register_client(struct pci_dev *dev,
 				   const struct vga_switcheroo_client_ops *ops);
@@ -66,6 +77,9 @@ int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 #else
 
+static inline int vga_switcheroo_register_notifier(struct notifier_block *nb) { return 0; }
+static inline int vga_switcheroo_unregister_notifier(struct notifier_block *nb) { return 0; }
+static inline bool vga_switcheroo_handler_registered(void) { return false; }
 static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops) { return 0; }
-- 
1.7.9.5

[RFC PATCH 4/7] apple-gmux: Add switch_ddc support

[Seth Forshee]

The gmux allows muxing the DDC independently from the display, so
support this functionality. This will allow reading the EDID for the
inactive GPU, fixing issues with machines that either don't have a VBT
or have invalid mode data in the VBT.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/apple-gmux.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index c72e81e..f702e90 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -269,14 +269,21 @@ static const struct backlight_ops gmux_bl_ops = {
 	.update_status = gmux_update_status,
 };
 
+static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
+{
+	if (id == VGA_SWITCHEROO_IGD)
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
+	else
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
+	return 0;
+}
+
 static int gmux_switchto(enum vga_switcheroo_client_id id)
 {
 	if (id == VGA_SWITCHEROO_IGD) {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 2);
 	} else {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 3);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3);
 	}
@@ -343,6 +350,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data)
 }
 
 static struct vga_switcheroo_handler gmux_handler = {
+	.switch_ddc = gmux_switch_ddc,
 	.switchto = gmux_switchto,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,
-- 
1.7.9.5

[RFC PATCH 5/7] drm/edid: Switch DDC when reading the EDID

[Seth Forshee]

Some dual graphics machines support muxing the DDC separately from the
display, so make use of this functionality when reading the EDID on the
inactive GPU. Also serialize drm_get_edid() with a mutex to avoid races
on the DDC mux state.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/drm_edid.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..1a4b661 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/vga_switcheroo.h>
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -82,6 +83,8 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
+static DEFINE_MUTEX(drm_edid_mutex);
+
 static struct edid_quirk {
 	char vendor[4];
 	int product_id;
@@ -395,12 +398,26 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 			  struct i2c_adapter *adapter)
 {
 	struct edid *edid = NULL;
+	struct pci_dev *pdev = connector->dev->pdev;
+	struct pci_dev *active_pdev = NULL;
+
+	mutex_lock(&drm_edid_mutex);
+
+	if (pdev) {
+		active_pdev = vga_switcheroo_get_active_client();
+		if (active_pdev != pdev)
+			vga_switcheroo_switch_ddc(pdev);
+	}
 
 	if (drm_probe_ddc(adapter))
 		edid = (struct edid *)drm_do_get_edid(connector, adapter);
 
+	if (active_pdev && active_pdev != pdev)
+		vga_switcheroo_switch_ddc(active_pdev);
+
 	connector->display_info.raw_edid = (char *)edid;
 
+	mutex_unlock(&drm_edid_mutex);
 	return edid;
 
 }
-- 
1.7.9.5

[RFC PATCH 6/7] drm/pci: Add drm_put_pci_dev()

[Seth Forshee]

When deferred initialization support for pci devices is added some
additional cleanup will be needed. Add a pci-specific put function to
serve this purpose, and convert the pci drivers over to using it. For
now it just calls drm_put_dev(), so this commit has no functional
change.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/ast/ast_drv.c         |    2 +-
 drivers/gpu/drm/cirrus/cirrus_drv.c   |    2 +-
 drivers/gpu/drm/drm_pci.c             |    6 ++++++
 drivers/gpu/drm/gma500/psb_drv.c      |    2 +-
 drivers/gpu/drm/i915/i915_drv.c       |    2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c |    2 +-
 drivers/gpu/drm/nouveau/nouveau_drv.c |    2 +-
 drivers/gpu/drm/radeon/radeon_drv.c   |    2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c   |    2 +-
 include/drm/drmP.h                    |    1 +
 10 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index d0c4574..001298d 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -72,7 +72,7 @@ ast_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c b/drivers/gpu/drm/cirrus/cirrus_drv.c
index 7053140..c7ca02b 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -64,7 +64,7 @@ static void cirrus_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static const struct file_operations cirrus_driver_fops = {
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 5320364..4896c96 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -388,6 +388,12 @@ err_g1:
 }
 EXPORT_SYMBOL(drm_get_pci_dev);
 
+void drm_put_pci_dev(struct drm_device *dev)
+{
+	drm_put_dev(dev);
+}
+EXPORT_SYMBOL(drm_put_pci_dev);
+
 /**
  * PCI device initialization. Called direct from modules at load time.
  *
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 0c47374..d7c3c9c 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -585,7 +585,7 @@ static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv)
 static void psb_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static const struct dev_pm_ops psb_pm_ops = {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a24ffbe..86ae5a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -856,7 +856,7 @@ i915_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static int i915_pm_suspend(struct device *dev)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index ea1024d..a3b0a4a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -73,7 +73,7 @@ static void mga_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static const struct file_operations mgag200_driver_fops = {
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.c b/drivers/gpu/drm/nouveau/nouveau_drv.c
index 9a36f5f..b74b02a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.c
@@ -168,7 +168,7 @@ nouveau_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 int
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index d7269f4..298697a 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -308,7 +308,7 @@ radeon_pci_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static int
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 4d9edea..cf901cc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -982,7 +982,7 @@ static void vmw_remove(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
 
-	drm_put_dev(dev);
+	drm_put_pci_dev(dev);
 }
 
 static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d6b67bb..eb99e96 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1748,6 +1748,7 @@ extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
 extern int drm_get_pci_dev(struct pci_dev *pdev,
 			   const struct pci_device_id *ent,
 			   struct drm_driver *driver);
+extern void drm_put_pci_dev(struct drm_device *dev);
 
 #define DRM_PCIE_SPEED_25 1
 #define DRM_PCIE_SPEED_50 2
-- 
1.7.9.5

[RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Seth Forshee]

Deferring initiailzation of the secondary GPU until switcheroo is ready
will allow successfully reading the EDID in systems which support muxing
the DDC seperately from the display.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/drm_drv.c |    3 +
 drivers/gpu/drm/drm_pci.c |  141 +++++++++++++++++++++++++++++++++++++++------
 include/drm/drmP.h        |    2 +
 3 files changed, 129 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9238de4..124fd8a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -276,6 +276,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_pci_module_init();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
@@ -291,6 +293,7 @@ err_p1:
 
 static void __exit drm_core_exit(void)
 {
+	drm_pci_module_exit();
 	remove_proc_entry("dri", NULL);
 	debugfs_remove(drm_debugfs_root);
 	drm_sysfs_destroy();
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 4896c96..9da0cd2 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -40,6 +40,9 @@
 #include <linux/slab.h>
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
+#include <linux/notifier.h>
+#include <linux/vgaarb.h>
+#include <linux/vga_switcheroo.h>
 #include "drmP.h"
 
 /**********************************************************************/
@@ -297,19 +300,8 @@ static struct drm_bus drm_pci_bus = {
 	.agp_init = drm_pci_agp_init,
 };
 
-/**
- * Register.
- *
- * \param pdev - PCI device structure
- * \param ent entry from the PCI ID table with device type flags
- * \return zero on success or a negative number on failure.
- *
- * Attempt to gets inter module "drm" information. If we are first
- * then register the character device and inter module information.
- * Try and register, if we fail to register, backout previous work.
- */
-int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
-		    struct drm_driver *driver)
+int __drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
+		      struct drm_driver *driver)
 {
 	struct drm_device *dev;
 	int ret;
@@ -334,8 +326,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	dev->hose = pdev->sysdata;
 #endif
 
-	mutex_lock(&drm_global_mutex);
-
 	if ((ret = drm_fill_in_dev(dev, ent, driver))) {
 		printk(KERN_ERR "DRM: Fill_in_dev failed.\n");
 		goto err_g2;
@@ -371,7 +361,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 		 driver->name, driver->major, driver->minor, driver->patchlevel,
 		 driver->date, pci_name(pdev), dev->primary->index);
 
-	mutex_unlock(&drm_global_mutex);
 	return 0;
 
 err_g4:
@@ -386,10 +375,116 @@ err_g1:
 	mutex_unlock(&drm_global_mutex);
 	return ret;
 }
+
+struct deferred_init_data {
+	struct list_head list;
+	struct pci_dev *pdev;
+	const struct pci_device_id *ent;
+	struct drm_driver *driver;
+};
+
+static LIST_HEAD(deferred_init_list);
+
+static void drm_deferred_init_work_fn(struct work_struct *work)
+{
+	struct deferred_init_data *di_data, *temp;
+
+	mutex_lock(&drm_global_mutex);
+
+	if (!vga_switcheroo_handler_registered() ||
+	    !vga_switcheroo_get_active_client()) {
+		mutex_unlock(&drm_global_mutex);
+		return;
+	}
+
+	list_for_each_entry_safe(di_data, temp, &deferred_init_list, list) {
+		if (__drm_get_pci_dev(di_data->pdev, di_data->ent,
+				      di_data->driver))
+			DRM_ERROR("pci device initialization failed\n");
+		list_del(&di_data->list);
+		kfree(di_data);
+	}
+	mutex_unlock(&drm_global_mutex);
+}
+static DECLARE_WORK(deferred_init_work, drm_deferred_init_work_fn);
+
+static int drm_switcheroo_notifier_fn(struct notifier_block *nb,
+				      unsigned long val, void *unused)
+{
+	if (val == VGA_SWITCHEROO_CLIENT_REGISTERED ||
+	    val == VGA_SWITCHEROO_HANDLER_REGISTERED)
+		queue_work(system_nrt_wq, &deferred_init_work);
+	return NOTIFY_OK;
+}
+static struct notifier_block drm_switcheroo_notifier = {
+	.notifier_call = drm_switcheroo_notifier_fn,
+};
+
+/**
+ * Register.
+ *
+ * \param pdev - PCI device structure
+ * \param ent entry from the PCI ID table with device type flags
+ * \return zero on success or a negative number on failure.
+ *
+ * Attempt to gets inter module "drm" information. If we are first
+ * then register the character device and inter module information.
+ * Try and register, if we fail to register, backout previous work.
+ */
+int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
+		    struct drm_driver *driver)
+{
+	int ret = 0;
+
+	mutex_lock(&drm_global_mutex);
+
+	/*
+	 * For secondary graphics devices shouldn't be initialized
+	 * until the handler and primary graphics device have been
+	 * registered with vga_switcheroo.
+	 *
+	 * FIXME: Is vga_default_device() reliable enough for this
+	 * purpose?
+	 *
+	 * FIXME: If vga_switcheroo is disabled secondary devices
+	 * never gets initialized. Is this okay? Maybe it is, since
+	 * we can't switch to the secondary GPU anyway.
+	 */
+	if (vga_default_device() == pdev ||
+	    (vga_switcheroo_handler_registered() &&
+	     vga_switcheroo_get_active_client())) {
+		ret = __drm_get_pci_dev(pdev, ent, driver);
+	} else {
+		struct deferred_init_data *di_data =
+			kmalloc(sizeof(*di_data), GFP_KERNEL);
+		if (!di_data) {
+			ret = -ENOMEM;
+		} else {
+			di_data->pdev = pdev;
+			di_data->ent = ent;
+			di_data->driver = driver;
+			list_add_tail(&di_data->list, &deferred_init_list);
+		}
+	}
+
+	return ret;
+}
 EXPORT_SYMBOL(drm_get_pci_dev);
 
 void drm_put_pci_dev(struct drm_device *dev)
 {
+	struct deferred_init_data *di_data;
+
+	mutex_lock(&drm_global_mutex);
+	list_for_each_entry(di_data, &deferred_init_list, list) {
+		if (di_data->pdev == dev->pdev) {
+			list_del(&di_data->list);
+			kfree(di_data);
+			break;
+		}
+	}
+	mutex_unlock(&drm_global_mutex);
+
 	drm_put_dev(dev);
 }
 EXPORT_SYMBOL(drm_put_pci_dev);
@@ -466,7 +561,7 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver)
 		pci_unregister_driver(pdriver);
 	} else {
 		list_for_each_entry_safe(dev, tmp, &driver->device_list, driver_item)
-			drm_put_dev(dev);
+			drm_put_pci_dev(dev);
 	}
 	DRM_INFO("Module unloaded\n");
 }
@@ -520,3 +615,15 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask)
 	return 0;
 }
 EXPORT_SYMBOL(drm_pcie_get_speed_cap_mask);
+
+int drm_pci_module_init(void)
+{
+	return vga_switcheroo_register_notifier(&drm_switcheroo_notifier);
+}
+EXPORT_SYMBOL(drm_pci_module_init);
+
+void drm_pci_module_exit(void)
+{
+	vga_switcheroo_unregister_notifier(&drm_switcheroo_notifier);
+}
+EXPORT_SYMBOL(drm_pci_module_exit);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index eb99e96..0e9401f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1749,6 +1749,8 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
 			   const struct pci_device_id *ent,
 			   struct drm_driver *driver);
 extern void drm_put_pci_dev(struct drm_device *dev);
+extern int drm_pci_module_init(void);
+extern void drm_pci_module_exit(void);
 
 #define DRM_PCIE_SPEED_25 1
 #define DRM_PCIE_SPEED_50 2
-- 
1.7.9.5

Re: [RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Matthew Garrett]

On Mon, Aug 20, 2012 at 10:31:04AM -0500, Seth Forshee wrote:
> +	/*
> +	 * For secondary graphics devices shouldn't be initialized
> +	 * until the handler and primary graphics device have been
> +	 * registered with vga_switcheroo.
> +	 *
> +	 * FIXME: Is vga_default_device() reliable enough for this
> +	 * purpose?
> +	 *
> +	 * FIXME: If vga_switcheroo is disabled secondary devices
> +	 * never gets initialized. Is this okay? Maybe it is, since
> +	 * we can't switch to the secondary GPU anyway.
> +	 */

Won't this break the multiple cards with independent outputs case?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Seth Forshee]

On Mon, Aug 20, 2012 at 04:36:40PM +0100, Matthew Garrett wrote:
> On Mon, Aug 20, 2012 at 10:31:04AM -0500, Seth Forshee wrote:
> > +	/*
> > +	 * For secondary graphics devices shouldn't be initialized
> > +	 * until the handler and primary graphics device have been
> > +	 * registered with vga_switcheroo.
> > +	 *
> > +	 * FIXME: Is vga_default_device() reliable enough for this
> > +	 * purpose?
> > +	 *
> > +	 * FIXME: If vga_switcheroo is disabled secondary devices
> > +	 * never gets initialized. Is this okay? Maybe it is, since
> > +	 * we can't switch to the secondary GPU anyway.
> > +	 */
> 
> Won't this break the multiple cards with independent outputs case?

Yes, if they don't have a switcheroo handler. I only have experience
with one such machine, which had optimus graphics. My recollection is
that it did have a switcheroo handler, which was only capable of
controlling power to the discrete card.

Re: [RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Matthew Garrett]

On Mon, Aug 20, 2012 at 10:56:33AM -0500, Seth Forshee wrote:
> On Mon, Aug 20, 2012 at 04:36:40PM +0100, Matthew Garrett wrote:
> > Won't this break the multiple cards with independent outputs case?
> 
> Yes, if they don't have a switcheroo handler. I only have experience
> with one such machine, which had optimus graphics. My recollection is
> that it did have a switcheroo handler, which was only capable of
> controlling power to the discrete card.

So if I have a desktop machine and install two graphics cards?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Seth Forshee]

On Mon, Aug 20, 2012 at 04:57:41PM +0100, Matthew Garrett wrote:
> On Mon, Aug 20, 2012 at 10:56:33AM -0500, Seth Forshee wrote:
> > On Mon, Aug 20, 2012 at 04:36:40PM +0100, Matthew Garrett wrote:
> > > Won't this break the multiple cards with independent outputs case?
> > 
> > Yes, if they don't have a switcheroo handler. I only have experience
> > with one such machine, which had optimus graphics. My recollection is
> > that it did have a switcheroo handler, which was only capable of
> > controlling power to the discrete card.
> 
> So if I have a desktop machine and install two graphics cards?

Yeah, that would likely be broken.

I'm not sure how we support both of these cases without doing something
more like what I originally proposed, i.e. registering the LVDS
connector even if it doesn't look like a panel is attached. I still
honestly favor that approach, although it does come with its own set of
challenges.

The only other option I can come up with is to reprobe LVDS after
switcheroo and add the connector at that time. I haven't investigated
this option in detail, but at first glance it looks like there are at
least some places where DRM isn't prepared to cope with adding
connectors after initialization.

Re: [RFC PATCH 7/7] drm/pci: Defer initialization of secondary graphics devices until switcheroo is ready

[Matthew Garrett]

On Mon, Aug 20, 2012 at 11:24:44AM -0500, Seth Forshee wrote:

> I'm not sure how we support both of these cases without doing something
> more like what I originally proposed, i.e. registering the LVDS
> connector even if it doesn't look like a panel is attached. I still
> honestly favor that approach, although it does come with its own set of
> challenges.
> 
> The only other option I can come up with is to reprobe LVDS after
> switcheroo and add the connector at that time. I haven't investigated
> this option in detail, but at first glance it looks like there are at
> least some places where DRM isn't prepared to cope with adding
> connectors after initialization.

Well, one option is to identify whether the hardware is switcheroo 
capable without the use of the switcheroo driver. It looks like we can 
do that in the majority of cases - Apple is the only special case I can 
see, and that one's a fairly easy workaround.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Mon, Aug 06, 2012 at 07:44:16AM +1000, Dave Airlie wrote:
> >> The "correct" approach is clearly to just have the drm core change the
> >> i2c mux before requesting edid, but that's made difficult because of the
> >> absence of ordering guarantees in initialisation. I don't like quirking
> >> this, since we're then back to the situation of potentially having to
> >> add every new piece of related hardware to the quirk list.
> >
> > The "correct" approach of switching the mux before we fetch the edid is
> > actualy the one I fear will result in fragile code: Only run on few
> > machines, and as you say with tons of funky interactions with the init
> > sequence ordering. And I guess people will bitch&moan about the flickering
> > this will cause ;-)
> >
> > As long as it's only apple shipping multi-gpu machines with
> > broken/non-existing vbt, I'll happily stomach the quirk list entries.
> > They're bad, but imo the lesser evil.
> 
> Well in theory you can switch the ddc lines without switching the other lines,
> so we could do a mutex protected mux switch around edid retrival,
> 
> Of course someone would have to code it up first then we could see how
> ugly it would be.

I coded it up, and it's not really too bad. I've put a dump of my local
changes below. But there are a couple of problems.

First, I don't have a solution for the ordering of initialization. It
just happens to work out for me right now.

Even so, the code only works if I delay loading i915. apple-gmux is
definitely initializing first so the i2c mux should be getting switched,
but the transactions are failing.

[   19.445658] [drm:gmbus_xfer], GMBUS [i915 gmbus panel] timed out after NAK 
[   19.445672] [drm:gmbus_xfer], GMBUS [i915 gmbus panel] NAK for addr: 0050 r(1) 

But if I prevent i915 from being auto-loaded and load later on it works
fine. I haven't been able to figure out what's going on. Any ideas?

Thanks,
Seth


diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..3f18e8a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/vga_switcheroo.h>
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -82,6 +83,8 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
+static DEFINE_MUTEX(drm_edid_mutex);
+
 static struct edid_quirk {
 	char vendor[4];
 	int product_id;
@@ -395,12 +398,25 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 			  struct i2c_adapter *adapter)
 {
 	struct edid *edid = NULL;
+	struct pci_dev *pdev = connector->dev->pdev;
+	struct pci_dev *active_pdev = NULL;
+
+	mutex_lock(&drm_edid_mutex);
+
+	if (pdev) {
+		active_pdev = vga_switcheroo_get_active_client();
+		vga_switcheroo_switch_ddc(pdev);
+	}
 
 	if (drm_probe_ddc(adapter))
 		edid = (struct edid *)drm_do_get_edid(connector, adapter);
 
+	if (active_pdev)
+		vga_switcheroo_switch_ddc(active_pdev);
+
 	connector->display_info.raw_edid = (char *)edid;
 
+	mutex_unlock(&drm_edid_mutex);
 	return edid;
 
 }
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index e25cf31..e53f67d 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -205,6 +205,20 @@ find_active_client(struct list_head *head)
 	return NULL;
 }
 
+struct pci_dev *vga_switcheroo_get_active_client(void)
+{
+	struct vga_switcheroo_client *client;
+	struct pci_dev *pdev = NULL;
+
+	mutex_lock(&vgasr_mutex);
+	client = find_active_client(&vgasr_priv.clients);
+	if (client)
+		pdev = client->pdev;
+	mutex_unlock(&vgasr_mutex);
+	return pdev;
+}
+EXPORT_SYMBOL(vga_switcheroo_get_active_client);
+
 int vga_switcheroo_get_client_state(struct pci_dev *pdev)
 {
 	struct vga_switcheroo_client *client;
@@ -252,6 +266,29 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
 }
 EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int id;
+
+	mutex_lock(&vgasr_mutex);
+
+	if (!vgasr_priv.handler) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (vgasr_priv.handler->switch_ddc) {
+		id = vgasr_priv.handler->get_client_id(pdev);
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+
+out:
+	mutex_unlock(&vgasr_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(vga_switcheroo_switch_ddc);
+
 static int vga_switcheroo_show(struct seq_file *m, void *v)
 {
 	struct vga_switcheroo_client *client;
@@ -342,9 +379,15 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
 	}
 
+	if (vgasr_priv.handler->switch_ddc) {
+		ret = vgasr_priv.handler->switch_ddc(new_client->id);
+		if (ret)
+			return ret;
+	}
+
 	ret = vgasr_priv.handler->switchto(new_client->id);
 	if (ret)
-		return ret;
+		goto restore_ddc;
 
 	if (new_client->ops->reprobe)
 		new_client->ops->reprobe(new_client->pdev);
@@ -356,6 +399,14 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 
 	new_client->active = true;
 	return 0;
+
+restore_ddc:
+	if (vgasr_priv.handler->switch_ddc) {
+		int id = (new_client->id == VGA_SWITCHEROO_IGD) ?
+				VGA_SWITCHEROO_DIS : VGA_SWITCHEROO_IGD;
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+	return ret;
 }
 
 static bool check_can_switch(void)
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 8ea2640..38f49fb 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -122,14 +122,21 @@ static const struct backlight_ops gmux_bl_ops = {
 	.update_status = gmux_update_status,
 };
 
+static int gmux_switchddc(enum vga_switcheroo_client_id id)
+{
+	if (id == VGA_SWITCHEROO_IGD)
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
+	else
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
+	return 0;
+}
+
 static int gmux_switchto(enum vga_switcheroo_client_id id)
 {
 	if (id == VGA_SWITCHEROO_IGD) {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 2);
 	} else {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 3);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3);
 	}
@@ -196,6 +203,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data)
 }
 
 static struct vga_switcheroo_handler gmux_handler = {
+	.switch_ddc = gmux_switchddc,
 	.switchto = gmux_switchto,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index ddb419c..e361858 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -29,6 +29,7 @@ enum vga_switcheroo_client_id {
 };
 
 struct vga_switcheroo_handler {
+	int (*switch_ddc)(enum vga_switcheroo_client_id id);
 	int (*switchto)(enum vga_switcheroo_client_id id);
 	int (*power_state)(enum vga_switcheroo_client_id id,
 			   enum vga_switcheroo_state state);
@@ -53,11 +54,14 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev);
+
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler);
 void vga_switcheroo_unregister_handler(void);
 
 int vga_switcheroo_process_delayed_switch(void);
 
+struct pci_dev *vga_switcheroo_get_active_client(void);
 int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 #else
@@ -66,12 +70,14 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
+static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; }
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,
 	int id, bool active) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
+static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; }
 static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
 
 

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Matthew Garrett]

On Sun, Aug 05, 2012 at 11:40:16PM +0200, Daniel Vetter wrote:

> As long as it's only apple shipping multi-gpu machines with
> broken/non-existing vbt, I'll happily stomach the quirk list entries.
> They're bad, but imo the lesser evil.

Doing this via quirks means that we'll always be broken on the hardware 
at the point where it ships. Implementing the functionality means we 
stand some chance of working out of the box.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFC PATCH 3/5] drm/i915: register LVDS connector even if we can't get a panel mode

[Seth Forshee]

On Mon, Aug 06, 2012 at 01:23:17PM +0100, Matthew Garrett wrote:
> On Sun, Aug 05, 2012 at 11:40:16PM +0200, Daniel Vetter wrote:
> 
> > As long as it's only apple shipping multi-gpu machines with
> > broken/non-existing vbt, I'll happily stomach the quirk list entries.
> > They're bad, but imo the lesser evil.
> 
> Doing this via quirks means that we'll always be broken on the hardware 
> at the point where it ships. Implementing the functionality means we 
> stand some chance of working out of the box.

I've been thinking some today about how this functionality might be
implemented.

It looks like the simplest way to let the inactive GPU have the i2c bus
temporarily is to have drm_get_edid() control the mux and serialize it
with a mutex. It should be pretty easy to make vga_switcheroo support
muxing the DDC separately from the display.

There is the problem of vga_switcheroo not really being operational
until it has two clients and a handler, and the graphics drivers not
registering clients until they've initialized LVDS. This probably won't
be too dificult to solve.

The bigger problem is still making sure the switcheroo handler is
registered, when it's needed, before trying to read the EDID. We could
potentially delay initializing non-active graphics devices until after a
switcheroo handler has been registered, but that's a problem if the
handler is implemented by the driver for the secondary GPU (is this ever
the case?). Otherwise we seem to be stuck with making i915 able to cope
with getting modes for LVDS after initialization, which kind of puts us
back where we started.

Any other ideas?

[RFC PATCH 4/5] drm/i915: make intel_lvds_get_edid() more robust

[Seth Forshee]

intel_lvds_get_edid() needs to be called when switching GPUs, but it's
currently making assumptions that it will only be called once and that
there's always an LVDS connector present when it's called. Fix these
assumptions.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/intel_lvds.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index c1ab632..9d05a90 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -906,9 +906,18 @@ static bool intel_lvds_get_edid(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector = dev_priv->int_lvds_connector;
-	struct intel_lvds *intel_lvds = intel_attached_lvds(connector);
+	struct intel_lvds *intel_lvds;
 	struct drm_display_mode *scan; /* *modes, *bios_mode; */
 
+	if (!connector)
+		return false;
+
+	intel_lvds = intel_attached_lvds(connector);
+
+	/* If we already have an EDID, no need to check again */
+	if (intel_lvds->edid)
+		return true;
+
 	/*
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
@@ -939,6 +948,12 @@ static bool intel_lvds_get_edid(struct drm_device *dev)
 
 	list_for_each_entry(scan, &connector->probed_modes, head) {
 		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
+			/*
+			 * If we already have a preferred mode from another
+			 * source, prefer the one from the EDID.
+			 */
+			if (intel_lvds->fixed_mode)
+				drm_mode_destroy(dev, intel_lvds->fixed_mode);
 			intel_lvds->fixed_mode =
 				drm_mode_duplicate(dev, scan);
 			intel_find_lvds_downclock(dev,
-- 
1.7.9.5

[RFC PATCH 5/5] drm/i915: check LVDS for EDID on GPU switches

[Seth Forshee]

If the LVDS panel wasn't connected at boot then we won't have an EDID
for it. To fix this, call intel_lvds_get_edid() from the vga_switcheroo
reprobe callback.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/gpu/drm/i915/i915_dma.c   |    1 +
 drivers/gpu/drm/i915/intel_drv.h  |    1 +
 drivers/gpu/drm/i915/intel_lvds.c |    2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5b5176d..c9c942e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1266,6 +1266,7 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_
 static void i915_switcheroo_reprobe(struct pci_dev *pdev)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
+	intel_lvds_get_edid(dev);
 	intel_fb_output_poll_changed(dev);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8435355..bcc14f9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -356,6 +356,7 @@ extern void intel_dvo_init(struct drm_device *dev);
 extern void intel_tv_init(struct drm_device *dev);
 extern void intel_mark_busy(struct drm_device *dev,
 			    struct drm_i915_gem_object *obj);
+extern bool intel_lvds_get_edid(struct drm_device *dev);
 extern bool intel_lvds_init(struct drm_device *dev);
 extern void intel_dp_init(struct drm_device *dev, int dp_reg);
 void
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 9d05a90..39dbefc 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -902,7 +902,7 @@ static bool intel_lvds_supported(struct drm_device *dev)
 	return IS_MOBILE(dev) && !IS_I830(dev);
 }
 
-static bool intel_lvds_get_edid(struct drm_device *dev)
+bool intel_lvds_get_edid(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector = dev_priv->int_lvds_connector;
-- 
1.7.9.5