diff mbox

[5/5] drm/i915/opregion: let user specify override VBT via firmware load

Message ID 1322bae4a9b2b1cf68ffa00240071ccda92ee777.1487695041.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jani Nikula Feb. 21, 2017, 4:40 p.m. UTC
Sometimes it would be most enlightening to debug systems by replacing
the VBT to be used. For example, in the referenced bug the BIOS provides
different VBT depending on the boot mode (UEFI vs. legacy). It would be
interesting to try the failing boot mode with the VBT from the working
boot, and see if that makes a difference.

Add a module parameter to load the VBT using the firmware loader, not
unlike the EDID firmware mechanism.

References: https://bugs.freedesktop.org/show_bug.cgi?id=97822#c83
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c    |  4 ++++
 drivers/gpu/drm/i915/i915_params.h    |  1 +
 drivers/gpu/drm/i915/intel_opregion.c | 40 +++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)

Comments

Chris Wilson Feb. 21, 2017, 4:46 p.m. UTC | #1
On Tue, Feb 21, 2017 at 06:40:25PM +0200, Jani Nikula wrote:
> Sometimes it would be most enlightening to debug systems by replacing
> the VBT to be used. For example, in the referenced bug the BIOS provides
> different VBT depending on the boot mode (UEFI vs. legacy). It would be
> interesting to try the failing boot mode with the VBT from the working
> boot, and see if that makes a difference.
> 
> Add a module parameter to load the VBT using the firmware loader, not
> unlike the EDID firmware mechanism.

What's the mechanism by which they would create a valid VBT file?
Does
	cat /sys/kernel/debug/dri/0/i915_opregion > vbt.fw
work and provide a starting point for corrections?
-Chris
Jani Nikula Feb. 22, 2017, 8:15 a.m. UTC | #2
On Tue, 21 Feb 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Tue, Feb 21, 2017 at 06:40:25PM +0200, Jani Nikula wrote:
>> Sometimes it would be most enlightening to debug systems by replacing
>> the VBT to be used. For example, in the referenced bug the BIOS provides
>> different VBT depending on the boot mode (UEFI vs. legacy). It would be
>> interesting to try the failing boot mode with the VBT from the working
>> boot, and see if that makes a difference.
>> 
>> Add a module parameter to load the VBT using the firmware loader, not
>> unlike the EDID firmware mechanism.
>
> What's the mechanism by which they would create a valid VBT file?
> Does
> 	cat /sys/kernel/debug/dri/0/i915_opregion > vbt.fw
> work and provide a starting point for corrections?
 
The debugfs file for just the VBT is i915_vbt.

For starters I only thought of using the VBT from the other boot mode,
and didn't really think of actually modifying the file, let alone making
it easy to modify the file using some tool for example.

There is a Windows-only BIOS Modification Program, or BMP, to modify the
VBT. It's referenced in a bunch of public documentation, but apparently
the tool itself is not publicly available. And even if it were, it's
neither free nor for Linux. I've never touched it myself.

Given the fact that we have tools/intel_vbt_decode in IGT, it should not
be hard to write such a tool. The information is all there for an
interested party. Just a SMOP. ;)

BR,
Jani.
Chris Wilson Feb. 22, 2017, 8:50 a.m. UTC | #3
On Wed, Feb 22, 2017 at 10:15:12AM +0200, Jani Nikula wrote:
> On Tue, 21 Feb 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Tue, Feb 21, 2017 at 06:40:25PM +0200, Jani Nikula wrote:
> >> Sometimes it would be most enlightening to debug systems by replacing
> >> the VBT to be used. For example, in the referenced bug the BIOS provides
> >> different VBT depending on the boot mode (UEFI vs. legacy). It would be
> >> interesting to try the failing boot mode with the VBT from the working
> >> boot, and see if that makes a difference.
> >> 
> >> Add a module parameter to load the VBT using the firmware loader, not
> >> unlike the EDID firmware mechanism.
> >
> > What's the mechanism by which they would create a valid VBT file?
> > Does
> > 	cat /sys/kernel/debug/dri/0/i915_opregion > vbt.fw
> > work and provide a starting point for corrections?
>  
> The debugfs file for just the VBT is i915_vbt.
> 
> For starters I only thought of using the VBT from the other boot mode,
> and didn't really think of actually modifying the file, let alone making
> it easy to modify the file using some tool for example.
> 
> There is a Windows-only BIOS Modification Program, or BMP, to modify the
> VBT. It's referenced in a bunch of public documentation, but apparently
> the tool itself is not publicly available. And even if it were, it's
> neither free nor for Linux. I've never touched it myself.
> 
> Given the fact that we have tools/intel_vbt_decode in IGT, it should not
> be hard to write such a tool. The information is all there for an
> interested party. Just a SMOP. ;)

No worries, all I wanted was a very short step-by-step guide as to how
one would experiment with, use, or test this feature.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e6555a..6d1d127a0f60 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -113,6 +113,10 @@  MODULE_PARM_DESC(vbt_sdvo_panel_type,
 	"Override/Ignore selection of SDVO panel mode in the VBT "
 	"(-2=ignore, -1=auto [default], index in VBT BIOS table)");
 
+module_param_named_unsafe(vbt_firmware, i915.vbt_firmware, charp, 0400);
+MODULE_PARM_DESC(vbt_firmware,
+		 "Load VBT from specified file under /lib/firmware");
+
 module_param_named_unsafe(reset, i915.reset, bool, 0600);
 MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
 
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 55d47eea172e..a0fe557c666e 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -28,6 +28,7 @@ 
 #include <linux/cache.h> /* for __read_mostly */
 
 #define I915_PARAMS_FOR_EACH(func) \
+	func(char *, vbt_firmware); \
 	func(int, modeset); \
 	func(int, panel_ignore_lid); \
 	func(int, semaphores); \
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 30ad721fa93d..a85bbdd3131d 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -27,6 +27,7 @@ 
 
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/firmware.h>
 #include <acpi/video.h>
 
 #include <drm/drmP.h>
@@ -903,6 +904,42 @@  static const struct dmi_system_id intel_no_opregion_vbt[] = {
 	{ }
 };
 
+static int intel_load_vbt_firmware(struct drm_i915_private *dev_priv)
+{
+	struct intel_opregion *opregion = &dev_priv->opregion;
+	const struct firmware *fw = NULL;
+	const char *name = i915.vbt_firmware;
+	int ret;
+
+	if (!name || !*name)
+		return -ENOENT;
+
+	ret = request_firmware(&fw, name, &dev_priv->drm.pdev->dev);
+	if (ret) {
+		DRM_ERROR("Requesting VBT firmware \"%s\" failed (%d)\n",
+			  name, ret);
+		return ret;
+	}
+
+	if (intel_bios_is_valid_vbt(fw->data, fw->size)) {
+		opregion->vbt = kmemdup(fw->data, fw->size, GFP_KERNEL);
+		if (opregion->vbt) {
+			DRM_DEBUG_KMS("Found valid VBT firmware \"%s\"\n", name);
+			opregion->vbt_size = fw->size;
+			ret = 0;
+		} else {
+			ret = -ENOMEM;
+		}
+	} else {
+		DRM_DEBUG_KMS("Invalid VBT firmware \"%s\"\n", name);
+		ret = -EINVAL;
+	}
+
+	release_firmware(fw);
+
+	return ret;
+}
+
 int intel_opregion_setup(struct drm_i915_private *dev_priv)
 {
 	struct intel_opregion *opregion = &dev_priv->opregion;
@@ -965,6 +1002,9 @@  int intel_opregion_setup(struct drm_i915_private *dev_priv)
 	if (mboxes & MBOX_ASLE_EXT)
 		DRM_DEBUG_DRIVER("ASLE extension supported\n");
 
+	if (!intel_load_vbt_firmware(dev_priv))
+		goto out;
+
 	if (dmi_check_system(intel_no_opregion_vbt))
 		goto out;