Re: [PATCH v2.1 3/3] drm/edid: Switch DDC when reading the EDID

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>, dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Seth Forshee <seth.forshee@canonical.com>,
	Dave Airlie <airlied@redhat.com>
Subject: Re: [PATCH v2.1 3/3] drm/edid: Switch DDC when reading the EDID
Date: Tue, 18 Aug 2015 10:02:43 +0300	[thread overview]
Message-ID: <87r3n1m63w.fsf@intel.com> (raw)
In-Reply-To: <3d6de562c9dd7387318f90f2dc1578e56cb666c1.1439739853.git.lukas@wunner.de>

On Fri, 14 Aug 2015, Lukas Wunner <lukas@wunner.de> wrote:
> Originally by Seth Forshee <seth.forshee@canonical.com>, 2012-10-04:
> 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.

At a glance, all the referenced bugs have LVDS displays. Is this
supposed to work for eDP as well? How about muxing and locking of DPCD
access?

BR,
Jani.

>
> Modified by Dave Airlie <airlied@gmail.com>, 2012-12-22:
> I can't figure out why I didn't like this, but I rewrote this [...] to
> lock/unlock the ddc lines [...]. I think I'd prefer something like that
> otherwise the interface got really ugly.
>
> Modified by Lukas Wunner <lukas@wunner.de>, 2015-03-27:
> Unlock DDC lines if drm_probe_ddc() fails.
>
> 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>
> Cc: Dave Airlie <airlied@gmail.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/gpu/drm/drm_edid.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e6e05bb..cdb2fa1 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -32,6 +32,7 @@
>  #include <linux/hdmi.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
> +#include <linux/vga_switcheroo.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_displayid.h>
> @@ -1377,13 +1378,21 @@ struct edid *drm_get_edid(struct drm_connector *connector,
>  			  struct i2c_adapter *adapter)
>  {
>  	struct edid *edid;
> +	struct pci_dev *pdev = connector->dev->pdev;
>  
> -	if (!drm_probe_ddc(adapter))
> +	vga_switcheroo_lock_ddc(pdev);
> +
> +	if (!drm_probe_ddc(adapter)) {
> +		vga_switcheroo_unlock_ddc(pdev);
>  		return NULL;
> +	}
>  
>  	edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
>  	if (edid)
>  		drm_get_displayid(connector, edid);
> +
> +	vga_switcheroo_unlock_ddc(pdev);
> +
>  	return edid;
>  }
>  EXPORT_SYMBOL(drm_get_edid);
> -- 
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel