> Subject: RE: [PATCH v4 06/10] drm/i915/buf_trans: abstract VS/PE-O access and
> index computation
>
> On Mon, 1 Jun 2026, Kandpal, Suraj wrote:
> >> Subject: [PATCH v4 06/10] drm/i915/buf_trans: abstract VS/PE-O access
> >> and index computation
> >>
> >> Lay into intel_ddi_buf_trans_get() infrastructure for VS/PE-O.
> >>
> >> Add (*get_phy_vspeo_index)() function hook into struct intel_encoder.
> >> This function pointer is responsible for computing index of requested
> >> table from VBT #57.
> >>
> >> Add also (*get_phy_vspeo)() function hook into struct intel_encoder.
> >> This function pointer will be the accessor which parses and sets
> >> port's buffer basing on VBT #57's data.
> >>
> >> Note that these two pointer need to be separate since they are taking
> >> place during two different abstraction layers:
> >> (*get_phy_vspeo_index()) operates on crtc_state, while (*get_phy_vspeo)()
> consumes devdata.
> >>
> >> Add generic function pointer indicating lack of VS/PE-O's support.
> >>
> >> Signed-off-by: Michał Grzelak <[email protected]>
> >> ---
> >> .../drm/i915/display/intel_ddi_buf_trans.c | 30 ++++++++++++++++++-
> >> .../drm/i915/display/intel_display_types.h | 5 ++++
> >> 2 files changed, 34 insertions(+), 1 deletion(-)
> >>
> >> 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 4cd1e4d76c7a..fbd5efc5ca6c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> >> @@ -1784,10 +1784,20 @@ xe3plpd_get_lt_buf_trans(struct intel_encoder
> >> *encoder,
> >> return intel_get_buf_trans(&xe3plpd_lt_trans_dp14,
> >> n_entries); }
> >>
> >> +static int
> >> +_get_phy_vspeo_index(struct intel_encoder *encoder,
> >> + const struct intel_crtc_state *crtc_state) {
> >> + return -EOPNOTSUPP;
> >> +}
This can be removed.
> >> +
> >> void intel_ddi_buf_trans_init(struct intel_encoder *encoder) {
> >> struct intel_display *display = to_intel_display(encoder);
> >>
> >> + encoder->get_phy_vspeo_index = _get_phy_vspeo_index;
> >> + encoder->get_phy_vspeo = NULL;
> >> +
Skip defining this for all platforms by default. Will help later.
> >> if (HAS_LT_PHY(display)) {
> >> encoder->get_buf_trans = xe3plpd_get_lt_buf_trans;
> >> } else if (DISPLAY_VER(display) >= 14) { @@ -1857,5 +1867,23 @@
> >> const struct intel_ddi_buf_trans *intel_ddi_buf_trans_get(struct
> >> intel_encoder *
> >> const struct
> >> intel_crtc_state *crtc_state,
> >> int *n_entries)
> >> {
> >> - return encoder->get_buf_trans(encoder, crtc_state, n_entries);
> >> + struct intel_display *display = to_intel_display(encoder);
> >> + const struct intel_ddi_buf_trans *buf_trans;
> >> + bool vspeo;
> >> + int table;
> >> +
> >> + vspeo = intel_bios_encoder_requests_vspeo(encoder->devdata);
> >> + if (!vspeo)
> >> + return encoder->get_buf_trans(encoder, crtc_state,
> >> n_entries);
> >> +
> >> + table = encoder->get_phy_vspeo_index(encoder, crtc_state);
> >> + if (table < 0) {
> >> + drm_WARN_ONCE(display->drm, 1,
> >> + "platform does not support VS/PE-O, setting
> >> default\n");
Now with the above two comments implemented what you can do here is
vspeo = intel_bios_encoder_requests_vspeo(encoder->devdata);
if (!vspeo)
goto out;
If (encoder->get_phy_vspeo_index) {
table = encoder->get_phy_vspeo_index(encoder, crtc_state);
buf_trans = encoder->get_phy_vspeo(encoder->devdata, table);
return intel_get_buf_trans(buf_trans, n_entries);
}
Out :
return encoder->get_buf_trans(encoder, crtc_state, n_entries);
Also after looking at the code this function is a get which I dont needs to
have in its job description what platform does/does not
Support VS/pe-o
Regards,
Suraj Kandpal
> >> +
> >
> > WARN does not belong here maybe a drm_dbg_kms() saying "VBT requests
> vspeo but does not provide index table"
> > The reason being panel vendors may have broken VBT requesting for VS/PE
> but not providing a table.
> > This will just cause Alarms to go off in CI, where you will be
> > debugging broken VBTs which wont be fixed specially for the older
> Generations.
>
> I agree that drm_dbg_kms() would suffice. I'm not sure on the phrasing
> though. Current patch series does not support DKL nor MG PHY, and this
> message was targeted to be shown when eg. TGL's DKL requests Override when
> there is no code to handle it.
>
> Now I think that probably it would be better to do what you have already said
> [1], that the debug message should be issued only when VS/PE-O would
> successfully take place, instead of spamming it all along the way.
>
> BR,
> Michał
>
> [1] https://lore.kernel.org/intel-
> gfx/[email protected]
> amprd11.prod.outlook.com/
>
> >
> > Regards,
> > Suraj Kandpal
> >
> >> + return encoder->get_buf_trans(encoder, crtc_state,
> >> n_entries);
> >> + }
> >> +
> >> + buf_trans = encoder->get_phy_vspeo(encoder->devdata, table);
> >> + return intel_get_buf_trans(buf_trans, n_entries);
> >> }
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> index c21e0c0ef0b1..829e17fbfb0a 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> >> @@ -292,6 +292,11 @@ struct intel_encoder {
> >> void (*set_signal_levels)(struct intel_encoder *encoder,
> >> const struct intel_crtc_state *crtc_state);
> >>
> >> + int (*get_phy_vspeo_index)(struct intel_encoder *encoder,
> >> + const struct intel_crtc_state *crtc_state);
> >> + const struct intel_ddi_buf_trans *(*get_phy_vspeo)(const struct
> >> intel_bios_encoder_data *devdata,
> >> + int idx);
> >> +
> >> enum hpd_pin hpd_pin;
> >> enum intel_display_power_domain power_domain;
> >>
> >> --
> >> 2.45.2
> >
> >
