Re: [PATCH v3] drm/i915/audio: rewrite vlv/chv and gen 5-7 audio codec enable sequence

Jani Nikula
2014-10-31T09:17:15Z

On Thu, 30 Oct 2014, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> On Tue, Oct 28, 2014 at 5:04 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> Similar to the hsw/bdw enable sequence rewrite.
>>
>> v3: replace vblank wait with a comment
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_audio.c | 58 +++++++++++++++++---------------------
>> 1 file changed, 26 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> index 2e7d42878b9d..c06047361e2a 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -210,6 +210,10 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>> {
>> struct drm_i915_private *dev_priv = connector->dev->dev_private;
>> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
>> + struct intel_digital_port *intel_dig_port =
>> + enc_to_dig_port(&encoder->base);
>> + enum port port = intel_dig_port->port;
>> + enum pipe pipe = intel_crtc->pipe;
>> uint8_t *eld = connector->eld;
>> uint32_t eldv;
>> uint32_t tmp;
>> @@ -218,8 +222,11 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>> int aud_config;
>> int aud_cntl_st;
>> int aud_cntrl_st2;
>> - enum pipe pipe = intel_crtc->pipe;
>> - enum port port;
>> +
>> + DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
>> + port_name(port), pipe_name(pipe), eld[2]);
>
> On SNB spec we should
>
> Enable audio Presence Detect
>
> Set the port control register (HDMI_CTL or DP_CTL) Audio_Output_Enable
> (bit 6) to "1"
>
> at this point

That's done a bit earlier in intel_enable_hdmi and intel_dp_enable_port
(where intel_dp->DP has had the DP_AUDIO_OUTPUT_ENABLE bit set in
intel_dp_prepare).

>
>> +
>> + /* XXX: vblank wait here */
>
> same question from previous email... aren't we enabling it to soon?

Same answers as in previous mails to this and the comments below.

BR,
Jani.

>
>>
>> if (HAS_PCH_IBX(connector->dev)) {
>> hdmiw_hdmiedid = IBX_HDMIW_HDMIEDID(pipe);
>> @@ -238,57 +245,44 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
>> aud_cntrl_st2 = CPT_AUD_CNTRL_ST2;
>> }
>>
>> - DRM_DEBUG_DRIVER("ELD on pipe %c\n", pipe_name(pipe));
>> -
>> - if (IS_VALLEYVIEW(connector->dev)) {
>> - struct intel_digital_port *intel_dig_port;
>> -
>> - intel_dig_port = enc_to_dig_port(&encoder->base);
>> - port = intel_dig_port->port;
>> - } else {
>> - tmp = I915_READ(aud_cntl_st);
>> - port = (tmp >> 29) & DIP_PORT_SEL_MASK;
>> - /* DIP_Port_Select, 0x1 = PortB */
>> - }
>> -
>> - if (!port) {
>> - DRM_DEBUG_DRIVER("Audio directed to unknown port\n");
>> - /* operate blindly on all ports */
>> + if (WARN_ON(!port)) {
>> eldv = IBX_ELD_VALIDB;
>> eldv |= IBX_ELD_VALIDB << 4;
>> eldv |= IBX_ELD_VALIDB << 8;
>> } else {
>> - DRM_DEBUG_DRIVER("ELD on port %c\n", port_name(port));
>> eldv = IBX_ELD_VALIDB << ((port - 1) * 4);
>> }
>>
>> - if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> - I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */
>> - else
>> - I915_WRITE(aud_config, audio_config_hdmi_pixel_clock(mode));
>> -
>> - if (intel_eld_uptodate(connector,
>> - aud_cntrl_st2, eldv,
>> - aud_cntl_st, IBX_ELD_ADDRESS_MASK,
>> - hdmiw_hdmiedid))
>> - return;
>> -
>> + /* Invalidate ELD */
>> tmp = I915_READ(aud_cntrl_st2);
>> tmp &= ~eldv;
>> I915_WRITE(aud_cntrl_st2, tmp);
>>
>> + /* Reset ELD write address */
>> tmp = I915_READ(aud_cntl_st);
>> tmp &= ~IBX_ELD_ADDRESS_MASK;
>> I915_WRITE(aud_cntl_st, tmp);
>
> same as last patch... this instruction doesn't appear at spec at this point.
>
>>
>> - len = min_t(int, eld[2], 21); /* 84 bytes of hw ELD buffer */
>> - DRM_DEBUG_DRIVER("ELD size %d\n", len);
>> + /* Up to 84 bytes of hw ELD buffer */
>> + len = min_t(int, eld[2], 21);
>> for (i = 0; i < len; i++)
>> I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + i));
> neither this.
>
>>
>> + /* ELD valid */
>> tmp = I915_READ(aud_cntrl_st2);
>> tmp |= eldv;
>> I915_WRITE(aud_cntrl_st2, tmp);
>> +
>> + /* Enable timestamps */
>> + tmp = I915_READ(aud_config);
>> + tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> + tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
>> + if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
>> + tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> + else
>> + tmp |= audio_config_hdmi_pixel_clock(mode);
>> + I915_WRITE(aud_config, tmp);
>> }
>>
>> /**
>> --
>> 2.1.1
>>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

--
Jani Nikula, Intel Open Source Technology Center