Re: [PATCH 01/10] drm/i915/display: Add helper to compute link M/N ratio for reuse

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 11 Sep 2026, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 11 Sep 2026, Suraj Kandpal <suraj.kandpal@intel.com> wrote:
>> From: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>
>> Introduce `intel_display_get_link_m_n()` to compute the link M/N ratio
>
> Please drop the backticks. IMO it's just ugly fluff in commit messages.
>
>> based on pixel clock and link clock. This logic was previously embedded
>> within `intel_link_compute_m_n()`.
>>
>> The new helper allows reuse of link M/N computation in other contexts
>> without requiring the full data M/N calculation.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index fc30a455bed3..b5682c196646 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -2633,13 +2633,24 @@ static void compute_m_n(u32 *ret_m, u32 *ret_n,
>>  	intel_reduce_m_n_ratio(ret_m, ret_n);
>>  }
>>  
>> +static
>> +void intel_display_get_link_m_n(u32 *link_m, u32 *link_n,
>> +				u32 pixel_clock,
>> +				u32 link_clock)
>
> Now we have two functions:
>
> - intel_display_get_link_m_n()
> - intel_link_compute_m_n()
>
> in which one is a part of the other, but the names are quite different.
>
> Why? Shouldn't the names be somewhat similar?
>
> BR,
> Jani.
>
>
>> +{
>> +	u32 link_symbol_clock = intel_dp_link_symbol_clock(link_clock);

Moreover, this is now DP specific.

Maybe you need a different abstraction level to keep the DP link symbol
clock stuff inside intel_dp.c.

BR,
Jani.

>> +
>> +	compute_m_n(link_m, link_n,
>> +		    pixel_clock, link_symbol_clock,
>> +		    0x80000);
>> +}
>> +
>>  void
>>  intel_link_compute_m_n(u16 bits_per_pixel_x16, int nlanes,
>>  		       int pixel_clock, int link_clock,
>>  		       int bw_overhead,
>>  		       struct intel_link_m_n *m_n)
>>  {
>> -	u32 link_symbol_clock = intel_dp_link_symbol_clock(link_clock);
>>  	u32 data_m = intel_dp_effective_data_rate(pixel_clock, bits_per_pixel_x16,
>>  						  bw_overhead);
>>  	u32 data_n = drm_dp_max_dprx_data_rate(link_clock, nlanes);
>> @@ -2656,9 +2667,8 @@ intel_link_compute_m_n(u16 bits_per_pixel_x16, int nlanes,
>>  		    data_m, data_n,
>>  		    0x8000000);
>>  
>> -	compute_m_n(&m_n->link_m, &m_n->link_n,
>> -		    pixel_clock, link_symbol_clock,
>> -		    0x80000);
>> +	intel_display_get_link_m_n(&m_n->link_m, &m_n->link_n,
>> +				   pixel_clock, link_clock);
>>  }
>>  
>>  void intel_panel_sanitize_ssc(struct intel_display *display)

-- 
Jani Nikula, Intel