Re: [PATCH 11/12] drm/i915/panel: Add helper to get highest fixed mode

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 8/11/2025 8:52 PM, Jani Nikula wrote:
> On Thu, 07 Aug 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> Add intel_panel_highest_fixed_mode() to return the fixed mode with the
>> highest pixel clock. Unlike intel_panel_highest_mode(), this function
>> does not fall back to the adjusted mode and returns NULL if no fixed
>> modes are available.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_panel.c | 13 +++++++++++++
>>   drivers/gpu/drm/i915/display/intel_panel.h |  2 ++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
>> index 2a20aaaaac39..ea4351d11e63 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
>> @@ -158,6 +158,19 @@ intel_panel_highest_mode(struct intel_connector *connector,
>>   	return best_mode;
>>   }
>>   
>> +const struct drm_display_mode *
>> +intel_panel_highest_fixed_mode(struct intel_connector *connector)
>> +{
>> +	const struct drm_display_mode *fixed_mode, *highest_mode = NULL;
>> +
>> +	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
>> +		if (!highest_mode || fixed_mode->clock > highest_mode->clock)
>> +			highest_mode = fixed_mode;
>> +	}
>> +
>> +	return highest_mode;
>> +}
> The difference in naming here does not adequately describe the
> difference in the functions.
>
> Or you could just make the single user of intel_panel_highest_mode()
> fall back to adjusted mode when the return value is NULL, and avoid that
> problem altogether.

Agreed. I can refactor the existing function and fix the caller to check 
for NULL.

Also there is a fix me in the caller where we are using mode->clock 
instead of mode->crtc_clock which can be fixed with the above refactor.

With the refactor if highest fixed mode is found, mode->clock can be 
used, otherwise adjusted_mode->crtc_clock can be used.

Regards,

Ankit


>
> BR,
> Jani.
>
>> +
>>   int intel_panel_get_modes(struct intel_connector *connector)
>>   {
>>   	const struct drm_display_mode *fixed_mode;
>> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
>> index 56a6412cf0fb..60f6873cdbaa 100644
>> --- a/drivers/gpu/drm/i915/display/intel_panel.h
>> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
>> @@ -39,6 +39,8 @@ intel_panel_downclock_mode(struct intel_connector *connector,
>>   const struct drm_display_mode *
>>   intel_panel_highest_mode(struct intel_connector *connector,
>>   			 const struct drm_display_mode *adjusted_mode);
>> +const struct drm_display_mode *
>> +intel_panel_highest_fixed_mode(struct intel_connector *connector);
>>   int intel_panel_get_modes(struct intel_connector *connector);
>>   enum drrs_type intel_panel_drrs_type(struct intel_connector *connector);
>>   enum drm_mode_status