Sorry, I missed the emails.
On 12/11/2015 09:58 PM, Takashi Iwai wrote:
On Fri, 11 Dec 2015 11:43:51 +0100, Takashi Iwai 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()?
Based on the discussion, I think you and Jani are right. It should be in hsw_audio_codec_enable() and not in ild_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.
OK, I will use a macro.
Also, this patch still has an issue about the encoder type, namely, it passes intel_encoder from MST, where you can't apply enc_to_dig_port(). We need another help to get the digital port depending on the encoder type, e.g.
static struct intel_digital_port * intel_encoder_to_dig_port(struct intel_encoder *intel_encoder) { struct drm_encoder *encoder = &intel_encoder->base;
if (intel_encoder->type == INTEL_OUTPUT_DP_MST) return enc_to_mst(encoder)->primary; return enc_to_dig_port(encoder); }
Yes, I will fix it in the new version patch.
Best Regards, Libin
Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel