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

[Jani Nikula <jani.nikula@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx