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@xxxxxxxxx> for figuring this out. > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Cc: Joseph Nuzman <jnuzman@xxxxxxxxx> > Reported-by: Joseph Nuzman <jnuzman@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101583 > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
