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

Imre Deak
2014-12-05T11:42:24Z

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