| Message ID | 1457710386-4466-1-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote: > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously > added a check for MST encoder to ilk_audio_codec_enable instead of the > hsw+ equivalent. That causes the HDMI path to be used, resulting in a > wrong register value. > > Cc: Libin Yang <libin.yang@linux.intel.com> > Cc: drm-intel-fixes@lists.freedesktop.org > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_audio.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > index 30f9214..f5fccf7 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, > tmp = I915_READ(HSW_AUD_CFG(pipe)); > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || > + intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) Humm. I thought had I asked all of these to be changed to has_dp_encoder. I wonder if I imagined it or if I was just ignored. > tmp |= AUD_CONFIG_N_VALUE_INDEX; > else > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || > - intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) > tmp |= AUD_CONFIG_N_VALUE_INDEX; > else > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); > -- > 2.4.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2016-03-11 at 17:37 +0200, Ville Syrjälä wrote: > On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote: > > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously > > added a check for MST encoder to ilk_audio_codec_enable instead of the > > hsw+ equivalent. That causes the HDMI path to be used, resulting in a > > wrong register value. > > > > Cc: Libin Yang <libin.yang@linux.intel.com> > > Cc: drm-intel-fixes@lists.freedesktop.org > > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") > > Signed-off-by: Ander Conselvan de Oliveira < > > ander.conselvan.de.oliveira@intel.com> > > --- > > drivers/gpu/drm/i915/intel_audio.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > > b/drivers/gpu/drm/i915/intel_audio.c > > index 30f9214..f5fccf7 100644 > > --- a/drivers/gpu/drm/i915/intel_audio.c > > +++ b/drivers/gpu/drm/i915/intel_audio.c > > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector > > *connector, > > tmp = I915_READ(HSW_AUD_CFG(pipe)); > > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) > > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || > > + intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) > > Humm. I thought had I asked all of these to be changed to has_dp_encoder. > I wonder if I imagined it or if I was just ignored. Ah, yes. Libin sent a patch to fix the same bug and you asked for that [1] and even R-b the follow up version even though it didn't use it everywhere. But it looks like Daniel didn't merge it since he wanted all uses converted to has_dp_encoders. [1] https://patchwork.freedesktop.org/patch/68902/ [2] https://patchwork.freedesktop.org/patch/69525/ > > > tmp |= AUD_CONFIG_N_VALUE_INDEX; > > else > > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); > > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector > > *connector, > > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; > > tmp &= ~AUD_CONFIG_N_PROG_ENABLE; > > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; > > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || > > - intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) > > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) > > tmp |= AUD_CONFIG_N_VALUE_INDEX; > > else > > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); > > -- > > 2.4.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Fri, 11 Mar 2016, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote: > [ text/plain ] > On Fri, 2016-03-11 at 17:37 +0200, Ville Syrjälä wrote: >> On Fri, Mar 11, 2016 at 05:33:06PM +0200, Ander Conselvan de Oliveira wrote: >> > Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously >> > added a check for MST encoder to ilk_audio_codec_enable instead of the >> > hsw+ equivalent. That causes the HDMI path to be used, resulting in a >> > wrong register value. >> > >> > Cc: Libin Yang <libin.yang@linux.intel.com> >> > Cc: drm-intel-fixes@lists.freedesktop.org >> > Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") >> > Signed-off-by: Ander Conselvan de Oliveira < >> > ander.conselvan.de.oliveira@intel.com> >> > --- >> > drivers/gpu/drm/i915/intel_audio.c | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c >> > b/drivers/gpu/drm/i915/intel_audio.c >> > index 30f9214..f5fccf7 100644 >> > --- a/drivers/gpu/drm/i915/intel_audio.c >> > +++ b/drivers/gpu/drm/i915/intel_audio.c >> > @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector >> > *connector, >> > tmp = I915_READ(HSW_AUD_CFG(pipe)); >> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; >> > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; >> > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) >> > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || >> > + intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) >> >> Humm. I thought had I asked all of these to be changed to has_dp_encoder. >> I wonder if I imagined it or if I was just ignored. > > Ah, yes. Libin sent a patch to fix the same bug and you asked for that [1] and > even R-b the follow up version even though it didn't use it everywhere. But it > looks like Daniel didn't merge it since he wanted all uses converted to > has_dp_encoders. We want the minimal fix first now because by bikeshedding we let the bug slip to v4.5, and we need cc: stable on this. BR, Jani. > > [1] https://patchwork.freedesktop.org/patch/68902/ > [2] https://patchwork.freedesktop.org/patch/69525/ > > >> >> > tmp |= AUD_CONFIG_N_VALUE_INDEX; >> > else >> > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); >> > @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector >> > *connector, >> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX; >> > tmp &= ~AUD_CONFIG_N_PROG_ENABLE; >> > tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; >> > - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || >> > - intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) >> > + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) >> > tmp |= AUD_CONFIG_N_VALUE_INDEX; >> > else >> > tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); >> > -- >> > 2.4.3 >> > >> > _______________________________________________ >> > Intel-gfx mailing list >> > Intel-gfx@lists.freedesktop.org >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 30f9214..f5fccf7 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -329,7 +329,8 @@ static void hsw_audio_codec_enable(struct drm_connector *connector, tmp = I915_READ(HSW_AUD_CFG(pipe)); tmp &= ~AUD_CONFIG_N_VALUE_INDEX; tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || + intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) tmp |= AUD_CONFIG_N_VALUE_INDEX; else tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); @@ -476,8 +477,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector, tmp &= ~AUD_CONFIG_N_VALUE_INDEX; tmp &= ~AUD_CONFIG_N_PROG_ENABLE; tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK; - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT) || - intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DP_MST)) + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT)) tmp |= AUD_CONFIG_N_VALUE_INDEX; else tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
Commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") erroneously added a check for MST encoder to ilk_audio_codec_enable instead of the hsw+ equivalent. That causes the HDMI path to be used, resulting in a wrong register value. Cc: Libin Yang <libin.yang@linux.intel.com> Cc: drm-intel-fixes@lists.freedesktop.org Fixes: commit 3d52ccf52f2c ("drm/i915: start adding dp mst audio") Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_audio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)