Re: [Intel-gfx] [PATCH 8/9] drm/i915/mtl: Add C10 phy programming for HDMI

[Imre Deak <imre.deak@intel.com>]

On Wed, Apr 12, 2023 at 03:49:24PM -0700, Radhakrishna Sripada wrote:
> [...]
> +/* Precomputed C10 HDMI PLL tables */
> +static const struct intel_c10pll_state mtl_c10_hdmi_25175 = {
> +	.clock = 25175,
> +	.tx = 0x10,
> +	.cmn = 0x1,
> +	.pll[0] = 0x34,
> +	.pll[1] = 0x00,
> +	.pll[2] = 0xB0,
> +	.pll[3] = 0x00,
> +	.pll[4] = 0x00,
> +	.pll[5] = 0x00,
> +	.pll[6] = 0x00,
> +	.pll[7] = 0x00,
> +	.pll[8] = 0x20,
> +	.pll[9] = 0xFF,
> +	.pll[10] = 0xFF,
> +	.pll[11] = 0x55,
> +	.pll[12] = 0xE5,
> +	.pll[13] = 0x55,
> +	.pll[14] = 0x55,
> +	.pll[15] = 0x0D,
> +	.pll[16] = 0x09,
> +	.pll[17] = 0x8F,
> +	.pll[18] = 0x84,
> +	.pll[19] = 0x23,
> +};

Something off with the above table, 
intel_c10pll_calc_port_clock() calculates 25200 clock rate for it. So
either .clock above needs to be the same rate, or the PLL params need to
be corrected for the 25175 rate.

> [...]
> @@ -690,9 +1315,20 @@ static void intel_c10pll_update_pll(struct intel_crtc_state *crtc_state,
>  static int intel_c10pll_calc_state(struct intel_crtc_state *crtc_state,
>  				   struct intel_encoder *encoder)
>  {
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  	const struct intel_c10pll_state * const *tables;
> +	enum phy phy = intel_port_to_phy(i915, encoder->port);
>  	int i;
>  
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> +		if (intel_c10_phy_check_hdmi_link_rate(crtc_state->port_clock)
> +		    != MODE_OK) {
> +			drm_dbg_kms(&i915->drm, "Can't support HDMI link rate %d on phy %c.\n",
> +				    crtc_state->port_clock, phy_name(phy));
> +			return -EINVAL;
> +		}
> +	}

The above check is not needed, covered already by the loop later in
the function.

> +
>  	tables = intel_c10pll_tables_get(crtc_state, encoder);
>  	if (!tables)
>  		return -EINVAL;