Re: [PATCH] drm/i915: Fix bogus dig_port_map[] assignment for pre-HSW

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

On Wed, Feb 24, 2016 at 03:35:22PM +0100, Takashi Iwai wrote:
> The recent commit [0bdf5a05647a: drm/i915: Add reverse mapping between
> port and intel_encoder] introduced a reverse mapping to retrieve
> intel_dig_port object from the port number.  The code assumed that the
> port vs intel_dig_port are 1:1 mapping.  But in reality, this was a
> too naive assumption.
> 
> As Martin reported about the missing HDMI audio on his SNB machine,
> pre-HSW chips may have multiple intel_dig_port objects corresponding
> to the same port.  Since we assign the mapping statically at the init
> time and the multiple objects override the map, it may not match with
> the actually enabled output.
> 
> This patch tries to address the regression above.  The reverse mapping
> is provided basically only for the audio callbacks, so now we set /
> clear the mapping dynamically at enabling and disabling HDMI/DP audio,
> so that we can always track the latest and correct object
> corresponding to the given port.
> 
> Fixes: 0bdf5a05647a ('drm/i915: Add reverse mapping between port and intel_encoder')
> Reported-and-tested-by: Martin Kepplinger <martink@posteo.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

lgtm

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_audio.c | 3 +++
>  drivers/gpu/drm/i915/intel_ddi.c   | 1 -
>  drivers/gpu/drm/i915/intel_dp.c    | 1 -
>  drivers/gpu/drm/i915/intel_hdmi.c  | 2 --
>  4 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 31f6d212fb1b..30f921421b0c 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -527,6 +527,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  	intel_dig_port->audio_connector = connector;
> +	/* referred in audio callbacks */
> +	dev_priv->dig_port_map[port] = intel_encoder;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
>  	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> @@ -554,6 +556,7 @@ void intel_audio_codec_disable(struct intel_encoder *intel_encoder)
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  	intel_dig_port->audio_connector = NULL;
> +	dev_priv->dig_port_map[port] = NULL;
>  	mutex_unlock(&dev_priv->av_mutex);
>  
>  	if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 54a165b9c92d..a50fc452d5f1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3312,7 +3312,6 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  	intel_encoder->get_config = intel_ddi_get_config;
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>  					  (DDI_BUF_PORT_REVERSAL |
>  					   DDI_A_4_LANES);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 1bbd67b046da..acf918728492 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6035,7 +6035,6 @@ intel_dp_init(struct drm_device *dev,
>  	}
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->dp.output_reg = output_reg;
>  
>  	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4a77639a489d..23ee48dc765f 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2146,7 +2146,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  void intel_hdmi_init(struct drm_device *dev,
>  		     i915_reg_t hdmi_reg, enum port port)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
> @@ -2215,7 +2214,6 @@ void intel_hdmi_init(struct drm_device *dev,
>  		intel_encoder->cloneable |= 1 << INTEL_OUTPUT_HDMI;
>  
>  	intel_dig_port->port = port;
> -	dev_priv->dig_port_map[port] = intel_encoder;
>  	intel_dig_port->hdmi.hdmi_reg = hdmi_reg;
>  	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>  
> -- 
> 2.7.2

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