| Message ID | 1443430564-12547-1-git-send-email-sonika.jindal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 28, 2015 at 02:26:04PM +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 and changing the patch > subject. Also calling this hotplug handler from intel_hpd_init to take care > of init resume scenarios. > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > --- > drivers/gpu/drm/i915/intel_hdmi.c | 54 +++++++++++++++++++++++++++------- > drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++ > 2 files changed, 54 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); > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c > index 53c0173..36e16f6 100644 > --- a/drivers/gpu/drm/i915/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/intel_hotplug.c > @@ -459,6 +459,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) > struct drm_device *dev = dev_priv->dev; > struct drm_mode_config *mode_config = &dev->mode_config; > struct drm_connector *connector; > + struct intel_encoder *encoder; > int i; > > for_each_hpd_pin(i) { > @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) > if (dev_priv->display.hpd_irq_setup) > dev_priv->display.hpd_irq_setup(dev); > spin_unlock_irq(&dev_priv->irq_lock); > + > + /* > + * Connected boot / resume scenarios can't generate new hot plug. > + * So, probe it manually. > + */ > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + if (encoder->hot_plug) > + encoder->hot_plug(encoder); > + } Since this doesn't just change hdmi but every encoder I think we should pull it out into a separate prep patch. Especially interactions with dp mst handling look tricky, have you tested that nothing blows up with that? The split-out patch should definitely explain in detail how this interacts with dp hpd handling. -Daniel
On 9/28/2015 7:04 PM, Daniel Vetter wrote: > On Mon, Sep 28, 2015 at 02:26:04PM +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 and changing the patch >> subject. Also calling this hotplug handler from intel_hpd_init to take care >> of init resume scenarios. >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> >> --- >> drivers/gpu/drm/i915/intel_hdmi.c | 54 +++++++++++++++++++++++++++------- >> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++ >> 2 files changed, 54 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); >> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c >> index 53c0173..36e16f6 100644 >> --- a/drivers/gpu/drm/i915/intel_hotplug.c >> +++ b/drivers/gpu/drm/i915/intel_hotplug.c >> @@ -459,6 +459,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) >> struct drm_device *dev = dev_priv->dev; >> struct drm_mode_config *mode_config = &dev->mode_config; >> struct drm_connector *connector; >> + struct intel_encoder *encoder; >> int i; >> >> for_each_hpd_pin(i) { >> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) >> if (dev_priv->display.hpd_irq_setup) >> dev_priv->display.hpd_irq_setup(dev); >> spin_unlock_irq(&dev_priv->irq_lock); >> + >> + /* >> + * Connected boot / resume scenarios can't generate new hot plug. >> + * So, probe it manually. >> + */ >> + list_for_each_entry(encoder, &dev->mode_config.encoder_list, >> + base.head) { >> + if (encoder->hot_plug) >> + encoder->hot_plug(encoder); >> + } > > Since this doesn't just change hdmi but every encoder I think we should > pull it out into a separate prep patch. Especially interactions with dp > mst handling look tricky, have you tested that nothing blows up with that? > The split-out patch should definitely explain in detail how this interacts > with dp hpd handling. > -Daniel > Right now we don't set hot_plug for DP/edp encoder. It was used only for sdvo and with this patch we add for hdmi. Also, in sdvo_init right after assigning the hot_plug function pointer, we call its hot_plug function for initialization. Maybe with this patch, we can remove that part also from sdvo_init. I can split this patch but without this change, it will result in regression with just one patch. So I thought it would be better to keep them together in one patch. Regards, Sonika
On Tue, Sep 29, 2015 at 09:43:10AM +0530, Jindal, Sonika wrote: > > > On 9/28/2015 7:04 PM, Daniel Vetter wrote: > >On Mon, Sep 28, 2015 at 02:26:04PM +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 and changing the patch > >>subject. Also calling this hotplug handler from intel_hpd_init to take care > >>of init resume scenarios. > >> > >>Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >>Signed-off-by: Sonika Jindal <sonika.jindal@intel.com> > >>--- > >> drivers/gpu/drm/i915/intel_hdmi.c | 54 +++++++++++++++++++++++++++------- > >> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++ > >> 2 files changed, 54 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); > >>diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c > >>index 53c0173..36e16f6 100644 > >>--- a/drivers/gpu/drm/i915/intel_hotplug.c > >>+++ b/drivers/gpu/drm/i915/intel_hotplug.c > >>@@ -459,6 +459,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) > >> struct drm_device *dev = dev_priv->dev; > >> struct drm_mode_config *mode_config = &dev->mode_config; > >> struct drm_connector *connector; > >>+ struct intel_encoder *encoder; > >> int i; > >> > >> for_each_hpd_pin(i) { > >>@@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) > >> if (dev_priv->display.hpd_irq_setup) > >> dev_priv->display.hpd_irq_setup(dev); > >> spin_unlock_irq(&dev_priv->irq_lock); > >>+ > >>+ /* > >>+ * Connected boot / resume scenarios can't generate new hot plug. > >>+ * So, probe it manually. > >>+ */ > >>+ list_for_each_entry(encoder, &dev->mode_config.encoder_list, > >>+ base.head) { > >>+ if (encoder->hot_plug) > >>+ encoder->hot_plug(encoder); > >>+ } > > > >Since this doesn't just change hdmi but every encoder I think we should > >pull it out into a separate prep patch. Especially interactions with dp > >mst handling look tricky, have you tested that nothing blows up with that? > >The split-out patch should definitely explain in detail how this interacts > >with dp hpd handling. > >-Daniel > > > Right now we don't set hot_plug for DP/edp encoder. > It was used only for sdvo and with this patch we add for hdmi. > Also, in sdvo_init right after assigning the hot_plug function pointer, we > call its hot_plug function for initialization. > Maybe with this patch, we can remove that part also from sdvo_init. Hm right, I forgot that we have too many hotplug callbacks in our driver ;-) > I can split this patch but without this change, it will result in regression > with just one patch. So I thought it would be better to keep them together > in one patch. If you split it out and make it patch 1 and then the hdmi changes patch 2 nothing breaks in between. -Daniel
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); diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 53c0173..36e16f6 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -459,6 +459,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) struct drm_device *dev = dev_priv->dev; struct drm_mode_config *mode_config = &dev->mode_config; struct drm_connector *connector; + struct intel_encoder *encoder; int i; for_each_hpd_pin(i) { @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); spin_unlock_irq(&dev_priv->irq_lock); + + /* + * Connected boot / resume scenarios can't generate new hot plug. + * So, probe it manually. + */ + list_for_each_entry(encoder, &dev->mode_config.encoder_list, + base.head) { + if (encoder->hot_plug) + encoder->hot_plug(encoder); + } } void intel_hpd_init_work(struct drm_i915_private *dev_priv)