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

Jani Nikula
2016-02-29T14:39:53Z

On Mon, 29 Feb 2016, Martin Kepplinger <martink@posteo.de> wrote:
> Am 2016-02-26 um 20:59 schrieb Takashi Iwai:
>> Sorry, Cc to Jani was missing mistakenly.
>>
>> Please check this. It's a regression in 4.5-rc.
>>
>>
>> thanks,
>>
>> Takashi
>>
>
> I can confirm that with -rc6 nothing changed and this fixes audio over
> HDMI for me. I added David Airlie and dri-devel to CC aswell and hope
> that this can go in for 4.5 in the last minute.

I'll pick this up when we get the CI results. (This had fallen between
the cracks...)

BR,
Jani.

>
> thanks
> martin
>
>
>> On Wed, 24 Feb 2016 15:35:22 +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>
>>> ---
>>> 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
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

--
Jani Nikula, Intel Open Source Technology Center