> Subject: RE: [PATCH v4 07/10] drm/i915: override LT's VS/PE when requested
>
> On Tue, 2 Jun 2026, Kandpal, Suraj wrote:
> >> Subject: [PATCH v4 07/10] drm/i915: override LT's VS/PE when
> >> requested
> >>
> >> Add accessor function for LT to read requested table from VBT #57.
> >> Parse the requested table and transform data into port's buffer.
> >>
> >> LT's VS/PE-O tables have less columns than xe3plpd_lt_phy_buf_trans
> >> contains fields. Warn about missing fields.
> >>
> >> Use 6th table if encoder supports DP 2.0 or higher. Otherwise use 5th
> >> table for DP.
> >>
> >> Warn if encoder does not support DP. In that case fallback to using
> >> default VS/PE tables.
> >>
> >> There are no changes to intel_ddi_dp_level() since selection of
> >> correct row of intel_ddi_buf_trans_entry is same as when no override
> >> request has been done.
> >>
> >> Tables 1-4 are not used at all and are most likely to be zeroed. 5th
> >> table is used for any mode below DP 2.0 (exclusive). 6th table is
> >> used for any mode above DP 2.0 (inclusive).
> >>
> >> Indices for other tables have not yet been observed to be used as of now.
> >>
> >> v3->v4
> >> - stick to solely changing VBT data into current structures (Jani)
> >> - move iterator declaration to declaration block (Suraj)
> >>
> >> v2->v3
> >> - remove unnecessary braces from if block (Suraj)
> >> - return -EINVAL instead of -1 (Suraj)
> >>
> >> Signed-off-by: Michał Grzelak <[email protected]>
> >> ---
> >> drivers/gpu/drm/i915/display/intel_bios.c | 39 +++++++++++++++++++
> >> drivers/gpu/drm/i915/display/intel_bios.h | 4 ++
> >> .../drm/i915/display/intel_ddi_buf_trans.c | 24 +++++++++++-
> >> .../drm/i915/display/intel_ddi_buf_trans.h | 10 +++++
> >> 4 files changed, 76 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> >> b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index e47ac169271f..4f1a7d5101c6 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -3860,6 +3860,45 @@ bool intel_bios_encoder_supports_tbt(const
> >> struct intel_bios_encoder_data *devda
> >> return devdata->display->vbt.version >= 209 && devdata->child.tbt;
> >> }
> >>
> >> +const struct intel_ddi_buf_trans *
> >> +intel_bios_encoder_get_lt_vspeo(const struct intel_bios_encoder_data
> >> *devdata,
> >> + int idx)
> >> +{
> >> + struct intel_display *display = devdata->display;
> >> + struct intel_ddi_buf_trans *vspeo = (void *) devdata->vspeo;
> >
> > Unnecessary space after type casting
> >
> >> + union intel_ddi_buf_trans_entry *entries = (void *) devdata->vspeo-
> >>> entries;
> >
> > Ditto
> >
> >> + const u32 *tables = display->vbt.vspeo.tables;
> >> + int num_columns = display->vbt.vspeo.num_columns;
> >> + int num_rows = display->vbt.vspeo.num_rows;
> >> + size_t offset = 0;
> >> + int level;
> >> +
> >> + INTEL_DISPLAY_STATE_WARN(display, idx < _LT_unused0 || idx >
> >> LT_DP2X,
> >> + "LT requested invalid VS/PE-O table: %d\n",
> >> idx);
> >
> > No need for a display state warn here the only value you can possibly
> > end up with here is -EINVAL , LT_DP_14/2x So just do a return no need
> > to log it here. Later the function calling this can fallback to its usual
> > pathway.
> (that would mean In the previous patch you will have to check what
> get_phy_vspeo returns and just use default path if it returns failure) Also I
> think
> you do not require _LT_unused at all.
> >
> >
> >> +
> >> + drm_WARN_ONCE(display->drm, 1,
> >> + "setting txswing & _level to 0 on LT's VS/PE-O
> >> request\n");
> >
> > WARN not required
> > Also Why ? Add bspec link.
>
> Of all WARNs from the series this one is actually justified. LT's VS/PE-O
> tables
> have less columns than xe3plpd_lt_phy_buf_trans contains fields. Thus I don't
> see much other solution then set some safe-ish value and proceed, apart from
> rolling back from the Override.
If that is really the case we shouldn't be enabling it on LT PHY until we know
the correct way to fill in txswing and level info.
Here is the thing, with this every time LT PHY is used for PE Override it will
pollute the dmesg logs with a WARN
Which we ourselves have created (we want to usually WARN only when something
that we are not expecting has happened).
This seems to be something we expect and we end up calling a WARN for it.
Regards,
Suraj Kandpal
> I don't think there is any Bspec for it, since the
> other driver has same problem as well.
>
> BR,
> Michał
>
> >
> >> +
> >> + offset += idx * num_rows * num_columns;
> >> +
> >> + for (level = 0; level < num_rows; level++) {
> >> + u32 main_cursor = tables[offset];
> >> + u32 pre_cursor = tables[offset+1];
> >
> > Add blank space around operator.
> >
> >> + u32 post_cursor = tables[offset+2];
> >
> > Same here.
> >
> >> +
> >> + entries[level].lt.txswing = 0;
> >> + entries[level].lt.txswing_level = 0;
> >> + entries[level].lt.main_cursor = main_cursor;
> >> + entries[level].lt.pre_cursor = pre_cursor;
> >> + entries[level].lt.post_cursor = post_cursor;
> >> +
> >> + offset += num_columns;
> >> + }
> >> +
> >> + vspeo->num_entries = num_rows;
> >> + return vspeo;
> >> +}
> >> +
> >> bool intel_bios_encoder_is_dedicated_external(const struct
> >> intel_bios_encoder_data *devdata) {
> >> return devdata->display->vbt.version >= 264 && diff --git
> >> a/drivers/gpu/drm/i915/display/intel_bios.h
> >> b/drivers/gpu/drm/i915/display/intel_bios.h
> >> index 7a50a272cd27..73786832b402 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.h
> >> @@ -73,6 +73,10 @@ bool intel_bios_get_dsc_params(struct
> >> intel_encoder *encoder, const struct intel_bios_encoder_data *
> >> intel_bios_encoder_data_lookup(struct intel_display *display, enum
> >> port port);
> >>
> >> +const struct intel_ddi_buf_trans *
> >> +intel_bios_encoder_get_lt_vspeo(const struct intel_bios_encoder_data
> >> *devdata,
> >> + int table);
> >> +
> >> bool intel_bios_encoder_requests_vspeo(const struct
> >> intel_bios_encoder_data *devdata); bool
> >> intel_bios_encoder_supports_dvi(const struct intel_bios_encoder_data
> >> *devdata); bool intel_bios_encoder_supports_hdmi(const struct
> >> intel_bios_encoder_data *devdata); diff --git
> >> a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> index fbd5efc5ca6c..68258fbfa43e 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> @@ -3,6 +3,8 @@
> >> * Copyright © 2020 Intel Corporation
> >> */
> >>
> >> +#include <drm/drm_print.h>
> >> +
> >> #include "intel_cx0_phy.h"
> >> #include "intel_ddi.h"
> >> #include "intel_ddi_buf_trans.h"
> >> @@ -1784,6 +1786,20 @@ xe3plpd_get_lt_buf_trans(struct intel_encoder
> >> *encoder,
> >> return intel_get_buf_trans(&xe3plpd_lt_trans_dp14,
> >> n_entries); }
> >>
> >> +static int
> >> +xe3plpd_get_lt_vspeo_index(struct intel_encoder *encoder,
> >> + const struct intel_crtc_state *crtc_state) {
> >> + if (intel_crtc_has_dp_encoder(crtc_state)) {
> >> + if (intel_dp_is_uhbr(crtc_state))
> >> + return LT_DP2X;
> >> + else
> >> + return LT_DP14;
> >> + }
> >> +
> >> + return -EINVAL;
> >> +}
> >> +
> >> static int
> >> _get_phy_vspeo_index(struct intel_encoder *encoder,
> >> const struct intel_crtc_state *crtc_state) @@ -1800,6
> >> +1816,8 @@ void intel_ddi_buf_trans_init(struct intel_encoder
> >> +*encoder)
> >>
> >> if (HAS_LT_PHY(display)) {
> >> encoder->get_buf_trans = xe3plpd_get_lt_buf_trans;
> >> + encoder->get_phy_vspeo_index =
> >> xe3plpd_get_lt_vspeo_index;
> >> + encoder->get_phy_vspeo = intel_bios_encoder_get_lt_vspeo;
> >> } else if (DISPLAY_VER(display) >= 14) {
> >> if (intel_encoder_is_c10phy(encoder))
> >> encoder->get_buf_trans = mtl_get_c10_buf_trans;
> @@ -1878,9
> >> +1896,13 @@ const struct intel_ddi_buf_trans
> >> *intel_ddi_buf_trans_get(struct intel_encoder *
> >>
> >> table = encoder->get_phy_vspeo_index(encoder, crtc_state);
> >> if (table < 0) {
> >> - drm_WARN_ONCE(display->drm, 1,
> >> + drm_WARN_ONCE(display->drm, table == EOPNOTSUPP,
> >> "platform does not support VS/PE-O, setting
> default\n");
> >>
> >> + drm_WARN_ONCE(display->drm, table != EOPNOTSUPP,
> >> + "non-DP (%d) encoder asks for VS/PE-O, setting
> >> default\n",
> >> + crtc_state->output_types);
> >> +
> >> return encoder->get_buf_trans(encoder, crtc_state, n_entries);
> >> }
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.h
> >> b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.h
> >> index 9698697f3917..560892b4ef8c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.h
> >> @@ -74,6 +74,16 @@ struct intel_ddi_buf_trans {
> >> u8 hdmi_default_entry;
> >> };
> >>
> >> +enum intel_lt_vspeo {
> >> + _LT_unused0 = 0,
> >> + _LT_unused1,
> >> + _LT_unused2,
> >> + _LT_unused3,
> >> +
> >> + LT_DP14 = 4,
> >> + LT_DP2X
> >> +};
> >
> > Just
> > enum intel_lt_vspeo_index {
> > LT_DP14 = 4,
> > LT_DP2X
> > };
> >
> > Regards,
> > Suraj Kandpal
> >
> >> +
> >> bool is_hobl_buf_trans(const struct intel_ddi_buf_trans *table);
> >>
> >> void intel_ddi_buf_trans_init(struct intel_encoder *encoder);
> >> --
> >> 2.45.2
> >
> >
