On Wed, 01 Feb 2017 15:45:24 +0100,
Ville Syrjälä wrote:
>
> On Tue, Jan 31, 2017 at 10:36:44PM +0100, Takashi Iwai wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> >
> > Enable unmute/mute amp notification. This doesn't seem to affect
> > HDMI support so this is done unconditionally.
> >
> > An earlier version of this patch set a chicken bit at address 0x62F38
> > prior to the mute/unmute but this register doesn't seem to do anything
> > so this phase was removed.
> >
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
> > drivers/gpu/drm/i915/intel_lpe_audio.c | 17 +++++++++++++++++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index a9ffc8df241b..8fcc80cb864b 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2061,6 +2061,16 @@ enum skl_disp_power_wells {
> > #define I915_HDMI_LPE_AUDIO_BASE (VLV_DISPLAY_BASE + 0x65000)
> > #define I915_HDMI_LPE_AUDIO_SIZE 0x1000
> >
> > +/* DisplayPort Audio w/ LPE */
> > +#define VLV_AUD_PORT_EN_B_DBG (VLV_DISPLAY_BASE + 0x62F20)
> > +#define VLV_AUD_PORT_EN_C_DBG (VLV_DISPLAY_BASE + 0x62F30)
> > +#define VLV_AUD_PORT_EN_D_DBG (VLV_DISPLAY_BASE + 0x62F34)
> > +#define VLV_AUD_PORT_EN_DBG(port) _MMIO_PORT3((port) - PORT_B, \
> > + VLV_AUD_PORT_EN_B_DBG, \
> > + VLV_AUD_PORT_EN_C_DBG, \
> > + VLV_AUD_PORT_EN_D_DBG)
> > +#define VLV_AMP_MUTE (1 << 1)
> > +
> > #define GEN6_BSD_RNCID _MMIO(0x12198)
> >
> > #define GEN7_FF_THREAD_MODE _MMIO(0x20a0)
> > diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > index 245523e14418..f95624a46f27 100644
> > --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> > @@ -337,15 +337,25 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
> > {
> > unsigned long irq_flags;
> > struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
> > + u32 audio_enable;
> > + i915_reg_t mmio;
> >
> > if (!HAS_LPE_AUDIO(dev_priv))
> > return;
> >
> > + if (port == PORT_A) {
> > + DRM_ERROR("PORT_A is not valid for HDMI/DP usages\n");
> > + return;
> > + }
>
> Port A doesn't exists on these platforms at all. So this is
> just dead code.
>
> > +
> > pdata = dev_get_platdata(
> > &(dev_priv->lpe_audio.platdev->dev));
> >
> > spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
> >
> > + mmio = VLV_AUD_PORT_EN_DBG(port);
>
> I'm not a fan of these temporary reg offset variables. Makes it hard to
> figure out later which register is being frobbed, especially when the
> variable name doesn't say what it actually is (like in this case). So
> I'd like to see this killed off.
How about something like below?
thanks,
Takashi
-- 8< --
From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Subject: [PATCH v2] drm/i915: add DisplayPort amp unmute for LPE audio mode
Enable unmute/mute amp notification. This doesn't seem to affect
HDMI support so this is done unconditionally.
An earlier version of this patch set a chicken bit at address 0x62F38
prior to the mute/unmute but this register doesn't seem to do anything
so this phase was removed.
v1->v2: Drop needless pipe A check, avoid temporary reg offset variable.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
drivers/gpu/drm/i915/intel_lpe_audio.c | 12 ++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a9ffc8df241b..8fcc80cb864b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,16 @@ enum skl_disp_power_wells {
#define I915_HDMI_LPE_AUDIO_BASE (VLV_DISPLAY_BASE + 0x65000)
#define I915_HDMI_LPE_AUDIO_SIZE 0x1000
+/* DisplayPort Audio w/ LPE */
+#define VLV_AUD_PORT_EN_B_DBG (VLV_DISPLAY_BASE + 0x62F20)
+#define VLV_AUD_PORT_EN_C_DBG (VLV_DISPLAY_BASE + 0x62F30)
+#define VLV_AUD_PORT_EN_D_DBG (VLV_DISPLAY_BASE + 0x62F34)
+#define VLV_AUD_PORT_EN_DBG(port) _MMIO_PORT3((port) - PORT_B, \
+ VLV_AUD_PORT_EN_B_DBG, \
+ VLV_AUD_PORT_EN_C_DBG, \
+ VLV_AUD_PORT_EN_D_DBG)
+#define VLV_AMP_MUTE (1 << 1)
+
#define GEN6_BSD_RNCID _MMIO(0x12198)
#define GEN7_FF_THREAD_MODE _MMIO(0x20a0)
diff --git a/drivers/gpu/drm/i915/intel_lpe_audio.c b/drivers/gpu/drm/i915/intel_lpe_audio.c
index 245523e14418..5da14f40f94a 100644
--- a/drivers/gpu/drm/i915/intel_lpe_audio.c
+++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
@@ -337,6 +337,7 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
{
unsigned long irq_flags;
struct intel_hdmi_lpe_audio_pdata *pdata = NULL;
+ u32 audio_enable;
if (!HAS_LPE_AUDIO(dev_priv))
return;
@@ -346,6 +347,8 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
spin_lock_irqsave(&pdata->lpe_audio_slock, irq_flags);
+ audio_enable = I915_READ(VLV_AUD_PORT_EN_DBG(port));
+
if (eld != NULL) {
memcpy(pdata->eld.eld_data, eld,
HDMI_MAX_ELD_BYTES);
@@ -357,11 +360,20 @@ void intel_lpe_audio_notify(struct drm_i915_private *dev_priv,
pdata->tmds_clock_speed = tmds_clk_speed;
if (link_rate)
pdata->link_rate = link_rate;
+
+ /* Unmute the amp for both DP and HDMI */
+ I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+ audio_enable & ~VLV_AMP_MUTE);
+
} else {
memset(pdata->eld.eld_data, 0,
HDMI_MAX_ELD_BYTES);
pdata->hdmi_connected = false;
pdata->dp_output = false;
+
+ /* Mute the amp for both DP and HDMI */
+ I915_WRITE(VLV_AUD_PORT_EN_DBG(port),
+ audio_enable | VLV_AMP_MUTE);
}
if (pdata->notify_audio_lpe)
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
