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

[Jani Nikula <jani.nikula@linux.intel.com>]

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

We do not need to duplicate the checks here if we ensure only good stuff
is ever put into av_enc_map.

>
> 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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx