On Tue, 15 Nov 2016, libin.yang@xxxxxxxxx wrote:
> From: Libin Yang <libin.yang@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
