> -----Original Message-----
> From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Sent: Monday, August 12, 2024 7:58 PM
> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>
> Subject: Re: [PATCH] drm/i915/bios: Update new entries in VBT BDB block
> definations
>
> On Mon, 12 Aug 2024, Dnyaneshwar Bhadane
> <dnyaneshwar.bhadane@xxxxxxxxx> wrote:
> > New entries updated in BDB defination from VBT v257- v260.
>
> *definition
>
> Please explain what you're adding.
Thank you, Jani for the feedback,
I will address all the suggestion above and below in the next revision.
>
> 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@xxxxxxxxx>
> > ---
> > 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.
Noted, Thanks.
>
> 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.
Noted, I missed to add as an array of [16]. Thank you.
>
> (Where does t16 come from?)
Noted, Renaming needed.
>
> > } __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
Noted. Thank you.
>
> > +} __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.
Noted. Thank you.
>
> > + 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.
Noted. Thank you.
>
> > 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
Noted. Thank you.
>
> > + 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.
Renaming needed. Noted. Thank you.
Dnyaneshwar
>
> > } __packed;
> >
> > /*
>
> --
> Jani Nikula, Intel
