diff mbox series

[v2] drm/i915: Implement Audio WA_14020863754

Message ID 20240410135046.933254-1-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: Implement Audio WA_14020863754 | expand

Commit Message

Shankar, Uma April 10, 2024, 1:50 p.m. UTC
WA_14020863754: Corner case with Min Hblank Fix can cause
audio hang

Issue: Previously a fix was made to avoid issues with extremely
small hblanks, called the "Min Hblank Fix". However, this can
potentially cause an audio hang.

Workaround :
During "Audio Programming Sequence" Audio Enabling -
When DP mode is enabled Set mmio offset 0x65F1C bit 18 = 1b,
before step #1 "Enable audio Presence Detect"

During "Audio Programming Sequence" Audio Disabling -
When DP mode is enabled Clear mmio offset 0x65F1C bit 18 = 0b,
after step #6 "Disable Audio PD (Presence Detect)"
If not clearing PD bit, must also not clear 0x65F1C bit 18 (leave = 1b)

v2: Update the platform checks (Jani Nikula)

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c      | 14 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_audio_regs.h |  3 +++
 2 files changed, 17 insertions(+)

Comments

Matt Roper April 11, 2024, 10:36 p.m. UTC | #1
On Wed, Apr 10, 2024 at 07:20:46PM +0530, Uma Shankar wrote:
> WA_14020863754: Corner case with Min Hblank Fix can cause
> audio hang
> 
> Issue: Previously a fix was made to avoid issues with extremely
> small hblanks, called the "Min Hblank Fix". However, this can
> potentially cause an audio hang.
> 
> Workaround :
> During "Audio Programming Sequence" Audio Enabling -
> When DP mode is enabled Set mmio offset 0x65F1C bit 18 = 1b,
> before step #1 "Enable audio Presence Detect"
> 
> During "Audio Programming Sequence" Audio Disabling -
> When DP mode is enabled Clear mmio offset 0x65F1C bit 18 = 0b,
> after step #6 "Disable Audio PD (Presence Detect)"
> If not clearing PD bit, must also not clear 0x65F1C bit 18 (leave = 1b)
> 
> v2: Update the platform checks (Jani Nikula)
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_audio.c      | 14 ++++++++++++++
>  drivers/gpu/drm/i915/display/intel_audio_regs.h |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 07e0c73204f3..61df5115c970 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -512,6 +512,13 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
>  		     AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
>  
> +	/*
> +	 * WA_14020863754: Implement Audio Workaround
> +	 * Corner case with Min Hblank Fix can cause audio hang
> +	 */
> +	if (DISPLAY_VER(i915) >= 20)

The workaround is currently listed as applying to both Xe2_LPD (20.00)
and Xe2_HPD (14.01).  So we should match on those precise IP versions
for now.  Future platforms and/or refreshes may or may not need this
workaround and we don't want to just assume the workaround will carry
forward forever, so the condition may get updated further as new
platforms/IP versions are added to the driver.


Matt

> +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, CHICKEN3_SPARE18, 0);
> +
>  	mutex_unlock(&i915->display.audio.mutex);
>  }
>  
> @@ -637,6 +644,13 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
>  		enable_audio_dsc_wa(encoder, crtc_state);
>  
> +	/*
> +	 * WA_14020863754: Implement Audio Workaround
> +	 * Corner case with Min Hblank Fix can cause audio hang
> +	 */
> +	if (DISPLAY_VER(i915) >= 20)
> +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, 0, CHICKEN3_SPARE18);
> +
>  	/* Enable audio presence detect */
>  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
>  		     0, AUDIO_OUTPUT_ENABLE(cpu_transcoder));
> diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> index 616e7b1275c4..6f8d33299ecd 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> @@ -148,4 +148,7 @@
>  #define HBLANK_START_COUNT_96	4
>  #define HBLANK_START_COUNT_128	5
>  
> +#define AUD_CHICKENBIT_REG3		_MMIO(0x65F1C)
> +#define  CHICKEN3_SPARE18		REG_BIT(18)
> +
>  #endif /* __INTEL_AUDIO_REGS_H__ */
> -- 
> 2.42.0
>
Matt Roper April 11, 2024, 10:39 p.m. UTC | #2
On Thu, Apr 11, 2024 at 03:36:37PM -0700, Matt Roper wrote:
> On Wed, Apr 10, 2024 at 07:20:46PM +0530, Uma Shankar wrote:
> > WA_14020863754: Corner case with Min Hblank Fix can cause
> > audio hang
> > 
> > Issue: Previously a fix was made to avoid issues with extremely
> > small hblanks, called the "Min Hblank Fix". However, this can
> > potentially cause an audio hang.
> > 
> > Workaround :
> > During "Audio Programming Sequence" Audio Enabling -
> > When DP mode is enabled Set mmio offset 0x65F1C bit 18 = 1b,
> > before step #1 "Enable audio Presence Detect"
> > 
> > During "Audio Programming Sequence" Audio Disabling -
> > When DP mode is enabled Clear mmio offset 0x65F1C bit 18 = 0b,
> > after step #6 "Disable Audio PD (Presence Detect)"
> > If not clearing PD bit, must also not clear 0x65F1C bit 18 (leave = 1b)
> > 
> > v2: Update the platform checks (Jani Nikula)
> > 
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_audio.c      | 14 ++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_audio_regs.h |  3 +++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> > index 07e0c73204f3..61df5115c970 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > @@ -512,6 +512,13 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
> >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> >  		     AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
> >  
> > +	/*
> > +	 * WA_14020863754: Implement Audio Workaround
> > +	 * Corner case with Min Hblank Fix can cause audio hang
> > +	 */
> > +	if (DISPLAY_VER(i915) >= 20)
> 
> The workaround is currently listed as applying to both Xe2_LPD (20.00)
> and Xe2_HPD (14.01).  So we should match on those precise IP versions
> for now.  Future platforms and/or refreshes may or may not need this
> workaround and we don't want to just assume the workaround will carry
> forward forever, so the condition may get updated further as new
> platforms/IP versions are added to the driver.

Also, since this is a workaround that requires logic in multiple spots
and already applies to multiple IP versions, it may be a good idea to
separate out the condition into a helper function so that if/when new
versions get added to the list in the future we can update a single
place rather than tracking down multiple 'if' statements.


Matt

> 
> 
> Matt
> 
> > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, CHICKEN3_SPARE18, 0);
> > +
> >  	mutex_unlock(&i915->display.audio.mutex);
> >  }
> >  
> > @@ -637,6 +644,13 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
> >  		enable_audio_dsc_wa(encoder, crtc_state);
> >  
> > +	/*
> > +	 * WA_14020863754: Implement Audio Workaround
> > +	 * Corner case with Min Hblank Fix can cause audio hang
> > +	 */
> > +	if (DISPLAY_VER(i915) >= 20)
> > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, 0, CHICKEN3_SPARE18);
> > +
> >  	/* Enable audio presence detect */
> >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> >  		     0, AUDIO_OUTPUT_ENABLE(cpu_transcoder));
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > index 616e7b1275c4..6f8d33299ecd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > @@ -148,4 +148,7 @@
> >  #define HBLANK_START_COUNT_96	4
> >  #define HBLANK_START_COUNT_128	5
> >  
> > +#define AUD_CHICKENBIT_REG3		_MMIO(0x65F1C)
> > +#define  CHICKEN3_SPARE18		REG_BIT(18)
> > +
> >  #endif /* __INTEL_AUDIO_REGS_H__ */
> > -- 
> > 2.42.0
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
Shankar, Uma April 12, 2024, 10:25 a.m. UTC | #3
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Friday, April 12, 2024 4:07 AM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Borah,
> Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> jani.nikula@linux.intel.com
> Subject: Re: [v2] drm/i915: Implement Audio WA_14020863754
> 
> On Wed, Apr 10, 2024 at 07:20:46PM +0530, Uma Shankar wrote:
> > WA_14020863754: Corner case with Min Hblank Fix can cause audio hang
> >
> > Issue: Previously a fix was made to avoid issues with extremely small
> > hblanks, called the "Min Hblank Fix". However, this can potentially
> > cause an audio hang.
> >
> > Workaround :
> > During "Audio Programming Sequence" Audio Enabling - When DP mode is
> > enabled Set mmio offset 0x65F1C bit 18 = 1b, before step #1 "Enable
> > audio Presence Detect"
> >
> > During "Audio Programming Sequence" Audio Disabling - When DP mode is
> > enabled Clear mmio offset 0x65F1C bit 18 = 0b, after step #6 "Disable
> > Audio PD (Presence Detect)"
> > If not clearing PD bit, must also not clear 0x65F1C bit 18 (leave =
> > 1b)
> >
> > v2: Update the platform checks (Jani Nikula)
> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_audio.c      | 14 ++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_audio_regs.h |  3 +++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> > b/drivers/gpu/drm/i915/display/intel_audio.c
> > index 07e0c73204f3..61df5115c970 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > @@ -512,6 +512,13 @@ static void hsw_audio_codec_disable(struct
> intel_encoder *encoder,
> >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> >  		     AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
> >
> > +	/*
> > +	 * WA_14020863754: Implement Audio Workaround
> > +	 * Corner case with Min Hblank Fix can cause audio hang
> > +	 */
> > +	if (DISPLAY_VER(i915) >= 20)
> 
> The workaround is currently listed as applying to both Xe2_LPD (20.00) and
> Xe2_HPD (14.01).  So we should match on those precise IP versions for now.
> Future platforms and/or refreshes may or may not need this workaround and we
> don't want to just assume the workaround will carry forward forever, so the
> condition may get updated further as new platforms/IP versions are added to the
> driver.

Hi Matt,
Yes, agree to limit till platforms where we have visibility.

Should I just keep it for LNL and add BMG later once the PE changes get merged and the
macros become available?
Also, will add the helper as you suggested.

Regards,
Uma Shankar

> 
> Matt
> 
> > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3,
> CHICKEN3_SPARE18, 0);
> > +
> >  	mutex_unlock(&i915->display.audio.mutex);
> >  }
> >
> > @@ -637,6 +644,13 @@ static void hsw_audio_codec_enable(struct
> intel_encoder *encoder,
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
> >  		enable_audio_dsc_wa(encoder, crtc_state);
> >
> > +	/*
> > +	 * WA_14020863754: Implement Audio Workaround
> > +	 * Corner case with Min Hblank Fix can cause audio hang
> > +	 */
> > +	if (DISPLAY_VER(i915) >= 20)
> > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, 0,
> CHICKEN3_SPARE18);
> > +
> >  	/* Enable audio presence detect */
> >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> >  		     0, AUDIO_OUTPUT_ENABLE(cpu_transcoder));
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > index 616e7b1275c4..6f8d33299ecd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > @@ -148,4 +148,7 @@
> >  #define HBLANK_START_COUNT_96	4
> >  #define HBLANK_START_COUNT_128	5
> >
> > +#define AUD_CHICKENBIT_REG3		_MMIO(0x65F1C)
> > +#define  CHICKEN3_SPARE18		REG_BIT(18)
> > +
> >  #endif /* __INTEL_AUDIO_REGS_H__ */
> > --
> > 2.42.0
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
Matt Roper April 12, 2024, 3:24 p.m. UTC | #4
On Fri, Apr 12, 2024 at 03:25:23AM -0700, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Roper, Matthew D <matthew.d.roper@intel.com>
> > Sent: Friday, April 12, 2024 4:07 AM
> > To: Shankar, Uma <uma.shankar@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org; Borah,
> > Chaitanya Kumar <chaitanya.kumar.borah@intel.com>;
> > jani.nikula@linux.intel.com
> > Subject: Re: [v2] drm/i915: Implement Audio WA_14020863754
> > 
> > On Wed, Apr 10, 2024 at 07:20:46PM +0530, Uma Shankar wrote:
> > > WA_14020863754: Corner case with Min Hblank Fix can cause audio hang
> > >
> > > Issue: Previously a fix was made to avoid issues with extremely small
> > > hblanks, called the "Min Hblank Fix". However, this can potentially
> > > cause an audio hang.
> > >
> > > Workaround :
> > > During "Audio Programming Sequence" Audio Enabling - When DP mode is
> > > enabled Set mmio offset 0x65F1C bit 18 = 1b, before step #1 "Enable
> > > audio Presence Detect"
> > >
> > > During "Audio Programming Sequence" Audio Disabling - When DP mode is
> > > enabled Clear mmio offset 0x65F1C bit 18 = 0b, after step #6 "Disable
> > > Audio PD (Presence Detect)"
> > > If not clearing PD bit, must also not clear 0x65F1C bit 18 (leave =
> > > 1b)
> > >
> > > v2: Update the platform checks (Jani Nikula)
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_audio.c      | 14 ++++++++++++++
> > >  drivers/gpu/drm/i915/display/intel_audio_regs.h |  3 +++
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> > > b/drivers/gpu/drm/i915/display/intel_audio.c
> > > index 07e0c73204f3..61df5115c970 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > > @@ -512,6 +512,13 @@ static void hsw_audio_codec_disable(struct
> > intel_encoder *encoder,
> > >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> > >  		     AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
> > >
> > > +	/*
> > > +	 * WA_14020863754: Implement Audio Workaround
> > > +	 * Corner case with Min Hblank Fix can cause audio hang
> > > +	 */
> > > +	if (DISPLAY_VER(i915) >= 20)
> > 
> > The workaround is currently listed as applying to both Xe2_LPD (20.00) and
> > Xe2_HPD (14.01).  So we should match on those precise IP versions for now.
> > Future platforms and/or refreshes may or may not need this workaround and we
> > don't want to just assume the workaround will carry forward forever, so the
> > condition may get updated further as new platforms/IP versions are added to the
> > driver.
> 
> Hi Matt,
> Yes, agree to limit till platforms where we have visibility.
> 
> Should I just keep it for LNL and add BMG later once the PE changes get merged and the
> macros become available?

You should probably sync with Bala on that and see what he thinks.  I
suspect that changes here won't conflict with anything else in the BMG
enablement series, so it's probably okay to include 14.01 in the
condition right away, even before that series lands, but Bala would know
best.


Matt


> Also, will add the helper as you suggested.
> 
> Regards,
> Uma Shankar
> 
> > 
> > Matt
> > 
> > > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3,
> > CHICKEN3_SPARE18, 0);
> > > +
> > >  	mutex_unlock(&i915->display.audio.mutex);
> > >  }
> > >
> > > @@ -637,6 +644,13 @@ static void hsw_audio_codec_enable(struct
> > intel_encoder *encoder,
> > >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
> > >  		enable_audio_dsc_wa(encoder, crtc_state);
> > >
> > > +	/*
> > > +	 * WA_14020863754: Implement Audio Workaround
> > > +	 * Corner case with Min Hblank Fix can cause audio hang
> > > +	 */
> > > +	if (DISPLAY_VER(i915) >= 20)
> > > +		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, 0,
> > CHICKEN3_SPARE18);
> > > +
> > >  	/* Enable audio presence detect */
> > >  	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
> > >  		     0, AUDIO_OUTPUT_ENABLE(cpu_transcoder));
> > > diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > > b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > > index 616e7b1275c4..6f8d33299ecd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
> > > @@ -148,4 +148,7 @@
> > >  #define HBLANK_START_COUNT_96	4
> > >  #define HBLANK_START_COUNT_128	5
> > >
> > > +#define AUD_CHICKENBIT_REG3		_MMIO(0x65F1C)
> > > +#define  CHICKEN3_SPARE18		REG_BIT(18)
> > > +
> > >  #endif /* __INTEL_AUDIO_REGS_H__ */
> > > --
> > > 2.42.0
> > >
> > 
> > --
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 07e0c73204f3..61df5115c970 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -512,6 +512,13 @@  static void hsw_audio_codec_disable(struct intel_encoder *encoder,
 	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
 		     AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
 
+	/*
+	 * WA_14020863754: Implement Audio Workaround
+	 * Corner case with Min Hblank Fix can cause audio hang
+	 */
+	if (DISPLAY_VER(i915) >= 20)
+		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, CHICKEN3_SPARE18, 0);
+
 	mutex_unlock(&i915->display.audio.mutex);
 }
 
@@ -637,6 +644,13 @@  static void hsw_audio_codec_enable(struct intel_encoder *encoder,
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
 		enable_audio_dsc_wa(encoder, crtc_state);
 
+	/*
+	 * WA_14020863754: Implement Audio Workaround
+	 * Corner case with Min Hblank Fix can cause audio hang
+	 */
+	if (DISPLAY_VER(i915) >= 20)
+		intel_de_rmw(i915, AUD_CHICKENBIT_REG3, 0, CHICKEN3_SPARE18);
+
 	/* Enable audio presence detect */
 	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
 		     0, AUDIO_OUTPUT_ENABLE(cpu_transcoder));
diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h b/drivers/gpu/drm/i915/display/intel_audio_regs.h
index 616e7b1275c4..6f8d33299ecd 100644
--- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
@@ -148,4 +148,7 @@ 
 #define HBLANK_START_COUNT_96	4
 #define HBLANK_START_COUNT_128	5
 
+#define AUD_CHICKENBIT_REG3		_MMIO(0x65F1C)
+#define  CHICKEN3_SPARE18		REG_BIT(18)
+
 #endif /* __INTEL_AUDIO_REGS_H__ */