Re: [PATCH v3 RESEND] drm/i915/opregion: let user specify override VBT via firmware load

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

On Thu, 17 Aug 2017, Jani Nikula <jani.nikula@intel.com> 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.
>
> As a starting point for experimenting, one can pick up the BIOS provided
> VBT from /sys/kernel/debug/dri/0/i915_opregion/i915_vbt.
>
> v2: clarify firmware load return value check (Bob)
>
> v3: kfree the loaded firmware blob
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=97822#c83
> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

And pushed, with Daniel's additional irc ack.

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/i915_params.c    |  4 ++++
>  drivers/gpu/drm/i915/i915_params.h    |  1 +
>  drivers/gpu/drm/i915/intel_opregion.c | 45 +++++++++++++++++++++++++++++++++++
>  4 files changed, 51 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6c25c8520c87..3ee4fd2a9b41 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -646,6 +646,7 @@ struct intel_opregion {
>  	u32 swsci_sbcb_sub_functions;
>  	struct opregion_asle *asle;
>  	void *rvda;
> +	void *vbt_firmware;
>  	const void *vbt;
>  	u32 vbt_size;
>  	u32 *lid_state;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 14e2c2e57f96..8ab003dca113 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -118,6 +118,10 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
>  module_param_named_unsafe(reset, i915.reset, int, 0600);
>  MODULE_PARM_DESC(reset, "Attempt GPU resets (0=disabled, 1=full gpu reset, 2=engine reset [default])");
>  
> +module_param_named_unsafe(vbt_firmware, i915.vbt_firmware, charp, 0400);
> +MODULE_PARM_DESC(vbt_firmware,
> +		 "Load VBT from specified file under /lib/firmware");
> +
>  #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
>  module_param_named(error_capture, i915.error_capture, bool, 0600);
>  MODULE_PARM_DESC(error_capture,
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index febbfdbd30bd..ac844709c97e 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 2bd03001cc70..98154efcb2f4 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>
> @@ -829,6 +830,10 @@ void intel_opregion_unregister(struct drm_i915_private *dev_priv)
>  		memunmap(opregion->rvda);
>  		opregion->rvda = NULL;
>  	}
> +	if (opregion->vbt_firmware) {
> +		kfree(opregion->vbt_firmware);
> +		opregion->vbt_firmware = NULL;
> +	}
>  	opregion->header = NULL;
>  	opregion->acpi = NULL;
>  	opregion->swsci = NULL;
> @@ -912,6 +917,43 @@ 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_firmware = kmemdup(fw->data, fw->size, GFP_KERNEL);
> +		if (opregion->vbt_firmware) {
> +			DRM_DEBUG_KMS("Found valid VBT firmware \"%s\"\n", name);
> +			opregion->vbt = opregion->vbt_firmware;
> +			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;
> @@ -974,6 +1016,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) == 0)
> +		goto out;
> +
>  	if (dmi_check_system(intel_no_opregion_vbt))
>  		goto out;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx