On Wed, 30 Mar 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Wed, Mar 30, 2022 at 12:41:08PM +0300, Jani Nikula wrote:
>> The audio codec enable/disable debug logging is spread around in callers
>> and the platform specific hooks. Put them all together in one place on
>> both the enable and disable paths.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/display/g4x_dp.c | 7 +---
>> drivers/gpu/drm/i915/display/g4x_hdmi.c | 3 --
>> drivers/gpu/drm/i915/display/intel_audio.c | 39 ++++++----------------
>> 3 files changed, 11 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
>> index 8e1338678d91..55fefc950f41 100644
>> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
>> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
>> @@ -658,9 +658,7 @@ static void intel_enable_dp(struct intel_atomic_state *state,
>> {
>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>> - struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>> u32 dp_reg = intel_de_read(dev_priv, intel_dp->output_reg);
>> - enum pipe pipe = crtc->pipe;
>> intel_wakeref_t wakeref;
>>
>> if (drm_WARN_ON(&dev_priv->drm, dp_reg & DP_PORT_EN))
>> @@ -694,11 +692,8 @@ static void intel_enable_dp(struct intel_atomic_state *state,
>> intel_dp_start_link_train(intel_dp, pipe_config);
>> intel_dp_stop_link_train(intel_dp, pipe_config);
>>
>> - if (pipe_config->has_audio) {
>> - drm_dbg(&dev_priv->drm, "Enabling DP audio on pipe %c\n",
>> - pipe_name(pipe));
>> + if (pipe_config->has_audio)
>> intel_audio_codec_enable(encoder, pipe_config, conn_state);
>> - }
>> }
>>
>> static void g4x_enable_dp(struct intel_atomic_state *state,
>> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> index 06e00b1eaa7c..39ba5dc51f8e 100644
>> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> @@ -148,11 +148,8 @@ static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
>> const struct drm_connector_state *conn_state)
>> {
>> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> - struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>>
>> drm_WARN_ON(&i915->drm, !pipe_config->has_hdmi_sink);
>> - drm_dbg_kms(&i915->drm, "Enabling HDMI audio on pipe %c\n",
>> - pipe_name(crtc->pipe));
>> intel_audio_codec_enable(encoder, pipe_config, conn_state);
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
>> index 3bdca0fe2cee..24d20817a5e5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
>> @@ -337,8 +337,6 @@ static void g4x_audio_codec_disable(struct intel_encoder *encoder,
>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> u32 eldv, tmp;
>>
>> - drm_dbg_kms(&dev_priv->drm, "Disable audio codec\n");
>> -
>> tmp = intel_de_read(dev_priv, G4X_AUD_VID_DID);
>> if (tmp == INTEL_AUDIO_DEVBLC || tmp == INTEL_AUDIO_DEVCL)
>> eldv = G4X_ELDV_DEVCL_DEVBLC;
>> @@ -362,9 +360,6 @@ static void g4x_audio_codec_enable(struct intel_encoder *encoder,
>> u32 tmp;
>> int len, i;
>>
>> - drm_dbg_kms(&dev_priv->drm, "Enable audio codec, %u bytes ELD\n",
>> - drm_eld_size(eld));
>> -
>> tmp = intel_de_read(dev_priv, G4X_AUD_VID_DID);
>> if (tmp == INTEL_AUDIO_DEVBLC || tmp == INTEL_AUDIO_DEVCL)
>> eldv = G4X_ELDV_DEVCL_DEVBLC;
>> @@ -383,7 +378,6 @@ static void g4x_audio_codec_enable(struct intel_encoder *encoder,
>> intel_de_write(dev_priv, G4X_AUD_CNTL_ST, tmp);
>>
>> len = min(drm_eld_size(eld) / 4, len);
>> - drm_dbg(&dev_priv->drm, "ELD size %d\n", len);
>> for (i = 0; i < len; i++)
>> intel_de_write(dev_priv, G4X_HDMIW_HDMIEDID,
>> *((const u32 *)eld + i));
>> @@ -501,9 +495,6 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder,
>> enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>> u32 tmp;
>>
>> - drm_dbg_kms(&dev_priv->drm, "Disable audio codec on transcoder %s\n",
>> - transcoder_name(cpu_transcoder));
>> -
>> mutex_lock(&dev_priv->audio.mutex);
>>
>> /* Disable timestamps */
>> @@ -647,10 +638,6 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
>> u32 tmp;
>> int len, i;
>>
>> - drm_dbg_kms(&dev_priv->drm,
>> - "Enable audio codec on transcoder %s, %u bytes ELD\n",
>> - transcoder_name(cpu_transcoder), drm_eld_size(eld));
>> -
>> mutex_lock(&dev_priv->audio.mutex);
>>
>> /* Enable Audio WA for 4k DSC usecases */
>> @@ -703,11 +690,6 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder,
>> u32 tmp, eldv;
>> i915_reg_t aud_config, aud_cntrl_st2;
>>
>> - drm_dbg_kms(&dev_priv->drm,
>> - "Disable audio codec on [ENCODER:%d:%s], pipe %c\n",
>> - encoder->base.base.id, encoder->base.name,
>> - pipe_name(pipe));
>> -
>> if (drm_WARN_ON(&dev_priv->drm, port == PORT_A))
>> return;
>>
>> @@ -754,11 +736,6 @@ static void ilk_audio_codec_enable(struct intel_encoder *encoder,
>> int len, i;
>> i915_reg_t hdmiw_hdmiedid, aud_config, aud_cntl_st, aud_cntrl_st2;
>>
>> - drm_dbg_kms(&dev_priv->drm,
>> - "Enable audio codec on [ENCODER:%d:%s], pipe %c, %u bytes ELD\n",
>> - encoder->base.base.id, encoder->base.name,
>> - pipe_name(pipe), drm_eld_size(eld));
>> -
>> if (drm_WARN_ON(&dev_priv->drm, port == PORT_A))
>> return;
>>
>> @@ -844,18 +821,17 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
>> enum port port = encoder->port;
>> enum pipe pipe = crtc->pipe;
>>
>> + drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s] Enable audio codec on pipe %c, %u bytes ELD\n",
>> + connector->base.id, connector->name,
>> + encoder->base.base.id, encoder->base.name,
>> + pipe, drm_eld_size(connector->eld));
>
> Could also use the [CRTC:%d:%s] format for the pipe.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Pushed to din, left the nitpicks for another time, thanks,
BR,
Jani.
>
>> +
>> /* FIXME precompute the ELD in .compute_config() */
>> if (!connector->eld[0])
>> drm_dbg_kms(&dev_priv->drm,
>> "Bogus ELD on [CONNECTOR:%d:%s]\n",
>> connector->base.id, connector->name);
>>
>> - drm_dbg(&dev_priv->drm, "ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
>> - connector->base.id,
>> - connector->name,
>> - encoder->base.base.id,
>> - encoder->base.name);
>> -
>> connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
>>
>> if (dev_priv->audio.funcs)
>> @@ -900,9 +876,14 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
>> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>> struct i915_audio_component *acomp = dev_priv->audio.component;
>> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->uapi.crtc);
>> + struct drm_connector *connector = old_conn_state->connector;
>> enum port port = encoder->port;
>> enum pipe pipe = crtc->pipe;
>>
>> + drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s][ENCODER:%d:%s] Disable audio codec on pipe %c\n",
>> + connector->base.id, connector->name,
>> + encoder->base.base.id, encoder->base.name, pipe);
>> +
>> if (dev_priv->audio.funcs)
>> dev_priv->audio.funcs->audio_codec_disable(encoder,
>> old_crtc_state,
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
