All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: alsa-devel@alsa-project.org, intel-gfx@lists.freedesktop.org,
	rakesh.a.ughreja@intel.com
Subject: Re: [PATCH 2/7] drm/i915: add DisplayPort amp unmute for LPE audio mode
Date: Wed, 01 Feb 2017 15:53:49 +0100	[thread overview]
Message-ID: <s5hh94earnm.wl-tiwai@suse.de> (raw)
In-Reply-To: <20170201144524.GX31595@intel.com>

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)
+#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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-02-01 14:53 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-31 21:36 [PATCH 0/7] DisplayPort audio support on Cherrytrail Takashi Iwai
2017-01-31 21:36 ` [PATCH 1/7] drm/i915: add DP support in LPE audio mode Takashi Iwai
2017-01-31 21:36 ` [PATCH 2/7] drm/i915: add DisplayPort amp unmute for " Takashi Iwai
2017-02-01 14:45   ` Ville Syrjälä
2017-02-01 14:53     ` Takashi Iwai [this message]
2017-02-01 15:11       ` Ville Syrjälä
2017-02-01 15:24         ` Takashi Iwai
2017-02-02  9:57   ` Takashi Iwai
2017-02-02 10:06     ` Ville Syrjälä
2017-02-02 10:13       ` Takashi Iwai
2017-01-31 21:36 ` [PATCH 3/7] drm/i915: Avoid MST pipe handling for LPE audio Takashi Iwai
2017-01-31 21:36 ` [PATCH 4/7] drm/i915: Pass pipe to LPE audio notification Takashi Iwai
2017-01-31 21:36 ` [PATCH 5/7] ALSA: x86: intel_hdmi: add definitions and logic for DP audio Takashi Iwai
2017-01-31 21:36 ` [PATCH 6/7] ALSA: x86: Use config base depending on the pipe Takashi Iwai
2017-01-31 21:36 ` [PATCH 7/7] drm/i915: Pass platform device to LPE audio notifier Takashi Iwai
2017-01-31 22:24 ` ✓ Fi.CI.BAT: success for DisplayPort audio support on Cherrytrail Patchwork
2017-03-13  8:33 ` [PATCH 0/7] " Daniel Vetter
2017-03-31  6:29   ` Daniel Vetter
2017-03-31  6:40     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hh94earnm.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rakesh.a.ughreja@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.