On Fri, 01 Mar 2024, Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx> wrote:
> Make debugfs vbt only shows valid vbt when read from ACPI opregion.
> Make it work when read from firmware/spi/pci oprom cases. In the cases
> where VBT needs to be read from spi/pci oprom, take the wakeref to
> prevent WARN while reading DE registers during debugfs vbt dump.
>
> v2: Extract getting vbt from different sources to its own function.
> Protect sysfs write with vbt check(Jani)
> v3: Fix CI error by probing bios vbt with runtime_pm wakeref
> v4: Update commit message and skip waking up runtime while accessing
> vbt from opregion/firmware(Jani)
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 62 ++++++++++++-----------
> 1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 44c9dfe86a00..9a8c7fe381b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -3135,6 +3135,32 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915,
> return NULL;
> }
>
> +static const struct vbt_header *intel_bios_get_vbt(struct drm_i915_private *i915,
> + size_t *sizep)
> +{
> + const struct vbt_header *vbt = NULL;
> + intel_wakeref_t wakeref;
> +
> + vbt = firmware_get_vbt(i915, sizep);
> +
> + if (!vbt)
> + vbt = intel_opregion_get_vbt(i915, sizep);
> +
> + /*
> + * If the OpRegion does not have VBT, look in SPI flash
> + * through MMIO or PCI mapping
> + */
> + with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> + if (!vbt && IS_DGFX(i915))
> + vbt = spi_oprom_get_vbt(i915, sizep);
> +
> + if (!vbt)
> + vbt = oprom_get_vbt(i915, sizep);
> + }
This will still enable power even if intel_opregion_get_vbt() returned a
non-NULL pointer.
BR,
Jani.
> +
> + return vbt;
> +}
> +
> /**
> * intel_bios_init - find VBT and initialize settings from the BIOS
> * @i915: i915 device instance
> @@ -3146,7 +3172,6 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *i915,
> void intel_bios_init(struct drm_i915_private *i915)
> {
> const struct vbt_header *vbt;
> - struct vbt_header *oprom_vbt = NULL;
> const struct bdb_header *bdb;
>
> INIT_LIST_HEAD(&i915->display.vbt.display_devices);
> @@ -3160,27 +3185,7 @@ void intel_bios_init(struct drm_i915_private *i915)
>
> init_vbt_defaults(i915);
>
> - oprom_vbt = firmware_get_vbt(i915, NULL);
> - vbt = oprom_vbt;
> -
> - if (!vbt) {
> - oprom_vbt = intel_opregion_get_vbt(i915, NULL);
> - vbt = oprom_vbt;
> - }
> -
> - /*
> - * If the OpRegion does not have VBT, look in SPI flash through MMIO or
> - * PCI mapping
> - */
> - if (!vbt && IS_DGFX(i915)) {
> - oprom_vbt = spi_oprom_get_vbt(i915, NULL);
> - vbt = oprom_vbt;
> - }
> -
> - if (!vbt) {
> - oprom_vbt = oprom_get_vbt(i915, NULL);
> - vbt = oprom_vbt;
> - }
> + vbt = intel_bios_get_vbt(i915, NULL);
>
> if (!vbt)
> goto out;
> @@ -3213,7 +3218,7 @@ void intel_bios_init(struct drm_i915_private *i915)
> parse_sdvo_device_mapping(i915);
> parse_ddi_ports(i915);
>
> - kfree(oprom_vbt);
> + kfree(vbt);
> }
>
> static void intel_bios_init_panel(struct drm_i915_private *i915,
> @@ -3743,13 +3748,12 @@ static int intel_bios_vbt_show(struct seq_file *m, void *unused)
> const void *vbt;
> size_t vbt_size;
>
> - /*
> - * FIXME: VBT might originate from other places than opregion, and then
> - * this would be incorrect.
> - */
> - vbt = intel_opregion_get_vbt(i915, &vbt_size);
> - if (vbt)
> + vbt = intel_bios_get_vbt(i915, &vbt_size);
> +
> + if (vbt) {
> seq_write(m, vbt, vbt_size);
> + kfree(vbt);
> + }
>
> return 0;
> }
--
Jani Nikula, Intel
