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. > +} [ ... ] > @@ -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? > + > + kfree(devdata->vspeo); > kfree(devdata->dsc); > kfree(devdata); > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
