Re: [PATCH v3 2/3] drm/i915/bios: Add function to check if edp data override is needed

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

On 7/31/2025 10:46 AM, Suraj Kandpal wrote:
> Add a function that helps identify if the rate provided needs to
> be overridden. For this we need a function that compares the rate
> provided and bitmask of rates provided in VBT.
>
> --v2
> -Rename functions [Jani]
> -Return the mask instead of parsing it in function [Jani]
> -Move the declaration in header [Jani]
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_bios.c | 29 +++++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_bios.h |  2 ++
>   2 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 8337ebe0f2c8..7adb7c4b0432 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2480,6 +2480,25 @@ static int parse_bdb_216_dp_max_link_rate(const int vbt_max_link_rate)
>   	}
>   }
>   
> +static u32 edp_rate_override_mask(int rate)
> +{
> +	switch (rate) {
> +	case 2000000: return BDB_263_VBT_EDP_LINK_RATE_20;
> +	case 1350000: return BDB_263_VBT_EDP_LINK_RATE_13_5;
> +	case 1000000: return BDB_263_VBT_EDP_LINK_RATE_10;
> +	case 810000: return BDB_263_VBT_EDP_LINK_RATE_8_1;
> +	case 675000: return BDB_263_VBT_EDP_LINK_RATE_6_75;
> +	case 540000: return BDB_263_VBT_EDP_LINK_RATE_5_4;
> +	case 432000: return BDB_263_VBT_EDP_LINK_RATE_4_32;
> +	case 324000: return BDB_263_VBT_EDP_LINK_RATE_3_24;
> +	case 270000: return BDB_263_VBT_EDP_LINK_RATE_2_7;
> +	case 243000: return BDB_263_VBT_EDP_LINK_RATE_2_43;
> +	case 216000: return BDB_263_VBT_EDP_LINK_RATE_2_16;
> +	case 162000: return BDB_263_VBT_EDP_LINK_RATE_1_62;
> +	default: return 0;
> +	}
> +}
> +
>   int intel_bios_dp_max_link_rate(const struct intel_bios_encoder_data *devdata)
>   {
>   	if (!devdata || devdata->display->vbt.version < 216)
> @@ -2499,6 +2518,16 @@ int intel_bios_dp_max_lane_count(const struct intel_bios_encoder_data *devdata)
>   	return devdata->child.dp_max_lane_count + 1;
>   }
>   
> +bool
> +intel_bios_encoder_supports_edp_rate(const struct intel_bios_encoder_data *devdata,
> +				     int rate)
> +{
> +	if (!devdata || devdata->display->vbt.version < 263)
> +		return false;
> +
> +	return devdata->child.edp_data_rate_override & edp_rate_override_mask(rate);


The function name intel_bios_encoder_supports_edp_rate() suggests that 
it returns true if a given eDP rate is supported.
However, the current implementation returns true when the corresponding 
bit is set in edp_data_rate_override,
which actually indicates that the rate is rejected.

To avoid confusion, either the function should be renamed to 
intel_bios_encoder_rejects_edp_rate()
to reflect its actual behavior, or the logic should be inverted so that 
the function returns true
only when the rate is acceptable.

Regards,

Ankit

> +}
> +
>   static void sanitize_device_type(struct intel_bios_encoder_data *devdata,
>   				 enum port port)
>   {
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index 6cd7a011b8c4..a4abaa89a682 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -251,6 +251,8 @@ bool intel_bios_encoder_supports_dvi(const struct intel_bios_encoder_data *devda
>   bool intel_bios_encoder_supports_hdmi(const struct intel_bios_encoder_data *devdata);
>   bool intel_bios_encoder_supports_dp(const struct intel_bios_encoder_data *devdata);
>   bool intel_bios_encoder_supports_edp(const struct intel_bios_encoder_data *devdata);
> +bool intel_bios_encoder_supports_edp_rate(const struct intel_bios_encoder_data *devdata,
> +					  int rate);
>   bool intel_bios_encoder_supports_typec_usb(const struct intel_bios_encoder_data *devdata);
>   bool intel_bios_encoder_supports_tbt(const struct intel_bios_encoder_data *devdata);
>   bool intel_bios_encoder_supports_dsi(const struct intel_bios_encoder_data *devdata);