On Mon, 15 Jun 2026, Kandpal, Suraj wrote:
Subject: RE: [PATCH v7 5/8] drm/i915: override Snps's VS/PE when requested
Subject: [PATCH v7 5/8] drm/i915: override Snps's VS/PE when requested
Add accessor function for Snps to read requested table from VBT #57.
Parse the requested table and transform data into port's buffer.
Choose appropriate accessor function in intel_ddi_buf_trans_get()
basing on
* based
display version and PHY type.
For C20, use 6th table if encoder supports DP 2.0 or higher. Otherwise
use 5th table for DP.
For C20, 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).
For C10, use 2nd table for external DP if encoder supports any mode
beyond or including HBR2. Use 1st table if external DP encoder
supports anything lower than HBR2. For eDP, use 4th table if encoder
supports HBR3. Otherwise use 3rd table for eDP.
For C10, 1st table is used for external DP with modes below HBR2
(exclusive).
1st table is also used as a fallback for non-DPs. 2nd table is used
for external DP with modes higher than HBR2 (inclusive).
3rd table is used for eDP with modes lower than HBR3 (exclusive). 4th
table is used for eDP with modes higher than HBR3 (inclusive).
Indices for other tables have not yet been observed to be used as of now.
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.
v6->v7
- handle VS/PE-O's VBT details in intel_bios_* functions (Jani)
- remove vspeo's cast to (void *) (Jani)
- check devdata->vspeo if VS/PE-O was requested
- call encoder->get_buf_trans() once (Jani)
- return NULL from intel_bios_get_* when using default (Jani)
- validate VS/PE-O in intel_bios.c (Jani)
- inline mtl_{c10,c20}_get_vspeo_buf_trans()
- remove temporarily LT
v4->v5
- blend index computation with table parsing
- remove enums entirely
- change funcs prefix from snps_ to mtl_ (Suraj)
- add spaces around operators (Suraj)
- remove spaces after type casting (Suraj)
- remove INTEL_DISPLAY_STATE_WARN (Suraj)
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 | 100 ++++++++++++++++++
drivers/gpu/drm/i915/display/intel_bios.h | 7 ++
.../drm/i915/display/intel_ddi_buf_trans.c | 21 ++++
3 files changed, 128 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
b/drivers/gpu/drm/i915/display/intel_bios.c
index 3d8864374cac..59ffb5bc8848 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -3861,6 +3861,106 @@ bool intel_bios_encoder_supports_tbt(const
struct intel_bios_encoder_data *devda
return devdata->display->vbt.version >= 209 && devdata->child.tbt;
}
+static bool
+validate_vspeo(const struct intel_bios_encoder_data *devdata, bool
+has_dp) {
Ahh ohkay so you deal with that allocation problem this way. This seems good
too.
+ struct intel_ddi_buf_trans *vspeo;
+
+ if (!devdata)
+ return false;
+
+ /* if vspeo is allocated then VS/PE-O was requested */
+ vspeo = devdata->vspeo;
+ if (!vspeo)
+ return false;
+
+ /* if crtc_state has eDP it also has DP */
+ if (!has_dp)
+ return false;
+
+ return true;
+}
+
+const struct intel_ddi_buf_trans *
+intel_bios_get_c20_vspeo(const struct intel_bios_encoder_data *devdata,
+ bool has_dp, bool is_uhbr)
+{
+ struct intel_display *display;
+ union intel_ddi_buf_trans_entry *entries;
+ int num_columns, num_rows, level, idx;
+ struct intel_ddi_buf_trans *vspeo;
+ const u32 *tables;
+ size_t offset = 0;
+
+ if (!validate_vspeo(devdata, has_dp))
+ return NULL;
+
+ display = devdata->display;
+ entries = (void *)vspeo->entries;
+ tables = display->vbt.vspeo.tables;
+ num_columns = display->vbt.vspeo.num_columns;
+ num_rows = display->vbt.vspeo.num_rows;
+ idx = is_uhbr ? 5 : 4;
+
+ offset += idx * num_rows * num_columns;
+
+ for (level = 0; level < num_rows; level++) {
+ u32 vswing = tables[offset];
+ u32 pre_cursor = tables[offset + 1];
+ u32 post_cursor = tables[offset + 2];
+
+ entries[level].snps.vswing = vswing;
+ entries[level].snps.pre_cursor = pre_cursor;
+ entries[level].snps.post_cursor = post_cursor;
+
+ offset += num_columns;
+ }
+
+ return vspeo;
+}
+
+const struct intel_ddi_buf_trans *
+intel_bios_get_c10_vspeo(const struct intel_bios_encoder_data *devdata,
+ bool has_dp, int port_clock, bool has_edp) {
+ struct intel_display *display;
+ union intel_ddi_buf_trans_entry *entries;
+ int num_columns, num_rows, level, idx;
+ struct intel_ddi_buf_trans *vspeo;
+ const u32 *tables;
+ size_t offset = 0;
+
+ if (!validate_vspeo(devdata, has_dp))
+ return NULL;
+
+ display = devdata->display;
+ vspeo = devdata->vspeo;
+ entries = (void *)vspeo->entries;
+ tables = display->vbt.vspeo.tables;
+ num_columns = display->vbt.vspeo.num_columns;
+ num_rows = display->vbt.vspeo.num_rows;
+
+ idx = port_clock > 270000 ? 1 : 0;
+ if (has_edp)
+ idx = port_clock > 540000 ? 3 : 2;
+
+ offset += idx * num_rows * num_columns;
+
+ for (level = 0; level < num_rows; level++) {
+ u32 vswing = tables[offset];
+ u32 pre_cursor = tables[offset + 1];
+ u32 post_cursor = tables[offset + 2];
+
+ entries[level].snps.vswing = vswing;
+ entries[level].snps.pre_cursor = pre_cursor;
+ entries[level].snps.post_cursor = post_cursor;
+
+ offset += num_columns;
+ }
+
+ 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..49acf8c405e2 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.h
+++ b/drivers/gpu/drm/i915/display/intel_bios.h
@@ -73,6 +73,13 @@ 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_get_c20_vspeo(const struct intel_bios_encoder_data *devdata,
+ bool has_dp, bool is_uhbr);
+const struct intel_ddi_buf_trans *
+intel_bios_get_c10_vspeo(const struct intel_bios_encoder_data *devdata,
+ bool has_dp, int port_clock, bool has_edp);
+
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 4cd1e4d76c7a..b4120b9c49b2 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
@@ -1857,5 +1857,26 @@ const struct intel_ddi_buf_trans
*intel_ddi_buf_trans_get(struct intel_encoder *
const struct
intel_crtc_state *crtc_state,
int *n_entries)
{
+ struct intel_display *display = to_intel_display(encoder);
+ const struct intel_bios_encoder_data *devdata = encoder->devdata;
+ const struct intel_ddi_buf_trans *buf_trans = NULL;
+ bool has_edp, has_dp, is_uhbr;
+ int port_clock;
+
+
+ is_uhbr = intel_dp_is_uhbr(crtc_state);
+ port_clock = crtc_state->port_clock;
We don't need a separate port_clock variable its just called at one place just
directly use crtc_state->port_clock.
Later patches also use port_clock for eg. Combo, so I think it
could be better to retain it in current state instead of overwritting
it. Assuming that it is right approach of course.
BR,
Michał
+
+ if (DISPLAY_VER(display) >= 14) {
This needs to use >= 14 && < 35 so that this does not spill into LT PHY
territory
+ if (intel_encoder_is_c10phy(encoder))
+ buf_trans = intel_bios_get_c10_vspeo(devdata,
has_dp, port_clock, has_edp);
+ else
+ buf_trans = intel_bios_get_c20_vspeo(devdata,
has_dp, is_uhbr);
+ }
+
+ if (buf_trans)
+ return intel_get_buf_trans(buf_trans, n_entries);
I think this code block belongs inside the above if block guarded by
DISPLAY_VER check
Or maybe not since this later becomes a else if ladder you can ignore this
comment.
Regards,
Suraj Kandpal
Regards,
Suraj Kandpal
+
return encoder->get_buf_trans(encoder, crtc_state, n_entries); }
--
2.45.2
