| Message ID | 1444043595-26502-2-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 05, 2015 at 04:43:15PM +0530, Sonika Jindal wrote: > This patch adds a separate probe function for HDMI > EDID read over DDC channel. This function has been > registered as a .hot_plug handler for HDMI encoder. > > The current implementation of hdmi_detect() > function re-sets the cached HDMI edid (in connector->detect_edid) in > every detect call.This function gets called many times, sometimes > directly from userspace probes, forcing drivers to read EDID every > detect function call.This causes several problems like: > > 1. Race conditions in multiple hot_plug / unplug cases, between > interrupts bottom halves and userspace detections. > 2. Many Un-necessary EDID reads for single hotplug/unplug > 3. HDMI complaince failures which expects only one EDID read per hotplug > > This function will be serving the purpose of really reading the EDID > by really probing the DDC channel, and updating the cached EDID. > > The plan is to: > 1. i915 IRQ handler bottom half function already calls > intel_encoder->hotplug() function. Adding This probe function which > will read the EDID only in case of a hotplug / unplug. > 2. During init_connector this probe will be called to read the edid > 3. Reuse the cached EDID in hdmi_detect() function. > > The "< gen7" check is there because this was tested only for >=gen7 > platforms. For older platforms the hotplug/reading edid path remains same. > > v2: Calling set_edid instead of hdmi_probe during init. > Also, for platforms having DDI, intel_encoder for DP and HDMI is same > (taken from intel_dig_port), so for DP also, hot_plug function gets called > which is not intended here. So, check for HDMI in intel_hdmi_probe > Rely on HPD for updating edid only for platforms gen > 8 and also for VLV. > > v3: Dropping the gen < 8 || !VLV check. Now all platforms should rely on > hotplug or init for updating the edid.(Daniel) > Also, calling hdmi_probe in init instead of set_edid > > v4: Renaming intel_hdmi_probe to intel_hdmi_hot_plug. > Also calling this hotplug handler from intel_hpd_init to take care of init > resume scenarios. > > v5: Moved the call to encoder hotplug during init to separate patch(Daniel) > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> I merged the first patch in this series, but I have a quick question on this one here. Can you pls ping me on irc? Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_hdmi.c | 54 +++++++++++++++++++++++++++++-------- > 1 file changed, 43 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index bb33c66..9c1a308 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -1369,18 +1369,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force) > return connected; > } > > -static enum drm_connector_status > -intel_hdmi_detect(struct drm_connector *connector, bool force) > +void intel_hdmi_hot_plug(struct intel_encoder *intel_encoder) > { > - enum drm_connector_status status; > - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > - struct drm_i915_private *dev_priv = to_i915(connector->dev); > + struct intel_hdmi *intel_hdmi = > + enc_to_intel_hdmi(&intel_encoder->base); > + struct intel_connector *intel_connector = > + intel_hdmi->attached_connector; > + struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev); > bool live_status = false; > unsigned int retry = 3; > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > - connector->base.id, connector->name); > - > while (!live_status && --retry) { > live_status = intel_digital_port_connected(dev_priv, > hdmi_to_dig_port(intel_hdmi)); > @@ -1390,15 +1388,48 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) > if (!live_status) > DRM_DEBUG_KMS("Live status not up!"); > > - intel_hdmi_unset_edid(connector); > + /* > + * We are here, means there is a hotplug or a force > + * detection. Clear the cached EDID and probe the > + * DDC bus to check the current status of HDMI. > + */ > + intel_hdmi_unset_edid(&intel_connector->base); > + if (intel_hdmi_set_edid(&intel_connector->base, live_status)) > + DRM_DEBUG_DRIVER("DDC probe: got EDID\n"); > + else > + DRM_DEBUG_DRIVER("DDC probe: no EDID\n"); > +} > > - if (intel_hdmi_set_edid(connector, live_status)) { > +static enum drm_connector_status > +intel_hdmi_detect(struct drm_connector *connector, bool force) > +{ > + enum drm_connector_status status; > + struct intel_connector *intel_connector = > + to_intel_connector(connector); > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > + connector->base.id, connector->name); > + > + /* > + * There are many userspace calls which probe EDID from > + * detect path. In case of multiple hotplug/unplug, these > + * can cause race conditions while probing EDID. Also its > + * waste of CPU cycles to read the EDID again and again > + * unless there is a real hotplug. > + * So, rely on hotplugs and init to read edid. > + * Check connector status based on availability of cached EDID. > + */ > + > + if (intel_connector->detect_edid) { > struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); > > hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI; > status = connector_status_connected; > - } else > + DRM_DEBUG_DRIVER("hdmi status = connected\n"); > + } else { > status = connector_status_disconnected; > + DRM_DEBUG_DRIVER("hdmi status = disconnected\n"); > + } > > return status; > } > @@ -2131,6 +2162,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > intel_connector->unregister = intel_connector_unregister; > > intel_hdmi_add_properties(intel_hdmi, connector); > + intel_encoder->hot_plug = intel_hdmi_hot_plug; > > intel_connector_attach_encoder(intel_connector, intel_encoder); > drm_connector_register(connector); > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bb33c66..9c1a308 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -1369,18 +1369,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force) return connected; } -static enum drm_connector_status -intel_hdmi_detect(struct drm_connector *connector, bool force) +void intel_hdmi_hot_plug(struct intel_encoder *intel_encoder) { - enum drm_connector_status status; - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); - struct drm_i915_private *dev_priv = to_i915(connector->dev); + struct intel_hdmi *intel_hdmi = + enc_to_intel_hdmi(&intel_encoder->base); + struct intel_connector *intel_connector = + intel_hdmi->attached_connector; + struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev); bool live_status = false; unsigned int retry = 3; - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.id, connector->name); - while (!live_status && --retry) { live_status = intel_digital_port_connected(dev_priv, hdmi_to_dig_port(intel_hdmi)); @@ -1390,15 +1388,48 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) if (!live_status) DRM_DEBUG_KMS("Live status not up!"); - intel_hdmi_unset_edid(connector); + /* + * We are here, means there is a hotplug or a force + * detection. Clear the cached EDID and probe the + * DDC bus to check the current status of HDMI. + */ + intel_hdmi_unset_edid(&intel_connector->base); + if (intel_hdmi_set_edid(&intel_connector->base, live_status)) + DRM_DEBUG_DRIVER("DDC probe: got EDID\n"); + else + DRM_DEBUG_DRIVER("DDC probe: no EDID\n"); +} - if (intel_hdmi_set_edid(connector, live_status)) { +static enum drm_connector_status +intel_hdmi_detect(struct drm_connector *connector, bool force) +{ + enum drm_connector_status status; + struct intel_connector *intel_connector = + to_intel_connector(connector); + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); + + /* + * There are many userspace calls which probe EDID from + * detect path. In case of multiple hotplug/unplug, these + * can cause race conditions while probing EDID. Also its + * waste of CPU cycles to read the EDID again and again + * unless there is a real hotplug. + * So, rely on hotplugs and init to read edid. + * Check connector status based on availability of cached EDID. + */ + + if (intel_connector->detect_edid) { struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI; status = connector_status_connected; - } else + DRM_DEBUG_DRIVER("hdmi status = connected\n"); + } else { status = connector_status_disconnected; + DRM_DEBUG_DRIVER("hdmi status = disconnected\n"); + } return status; } @@ -2131,6 +2162,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, intel_connector->unregister = intel_connector_unregister; intel_hdmi_add_properties(intel_hdmi, connector); + intel_encoder->hot_plug = intel_hdmi_hot_plug; intel_connector_attach_encoder(intel_connector, intel_encoder); drm_connector_register(connector);