Re: [PATCH 01/44] drm/i915/hdmi: Parse frl max link rate from vbt

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

On Fri, 07 Aug 2026, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> From: Vandita Kulkarni <vandita.kulkarni@intel.com>
>
> Parse the maximum FRL link rate from VBT and store it for use
> during FRL mode selection and link training.
>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 53 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_bios.h     |  1 +
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h |  7 +++
>  3 files changed, 61 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 97cbae2e547e..6354fa4041dc 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2734,6 +2734,43 @@ static bool is_port_valid(struct intel_display *display, enum port port)
>  	return true;
>  }
>  
> +static int _intel_bios_hdmi_max_frl_rate(const struct intel_bios_encoder_data *devdata)
> +{
> +	struct intel_display *display = devdata->display;
> +
> +	if (display->vbt.version >= 237 &&
> +	    devdata->child.hdmi_max_frl_rate_valid) {
> +		switch (devdata->child.hdmi_max_frl_rate) {
> +		default:
> +		case HDMI_MAX_FRL_RATE_PLATFORM:

That's 0, and we already checked that in the if.

> +			drm_dbg_kms(display->drm,
> +				    "HDMI limited to support only TMDS modes\n");
> +			return 0;
> +		case HDMI_MAX_FRL_RATE_3G:
> +			return 3000000;
> +		case HDMI_MAX_FRL_RATE_6G:
> +			return 6000000;
> +		case HDMI_MAX_FRL_RATE_8G:
> +			return 8000000;
> +		case HDMI_MAX_FRL_RATE_10G:
> +			return 10000000;
> +		case HDMI_MAX_FRL_RATE_12G:
> +			return 12000000;
> +		}
> +	}
> +
> +	/*
> +	 * When hdmi_max_frl_rate_valid is 0
> +	 * Don't consider the hdmi_max_frl_rate for
> +	 * limiting the FRL Rates on FRL-capable displays
> +	 */
> +	if (display->vbt.version >= 237 &&
> +	    display->platform.meteorlake)
> +		return 12000000;

Both the if branches have version >= 237, so you could just have if
version < 237 at the top and return early.

And what's the point of having if
(devdata->child.hdmi_max_frl_rate_valid) and switch
(devdata->child.hdmi_max_frl_rate_valid) back to back? Just handle
everything in the switch case?

> +
> +	return 0;
> +}
> +
>  static void print_ddi_port(const struct intel_bios_encoder_data *devdata)
>  {
>  	struct intel_display *display = devdata->display;
> @@ -2741,6 +2778,7 @@ static void print_ddi_port(const struct intel_bios_encoder_data *devdata)
>  	bool is_dvi, is_hdmi, is_dp, is_edp, is_dsi, is_crt, supports_typec_usb, supports_tbt;
>  	int dp_boost_level, dp_max_link_rate, hdmi_boost_level, hdmi_level_shift, max_tmds_clock;
>  	enum port port;
> +	int hdmi_max_frl_rate;
>  
>  	port = intel_bios_encoder_port(devdata);
>  	if (port == PORT_NONE)
> @@ -2806,6 +2844,12 @@ static void print_ddi_port(const struct intel_bios_encoder_data *devdata)
>  			    "Port %c VBT DP max link rate: %d\n",
>  			    port_name(port), dp_max_link_rate);
>  
> +	hdmi_max_frl_rate = _intel_bios_hdmi_max_frl_rate(devdata);
> +	if (hdmi_max_frl_rate)
> +		drm_dbg_kms(display->drm,
> +			    "VBT HDMI max frl rate for port %c: %d\n",

Please stick to the same format as the other debug messages. They'll be
easier to read in the dmesg. frl -> FRL too.

> +			    port_name(port), hdmi_max_frl_rate);
> +
>  	/*
>  	 * FIXME need to implement support for VBT
>  	 * vswing/preemph tables should this ever trigger.
> @@ -3879,3 +3923,12 @@ void intel_bios_debugfs_register(struct intel_display *display)
>  	debugfs_create_file("i915_vbt", 0444, display->drm->debugfs_root,
>  			    display, &intel_bios_vbt_fops);
>  }
> +
> +int intel_bios_hdmi_max_frl_rate(struct intel_encoder *encoder)
> +{
> +	struct intel_display *display = to_intel_display(encoder);
> +	const struct intel_bios_encoder_data *devdata;
> +
> +	devdata = intel_bios_encoder_data_lookup(display, encoder->port);
> +	return _intel_bios_hdmi_max_frl_rate(devdata);
> +}

I don't think we want this wrapper at all. The caller can pass
encoder->devdata.

> diff --git a/drivers/gpu/drm/i915/display/intel_bios.h b/drivers/gpu/drm/i915/display/intel_bios.h
> index 75dff27b4228..53d46c9fa6e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> @@ -102,6 +102,7 @@ int intel_bios_hdmi_max_tmds_clock(const struct intel_bios_encoder_data *devdata
>  void intel_bios_for_each_encoder(struct intel_display *display,
>  				 void (*func)(struct intel_display *display,
>  					      const struct intel_bios_encoder_data *devdata));
> +int intel_bios_hdmi_max_frl_rate(struct intel_encoder *encoder);
>  
>  void intel_bios_debugfs_register(struct intel_display *display);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 0dc13d080e8a..db7484542dc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -381,6 +381,13 @@ struct bdb_general_features {
>  #define HDMI_MAX_DATA_RATE_340		4			/* 249+ */
>  #define HDMI_MAX_DATA_RATE_300		5			/* 249+ */
>  
> +#define HDMI_MAX_FRL_RATE_PLATFORM	0			/* 237 */
> +#define HDMI_MAX_FRL_RATE_3G		1			/* 237 */
> +#define HDMI_MAX_FRL_RATE_6G		2			/* 237 */
> +#define HDMI_MAX_FRL_RATE_8G		3			/* 237 */
> +#define HDMI_MAX_FRL_RATE_10G		4			/* 237 */
> +#define HDMI_MAX_FRL_RATE_12G		5			/* 237 */
> +
>  #define LEGACY_CHILD_DEVICE_CONFIG_SIZE		33
>  
>  /* DDC Bus DDI Type 155+ */

-- 
Jani Nikula, Intel