On Mon, 14 Dec 2015 11:34:53 +0100, Jani Nikula wrote: > > On Mon, 14 Dec 2015, Takashi Iwai <tiwai@xxxxxxx> wrote: > > On Mon, 14 Dec 2015 10:33:44 +0100, > > Jani Nikula wrote: > >> > >> On Fri, 11 Dec 2015, Takashi Iwai <tiwai@xxxxxxx> wrote: > >> > On Fri, 11 Dec 2015 07:07:53 +0100, > >> > Libin Yang wrote: > >> >> > >> >> >>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c > >> >> >>> index 9aa83e7..5ad2e66 100644 > >> >> >>> --- a/drivers/gpu/drm/i915/intel_audio.c > >> >> >>> +++ b/drivers/gpu/drm/i915/intel_audio.c > >> >> >>> @@ -262,7 +262,8 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder) > >> >> >>> tmp |= AUD_CONFIG_N_PROG_ENABLE; > >> >> >>> tmp &= ~AUD_CONFIG_UPPER_N_MASK; > >> >> >>> tmp &= ~AUD_CONFIG_LOWER_N_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; > >> > > >> > The same check is missing in hsw_audio_codec_enable()? > >> > > >> >> >>> I915_WRITE(HSW_AUD_CFG(pipe), tmp); > >> >> >>> > >> >> >>> @@ -474,7 +475,8 @@ 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)) > >> >> >>> + 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; > >> > > >> > ... and missing for ilk_audio_codec_disable()? > >> > > >> > > >> >> >>> else > >> >> >>> tmp |= audio_config_hdmi_pixel_clock(adjusted_mode); > >> >> >>> @@ -512,7 +514,8 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder) > >> >> >>> > >> >> >>> /* ELD Conn_Type */ > >> >> >>> connector->eld[5] &= ~(3 << 2); > >> >> >>> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) > >> >> >>> + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) || > >> >> >>> + intel_pipe_has_type(crtc, INTEL_OUTPUT_DP_MST)) > >> > > >> > IMO, it's better to have a macro to cover this two-line check instead > >> > of open-coding at each place. We'll have 5 places in the end. > >> > >> I don't think that's necessary. Only the hsw_* functions need it, the > >> other platforms (using the other functions) don't support DP MST. > > > > Then the patch changes the functions. And still three places have > > this check. > > Probably the mistake in the patch was to add the check to > ilk_audio_codec_enable when it should have been added to > hsw_audio_codec_enable. Yeah, I wanted to write "the patch changes the wrong functions", too. Takashi _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
