[PATCH] [RFC] drm/i915/dp: add support for load detect Apple DP->VGA dongles
Jani Nikula
jani.nikula at linux.intel.com
Fri Jun 6 02:57:46 PDT 2014
On Fri, 06 Jun 2014, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> The DP1.2 spec has some bits for forced load sensing on VGA dongles,
> however the apple dongle I have doesn't do DP 1.2 yet, so I dug into
> its vendor specific area and found a register that seems to correspond
> to load detected on the outputs.
>
> The main reason I need this was at LCA this year the slide capture system
> failed to provide EDID, but I realised OSX worked. Really need to add support
> to nouveau, but hey i915 is a start.
>
> The dongle also appears to use short IRQs to denote a plug/unplug event,
> though something seems to be failing in reading back the dpcd values from it.
> (also this is based on my MST work just because.)
Good timing for making use of the OUI! See
http://mid.gmane.org/1401920981-3137-1-git-send-email-clinton.a.taylor@intel.com
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 29 +++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> include/drm/drm_dp_helper.h | 19 +++++++++++++++++++
> 3 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index c500f63..49afd7d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3173,6 +3173,16 @@ intel_dp_probe_oui(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n",
> buf[0], buf[1], buf[2]);
>
> + intel_dp->is_apple_vga = false;
> + if (drm_dp_branch_is_apple(buf)) {
> + u8 mybuf[8];
> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI + 3, mybuf, 8)) {
That may return negative values for errors.
FWIW I've thought about reading the sink/branch device identification
strings unconditionally along with the OUI just for debug purposes. I
wouldn't be opposed to such a change here.
> + if (drm_dp_apple_has_load_detect(mybuf)) {
> + DRM_DEBUG_KMS("detected Apple DP VGA dongle\n");
> + intel_dp->is_apple_vga = true;
> + }
> + }
> + }
I think I'd like this abstracted to a dedicated function and doing:
intel_dp->is_apple_vga = is_apple_vga(intel_dp, buf);
or something.
> edp_panel_vdd_off(intel_dp, false);
> }
>
> @@ -3404,6 +3414,21 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> if (drm_probe_ddc(&intel_dp->aux.ddc))
> return connector_status_connected;
>
> + if (intel_dp->is_apple_vga) {
> + u8 detect_buf;
> + int ret;
> + ret = drm_dp_dpcd_readb(&intel_dp->aux, DP_APPLE_LOAD_DETECT,
> + &detect_buf);
> +
> + if (ret == 1) {
> + DRM_DEBUG_KMS("Apple VGA detect is 0x%x\n", detect_buf);
> + /* I suspect 4 == load, 8 == edid */
> + if (detect_buf)
> + return connector_status_connected;
> + else
> + return connector_status_disconnected;
> + }
> + }
> /* Well we tried, say unknown for unreliable port types */
> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
> type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> @@ -3854,6 +3879,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> * but for short hpds we should check it now
> */
> intel_dp_check_link_status(intel_dp);
> +
> + /* if we get a short hpd on apple vga - its a hotplug */
> + if (intel_dp->is_apple_vga)
> + return true;
> }
> }
> return false;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1edb38a..23f767c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -549,6 +549,7 @@ struct intel_dp {
> bool use_tps3;
> bool can_mst; /* this port supports mst */
> bool is_mst;
> + bool is_apple_vga;
> int active_mst_links;
> /* connector directly attached - won't be use for modeset in mst world */
> struct intel_connector *attached_connector;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 517d6c1..c8ebd27 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -612,4 +612,23 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
> int drm_dp_aux_register(struct drm_dp_aux *aux);
> void drm_dp_aux_unregister(struct drm_dp_aux *aux);
>
> +#define DP_APPLE_OUI 0x10fa
> +
> +#define DP_APPLE_LOAD_DETECT (DP_BRANCH_OUI + 12)
> +
> +static inline bool drm_dp_branch_is_apple(const u8 buf[3])
Naming buf and its size similar to other drm_dp_* helpers use would be
nice.
> +{
> + if (buf[0] == ((DP_APPLE_OUI >> 16) & 0xff) &&
> + buf[1] == ((DP_APPLE_OUI >> 8) & 0xff) &&
> + buf[2] == ((DP_APPLE_OUI & 0xff)))
> + return true;
> + return false;
> +}
> +
> +static inline bool drm_dp_apple_has_load_detect(const u8 buf[8])
Ditto.
> +{
> + if (!memcmp((const char *)buf, "mVGAa", 5))
Why the cast?
BR,
Jani.
> + return true;
> + return false;
> +}
> #endif /* _DRM_DP_HELPER_H_ */
> --
> 1.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
More information about the dri-devel
mailing list