[Intel-gfx] [PATCH] drm/i915/display: Audio keep alive timestamp cdclk divisors

[Kai Vehmanen]

Hi,

On Fri, 22 Jul 2022, Taylor, Clinton A wrote:

> Use BSPEC values for the Audio Keep alive M and N values as included in
> the cdclk BSPEC pages for display > 13

looks good, only a few very minor style comments. 

I had a table in my original patch, but Ville preferred calculating 
on-the-fly back then, so I changed the patch.

Now the recommended values have changed, so table again seems more 
fit for purpose. But yeah, if ok to Ville, ok for me as well.

>  static const struct intel_cdclk_vals adlp_cdclk_table[] = {
> -	{ .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27 },
> -	{ .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20 },
> -	{ .refclk = 19200, .cdclk = 307200, .divider = 2, .ratio = 32 },
> -	{ .refclk = 19200, .cdclk = 556800, .divider = 2, .ratio = 58 },
> -	{ .refclk = 19200, .cdclk = 652800, .divider = 2, .ratio = 68 },
> -
> -	{ .refclk = 24000, .cdclk = 176000, .divider = 3, .ratio = 22 },
> -	{ .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16 },
> -	{ .refclk = 24000, .cdclk = 312000, .divider = 2, .ratio = 26 },
> -	{ .refclk = 24000, .cdclk = 552000, .divider = 2, .ratio = 46 },
> -	{ .refclk = 24400, .cdclk = 648000, .divider = 2, .ratio = 54 },
> -
> -	{ .refclk = 38400, .cdclk = 179200, .divider = 3, .ratio = 14 },
> -	{ .refclk = 38400, .cdclk = 192000, .divider = 2, .ratio = 10 },
> -	{ .refclk = 38400, .cdclk = 307200, .divider = 2, .ratio = 16 },
> -	{ .refclk = 38400, .cdclk = 556800, .divider = 2, .ratio = 29 },
> -	{ .refclk = 38400, .cdclk = 652800, .divider = 2, .ratio = 34 },
> +	{ .refclk = 19200, .cdclk = 172800, .divider = 3, .ratio = 27, .aud_m = 0x3C, .aud_n = 0x140 },
> +	{ .refclk = 19200, .cdclk = 192000, .divider = 2, .ratio = 20, .aud_m = 0x3C, .aud_n = 0x1E0 },

Minor nit, but lower-case letters are used elsewhere for hex constants, so 
would look nicer to use 0x3c et al for the aud_m/n fields as well.

> +	{ .refclk = 24000, .cdclk = 192000, .divider = 2, .ratio = 16, .aud_m = 0x3C, .aud_n = 0x1E0  },

Another minor nit, extra space at end.

> +	{ .refclk = 38400, .cdclk = 163200, .divider = 2, .ratio = 34, .waveform = 0x8888, .aud_m = 0x3C, .aud_n = 0x198 },
> +	{ .refclk = 38400, .cdclk = 204000, .divider = 2, .ratio = 34, .waveform = 0x9248, .aud_m = 0x3C, .aud_n = 0x1FE },
> +	{ .refclk = 38400, .cdclk = 244800, .divider = 2, .ratio = 34, .waveform = 0xa4a4, .aud_m = 0x3C, .aud_n = 0x264 },

Ditto here for hex constant style. Looks even more unpleasent when 
.waveform is initialized with lower-case style.

Br, kai