[Intel-gfx] [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for DP MST

[Yang, Libin]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
>Sent: Wednesday, November 30, 2016 12:50 AM
>To: Jani Nikula <jani.nikula at linux.intel.com>
>Cc: Yang, Libin <libin.yang at intel.com>; intel-gfx at lists.freedesktop.org; Vetter,
>Daniel <daniel.vetter at intel.com>; Pandiyan, Dhinakaran
><dhinakaran.pandiyan at intel.com>; Kp, Jeeja <jeeja.kp at intel.com>;
>tiwai at suse.de
>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 at intel.com wrote:
>> > From: Libin Yang <libin.yang at intel.com>
>> >
>> > This patch extends the support of audio sample rate sync up to DP
>> > MST.
>> >
>> > Signed-off-by: Libin Yang <libin.yang at intel.com>
>> > ---
>> >  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