On Mon, 14 Dec 2015 10:33:44 +0100,
Jani Nikula wrote:
> 
> On Fri, 11 Dec 2015, Takashi Iwai <ti...@suse.de> 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.


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to