diff mbox series

[v5] drm/i915: Show bios vbt when read from firmware/spi/oprom

Message ID 20240301221239.180884-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series [v5] drm/i915: Show bios vbt when read from firmware/spi/oprom | expand

Commit Message

Sripada, Radhakrishna March 1, 2024, 10:12 p.m. UTC
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@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 62 ++++++++++++-----------
 1 file changed, 33 insertions(+), 29 deletions(-)

Comments

Jani Nikula March 4, 2024, 9:59 a.m. UTC | #1
On Fri, 01 Mar 2024, Radhakrishna Sripada <radhakrishna.sripada@intel.com> 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@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  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;
>  }
diff mbox series

Patch

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);
+	}
+
+	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;
 }