From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 3/3] drm/i915/hdmi: Cache EDID for a detection cycle Date: Tue, 2 Sep 2014 13:45:37 +0300 Message-ID: <20140902104537.GK4193@intel.com> References: <20140901120526.GD4193@intel.com> <1409646288-29732-1-git-send-email-chris@chris-wilson.co.uk> <1409646288-29732-3-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 4CA6F6E446 for ; Tue, 2 Sep 2014 03:46:20 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1409646288-29732-3-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org 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 > Cc: Ville Syrj=E4l=E4 > --- > 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/int= el_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_encod= er *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 =3D connector->dev; > struct intel_hdmi *intel_hdmi =3D intel_attached_hdmi(connector); > - struct intel_digital_port *intel_dig_port =3D > - hdmi_to_dig_port(intel_hdmi); > - struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > - struct drm_i915_private *dev_priv =3D dev->dev_private; > - struct edid *edid; > - enum intel_display_power_domain power_domain; > - enum drm_connector_status status =3D connector_status_disconnected; > = > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > - connector->base.id, connector->name); > + intel_hdmi->has_hdmi_sink =3D false; > + intel_hdmi->has_audio =3D false; > + intel_hdmi->rgb_quant_range_selectable =3D false; > + > + kfree(to_intel_connector(connector)->detect_edid); > + to_intel_connector(connector)->detect_edid =3D NULL; > +} > + > +static bool > +intel_hdmi_set_edid(struct drm_connector *connector) > +{ > + struct drm_i915_private *dev_priv =3D to_i915(connector->dev); > + struct intel_hdmi *intel_hdmi =3D intel_attached_hdmi(connector); > + struct intel_encoder *intel_encoder =3D > + &hdmi_to_dig_port(intel_hdmi)->base; > + enum intel_display_power_domain power_domain; > + struct edid *edid; > + bool connected =3D false; > = > power_domain =3D intel_display_port_power_domain(intel_encoder); > intel_display_power_get(dev_priv, power_domain); > = > - intel_hdmi->has_hdmi_sink =3D false; > - intel_hdmi->has_audio =3D false; > - intel_hdmi->rgb_quant_range_selectable =3D false; > edid =3D drm_get_edid(connector, > intel_gmbus_get_adapter(dev_priv, > intel_hdmi->ddc_bus)); > = > - if (edid) { > - if (edid->input & DRM_EDID_INPUT_DIGITAL) { > - status =3D connector_status_connected; > - if (intel_hdmi->force_audio !=3D HDMI_AUDIO_OFF_DVI) > - intel_hdmi->has_hdmi_sink =3D > - drm_detect_hdmi_monitor(edid); > - intel_hdmi->has_audio =3D drm_detect_monitor_audio(edid); > - intel_hdmi->rgb_quant_range_selectable =3D > - drm_rgb_quant_range_selectable(edid); > - } > - kfree(edid); > - } > + intel_display_power_put(dev_priv, power_domain); > + > + to_intel_connector(connector)->detect_edid =3D edid; > + if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { > + intel_hdmi->rgb_quant_range_selectable =3D > + drm_rgb_quant_range_selectable(edid); > = > - if (status =3D=3D connector_status_connected) { > + intel_hdmi->has_audio =3D drm_detect_monitor_audio(edid); > if (intel_hdmi->force_audio !=3D HDMI_AUDIO_AUTO) > intel_hdmi->has_audio =3D > - (intel_hdmi->force_audio =3D=3D HDMI_AUDIO_ON); > - intel_encoder->type =3D INTEL_OUTPUT_HDMI; > + intel_hdmi->force_audio =3D=3D HDMI_AUDIO_ON; > + > + if (intel_hdmi->force_audio !=3D HDMI_AUDIO_OFF_DVI) > + intel_hdmi->has_hdmi_sink =3D > + drm_detect_hdmi_monitor(edid); I wonder if we should be updating has_hdmi_sink also in intel_hdmi_set_property() when force_audio !=3D HDMI_AUDIO_OFF_DVI. But that's a separate issue so material for another patch. > + > + connected =3D 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 =3D intel_attached_hdmi(connector); > + > + hdmi_to_dig_port(intel_hdmi)->base.type =3D INTEL_OUTPUT_HDMI; > + status =3D connector_status_connected; > + } else > + status =3D 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 =3D intel_attached_encoder(connecto= r); > - struct intel_hdmi *intel_hdmi =3D enc_to_intel_hdmi(&intel_encoder->bas= e); > - struct drm_i915_private *dev_priv =3D connector->dev->dev_private; > - enum intel_display_power_domain power_domain; > - int ret; > + struct intel_hdmi *intel_hdmi =3D 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 =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > + intel_hdmi_unset_edid(connector); > = > - ret =3D intel_ddc_get_modes(connector, > - intel_gmbus_get_adapter(dev_priv, > - intel_hdmi->ddc_bus)); > + if (connector->status =3D=3D connector_status_connected) !=3D Apart from that I didn't spot anything suspicious. So with this one thing fixed you can slap on Reviewed-by: Ville Syrj=E4l=E4 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 =3D INTEL_OUTPUT_HDMI; > +} > = > - return ret; > +static int intel_hdmi_get_modes(struct drm_connector *connector) > +{ > + struct edid *edid; > + > + edid =3D to_intel_connector(connector)->detect_edid; > + if (edid =3D=3D 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 =3D intel_attached_encoder(connecto= r); > - struct intel_hdmi *intel_hdmi =3D enc_to_intel_hdmi(&intel_encoder->bas= e); > - struct drm_i915_private *dev_priv =3D connector->dev->dev_private; > - enum intel_display_power_domain power_domain; > - struct edid *edid; > bool has_audio =3D false; > + struct edid *edid; > = > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > - > - edid =3D 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 =3D drm_detect_monitor_audio(edid); > - kfree(edid); > - } > - > - intel_display_power_put(dev_priv, power_domain); > + edid =3D to_intel_connector(connector)->detect_edid; > + if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) > + has_audio =3D drm_detect_monitor_audio(edid); > = > return has_audio; > } > @@ -1479,6 +1492,7 @@ static void chv_hdmi_pre_enable(struct intel_encode= r *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 =3D { > .dpms =3D intel_connector_dpms, > .detect =3D intel_hdmi_detect, > + .force =3D intel_hdmi_force, > .fill_modes =3D drm_helper_probe_single_connector_modes, > .set_property =3D intel_hdmi_set_property, > .destroy =3D intel_hdmi_destroy, > -- = > 2.1.0 -- = Ville Syrj=E4l=E4 Intel OTC