[PATCH 2/5] drm/i915/snps_phy: Use HDMI PLL algorithm for DG2

[Nautiyal, Ankit K]

On 6/26/2024 3:37 PM, Jani Nikula wrote:
> On Wed, 26 Jun 2024, Ankit Nautiyal <ankit.k.nautiyal at intel.com> wrote:
>> Try SNPS_PHY HDMI tables computed using the algorithm, before using
>> consolidated tables.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_snps_phy.c | 20 ++++++++-----------
>>   1 file changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c
>> index e6df1f92def5..10fe28af0d11 100644
>> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
>> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
>> @@ -12,6 +12,7 @@
>>   #include "intel_display_types.h"
>>   #include "intel_snps_phy.h"
>>   #include "intel_snps_phy_regs.h"
>> +#include "intel_pll_algorithm.h"
> Keep includes sorted.
Noted. Thanks for pointing this out.
>
>>   
>>   /**
>>    * DOC: Synopsis PHY support
>> @@ -1787,22 +1788,14 @@ intel_mpllb_tables_get(struct intel_crtc_state *crtc_state,
>>   int intel_mpllb_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_mpllb_state * const *tables;
>>   	int i;
>>   
>>   	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
>> -		if (intel_snps_phy_check_hdmi_link_rate(crtc_state->port_clock)
>> -		    != MODE_OK) {
>> -			/*
>> -			 * FIXME: Can only support fixed HDMI frequencies
>> -			 * until we have a proper algorithm under a valid
>> -			 * license.
>> -			 */
>> -			drm_dbg_kms(&i915->drm, "Can't support HDMI link rate %d\n",
>> -				    crtc_state->port_clock);
>> -			return -EINVAL;
>> -		}
>> +		/* try computed SNPS_PHY HDMI tables before using consolidated tables */
> Computed tables vs. consolidated tables? Huh?
>
> Anyway, I think we have two choices here:
>
> - Always use computed values.
>
> - Prefer fixed tables, fall back to computed values.
>
> But we definitely should not try to compute first and fall back to fixed
> tables.

Hmm I was not sure if we need the fixed tables after this and whether we 
should remove them altogether.

But it makes more sense to use prefer the fixed tables and fall back to 
computed values.

I'll make the changes in the next version.


>
>> +		if (intel_snps_phy_compute_hdmi_tmds_pll(crtc_state->port_clock,
>> +							 &crtc_state->dpll_hw_state.mpllb) == 0)
>> +			return 0;
>>   	}
>>   
>>   	tables = intel_mpllb_tables_get(crtc_state, encoder);
>> @@ -1991,6 +1984,9 @@ int intel_snps_phy_check_hdmi_link_rate(int clock)
>>   			return MODE_OK;
>>   	}
>>   
>> +	if (clock >= 25175 && clock <= 594000)
>> +		return MODE_OK;
>> +
> How's this related to the patch at hand?

Currently we prune the modes if the clock does not match that given in 
the table.

Now that we support all clocks between 25175 and 594000 we need this, 
but perhaps will add as a separate patch.

Perhaps I can remove this function all together and put the condition in 
hdmi_port_clock valid, in separate patch.

Regards,

Ankit


>
>>   	return MODE_CLOCK_RANGE;
>>   }