[PATCH v5 3/3] drm/i915/edp: eDP Data Overrride

[Nautiyal, Ankit K]

On 8/19/2025 5:33 PM, Kandpal, Suraj wrote:
>> On Tue, 19 Aug 2025, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
>>> We need override certain link rates in favour of the next available
>>> higher link rate. The Link rates that need to be overridden are
>>> indicated by a mask in VBT. To make sure these modes are skipped we
>>> don't add them in them in the sink rates array.
>>>
>>> --v2
>>> -Update the link rates after we have a final set of link rates [Ankit]
>>> -Break this patch up [Ankit] -Optimize the assingment during loop
>>> [Ankit]
>>>
>>> --v3
>>> -Add protection against broken VBTs [Jani]
>>>
>>> --v4
>>> -Fix build errors
>>> -Create a seprate function to check if edp data override is selected
>>> and using the correct vbt
>>>
>>> --v5
>>> -Use correct number to check the num of edp rates [Ankit]
>>>
>>> Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_bios.c | 15 ++++++++++++++-
>>> drivers/gpu/drm/i915/display/intel_bios.h |  2 ++
>>>   drivers/gpu/drm/i915/display/intel_dp.c   | 22 ++++++++++++++++++++++
>>>   3 files changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
>>> b/drivers/gpu/drm/i915/display/intel_bios.c
>>> index 444ed54f7c35..05a74c3bc9af 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>>> @@ -2521,11 +2521,24 @@ int intel_bios_dp_max_lane_count(const struct
>>> intel_bios_encoder_data *devdata)  bool
>>> intel_bios_encoder_reject_edp_rate(const struct intel_bios_encoder_data
>> *devdata,
>>>   				   int rate)
>>> +{
>>> +	return devdata->child.edp_data_rate_override &
>>> +edp_rate_override_mask(rate); }
>>> +
>>> +bool
>>> +intel_bios_vbt_supports_edp_data_override(const struct
>>> +intel_bios_encoder_data *devdata)
>> Why are you adding this function? Earlier versions didn't have it, and the reason
>> for its existence isn't explained or clear.
>>
> This function was added in v3 (I did add a small mention in the commit message I can maybe elaborate more there)
>   in response to one of your comments in which you mentioned the
> Possibility of ending up with a broken vbt that masked all the link rates causing us to have no link rate
> To drive the edp with this function now checks if we have at least one edp link rate using which we can
> Drive edp if not we do not modify the list of sink rates at all.

Rethinking about this, instead of hweight() perhaps we can define and 
use mask:

#define BDB_263_EDP_NUM_LINK_RATES  12
#define BDB_263_EDP_ALL_LINK_RATES_MASK 
GENMASK(BDB_263_EDP_NUM_LINK_RATES - 1, 0)


Then this can be used as:
if (devdata->child.edp_data_rate_override == 
BDB_263_EDP_ALL_LINK_RATES_MASK)
     return false;

This will avoid hard coding to 12 and will be more intuitive I guess.

Regards,

Ankit

>
> Regards,
> Suraj Kandpal
>
>> BR,
>> Jani.
>>
>>
>>>   {
>>>   	if (!devdata || devdata->display->vbt.version < 263)
>>>   		return false;
>>>
>>> -	return devdata->child.edp_data_rate_override &
>> edp_rate_override_mask(rate);
>>> +	/*
>>> +	 * This means the VBT ends up asking us to override every possible rate
>>> +	 * indicating the VBT is broken so skip this
>>> +	 */
>>> +	if (hweight32(devdata->child.edp_data_rate_override) >=
>> BDB_263_VBT_EDP_NUM_RATES)
>>> +		return false;
>>> +
>>> +	return true;
>>>   }
>>>
>>>   static void sanitize_device_type(struct intel_bios_encoder_data
>>> *devdata, diff --git a/drivers/gpu/drm/i915/display/intel_bios.h
>>> b/drivers/gpu/drm/i915/display/intel_bios.h
>>> index 781e08f7eeb2..d24660bcc7f3 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bios.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
>>> @@ -276,5 +276,7 @@ void intel_bios_for_each_encoder(struct intel_display
>> *display,
>>>   					      const struct
>> intel_bios_encoder_data *devdata));
>>>   void intel_bios_debugfs_register(struct intel_display *display);
>>> +bool
>>> +intel_bios_vbt_supports_edp_data_override(const struct
>>> +intel_bios_encoder_data *devdata);
>>>
>>>   #endif /* _INTEL_BIOS_H_ */
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index 54d88f24b689..f6fad42182ae 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -4277,6 +4277,26 @@ static void intel_edp_mso_init(struct intel_dp
>> *intel_dp)
>>>   	intel_dp->mso_pixel_overlap = mso ? info->mso_pixel_overlap : 0;  }
>>>
>>> +static void
>>> +intel_edp_set_data_override_rates(struct intel_dp *intel_dp) {
>>> +	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>>> +	int *sink_rates = intel_dp->sink_rates;
>>> +	int i, j = 0;
>>> +
>>> +	if (!intel_bios_vbt_supports_edp_data_override(encoder->devdata))
>>> +		return;
>>> +
>>> +	for (i = 0; i < intel_dp->num_sink_rates; i++) {
>>> +		if (intel_bios_encoder_reject_edp_rate(encoder->devdata,
>>> +						       intel_dp->sink_rates[i]))
>>> +			continue;
>>> +
>>> +		sink_rates[j++] = intel_dp->sink_rates[i];
>>> +	}
>>> +	intel_dp->num_sink_rates = j;
>>> +}
>>> +
>>>   static void
>>>   intel_edp_set_sink_rates(struct intel_dp *intel_dp)  { @@ -4327,6
>>> +4347,8 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>>   		intel_dp->use_rate_select = true;
>>>   	else
>>>   		intel_dp_set_sink_rates(intel_dp);
>>> +
>>> +	intel_edp_set_data_override_rates(intel_dp);
>>>   }
>>>
>>>   static bool
>> --
>> Jani Nikula, Intel