On Fri, 25 Jul 2025, Suraj Kandpal <[email protected]> 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.

> Bspec: 20124
> Signed-off-by: Suraj Kandpal <[email protected]>
> ---
>  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?

> +#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.

> +
>  /*
>   * 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.

BR,
Jani.


>  } __packed;
>  
>  struct bdb_general_definitions {

-- 
Jani Nikula, Intel

Reply via email to