On Thu, Aug 13, 2026 at 04:17:45PM GMT, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Medium] Memory leak of devdata->entries during driver removal.
> - [Low] Missing initialization of `num_entries` in `devdata->vspeo`.
> --
>
> --- Patch [4]: [PATCH v11 4/8] drm/i915/bios: de/allocate VS/PE-O buffers for
> each port ---
> commit f4e52d6386e66ed2695365f0776e18c426ae4237
> Author: Michał Grzelak <[email protected]>
>
> drm/i915/bios: de/allocate VS/PE-O buffers for each port
>
> This patch allocates dedicated VS/PE-O buffers for each port if an
> override is requested and supported. It adds intel_ddi_buf_trans and
> entries fields to intel_bios_encoder_data and handles their allocation
> and deallocation.
>
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c
> > b/drivers/gpu/drm/i915/display/intel_bios.c
> > index 9610b794bc147..a491b85006117 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> [ ... ]
> > @@ -2648,6 +2651,29 @@ static void sanitize_device_type(struct
> > intel_bios_encoder_data *devdata,
> > +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;
> > + }
> > +
> > + devdata->vspeo = vspeo;
> > + devdata->entries = entries;
>
> [Severity: Low]
> Is this code missing the initialization of devdata->vspeo->num_entries?
>
> The commit message notes for v4->v5 say "- set devdata->vspeo->num_entries in
> intel_bios.c", but this doesn't seem to be happening here. Leaving this at 0
> would make the table appear empty.
Changelog is missing this but devdata->vspeo->num_entries was again
moved into per-PHY functions. Now I think it is a better idea to
move it back here again.
>
> > +}
> [ ... ]
> > @@ -3403,6 +3430,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);
>
> [Severity: Medium]
> Will this leak the entries buffer allocated in
> allocate_vswing_preemph_override()?
>
> Because devdata->vspeo->entries is never initialized in this patch, it remains
> NULL, and kfree(NULL) is a no-op. Should this be kfree(devdata->entries)
> instead?
Yes.
BR,
Michał
>
> > +
> > + kfree(devdata->vspeo);
> > kfree(devdata->dsc);
> > kfree(devdata);
> > }
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=4
