Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2017-01-05 Thread Pandiyan, Dhinakaran
On Wed, 2017-01-04 at 11:42 +0100, Peter Frühberger wrote:
> Forgot to CC the list, sorry.
> 
> On Wed, Jan 4, 2017 at 11:42 AM, Peter Frühberger 
> wrote:
> Hi Jani,
> thanks for your reply
> 
> On Wed, Jan 4, 2017 at 10:34 AM, Jani Nikula
>  wrote:
> On Wed, 04 Jan 2017, Peter Frühberger
>  wrote:
> > Hi
> >
> > On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran
> <
> > dhinakaran.pandi...@intel.com> wrote:
> >
> >> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula
> wrote:
> >> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
> >> dhinakaran.pandi...@intel.com> wrote:
> >> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula
> wrote:
> >> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
> >> dhinakaran.pandi...@intel.com> wrote:
> >> > >> > Enabling DP audio stall fix is necessary to
> play audio over DP
> >> HBR2. So,
> >> > >> > let's set this bit right before enabling the
> audio codec. Playing
> >> audio
> >> > >> > without setting this bit results in pipe
> FIFO underruns.
> >> > >> >
> >> > >> > This workaround is applicable only for audio
> sample rates up to
> >> 96kHz. For
> >> > >> > frequencies above 96kHz, this is
> insufficient and cdclk should be
> >> increased
> >> > >> > to at least 432 MHz, just like BDW. Since,
> the audio driver does not
> >> > >> > support sample rates > 48 kHz, we are safe
> with this fix for now.
> >> > >>
> >> > >> Do we still need this patch now that these two
> have been pushed?
> >> > >>
> >> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP
> audio
> >> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio
> workaround to GEN9
> >> platforms
> >> > >>
> >> > >> BR,
> >> > >> Jani.
> >> > >>
> >> > >>
> >> > >>
> >> > >
> >> > > No, we are good afaik. This patch would have
> helped us to make use of a
> >> > > lower cdclk (337.5 MHz), with constraints on
> audio bit rate. Operating
> >> > > at 432 MHz, like we do now, rules out the need
> for this patch.
> >> >
> >> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> >> >
> >> > BR,
> >> > Jani.
> >> >
> >>
> >> Good point, I think it will depend on the audio
> sampling rate. But, I
> >> have to figure out a way to play high sampling rate
> audio (> 96 KHz) and
> >> test 5.4 Gbps with 1 or 2 lanes.
> >>
> >> The other option is to play safe and apply this
> patch with even lesser
> >> restrictions, say link rate >= 2.7 Gbps.
> >>
> >>
> >> -DK
> >>
> >
> > as we are currently talking about high samplerates
> in this context. I
> > wanted to post a perhaps related issue. On my Apollo
> Lake (J4205) I have
> > two outputs. One DVI and one HDMI 2.0 via internal
> DP. Via DVI the
> > following works without issues, via DP it fails. As
> the original commit
> > mentions HBR, I think there is still something
> missing. We submit TrueHD,
> > DTS-HD via 192 khz and 16 bit format while setting
> AES0=2
> >
> > You can easily reproduce with (you obviously need a
> DTS-HD, TrueHD capable
> > AVR attached to your HDMI 2.0 (DP) out):
> 
+Libin


> Just to clarify, is the DP -> HDMI2.0 converter
> internal to the machine?
> LSPCON related messages in the dmesg with
> drm.debug=14? Do you have a DP
> or an HDMI physical connector in the chassis?
> 
> 
> The chip used is: https://media.digikey.com/pdf/Data%
> 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2017-01-04 Thread Peter Frühberger
Forgot to CC the list, sorry.

On Wed, Jan 4, 2017 at 11:42 AM, Peter Frühberger  wrote:

> Hi Jani,
> thanks for your reply
>
> On Wed, Jan 4, 2017 at 10:34 AM, Jani Nikula 
> wrote:
>
>> On Wed, 04 Jan 2017, Peter Frühberger  wrote:
>> > Hi
>> >
>> > On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
>> > dhinakaran.pandi...@intel.com> wrote:
>> >
>> >> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
>> >> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>> >> dhinakaran.pandi...@intel.com> wrote:
>> >> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> >> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>> >> dhinakaran.pandi...@intel.com> wrote:
>> >> > >> > Enabling DP audio stall fix is necessary to play audio over DP
>> >> HBR2. So,
>> >> > >> > let's set this bit right before enabling the audio codec.
>> Playing
>> >> audio
>> >> > >> > without setting this bit results in pipe FIFO underruns.
>> >> > >> >
>> >> > >> > This workaround is applicable only for audio sample rates up to
>> >> 96kHz. For
>> >> > >> > frequencies above 96kHz, this is insufficient and cdclk should
>> be
>> >> increased
>> >> > >> > to at least 432 MHz, just like BDW. Since, the audio driver
>> does not
>> >> > >> > support sample rates > 48 kHz, we are safe with this fix for
>> now.
>> >> > >>
>> >> > >> Do we still need this patch now that these two have been pushed?
>> >> > >>
>> >> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> >> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
>> >> platforms
>> >> > >>
>> >> > >> BR,
>> >> > >> Jani.
>> >> > >>
>> >> > >>
>> >> > >>
>> >> > >
>> >> > > No, we are good afaik. This patch would have helped us to make use
>> of a
>> >> > > lower cdclk (337.5 MHz), with constraints on audio bit rate.
>> Operating
>> >> > > at 432 MHz, like we do now, rules out the need for this patch.
>> >> >
>> >> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>> >> >
>> >> > BR,
>> >> > Jani.
>> >> >
>> >>
>> >> Good point, I think it will depend on the audio sampling rate. But, I
>> >> have to figure out a way to play high sampling rate audio (> 96 KHz)
>> and
>> >> test 5.4 Gbps with 1 or 2 lanes.
>> >>
>> >> The other option is to play safe and apply this patch with even lesser
>> >> restrictions, say link rate >= 2.7 Gbps.
>> >>
>> >>
>> >> -DK
>> >>
>> >
>> > as we are currently talking about high samplerates in this context. I
>> > wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
>> > two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
>> > following works without issues, via DP it fails. As the original commit
>> > mentions HBR, I think there is still something missing. We submit
>> TrueHD,
>> > DTS-HD via 192 khz and 16 bit format while setting AES0=2
>> >
>> > You can easily reproduce with (you obviously need a DTS-HD, TrueHD
>> capable
>> > AVR attached to your HDMI 2.0 (DP) out):
>>
>> Just to clarify, is the DP -> HDMI2.0 converter internal to the machine?
>> LSPCON related messages in the dmesg with drm.debug=14? Do you have a DP
>> or an HDMI physical connector in the chassis?
>>
>
> The chip used is: https://media.digikey.com/pdf/Data%20Sheets/MegaChips%
> 20PDFs/MCDP28x0_Datasheet.pdf which is the same on all intel nucs,
> including the new Kabilake ones. So it's internal.
> Mainboard: http://www.asrock.com/MB/Intel/J4205-ITX/index.us.asp (DVI-D,
> HDMI (2.0 via this above chip), VGA)
> Here is the output of the a boot up with drm.debug=14:
> http://paste.ubuntu.com/23738282/
> (I used a pastebin site to not spam the ML, is that okay for the future?)
>
> Best regards
> Peter
>
>
>>
>>
>> BR,
>> Jani.
>>
>> >
>> > #TrueHD
>> > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
>> > testi.truehd.anssi1.ff.60s.spdif
>> > #DTS-HD
>> > aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
>> > testi.dtshd.anssi1.ma-71-24.spdif
>> > Samples:
>> > http://www.avenard.org/files/media/mediatest/audiotest/HDAUD
>> IO/Passthrough/
>> >
>> > For the old HDMI 1.x chips it was fixed via:
>> > https://bugs.freedesktop.org/show_bug.cgi?id=49055
>> >
>> > Is this also planned for DP within that patch series?
>> >
>> > Best regards
>> > Peter
>> >
>> >>
>> >> > >
>> >> > > -DK
>> >> > >
>> >> > >> >
>> >> > >> > v2: Inlined the code change within hsw_audio_codec_enable()
>> (Jani)
>> >> > >> > Fixed the port clock typo
>> >> > >> > Added TODO comment
>> >> > >> > Signed-off-by: Dhinakaran Pandiyan <
>> dhinakaran.pandi...@intel.com>
>> >> > >> > ---
>> >> > >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
>> >> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
>> >> +-
>> >> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >> > >> >
>> >> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >> b/drivers/gpu/drm/i915/i915_reg.h
>> >> > >> > index 00efaa1..76dac48 100644
>> >> > >> > --- 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2017-01-04 Thread Jani Nikula
On Wed, 04 Jan 2017, Peter Frühberger  wrote:
> Hi
>
> On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
> dhinakaran.pandi...@intel.com> wrote:
>
>> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
>> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
>> dhinakaran.pandi...@intel.com> wrote:
>> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
>> dhinakaran.pandi...@intel.com> wrote:
>> > >> > Enabling DP audio stall fix is necessary to play audio over DP
>> HBR2. So,
>> > >> > let's set this bit right before enabling the audio codec. Playing
>> audio
>> > >> > without setting this bit results in pipe FIFO underruns.
>> > >> >
>> > >> > This workaround is applicable only for audio sample rates up to
>> 96kHz. For
>> > >> > frequencies above 96kHz, this is insufficient and cdclk should be
>> increased
>> > >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > >> > support sample rates > 48 kHz, we are safe with this fix for now.
>> > >>
>> > >> Do we still need this patch now that these two have been pushed?
>> > >>
>> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
>> platforms
>> > >>
>> > >> BR,
>> > >> Jani.
>> > >>
>> > >>
>> > >>
>> > >
>> > > No, we are good afaik. This patch would have helped us to make use of a
>> > > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
>> > > at 432 MHz, like we do now, rules out the need for this patch.
>> >
>> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
>> >
>> > BR,
>> > Jani.
>> >
>>
>> Good point, I think it will depend on the audio sampling rate. But, I
>> have to figure out a way to play high sampling rate audio (> 96 KHz) and
>> test 5.4 Gbps with 1 or 2 lanes.
>>
>> The other option is to play safe and apply this patch with even lesser
>> restrictions, say link rate >= 2.7 Gbps.
>>
>>
>> -DK
>>
>
> as we are currently talking about high samplerates in this context. I
> wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
> two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
> following works without issues, via DP it fails. As the original commit
> mentions HBR, I think there is still something missing. We submit TrueHD,
> DTS-HD via 192 khz and 16 bit format while setting AES0=2
>
> You can easily reproduce with (you obviously need a DTS-HD, TrueHD capable
> AVR attached to your HDMI 2.0 (DP) out):

Just to clarify, is the DP -> HDMI2.0 converter internal to the machine?
LSPCON related messages in the dmesg with drm.debug=14? Do you have a DP
or an HDMI physical connector in the chassis?


BR,
Jani.

>
> #TrueHD
> aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
> testi.truehd.anssi1.ff.60s.spdif
> #DTS-HD
> aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
> testi.dtshd.anssi1.ma-71-24.spdif
> Samples:
> http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/
>
> For the old HDMI 1.x chips it was fixed via:
> https://bugs.freedesktop.org/show_bug.cgi?id=49055
>
> Is this also planned for DP within that patch series?
>
> Best regards
> Peter
>
>>
>> > >
>> > > -DK
>> > >
>> > >> >
>> > >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> > >> > Fixed the port clock typo
>> > >> > Added TODO comment
>> > >> > Signed-off-by: Dhinakaran Pandiyan 
>> > >> > ---
>> > >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
>> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
>> +-
>> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> > >> >
>> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> > >> > index 00efaa1..76dac48 100644
>> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > >> > @@ -6236,6 +6236,11 @@ enum {
>> > >> >  #define SLICE_ECO_CHICKEN0  _MMIO(0x7308)
>> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE(1 << 14)
>> > >> >
>> > >> > +#define _CHICKEN_TRANS_A0x420C0
>> > >> > +#define _CHICKEN_TRANS_B0x420C4
>> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
>> _CHICKEN_TRANS_B)
>> > >> > +#define SPARE_13(1<<13)
>> > >> > +
>> > >> >  /* WaCatErrorRejectionIssue */
>> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG  _MMIO(0x9030)
>> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB   (1<<11)
>> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> > >> > index 7093cfb..894f11e 100644
>> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
>> intel_encoder *encoder)
>> > >> >  {
>> > >> >  struct drm_i915_private *dev_priv =
>> 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2017-01-04 Thread Peter Frühberger
Hi

On Sun, Nov 6, 2016 at 1:23 AM, Pandiyan, Dhinakaran <
dhinakaran.pandi...@intel.com> wrote:

> On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
> > On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran" <
> dhinakaran.pandi...@intel.com> wrote:
> > > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> > >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan <
> dhinakaran.pandi...@intel.com> wrote:
> > >> > Enabling DP audio stall fix is necessary to play audio over DP
> HBR2. So,
> > >> > let's set this bit right before enabling the audio codec. Playing
> audio
> > >> > without setting this bit results in pipe FIFO underruns.
> > >> >
> > >> > This workaround is applicable only for audio sample rates up to
> 96kHz. For
> > >> > frequencies above 96kHz, this is insufficient and cdclk should be
> increased
> > >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > >> > support sample rates > 48 kHz, we are safe with this fix for now.
> > >>
> > >> Do we still need this patch now that these two have been pushed?
> > >>
> > >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> > >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9
> platforms
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >>
> > >
> > > No, we are good afaik. This patch would have helped us to make use of a
> > > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> > > at 432 MHz, like we do now, rules out the need for this patch.
> >
> > Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> >
> > BR,
> > Jani.
> >
>
> Good point, I think it will depend on the audio sampling rate. But, I
> have to figure out a way to play high sampling rate audio (> 96 KHz) and
> test 5.4 Gbps with 1 or 2 lanes.
>
> The other option is to play safe and apply this patch with even lesser
> restrictions, say link rate >= 2.7 Gbps.
>
>
> -DK
>

as we are currently talking about high samplerates in this context. I
wanted to post a perhaps related issue. On my Apollo Lake (J4205) I have
two outputs. One DVI and one HDMI 2.0 via internal DP. Via DVI the
following works without issues, via DP it fails. As the original commit
mentions HBR, I think there is still something missing. We submit TrueHD,
DTS-HD via 192 khz and 16 bit format while setting AES0=2

You can easily reproduce with (you obviously need a DTS-HD, TrueHD capable
AVR attached to your HDMI 2.0 (DP) out):

#TrueHD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.truehd.anssi1.ff.60s.spdif
#DTS-HD
aplay -D 'hdmi:CARD=PCH,DEV=0,AES0=2' -c8 -fs16_le -r192000
testi.dtshd.anssi1.ma-71-24.spdif
Samples:
http://www.avenard.org/files/media/mediatest/audiotest/HDAUDIO/Passthrough/

For the old HDMI 1.x chips it was fixed via:
https://bugs.freedesktop.org/show_bug.cgi?id=49055

Is this also planned for DP within that patch series?

Best regards
Peter

>
> > >
> > > -DK
> > >
> > >> >
> > >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > >> > Fixed the port clock typo
> > >> > Added TODO comment
> > >> > Signed-off-by: Dhinakaran Pandiyan 
> > >> > ---
> > >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> > >> >  drivers/gpu/drm/i915/intel_audio.c | 30
> +-
> > >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > >> > index 00efaa1..76dac48 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > >> > @@ -6236,6 +6236,11 @@ enum {
> > >> >  #define SLICE_ECO_CHICKEN0  _MMIO(0x7308)
> > >> >  #define   PIXEL_MASK_CAMMING_DISABLE(1 << 14)
> > >> >
> > >> > +#define _CHICKEN_TRANS_A0x420C0
> > >> > +#define _CHICKEN_TRANS_B0x420C4
> > >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A,
> _CHICKEN_TRANS_B)
> > >> > +#define SPARE_13(1<<13)
> > >> > +
> > >> >  /* WaCatErrorRejectionIssue */
> > >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG  _MMIO(0x9030)
> > >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB   (1<<11)
> > >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > >> > index 7093cfb..894f11e 100644
> > >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
> intel_encoder *encoder)
> > >> >  {
> > >> >  struct drm_i915_private *dev_priv =
> to_i915(encoder->base.dev);
> > >> >  struct intel_crtc *intel_crtc =
> to_intel_crtc(encoder->base.crtc);
> > >> > +struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > >> > +enum transcoder cpu_transcoder =
> crtc_config->cpu_transcoder;
> > >> >  enum pipe pipe = intel_crtc->pipe;
> > >> >  uint32_t tmp;
> > >> >
> > >> > @@ -290,13 +292,21 @@ static void 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-11-05 Thread Pandiyan, Dhinakaran
On Sat, 2016-11-05 at 21:40 +0200, Jani Nikula wrote:
> On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran"  
> wrote:
> > On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
> >> wrote:
> >> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> >> > let's set this bit right before enabling the audio codec. Playing audio
> >> > without setting this bit results in pipe FIFO underruns.
> >> >
> >> > This workaround is applicable only for audio sample rates up to 96kHz. 
> >> > For
> >> > frequencies above 96kHz, this is insufficient and cdclk should be 
> >> > increased
> >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> >> > support sample rates > 48 kHz, we are safe with this fix for now.
> >> 
> >> Do we still need this patch now that these two have been pushed?
> >> 
> >> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> >> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> 
> >
> > No, we are good afaik. This patch would have helped us to make use of a
> > lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> > at 432 MHz, like we do now, rules out the need for this patch.
> 
> Hmm, what about 5.4 Gbps link with 1 or 2 lanes?
> 
> BR,
> Jani.
> 

Good point, I think it will depend on the audio sampling rate. But, I
have to figure out a way to play high sampling rate audio (> 96 KHz) and
test 5.4 Gbps with 1 or 2 lanes.

The other option is to play safe and apply this patch with even lesser
restrictions, say link rate >= 2.7 Gbps.


-DK

> >
> > -DK
> >
> >> >
> >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >> > Fixed the port clock typo
> >> > Added TODO comment
> >> > Signed-off-by: Dhinakaran Pandiyan 
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> >> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> >> > b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 00efaa1..76dac48 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -6236,6 +6236,11 @@ enum {
> >> >  #define SLICE_ECO_CHICKEN0  _MMIO(0x7308)
> >> >  #define   PIXEL_MASK_CAMMING_DISABLE(1 << 14)
> >> >  
> >> > +#define _CHICKEN_TRANS_A0x420C0
> >> > +#define _CHICKEN_TRANS_B0x420C4
> >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> >> > _CHICKEN_TRANS_B)
> >> > +#define SPARE_13(1<<13)
> >> > +
> >> >  /* WaCatErrorRejectionIssue */
> >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG  _MMIO(0x9030)
> >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB   (1<<11)
> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> >> > b/drivers/gpu/drm/i915/intel_audio.c
> >> > index 7093cfb..894f11e 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> >> > intel_encoder *encoder)
> >> >  {
> >> >  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> >  struct intel_crtc *intel_crtc = 
> >> > to_intel_crtc(encoder->base.crtc);
> >> > +struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  enum pipe pipe = intel_crtc->pipe;
> >> >  uint32_t tmp;
> >> >  
> >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> >> > intel_encoder *encoder)
> >> >  
> >> >  mutex_lock(_priv->av_mutex);
> >> >  
> >> > +/*Disable DP audio stall fix for HBR2*/
> >> > +if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) 
> >> > &&
> >> > +crtc_config->port_clock >= 54) {
> >> > +tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +tmp &= ~SPARE_13;
> >> > +I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +}
> >> > +
> >> >  /* Disable timestamps */
> >> >  tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >  tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >> >  tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> >  tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >> >  tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> >> > -if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >> > +if (intel_crtc_has_dp_encoder(crtc_config))
> >> >  tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >  I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >  
> >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct 
> >> > drm_connector *connector,
> >> >  {
> >> >  struct 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-11-05 Thread Jani Nikula
On Fri, 04 Nov 2016, "Pandiyan, Dhinakaran"  
wrote:
> On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
>> wrote:
>> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
>> > let's set this bit right before enabling the audio codec. Playing audio
>> > without setting this bit results in pipe FIFO underruns.
>> >
>> > This workaround is applicable only for audio sample rates up to 96kHz. For
>> > frequencies above 96kHz, this is insufficient and cdclk should be increased
>> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > support sample rates > 48 kHz, we are safe with this fix for now.
>> 
>> Do we still need this patch now that these two have been pushed?
>> 
>> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
>> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>
> No, we are good afaik. This patch would have helped us to make use of a
> lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
> at 432 MHz, like we do now, rules out the need for this patch.

Hmm, what about 5.4 Gbps link with 1 or 2 lanes?

BR,
Jani.


>
> -DK
>
>> >
>> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> > Fixed the port clock typo
>> > Added TODO comment
>> > Signed-off-by: Dhinakaran Pandiyan 
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
>> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
>> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> > b/drivers/gpu/drm/i915/i915_reg.h
>> > index 00efaa1..76dac48 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -6236,6 +6236,11 @@ enum {
>> >  #define SLICE_ECO_CHICKEN0_MMIO(0x7308)
>> >  #define   PIXEL_MASK_CAMMING_DISABLE  (1 << 14)
>> >  
>> > +#define _CHICKEN_TRANS_A  0x420C0
>> > +#define _CHICKEN_TRANS_B  0x420C4
>> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
>> > _CHICKEN_TRANS_B)
>> > +#define SPARE_13  (1<<13)
>> > +
>> >  /* WaCatErrorRejectionIssue */
>> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG_MMIO(0x9030)
>> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB (1<<11)
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7093cfb..894f11e 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
>> > intel_encoder *encoder)
>> >  {
>> >struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> > +  struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +  enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >enum pipe pipe = intel_crtc->pipe;
>> >uint32_t tmp;
>> >  
>> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
>> > intel_encoder *encoder)
>> >  
>> >mutex_lock(_priv->av_mutex);
>> >  
>> > +  /*Disable DP audio stall fix for HBR2*/
>> > +  if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +  crtc_config->port_clock >= 54) {
>> > +  tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +  tmp &= ~SPARE_13;
>> > +  I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +  }
>> > +
>> >/* Disable timestamps */
>> >tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> >tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> > -  if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> > +  if (intel_crtc_has_dp_encoder(crtc_config))
>> >tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >  
>> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct 
>> > drm_connector *connector,
>> >  {
>> >struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> >struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>> > +  struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +  enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >enum pipe pipe = intel_crtc->pipe;
>> >enum port port = intel_encoder->port;
>> >const uint8_t *eld = connector->eld;
>> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
>> > drm_connector *connector,
>> >  
>> >mutex_lock(_priv->av_mutex);
>> >  
>> > +  /* Enable DP audio stall fix for HBR2
>> > +   *
>> > +   * TODO: This workaround is applicable only for audio sample rates up
>> > +   * to 96kHz. For frequencies above 96kHz, this is insufficient and
>> > +   

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-11-04 Thread Pandiyan, Dhinakaran
On Fri, 2016-11-04 at 17:48 +0200, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
> wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> >
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> 
> Do we still need this patch now that these two have been pushed?
> 
> b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
> 9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms
> 
> BR,
> Jani.
> 
> 
> 

No, we are good afaik. This patch would have helped us to make use of a
lower cdclk (337.5 MHz), with constraints on audio bit rate. Operating
at 432 MHz, like we do now, rules out the need for this patch.

-DK

> >
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > Fixed the port clock typo
> > Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0 _MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE   (1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A   0x420C0
> > +#define _CHICKEN_TRANS_B   0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> > _CHICKEN_TRANS_B)
> > +#define SPARE_13   (1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG _MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB  (1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /*Disable DP audio stall fix for HBR2*/
> > +   if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +   crtc_config->port_clock >= 54) {
> > +   tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +   tmp &= ~SPARE_13;
> > +   I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +   }
> > +
> > /* Disable timestamps */
> > tmp = I915_READ(HSW_AUD_CFG(pipe));
> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -   if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +   if (intel_crtc_has_dp_encoder(crtc_config))
> > tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> > *connector,
> >  {
> > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > enum port port = intel_encoder->port;
> > const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
> > drm_connector *connector,
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /* Enable DP audio stall fix for HBR2
> > +*
> > +* TODO: This workaround is applicable only for audio sample rates up
> > +* to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +* cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +* the audio driver does not support sample rates > 48 kHz, we are safe
> > +* with this fix for now.
> > +*/
> > +
> > +   if 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-11-04 Thread Jani Nikula
On Wed, 26 Oct 2016, Dhinakaran Pandiyan  wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
>
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.

Do we still need this patch now that these two have been pushed?

b30ce9e0552a drm/i915/dp: BDW cdclk fix for DP audio
9c7540241885 drm/i915/dp: Extend BDW DP audio workaround to GEN9 platforms

BR,
Jani.



>
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> Fixed the port clock typo
> Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|  5 +
>  drivers/gpu/drm/i915/intel_audio.c | 30 +-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0   _MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE (1 << 14)
>  
> +#define _CHICKEN_TRANS_A 0x420C0
> +#define _CHICKEN_TRANS_B 0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> _CHICKEN_TRANS_B)
> +#define SPARE_13 (1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG   _MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder 
> *encoder)
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> intel_encoder *encoder)
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /*Disable DP audio stall fix for HBR2*/
> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp &= ~SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }
> +
>   /* Disable timestamps */
>   tmp = I915_READ(HSW_AUD_CFG(pipe));
>   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>   tmp |= AUD_CONFIG_N_PROG_ENABLE;
>   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(crtc_config))
>   tmp |= AUD_CONFIG_N_VALUE_INDEX;
>   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  {
>   struct drm_i915_private *dev_priv = to_i915(connector->dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   enum port port = intel_encoder->port;
>   const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /* Enable DP audio stall fix for HBR2
> +  *
> +  * TODO: This workaround is applicable only for audio sample rates up
> +  * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +  * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +  * the audio driver does not support sample rates > 48 kHz, we are safe
> +  * with this fix for now.
> +  */
> +
> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp |= SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }
> +
>   /* Enable audio presence detect, invalidate ELD */
>   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>   tmp |= 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-28 Thread Yang, Libin

> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> Pandiyan, Dhinakaran
> Sent: Thursday, October 27, 2016 2:14 AM
> To: ville.syrj...@linux.intel.com
> Cc: Nikula, Jani <jani.nik...@intel.com>; Kp, Jeeja <jeeja...@intel.com>;
> intel-gfx@lists.freedesktop.org; libin.y...@linux.intel.com
> Subject: Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix
> for gen9 platforms
> 
> On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> > On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > > Enabling DP audio stall fix is necessary to play audio over DP HBR2.
> > > So, let's set this bit right before enabling the audio codec.
> > > Playing audio without setting this bit results in pipe FIFO underruns.
> > >
> > > This workaround is applicable only for audio sample rates up to
> > > 96kHz. For frequencies above 96kHz, this is insufficient and cdclk
> > > should be increased to at least 432 MHz, just like BDW. Since, the
> > > audio driver does not support sample rates > 48 kHz, we are safe with
> this fix for now.
> > >
> > > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > > Fixed the port clock typo
> > > Added TODO comment
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> > >  drivers/gpu/drm/i915/intel_audio.c | 30
> > > +-
> > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index 00efaa1..76dac48 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6236,6 +6236,11 @@ enum {
> > >  #define SLICE_ECO_CHICKEN0   _MMIO(0x7308)
> > >  #define   PIXEL_MASK_CAMMING_DISABLE (1 << 14)
> > >
> > > +#define _CHICKEN_TRANS_A 0x420C0
> > > +#define _CHICKEN_TRANS_B 0x420C4
> > > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran,
> _CHICKEN_TRANS_A, _CHICKEN_TRANS_B)
> > > +#define SPARE_13 (1<<13)
> > > +
> > >  /* WaCatErrorRejectionIssue */
> > >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG
>   _MMIO(0x9030)
> > >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB(1<<11)
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > > b/drivers/gpu/drm/i915/intel_audio.c
> > > index 7093cfb..894f11e 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct
> > > intel_encoder *encoder)  {
> > >   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >   enum pipe pipe = intel_crtc->pipe;
> > >   uint32_t tmp;
> > >
> > > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct
> > > intel_encoder *encoder)
> > >
> > >   mutex_lock(_priv->av_mutex);
> > >
> > > + /*Disable DP audio stall fix for HBR2*/
> > > + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > + crtc_config->port_clock >= 54) {
> > > + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > + tmp &= ~SPARE_13;
> > > + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > + }
> > > +
> > >   /* Disable timestamps */
> > >   tmp = I915_READ(HSW_AUD_CFG(pipe));
> > >   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > >   tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > >   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > >   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > > - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > > + if (intel_crtc_has_dp_encoder(crtc_config))
> > >   tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > >   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >
> > > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct
> > > drm_connector *connector,  {
> > >   struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > >   struct intel_cr

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-27 Thread Pandiyan, Dhinakaran
On Wed, 2016-10-26 at 18:14 +, Pandiyan, Dhinakaran wrote:
> On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> > On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > > let's set this bit right before enabling the audio codec. Playing audio
> > > without setting this bit results in pipe FIFO underruns.
> > > 
> > > This workaround is applicable only for audio sample rates up to 96kHz. For
> > > frequencies above 96kHz, this is insufficient and cdclk should be 
> > > increased
> > > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > > support sample rates > 48 kHz, we are safe with this fix for now.
> > > 
> > > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > > Fixed the port clock typo
> > > Added TODO comment
> > > Signed-off-by: Dhinakaran Pandiyan 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> > >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> > >  2 files changed, 34 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 00efaa1..76dac48 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6236,6 +6236,11 @@ enum {
> > >  #define SLICE_ECO_CHICKEN0   _MMIO(0x7308)
> > >  #define   PIXEL_MASK_CAMMING_DISABLE (1 << 14)
> > >  
> > > +#define _CHICKEN_TRANS_A 0x420C0
> > > +#define _CHICKEN_TRANS_B 0x420C4
> > > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> > > _CHICKEN_TRANS_B)
> > > +#define SPARE_13 (1<<13)
> > > +
> > >  /* WaCatErrorRejectionIssue */
> > >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG   _MMIO(0x9030)
> > >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB(1<<11)
> > > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> > > b/drivers/gpu/drm/i915/intel_audio.c
> > > index 7093cfb..894f11e 100644
> > > --- a/drivers/gpu/drm/i915/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> > > intel_encoder *encoder)
> > >  {
> > >   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > > + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >   enum pipe pipe = intel_crtc->pipe;
> > >   uint32_t tmp;
> > >  
> > > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> > > intel_encoder *encoder)
> > >  
> > >   mutex_lock(_priv->av_mutex);
> > >  
> > > + /*Disable DP audio stall fix for HBR2*/
> > > + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > > + crtc_config->port_clock >= 54) {
> > > + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > > + tmp &= ~SPARE_13;
> > > + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > > + }
> > > +
> > >   /* Disable timestamps */
> > >   tmp = I915_READ(HSW_AUD_CFG(pipe));
> > >   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > >   tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > >   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > >   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > > - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > > + if (intel_crtc_has_dp_encoder(crtc_config))
> > >   tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > >   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> > >  
> > > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct 
> > > drm_connector *connector,
> > >  {
> > >   struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > >   struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > > + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > > + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > >   enum pipe pipe = intel_crtc->pipe;
> > >   enum port port = intel_encoder->port;
> > >   const uint8_t *eld = connector->eld;
> > > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
> > > drm_connector *connector,
> > >  
> > >   mutex_lock(_priv->av_mutex);
> > >  
> > > + /* Enable DP audio stall fix for HBR2
> > > +  *
> > > +  * TODO: This workaround is applicable only for audio sample rates up
> > > +  * to 96kHz. For frequencies above 96kHz, this is insufficient and
> > > +  * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > > +  * the audio driver does not support sample rates > 48 kHz, we are safe
> > > +  * with this fix for now.
> > 
> > Where in the sound driver is this supposed 96kHz limit? I see a lot of
> > stuff for >96kHz in the code at least.
> > 
> 
> Libin/Jeeja can you help me here?
> 
> 

I see another problem, we cannot change cdclk without a full modeset. So
choosing the workarounds based on audio sampling rate is 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-27 Thread Pandiyan, Dhinakaran
On Wed, 2016-10-26 at 12:11 +0300, Ville Syrjälä wrote:
> On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> > 
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> > 
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > Fixed the port clock typo
> > Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0 _MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE   (1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A   0x420C0
> > +#define _CHICKEN_TRANS_B   0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> > _CHICKEN_TRANS_B)
> > +#define SPARE_13   (1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG _MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB  (1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /*Disable DP audio stall fix for HBR2*/
> > +   if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +   crtc_config->port_clock >= 54) {
> > +   tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +   tmp &= ~SPARE_13;
> > +   I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +   }
> > +
> > /* Disable timestamps */
> > tmp = I915_READ(HSW_AUD_CFG(pipe));
> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -   if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +   if (intel_crtc_has_dp_encoder(crtc_config))
> > tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> > *connector,
> >  {
> > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > enum port port = intel_encoder->port;
> > const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
> > drm_connector *connector,
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /* Enable DP audio stall fix for HBR2
> > +*
> > +* TODO: This workaround is applicable only for audio sample rates up
> > +* to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +* cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +* the audio driver does not support sample rates > 48 kHz, we are safe
> > +* with this fix for now.
> 
> Where in the sound driver is this supposed 96kHz limit? I see a lot of
> stuff for >96kHz in the code at least.
> 

Libin/Jeeja can you help me here?


> > +*/
> > +
> > +   if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +   crtc_config->port_clock >= 54) {
> > +   tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +   tmp |= SPARE_13;
> > +   I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +   }
> > +
> > /* Enable audio 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-27 Thread Jani Nikula
On Wed, 26 Oct 2016, Dhinakaran Pandiyan  wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
>
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.
>
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> Fixed the port clock typo
> Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|  5 +
>  drivers/gpu/drm/i915/intel_audio.c | 30 +-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0   _MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE (1 << 14)
>  
> +#define _CHICKEN_TRANS_A 0x420C0
> +#define _CHICKEN_TRANS_B 0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> _CHICKEN_TRANS_B)
> +#define SPARE_13 (1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG   _MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder 
> *encoder)
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> intel_encoder *encoder)
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /*Disable DP audio stall fix for HBR2*/

Nitpick, spaces after /* and before */.

> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp &= ~SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }

Hmm. Why don't we just do this unconditionally?

> +
>   /* Disable timestamps */
>   tmp = I915_READ(HSW_AUD_CFG(pipe));
>   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>   tmp |= AUD_CONFIG_N_PROG_ENABLE;
>   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(crtc_config))
>   tmp |= AUD_CONFIG_N_VALUE_INDEX;
>   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  {
>   struct drm_i915_private *dev_priv = to_i915(connector->dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   enum port port = intel_encoder->port;
>   const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /* Enable DP audio stall fix for HBR2
> +  *
> +  * TODO: This workaround is applicable only for audio sample rates up
> +  * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +  * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +  * the audio driver does not support sample rates > 48 kHz, we are safe
> +  * with this fix for now.
> +  */

Is this TODO required if you already have that check in patch 2/2?

> +
> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp |= SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }
> +
>   /* Enable audio presence detect, invalidate ELD */
>   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>   tmp |= AUDIO_OUTPUT_ENABLE(pipe);

-- 
Jani Nikula, Intel Open Source 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-26 Thread Pandiyan, Dhinakaran
On Wed, 2016-10-26 at 22:06 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran"  
> wrote:
> > On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
> >> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
> >> wrote:
> >> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> >> > let's set this bit right before enabling the audio codec. Playing audio
> >> > without setting this bit results in pipe FIFO underruns.
> >> >
> >> > This workaround is applicable only for audio sample rates up to 96kHz. 
> >> > For
> >> > frequencies above 96kHz, this is insufficient and cdclk should be 
> >> > increased
> >> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> >> > support sample rates > 48 kHz, we are safe with this fix for now.
> >> >
> >> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> >> > Fixed the port clock typo
> >> > Added TODO comment
> >> > Signed-off-by: Dhinakaran Pandiyan 
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> >> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> >> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> >> > b/drivers/gpu/drm/i915/i915_reg.h
> >> > index 00efaa1..76dac48 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -6236,6 +6236,11 @@ enum {
> >> >  #define SLICE_ECO_CHICKEN0  _MMIO(0x7308)
> >> >  #define   PIXEL_MASK_CAMMING_DISABLE(1 << 14)
> >> >  
> >> > +#define _CHICKEN_TRANS_A0x420C0
> >> > +#define _CHICKEN_TRANS_B0x420C4
> >> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> >> > _CHICKEN_TRANS_B)
> >> > +#define SPARE_13(1<<13)
> >> > +
> >> >  /* WaCatErrorRejectionIssue */
> >> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG  _MMIO(0x9030)
> >> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB   (1<<11)
> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> >> > b/drivers/gpu/drm/i915/intel_audio.c
> >> > index 7093cfb..894f11e 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> >> > intel_encoder *encoder)
> >> >  {
> >> >  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >> >  struct intel_crtc *intel_crtc = 
> >> > to_intel_crtc(encoder->base.crtc);
> >> > +struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  enum pipe pipe = intel_crtc->pipe;
> >> >  uint32_t tmp;
> >> >  
> >> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> >> > intel_encoder *encoder)
> >> >  
> >> >  mutex_lock(_priv->av_mutex);
> >> >  
> >> > +/*Disable DP audio stall fix for HBR2*/
> >> 
> >> Nitpick, spaces after /* and before */.
> >> 
> >> > +if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) 
> >> > &&
> >> > +crtc_config->port_clock >= 54) {
> >> > +tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> >> > +tmp &= ~SPARE_13;
> >> > +I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> >> > +}
> >> 
> >> Hmm. Why don't we just do this unconditionally?
> >> 
> >
> > That bit is disabled by default, so avoiding  two MMIO ops. that are not
> > required.
> 
> That makes no difference on the modeset path.
> 

Because we do a lot of MMIO ops. anyway?


> >
> >> > +
> >> >  /* Disable timestamps */
> >> >  tmp = I915_READ(HSW_AUD_CFG(pipe));
> >> >  tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> >> >  tmp |= AUD_CONFIG_N_PROG_ENABLE;
> >> >  tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> >> >  tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> >> > -if (intel_crtc_has_dp_encoder(intel_crtc->config))
> >> > +if (intel_crtc_has_dp_encoder(crtc_config))
> >> >  tmp |= AUD_CONFIG_N_VALUE_INDEX;
> >> >  I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >> >  
> >> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct 
> >> > drm_connector *connector,
> >> >  {
> >> >  struct drm_i915_private *dev_priv = to_i915(connector->dev);
> >> >  struct intel_crtc *intel_crtc = 
> >> > to_intel_crtc(intel_encoder->base.crtc);
> >> > +struct intel_crtc_state *crtc_config =  intel_crtc->config;
> >> > +enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> >> >  enum pipe pipe = intel_crtc->pipe;
> >> >  enum port port = intel_encoder->port;
> >> >  const uint8_t *eld = connector->eld;
> >> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
> >> > drm_connector 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-26 Thread Jani Nikula
On Wed, 26 Oct 2016, "Pandiyan, Dhinakaran"  
wrote:
> On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
>> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
>> wrote:
>> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
>> > let's set this bit right before enabling the audio codec. Playing audio
>> > without setting this bit results in pipe FIFO underruns.
>> >
>> > This workaround is applicable only for audio sample rates up to 96kHz. For
>> > frequencies above 96kHz, this is insufficient and cdclk should be increased
>> > to at least 432 MHz, just like BDW. Since, the audio driver does not
>> > support sample rates > 48 kHz, we are safe with this fix for now.
>> >
>> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
>> > Fixed the port clock typo
>> > Added TODO comment
>> > Signed-off-by: Dhinakaran Pandiyan 
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
>> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
>> >  2 files changed, 34 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> > b/drivers/gpu/drm/i915/i915_reg.h
>> > index 00efaa1..76dac48 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -6236,6 +6236,11 @@ enum {
>> >  #define SLICE_ECO_CHICKEN0_MMIO(0x7308)
>> >  #define   PIXEL_MASK_CAMMING_DISABLE  (1 << 14)
>> >  
>> > +#define _CHICKEN_TRANS_A  0x420C0
>> > +#define _CHICKEN_TRANS_B  0x420C4
>> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
>> > _CHICKEN_TRANS_B)
>> > +#define SPARE_13  (1<<13)
>> > +
>> >  /* WaCatErrorRejectionIssue */
>> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG_MMIO(0x9030)
>> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB (1<<11)
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
>> > b/drivers/gpu/drm/i915/intel_audio.c
>> > index 7093cfb..894f11e 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
>> > intel_encoder *encoder)
>> >  {
>> >struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> >struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> > +  struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +  enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >enum pipe pipe = intel_crtc->pipe;
>> >uint32_t tmp;
>> >  
>> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
>> > intel_encoder *encoder)
>> >  
>> >mutex_lock(_priv->av_mutex);
>> >  
>> > +  /*Disable DP audio stall fix for HBR2*/
>> 
>> Nitpick, spaces after /* and before */.
>> 
>> > +  if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
>> > +  crtc_config->port_clock >= 54) {
>> > +  tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
>> > +  tmp &= ~SPARE_13;
>> > +  I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
>> > +  }
>> 
>> Hmm. Why don't we just do this unconditionally?
>> 
>
> That bit is disabled by default, so avoiding  two MMIO ops. that are not
> required.

That makes no difference on the modeset path.

>
>> > +
>> >/* Disable timestamps */
>> >tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> >tmp |= AUD_CONFIG_N_PROG_ENABLE;
>> >tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>> >tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>> > -  if (intel_crtc_has_dp_encoder(intel_crtc->config))
>> > +  if (intel_crtc_has_dp_encoder(crtc_config))
>> >tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>> >  
>> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct 
>> > drm_connector *connector,
>> >  {
>> >struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> >struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
>> > +  struct intel_crtc_state *crtc_config =  intel_crtc->config;
>> > +  enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>> >enum pipe pipe = intel_crtc->pipe;
>> >enum port port = intel_encoder->port;
>> >const uint8_t *eld = connector->eld;
>> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
>> > drm_connector *connector,
>> >  
>> >mutex_lock(_priv->av_mutex);
>> >  
>> > +  /* Enable DP audio stall fix for HBR2
>> > +   *
>> > +   * TODO: This workaround is applicable only for audio sample rates up
>> > +   * to 96kHz. For frequencies above 96kHz, this is insufficient and
>> > +   * cdclk should be increased to at least 432 MHz, just like BDW. Since,
>> > +   * the audio driver does not support sample rates > 48 kHz, we are safe
>> > +   * with this fix for now.
>> > +   */
>> 
>> Is this TODO required if you already have that check in patch 2/2?
>> 
>
> 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-26 Thread Pandiyan, Dhinakaran
On Wed, 2016-10-26 at 11:57 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Dhinakaran Pandiyan  
> wrote:
> > Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> > let's set this bit right before enabling the audio codec. Playing audio
> > without setting this bit results in pipe FIFO underruns.
> >
> > This workaround is applicable only for audio sample rates up to 96kHz. For
> > frequencies above 96kHz, this is insufficient and cdclk should be increased
> > to at least 432 MHz, just like BDW. Since, the audio driver does not
> > support sample rates > 48 kHz, we are safe with this fix for now.
> >
> > v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> > Fixed the port clock typo
> > Added TODO comment
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h|  5 +
> >  drivers/gpu/drm/i915/intel_audio.c | 30 +-
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 00efaa1..76dac48 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6236,6 +6236,11 @@ enum {
> >  #define SLICE_ECO_CHICKEN0 _MMIO(0x7308)
> >  #define   PIXEL_MASK_CAMMING_DISABLE   (1 << 14)
> >  
> > +#define _CHICKEN_TRANS_A   0x420C0
> > +#define _CHICKEN_TRANS_B   0x420C4
> > +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> > _CHICKEN_TRANS_B)
> > +#define SPARE_13   (1<<13)
> > +
> >  /* WaCatErrorRejectionIssue */
> >  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG _MMIO(0x9030)
> >  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB  (1<<11)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 7093cfb..894f11e 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  {
> > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > uint32_t tmp;
> >  
> > @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> > intel_encoder *encoder)
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /*Disable DP audio stall fix for HBR2*/
> 
> Nitpick, spaces after /* and before */.
> 
> > +   if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> > +   crtc_config->port_clock >= 54) {
> > +   tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> > +   tmp &= ~SPARE_13;
> > +   I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> > +   }
> 
> Hmm. Why don't we just do this unconditionally?
> 

That bit is disabled by default, so avoiding  two MMIO ops. that are not
required.

> > +
> > /* Disable timestamps */
> > tmp = I915_READ(HSW_AUD_CFG(pipe));
> > tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
> > tmp |= AUD_CONFIG_N_PROG_ENABLE;
> > tmp &= ~AUD_CONFIG_UPPER_N_MASK;
> > tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> > -   if (intel_crtc_has_dp_encoder(intel_crtc->config))
> > +   if (intel_crtc_has_dp_encoder(crtc_config))
> > tmp |= AUD_CONFIG_N_VALUE_INDEX;
> > I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> >  
> > @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> > *connector,
> >  {
> > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> > +   struct intel_crtc_state *crtc_config =  intel_crtc->config;
> > +   enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
> > enum pipe pipe = intel_crtc->pipe;
> > enum port port = intel_encoder->port;
> > const uint8_t *eld = connector->eld;
> > @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct 
> > drm_connector *connector,
> >  
> > mutex_lock(_priv->av_mutex);
> >  
> > +   /* Enable DP audio stall fix for HBR2
> > +*
> > +* TODO: This workaround is applicable only for audio sample rates up
> > +* to 96kHz. For frequencies above 96kHz, this is insufficient and
> > +* cdclk should be increased to at least 432 MHz, just like BDW. Since,
> > +* the audio driver does not support sample rates > 48 kHz, we are safe
> > +* with this fix for now.
> > +*/
> 
> Is this TODO required if you already have that check in patch 2/2?
> 

In fact, this patch itself is not required if we are increasing the
cdclk to 432 MHz for gen9 platforms (like patch 2/2). Having this
workaround gives us the option running the pipeline at a lower cdclk
frequency 

Re: [Intel-gfx] [PATCH v4 1/2] drm/i915/dp: Enable DP audio stall fix for gen9 platforms

2016-10-26 Thread Ville Syrjälä
On Tue, Oct 25, 2016 at 07:37:36PM -0700, Dhinakaran Pandiyan wrote:
> Enabling DP audio stall fix is necessary to play audio over DP HBR2. So,
> let's set this bit right before enabling the audio codec. Playing audio
> without setting this bit results in pipe FIFO underruns.
> 
> This workaround is applicable only for audio sample rates up to 96kHz. For
> frequencies above 96kHz, this is insufficient and cdclk should be increased
> to at least 432 MHz, just like BDW. Since, the audio driver does not
> support sample rates > 48 kHz, we are safe with this fix for now.
> 
> v2: Inlined the code change within hsw_audio_codec_enable() (Jani)
> Fixed the port clock typo
> Added TODO comment
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/i915_reg.h|  5 +
>  drivers/gpu/drm/i915/intel_audio.c | 30 +-
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00efaa1..76dac48 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6236,6 +6236,11 @@ enum {
>  #define SLICE_ECO_CHICKEN0   _MMIO(0x7308)
>  #define   PIXEL_MASK_CAMMING_DISABLE (1 << 14)
>  
> +#define _CHICKEN_TRANS_A 0x420C0
> +#define _CHICKEN_TRANS_B 0x420C4
> +#define CHICKEN_TRANS(tran) _MMIO_TRANS(tran, _CHICKEN_TRANS_A, 
> _CHICKEN_TRANS_B)
> +#define SPARE_13 (1<<13)
> +
>  /* WaCatErrorRejectionIssue */
>  #define GEN7_SQ_CHICKEN_MBCUNIT_CONFIG   _MMIO(0x9030)
>  #define  GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB(1<<11)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 7093cfb..894f11e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -283,6 +283,8 @@ static void hsw_audio_codec_disable(struct intel_encoder 
> *encoder)
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   uint32_t tmp;
>  
> @@ -290,13 +292,21 @@ static void hsw_audio_codec_disable(struct 
> intel_encoder *encoder)
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /*Disable DP audio stall fix for HBR2*/
> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp &= ~SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }
> +
>   /* Disable timestamps */
>   tmp = I915_READ(HSW_AUD_CFG(pipe));
>   tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>   tmp |= AUD_CONFIG_N_PROG_ENABLE;
>   tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>   tmp &= ~AUD_CONFIG_LOWER_N_MASK;
> - if (intel_crtc_has_dp_encoder(intel_crtc->config))
> + if (intel_crtc_has_dp_encoder(crtc_config))
>   tmp |= AUD_CONFIG_N_VALUE_INDEX;
>   I915_WRITE(HSW_AUD_CFG(pipe), tmp);
>  
> @@ -315,6 +325,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  {
>   struct drm_i915_private *dev_priv = to_i915(connector->dev);
>   struct intel_crtc *intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> + struct intel_crtc_state *crtc_config =  intel_crtc->config;
> + enum transcoder cpu_transcoder = crtc_config->cpu_transcoder;
>   enum pipe pipe = intel_crtc->pipe;
>   enum port port = intel_encoder->port;
>   const uint8_t *eld = connector->eld;
> @@ -326,6 +338,22 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>  
>   mutex_lock(_priv->av_mutex);
>  
> + /* Enable DP audio stall fix for HBR2
> +  *
> +  * TODO: This workaround is applicable only for audio sample rates up
> +  * to 96kHz. For frequencies above 96kHz, this is insufficient and
> +  * cdclk should be increased to at least 432 MHz, just like BDW. Since,
> +  * the audio driver does not support sample rates > 48 kHz, we are safe
> +  * with this fix for now.

Where in the sound driver is this supposed 96kHz limit? I see a lot of
stuff for >96kHz in the code at least.

> +  */
> +
> + if (IS_GEN9(dev_priv) && intel_crtc_has_dp_encoder(crtc_config) &&
> + crtc_config->port_clock >= 54) {
> + tmp = I915_READ(CHICKEN_TRANS(cpu_transcoder));
> + tmp |= SPARE_13;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), tmp);
> + }
> +
>   /* Enable audio presence detect, invalidate ELD */
>   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
>   tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC
___