>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] >Sent: Wednesday, November 30, 2016 12:50 AM >To: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> >Cc: Yang, Libin <libin.yang@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Vetter, >Daniel <daniel.vetter@xxxxxxxxx>; Pandiyan, Dhinakaran ><dhinakaran.pandiyan@xxxxxxxxx>; Kp, Jeeja <jeeja.kp@xxxxxxxxx>; >tiwai@xxxxxxx >Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for >DP MST > >On Tue, Nov 29, 2016 at 06:33:50PM +0200, Jani Nikula wrote: >> On Tue, 15 Nov 2016, libin.yang@xxxxxxxxx wrote: >> > From: Libin Yang <libin.yang@xxxxxxxxx> >> > >> > This patch extends the support of audio sample rate sync up to DP >> > MST. >> > >> > Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/intel_audio.c | 3 ++- >> > 1 file changed, 2 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c >> > b/drivers/gpu/drm/i915/intel_audio.c >> > index c8a1345..88ed869 100644 >> > --- a/drivers/gpu/drm/i915/intel_audio.c >> > +++ b/drivers/gpu/drm/i915/intel_audio.c >> > @@ -807,7 +807,8 @@ static int >i915_audio_component_sync_audio_rate(struct device *kdev, int port, >> > intel_encoder = get_saved_enc(dev_priv, port, pipe); >> > if (!intel_encoder || !intel_encoder->base.crtc || >> > (intel_encoder->type != INTEL_OUTPUT_HDMI && >> > - intel_encoder->type != INTEL_OUTPUT_DP)) { >> > + intel_encoder->type != INTEL_OUTPUT_DP && >> > + intel_encoder->type != INTEL_OUTPUT_DP_MST)) { >> >> I think the better option is to make absolutely sure we never store >> other kinds of encoders in dev_priv->av_enc_map[pipe] to begin with. I >> think that's true already, but please add >> >> if (WARN_ON(intel_encoder->type != INTEL_OUTPUT_HDMI && >> intel_encoder->type != INTEL_OUTPUT_DP && >> intel_encoder->type != INTEL_OUTPUT_DP_MST)) >> return; >> >> near the beginning of intel_audio_codec_enable(), and remove the type >> checks here. This reduces the confusion about different kinds of >> checks after calling get_saved_enc(). > >This while encoder->type thing is pretty broken actually. Currently we're semi- >randomly flipping DDI encoders between UNKNOWN,HDMI and DP. >I want to eliminate that so that their type will always be just "DDI". >For iguring out what kind of signal we are driving out you should be looking at >crtc_state->output_types. Do you mean the code should be like: @@ -807,10 +807,7 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, /* 1. get the pipe */ intel_encoder = get_saved_enc(dev_priv, port, pipe); - if (!intel_encoder || !intel_encoder->base.crtc || - (intel_encoder->type != INTEL_OUTPUT_HDMI && - intel_encoder->type != INTEL_OUTPUT_DP && - intel_encoder->type != INTEL_OUTPUT_DP_MST)) { + if (!intel_encoder || !intel_encoder->base.crtc) { DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port)); err = -ENODEV; goto unlock; @@ -819,7 +816,12 @@ static int i915_audio_component_sync_audio_rate(struct device *kdev, int port, /* pipe passed from the audio driver will be -1 for Non-MST case */ crtc = to_intel_crtc(intel_encoder->base.crtc); crtc_state = crtc->config; + if (WARN_ON((crtc_state->output_types & + ((1 << INTEL_OUTPUT_HDMI) | + (1 << INTEL_OUTPUT_DP) | + (1 << INTEL_OUTPUT_DP_MST))) == 0)) + return 0; + pipe = crtc->pipe; I did the test, and it seems intel_encoder->type can show the correct type on my platform. It is similar with crtc_state->output_types. Regards, Libin > >-- >Ville Syrjälä >Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
