Re: [PATCH v2 0/4] VBT read cleanup

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 20 Feb 2024, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote:
> This series is originally based out of [1], and built on top of [2].
>
> The primary departure from [1] was that vbt is no longer cached. During vbt
> show, based on the source of vbt, it would simply be re-read reducing the
> read/cleanup complexity. With this series debugfs dump of vbt should work on
> all the platforms that support display.

I think I'd structure this series differently. Please consider something
like this:

1) Add intel_opregion_vbt_present() to check if the VBT is there. Use it
   in intel_bios_is_lvds_present().

2) Always kmemdup and return an allocated buffer from
   intel_opregion_get_vbt(). Always kfree() it. See how that plays with
   the above, and helps clean up the paths and remove the oprom_vbt
   variable in intel_bios_init(). The extra allocation is a bummer, but
   otherwise the cleanup paths will be messy.

3) Add a static function in intel_bios.c to abstract the ways to get the
   VBT. Use it in both intel_bios_init() and intel_bios_vbt_show().

4) Add the size return parameters. This could perhaps be done
   earlier. *shrug*

5) Move firmware VBT to the above function.

BR,
Jani.



>
> 1. https://patchwork.freedesktop.org/series/128341/
> 2. https://patchwork.freedesktop.org/series/128683/
>
> Radhakrishna Sripada (4):
>   drm/i915: Pass size to oprom_get_vbt
>   drm/i915: Pass size to spi_oprom_get_vbt
>   drm/i915: Move vbt read from firmware to intel_bios.c
>   drm/i915: Show bios vbt when read from firmware/spi/oprom
>
>  drivers/gpu/drm/i915/display/intel_bios.c     | 104 +++++++++++++++---
>  drivers/gpu/drm/i915/display/intel_opregion.c |  46 --------
>  2 files changed, 86 insertions(+), 64 deletions(-)

-- 
Jani Nikula, Intel