[Intel-gfx] [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec wakeup

[Lu, Han]

Hi Jani,

Sorry I didn't find method to access chicken bit registers or any other 0x65000 based registers
through audio mmio bar in ALSA side. It looks like the only way to access i915 registers from audio
side is through i915 callback (please correct me if I was wrong). So I apply your patch and it works
well. Thanks a lot!
(and yes, in my case, neither refcounting nor locking is must in ALSA side, so I will not add them this
time.)

BR,
Han Lu

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, April 28, 2015 10:22 PM
> To: Lu, Han; Vetter, Daniel; tiwai at suse.de; Yang, Libin; Lin, Mengdong; intel-
> gfx at lists.freedesktop.org
> Cc: Lu, Han; Deak, Imre
> Subject: Re: [PATCH-V2 1/2] drm/i915: add callback to enable/disable codec
> wakeup
> 
> On Tue, 28 Apr 2015, han.lu at intel.com wrote:
> > From: "Lu, Han" <han.lu at intel.com>
> >
> > In SKL, HDMI/DP codec and PCH HD Audio Controller are in different
> > power wells, so it's necessary to reset display audio codecs when
> > power well on, otherwise display audio codecs will disappear when
> > resume from low power state.
> > The reset step when power on is:
> >     enable codec wakeup -> azx_init_chip() -> disable codec wakeup
> >
> > The callback is defined in drivers/gpu/drm/i915/.
> > The caller is in sound/pci/hda/.
> 
> Han Lu, two more things:
> 
> First, is there any chance the chicken bit register is mirrored in the audio
> device mmio bar? If yes, you could use it directly in the audio driver, and we
> could drop patch 1/2...
> 
> Second, looking at the audio driver code for the usage of this callback in patch
> 2/2, I don't think the locking and refcounting add any value. Especially since
> you ignore the refcounting for disable. You're the only user, and anything
> beyond that would be broken anyway, with or without refcounting or locking.
> In fact, I'd like to push back the locking and refcounting to audio driver side,
> where you may realize they are not needed at all. Thus, I propose this as
> patch 1/2:
> 
> From b06bf55b94cdbfad8ec35923b2b657674472cce5 Mon Sep 17 00:00:00
> 2001
> From: "Lu, Han" <han.lu at intel.com>
> Date: Tue, 28 Apr 2015 17:03:08 +0300
> Subject: [PATCH] drm/i915/audio: add codec wakeup override
> enabled/disable  callback
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160
> Espoo
> Cc: Jani Nikula <jani.nikula at intel.com>
> 
> Add support for enabling codec wakeup override signal to allow re-
> enumeration of the controller on SKL after resume from low power state.
> 
> v3 by Jani: Simplify to only support toggling the appropriate chicken bit.
> 
> Signed-off-by: Lu, Han <han.lu at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
>  drivers/gpu/drm/i915/intel_audio.c | 27 +++++++++++++++++++++++++++
>  include/drm/i915_component.h       |  1 +
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 36805b64036b..435c372d001e
> 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6881,6 +6881,9 @@ enum skl_disp_power_wells {
>  #define   AUDIO_CP_READY(trans)		((1 << 1) << ((trans) * 4))
>  #define   AUDIO_ELD_VALID(trans)	((1 << 0) << ((trans) * 4))
> 
> +#define HSW_AUD_CHICKENBIT			0x65f10
> +#define   SKL_AUD_CODEC_WAKE_SIGNAL		(1 << 15)
> +
>  /* HSW Power Wells */
>  #define HSW_PWR_WELL_BIOS			0x45400 /* CTL1 */
>  #define HSW_PWR_WELL_DRIVER			0x45404 /* CTL2 */
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index f72e93a45e11..ceb232870d5a 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -474,6 +474,32 @@ static void
> i915_audio_component_put_power(struct device *dev)
>  	intel_display_power_put(dev_to_i915(dev),
> POWER_DOMAIN_AUDIO);  }
> 
> +static void i915_audio_component_codec_wake_override(struct device
> *dev,
> +						     bool enable)
> +{
> +	struct drm_i915_private *dev_priv = dev_to_i915(dev);
> +	u32 tmp;
> +
> +	if (!IS_SKYLAKE(dev_priv))
> +		return;
> +
> +	/*
> +	 * Enable/disable generating the codec wake signal, overriding the
> +	 * internal logic to generate the codec wake to controller.
> +	 */
> +	tmp = I915_READ(HSW_AUD_CHICKENBIT);
> +	tmp &= ~SKL_AUD_CODEC_WAKE_SIGNAL;
> +	I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
> +	usleep_range(1000, 1500);
> +
> +	if (enable) {
> +		tmp = I915_READ(HSW_AUD_CHICKENBIT);
> +		tmp |= SKL_AUD_CODEC_WAKE_SIGNAL;
> +		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
> +		usleep_range(1000, 1500);
> +	}
> +}
> +
>  /* Get CDCLK in kHz  */
>  static int i915_audio_component_get_cdclk_freq(struct device *dev)  { @@
> -495,6 +521,7 @@ static const struct i915_audio_component_ops
> i915_audio_component_ops = {
>  	.owner		= THIS_MODULE,
>  	.get_power	= i915_audio_component_get_power,
>  	.put_power	= i915_audio_component_put_power,
> +	.codec_wake_override =
> i915_audio_component_codec_wake_override,
>  	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
>  };
> 
> diff --git a/include/drm/i915_component.h
> b/include/drm/i915_component.h index 3e2f22e5bf3c..c9a8b64aa33b 100644
> --- a/include/drm/i915_component.h
> +++ b/include/drm/i915_component.h
> @@ -31,6 +31,7 @@ struct i915_audio_component {
>  		struct module *owner;
>  		void (*get_power)(struct device *);
>  		void (*put_power)(struct device *);
> +		void (*codec_wake_override)(struct device *, bool enable);
>  		int (*get_cdclk_freq)(struct device *);
>  	} *ops;
>  };
> --
> 2.1.4