> -----Original Message-----
> From: Deak, Imre <imre.deak@xxxxxxxxx>
> Sent: Thursday, April 13, 2023 9:37 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@xxxxxxxxx>
> Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 8/9] drm/i915/mtl: Add C10 phy programming for HDMI
>
> 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.
We do have a pre-computed table for 25200 clock which has different values.
Do we skip this clock for now?
-RK
>
> > [...]
> > @@ -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;
