diff mbox

drm/i915/chv: Enable HDMI Clock recovery for Pipe C

Message ID 1417630230-3171-1-git-send-email-clinton.a.taylor@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Taylor, Clinton A Dec. 3, 2014, 6:10 p.m. UTC
From: Clint Taylor <clinton.a.taylor@intel.com>

Added PIPE C register support for CHV audio programming.

Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Ville Syrjälä Dec. 3, 2014, 9:01 p.m. UTC | #1
On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> Added PIPE C register support for CHV audio programming.

nak. The offset between the pipes looks constant so it should work
just fine with _PIPE().

> 
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc03fac..3d5813a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>  
>  #define _VLV_HDMIW_HDMIEDID_A		(VLV_DISPLAY_BASE + 0x62050)
>  #define _VLV_HDMIW_HDMIEDID_B		(VLV_DISPLAY_BASE + 0x62150)
> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
> +#define _CHV_HDMIW_HDMIEDID_C		(VLV_DISPLAY_BASE + 0x62250)
> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>  					_VLV_HDMIW_HDMIEDID_A, \
> -					_VLV_HDMIW_HDMIEDID_B)
> +					_VLV_HDMIW_HDMIEDID_B, \
> +					_CHV_HDMIW_HDMIEDID_C)
>  #define _VLV_AUD_CNTL_ST_A		(VLV_DISPLAY_BASE + 0x620B4)
>  #define _VLV_AUD_CNTL_ST_B		(VLV_DISPLAY_BASE + 0x621B4)
> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
> +#define _CHV_AUD_CNTL_ST_C		(VLV_DISPLAY_BASE + 0x622B4)
> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>  					_VLV_AUD_CNTL_ST_A, \
> -					_VLV_AUD_CNTL_ST_B)
> +					_VLV_AUD_CNTL_ST_B, \
> +					_CHV_AUD_CNTL_ST_C)
>  #define VLV_AUD_CNTL_ST2		(VLV_DISPLAY_BASE + 0x620C0)
>  
>  /* These are the 4 32-bit write offset registers for each stream
> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>  					_CPT_AUD_CONFIG_B)
>  #define _VLV_AUD_CONFIG_A		(VLV_DISPLAY_BASE + 0x62000)
>  #define _VLV_AUD_CONFIG_B		(VLV_DISPLAY_BASE + 0x62100)
> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
> +#define _CHV_AUD_CONFIG_C		(VLV_DISPLAY_BASE + 0x62200)
> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>  					_VLV_AUD_CONFIG_A, \
> -					_VLV_AUD_CONFIG_B)
> +					_VLV_AUD_CONFIG_B, \
> +					_CHV_AUD_CONFIG_C)
>  
>  #define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
>  #define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Taylor, Clinton A Dec. 3, 2014, 9:09 p.m. UTC | #2
On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> Added PIPE C register support for CHV audio programming.
>
> nak. The offset between the pipes looks constant so it should work
> just fine with _PIPE().

Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after 
ChromeOS boot.

-Clint

>
>>
>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index dc03fac..3d5813a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>
>>   #define _VLV_HDMIW_HDMIEDID_A		(VLV_DISPLAY_BASE + 0x62050)
>>   #define _VLV_HDMIW_HDMIEDID_B		(VLV_DISPLAY_BASE + 0x62150)
>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>> +#define _CHV_HDMIW_HDMIEDID_C		(VLV_DISPLAY_BASE + 0x62250)
>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>   					_VLV_HDMIW_HDMIEDID_A, \
>> -					_VLV_HDMIW_HDMIEDID_B)
>> +					_VLV_HDMIW_HDMIEDID_B, \
>> +					_CHV_HDMIW_HDMIEDID_C)
>>   #define _VLV_AUD_CNTL_ST_A		(VLV_DISPLAY_BASE + 0x620B4)
>>   #define _VLV_AUD_CNTL_ST_B		(VLV_DISPLAY_BASE + 0x621B4)
>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>> +#define _CHV_AUD_CNTL_ST_C		(VLV_DISPLAY_BASE + 0x622B4)
>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>   					_VLV_AUD_CNTL_ST_A, \
>> -					_VLV_AUD_CNTL_ST_B)
>> +					_VLV_AUD_CNTL_ST_B, \
>> +					_CHV_AUD_CNTL_ST_C)
>>   #define VLV_AUD_CNTL_ST2		(VLV_DISPLAY_BASE + 0x620C0)
>>
>>   /* These are the 4 32-bit write offset registers for each stream
>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>   					_CPT_AUD_CONFIG_B)
>>   #define _VLV_AUD_CONFIG_A		(VLV_DISPLAY_BASE + 0x62000)
>>   #define _VLV_AUD_CONFIG_B		(VLV_DISPLAY_BASE + 0x62100)
>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>> +#define _CHV_AUD_CONFIG_C		(VLV_DISPLAY_BASE + 0x62200)
>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>   					_VLV_AUD_CONFIG_A, \
>> -					_VLV_AUD_CONFIG_B)
>> +					_VLV_AUD_CONFIG_B, \
>> +					_CHV_AUD_CONFIG_C)
>>
>>   #define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
>>   #define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Jani Nikula Dec. 4, 2014, 8:41 a.m. UTC | #3
On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com wrote:
>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>
>>> Added PIPE C register support for CHV audio programming.
>>
>> nak. The offset between the pipes looks constant so it should work
>> just fine with _PIPE().
>
> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after 
> ChromeOS boot.

"after boot" is a long time! ;)

Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
called? With drm.debug=14 you should see "Disable audio codec on port
%c, pipe %c\n".

BR,
Jani.




>
> -Clint
>
>>
>>>
>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index dc03fac..3d5813a 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>>
>>>   #define _VLV_HDMIW_HDMIEDID_A		(VLV_DISPLAY_BASE + 0x62050)
>>>   #define _VLV_HDMIW_HDMIEDID_B		(VLV_DISPLAY_BASE + 0x62150)
>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>>> +#define _CHV_HDMIW_HDMIEDID_C		(VLV_DISPLAY_BASE + 0x62250)
>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>>   					_VLV_HDMIW_HDMIEDID_A, \
>>> -					_VLV_HDMIW_HDMIEDID_B)
>>> +					_VLV_HDMIW_HDMIEDID_B, \
>>> +					_CHV_HDMIW_HDMIEDID_C)
>>>   #define _VLV_AUD_CNTL_ST_A		(VLV_DISPLAY_BASE + 0x620B4)
>>>   #define _VLV_AUD_CNTL_ST_B		(VLV_DISPLAY_BASE + 0x621B4)
>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>>> +#define _CHV_AUD_CNTL_ST_C		(VLV_DISPLAY_BASE + 0x622B4)
>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>>   					_VLV_AUD_CNTL_ST_A, \
>>> -					_VLV_AUD_CNTL_ST_B)
>>> +					_VLV_AUD_CNTL_ST_B, \
>>> +					_CHV_AUD_CNTL_ST_C)
>>>   #define VLV_AUD_CNTL_ST2		(VLV_DISPLAY_BASE + 0x620C0)
>>>
>>>   /* These are the 4 32-bit write offset registers for each stream
>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>>   					_CPT_AUD_CONFIG_B)
>>>   #define _VLV_AUD_CONFIG_A		(VLV_DISPLAY_BASE + 0x62000)
>>>   #define _VLV_AUD_CONFIG_B		(VLV_DISPLAY_BASE + 0x62100)
>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>>> +#define _CHV_AUD_CONFIG_C		(VLV_DISPLAY_BASE + 0x62200)
>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>>   					_VLV_AUD_CONFIG_A, \
>>> -					_VLV_AUD_CONFIG_B)
>>> +					_VLV_AUD_CONFIG_B, \
>>> +					_CHV_AUD_CONFIG_C)
>>>
>>>   #define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
>>>   #define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
>>> --
>>> 1.7.9.5
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Taylor, Clinton A Dec. 4, 2014, 7:08 p.m. UTC | #4
On 12/04/2014 12:41 AM, Jani Nikula wrote:
> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com wrote:
>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>
>>>> Added PIPE C register support for CHV audio programming.
>>>
>>> nak. The offset between the pipes looks constant so it should work
>>> just fine with _PIPE().
>>
>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
>> ChromeOS boot.
>
> "after boot" is a long time! ;)
>
> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
> called? With drm.debug=14 you should see "Disable audio codec on port
> %c, pipe %c\n".
>
> BR,
> Jani.
>

I've actually instrumented the driver with more logging and at 22.43 in 
the dmesg log the last call to audio_codec_enable() is called and is 
setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for 
1080p60. Sometime after 22.436 and before the ChromeOS login screen 
appears the register is being cleared to 0x0 without knowledge/action of 
the i915 driver. Mode change and power state change seem to restore the 
programming correctly.

  Most likely candidate right now is an ordering issue with snd_hda 
during boot.

[    8.607022] [drm:intel_audio_codec_enable] ELD on 
[CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
[    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port 
D, pipe C, 8 bytes ELD
[    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
[   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port 
D, pipe C
[   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
[   15.773618] [drm:intel_audio_codec_enable] ELD on 
[CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
[   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port 
D, pipe C, 8 bytes ELD
[   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
[   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port 
D, pipe C
[   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
[   20.292920] [drm:intel_audio_codec_enable] ELD on 
[CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
[   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port 
D, pipe C, 8 bytes ELD
[   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
[   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port 
D, pipe C
[   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
[   22.430931] [drm:intel_audio_codec_enable] ELD on 
[CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
[   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port 
D, pipe C, 8 bytes ELD
[   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000

>
>
>
>>
>> -Clint
>>
>>>
>>>>
>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>> index dc03fac..3d5813a 100644
>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>>>
>>>>    #define _VLV_HDMIW_HDMIEDID_A		(VLV_DISPLAY_BASE + 0x62050)
>>>>    #define _VLV_HDMIW_HDMIEDID_B		(VLV_DISPLAY_BASE + 0x62150)
>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>>>> +#define _CHV_HDMIW_HDMIEDID_C		(VLV_DISPLAY_BASE + 0x62250)
>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>>>    					_VLV_HDMIW_HDMIEDID_A, \
>>>> -					_VLV_HDMIW_HDMIEDID_B)
>>>> +					_VLV_HDMIW_HDMIEDID_B, \
>>>> +					_CHV_HDMIW_HDMIEDID_C)
>>>>    #define _VLV_AUD_CNTL_ST_A		(VLV_DISPLAY_BASE + 0x620B4)
>>>>    #define _VLV_AUD_CNTL_ST_B		(VLV_DISPLAY_BASE + 0x621B4)
>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>>>> +#define _CHV_AUD_CNTL_ST_C		(VLV_DISPLAY_BASE + 0x622B4)
>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>>>    					_VLV_AUD_CNTL_ST_A, \
>>>> -					_VLV_AUD_CNTL_ST_B)
>>>> +					_VLV_AUD_CNTL_ST_B, \
>>>> +					_CHV_AUD_CNTL_ST_C)
>>>>    #define VLV_AUD_CNTL_ST2		(VLV_DISPLAY_BASE + 0x620C0)
>>>>
>>>>    /* These are the 4 32-bit write offset registers for each stream
>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>>>    					_CPT_AUD_CONFIG_B)
>>>>    #define _VLV_AUD_CONFIG_A		(VLV_DISPLAY_BASE + 0x62000)
>>>>    #define _VLV_AUD_CONFIG_B		(VLV_DISPLAY_BASE + 0x62100)
>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>>>> +#define _CHV_AUD_CONFIG_C		(VLV_DISPLAY_BASE + 0x62200)
>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>>>    					_VLV_AUD_CONFIG_A, \
>>>> -					_VLV_AUD_CONFIG_B)
>>>> +					_VLV_AUD_CONFIG_B, \
>>>> +					_CHV_AUD_CONFIG_C)
>>>>
>>>>    #define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
>>>>    #define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)
>>>> --
>>>> 1.7.9.5
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Taylor, Clinton A Dec. 4, 2014, 10:21 p.m. UTC | #5
On 12/04/2014 11:08 AM, Clint Taylor wrote:
> On 12/04/2014 12:41 AM, Jani Nikula wrote:
>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
>>>> wrote:
>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>
>>>>> Added PIPE C register support for CHV audio programming.
>>>>
>>>> nak. The offset between the pipes looks constant so it should work
>>>> just fine with _PIPE().
>>>
>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
>>> ChromeOS boot.
>>
>> "after boot" is a long time! ;)
>>
>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
>> called? With drm.debug=14 you should see "Disable audio codec on port
>> %c, pipe %c\n".
>>
>> BR,
>> Jani.
>>
>
> I've actually instrumented the driver with more logging and at 22.43 in
> the dmesg log the last call to audio_codec_enable() is called and is
> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
> appears the register is being cleared to 0x0 without knowledge/action of
> the i915 driver. Mode change and power state change seem to restore the
> programming correctly.
>
>   Most likely candidate right now is an ordering issue with snd_hda
> during boot.
>

Ok, removing snd_hda drivers from the boot process did not change the 
result. Any ideas about what could reset the HD_AUDIO registers to their 
default?

-clint


> [    8.607022] [drm:intel_audio_codec_enable] ELD on
> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
> D, pipe C, 8 bytes ELD
> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
> D, pipe C
> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> [   15.773618] [drm:intel_audio_codec_enable] ELD on
> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
> D, pipe C, 8 bytes ELD
> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
> D, pipe C
> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> [   20.292920] [drm:intel_audio_codec_enable] ELD on
> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
> D, pipe C, 8 bytes ELD
> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
> D, pipe C
> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> [   22.430931] [drm:intel_audio_codec_enable] ELD on
> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
> D, pipe C, 8 bytes ELD
> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>
>>
>>
>>
>>>
>>> -Clint
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>>> index dc03fac..3d5813a 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>>>>
>>>>>    #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
>>>>>    #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>>>>                        _VLV_HDMIW_HDMIEDID_A, \
>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
>>>>>    #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
>>>>>    #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>>>>                        _VLV_AUD_CNTL_ST_A, \
>>>>> -                    _VLV_AUD_CNTL_ST_B)
>>>>> +                    _VLV_AUD_CNTL_ST_B, \
>>>>> +                    _CHV_AUD_CNTL_ST_C)
>>>>>    #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
>>>>>
>>>>>    /* These are the 4 32-bit write offset registers for each stream
>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>>>>                        _CPT_AUD_CONFIG_B)
>>>>>    #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
>>>>>    #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>>>>                        _VLV_AUD_CONFIG_A, \
>>>>> -                    _VLV_AUD_CONFIG_B)
>>>>> +                    _VLV_AUD_CONFIG_B, \
>>>>> +                    _CHV_AUD_CONFIG_C)
>>>>>
>>>>>    #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
>>>>>    #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Dec. 5, 2014, 8:37 a.m. UTC | #6
On Fri, 05 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> On 12/04/2014 11:08 AM, Clint Taylor wrote:
>> On 12/04/2014 12:41 AM, Jani Nikula wrote:
>>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
>>>>> wrote:
>>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>>
>>>>>> Added PIPE C register support for CHV audio programming.
>>>>>
>>>>> nak. The offset between the pipes looks constant so it should work
>>>>> just fine with _PIPE().
>>>>
>>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
>>>> ChromeOS boot.
>>>
>>> "after boot" is a long time! ;)
>>>
>>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
>>> called? With drm.debug=14 you should see "Disable audio codec on port
>>> %c, pipe %c\n".
>>>
>>> BR,
>>> Jani.
>>>
>>
>> I've actually instrumented the driver with more logging and at 22.43 in
>> the dmesg log the last call to audio_codec_enable() is called and is
>> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
>> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
>> appears the register is being cleared to 0x0 without knowledge/action of
>> the i915 driver. Mode change and power state change seem to restore the
>> programming correctly.
>>
>>   Most likely candidate right now is an ordering issue with snd_hda
>> during boot.
>>
>
> Ok, removing snd_hda drivers from the boot process did not change the 
> result. Any ideas about what could reset the HD_AUDIO registers to their 
> default?

Maybe it needs some power domain/well we fail to get? Similar to what
DDI based platforms do before and after calls to
intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c.

Not really my area, maybe Imre and Ville know better?


BR,
Jani.



>
> -clint
>
>
>> [    8.607022] [drm:intel_audio_codec_enable] ELD on
>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> D, pipe C, 8 bytes ELD
>> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> D, pipe C
>> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> [   15.773618] [drm:intel_audio_codec_enable] ELD on
>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> D, pipe C, 8 bytes ELD
>> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> D, pipe C
>> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> [   20.292920] [drm:intel_audio_codec_enable] ELD on
>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> D, pipe C, 8 bytes ELD
>> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> D, pipe C
>> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> [   22.430931] [drm:intel_audio_codec_enable] ELD on
>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> D, pipe C, 8 bytes ELD
>> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>>
>>>
>>>
>>>
>>>>
>>>> -Clint
>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> index dc03fac..3d5813a 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>>>>>
>>>>>>    #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
>>>>>>    #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
>>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
>>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>>>>>                        _VLV_HDMIW_HDMIEDID_A, \
>>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
>>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
>>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
>>>>>>    #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
>>>>>>    #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
>>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
>>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>>>>>                        _VLV_AUD_CNTL_ST_A, \
>>>>>> -                    _VLV_AUD_CNTL_ST_B)
>>>>>> +                    _VLV_AUD_CNTL_ST_B, \
>>>>>> +                    _CHV_AUD_CNTL_ST_C)
>>>>>>    #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
>>>>>>
>>>>>>    /* These are the 4 32-bit write offset registers for each stream
>>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>>>>>                        _CPT_AUD_CONFIG_B)
>>>>>>    #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
>>>>>>    #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
>>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
>>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>>>>>                        _VLV_AUD_CONFIG_A, \
>>>>>> -                    _VLV_AUD_CONFIG_B)
>>>>>> +                    _VLV_AUD_CONFIG_B, \
>>>>>> +                    _CHV_AUD_CONFIG_C)
>>>>>>
>>>>>>    #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
>>>>>>    #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>>> _______________________________________________
>>>>>> Intel-gfx mailing list
>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Imre Deak Dec. 5, 2014, 11:42 a.m. UTC | #7
On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote:
> On Fri, 05 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> > On 12/04/2014 11:08 AM, Clint Taylor wrote:
> >> On 12/04/2014 12:41 AM, Jani Nikula wrote:
> >>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> >>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
> >>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
> >>>>> wrote:
> >>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
> >>>>>>
> >>>>>> Added PIPE C register support for CHV audio programming.
> >>>>>
> >>>>> nak. The offset between the pipes looks constant so it should work
> >>>>> just fine with _PIPE().
> >>>>
> >>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
> >>>> ChromeOS boot.
> >>>
> >>> "after boot" is a long time! ;)
> >>>
> >>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
> >>> called? With drm.debug=14 you should see "Disable audio codec on port
> >>> %c, pipe %c\n".
> >>>
> >>> BR,
> >>> Jani.
> >>>
> >>
> >> I've actually instrumented the driver with more logging and at 22.43 in
> >> the dmesg log the last call to audio_codec_enable() is called and is
> >> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
> >> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
> >> appears the register is being cleared to 0x0 without knowledge/action of
> >> the i915 driver. Mode change and power state change seem to restore the
> >> programming correctly.
> >>
> >>   Most likely candidate right now is an ordering issue with snd_hda
> >> during boot.
> >>
> >
> > Ok, removing snd_hda drivers from the boot process did not change the 
> > result. Any ideas about what could reset the HD_AUDIO registers to their 
> > default?
> 
> Maybe it needs some power domain/well we fail to get? Similar to what
> DDI based platforms do before and after calls to
> intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c.
> 
> Not really my area, maybe Imre and Ville know better?

The audio power domain->power well mapping is mapped correctly in the
i915 driver (HDA belongs to the display aka pipe-A power well there).
But AFAICS the intel_hda driver doesn't ask for this power domain on
VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda
device capability flags. CC'ing Takashi for this.

Still, I'm not sure why the register gets zeroed, if there is no power
well off->on transition. You could first check if this is really true,
via the debug print intel_display_power_get() and
intel_display_power_put() emits.

Another idea would be to trace writes to this register through
I915_WRITE.

--Imre

> 
> 
> BR,
> Jani.
> 
> 
> 
> >
> > -clint
> >
> >
> >> [    8.607022] [drm:intel_audio_codec_enable] ELD on
> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> D, pipe C, 8 bytes ELD
> >> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> D, pipe C
> >> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> [   15.773618] [drm:intel_audio_codec_enable] ELD on
> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> D, pipe C, 8 bytes ELD
> >> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> D, pipe C
> >> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> [   20.292920] [drm:intel_audio_codec_enable] ELD on
> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> D, pipe C, 8 bytes ELD
> >> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> D, pipe C
> >> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> [   22.430931] [drm:intel_audio_codec_enable] ELD on
> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> D, pipe C, 8 bytes ELD
> >> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >>
> >>>
> >>>
> >>>
> >>>>
> >>>> -Clint
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >>>>>> ---
> >>>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
> >>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >>>>>> b/drivers/gpu/drm/i915/i915_reg.h
> >>>>>> index dc03fac..3d5813a 100644
> >>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
> >>>>>>
> >>>>>>    #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
> >>>>>>    #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
> >>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
> >>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
> >>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
> >>>>>>                        _VLV_HDMIW_HDMIEDID_A, \
> >>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
> >>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
> >>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
> >>>>>>    #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
> >>>>>>    #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
> >>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
> >>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
> >>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
> >>>>>>                        _VLV_AUD_CNTL_ST_A, \
> >>>>>> -                    _VLV_AUD_CNTL_ST_B)
> >>>>>> +                    _VLV_AUD_CNTL_ST_B, \
> >>>>>> +                    _CHV_AUD_CNTL_ST_C)
> >>>>>>    #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
> >>>>>>
> >>>>>>    /* These are the 4 32-bit write offset registers for each stream
> >>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
> >>>>>>                        _CPT_AUD_CONFIG_B)
> >>>>>>    #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
> >>>>>>    #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
> >>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
> >>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
> >>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
> >>>>>>                        _VLV_AUD_CONFIG_A, \
> >>>>>> -                    _VLV_AUD_CONFIG_B)
> >>>>>> +                    _VLV_AUD_CONFIG_B, \
> >>>>>> +                    _CHV_AUD_CONFIG_C)
> >>>>>>
> >>>>>>    #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
> >>>>>>    #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
> >>>>>> --
> >>>>>> 1.7.9.5
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Intel-gfx mailing list
> >>>>>> Intel-gfx@lists.freedesktop.org
> >>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>>>
> >>>>
> >>>> _______________________________________________
> >>>> Intel-gfx mailing list
> >>>> Intel-gfx@lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>>
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
Jani Nikula Dec. 5, 2014, 11:57 a.m. UTC | #8
On Fri, 05 Dec 2014, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote:
>> On Fri, 05 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>> > On 12/04/2014 11:08 AM, Clint Taylor wrote:
>> >> On 12/04/2014 12:41 AM, Jani Nikula wrote:
>> >>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>> >>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>> >>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
>> >>>>> wrote:
>> >>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>> >>>>>>
>> >>>>>> Added PIPE C register support for CHV audio programming.
>> >>>>>
>> >>>>> nak. The offset between the pipes looks constant so it should work
>> >>>>> just fine with _PIPE().
>> >>>>
>> >>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
>> >>>> ChromeOS boot.
>> >>>
>> >>> "after boot" is a long time! ;)
>> >>>
>> >>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
>> >>> called? With drm.debug=14 you should see "Disable audio codec on port
>> >>> %c, pipe %c\n".
>> >>>
>> >>> BR,
>> >>> Jani.
>> >>>
>> >>
>> >> I've actually instrumented the driver with more logging and at 22.43 in
>> >> the dmesg log the last call to audio_codec_enable() is called and is
>> >> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
>> >> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
>> >> appears the register is being cleared to 0x0 without knowledge/action of
>> >> the i915 driver. Mode change and power state change seem to restore the
>> >> programming correctly.
>> >>
>> >>   Most likely candidate right now is an ordering issue with snd_hda
>> >> during boot.
>> >>
>> >
>> > Ok, removing snd_hda drivers from the boot process did not change the 
>> > result. Any ideas about what could reset the HD_AUDIO registers to their 
>> > default?
>> 
>> Maybe it needs some power domain/well we fail to get? Similar to what
>> DDI based platforms do before and after calls to
>> intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c.
>> 
>> Not really my area, maybe Imre and Ville know better?
>
> The audio power domain->power well mapping is mapped correctly in the
> i915 driver (HDA belongs to the display aka pipe-A power well there).
> But AFAICS the intel_hda driver doesn't ask for this power domain on
> VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda
> device capability flags. CC'ing Takashi for this.

Imre, i915_request_power_well() only works for hsw/bdw.

BR,
Jani.

>
> Still, I'm not sure why the register gets zeroed, if there is no power
> well off->on transition. You could first check if this is really true,
> via the debug print intel_display_power_get() and
> intel_display_power_put() emits.
>
> Another idea would be to trace writes to this register through
> I915_WRITE.
>
> --Imre
>
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>> >
>> > -clint
>> >
>> >
>> >> [    8.607022] [drm:intel_audio_codec_enable] ELD on
>> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> >> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> >> D, pipe C, 8 bytes ELD
>> >> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> >> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> >> D, pipe C
>> >> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> >> [   15.773618] [drm:intel_audio_codec_enable] ELD on
>> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> >> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> >> D, pipe C, 8 bytes ELD
>> >> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> >> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> >> D, pipe C
>> >> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> >> [   20.292920] [drm:intel_audio_codec_enable] ELD on
>> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> >> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> >> D, pipe C, 8 bytes ELD
>> >> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> >> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
>> >> D, pipe C
>> >> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>> >> [   22.430931] [drm:intel_audio_codec_enable] ELD on
>> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>> >> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
>> >> D, pipe C, 8 bytes ELD
>> >> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>> >>
>> >>>
>> >>>
>> >>>
>> >>>>
>> >>>> -Clint
>> >>>>
>> >>>>>
>> >>>>>>
>> >>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>> >>>>>> ---
>> >>>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>> >>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>> >>>>>>
>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>> >>>>>> index dc03fac..3d5813a 100644
>> >>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> >>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>> >>>>>>
>> >>>>>>    #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
>> >>>>>>    #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
>> >>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>> >>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
>> >>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>> >>>>>>                        _VLV_HDMIW_HDMIEDID_A, \
>> >>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
>> >>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
>> >>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
>> >>>>>>    #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
>> >>>>>>    #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
>> >>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>> >>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
>> >>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>> >>>>>>                        _VLV_AUD_CNTL_ST_A, \
>> >>>>>> -                    _VLV_AUD_CNTL_ST_B)
>> >>>>>> +                    _VLV_AUD_CNTL_ST_B, \
>> >>>>>> +                    _CHV_AUD_CNTL_ST_C)
>> >>>>>>    #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
>> >>>>>>
>> >>>>>>    /* These are the 4 32-bit write offset registers for each stream
>> >>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>> >>>>>>                        _CPT_AUD_CONFIG_B)
>> >>>>>>    #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
>> >>>>>>    #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
>> >>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>> >>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
>> >>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>> >>>>>>                        _VLV_AUD_CONFIG_A, \
>> >>>>>> -                    _VLV_AUD_CONFIG_B)
>> >>>>>> +                    _VLV_AUD_CONFIG_B, \
>> >>>>>> +                    _CHV_AUD_CONFIG_C)
>> >>>>>>
>> >>>>>>    #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
>> >>>>>>    #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
>> >>>>>> --
>> >>>>>> 1.7.9.5
>> >>>>>>
>> >>>>>> _______________________________________________
>> >>>>>> Intel-gfx mailing list
>> >>>>>> Intel-gfx@lists.freedesktop.org
>> >>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>>>>
>> >>>>
>> >>>> _______________________________________________
>> >>>> Intel-gfx mailing list
>> >>>> Intel-gfx@lists.freedesktop.org
>> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>>
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> 
>
>
Imre Deak Dec. 5, 2014, 1:23 p.m. UTC | #9
On Fri, 2014-12-05 at 13:57 +0200, Jani Nikula wrote:
> On Fri, 05 Dec 2014, Imre Deak <imre.deak@intel.com> wrote:
> > On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote:
> >> On Fri, 05 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> >> > On 12/04/2014 11:08 AM, Clint Taylor wrote:
> >> >> On 12/04/2014 12:41 AM, Jani Nikula wrote:
> >> >>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
> >> >>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
> >> >>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
> >> >>>>> wrote:
> >> >>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
> >> >>>>>>
> >> >>>>>> Added PIPE C register support for CHV audio programming.
> >> >>>>>
> >> >>>>> nak. The offset between the pipes looks constant so it should work
> >> >>>>> just fine with _PIPE().
> >> >>>>
> >> >>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
> >> >>>> ChromeOS boot.
> >> >>>
> >> >>> "after boot" is a long time! ;)
> >> >>>
> >> >>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
> >> >>> called? With drm.debug=14 you should see "Disable audio codec on port
> >> >>> %c, pipe %c\n".
> >> >>>
> >> >>> BR,
> >> >>> Jani.
> >> >>>
> >> >>
> >> >> I've actually instrumented the driver with more logging and at 22.43 in
> >> >> the dmesg log the last call to audio_codec_enable() is called and is
> >> >> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
> >> >> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
> >> >> appears the register is being cleared to 0x0 without knowledge/action of
> >> >> the i915 driver. Mode change and power state change seem to restore the
> >> >> programming correctly.
> >> >>
> >> >>   Most likely candidate right now is an ordering issue with snd_hda
> >> >> during boot.
> >> >>
> >> >
> >> > Ok, removing snd_hda drivers from the boot process did not change the 
> >> > result. Any ideas about what could reset the HD_AUDIO registers to their 
> >> > default?
> >> 
> >> Maybe it needs some power domain/well we fail to get? Similar to what
> >> DDI based platforms do before and after calls to
> >> intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c.
> >> 
> >> Not really my area, maybe Imre and Ville know better?
> >
> > The audio power domain->power well mapping is mapped correctly in the
> > i915 driver (HDA belongs to the display aka pipe-A power well there).
> > But AFAICS the intel_hda driver doesn't ask for this power domain on
> > VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda
> > device capability flags. CC'ing Takashi for this.
> 
> Imre, i915_request_power_well() only works for hsw/bdw.

Hm, right I should have checked that first. And it could be also the
reason why AZX_DCAPS_I915_POWERWELL was left out for everything but
HSW/BDW. I will send a patch to fix that in i915.

Thanks,
Imre

> 
> BR,
> Jani.
> 
> >
> > Still, I'm not sure why the register gets zeroed, if there is no power
> > well off->on transition. You could first check if this is really true,
> > via the debug print intel_display_power_get() and
> > intel_display_power_put() emits.
> >
> > Another idea would be to trace writes to this register through
> > I915_WRITE.
> >
> > --Imre
> >
> >> 
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> 
> >> >
> >> > -clint
> >> >
> >> >
> >> >> [    8.607022] [drm:intel_audio_codec_enable] ELD on
> >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> >> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> >> D, pipe C, 8 bytes ELD
> >> >> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> >> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> >> D, pipe C
> >> >> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> >> [   15.773618] [drm:intel_audio_codec_enable] ELD on
> >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> >> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> >> D, pipe C, 8 bytes ELD
> >> >> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> >> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> >> D, pipe C
> >> >> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> >> [   20.292920] [drm:intel_audio_codec_enable] ELD on
> >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> >> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> >> D, pipe C, 8 bytes ELD
> >> >> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> >> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
> >> >> D, pipe C
> >> >> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
> >> >> [   22.430931] [drm:intel_audio_codec_enable] ELD on
> >> >> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
> >> >> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
> >> >> D, pipe C, 8 bytes ELD
> >> >> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
> >> >>
> >> >>>
> >> >>>
> >> >>>
> >> >>>>
> >> >>>> -Clint
> >> >>>>
> >> >>>>>
> >> >>>>>>
> >> >>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> >> >>>>>> ---
> >> >>>>>>    drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
> >> >>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
> >> >>>>>>
> >> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> >>>>>> b/drivers/gpu/drm/i915/i915_reg.h
> >> >>>>>> index dc03fac..3d5813a 100644
> >> >>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> >>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> >>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
> >> >>>>>>
> >> >>>>>>    #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
> >> >>>>>>    #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
> >> >>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
> >> >>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
> >> >>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
> >> >>>>>>                        _VLV_HDMIW_HDMIEDID_A, \
> >> >>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
> >> >>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
> >> >>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
> >> >>>>>>    #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
> >> >>>>>>    #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
> >> >>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
> >> >>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
> >> >>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
> >> >>>>>>                        _VLV_AUD_CNTL_ST_A, \
> >> >>>>>> -                    _VLV_AUD_CNTL_ST_B)
> >> >>>>>> +                    _VLV_AUD_CNTL_ST_B, \
> >> >>>>>> +                    _CHV_AUD_CNTL_ST_C)
> >> >>>>>>    #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
> >> >>>>>>
> >> >>>>>>    /* These are the 4 32-bit write offset registers for each stream
> >> >>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
> >> >>>>>>                        _CPT_AUD_CONFIG_B)
> >> >>>>>>    #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
> >> >>>>>>    #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
> >> >>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
> >> >>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
> >> >>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
> >> >>>>>>                        _VLV_AUD_CONFIG_A, \
> >> >>>>>> -                    _VLV_AUD_CONFIG_B)
> >> >>>>>> +                    _VLV_AUD_CONFIG_B, \
> >> >>>>>> +                    _CHV_AUD_CONFIG_C)
> >> >>>>>>
> >> >>>>>>    #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
> >> >>>>>>    #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
> >> >>>>>> --
> >> >>>>>> 1.7.9.5
> >> >>>>>>
> >> >>>>>> _______________________________________________
> >> >>>>>> Intel-gfx mailing list
> >> >>>>>> Intel-gfx@lists.freedesktop.org
> >> >>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >>>>>
> >> >>>>
> >> >>>> _______________________________________________
> >> >>>> Intel-gfx mailing list
> >> >>>> Intel-gfx@lists.freedesktop.org
> >> >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >>>
> >> >>
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> 
> >
> >
>
Daniel Vetter Dec. 5, 2014, 2:27 p.m. UTC | #10
On Fri, Dec 05, 2014 at 03:23:59PM +0200, Imre Deak wrote:
> Hm, right I should have checked that first. And it could be also the
> reason why AZX_DCAPS_I915_POWERWELL was left out for everything but
> HSW/BDW. I will send a patch to fix that in i915.

Not terribly in favour of perpetuing that hack. It's proven sufficient
trouble imo already.
-Daniel
Taylor, Clinton A Dec. 5, 2014, 11:30 p.m. UTC | #11
On 12/05/2014 05:23 AM, Imre Deak wrote:
> On Fri, 2014-12-05 at 13:57 +0200, Jani Nikula wrote:
>> On Fri, 05 Dec 2014, Imre Deak <imre.deak@intel.com> wrote:
>>> On Fri, 2014-12-05 at 10:37 +0200, Jani Nikula wrote:
>>>> On Fri, 05 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>>>>> On 12/04/2014 11:08 AM, Clint Taylor wrote:
>>>>>> On 12/04/2014 12:41 AM, Jani Nikula wrote:
>>>>>>> On Wed, 03 Dec 2014, Clint Taylor <clinton.a.taylor@intel.com> wrote:
>>>>>>>> On 12/03/2014 01:01 PM, Ville Syrjälä wrote:
>>>>>>>>> On Wed, Dec 03, 2014 at 10:10:30AM -0800, clinton.a.taylor@intel.com
>>>>>>>>> wrote:
>>>>>>>>>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>>>>>>
>>>>>>>>>> Added PIPE C register support for CHV audio programming.
>>>>>>>>>
>>>>>>>>> nak. The offset between the pipes looks constant so it should work
>>>>>>>>> just fine with _PIPE().
>>>>>>>>
>>>>>>>> Correct. Now I just need to figure out why AUD_CONFIG_C is cleared after
>>>>>>>> ChromeOS boot.
>>>>>>>
>>>>>>> "after boot" is a long time! ;)
>>>>>>>
>>>>>>> Do your logs show that ilk_audio_codec_disable (see intel_audio.c) gets
>>>>>>> called? With drm.debug=14 you should see "Disable audio codec on port
>>>>>>> %c, pipe %c\n".
>>>>>>>
>>>>>>> BR,
>>>>>>> Jani.
>>>>>>>
>>>>>>
>>>>>> I've actually instrumented the driver with more logging and at 22.43 in
>>>>>> the dmesg log the last call to audio_codec_enable() is called and is
>>>>>> setting the Pixel_Clock bits of AUD_CONFIG_C to 0x9 which is correct for
>>>>>> 1080p60. Sometime after 22.436 and before the ChromeOS login screen
>>>>>> appears the register is being cleared to 0x0 without knowledge/action of
>>>>>> the i915 driver. Mode change and power state change seem to restore the
>>>>>> programming correctly.
>>>>>>
>>>>>>    Most likely candidate right now is an ordering issue with snd_hda
>>>>>> during boot.
>>>>>>
>>>>>
>>>>> Ok, removing snd_hda drivers from the boot process did not change the
>>>>> result. Any ideas about what could reset the HD_AUDIO registers to their
>>>>> default?
>>>>
>>>> Maybe it needs some power domain/well we fail to get? Similar to what
>>>> DDI based platforms do before and after calls to
>>>> intel_audio_codec_enable and intel_audio_codec_disable in intel_ddi.c.
>>>>
>>>> Not really my area, maybe Imre and Ville know better?
>>>
>>> The audio power domain->power well mapping is mapped correctly in the
>>> i915 driver (HDA belongs to the display aka pipe-A power well there).
>>> But AFAICS the intel_hda driver doesn't ask for this power domain on
>>> VLV/CHV, AZX_DCAPS_I915_POWERWELL is not included in the relevant hda
>>> device capability flags. CC'ing Takashi for this.
>>
>> Imre, i915_request_power_well() only works for hsw/bdw.
>
> Hm, right I should have checked that first. And it could be also the
> reason why AZX_DCAPS_I915_POWERWELL was left out for everything but
> HSW/BDW. I will send a patch to fix that in i915.
>
> Thanks,
> Imre
>
I have applied Imre's request_power_well patch, and tried setting 
intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO) directly when the 
audio codec is enabled and the register is still not accepting pixel 
clock programming updates during boot.

I have found that the register will not accept the value until after 
snd_hda_intel initializes the PCI device. Unfortunately time wise this 
occurs after the last attempt made by the i915 driver.

I will continue to investigate next week.

[   25.993784]  *** CAT display power get
[   25.998016]  *** CAT enable aud_config_c read = 0x00000000

[   31.825282] snd_hda_intel 0000:00:1b.0: irq 403 for MSI/MSI-X

-Clint


>>
>> BR,
>> Jani.
>>
>>>
>>> Still, I'm not sure why the register gets zeroed, if there is no power
>>> well off->on transition. You could first check if this is really true,
>>> via the debug print intel_display_power_get() and
>>> intel_display_power_put() emits.
>>>
>>> Another idea would be to trace writes to this register through
>>> I915_WRITE.
>>>
>>> --Imre
>>>
>>>>
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>
>>>>>
>>>>> -clint
>>>>>
>>>>>
>>>>>> [    8.607022] [drm:intel_audio_codec_enable] ELD on
>>>>>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>>>>>> [    8.607027] [drm:ilk_audio_codec_enable] Enable audio codec on port
>>>>>> D, pipe C, 8 bytes ELD
>>>>>> [    8.607044]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>>>>>> [   14.741047] [drm:ilk_audio_codec_disable] Disable audio codec on port
>>>>>> D, pipe C
>>>>>> [   14.741055]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>>>>>> [   15.773618] [drm:intel_audio_codec_enable] ELD on
>>>>>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>>>>>> [   15.773624] [drm:ilk_audio_codec_enable] Enable audio codec on port
>>>>>> D, pipe C, 8 bytes ELD
>>>>>> [   15.779315]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>>>>>> [   19.936767] [drm:ilk_audio_codec_disable] Disable audio codec on port
>>>>>> D, pipe C
>>>>>> [   19.936778]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>>>>>> [   20.292920] [drm:intel_audio_codec_enable] ELD on
>>>>>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>>>>>> [   20.292928] [drm:ilk_audio_codec_enable] Enable audio codec on port
>>>>>> D, pipe C, 8 bytes ELD
>>>>>> [   20.298615]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>>>>>> [   20.389110] [drm:ilk_audio_codec_disable] Disable audio codec on port
>>>>>> D, pipe C
>>>>>> [   20.389121]  *** CAT disable aud_config reg 0x001E2200 = 0x10000000
>>>>>> [   22.430931] [drm:intel_audio_codec_enable] ELD on
>>>>>> [CONNECTOR:37:HDMI-A-3], [ENCODER:36:TMDS-36]
>>>>>> [   22.430940] [drm:ilk_audio_codec_enable] Enable audio codec on port
>>>>>> D, pipe C, 8 bytes ELD
>>>>>> [   22.436627]  *** CAT enable aud_config reg 0x001E2200 = 0x00090000
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> -Clint
>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>     drivers/gpu/drm/i915/i915_reg.h |   18 ++++++++++++------
>>>>>>>>>>     1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>>>>>>>>> b/drivers/gpu/drm/i915/i915_reg.h
>>>>>>>>>> index dc03fac..3d5813a 100644
>>>>>>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>>>>>>> @@ -6189,14 +6189,18 @@ enum punit_power_well {
>>>>>>>>>>
>>>>>>>>>>     #define _VLV_HDMIW_HDMIEDID_A        (VLV_DISPLAY_BASE + 0x62050)
>>>>>>>>>>     #define _VLV_HDMIW_HDMIEDID_B        (VLV_DISPLAY_BASE + 0x62150)
>>>>>>>>>> -#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
>>>>>>>>>> +#define _CHV_HDMIW_HDMIEDID_C        (VLV_DISPLAY_BASE + 0x62250)
>>>>>>>>>> +#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
>>>>>>>>>>                         _VLV_HDMIW_HDMIEDID_A, \
>>>>>>>>>> -                    _VLV_HDMIW_HDMIEDID_B)
>>>>>>>>>> +                    _VLV_HDMIW_HDMIEDID_B, \
>>>>>>>>>> +                    _CHV_HDMIW_HDMIEDID_C)
>>>>>>>>>>     #define _VLV_AUD_CNTL_ST_A        (VLV_DISPLAY_BASE + 0x620B4)
>>>>>>>>>>     #define _VLV_AUD_CNTL_ST_B        (VLV_DISPLAY_BASE + 0x621B4)
>>>>>>>>>> -#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
>>>>>>>>>> +#define _CHV_AUD_CNTL_ST_C        (VLV_DISPLAY_BASE + 0x622B4)
>>>>>>>>>> +#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
>>>>>>>>>>                         _VLV_AUD_CNTL_ST_A, \
>>>>>>>>>> -                    _VLV_AUD_CNTL_ST_B)
>>>>>>>>>> +                    _VLV_AUD_CNTL_ST_B, \
>>>>>>>>>> +                    _CHV_AUD_CNTL_ST_C)
>>>>>>>>>>     #define VLV_AUD_CNTL_ST2        (VLV_DISPLAY_BASE + 0x620C0)
>>>>>>>>>>
>>>>>>>>>>     /* These are the 4 32-bit write offset registers for each stream
>>>>>>>>>> @@ -6217,9 +6221,11 @@ enum punit_power_well {
>>>>>>>>>>                         _CPT_AUD_CONFIG_B)
>>>>>>>>>>     #define _VLV_AUD_CONFIG_A        (VLV_DISPLAY_BASE + 0x62000)
>>>>>>>>>>     #define _VLV_AUD_CONFIG_B        (VLV_DISPLAY_BASE + 0x62100)
>>>>>>>>>> -#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
>>>>>>>>>> +#define _CHV_AUD_CONFIG_C        (VLV_DISPLAY_BASE + 0x62200)
>>>>>>>>>> +#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
>>>>>>>>>>                         _VLV_AUD_CONFIG_A, \
>>>>>>>>>> -                    _VLV_AUD_CONFIG_B)
>>>>>>>>>> +                    _VLV_AUD_CONFIG_B, \
>>>>>>>>>> +                    _CHV_AUD_CONFIG_C)
>>>>>>>>>>
>>>>>>>>>>     #define   AUD_CONFIG_N_VALUE_INDEX        (1 << 29)
>>>>>>>>>>     #define   AUD_CONFIG_N_PROG_ENABLE        (1 << 28)
>>>>>>>>>> --
>>>>>>>>>> 1.7.9.5
>>>>>>>>>>
>>>>>>>>>> _______________________________________________
>>>>>>>>>> Intel-gfx mailing list
>>>>>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Intel-gfx mailing list
>>>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Intel-gfx mailing list
>>>>>> Intel-gfx@lists.freedesktop.org
>>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>>>
>>>>
>>>
>>>
>>
>
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc03fac..3d5813a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6189,14 +6189,18 @@  enum punit_power_well {
 
 #define _VLV_HDMIW_HDMIEDID_A		(VLV_DISPLAY_BASE + 0x62050)
 #define _VLV_HDMIW_HDMIEDID_B		(VLV_DISPLAY_BASE + 0x62150)
-#define VLV_HDMIW_HDMIEDID(pipe) _PIPE(pipe, \
+#define _CHV_HDMIW_HDMIEDID_C		(VLV_DISPLAY_BASE + 0x62250)
+#define VLV_HDMIW_HDMIEDID(pipe) _PIPE3(pipe, \
 					_VLV_HDMIW_HDMIEDID_A, \
-					_VLV_HDMIW_HDMIEDID_B)
+					_VLV_HDMIW_HDMIEDID_B, \
+					_CHV_HDMIW_HDMIEDID_C)
 #define _VLV_AUD_CNTL_ST_A		(VLV_DISPLAY_BASE + 0x620B4)
 #define _VLV_AUD_CNTL_ST_B		(VLV_DISPLAY_BASE + 0x621B4)
-#define VLV_AUD_CNTL_ST(pipe) _PIPE(pipe, \
+#define _CHV_AUD_CNTL_ST_C		(VLV_DISPLAY_BASE + 0x622B4)
+#define VLV_AUD_CNTL_ST(pipe) _PIPE3(pipe, \
 					_VLV_AUD_CNTL_ST_A, \
-					_VLV_AUD_CNTL_ST_B)
+					_VLV_AUD_CNTL_ST_B, \
+					_CHV_AUD_CNTL_ST_C)
 #define VLV_AUD_CNTL_ST2		(VLV_DISPLAY_BASE + 0x620C0)
 
 /* These are the 4 32-bit write offset registers for each stream
@@ -6217,9 +6221,11 @@  enum punit_power_well {
 					_CPT_AUD_CONFIG_B)
 #define _VLV_AUD_CONFIG_A		(VLV_DISPLAY_BASE + 0x62000)
 #define _VLV_AUD_CONFIG_B		(VLV_DISPLAY_BASE + 0x62100)
-#define VLV_AUD_CFG(pipe) _PIPE(pipe, \
+#define _CHV_AUD_CONFIG_C		(VLV_DISPLAY_BASE + 0x62200)
+#define VLV_AUD_CFG(pipe) _PIPE3(pipe, \
 					_VLV_AUD_CONFIG_A, \
-					_VLV_AUD_CONFIG_B)
+					_VLV_AUD_CONFIG_B, \
+					_CHV_AUD_CONFIG_C)
 
 #define   AUD_CONFIG_N_VALUE_INDEX		(1 << 29)
 #define   AUD_CONFIG_N_PROG_ENABLE		(1 << 28)