2014-01-07 09:58:40

by Ramkumar Ramachandra

[permalink] [raw]
Subject: [PATCH 0/4] Get EDID late for VGA switcheroo

Hi,

VGA switcheroo doesn't work on my 2013 MBP, and I'm trying to fix
it. From what I've gathered from previous patches, it seems that the
EDID is not computed at boottime, because LVDS isn't connected to the
i915 card (and is connected to the nouveau card instead). So, here's a
series to get switcheroo-reprobe to call get-edid. I think it's a step
in the right direction, although I think more stuff is required to fix
the issue.

Disclaimer: The series might be utter rubbish, because this is my
first attempt at any driver code.

Thanks.

Cc: Andreas Heider <[email protected]>
Cc: Seth Forshee <[email protected]>

Ramkumar Ramachandra (4):
drm/i915: add support for vga_switcheroo reprobe
drm/i915: factor out intel_lvds_get_edid()
drm/i915: prepare intel_lvds_get_edid() for multiple calls
drm/i915: check LVDS for EDID on GPU switches

drivers/gpu/drm/i915/i915_dma.c | 9 ++-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_lvds.c | 112 +++++++++++++++++++++++---------------
4 files changed, 78 insertions(+), 45 deletions(-)

--
1.8.5.2.229.g4448466


2014-01-07 09:58:43

by Ramkumar Ramachandra

[permalink] [raw]
Subject: [PATCH 1/4] drm/i915: add support for vga_switcheroo reprobe

Cc: Andreas Heider <[email protected]>
Cc: Seth Forshee <[email protected]>
Original-patch-by: Andreas Heider <[email protected]>
Signed-off-by: Ramkumar Ramachandra <[email protected]>
---
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 5c64842..336a835b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1265,6 +1265,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_fbdev_output_poll_changed(dev);
+}
+
static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
{
struct drm_device *dev = pci_get_drvdata(pdev);
@@ -1278,7 +1284,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.8.5.2.229.g4448466

2014-01-07 09:58:49

by Ramkumar Ramachandra

[permalink] [raw]
Subject: [PATCH 3/4] drm/i915: prepare intel_lvds_get_edid() for multiple calls

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

Cc: Andreas Heider <[email protected]>
Cc: Seth Forshee <[email protected]>
Original-patch-by: Seth Forshee <[email protected]>
Signed-off-by: Ramkumar Ramachandra <[email protected]>
---
drivers/gpu/drm/i915/intel_lvds.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 6c09617..8275551 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -927,10 +927,18 @@ static bool intel_lvds_get_edid(struct drm_device *dev)
struct edid *edid;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_connector *connector = dev_priv->int_lvds_connector;
- struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
+ struct intel_lvds_connector *lvds_connector;
struct drm_display_mode *scan; /* *modes, *bios_mode; */
struct drm_display_mode *fixed_mode = NULL;

+ if (!connector)
+ return false;
+ lvds_connector = to_lvds_connector(connector);
+
+ /* If we already have an EDID, no need to check again */
+ if (lvds_connector->base.edid)
+ return true;
+
/*
* Attempt to get the fixed panel mode from DDC. Assume that the
* preferred mode is the right one.
--
1.8.5.2.229.g4448466

2014-01-07 09:59:13

by Ramkumar Ramachandra

[permalink] [raw]
Subject: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

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.

Cc: Andreas Heider <[email protected]>
Cc: Seth Forshee <[email protected]>
Original-patch-by: Seth Forshee <[email protected]>
Signed-off-by: Ramkumar Ramachandra <[email protected]>
---
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 336a835b..5eb37a3 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1268,6 +1268,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_fbdev_output_poll_changed(dev);
}

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 79f91f2..686954f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -769,6 +769,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,


/* intel_lvds.c */
+bool intel_lvds_get_edid(struct drm_device *dev);
void intel_lvds_init(struct drm_device *dev);
bool intel_is_dual_link_lvds(struct drm_device *dev);

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 8275551..f700355 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -922,7 +922,7 @@ static bool intel_lvds_supported(struct drm_device *dev)
return false;
}

-static bool intel_lvds_get_edid(struct drm_device *dev)
+bool intel_lvds_get_edid(struct drm_device *dev)
{
struct edid *edid;
struct drm_i915_private *dev_priv = dev->dev_private;
--
1.8.5.2.229.g4448466

2014-01-07 09:59:51

by Ramkumar Ramachandra

[permalink] [raw]
Subject: [PATCH 2/4] drm/i915: factor out intel_lvds_get_edid()

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().

Cc: Andreas Heider <[email protected]>
Cc: Seth Forshee <[email protected]>
Original-patch-by: Seth Forshee <[email protected]>
Signed-off-by: Ramkumar Ramachandra <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_lvds.c | 104 ++++++++++++++++++++++----------------
2 files changed, 61 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 90fcccb..2c055f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1496,6 +1496,7 @@ typedef struct drm_i915_private {
} wm;

struct i915_package_c8 pc8;
+ struct drm_connector *int_lvds_connector;

/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index c3b4da7..6c09617 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -42,6 +42,7 @@
/* Private structure for the integrated LVDS support */
struct intel_lvds_connector {
struct intel_connector base;
+ u8 i2c_pin;

struct notifier_block lid_notifier;
};
@@ -921,6 +922,60 @@ static bool intel_lvds_supported(struct drm_device *dev)
return false;
}

+static bool intel_lvds_get_edid(struct drm_device *dev)
+{
+ struct edid *edid;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_connector *connector = dev_priv->int_lvds_connector;
+ struct intel_lvds_connector *lvds_connector = to_lvds_connector(connector);
+ struct drm_display_mode *scan; /* *modes, *bios_mode; */
+ struct drm_display_mode *fixed_mode = NULL;
+
+ /*
+ * Attempt to get the fixed panel mode from DDC. Assume that the
+ * preferred mode is the right one.
+ */
+ edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, lvds_connector->i2c_pin));
+ if (edid) {
+ if (drm_add_edid_modes(connector, edid)) {
+ drm_mode_connector_update_edid_property(connector,
+ edid);
+ } else {
+ kfree(edid);
+ edid = ERR_PTR(-EINVAL);
+ }
+ } else {
+ edid = ERR_PTR(-ENOENT);
+ }
+ lvds_connector->base.edid = edid;
+
+ if (IS_ERR_OR_NULL(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) {
+ DRM_DEBUG_KMS("using preferred mode from EDID: ");
+ drm_mode_debug_printmodeline(scan);
+
+ fixed_mode = drm_mode_duplicate(dev, scan);
+ if (fixed_mode) {
+ intel_find_lvds_downclock(dev, fixed_mode,
+ connector);
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
/**
* intel_lvds_init - setup LVDS connectors on this device
* @dev: drm device
@@ -937,9 +992,7 @@ void 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_display_mode *fixed_mode = NULL;
- struct edid *edid;
struct drm_crtc *crtc;
u32 lvds;
int pipe;
@@ -978,11 +1031,13 @@ void intel_lvds_init(struct drm_device *dev)
}

lvds_encoder->attached_connector = lvds_connector;
+ lvds_connector->i2c_pin = pin;

intel_encoder = &lvds_encoder->base;
encoder = &intel_encoder->base;
intel_connector = &lvds_connector->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);

@@ -1036,48 +1091,8 @@ void 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.
- */
- edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, pin));
- if (edid) {
- if (drm_add_edid_modes(connector, edid)) {
- drm_mode_connector_update_edid_property(connector,
- edid);
- } else {
- kfree(edid);
- edid = ERR_PTR(-EINVAL);
- }
- } else {
- edid = ERR_PTR(-ENOENT);
- }
- lvds_connector->base.edid = edid;
-
- if (IS_ERR_OR_NULL(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) {
- DRM_DEBUG_KMS("using preferred mode from EDID: ");
- drm_mode_debug_printmodeline(scan);
-
- fixed_mode = drm_mode_duplicate(dev, scan);
- if (fixed_mode) {
- intel_find_lvds_downclock(dev, fixed_mode,
- connector);
- goto out;
- }
- }
- }
+ if (intel_lvds_get_edid(dev))
+ goto out;

/* Failed to get EDID, what about VBT? */
if (dev_priv->vbt.lfp_lvds_vbt_mode) {
@@ -1149,6 +1164,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);
if (fixed_mode)
--
1.8.5.2.229.g4448466

2014-01-07 10:43:40

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 1/4] drm/i915: add support for vga_switcheroo reprobe

On Tue, Jan 07, 2014 at 03:28:40PM +0530, Ramkumar Ramachandra wrote:
> Cc: Andreas Heider <[email protected]>
> Cc: Seth Forshee <[email protected]>
> Original-patch-by: Andreas Heider <[email protected]>
> Signed-off-by: Ramkumar Ramachandra <[email protected]>
> ---
> 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 5c64842..336a835b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1265,6 +1265,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_fbdev_output_poll_changed(dev);
> +}

switcheroo should be sending a hotplug notification, which should
trigger both this kernel listener and userspace.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre

2014-01-07 10:46:04

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

On Tue, Jan 07, 2014 at 03:28:43PM +0530, Ramkumar Ramachandra wrote:
> 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.

I would rather have an iterator over all our connectors (or perhaps
encoders would be the right semantic, except we have
connectors==encoders) and a reprobe callback. Saves exporting private
functions and prevent me asking silly questions like what about eDP?
-Chris

--
Chris Wilson, Intel Open Source Technology Centre

2014-01-07 10:54:26

by Ramkumar Ramachandra

[permalink] [raw]
Subject: Re: [PATCH 1/4] drm/i915: add support for vga_switcheroo reprobe

Hi Chris,

Chris Wilson wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 5c64842..336a835b 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1265,6 +1265,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_fbdev_output_poll_changed(dev);
>> +}
>
> switcheroo should be sending a hotplug notification, which should
> trigger both this kernel listener and userspace.

If I understand correctly, switcheroo will send a hotplug notification
when LVDS is connected to the i915 device, and trigger this reprobe
function(), which in a later patch calls get-edid. Right?

2014-01-07 11:12:54

by Ramkumar Ramachandra

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

Chris Wilson wrote:
> On Tue, Jan 07, 2014 at 03:28:43PM +0530, Ramkumar Ramachandra wrote:
>> 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.
>
> I would rather have an iterator over all our connectors (or perhaps
> encoders would be the right semantic, except we have
> connectors==encoders) and a reprobe callback.

I don't follow; iterate over which connectors? There's one
lvds_connector for which we get EDID.

> Saves exporting private
> functions and prevent me asking silly questions like what about eDP?

Avoid exporting intel_lvds_get_edid()? Why?

p.s- Excuse the stupidity of the questions; I know close to nothing
about how i915 or switcheroo work.

2014-01-07 11:49:09

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

On Tue, Jan 07, 2014 at 04:42:12PM +0530, Ramkumar Ramachandra wrote:
> Chris Wilson wrote:
> > On Tue, Jan 07, 2014 at 03:28:43PM +0530, Ramkumar Ramachandra wrote:
> >> 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.
> >
> > I would rather have an iterator over all our connectors (or perhaps
> > encoders would be the right semantic, except we have
> > connectors==encoders) and a reprobe callback.
>
> I don't follow; iterate over which connectors? There's one
> lvds_connector for which we get EDID.

Rather than special casing lvds (especially when there are other panel
connectors that can also be muxed), extend the connector interface to
support a reprobe and walk over all connectors associated with i915
after a switcheroo event.

> > Saves exporting private
> > functions and prevent me asking silly questions like what about eDP?
>
> Avoid exporting intel_lvds_get_edid()? Why?

Because it's a special case and misses others.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre

2014-01-07 13:06:12

by Ramkumar Ramachandra

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

Chris Wilson wrote:
> Rather than special casing lvds (especially when there are other panel
> connectors that can also be muxed), extend the connector interface to
> support a reprobe and walk over all connectors associated with i915
> after a switcheroo event.

Okay, so I can see the following get_edid functions:

intel_lvds_get_edid()
intel_crt_get_edid()
intel_sdvo_get_edid()
intel_dp_get_edid()

In the hdmi case, drm_get_edid() is called in two places:
intel_hdmi_detect() and intel_hdmi_detect_audio(); should I factor out
something into a corresponding intel_hdmi_get_edid() function?

Now, in i915_switcheroo_reprobe(), I need to call all these functions,
right? They all accept a a generic drm_connector and an specific
i2c_adapter. But how do I do that without exporting each of these
functions and their adapters?

Thanks.

2014-01-07 13:14:18

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 4/4] drm/i915: check LVDS for EDID on GPU switches

On Tue, Jan 07, 2014 at 06:35:26PM +0530, Ramkumar Ramachandra wrote:
> Chris Wilson wrote:
> > Rather than special casing lvds (especially when there are other panel
> > connectors that can also be muxed), extend the connector interface to
> > support a reprobe and walk over all connectors associated with i915
> > after a switcheroo event.
>
> Okay, so I can see the following get_edid functions:
>
> intel_lvds_get_edid()
> intel_crt_get_edid()
> intel_sdvo_get_edid()
> intel_dp_get_edid()
>
> In the hdmi case, drm_get_edid() is called in two places:
> intel_hdmi_detect() and intel_hdmi_detect_audio(); should I factor out
> something into a corresponding intel_hdmi_get_edid() function?
>
> Now, in i915_switcheroo_reprobe(), I need to call all these functions,
> right? They all accept a a generic drm_connector and an specific
> i2c_adapter. But how do I do that without exporting each of these
> functions and their adapters?

If you look at struct intel_connector, you will see space for adding a
callback that each of the LVDS/CRT/SDVO/DP connectors can hook into if
they want to reprobe after a switcheroo.

struct intel_connector *connector;
list_for_each_entry(connector,
&dev->mode_config.connector_list,
base.head) {
connector->reprobe(connector);
}

(or something similar) would do in the switcheroo callback.

-Chris

--
Chris Wilson, Intel Open Source Technology Centre

2014-01-07 13:54:21

by Seth Forshee

[permalink] [raw]
Subject: Re: [PATCH 0/4] Get EDID late for VGA switcheroo

On Tue, Jan 07, 2014 at 03:28:39PM +0530, Ramkumar Ramachandra wrote:
> Hi,
>
> VGA switcheroo doesn't work on my 2013 MBP, and I'm trying to fix
> it. From what I've gathered from previous patches, it seems that the
> EDID is not computed at boottime, because LVDS isn't connected to the
> i915 card (and is connected to the nouveau card instead). So, here's a
> series to get switcheroo-reprobe to call get-edid. I think it's a step
> in the right direction, although I think more stuff is required to fix
> the issue.

It's been a while since I've thought about this, so my memory is
suspect. It's also a bit dated, and I haven't kept up to know what's
changed in since then. But I don't think just reading the mode on
reprobe gets you very far.

These are the main problems as I remember them:

1. On many Macbook models the firmware isn't putting LVDS information
in opregion. Reading the EDID during device initialization also
fails since the DDC is muxed to the DGPU, so i915 concludes there's
no LVDS and doesn't register an LVDS connector at all. I also think
it was impossible to register a new connector at a later time,
though I'm not sure about that.

2. If i915 is forced to register the LVDS connector (say via a quirk)
it still may not know the panel mode and guesses something rather
conservative. This mode it assumes is used to allocate framebuffer
memory, which is likely to end up being too small, and I don't think
there was any support for resizing it later. This resulted in VTs
not working because there isn't enough framebuffer memory for the
mode they're trying to use.

3. So what is really wanted is to be able to mux just the DDC over to
the IGPU when the device is initialized to read the panel mode. One
possible problem is that the hw might not support muxing the DDC
separately, but Apple hw does so we'll ignore that for now. The
basic code for muxing only the DDC isn't too difficult, and I had
some patches for this at one point. The problem is that the handler
must register with vga_switcheroo before this is possible, so
there's a module initialization ordering issue that wasn't easily
solved. Maybe now it would be possible to do something with
EPROBE_DEFER, though I'm not sure how i915 would know whether or not
it should wait for a switcheroo handler.

I just didn't have enough time to spend on this issue to work through
all of this. I also never looked at doing this with eDP, which is what I
thought all the Macbooks have been using for the last couple of years.
Are you sure your machine uses LVDS and not eDP?

Seth

2014-01-07 14:31:08

by Ramkumar Ramachandra

[permalink] [raw]
Subject: Re: [PATCH 0/4] Get EDID late for VGA switcheroo

Seth Forshee wrote:
> 1. On many Macbook models the firmware isn't putting LVDS information
> in opregion. Reading the EDID during device initialization also
> fails since the DDC is muxed to the DGPU, so i915 concludes there's
> no LVDS and doesn't register an LVDS connector at all. I also think
> it was impossible to register a new connector at a later time,
> though I'm not sure about that.

On a different thread, Chris is helping me write a series to fill in
the switcheroo-reprobe callback with corresponding get-edid functions.
Do you think it'll be useful?

> 2. If i915 is forced to register the LVDS connector (say via a quirk)
> it still may not know the panel mode and guesses something rather
> conservative. This mode it assumes is used to allocate framebuffer
> memory, which is likely to end up being too small, and I don't think
> there was any support for resizing it later. This resulted in VTs
> not working because there isn't enough framebuffer memory for the
> mode they're trying to use.

I saw your quirk-patch (https://lkml.org/lkml/2012/8/4/128). Seems
like a rather inelegant approach to register the LVDS connector
unconditionally during initialization; I'll try to avoid going down
this road.

> 3. So what is really wanted is to be able to mux just the DDC over to
> the IGPU when the device is initialized to read the panel mode. One
> possible problem is that the hw might not support muxing the DDC
> separately, but Apple hw does so we'll ignore that for now. The
> basic code for muxing only the DDC isn't too difficult, and I had
> some patches for this at one point. The problem is that the handler
> must register with vga_switcheroo before this is possible, so
> there's a module initialization ordering issue that wasn't easily
> solved. Maybe now it would be possible to do something with
> EPROBE_DEFER, though I'm not sure how i915 would know whether or not
> it should wait for a switcheroo handler.

I didn't understand much of the above; I suppose it'll start making
sense when I'm deeper into the issue.

> I just didn't have enough time to spend on this issue to work through
> all of this. I also never looked at doing this with eDP, which is what I
> thought all the Macbooks have been using for the last couple of years.
> Are you sure your machine uses LVDS and not eDP?

I just assumed that it was LVDS; turns out it's eDP.

I have time to work on the issue now, so I'd appreciate any help I can get.

2014-01-07 14:48:33

by Andreas Heider

[permalink] [raw]
Subject: Re: [PATCH 0/4] Get EDID late for VGA switcheroo

Am 2014-01-07 15:30, schrieb Ramkumar Ramachandra:
> Seth Forshee wrote:

>> 3. So what is really wanted is to be able to mux just the DDC over to
>> the IGPU when the device is initialized to read the panel mode.
>> One
>> possible problem is that the hw might not support muxing the DDC
>> separately, but Apple hw does so we'll ignore that for now. The
>> basic code for muxing only the DDC isn't too difficult, and I had
>> some patches for this at one point. The problem is that the
>> handler
>> must register with vga_switcheroo before this is possible, so
>> there's a module initialization ordering issue that wasn't easily
>> solved. Maybe now it would be possible to do something with
>> EPROBE_DEFER, though I'm not sure how i915 would know whether or
>> not
>> it should wait for a switcheroo handler.
>
> I didn't understand much of the above; I suppose it'll start making
> sense when I'm deeper into the issue.
>

You can switch the line used to detect the display separately from the
line which is used to display the picture, so you can just switch DDC to
the intel card when you know it will try to detect the display, without
having to introduce flickering. There were a few different versions of
patches that did this, I think this one is the latest:
http://lists.freedesktop.org/archives/dri-devel/2012-September/027528.html

Sorry that I forgot about this yesterday, it has been quite some time
since I worked on this.

2014-01-07 14:59:04

by Ramkumar Ramachandra

[permalink] [raw]
Subject: Re: [PATCH 0/4] Get EDID late for VGA switcheroo

[email protected] wrote:
> You can switch the line used to detect the display separately from the line
> which is used to display the picture, so you can just switch DDC to the
> intel card when you know it will try to detect the display, without having
> to introduce flickering. There were a few different versions of patches that
> did this, I think this one is the latest:
> http://lists.freedesktop.org/archives/dri-devel/2012-September/027528.html

Any idea why this series didn't get merged?