Re: [PATCH 2/3] drm/dp/i915: Clean up clock configuration for HDMI audio

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

On Thu, 11 Aug 2016, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> No functional change, just clean up. Debug messages now print out clock
> units. Additionally, the configuration bits, which are 1:1 mapped to the
> clock frequency and don't convey much information are not printed out.

IMO the cleanups here are subjective, i.e. another day someone might
come in and rewrite it back to having just one code path for return
instead of many. And someone might prefer early returns for errors.

If you really wanted to clean this up, you could abstract the config
lookup based on clock, and it would be a less subjective improvement.

>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 3efce0e..9465f54 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -104,21 +104,17 @@ static u32 audio_config_hdmi_pixel_clock(const struct drm_display_mode *adjusted
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(hdmi_audio_clock); i++) {
> -		if (adjusted_mode->crtc_clock == hdmi_audio_clock[i].clock)
> -			break;
> -	}
> -
> -	if (i == ARRAY_SIZE(hdmi_audio_clock)) {
> -		DRM_DEBUG_KMS("HDMI audio pixel clock setting for %d not found, falling back to defaults\n",
> -			      adjusted_mode->crtc_clock);
> -		i = 1;
> +		if (adjusted_mode->crtc_clock == hdmi_audio_clock[i].clock) {
> +			DRM_DEBUG_KMS("Configuring audio for HDMI pixel clk %dkHz\n",
> +				      hdmi_audio_clock[i].clock);
> +			return hdmi_audio_clock[i].config;
> +		}
>  	}
>  
> -	DRM_DEBUG_KMS("Configuring HDMI audio for pixel clock %d (0x%08x)\n",
> -		      hdmi_audio_clock[i].clock,
> -		      hdmi_audio_clock[i].config);
> +	DRM_DEBUG_KMS("No config. for HDMI pixel clk %dkHz, using default %dkHz\n",

Please at least fix the typos.

> +		      adjusted_mode->crtc_clock, hdmi_audio_clock[1].clock);
>  
> -	return hdmi_audio_clock[i].config;
> +	return hdmi_audio_clock[1].config;
>  }
>  
>  static int audio_config_get_n(const struct drm_display_mode *mode, int rate)

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx