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

2016-12-03 Thread Yang, Libin 

>-Original Message-
>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>Sent: Friday, December 2, 2016 10:25 PM
>To: Yang, Libin ; intel-gfx@lists.freedesktop.org;
>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>Pandiyan, Dhinakaran ; ti...@suse.de;
>liang.xi...@dell.com
>Subject: RE: [PATCH 2/2] drm/i915/audio: extend audio sync rate support for
>DP MST
>
>On Fri, 02 Dec 2016, "Yang, Libin"  wrote:
>> Any comments? Thanks.
>
>Both pushed to dinq, thanks for the patches.
>
>Libin and/or Dhinakaran, please improve the documentation of struct
>i915_audio_component_ops to describe the parameters of sync_audio_rate
>and get_eld hooks. For example, there's nothing about pipe being -1 for non-
>MST.

Thanks. Get it. I will update it next week.

Regards,
Libin

>
>BR,
>Jani.
>
>
>>
>> Regards,
>> Libin
>>
>>
>>>-Original Message-
>>>From: Yang, Libin
>>>Sent: Thursday, December 1, 2016 1:17 PM
>>>To: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
>>>ville.syrj...@linux.intel.com; Vetter, Daniel
>>>; Pandiyan, Dhinakaran
>>>; ti...@suse.de
>>>Cc: Yang, Libin 
>>>Subject: [PATCH 2/2] drm/i915/audio: extend audio sync rate support
>>>for DP MST
>>>
>>>From: Libin Yang 
>>>
>>>Remove the type judgement in i915_audio_component_sync_audio_rate().
>>>Audio rate sync is necessary for all i915 digital audio now.
>>>
>>>Signed-off-by: Libin Yang 
>>>---
>>> drivers/gpu/drm/i915/intel_audio.c | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/i915/intel_audio.c
>>>b/drivers/gpu/drm/i915/intel_audio.c
>>>index c8a1345..3bbc96c 100644
>>>--- a/drivers/gpu/drm/i915/intel_audio.c
>>>+++ b/drivers/gpu/drm/i915/intel_audio.c
>>>@@ -805,9 +805,7 @@ static int
>>>i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>>>
>>> /* 1. get the pipe */
>>> 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)) {
>>>+if (!intel_encoder || !intel_encoder->base.crtc) {
>>> DRM_DEBUG_KMS("Not valid for port %c\n",
>port_name(port));
>>> err = -ENODEV;
>>> goto unlock;
>>>--
>>>2.7.4
>>
>
>--
>Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-12-02 Thread Jani Nikula 
On Fri, 02 Dec 2016, "Yang, Libin"  wrote:
> Any comments? Thanks.

Both pushed to dinq, thanks for the patches.

Libin and/or Dhinakaran, please improve the documentation of struct
i915_audio_component_ops to describe the parameters of sync_audio_rate
and get_eld hooks. For example, there's nothing about pipe being -1 for
non-MST.

BR,
Jani.


>
> Regards,
> Libin
>
>
>>-Original Message-
>>From: Yang, Libin
>>Sent: Thursday, December 1, 2016 1:17 PM
>>To: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
>>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>>Pandiyan, Dhinakaran ; ti...@suse.de
>>Cc: Yang, Libin 
>>Subject: [PATCH 2/2] drm/i915/audio: extend audio sync rate support for DP
>>MST
>>
>>From: Libin Yang 
>>
>>Remove the type judgement in i915_audio_component_sync_audio_rate().
>>Audio rate sync is necessary for all i915 digital audio now.
>>
>>Signed-off-by: Libin Yang 
>>---
>> drivers/gpu/drm/i915/intel_audio.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_audio.c
>>b/drivers/gpu/drm/i915/intel_audio.c
>>index c8a1345..3bbc96c 100644
>>--- a/drivers/gpu/drm/i915/intel_audio.c
>>+++ b/drivers/gpu/drm/i915/intel_audio.c
>>@@ -805,9 +805,7 @@ static int
>>i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>>
>>  /* 1. get the pipe */
>>  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)) {
>>+ if (!intel_encoder || !intel_encoder->base.crtc) {
>>  DRM_DEBUG_KMS("Not valid for port %c\n",
>>port_name(port));
>>  err = -ENODEV;
>>  goto unlock;
>>--
>>2.7.4
>

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

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

2016-12-01 Thread Yang, Libin 
Any comments? Thanks.

Regards,
Libin


>-Original Message-
>From: Yang, Libin
>Sent: Thursday, December 1, 2016 1:17 PM
>To: intel-gfx@lists.freedesktop.org; jani.nik...@linux.intel.com;
>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>Pandiyan, Dhinakaran ; ti...@suse.de
>Cc: Yang, Libin 
>Subject: [PATCH 2/2] drm/i915/audio: extend audio sync rate support for DP
>MST
>
>From: Libin Yang 
>
>Remove the type judgement in i915_audio_component_sync_audio_rate().
>Audio rate sync is necessary for all i915 digital audio now.
>
>Signed-off-by: Libin Yang 
>---
> drivers/gpu/drm/i915/intel_audio.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_audio.c
>b/drivers/gpu/drm/i915/intel_audio.c
>index c8a1345..3bbc96c 100644
>--- a/drivers/gpu/drm/i915/intel_audio.c
>+++ b/drivers/gpu/drm/i915/intel_audio.c
>@@ -805,9 +805,7 @@ static int
>i915_audio_component_sync_audio_rate(struct device *kdev, int port,
>
>   /* 1. get the pipe */
>   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)) {
>+  if (!intel_encoder || !intel_encoder->base.crtc) {
>   DRM_DEBUG_KMS("Not valid for port %c\n",
>port_name(port));
>   err = -ENODEV;
>   goto unlock;
>--
>2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-11-30 Thread Yang, Libin 

>-Original Message-
>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>Sent: Wednesday, November 30, 2016 11:52 PM
>To: Yang, Libin ; intel-gfx@lists.freedesktop.org;
>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>Pandiyan, Dhinakaran ; Kp, Jeeja
>; ti...@suse.de
>Subject: RE: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for
>DP MST
>
>On Wed, 30 Nov 2016, "Yang, Libin"  wrote:
>>>-Original Message-
>>>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>>>Sent: Wednesday, November 30, 2016 12:34 AM
>>>To: Yang, Libin ;
>>>intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com;
>>>Vetter, Daniel ; Pandiyan, Dhinakaran
>>>; Kp, Jeeja ;
>>>ti...@suse.de
>>>Cc: Yang, Libin 
>>>Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate
>>>support for DP MST
>>>
>>>On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
 From: Libin Yang 

 This patch extends the support of audio sample rate sync up to DP MST.

 Signed-off-by: Libin Yang 
 ---
  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.

Yes, I agree. I will remove the type judgement.

Regards,
Libin

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

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

2016-11-30 Thread Jani Nikula 
On Wed, 30 Nov 2016, "Yang, Libin"  wrote:
>>-Original Message-
>>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>>Sent: Wednesday, November 30, 2016 12:34 AM
>>To: Yang, Libin ; intel-gfx@lists.freedesktop.org;
>>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>>Pandiyan, Dhinakaran ; Kp, Jeeja
>>; ti...@suse.de
>>Cc: Yang, Libin 
>>Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for
>>DP MST
>>
>>On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
>>> From: Libin Yang 
>>>
>>> This patch extends the support of audio sample rate sync up to DP MST.
>>>
>>> Signed-off-by: Libin Yang 
>>> ---
>>>  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

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

2016-11-30 Thread Yang, Libin 

>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Wednesday, November 30, 2016 12:50 AM
>To: Jani Nikula 
>Cc: Yang, Libin ; intel-gfx@lists.freedesktop.org; 
>Vetter,
>Daniel ; Pandiyan, Dhinakaran
>; Kp, Jeeja ;
>ti...@suse.de
>Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for
>DP MST
>
>On Tue, Nov 29, 2016 at 06:33:50PM +0200, Jani Nikula wrote:
>> On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
>> > From: Libin Yang 
>> >
>> > This patch extends the support of audio sample rate sync up to DP
>> > MST.
>> >
>> > Signed-off-by: Libin Yang 
>> > ---
>> >  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().
>
>This while encoder->type thing is pretty broken actually. Currently we're semi-
>randomly flipping DDI encoders between UNKNOWN,HDMI and DP.
>I want to eliminate that so that their type will always be just "DDI".
>For iguring out what kind of signal we are driving out you should be looking at
>crtc_state->output_types.

Do you mean the code should be like:
@@ -807,10 +807,7 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev, int port,
 
/* 1. get the pipe */
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_MST)) {
+   if (!intel_encoder || !intel_encoder->base.crtc) {
DRM_DEBUG_KMS("Not valid for port %c\n", port_name(port));
err = -ENODEV;
goto unlock;
@@ -819,7 +816,12 @@ static int i915_audio_component_sync_audio_rate(struct 
device *kdev, int port,
/* pipe passed from the audio driver will be -1 for Non-MST case */
crtc = to_intel_crtc(intel_encoder->base.crtc);
crtc_state = crtc->config;
+   if (WARN_ON((crtc_state->output_types &
+   ((1 << INTEL_OUTPUT_HDMI) |
+(1 << INTEL_OUTPUT_DP) |
+(1 << INTEL_OUTPUT_DP_MST))) == 0))
+   return 0;
+
pipe = crtc->pipe;

I did the test, and it seems intel_encoder->type can show the correct type 
on my platform. It is similar with crtc_state->output_types.

Regards,
Libin

>
>--
>Ville Syrjälä
>Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-11-30 Thread Yang, Libin 

>-Original Message-
>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>Sent: Wednesday, November 30, 2016 12:34 AM
>To: Yang, Libin ; intel-gfx@lists.freedesktop.org;
>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>Pandiyan, Dhinakaran ; Kp, Jeeja
>; ti...@suse.de
>Cc: Yang, Libin 
>Subject: Re: [PATCH 2/2] drm/i915/audio: Extend audio sync rate support for
>DP MST
>
>On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
>> From: Libin Yang 
>>
>> This patch extends the support of audio sample rate sync up to DP MST.
>>
>> Signed-off-by: Libin Yang 
>> ---
>>  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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-11-29 Thread Ville Syrjälä 
On Tue, Nov 29, 2016 at 06:33:50PM +0200, Jani Nikula wrote:
> On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
> > From: Libin Yang 
> >
> > This patch extends the support of audio sample rate
> > sync up to DP MST.
> >
> > Signed-off-by: Libin Yang 
> > ---
> >  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().

This while encoder->type thing is pretty broken actually. Currently
we're semi-randomly flipping DDI encoders between UNKNOWN,HDMI and DP.
I want to eliminate that so that their type will always be just "DDI".
For iguring out what kind of signal we are driving out you should be
looking at crtc_state->output_types.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-11-29 Thread Jani Nikula 
On Tue, 15 Nov 2016, libin.y...@intel.com wrote:
> From: Libin Yang 
>
> This patch extends the support of audio sample rate
> sync up to DP MST.
>
> Signed-off-by: Libin Yang 
> ---
>  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().

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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx