On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
> From: Libin Yang <libin.y...@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.y...@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

Reply via email to