Re: [PATCH v4 2/6] apple-gmux: Add switch_ddc support

From: Alex Deucher <alexdeucher@gmail.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Darren Hart <dvhart@infradead.org>
Subject: Re: [PATCH v4 2/6] apple-gmux: Add switch_ddc support
Date: Mon, 12 Oct 2015 17:10:20 -0400	[thread overview]
Message-ID: <CADnq5_P6OAkHCUzksdBeSzxZxrgD6XS+HeAvDXr=01oDDhM1TA@mail.gmail.com> (raw)
In-Reply-To: <CADnq5_MO89J-NVTuUMEcO-m0Xxd2WXLmZNuk-Si0jwDcoJJYNg@mail.gmail.com>

On Mon, Oct 12, 2015 at 5:07 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Aug 14, 2015 at 12:18 PM, Lukas Wunner <lukas@wunner.de> wrote:
>> Originally by Seth Forshee <seth.forshee@canonical.com>, 2012-10-04:
>>     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.
>>
>> Modified by Lukas Wunner <lukas@wunner.de>, 2015-10-07:
>>     Advertise ->switch_ddc handler callback only on the pre-retina
>>     Macbook Pro. The retina uses eDP and its mux is incapable of
>>     switching the AUX channel separately from the main link.
>>     It's an NXP CBTL06142 (alternate parts: TI HD3SS212,
>>     Pericom PPI3VDP12412).
>>
>>     Sources:
>>     http://www.electronicproducts.com/-whatsinside_text-145.aspx
>>     http://slideshare.net/jjwu6266/apple-2012-wwdc-apple-macbook-pro-with-retina-display
>>     http://www.techrepublic.com/blog/cracking-open/teardown-shows-retina-macbook-pro-is-nearly-impossible-to-upgrade-difficult-to-work-on/
>>
>>     Datasheets:
>>     http://www.nxp.com/documents/data_sheet/CBTL06141.pdf
>>     http://www.ti.com/lit/ds/symlink/hd3ss212.pdf
>>     https://www.pericom.com/assets/Datasheets/PI3VDP12412.pdf
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
>> Tested-by: Lukas Wunner <lukas@wunner.de>
>>     [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
>>
>> Cc: Seth Forshee <seth.forshee@canonical.com>
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> ---
>>  drivers/platform/x86/apple-gmux.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
>> index 0dec3f5..78997b7 100644
>> --- a/drivers/platform/x86/apple-gmux.c
>> +++ b/drivers/platform/x86/apple-gmux.c
>> @@ -276,11 +276,9 @@ static const struct backlight_ops gmux_bl_ops = {
>>  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);
>>         }
>
> Won't this hunk break manual switching until the later patches land?
> Seems like you might want to break this out as a separate patch later
> in the series.

Also. I'm not sure how the gmux works, but this might break ddc on
external displays that are muxed.

Alex

>
> Alex
>
>
>> @@ -288,6 +286,18 @@ static int gmux_switchto(enum vga_switcheroo_client_id id)
>>         return 0;
>>  }
>>
>> +static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
>> +{
>> +       pr_debug("Switching gmux DDC to %d\n", 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_set_discrete_state(struct apple_gmux_data *gmux_data,
>>                                    enum vga_switcheroo_state state)
>>  {
>> @@ -588,6 +598,13 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
>>                 gmux_data->gpe = -1;
>>         }
>>
>> +       /*
>> +        * The gmux in pre-retina MacBook Pros can switch DDC separately
>> +        * from the other pins of the outputs. It's distinguishable from
>> +        * the gmux in retinas by being non-indexed.
>> +        */
>> +       if (!gmux_data->indexed)
>> +               gmux_handler.switch_ddc = gmux_switch_ddc;
>>         if (vga_switcheroo_register_handler(&gmux_handler)) {
>>                 ret = -ENODEV;
>>                 goto err_register_handler;
>> --
>> 1.8.5.2 (Apple Git-48)
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel