On Fri, 25 Jul 2025, Suraj Kandpal <suraj.kand...@intel.com> 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.
>
> Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 58 +++++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_bios.h | 2 +
> 2 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 8337ebe0f2c8..3eac804b2449 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2480,6 +2480,54 @@ static int parse_bdb_216_dp_max_link_rate(const int
> vbt_max_link_rate)
> }
> }
>
> +static bool parse_bdb_263_edp_data_override(const u32
> edp_data_override_mask, int rate)
We don't really use const like this anywhere.
> +{
> + u32 val;
> +
> + switch (rate) {
> + case 2000000:
> + val = BDB_263_VBT_EDP_LINK_RATE_20;
> + break;
> + case 1350000:
> + val = BDB_263_VBT_EDP_LINK_RATE_13_5;
> + break;
> + case 1000000:
> + val = BDB_263_VBT_EDP_LINK_RATE_10;
> + break;
> + case 810000:
> + val = BDB_263_VBT_EDP_LINK_RATE_8_1;
> + break;
> + case 675000:
> + val = BDB_263_VBT_EDP_LINK_RATE_6_75;
> + break;
> + case 540000:
> + val = BDB_263_VBT_EDP_LINK_RATE_5_4;
> + break;
> + case 432000:
> + val = BDB_263_VBT_EDP_LINK_RATE_4_32;
> + break;
> + case 324000:
> + val = BDB_263_VBT_EDP_LINK_RATE_3_24;
> + break;
> + case 270000:
> + val = BDB_263_VBT_EDP_LINK_RATE_2_7;
> + break;
> + case 243000:
> + val = BDB_263_VBT_EDP_LINK_RATE_2_43;
> + break;
> + case 216000:
> + val = BDB_263_VBT_EDP_LINK_RATE_2_16;
> + break;
> + case 162000:
> + val = BDB_263_VBT_EDP_LINK_RATE_1_62;
> + break;
> + default:
> + break;
> + }
> +
> + return edp_data_override_mask & val;
I think you're better off having a helper function that returns the mask
for a rate, possibly 0, that you don't pass edp_data_override_mask at
all.
> +}
> +
> int intel_bios_dp_max_link_rate(const struct intel_bios_encoder_data
> *devdata)
> {
> if (!devdata || devdata->display->vbt.version < 216)
> @@ -2499,6 +2547,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_need_edp_data_override(const struct intel_bios_encoder_data
> *devdata,
> + int rate)
intel_bios_encoder_supports_edp_rate() or something, "need edp data
override" doesn't mean anything.
> +{
> + if (!devdata || devdata->display->vbt.version < 263)
> + return false;
> +
> + return
> parse_bdb_263_edp_data_override(devdata->child.edp_data_override, rate);
This would become:
return devdata->child.edp_data_override & edp_rate_override_mask(rate);
except negated with the positive function naming.
Also child.edp_data_override is a misleading name, because it's about
data *rate* override, not some random data override.
> +}
> +
> 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..aeba0e9b5228 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -274,5 +274,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_need_edp_data_override(const struct intel_bios_encoder_data
> *devdata, int rate);
Place this next to the other intel_bios_encoder_supports_* declarations.
BR,
Jani.
>
> #endif /* _INTEL_BIOS_H_ */
--
Jani Nikula, Intel
