On Thu, 11 Aug 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> wrote:
> No functional change, code clean up and improved debug.
>
> Chris suggested this code snippet while reviewing, I just made this into a
> patch.
>
> Credits-to: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_audio.c | 28 ++++++++++++----------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index d32f586..3efce0e 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -282,14 +282,9 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> enum pipe pipe = intel_crtc->pipe;
> - struct i915_audio_component *acomp = dev_priv->audio_component;
> const uint8_t *eld = connector->eld;
> - struct intel_digital_port *intel_dig_port =
> - enc_to_dig_port(&encoder->base);
> - enum port port = intel_dig_port->port;
> uint32_t tmp;
> int len, i;
> - int n, rate;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> pipe_name(pipe), drm_eld_size(eld));
> @@ -335,19 +330,20 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
>
> tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
> if (audio_rate_need_prog(intel_crtc, adjusted_mode)) {
> - if (!acomp)
> - rate = 0;
> - else if (port >= PORT_A && port <= PORT_E)
> + enum port port = enc_to_dig_port(&encoder->base)->port;
> + struct i915_audio_component *acomp = dev_priv->audio_component;
> + int rate, n;
Moving these here is probably fine, although originally I was hoping we
would always program the N, and thus wouldn't have this if
(audio_rate_need_prog(intel_crtc, adjusted_mode)) block here at all. The
idea was that we'd always use the same method instead of letting the hw
set it automatically for some rates, and manually setting others.
> +
> + rate = 0;
> + if (acomp && port >= PORT_A && port <= PORT_E) {
> rate = acomp->aud_sample_rate[port];
> - else {
> - DRM_ERROR("invalid port: %d\n", port);
I think I may have seen these in some dmesgs in otherwise unrelated
bugs. I'd like to preserve this at least as a debug breadcrumb.
> - rate = 0;
> +
> + n = audio_config_get_n(adjusted_mode, rate);
> + DRM_DEBUG_KMS("port %d audio rate %d => N=%x\n",
> + port, rate, n);
> + if (n)
> + tmp = audio_config_setup_n_reg(n, tmp);
> }
> - n = audio_config_get_n(adjusted_mode, rate);
> - if (n != 0)
> - tmp = audio_config_setup_n_reg(n, tmp);
> - else
> - DRM_DEBUG_KMS("no suitable N value is found\n");
Same here, I'd like to preserve the debug.
> }
>
> I915_WRITE(HSW_AUD_CFG(pipe), tmp);
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
