Re: [PATCH] drm/i915/bios: Update new entries in VBT BDB block definations

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

On Mon, 12 Aug 2024, Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com> wrote:
> New entries updated in BDB defination from VBT v257- v260.

*definition

Please explain what you're adding.

Also, the spec never ceases to amaze me. Like here, adding stuff for a
few revisions, obsoleting and starting over. Ugh.

> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 35 ++++++++++++++++---
>  1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index e613288937e4..65342f347bba 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -1080,6 +1080,8 @@ struct bdb_edp {
>  	u16 edp_fast_link_training_rate[16];			/* 224+ */
>  	u16 edp_max_port_link_rate[16];				/* 244+ */
>  	u16 edp_dsc_disable;					/* 251+ */
> +	u16 t16_delay;						/* 260+ */

Please let's follow the spec in member naming where feasible.

This would be t6_delay_support.

> +	u16 t16_link_idle_time;					/* 260+ */

And this would be u16 link_idle_time[16]. The size is 32 bytes, not 2.

(Where does t16 come from?)

>  } __packed;
>  
>  /*
> @@ -1330,12 +1332,27 @@ struct aggressiveness_profile2_entry {
>  	u8 elp_aggressiveness : 4;
>  } __packed;
>  
> +struct aggressiveness_profile3_entry {
> +	u8 adp_aggressiveness:4;

apd_aggressiveness

> +	u8 po_aggressiveness:4;

pixoptix_aggressiveness

> +} __packed;
> +
> +struct aggressiveness_profile4_entry {
> +	u8 xpst_aggressiveness:4;
> +	u8 tcon_aggressiveness:4;
> +} __packed;
> +
> +struct panel_identification {
> +	u8 panel_type:4;

panel_type is a loaded word in VBT. Let's avoid it. Maybe
panel_technology or something.

> +	u8 reserved:4;
> +} __packed;
> +
>  struct bdb_lfp_power {
>  	struct lfp_power_features features;				/* ???-227 */
>  	struct als_data_entry als[5];
>  	u8 lace_aggressiveness_profile:3;				/* 210-227 */
>  	u8 reserved1:5;
> -	u16 dpst;							/* 228+ */
> +	u16 dpst;							/* 228-256 */
>  	u16 psr;							/* 228+ */
>  	u16 drrs;							/* 228+ */
>  	u16 lace_support;						/* 228+ */
> @@ -1343,12 +1360,20 @@ struct bdb_lfp_power {
>  	u16 dmrrs;							/* 228+ */
>  	u16 adb;							/* 228+ */
>  	u16 lace_enabled_status;					/* 228+ */
> -	struct aggressiveness_profile_entry aggressiveness[16];		/* 228+ */
> +	struct aggressiveness_profile_entry aggressiveness[16];		/* 228-256 */

The LACE agressiveness is still valid. Please add the comment to struct
aggressiveness_profile dpst_aggressiveness member.

>  	u16 hobl;							/* 232+ */
>  	u16 vrr_feature_enabled;					/* 233+ */
> -	u16 elp;							/* 247+ */
> -	u16 opst;							/* 247+ */
> -	struct aggressiveness_profile2_entry aggressiveness2[16];	/* 247+ */
> +	u16 elp;							/* 247-256 */
> +	u16 opst;							/* 247-256 */
> +	struct aggressiveness_profile2_entry aggressiveness2[16];	/* 247-256 */
> +	u16 adp;							/* 253-256 */

apd

> +	u16 po;								/* 253-256 */

pixoptix

> +	struct aggressiveness_profile3_entry aggressiveness3[16];	/* 253-256 */
> +	struct panel_identification panel_identity[16];			/* 257+ */

panel_identification

> +	u16 xpst;							/* 257+ */

xpst_support

> +	u16 tcon;							/* 257+ */

tcon_based_backlight_optimization

> +	struct aggressiveness_profile4_entry aggressiveness4[16];	/* 257+ */
> +	u16 tcon_coexist_xpst;						/* 257+ */

Hrmh, this is where the member naming in spec is not feasible. Maybe
tcon_xpst_coexistence.

>  } __packed;
>  
>  /*

-- 
Jani Nikula, Intel