> <[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.
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
