| Message ID | 20210910131916.3782933-1-kai.vehmanen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: program audio CDCLK-TS for keepalives | expand |
On Fri, 10 Sep 2021, Kai Vehmanen <kai.vehmanen@linux.intel.com> wrote: > XE_LPD display adds support for display audio codec keepalive feature. > This feature works also when display codec is in D3 state and the audio > link is off (BCLK off). To enable this functionality, display driver > must update the AUD_TS_CDCLK_M/N registers whenever CDCLK is changed. > Actual timestamps are generated only when the audio codec driver > specifically enables the KeepAlive (KAE) feature. > > This patch adds new hooks to intel_set_cdclk() in order to inform > display audio driver when CDCLK change is started and when it is > complete. > > Bspec: 53679 > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_audio.h | 2 + > drivers/gpu/drm/i915/display/intel_cdclk.c | 5 ++ > drivers/gpu/drm/i915/i915_reg.h | 4 ++ > 4 files changed, 65 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c > index 532237588511..48cced7f56f0 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio.c > +++ b/drivers/gpu/drm/i915/display/intel_audio.c > @@ -936,6 +936,60 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv) > } > } > > +struct aud_ts_cdclk_m_n { > + u8 m; > + u16 n; > +}; > + > +void intel_audio_cdclk_change_pre(struct drm_i915_private *dev_priv) Nitpick, switching to i915 variable name instead of dev_priv is preferred for new code throughout. > +{ > + u32 tmp; > + > + if (DISPLAY_VER(dev_priv) >= 13) { > + tmp = intel_de_read(dev_priv, AUD_TS_CDCLK_M); > + tmp &= ~AUD_TS_CDCLK_M_EN; > + intel_de_write(dev_priv, AUD_TS_CDCLK_M, tmp); Nitpick, could use intel_de_rmw() to do this on a single line. > + } > +} > + > +static int get_aud_ts_cdclk_m_n(int refclk, int cdclk, struct aud_ts_cdclk_m_n *aud_ts) > +{ > + if (!aud_ts) > + return -EINVAL; > + > + if (refclk == 24000) > + aud_ts->m = 12; > + else > + aud_ts->m = 15; > + > + aud_ts->n = cdclk * aud_ts->m / 24000; > + > + return 0; Is the return code added for future? For now, it just complicates this for no apparent reason. > +} > + > +void intel_audio_cdclk_change_post(struct drm_i915_private *dev_priv) > +{ > + struct aud_ts_cdclk_m_n aud_ts; > + int res; > + > + if (DISPLAY_VER(dev_priv) >= 13) { > + res = get_aud_ts_cdclk_m_n(dev_priv->cdclk.hw.ref, > + dev_priv->cdclk.hw.cdclk, > + &aud_ts); > + > + if (!res) { > + intel_de_write(dev_priv, AUD_TS_CDCLK_N, aud_ts.n); > + intel_de_write(dev_priv, AUD_TS_CDCLK_M, aud_ts.m | AUD_TS_CDCLK_M_EN); > + drm_dbg(&dev_priv->drm, "aud_ts_cdclk set to M=%u, N=%u\n", > + aud_ts.m, aud_ts.n); Usually drm_dbg_kms() for display code, including audio. I'll need to look up the bspec too, can't do that right now... this was just the nitpicks. ;D BR, Jani. > + } else { > + drm_err(&dev_priv->drm, > + "failed to find aud_ts_cdclk values for refclk %u cdclk %u\n", > + dev_priv->cdclk.hw.ref, dev_priv->cdclk.hw.cdclk); > + } > + } > +} > + > static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state, > struct intel_crtc *crtc, > bool enable) > diff --git a/drivers/gpu/drm/i915/display/intel_audio.h b/drivers/gpu/drm/i915/display/intel_audio.h > index a3657c7a7ba2..dcb259dd2da7 100644 > --- a/drivers/gpu/drm/i915/display/intel_audio.h > +++ b/drivers/gpu/drm/i915/display/intel_audio.h > @@ -18,6 +18,8 @@ void intel_audio_codec_enable(struct intel_encoder *encoder, > void intel_audio_codec_disable(struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > const struct drm_connector_state *old_conn_state); > +void intel_audio_cdclk_change_pre(struct drm_i915_private *dev_priv); > +void intel_audio_cdclk_change_post(struct drm_i915_private *dev_priv); > void intel_audio_init(struct drm_i915_private *dev_priv); > void intel_audio_deinit(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 9aec17b33819..a1365f31142d 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -24,6 +24,7 @@ > #include <linux/time.h> > > #include "intel_atomic.h" > +#include "intel_audio.h" > #include "intel_bw.h" > #include "intel_cdclk.h" > #include "intel_de.h" > @@ -1943,6 +1944,8 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, > intel_psr_pause(intel_dp); > } > > + intel_audio_cdclk_change_pre(dev_priv); > + > /* > * Lock aux/gmbus while we change cdclk in case those > * functions use cdclk. Not all platforms/ports do, > @@ -1971,6 +1974,8 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, > intel_psr_resume(intel_dp); > } > > + intel_audio_cdclk_change_post(dev_priv); > + > if (drm_WARN(&dev_priv->drm, > intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_config), > "cdclk state doesn't match!\n")) { > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index bd63760207b0..795775c9e2eb 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9734,6 +9734,10 @@ enum { > #define AUD_PIN_BUF_CTL _MMIO(0x48414) > #define AUD_PIN_BUF_ENABLE REG_BIT(31) > > +#define AUD_TS_CDCLK_M _MMIO(0x65ea0) > +#define AUD_TS_CDCLK_M_EN REG_BIT(31) > +#define AUD_TS_CDCLK_N _MMIO(0x65ea4) > + > /* Display Audio Config Reg */ > #define AUD_CONFIG_BE _MMIO(0x65ef0) > #define HBLANK_EARLY_ENABLE_ICL(pipe) (0x1 << (20 - (pipe)))
Hi, On Fri, 10 Sep 2021, Jani Nikula wrote: > Nitpick, switching to i915 variable name instead of dev_priv is > preferred for new code throughout. ack, changed. > On Fri, 10 Sep 2021, Kai Vehmanen <kai.vehmanen@linux.intel.com> wrote: > > + if (DISPLAY_VER(dev_priv) >= 13) { > > + tmp = intel_de_read(dev_priv, AUD_TS_CDCLK_M); > > + tmp &= ~AUD_TS_CDCLK_M_EN; > > + intel_de_write(dev_priv, AUD_TS_CDCLK_M, tmp); > > Nitpick, could use intel_de_rmw() to do this on a single line. True, changed. > > +static int get_aud_ts_cdclk_m_n(int refclk, int cdclk, struct aud_ts_cdclk_m_n *aud_ts) > > +{ > > + if (!aud_ts) > > + return -EINVAL; > > + > > + if (refclk == 24000) > > + aud_ts->m = 12; > > + else > > + aud_ts->m = 15; > > + > > + aud_ts->n = cdclk * aud_ts->m / 24000; > > + > > + return 0; > > Is the return code added for future? For now, it just complicates this > for no apparent reason. An early version had a table lookup and some refclk values were not supported. But, but, this version just calculates with a formula, so there's no real reason to have the return code anymore. A good point, removed. > > + drm_dbg(&dev_priv->drm, "aud_ts_cdclk set to M=%u, N=%u\n", > > + aud_ts.m, aud_ts.n); > > Usually drm_dbg_kms() for display code, including audio. Gah, indeed, fixed. > I'll need to look up the bspec too, can't do that right now... this was > just the nitpicks. ;D Keep 'em coming! :) Br, Kai
diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c index 532237588511..48cced7f56f0 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.c +++ b/drivers/gpu/drm/i915/display/intel_audio.c @@ -936,6 +936,60 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv) } } +struct aud_ts_cdclk_m_n { + u8 m; + u16 n; +}; + +void intel_audio_cdclk_change_pre(struct drm_i915_private *dev_priv) +{ + u32 tmp; + + if (DISPLAY_VER(dev_priv) >= 13) { + tmp = intel_de_read(dev_priv, AUD_TS_CDCLK_M); + tmp &= ~AUD_TS_CDCLK_M_EN; + intel_de_write(dev_priv, AUD_TS_CDCLK_M, tmp); + } +} + +static int get_aud_ts_cdclk_m_n(int refclk, int cdclk, struct aud_ts_cdclk_m_n *aud_ts) +{ + if (!aud_ts) + return -EINVAL; + + if (refclk == 24000) + aud_ts->m = 12; + else + aud_ts->m = 15; + + aud_ts->n = cdclk * aud_ts->m / 24000; + + return 0; +} + +void intel_audio_cdclk_change_post(struct drm_i915_private *dev_priv) +{ + struct aud_ts_cdclk_m_n aud_ts; + int res; + + if (DISPLAY_VER(dev_priv) >= 13) { + res = get_aud_ts_cdclk_m_n(dev_priv->cdclk.hw.ref, + dev_priv->cdclk.hw.cdclk, + &aud_ts); + + if (!res) { + intel_de_write(dev_priv, AUD_TS_CDCLK_N, aud_ts.n); + intel_de_write(dev_priv, AUD_TS_CDCLK_M, aud_ts.m | AUD_TS_CDCLK_M_EN); + drm_dbg(&dev_priv->drm, "aud_ts_cdclk set to M=%u, N=%u\n", + aud_ts.m, aud_ts.n); + } else { + drm_err(&dev_priv->drm, + "failed to find aud_ts_cdclk values for refclk %u cdclk %u\n", + dev_priv->cdclk.hw.ref, dev_priv->cdclk.hw.cdclk); + } + } +} + static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state, struct intel_crtc *crtc, bool enable) diff --git a/drivers/gpu/drm/i915/display/intel_audio.h b/drivers/gpu/drm/i915/display/intel_audio.h index a3657c7a7ba2..dcb259dd2da7 100644 --- a/drivers/gpu/drm/i915/display/intel_audio.h +++ b/drivers/gpu/drm/i915/display/intel_audio.h @@ -18,6 +18,8 @@ void intel_audio_codec_enable(struct intel_encoder *encoder, void intel_audio_codec_disable(struct intel_encoder *encoder, const struct intel_crtc_state *old_crtc_state, const struct drm_connector_state *old_conn_state); +void intel_audio_cdclk_change_pre(struct drm_i915_private *dev_priv); +void intel_audio_cdclk_change_post(struct drm_i915_private *dev_priv); void intel_audio_init(struct drm_i915_private *dev_priv); void intel_audio_deinit(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c index 9aec17b33819..a1365f31142d 100644 --- a/drivers/gpu/drm/i915/display/intel_cdclk.c +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c @@ -24,6 +24,7 @@ #include <linux/time.h> #include "intel_atomic.h" +#include "intel_audio.h" #include "intel_bw.h" #include "intel_cdclk.h" #include "intel_de.h" @@ -1943,6 +1944,8 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, intel_psr_pause(intel_dp); } + intel_audio_cdclk_change_pre(dev_priv); + /* * Lock aux/gmbus while we change cdclk in case those * functions use cdclk. Not all platforms/ports do, @@ -1971,6 +1974,8 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv, intel_psr_resume(intel_dp); } + intel_audio_cdclk_change_post(dev_priv); + if (drm_WARN(&dev_priv->drm, intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_config), "cdclk state doesn't match!\n")) { diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index bd63760207b0..795775c9e2eb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9734,6 +9734,10 @@ enum { #define AUD_PIN_BUF_CTL _MMIO(0x48414) #define AUD_PIN_BUF_ENABLE REG_BIT(31) +#define AUD_TS_CDCLK_M _MMIO(0x65ea0) +#define AUD_TS_CDCLK_M_EN REG_BIT(31) +#define AUD_TS_CDCLK_N _MMIO(0x65ea4) + /* Display Audio Config Reg */ #define AUD_CONFIG_BE _MMIO(0x65ef0) #define HBLANK_EARLY_ENABLE_ICL(pipe) (0x1 << (20 - (pipe)))
XE_LPD display adds support for display audio codec keepalive feature. This feature works also when display codec is in D3 state and the audio link is off (BCLK off). To enable this functionality, display driver must update the AUD_TS_CDCLK_M/N registers whenever CDCLK is changed. Actual timestamps are generated only when the audio codec driver specifically enables the KeepAlive (KAE) feature. This patch adds new hooks to intel_set_cdclk() in order to inform display audio driver when CDCLK change is started and when it is complete. Bspec: 53679 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> --- drivers/gpu/drm/i915/display/intel_audio.c | 54 ++++++++++++++++++++++ drivers/gpu/drm/i915/display/intel_audio.h | 2 + drivers/gpu/drm/i915/display/intel_cdclk.c | 5 ++ drivers/gpu/drm/i915/i915_reg.h | 4 ++ 4 files changed, 65 insertions(+)