> 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.
> +
> + 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
Regards,
Suraj Kandpal
> +
> return encoder->get_buf_trans(encoder, crtc_state, n_entries); }
> --
> 2.45.2
