| Message ID | 1456324522-21591-1-git-send-email-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sorry, Cc to Jani was missing mistakenly. Please check this. It's a regression in 4.5-rc. thanks, Takashi 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
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
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. 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
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 >
On Mon, 29 Feb 2016 15:39:53 +0100, Jani Nikula wrote: > > 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...) As far as looking at linux-next, this one still seems remaining in the crevasse... thanks, Takashi > > 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 >
On Mon, Feb 29, 2016 at 02:26:52PM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: Fix bogus dig_port_map[] assignment for pre-HSW > URL : https://patchwork.freedesktop.org/series/3916/ > State : failure > > == Summary == > > Series 3916v1 drm/i915: Fix bogus dig_port_map[] assignment for pre-HSW > http://patchwork.freedesktop.org/api/1.0/series/3916/revisions/1/mbox/ > > Test drv_hangman: > Subgroup error-state-basic: > pass -> INCOMPLETE (snb-x220t) > Test drv_module_reload_basic: > pass -> INCOMPLETE (snb-x220t) ... > Test kms_flip: > Subgroup basic-flip-vs-dpms: > pass -> INCOMPLETE (skl-i5k-2) > pass -> INCOMPLETE (snb-x220t) ... One snb and skl seem to have dropped out in the middle of the run for whatever reason. > Test kms_force_connector_basic: > Subgroup force-load-detect: > fail -> DMESG-FAIL (snb-dellxps) Known and should have fixed since by: https://lists.freedesktop.org/archives/intel-gfx/2016-March/088743.html > Test kms_pipe_crc_basic: > Subgroup suspend-read-crc-pipe-c: > pass -> DMESG-WARN (bsw-nuc-2) [ 231.962372] [drm:intel_runtime_suspend [i915]] *ERROR* Unclaimed access detected prior to suspending https://bugs.freedesktop.org/show_bug.cgi?id=94349 > Test pm_rpm: > Subgroup basic-pci-d3-state: > pass -> DMESG-WARN (bsw-nuc-2) lockdep splat https://bugs.freedesktop.org/show_bug.cgi?id=94350
Am 2016-03-03 um 16:05 schrieb Takashi Iwai: > On Mon, 29 Feb 2016 15:39:53 +0100, > Jani Nikula wrote: >> >> 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...) > > As far as looking at linux-next, this one still seems remaining in the > crevasse... > This still isn't merged but still fixes a serious regression in v4.5. What's going on? thanks martin > > thanks, > > Takashi > >> >> 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 >>
On Mon, 07 Mar 2016, Martin Kepplinger <martink@posteo.de> wrote: > This still isn't merged but still fixes a serious regression in v4.5. > What's going on? Bumps in the CI road. Now pushed to drm-intel-next-queued and cherry-picked to drm-intel-fixes, en route to v4.5. Thanks for the patch, review, and testing. BR, Jani.
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;
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(-)