Re: [RFC PATCH 4/5] drm: i915: add DisplayPort amp unmute for LPE audio mode

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 26 Jan 2017, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote:
> Enable chicken bit on LPE mode setup and unmute amp on
> notification
>
> FIXME: should these two phases done somewhere else?
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h        | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_lpe_audio.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a9ffc8d..ee90f64 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2061,6 +2061,18 @@ 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 CHICKEN_BIT_DBG_ENABLE		(1 << 0)
> +#define AMP_UNMUTE		        (1 << 1)

The convention is to define registers first and the contents/bits for
each register immedialy below. For groups of registers (like
PORT_EN_B/C/D below) define all registers first and bits immediately
below. (But note that the chicken register is not part of the group.)

> +#define AUD_CHICKEN_BIT_REG		0x62F38
> +#define AUD_PORT_EN_B_DBG		0x62F20
> +#define AUD_PORT_EN_C_DBG		0x62F28
> +#define AUD_PORT_EN_D_DBG		0x62F2C

Please don't define these separately without the display base, use
(VLV_DISPLAY_BASE + 0x62f38) style instead. All the other uses of
VLV_DISPLAY_BASE in the file follow the same convention.

> +#define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + AUD_CHICKEN_BIT_REG)
> +#define VLV_AUD_PORT_EN_B_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_B_DBG)
> +#define VLV_AUD_PORT_EN_C_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_C_DBG)
> +#define VLV_AUD_PORT_EN_D_DBG		_MMIO(VLV_DISPLAY_BASE + AUD_PORT_EN_D_DBG)
> +

Would be nice to have a macro VLV_AUD_PORT_EN_DBG(port), but damn those
reg offsets don't make it easy...


>  #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 245523e..b3134ef 100644
> --- a/drivers/gpu/drm/i915/intel_lpe_audio.c
> +++ b/drivers/gpu/drm/i915/intel_lpe_audio.c
> @@ -248,6 +248,15 @@ static int lpe_audio_setup(struct drm_i915_private *dev_priv)
>  		goto err_free_irq;
>  	}
>  
> +	/* Enable DPAudio debug bits by default */
> +	if (IS_CHERRYVIEW(dev_priv)) {
> +		u32 chicken_bit;
> +
> +		chicken_bit = I915_READ(VLV_AUD_CHICKEN_BIT_REG);
> +		I915_WRITE(VLV_AUD_CHICKEN_BIT_REG,
> +			   chicken_bit | CHICKEN_BIT_DBG_ENABLE);
> +	}
> +
>  	return 0;
>  err_free_irq:
>  	irq_free_desc(dev_priv->lpe_audio.irq);
> @@ -357,6 +366,24 @@ 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;
> +
> +		if (dp_output) { /* unmute the amp */
> +			u32 audio_enable;
> +
> +			if (port == PORT_B) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_B_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_B_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			} else if (port == PORT_C) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_C_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_C_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			} else if (port == PORT_D) {
> +				audio_enable = I915_READ(VLV_AUD_PORT_EN_D_DBG);
> +				I915_WRITE(VLV_AUD_PORT_EN_D_DBG,
> +					   audio_enable & ~AMP_UNMUTE);
> +			}
> +		}
>  	} else {
>  		memset(pdata->eld.eld_data, 0,
>  			HDMI_MAX_ELD_BYTES);

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