Re: [PATCH] drm/i915: always update ELD connector type after get modes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 19, 2017 at 06:38:13PM +0300, Jani Nikula wrote:
> drm_edid_to_eld() initializes the connector ELD to zero, overwriting the
> ELD connector type initialized in intel_audio_codec_enable(). If
> userspace does getconnector and thus get_modes after modeset, a
> subsequent audio component i915_audio_component_get_eld() call will
> receive an ELD without the connector type properly set. It's fine for
> HDMI, but screws up audio for DP.
> 
> Always set the ELD connector type at intel_connector_update_modes()
> based on the connector type. We can drop the connector type update from
> intel_audio_codec_enable().
> 
> Credits to Joseph Nuzman <jnuzman@gmail.com> for figuring this out.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Joseph Nuzman <jnuzman@gmail.com>
> Reported-by: Joseph Nuzman <jnuzman@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101583
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> We'll still need to cache and fix the av sync delay as well, but we need
> adjusted mode for that.
> ---
>  drivers/gpu/drm/i915/intel_audio.c |  5 -----
>  drivers/gpu/drm/i915/intel_modes.c | 17 +++++++++++++++++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index d805b6e6fe71..27743be5b768 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -606,11 +606,6 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>  			 connector->encoder->base.id,
>  			 connector->encoder->name);
>  
> -	/* ELD Conn_Type */
> -	connector->eld[5] &= ~(3 << 2);
> -	if (intel_crtc_has_dp_encoder(crtc_state))
> -		connector->eld[5] |= (1 << 2);
> -
>  	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
>  
>  	if (dev_priv->display.audio_codec_enable)
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 951e834dd274..28a778b785ac 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -30,6 +30,21 @@
>  #include "intel_drv.h"
>  #include "i915_drv.h"
>  
> +static void intel_connector_update_eld_conn_type(struct drm_connector *connector)
> +{
> +	u8 conn_type;
> +
> +	if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> +	    connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +		conn_type = DRM_ELD_CONN_TYPE_DP;
> +	} else {
> +		conn_type = DRM_ELD_CONN_TYPE_HDMI;
> +	}
> +
> +	connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] &= ~DRM_ELD_CONN_TYPE_MASK;
> +	connector->eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= conn_type;
> +}

Or should we just smash this into drm_edid_to_eld() ? Would
require a quick review of other drivers I suppose.

It's a step forward so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

And I guess we should still do something about the av sync delay
always getting clobbered by get_modes()?

> +
>  /**
>   * intel_connector_update_modes - update connector from edid
>   * @connector: DRM connector device to use
> @@ -44,6 +59,8 @@ int intel_connector_update_modes(struct drm_connector *connector,
>  	ret = drm_add_edid_modes(connector, edid);
>  	drm_edid_to_eld(connector, edid);
>  
> +	intel_connector_update_eld_conn_type(connector);
> +
>  	return ret;
>  }
>  
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx