From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/audio: move has_audio checks to within codec enable/disable
Date: Wed, 30 Mar 2022 14:15:45 +0300 [thread overview]
Message-ID: <YkQ74bnGzk7cd9jA@intel.com> (raw)
In-Reply-To: <20220330094109.4164326-2-jani.nikula@intel.com>
On Wed, Mar 30, 2022 at 12:41:09PM +0300, Jani Nikula wrote:
> Reduce duplication.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/display/g4x_dp.c | 7 ++---
> drivers/gpu/drm/i915/display/g4x_hdmi.c | 33 +++++++--------------
> drivers/gpu/drm/i915/display/intel_audio.c | 6 ++++
> drivers/gpu/drm/i915/display/intel_ddi.c | 14 +++------
> drivers/gpu/drm/i915/display/intel_dp_mst.c | 8 ++---
> 5 files changed, 26 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 55fefc950f41..16bb21ad898b 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -495,9 +495,7 @@ static void intel_disable_dp(struct intel_atomic_state *state,
>
> intel_dp->link_trained = false;
>
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
>
> /*
> * Make sure the panel is off before trying to change the mode.
> @@ -692,8 +690,7 @@ 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)
> - intel_audio_codec_enable(encoder, pipe_config, conn_state);
> + 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 39ba5dc51f8e..8bfef08b7c43 100644
> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> @@ -143,16 +143,6 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> &pipe_config->infoframes.hdmi);
> }
>
> -static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
> - const struct intel_crtc_state *pipe_config,
> - const struct drm_connector_state *conn_state)
> -{
> - struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -
> - drm_WARN_ON(&i915->drm, !pipe_config->has_hdmi_sink);
> - intel_audio_codec_enable(encoder, pipe_config, conn_state);
> -}
> -
> static void g4x_enable_hdmi(struct intel_atomic_state *state,
> struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config,
> @@ -172,8 +162,9 @@ static void g4x_enable_hdmi(struct intel_atomic_state *state,
> intel_de_write(dev_priv, intel_hdmi->hdmi_reg, temp);
> intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
>
> - if (pipe_config->has_audio)
> - intel_enable_hdmi_audio(encoder, pipe_config, conn_state);
> + drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
> + !pipe_config->has_hdmi_sink);
I'd probably just drop these warns. We don't have them for hsw+ either.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> }
>
> static void ibx_enable_hdmi(struct intel_atomic_state *state,
> @@ -224,8 +215,9 @@ static void ibx_enable_hdmi(struct intel_atomic_state *state,
> intel_de_posting_read(dev_priv, intel_hdmi->hdmi_reg);
> }
>
> - if (pipe_config->has_audio)
> - intel_enable_hdmi_audio(encoder, pipe_config, conn_state);
> + drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
> + !pipe_config->has_hdmi_sink);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> }
>
> static void cpt_enable_hdmi(struct intel_atomic_state *state,
> @@ -278,8 +270,9 @@ static void cpt_enable_hdmi(struct intel_atomic_state *state,
> intel_de_read(dev_priv, TRANS_CHICKEN1(pipe)) & ~TRANS_CHICKEN1_HDMIUNIT_GC_DISABLE);
> }
>
> - if (pipe_config->has_audio)
> - intel_enable_hdmi_audio(encoder, pipe_config, conn_state);
> + drm_WARN_ON(&dev_priv->drm, pipe_config->has_audio &&
> + !pipe_config->has_hdmi_sink);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
> }
>
> static void vlv_enable_hdmi(struct intel_atomic_state *state,
> @@ -353,9 +346,7 @@ static void g4x_disable_hdmi(struct intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state *old_conn_state)
> {
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
>
> intel_disable_hdmi(state, encoder, old_crtc_state, old_conn_state);
> }
> @@ -365,9 +356,7 @@ static void pch_disable_hdmi(struct intel_atomic_state *state,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state *old_conn_state)
> {
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
> }
>
> static void pch_post_disable_hdmi(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
> index 24d20817a5e5..1c87bf66b092 100644
> --- a/drivers/gpu/drm/i915/display/intel_audio.c
> +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> @@ -821,6 +821,9 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
> enum port port = encoder->port;
> enum pipe pipe = crtc->pipe;
>
> + if (!crtc_state->has_audio)
> + return;
> +
> 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,
> @@ -880,6 +883,9 @@ void intel_audio_codec_disable(struct intel_encoder *encoder,
> enum port port = encoder->port;
> enum pipe pipe = crtc->pipe;
>
> + if (!old_crtc_state->has_audio)
> + return;
> +
> 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);
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index dc208df829f1..cec578efc4bd 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2819,8 +2819,7 @@ static void intel_enable_ddi_dp(struct intel_atomic_state *state,
> if (!dig_port->lspcon.active || dig_port->dp.has_hdmi_sink)
> intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
>
> - if (crtc_state->has_audio)
> - intel_audio_codec_enable(encoder, crtc_state, conn_state);
> + intel_audio_codec_enable(encoder, crtc_state, conn_state);
>
> trans_port_sync_stop_link_train(state, encoder, crtc_state);
> }
> @@ -2914,8 +2913,7 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
> intel_de_write(dev_priv, DDI_BUF_CTL(port),
> dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE);
>
> - if (crtc_state->has_audio)
> - intel_audio_codec_enable(encoder, crtc_state, conn_state);
> + intel_audio_codec_enable(encoder, crtc_state, conn_state);
> }
>
> static void intel_enable_ddi(struct intel_atomic_state *state,
> @@ -2956,9 +2954,7 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>
> intel_dp->link_trained = false;
>
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
>
> intel_psr_disable(intel_dp, old_crtc_state);
> intel_edp_backlight_off(old_conn_state);
> @@ -2978,9 +2974,7 @@ static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
> struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> struct drm_connector *connector = old_conn_state->connector;
>
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
>
> if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> false, false))
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index e30e698aa684..061b277e5ce7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -398,9 +398,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
> if (ret) {
> drm_dbg_kms(&i915->drm, "failed to update payload %d\n", ret);
> }
> - if (old_crtc_state->has_audio)
> - intel_audio_codec_disable(encoder,
> - old_crtc_state, old_conn_state);
> +
> + intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
> }
>
> static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
> @@ -599,8 +598,7 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>
> intel_crtc_vblank_on(pipe_config);
>
> - if (pipe_config->has_audio)
> - intel_audio_codec_enable(encoder, pipe_config, conn_state);
> + intel_audio_codec_enable(encoder, pipe_config, conn_state);
>
> /* Enable hdcp if it's desired */
> if (conn_state->content_protection ==
> --
> 2.30.2
--
Ville Syrjälä
Intel
next prev parent reply other threads:[~2022-03-30 11:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-30 9:41 [Intel-gfx] [PATCH 1/2] drm/i915/audio: unify audio codec enable/disable debug logging Jani Nikula
2022-03-30 9:41 ` [Intel-gfx] [PATCH 2/2] drm/i915/audio: move has_audio checks to within codec enable/disable Jani Nikula
2022-03-30 11:15 ` Ville Syrjälä [this message]
2022-03-30 11:12 ` [Intel-gfx] [PATCH 1/2] drm/i915/audio: unify audio codec enable/disable debug logging Ville Syrjälä
2022-03-30 16:37 ` Jani Nikula
2022-03-30 11:51 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] " Patchwork
2022-03-30 12:27 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-03-30 15:22 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YkQ74bnGzk7cd9jA@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.