[Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
Starting on CNL, we need to enable Audio Pin Buffer.
v4: Throw the exclusive hook and everything else away
and add set/unset bit along with codec awake.
Based on few spec links that I was checking recently
I now believe that on CNL we also need to keep the codec
awake chicken bit along with PG2 enable and also add
this extra pin buffer enable.
BSpec: 18057
BSpec: 21352
BSpec: 19621
Cc: Jani Nikula
Cc: Sanyog Kale
Cc: Guneshwor Singh
Cc: Abhay Kumar
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_reg.h| 3 +++
drivers/gpu/drm/i915/intel_audio.c | 11 ++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 65ba10ad1fe5..768e784ea241 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8424,6 +8424,9 @@ enum {
#define HSW_AUD_CHICKENBIT _MMIO(0x65f10)
#define SKL_AUD_CODEC_WAKE_SIGNAL(1 << 15)
+#define AUDIO_PIN_BUF_CTL _MMIO(0x48414)
+#define AUDIO_PIN_BUF_ENABLE (1 << 31)
+
/* HSW Power Wells */
#define _HSW_PWR_WELL_CTL1 0x45400
#define _HSW_PWR_WELL_CTL2 0x45404
diff --git a/drivers/gpu/drm/i915/intel_audio.c
b/drivers/gpu/drm/i915/intel_audio.c
index 522d54fecb53..34f18322c9bd 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -729,11 +729,20 @@ static void
i915_audio_component_codec_wake_override(struct device *kdev,
struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
u32 tmp;
- if (!IS_GEN9_BC(dev_priv))
+ if (!IS_GEN9_BC(dev_priv) && !IS_CANNONLAKE(dev_priv))
return;
i915_audio_component_get_power(kdev);
+ if (IS_CANNONLAKE(dev_priv)) {
+ tmp = I915_READ(AUDIO_PIN_BUF_CTL);
+ if (enable)
+ tmp |= AUDIO_PIN_BUF_ENABLE;
+ else
+ tmp &= ~AUDIO_PIN_BUF_ENABLE;
+ I915_WRITE(AUDIO_PIN_BUF_CTL, tmp);
+ }
+
/*
* Enable/disable generating the codec wake signal, overriding the
* internal logic to generate the codec wake to controller.
--
2.13.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
Starting on CNL, we need to enable Audio Pin Buffer.
By the spec it seems that this is part of audio programming,
so let's give them the hability to set/unset this as needed.
v2: With a hook so audio driver can control it.
v3: Put back reg definition lost on v2.
Cc: Dhinakaran Pandiyan
Cc: Jani Nikula
Cc: Sanyog Kale
Signed-off-by: Rodrigo Vivi
---
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)
+{
+ struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+ if (!IS_CANNONLAKE(dev_priv))
+ 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);
};
/**
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
Rodrigo Vivi writes:
> Starting on CNL, we need to enable Audio Pin Buffer.
Ok... I forgot to include ICL on this now.
But first let's check if this is working on CNL and later I sent a v5.
On top of that for CNL and ICL audio needs to know when we are
switching cdclk. But for this work I'd like to hear from audio
if they need a new hook for notification or what.
And I will wait the work on audio start first before attempt
to implement any new hook on our side.
Thanks,
Rodrigo.
>
> v4: Throw the exclusive hook and everything else away
> and add set/unset bit along with codec awake.
>
> Based on few spec links that I was checking recently
> I now believe that on CNL we also need to keep the codec
> awake chicken bit along with PG2 enable and also add
> this extra pin buffer enable.
>
> BSpec: 18057
> BSpec: 21352
> BSpec: 19621
>
> Cc: Jani Nikula
> Cc: Sanyog Kale
> Cc: Guneshwor Singh
> Cc: Abhay Kumar
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_reg.h| 3 +++
> drivers/gpu/drm/i915/intel_audio.c | 11 ++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 65ba10ad1fe5..768e784ea241 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8424,6 +8424,9 @@ enum {
> #define HSW_AUD_CHICKENBIT _MMIO(0x65f10)
> #define SKL_AUD_CODEC_WAKE_SIGNAL (1 << 15)
>
> +#define AUDIO_PIN_BUF_CTL_MMIO(0x48414)
> +#define AUDIO_PIN_BUF_ENABLE(1 << 31)
> +
> /* HSW Power Wells */
> #define _HSW_PWR_WELL_CTL1 0x45400
> #define _HSW_PWR_WELL_CTL2 0x45404
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 522d54fecb53..34f18322c9bd 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -729,11 +729,20 @@ static void
> i915_audio_component_codec_wake_override(struct device *kdev,
> struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> u32 tmp;
>
> - if (!IS_GEN9_BC(dev_priv))
> + if (!IS_GEN9_BC(dev_priv) && !IS_CANNONLAKE(dev_priv))
> return;
>
> i915_audio_component_get_power(kdev);
>
> + if (IS_CANNONLAKE(dev_priv)) {
> + tmp = I915_READ(AUDIO_PIN_BUF_CTL);
> + if (enable)
> + tmp |= AUDIO_PIN_BUF_ENABLE;
> + else
> + tmp &= ~AUDIO_PIN_BUF_ENABLE;
> + I915_WRITE(AUDIO_PIN_BUF_CTL, tmp);
> + }
> +
> /*
>* Enable/disable generating the codec wake signal, overriding the
>* internal logic to generate the codec wake to controller.
> --
> 2.13.6
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
Hi Rodrigo,
On Wed, Feb 07, 2018 at 04:37:33PM -0800, Rodrigo Vivi wrote:
> Rodrigo Vivi writes:
>
> > Starting on CNL, we need to enable Audio Pin Buffer.
>
> Ok... I forgot to include ICL on this now.
> But first let's check if this is working on CNL and later I sent a v5.
>
This does not work on CNL. With this, the vendor id response is
0x.
I am not sure though why/how the below works fine:
(commented out disable sequence)
if (IS_CANNONLAKE(dev_priv)) {
tmp = I915_READ(AUDIO_PIN_BUF_CTL);
if (enable)
tmp |= AUDIO_PIN_BUF_ENABLE;
/*
else
tmp &= ~AUDIO_PIN_BUF_ENABLE;
*/
I915_WRITE(AUDIO_PIN_BUF_CTL, tmp);
> On top of that for CNL and ICL audio needs to know when we are
> switching cdclk. But for this work I'd like to hear from audio
> if they need a new hook for notification or what.
>
> And I will wait the work on audio start first before attempt
> to implement any new hook on our side.
>
> Thanks,
> Rodrigo.
>
> >
> > v4: Throw the exclusive hook and everything else away
> > and add set/unset bit along with codec awake.
> >
> > Based on few spec links that I was checking recently
> > I now believe that on CNL we also need to keep the codec
> > awake chicken bit along with PG2 enable and also add
> > this extra pin buffer enable.
> >
> > BSpec: 18057
> > BSpec: 21352
> > BSpec: 19621
> >
> > Cc: Jani Nikula
> > Cc: Sanyog Kale
> > Cc: Guneshwor Singh
> > Cc: Abhay Kumar
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/i915_reg.h| 3 +++
> > drivers/gpu/drm/i915/intel_audio.c | 11 ++-
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 65ba10ad1fe5..768e784ea241 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8424,6 +8424,9 @@ enum {
> > #define HSW_AUD_CHICKENBIT _MMIO(0x65f10)
> > #define SKL_AUD_CODEC_WAKE_SIGNAL(1 << 15)
> >
> > +#define AUDIO_PIN_BUF_CTL _MMIO(0x48414)
> > +#define AUDIO_PIN_BUF_ENABLE (1 << 31)
> > +
> > /* HSW Power Wells */
> > #define _HSW_PWR_WELL_CTL1 0x45400
> > #define _HSW_PWR_WELL_CTL2 0x45404
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> > b/drivers/gpu/drm/i915/intel_audio.c
> > index 522d54fecb53..34f18322c9bd 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -729,11 +729,20 @@ static void
> > i915_audio_component_codec_wake_override(struct device *kdev,
> > struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > u32 tmp;
> >
> > - if (!IS_GEN9_BC(dev_priv))
> > + if (!IS_GEN9_BC(dev_priv) && !IS_CANNONLAKE(dev_priv))
> > return;
> >
> > i915_audio_component_get_power(kdev);
> >
> > + if (IS_CANNONLAKE(dev_priv)) {
> > + tmp = I915_READ(AUDIO_PIN_BUF_CTL);
> > + if (enable)
> > + tmp |= AUDIO_PIN_BUF_ENABLE;
> > + else
> > + tmp &= ~AUDIO_PIN_BUF_ENABLE;
> > + I915_WRITE(AUDIO_PIN_BUF_CTL, tmp);
> > + }
> > +
> > /*
> > * Enable/disable generating the codec wake signal, overriding the
> > * internal logic to generate the codec wake to controller.
> > --
> > 2.13.6
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
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.
> 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
> Cc: Jani Nikula
> Cc: Sanyog Kale
> Signed-off-by: Rodrigo Vivi
> ---
> 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
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
> > Cc: Jani Nikula
> > Cc: Sanyog Kale
> > Signed-off-by: Rodrigo Vivi
> > ---
> > 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
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
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
> > > Cc: Jani Nikula
> > > Cc: Sanyog Kale
> > > Signed-off-by: Rodrigo Vivi
> > > ---
> > > 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)(s
Re: [Intel-gfx] [PATCH] drm/i915/cnl: Enable Audio Pin Buffer.
On Thu, 2017-08-17 at 18:55 +, Vivi, Rodrigo wrote:
> 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.
>
That makes a lot of sense to me.
> >
> > > > 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
> > > > Cc: Jani Nikula
> > > > Cc: Sanyog Kale
> > > > Signed-off-by: Rodrigo Vivi
> > > > ---
> > > > 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
