Re: [PATCH] drm/i915/chv: Enable HDMI Clock recovery for Pipe C

Jani Nikula
2014-12-05T11:57:44Z

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

--
Jani Nikula, Intel Open Source Technology Center