On Thu, 2017-08-17 at 09:26 +0530, Sanyog Kale wrote: > 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.
Sanyog, I couldn't find all the old emails with spec pointing out the time that we needed to set this bit. Do you still have? Anything you could share when publishing the patches? > > > > 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. Ok, so let's put this on hold while audio drivers are not ready. It will be on internal branch anyways. When audio driver gets ready we re-submit all together. > > > > 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 <[email protected]> > > > Cc: Jani Nikula <[email protected]> > > > Cc: Sanyog Kale <[email protected]> > > > Signed-off-by: Rodrigo Vivi <[email protected]> > > > --- > > > 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); > > > }; > > > > > > /** > _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
