| Message ID | s5hh94earnm.wl-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 01, 2017 at 03:53:49PM +0100, Takashi Iwai wrote: > 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@linux.intel.com> > > > > > > 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@linux.intel.com> > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > --- > > > 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@linux.intel.com> > 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@linux.intel.com> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > 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) We generally like to prefix the raw register offsets with '_' to make it clear they're not to be used directly. With that fixed the i915 patches are Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > +#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
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)