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

[Yang, Libin]

>-----Original Message-----
>From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
>Sent: Wednesday, November 30, 2016 12:34 AM
>To: Yang, Libin <libin.yang at intel.com>; intel-gfx at lists.freedesktop.org;
>ville.syrjala at linux.intel.com; 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
>Cc: Yang, Libin <libin.yang at intel.com>
>Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for
>DP MST
>
>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().

We can't put the check at the beginning of the function.
We still need the check in the same place as we need get intel_encoder at first.

Regards,
Libin

>
>BR,
>Jani.
>
>
>>  		DRM_DEBUG_KMS("Not valid for port %c\n",
>port_name(port));
>>  		err = -ENODEV;
>>  		goto unlock;
>
>--
>Jani Nikula, Intel Open Source Technology Center