[PATCH 1/3] drm/i915/vbt: Add eDP Data Overrride field in VBT

[Kandpal, Suraj]

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Monday, July 28, 2025 6:15 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>; intel-xe at lists.freedesktop.org;
> intel-gfx at lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Kandpal, Suraj
> <suraj.kandpal at intel.com>
> Subject: Re: [PATCH 1/3] drm/i915/vbt: Add eDP Data Overrride field in VBT
> 
> On Fri, 25 Jul 2025, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> > Add a field which add the edp_data_override field VBT which gives us a
> > mask of rates which need to be skipped in favour of subsequent higher
> > rate.
> 
> Please look into the grammar.

Sure will fix this

> 
> > Bspec: 20124
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c     |  4 +++-
> >  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 16 ++++++++++++++++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> > b/drivers/gpu/drm/i915/display/intel_bios.c
> > index 9c268bed091d..8337ebe0f2c8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -2747,8 +2747,10 @@ static int child_device_expected_size(u16
> > version)  {
> >  	BUILD_BUG_ON(sizeof(struct child_device_config) < 40);
> >
> > -	if (version > 256)
> > +	if (version > 263)
> >  		return -ENOENT;
> > +	else if (version >= 263)
> > +		return 44;
> >  	else if (version >= 256)
> >  		return 40;
> >  	else if (version >= 216)
> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > index 92c04811aa28..8e29eeb01105 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > @@ -437,6 +437,20 @@ enum vbt_gmbus_ddi {
> >  #define BDB_230_VBT_DP_MAX_LINK_RATE_UHBR13P5	6
> >  #define BDB_230_VBT_DP_MAX_LINK_RATE_UHBR20	7
> >
> > +/* EDP link rate 262+ */
> 
> 263+ obviously?

Weird I could have sworn I wrote 263+ will fix this

> 
> > +#define BDB_263_VBT_EDP_LINK_RATE_1_62		BIT(0)
> > +#define BDB_263_VBT_EDP_LINK_RATE_2_16		BIT(1)
> > +#define BDB_263_VBT_EDP_LINK_RATE_2_43		BIT(2)
> > +#define BDB_263_VBT_EDP_LINK_RATE_2_7		BIT(3)
> > +#define BDB_263_VBT_EDP_LINK_RATE_3_24		BIT(4)
> > +#define BDB_263_VBT_EDP_LINK_RATE_4_32		BIT(5)
> > +#define BDB_263_VBT_EDP_LINK_RATE_5_4		BIT(6)
> > +#define BDB_263_VBT_EDP_LINK_RATE_6_75		BIT(7)
> > +#define BDB_263_VBT_EDP_LINK_RATE_8_1		BIT(8)
> > +#define BDB_263_VBT_EDP_LINK_RATE_10		BIT(9)
> > +#define BDB_263_VBT_EDP_LINK_RATE_13_5		BIT(10)
> > +#define BDB_263_VBT_EDP_LINK_RATE_20		BIT(11)
> 
> Please use BIT_U32() instead.
> 

Sure 

> > +
> >  /*
> >   * The child device config, aka the display device data structure, provides a
> >   * description of a port and its configuration on the platform.
> > @@ -547,6 +561,8 @@ struct child_device_config {
> >  	u8 dp_max_link_rate:3;					/* 216+ */
> >  	u8 dp_max_link_rate_reserved:5;				/*
> 216+ */
> >  	u8 efp_index;						/* 256+ */
> > +	u32 edp_data_override:5;				/* 263+ */
> > +	u32 edp_data_override_reserved:17;			/* 263+ */
> 
> The patch was sent three times, and nobody noticed you define 12 bits for
> the values per spec, but here the bitfield is 5 bits? And 17+5 don't even add
> up to 32 bits total. Neither 5 nor 17 are correct.

You are right this should have been 12 and 20 instead will fix this

Regards,
Suraj Kandpal

> 
> BR,
> Jani.
> 
> 
> >  } __packed;
> >
> >  struct bdb_general_definitions {
> 
> --
> Jani Nikula, Intel