Re: [PATCH v2.1 2/3] apple-gmux: Add switch_ddc support

From: Daniel Vetter <daniel@ffwll.ch>
To: Lukas Wunner <lukas@wunner.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Dave Airlie <airlied@redhat.com>,
	dri-devel@lists.freedesktop.org,
	Seth Forshee <seth.forshee@canonical.com>
Subject: Re: [PATCH v2.1 2/3] apple-gmux: Add switch_ddc support
Date: Tue, 25 Aug 2015 10:13:58 +0200	[thread overview]
Message-ID: <20150825081358.GP20434@phenom.ffwll.local> (raw)
In-Reply-To: <3f927fb1a1df18bfdbb501b98a6b2dcd5c064f02.1439739853.git.lukas@wunner.de>

On Fri, Aug 14, 2015 at 06:18:55PM +0200, Lukas Wunner 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-03-27:
> If the inactive client registers before the active client then
> old_ddc_owner cannot be determined with find_active_client()
> (null pointer dereference). Therefore change semantics of the
> ->switch_ddc handler callback to return old_ddc_owner.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
> Tested-by: Pierre Moreau <pierre.morrow@free.fr>
>     [MBP  5,3 2009  nvidia 9400M + 9600M GT   pre-retina]
> Tested-by: Paul Hordiienko <pvt.gord@gmail.com>
>     [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina]
> Tested-by: William Brown <william@blackhats.net.au>
>     [MBP  8,2 2011  intel SNB + amd turks     pre-retina]
> Tested-by: Lukas Wunner <lukas@wunner.de>
>     [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina]
> Tested-by: Bruno Bierbaumer <bruno@bierbaumer.net>
>     [MBP 11,3 2013  intel HSW + nvidia GK107  retina -- work in progress]
> 
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/platform/x86/apple-gmux.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 0dec3f5..08bdf1e 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -273,14 +273,34 @@ static const struct backlight_ops gmux_bl_ops = {
>  	.update_status = gmux_update_status,
>  };
>  
> +static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
> +{
> +	enum vga_switcheroo_client_id old_ddc_owner;
> +
> +	if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1)
> +		old_ddc_owner = VGA_SWITCHEROO_IGD;
> +	else
> +		old_ddc_owner = VGA_SWITCHEROO_DIS;
> +
> +	pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id);
> +
> +	if (id == old_ddc_owner)
> +		return old_ddc_owner;
> +
> +	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 old_ddc_owner;
> +}
> +
>  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);
>  	}
> @@ -347,6 +367,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data)
>  }

If we'd required that switchto also switches ddc then the error handling
in patch 1 would be a lot simpler. switch_ddc would then only be for
temporary switching while probing.
-Daniel

>  
>  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,
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel