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 at 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
