Re: [PATCH 1/2] drm/i915/audio: extend get_saved_enc() to support more scenarios

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

On Tue, 15 Nov 2016, libin.yang@intel.com wrote:
> From: Libin Yang <libin.yang@linux.intel.com>
>
> When bootup, audio driver may not know it is MST or not. The audio
> driver will poll all the port & pipe combinations in either MST or
> Non-MST mode. get_saved_enc() should handle this situation.
>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 49f1053..c8a1345 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -737,25 +737,49 @@ static int i915_audio_component_get_cdclk_freq(struct device *kdev)
>  	return dev_priv->cdclk_freq;
>  }
>  
> +/*
> + * get the intel_encoder according to the parameter port and pipe
> + * intel_encoder is saved by the index of pipe
> + * MST & (pipe >= 0): return the av_enc_map[pipe],
> + *   when port is matched
> + * MST & (pipe < 0): this is invalid
> + * Non-MST & (pipe >= 0): only pipe = 0 (the first device entry)
> + *   will get the right intel_encoder with port matched
> + * Non-MST & (pipe < 0): get the right intel_encoder with port matched

This essentially describes the code in English. I can read the code. I
need to know more about the higher level of what this does and *why*.

I also look at the call sites of get_saved_enc and wonder why they have
different checks for the return values. Also patch 2/2 only modifies one
of them, not both. Why?

So maybe I'm just too clueless about DP (MST) audio in general, and I
wouldn't even get offended if you told me that. But I don't think the
commit message and the comments in this patch properly help me either.

BR,
Jani.


> + */
>  static struct intel_encoder *get_saved_enc(struct drm_i915_private *dev_priv,
>  					       int port, int pipe)
>  {
> +	struct intel_encoder *encoder;
>  
>  	if (WARN_ON(pipe >= I915_MAX_PIPES))
>  		return NULL;
>  
>  	/* MST */
> -	if (pipe >= 0)
> -		return dev_priv->av_enc_map[pipe];
> +	if (pipe >= 0) {
> +		encoder = dev_priv->av_enc_map[pipe];
> +		/*
> +		 * when bootup, audio driver may not know it is
> +		 * MST or not. So it will poll all the port & pipe
> +		 * combinations
> +		 */
> +		if (encoder != NULL && encoder->port == port &&
> +		    encoder->type == INTEL_OUTPUT_DP_MST)
> +			return encoder;
> +	}
>  
>  	/* Non-MST */
> -	for_each_pipe(dev_priv, pipe) {
> -		struct intel_encoder *encoder;
> +	if (pipe > 0)
> +		return NULL;
>  
> +	for_each_pipe(dev_priv, pipe) {
>  		encoder = dev_priv->av_enc_map[pipe];
>  		if (encoder == NULL)
>  			continue;
>  
> +		if (encoder->type == INTEL_OUTPUT_DP_MST)
> +			continue;
> +
>  		if (port == encoder->port)
>  			return encoder;
>  	}

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