[Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.

[Sanyog Kale]

On Wed, Aug 16, 2017 at 06:46:26PM -0500, Pandiyan, Dhinakaran wrote:
> On Thu, 2017-07-06 at 14:03 -0700, Rodrigo Vivi wrote:
> > Starting on CNL, we need to enable Audio Pin Buffer.
> > 
> > By the spec it seems that this is part of audio programming,
> 
> I am not very clear where the pin buffer enabling/disabling step falls
> in the audio programming sequence. From what I understand, audio codec
> enable/disable is controlled by i915, if pin buffer enable/disable is
> part of that, then we don't need a call back. It's difficult to know
> where this function is going to be used without seeing the audio driver
> patch that makes of use of this.
>

>From audio point of view, we are working on correct sequence where this ops
will be called. However during CNL Power-ON we enabled the pin buf using ops
as part of azx_reset.
 
> > so let's give them the hability to set/unset this as needed.
> 
> typo s/hability/ability
> > 
> > v2: With a hook so audio driver can control it.
> > v3: Put back reg definition lost on v2.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Cc: Sanyog Kale <sanyog.r.kale at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
> >  drivers/gpu/drm/i915/intel_audio.c | 16 ++++++++++++++++
> >  include/drm/i915_component.h       |  6 ++++++
> >  3 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 64cc674..aab38da 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2643,6 +2643,9 @@ enum skl_disp_power_wells {
> >  #define I915_HDMI_LPE_AUDIO_BASE	(VLV_DISPLAY_BASE + 0x65000)
> >  #define I915_HDMI_LPE_AUDIO_SIZE	0x1000
> >  
> > +#define AUDIO_PIN_BUF_CTL		_MMIO(0x48414)
> > +#define AUDIO_PIN_BUF_ENABLE		(1 << 31)
> > +
> >  /* DisplayPort Audio w/ LPE */
> >  #define VLV_AUD_CHICKEN_BIT_REG		_MMIO(VLV_DISPLAY_BASE + 0x62F38)
> >  #define VLV_CHICKEN_BIT_DBG_ENABLE	(1 << 0)
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index d805b6e..0c83254 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -865,6 +865,21 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
> >  	return ret;
> >  }
> >  
> > +static void i915_audio_component_pin_buf(struct device *kdev, bool enable)
> > +{
> 
> Does it matter whether the audio codec is enabled or disabled when this
> call comes in? i.e., do we need some sort of check here? Or do we assume
> the audio driver knows exactly when to enable or disable pin buffer?
> 
> 
> 
> > +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > +
> > +	if (!IS_CANNONLAKE(dev_priv))
> 
> Should this be a INTEL_GEN() < 10 since the register is gen10+? I am not
> particular about either of the options.
> 
> > +		return;
> > +
> > +	if (enable)
> > +		I915_WRITE(AUDIO_PIN_BUF_CTL, I915_READ(AUDIO_PIN_BUF_CTL) |
> > +			   AUDIO_PIN_BUF_ENABLE);
> > +	else
> > +		I915_WRITE(AUDIO_PIN_BUF_CTL, I915_READ(AUDIO_PIN_BUF_CTL) &
> > +			   ~AUDIO_PIN_BUF_ENABLE);
> > +}
> > +
> >  static const struct i915_audio_component_ops i915_audio_component_ops = {
> >  	.owner		= THIS_MODULE,
> >  	.get_power	= i915_audio_component_get_power,
> > @@ -873,6 +888,7 @@ static int i915_audio_component_get_eld(struct device *kdev, int port,
> >  	.get_cdclk_freq	= i915_audio_component_get_cdclk_freq,
> >  	.sync_audio_rate = i915_audio_component_sync_audio_rate,
> >  	.get_eld	= i915_audio_component_get_eld,
> > +	.pin_buf	= i915_audio_component_pin_buf,
> >  };
> >  
> >  static int i915_audio_component_bind(struct device *i915_kdev,
> > diff --git a/include/drm/i915_component.h b/include/drm/i915_component.h
> > index 545c6e0..b8875d4 100644
> > --- a/include/drm/i915_component.h
> > +++ b/include/drm/i915_component.h
> > @@ -79,6 +79,12 @@ struct i915_audio_component_ops {
> >  	 */
> >  	int (*get_eld)(struct device *, int port, int pipe, bool *enabled,
> >  		       unsigned char *buf, int max_bytes);
> > +	/**
> > +	 * @pin_buf: Enable or disable pin buffer.
> > +	 *
> > +	 * Allow audio driver the toggle pin buffer.
> > +	 */
> > +	void (*pin_buf)(struct device *, bool enable);
> >  };
> >  
> >  /**

--