public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915: hsw+ audio regs are per-transocder
Date: Thu, 02 May 2019 15:16:37 +0300	[thread overview]
Message-ID: <871s1hoxui.fsf@intel.com> (raw)
In-Reply-To: <20190430142901.7302-2-ville.syrjala@linux.intel.com>

On Tue, 30 Apr 2019, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> s/pipe/transcoder/ when dealing with hsw+ audio registers. This
> won't actually make any real difference since there is no audio
> on the EDP transcoder. But this should avoid a bit of confusion
> when cross checking against the spec.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h    | 12 +++----
>  drivers/gpu/drm/i915/intel_audio.c | 55 ++++++++++++++----------------
>  2 files changed, 32 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6f0a0866c802..926e058d09ee 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9010,32 +9010,32 @@ enum {
>  /* HSW Audio */
>  #define _HSW_AUD_CONFIG_A		0x65000
>  #define _HSW_AUD_CONFIG_B		0x65100
> -#define HSW_AUD_CFG(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_CONFIG_A, _HSW_AUD_CONFIG_B)
> +#define HSW_AUD_CFG(trans)		_MMIO_TRANS(trans, _HSW_AUD_CONFIG_A, _HSW_AUD_CONFIG_B)
>  
>  #define _HSW_AUD_MISC_CTRL_A		0x65010
>  #define _HSW_AUD_MISC_CTRL_B		0x65110
> -#define HSW_AUD_MISC_CTRL(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
> +#define HSW_AUD_MISC_CTRL(trans)	_MMIO_TRANS(trans, _HSW_AUD_MISC_CTRL_A, _HSW_AUD_MISC_CTRL_B)
>  
>  #define _HSW_AUD_M_CTS_ENABLE_A		0x65028
>  #define _HSW_AUD_M_CTS_ENABLE_B		0x65128
> -#define HSW_AUD_M_CTS_ENABLE(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
> +#define HSW_AUD_M_CTS_ENABLE(trans)	_MMIO_TRANS(trans, _HSW_AUD_M_CTS_ENABLE_A, _HSW_AUD_M_CTS_ENABLE_B)
>  #define   AUD_M_CTS_M_VALUE_INDEX	(1 << 21)
>  #define   AUD_M_CTS_M_PROG_ENABLE	(1 << 20)
>  #define   AUD_CONFIG_M_MASK		0xfffff
>  
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_A	0x650b4
>  #define _HSW_AUD_DIP_ELD_CTRL_ST_B	0x651b4
> -#define HSW_AUD_DIP_ELD_CTRL(pipe)	_MMIO_PIPE(pipe, _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
> +#define HSW_AUD_DIP_ELD_CTRL(trans)	_MMIO_TRANS(trans, _HSW_AUD_DIP_ELD_CTRL_ST_A, _HSW_AUD_DIP_ELD_CTRL_ST_B)
>  
>  /* Audio Digital Converter */
>  #define _HSW_AUD_DIG_CNVT_1		0x65080
>  #define _HSW_AUD_DIG_CNVT_2		0x65180
> -#define AUD_DIG_CNVT(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_DIG_CNVT_1, _HSW_AUD_DIG_CNVT_2)
> +#define AUD_DIG_CNVT(trans)		_MMIO_TRANS(trans, _HSW_AUD_DIG_CNVT_1, _HSW_AUD_DIG_CNVT_2)
>  #define DIP_PORT_SEL_MASK		0x3
>  
>  #define _HSW_AUD_EDID_DATA_A		0x65050
>  #define _HSW_AUD_EDID_DATA_B		0x65150
> -#define HSW_AUD_EDID_DATA(pipe)		_MMIO_PIPE(pipe, _HSW_AUD_EDID_DATA_A, _HSW_AUD_EDID_DATA_B)
> +#define HSW_AUD_EDID_DATA(trans)	_MMIO_TRANS(trans, _HSW_AUD_EDID_DATA_A, _HSW_AUD_EDID_DATA_B)
>  
>  #define HSW_AUD_PIPE_CONV_CFG		_MMIO(0x6507c)
>  #define HSW_AUD_PIN_ELD_CP_VLD		_MMIO(0x650c0)
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 68a24dada44c..5c0b73f63843 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -319,9 +319,8 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct i915_audio_component *acomp = dev_priv->audio_component;
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	enum port port = encoder->port;
> -	enum pipe pipe = crtc->pipe;
>  	const struct dp_aud_n_m *nm;
>  	int rate;
>  	u32 tmp;
> @@ -333,7 +332,7 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>  	else
>  		DRM_DEBUG_KMS("using automatic Maud, Naud\n");
>  
> -	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp = I915_READ(HSW_AUD_CFG(cpu_transcoder));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> @@ -345,9 +344,9 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>  		tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	}
>  
> -	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +	I915_WRITE(HSW_AUD_CFG(cpu_transcoder), tmp);
>  
> -	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(cpu_transcoder));
>  	tmp &= ~AUD_CONFIG_M_MASK;
>  	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
>  	tmp &= ~AUD_M_CTS_M_PROG_ENABLE;
> @@ -358,7 +357,7 @@ hsw_dp_audio_config_update(struct intel_encoder *encoder,
>  		tmp |= AUD_M_CTS_M_PROG_ENABLE;
>  	}
>  
> -	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(cpu_transcoder), tmp);
>  }
>  
>  static void
> @@ -367,15 +366,14 @@ hsw_hdmi_audio_config_update(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct i915_audio_component *acomp = dev_priv->audio_component;
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	enum port port = encoder->port;
> -	enum pipe pipe = crtc->pipe;
>  	int n, rate;
>  	u32 tmp;
>  
>  	rate = acomp ? acomp->aud_sample_rate[port] : 0;
>  
> -	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp = I915_READ(HSW_AUD_CFG(cpu_transcoder));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>  	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> @@ -392,16 +390,16 @@ hsw_hdmi_audio_config_update(struct intel_encoder *encoder,
>  		DRM_DEBUG_KMS("using automatic N\n");
>  	}
>  
> -	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +	I915_WRITE(HSW_AUD_CFG(cpu_transcoder), tmp);
>  
>  	/*
>  	 * Let's disable "Enable CTS or M Prog bit"
>  	 * and let HW calculate the value
>  	 */
> -	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(pipe));
> +	tmp = I915_READ(HSW_AUD_M_CTS_ENABLE(cpu_transcoder));
>  	tmp &= ~AUD_M_CTS_M_PROG_ENABLE;
>  	tmp &= ~AUD_M_CTS_M_VALUE_INDEX;
> -	I915_WRITE(HSW_AUD_M_CTS_ENABLE(pipe), tmp);
> +	I915_WRITE(HSW_AUD_M_CTS_ENABLE(cpu_transcoder), tmp);
>  }
>  
>  static void
> @@ -419,28 +417,28 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>  				    const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> -	enum pipe pipe = crtc->pipe;
> +	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>  	u32 tmp;
>  
> -	DRM_DEBUG_KMS("Disable audio codec on pipe %c\n", pipe_name(pipe));
> +	DRM_DEBUG_KMS("Disable audio codec on transcoder %s\n",
> +		      transcoder_name(cpu_transcoder));
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
>  	/* Disable timestamps */
> -	tmp = I915_READ(HSW_AUD_CFG(pipe));
> +	tmp = I915_READ(HSW_AUD_CFG(cpu_transcoder));
>  	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>  	tmp |= AUD_CONFIG_N_PROG_ENABLE;
>  	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
>  	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
>  	if (intel_crtc_has_dp_encoder(old_crtc_state))
>  		tmp |= AUD_CONFIG_N_VALUE_INDEX;
> -	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
> +	I915_WRITE(HSW_AUD_CFG(cpu_transcoder), tmp);
>  
>  	/* Invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> -	tmp &= ~AUDIO_ELD_VALID(pipe);
> -	tmp &= ~AUDIO_OUTPUT_ENABLE(pipe);
> +	tmp &= ~AUDIO_ELD_VALID(cpu_transcoder);
> +	tmp &= ~AUDIO_OUTPUT_ENABLE(cpu_transcoder);
>  	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>  
>  	mutex_unlock(&dev_priv->av_mutex);
> @@ -451,22 +449,21 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>  				   const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_connector *connector = conn_state->connector;
> -	enum pipe pipe = crtc->pipe;
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	const u8 *eld = connector->eld;
>  	u32 tmp;
>  	int len, i;
>  
> -	DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> -		      pipe_name(pipe), drm_eld_size(eld));
> +	DRM_DEBUG_KMS("Enable audio codec on transcoder %s, %u bytes ELD\n",
> +		      transcoder_name(cpu_transcoder), drm_eld_size(eld));
>  
>  	mutex_lock(&dev_priv->av_mutex);
>  
>  	/* Enable audio presence detect, invalidate ELD */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> -	tmp |= AUDIO_OUTPUT_ENABLE(pipe);
> -	tmp &= ~AUDIO_ELD_VALID(pipe);
> +	tmp |= AUDIO_OUTPUT_ENABLE(cpu_transcoder);
> +	tmp &= ~AUDIO_ELD_VALID(cpu_transcoder);
>  	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>  
>  	/*
> @@ -477,18 +474,18 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>  	 */
>  
>  	/* Reset ELD write address */
> -	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(pipe));
> +	tmp = I915_READ(HSW_AUD_DIP_ELD_CTRL(cpu_transcoder));
>  	tmp &= ~IBX_ELD_ADDRESS_MASK;
> -	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
> +	I915_WRITE(HSW_AUD_DIP_ELD_CTRL(cpu_transcoder), tmp);
>  
>  	/* Up to 84 bytes of hw ELD buffer */
>  	len = min(drm_eld_size(eld), 84);
>  	for (i = 0; i < len / 4; i++)
> -		I915_WRITE(HSW_AUD_EDID_DATA(pipe), *((const u32 *)eld + i));
> +		I915_WRITE(HSW_AUD_EDID_DATA(cpu_transcoder), *((const u32 *)eld + i));
>  
>  	/* ELD valid */
>  	tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> -	tmp |= AUDIO_ELD_VALID(pipe);
> +	tmp |= AUDIO_ELD_VALID(cpu_transcoder);
>  	I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>  
>  	/* Enable timestamps */

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-05-02 12:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30 14:29 [PATCH 1/2] drm/i915: Don't skip audio enable if ELD is bogus Ville Syrjala
2019-04-30 14:29 ` [PATCH 2/2] drm/i915: hsw+ audio regs are per-transocder Ville Syrjala
2019-05-02 12:16   ` Jani Nikula [this message]
2019-05-02 14:11     ` Ville Syrjälä
2019-04-30 15:06 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Don't skip audio enable if ELD is bogus Patchwork
2019-04-30 15:08 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-04-30 15:26 ` ✓ Fi.CI.BAT: success " Patchwork
2019-05-01  8:08 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-05-02  8:07 ` ✓ Fi.CI.IGT: success " Patchwork
2019-05-02 12:16 ` [PATCH 1/2] " Jani Nikula

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=871s1hoxui.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox