On Tue, 2 Sep 2014 13:45:37 +0300
Ville Syrjälä <ville.syrj...@linux.intel.com> wrote:

> On Tue, Sep 02, 2014 at 09:24:48AM +0100, Chris Wilson wrote:
> > As we may query the edid multiple times following a detect, record
> > the EDID found during output discovery and reuse it. This is a
> > separate issue from caching the output EDID across detection cycles.
> > 
> > v2: Also hookup the force() callback for audio detection when the
> > user forces the connection status.
> > 
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 145
> > +++++++++++++++++++++----------------- 1 file changed, 80
> > insertions(+), 65 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c index
> > 9169786dbbc3..7428521d2e25 100644 ---
> > a/drivers/gpu/drm/i915/intel_hdmi.c +++
> > b/drivers/gpu/drm/i915/intel_hdmi.c @@ -962,104 +962,117 @@ bool
> > intel_hdmi_compute_config(struct intel_encoder *encoder, return
> > true; }
> >  
> > -static enum drm_connector_status
> > -intel_hdmi_detect(struct drm_connector *connector, bool force)
> > +static void
> > +intel_hdmi_unset_edid(struct drm_connector *connector)
> >  {
> > -   struct drm_device *dev = connector->dev;
> >     struct intel_hdmi *intel_hdmi =
> > intel_attached_hdmi(connector);
> > -   struct intel_digital_port *intel_dig_port =
> > -           hdmi_to_dig_port(intel_hdmi);
> > -   struct intel_encoder *intel_encoder =
> > &intel_dig_port->base;
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct edid *edid;
> > -   enum intel_display_power_domain power_domain;
> > -   enum drm_connector_status status =
> > connector_status_disconnected; 
> > -   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > -                 connector->base.id, connector->name);
> > +   intel_hdmi->has_hdmi_sink = false;
> > +   intel_hdmi->has_audio = false;
> > +   intel_hdmi->rgb_quant_range_selectable = false;
> > +
> > +   kfree(to_intel_connector(connector)->detect_edid);
> > +   to_intel_connector(connector)->detect_edid = NULL;
> > +}
> > +
> > +static bool
> > +intel_hdmi_set_edid(struct drm_connector *connector)
> > +{
> > +   struct drm_i915_private *dev_priv =
> > to_i915(connector->dev);
> > +   struct intel_hdmi *intel_hdmi =
> > intel_attached_hdmi(connector);
> > +   struct intel_encoder *intel_encoder =
> > +           &hdmi_to_dig_port(intel_hdmi)->base;
> > +   enum intel_display_power_domain power_domain;
> > +   struct edid *edid;
> > +   bool connected = false;
> >  
> >     power_domain =
> > intel_display_port_power_domain(intel_encoder);
> > intel_display_power_get(dev_priv, power_domain); 
> > -   intel_hdmi->has_hdmi_sink = false;
> > -   intel_hdmi->has_audio = false;
> > -   intel_hdmi->rgb_quant_range_selectable = false;
> >     edid = drm_get_edid(connector,
> >                         intel_gmbus_get_adapter(dev_priv,
> >                                                 intel_hdmi->ddc_bus));
> >  
> > -   if (edid) {
> > -           if (edid->input & DRM_EDID_INPUT_DIGITAL) {
> > -                   status = connector_status_connected;
> > -                   if (intel_hdmi->force_audio !=
> > HDMI_AUDIO_OFF_DVI)
> > -                           intel_hdmi->has_hdmi_sink =
> > -
> > drm_detect_hdmi_monitor(edid);
> > -                   intel_hdmi->has_audio =
> > drm_detect_monitor_audio(edid);
> > -                   intel_hdmi->rgb_quant_range_selectable =
> > -
> > drm_rgb_quant_range_selectable(edid);
> > -           }
> > -           kfree(edid);
> > -   }
> > +   intel_display_power_put(dev_priv, power_domain);
> > +
> > +   to_intel_connector(connector)->detect_edid = edid;
> > +   if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> > +           intel_hdmi->rgb_quant_range_selectable =
> > +                   drm_rgb_quant_range_selectable(edid);
> >  
> > -   if (status == connector_status_connected) {
> > +           intel_hdmi->has_audio =
> > drm_detect_monitor_audio(edid); if (intel_hdmi->force_audio !=
> > HDMI_AUDIO_AUTO) intel_hdmi->has_audio =
> > -                           (intel_hdmi->force_audio ==
> > HDMI_AUDIO_ON);
> > -           intel_encoder->type = INTEL_OUTPUT_HDMI;
> > +                           intel_hdmi->force_audio ==
> > HDMI_AUDIO_ON; +
> > +           if (intel_hdmi->force_audio != HDMI_AUDIO_OFF_DVI)
> > +                   intel_hdmi->has_hdmi_sink =
> > +                           drm_detect_hdmi_monitor(edid);
> 
> I wonder if we should be updating has_hdmi_sink also in
> intel_hdmi_set_property() when force_audio != HDMI_AUDIO_OFF_DVI.
> But that's a separate issue so material for another patch.
> 
> > +
> > +           connected = true;
> >     }
> >  
> > -   intel_display_power_put(dev_priv, power_domain);
> > +   return connected;
> > +}
> > +
> > +static enum drm_connector_status
> > +intel_hdmi_detect(struct drm_connector *connector, bool force)
> > +{
> > +   enum drm_connector_status status;
> > +
> > +   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > +                 connector->base.id, connector->name);
> > +
> > +   intel_hdmi_unset_edid(connector);
> > +
> > +   if (intel_hdmi_set_edid(connector)) {
> > +           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
> > +           status = connector_status_disconnected;
> >  
> >     return status;
> >  }
> >  
> > -static int intel_hdmi_get_modes(struct drm_connector *connector)
> > +static void
> > +intel_hdmi_force(struct drm_connector *connector)
> >  {
> > -   struct intel_encoder *intel_encoder =
> > intel_attached_encoder(connector);
> > -   struct intel_hdmi *intel_hdmi =
> > enc_to_intel_hdmi(&intel_encoder->base);
> > -   struct drm_i915_private *dev_priv =
> > connector->dev->dev_private;
> > -   enum intel_display_power_domain power_domain;
> > -   int ret;
> > +   struct intel_hdmi *intel_hdmi =
> > intel_attached_hdmi(connector); 
> > -   /* We should parse the EDID data and find out if it's an
> > HDMI sink so
> > -    * we can send audio to it.
> > -    */
> > +   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > +                 connector->base.id, connector->name);
> >  
> > -   power_domain =
> > intel_display_port_power_domain(intel_encoder);
> > -   intel_display_power_get(dev_priv, power_domain);
> > +   intel_hdmi_unset_edid(connector);
> >  
> > -   ret = intel_ddc_get_modes(connector,
> > -
> > intel_gmbus_get_adapter(dev_priv,
> > -
> > intel_hdmi->ddc_bus));
> > +   if (connector->status == connector_status_connected)
> 
> !=
> 
> Apart from that I didn't spot anything suspicious. So with this one
> thing fixed you can slap on
> Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> for the series.
> 
> > +           return;
> >  
> > -   intel_display_power_put(dev_priv, power_domain);
> > +   intel_hdmi_set_edid(connector);
> > +   hdmi_to_dig_port(intel_hdmi)->base.type =
> > INTEL_OUTPUT_HDMI; +}
> >  
> > -   return ret;
> > +static int intel_hdmi_get_modes(struct drm_connector *connector)
> > +{
> > +   struct edid *edid;
> > +
> > +   edid = to_intel_connector(connector)->detect_edid;
> > +   if (edid == NULL)
> > +           return 0;
> > +
> > +   return intel_connector_update_modes(connector, edid);
> >  }
> >  
> >  static bool
> >  intel_hdmi_detect_audio(struct drm_connector *connector)
> >  {
> > -   struct intel_encoder *intel_encoder =
> > intel_attached_encoder(connector);
> > -   struct intel_hdmi *intel_hdmi =
> > enc_to_intel_hdmi(&intel_encoder->base);
> > -   struct drm_i915_private *dev_priv =
> > connector->dev->dev_private;
> > -   enum intel_display_power_domain power_domain;
> > -   struct edid *edid;
> >     bool has_audio = false;
> > +   struct edid *edid;
> >  
> > -   power_domain =
> > intel_display_port_power_domain(intel_encoder);
> > -   intel_display_power_get(dev_priv, power_domain);
> > -
> > -   edid = drm_get_edid(connector,
> > -                       intel_gmbus_get_adapter(dev_priv,
> > -
> > intel_hdmi->ddc_bus));
> > -   if (edid) {
> > -           if (edid->input & DRM_EDID_INPUT_DIGITAL)
> > -                   has_audio = drm_detect_monitor_audio(edid);
> > -           kfree(edid);
> > -   }
> > -
> > -   intel_display_power_put(dev_priv, power_domain);
> > +   edid = to_intel_connector(connector)->detect_edid;
> > +   if (edid && edid->input & DRM_EDID_INPUT_DIGITAL)
> > +           has_audio = drm_detect_monitor_audio(edid);
> >  
> >     return has_audio;
> >  }
> > @@ -1479,6 +1492,7 @@ static void chv_hdmi_pre_enable(struct
> > intel_encoder *encoder) 
> >  static void intel_hdmi_destroy(struct drm_connector *connector)
> >  {
> > +   intel_hdmi_unset_edid(connector);
> >     drm_connector_cleanup(connector);
> >     kfree(connector);
> >  }
> > @@ -1486,6 +1500,7 @@ static void intel_hdmi_destroy(struct
> > drm_connector *connector) static const struct drm_connector_funcs
> > intel_hdmi_connector_funcs = { .dpms = intel_connector_dpms,
> >     .detect = intel_hdmi_detect,
> > +   .force = intel_hdmi_force,
> >     .fill_modes = drm_helper_probe_single_connector_modes,
> >     .set_property = intel_hdmi_set_property,
> >     .destroy = intel_hdmi_destroy,
> > -- 
> > 2.1.0
> 


Tried to find a JIRA for this but only came up with VIZ-1789.  Looks
like it might be close enough for Wayland?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to