On Mon, 15 Jun 2026, Kandpal, Suraj wrote:
Subject: RE: [PATCH v7 4/8] drm/i915/bios: de/allocate VS/PE-O buffer for
each port
<[email protected]>; Grzelak, Michal <[email protected]>
Subject: [PATCH v7 4/8] drm/i915/bios: de/allocate VS/PE-O buffer for
each port
Every devdata needs a separate intel_ddi_buf_trans since each port can
request an override. Add buffer's pointer into intel_bios_encoder_data.
Allocate struct intel_ddi_buf_trans for the port if VS/PE-O was
requested and is supported. At the same time, allocate struct
intel_ddi_buf_trans_entry and store it inside struct intel_ddi_buf_trans.
Deallocate the buffer as well as entries if the request is supported.
v4->v5
- set devdata->vspeo->num_entries in intel_bios.c
Signed-off-by: Michał Grzelak <[email protected]>
---
drivers/gpu/drm/i915/display/intel_bios.c | 32
+++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
b/drivers/gpu/drm/i915/display/intel_bios.c
index 70467344f08e..3d8864374cac 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -34,6 +34,7 @@
#include <drm/drm_fixed.h>
#include <drm/drm_print.h>
+#include "intel_ddi_buf_trans.h"
#include "intel_display.h"
#include "intel_display_core.h"
#include "intel_display_rpm.h"
@@ -72,6 +73,7 @@
struct intel_bios_encoder_data {
struct intel_display *display;
+ struct intel_ddi_buf_trans *vspeo;
struct child_device_config child;
struct dsc_compression_parameters_entry *dsc;
struct list_head node;
@@ -2628,6 +2630,30 @@ static void sanitize_device_type(struct
intel_bios_encoder_data *devdata,
devdata->child.device_type |= DEVICE_TYPE_NOT_HDMI_OUTPUT; }
+static void allocate_vswing_preemph_override(struct
+intel_bios_encoder_data *devdata) {
+ int num_rows = devdata->display->vbt.vspeo.num_rows;
+ union intel_ddi_buf_trans_entry *entries;
+ struct intel_ddi_buf_trans *vspeo;
+
+ if (!intel_bios_encoder_requests_vspeo(devdata))
+ return;
+
+ vspeo = kzalloc_obj(*vspeo);
+ if (!vspeo)
+ return;
+
+ entries = kzalloc_objs(*entries, num_rows);
+ if (!entries) {
+ kfree(vspeo);
+ return;
+ }
So there is a small chance you may end up with NULL pointer dereference in
this code.
So in the case vspeo or entries allocation fails we free the space and quietly
return. But we use
intel_bios_encoder_requests_vspeo() to decide if we want to use custom
VS/PE Tables, a function who is not aware if all the allocations happened in a
error free way.
So lets says it did not and now if anyone tries to deference devedata->vspeo
they’ll see a bug.
Maybe we can have a flag can_use in vspeo struct and fill that as true if all
allocations work out & keep it false by default. So along with
intel_bios_encoder_requests_vspeo() we also need to check this flag before
we decide if We want to go the VSPEO route or not.
Since you create a function validate_vspeo later which does this exact thing I
think I am good with this patch.
But I think it's still worth-mentioning in the commit message, so will
include it.
LGTM,
Reviewed-by: Suraj Kandpal <[email protected]>
Thanks :)
BR,
Michał
Regards,
Suraj Kandpal
+
+ devdata->vspeo = vspeo;
+ devdata->vspeo->entries = entries;
+ devdata->vspeo->num_entries = num_rows; }
+
static void sanitize_hdmi_level_shift(struct intel_bios_encoder_data
*devdata,
enum port port)
{
@@ -2846,6 +2872,7 @@ static void parse_ddi_port(struct
intel_bios_encoder_data *devdata)
sanitize_dedicated_external(devdata, port);
sanitize_device_type(devdata, port);
sanitize_hdmi_level_shift(devdata, port);
+ allocate_vswing_preemph_override(devdata);
}
static bool has_ddi_port_info(struct intel_display *display) @@
-3383,6
+3410,11 @@ void intel_bios_driver_remove(struct intel_display
+*display)
list_for_each_entry_safe(devdata, nd, &display->vbt.display_devices,
node) {
list_del(&devdata->node);
+
+ if (devdata->vspeo)
+ kfree(devdata->vspeo->entries);
+
+ kfree(devdata->vspeo);
kfree(devdata->dsc);
kfree(devdata);
}
--
2.45.2
