[PATCH] drm/i915/xe2lpd: Update C20 HDMI TMDS algorithm to include tx_misc

[Gustavo Sousa]

Quoting Jani Nikula (2024-10-21 09:29:30-03:00)
>On Thu, 17 Oct 2024, Gustavo Sousa <gustavo.sousa at intel.com> wrote:
>> There has been an update to the Bspec in which we need to set
>> tx_misc=0x5 field for C20 TX Context programming for HDMI TMDS for
>> Xe2_LPD and newer. That field is mapped to the bits 7:0 of
>> SRAM_GENERIC_<A/B>_TX_CNTX_CFG_1, which in turn translates to tx[1] of
>> our state struct. Update the algorithm to reflect this change.
>>
>> Bspec: 74489
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cx0_phy.c    | 17 ++++++++++++++---
>>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h   |  2 ++
>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> index f73d576fd99e..22184b2d5a11 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> @@ -2142,8 +2142,12 @@ static void intel_c10pll_dump_hw_state(struct drm_i915_private *i915,
>>                              i + 2, hw_state->pll[i + 2], i + 3, hw_state->pll[i + 3]);
>>  }
>>  
>> -static int intel_c20_compute_hdmi_tmds_pll(u64 pixel_clock, struct intel_c20pll_state *pll_state)
>> +static int intel_c20_compute_hdmi_tmds_pll(struct intel_encoder *encoder,
>> +                                           u64 pixel_clock,
>> +                                           struct intel_c20pll_state *pll_state)
>>  {
>> +        struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>
>No new struct drm_i915_private local variables or parameters, please.
>
>struct intel_display *display = to_intel_display(encoder);

I actually had thought about this when writing this patch, but then I
concluded that a future major refactor in the entire file would make
more sense, since we are not using struct intel_display anywhere in this
file yet...

I can send a v2 to use struct intel_display here. Or maybe I could send
a series containing preparatory patch to convert everything related to
C10/C20 to use the new struct and then this patch as the second in the
series (although those two patches would not be not totally related to
each other).

Let me know what is preferred.

--
Gustavo Sousa

>
>> +
>>          u64 datarate;
>>          u64 mpll_tx_clk_div;
>>          u64 vco_freq_shift;
>> @@ -2154,6 +2158,7 @@ static int intel_c20_compute_hdmi_tmds_pll(u64 pixel_clock, struct intel_c20pll_
>>          u64 mpll_fracn_rem;
>>          u8  mpllb_ana_freq_vco;
>>          u8  mpll_div_multiplier;
>> +        u16  tx_misc;
>>  
>>          if (pixel_clock < 25175 || pixel_clock > 600000)
>>                  return -EINVAL;
>> @@ -2171,6 +2176,11 @@ static int intel_c20_compute_hdmi_tmds_pll(u64 pixel_clock, struct intel_c20pll_
>>          mpll_div_multiplier = min_t(u8, div64_u64((vco_freq * 16 + (datarate >> 1)),
>>                                                    datarate), 255);
>>  
>> +        if (DISPLAY_VER(i915) >= 20)
>> +                tx_misc = 0x5;
>> +        else
>> +                tx_misc = 0x0;
>> +
>>          if (vco_freq <= DATARATE_3000000000)
>>                  mpllb_ana_freq_vco = MPLLB_ANA_FREQ_VCO_3;
>>          else if (vco_freq <= DATARATE_3500000000)
>> @@ -2182,7 +2192,7 @@ static int intel_c20_compute_hdmi_tmds_pll(u64 pixel_clock, struct intel_c20pll_
>>  
>>          pll_state->clock        = pixel_clock;
>>          pll_state->tx[0]        = 0xbe88;
>> -        pll_state->tx[1]        = 0x9800;
>> +        pll_state->tx[1]        = 0x9800 | C20_PHY_TX_MISC(tx_misc);
>>          pll_state->tx[2]        = 0x0000;
>>          pll_state->cmn[0]        = 0x0500;
>>          pll_state->cmn[1]        = 0x0005;
>> @@ -2266,7 +2276,8 @@ static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
>>  
>>          /* try computed C20 HDMI tables before using consolidated tables */
>>          if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
>> -                if (intel_c20_compute_hdmi_tmds_pll(crtc_state->port_clock,
>> +                if (intel_c20_compute_hdmi_tmds_pll(encoder,
>> +                                                    crtc_state->port_clock,
>
>Alternatively you could just pass crtc_state. *shrug*.
>
>>                                                      &crtc_state->dpll_hw_state.cx0pll.c20) == 0)
>>                          return 0;
>>          }
>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> index ab3ae110b68f..c1949aa1b909 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> @@ -280,6 +280,8 @@
>>  #define PHY_C20_B_TX_CNTX_CFG(i915, idx) \
>>                  ((_IS_XE2HPD_C20(i915) ? _XE2HPD_C20_B_TX_CNTX_CFG : _MTL_C20_B_TX_CNTX_CFG) - (idx))
>>  #define   C20_PHY_TX_RATE                REG_GENMASK(2, 0)
>> +#define   C20_PHY_TX_MISC_MASK                REG_GENMASK(7, 0)
>> +#define   C20_PHY_TX_MISC(val)                REG_FIELD_PREP16(C20_PHY_TX_MISC_MASK, (val))
>
>REG_FIELD_PREP16 should have a mask of REG_GENMASK16, and all the masks
>and fields and bits for a register should all use the same width.
>
>BR,
>Jani.
>
>
>>  
>>  #define PHY_C20_A_CMN_CNTX_CFG(i915, idx) \
>>                  ((_IS_XE2HPD_C20(i915) ? _XE2HPD_C20_A_CMN_CNTX_CFG : _MTL_C20_A_CMN_CNTX_CFG) - (idx))
>
>-- 
>Jani Nikula, Intel