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 <[email protected]> for figuring this out.
> 
> Cc: Ville Syrjälä <[email protected]>
> Cc: Joseph Nuzman <[email protected]>
> Reported-by: Joseph Nuzman <[email protected]>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101583
> Signed-off-by: Jani Nikula <[email protected]>
> 
> ---
> 
> 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ä <[email protected]>

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to