diff mbox series

[v2,3/4] drm/i915: Move vbt read from firmware to intel_bios.c

Message ID 20240220223129.3822480-4-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series VBT read cleanup | expand

Commit Message

Sripada, Radhakrishna Feb. 20, 2024, 10:31 p.m. UTC 
VBT read from firmware is currently nested within opregion vbt read.
Extract it and place it together with other vbt read mechanisms and
dis-associate vbt-firmware from opregion structure.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c     | 48 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_opregion.c | 46 ------------------
 2 files changed, 47 insertions(+), 47 deletions(-)

Comments

Jani Nikula Feb. 27, 2024, 12:42 p.m. UTC | #1 
On Tue, 20 Feb 2024, Radhakrishna Sripada <radhakrishna.sripada@intel.com> wrote:
> VBT read from firmware is currently nested within opregion vbt read.
> Extract it and place it together with other vbt read mechanisms and
> dis-associate vbt-firmware from opregion structure.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 48 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_opregion.c | 46 ------------------
>  2 files changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 2624a4528b21..4cd338ed362d 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -25,6 +25,8 @@
>   *
>   */
>  
> +#include <linux/firmware.h>
> +
>  #include <drm/display/drm_dp_helper.h>
>  #include <drm/display/drm_dsc_helper.h>
>  #include <drm/drm_edid.h>
> @@ -2950,6 +2952,47 @@ bool intel_bios_is_valid_vbt(struct drm_i915_private *i915,
>  	return vbt;
>  }
>  
> +static struct vbt_header *firmware_get_vbt(struct drm_i915_private *i915,
> +					   size_t *size)
> +{
> +	struct vbt_header *vbt = NULL;
> +	const struct firmware *fw = NULL;
> +	const char *name = i915->display.params.vbt_firmware;
> +	int ret;
> +
> +	if (!name || !*name)
> +		return vbt;

Please return NULL directly.

> +
> +	ret = request_firmware(&fw, name, i915->drm.dev);
> +	if (ret) {
> +		drm_err(&i915->drm,
> +			"Requesting VBT firmware \"%s\" failed (%d)\n",
> +			name, ret);
> +		return vbt;

Please return NULL directly.

> +	}
> +
> +	if (intel_bios_is_valid_vbt(i915, fw->data, fw->size)) {
> +		vbt = kmemdup(fw->data, fw->size, GFP_KERNEL);
> +		if (vbt) {
> +			drm_dbg_kms(&i915->drm,
> +				    "Found valid VBT firmware \"%s\"\n", name);

	if (size)

> +			*size = fw->size;
> +		} else {
> +			drm_err(&i915->drm,
> +				"Requesting VBT firmware \"%s\" failed (%d)\n",
> +				name, ENOMEM);

No error messages on ENOMEM, please.

> +			vbt = NULL;

You just checked if vbt is NULL.

> +		}
> +	} else {
> +		drm_dbg_kms(&i915->drm, "Invalid VBT firmware \"%s\"\n",
> +			    name);
> +	}
> +
> +	release_firmware(fw);
> +
> +	return vbt;
> +}
> +
>  static u32 intel_spi_read(struct intel_uncore *uncore, u32 offset)
>  {
>  	intel_uncore_write(uncore, PRIMARY_SPI_ADDRESS, offset);
> @@ -3100,7 +3143,10 @@ void intel_bios_init(struct drm_i915_private *i915)
>  
>  	init_vbt_defaults(i915);
>  
> -	vbt = intel_opregion_get_vbt(i915, NULL);
> +	vbt = firmware_get_vbt(i915, &size);
> +
> +	if (!vbt)
> +		vbt = intel_opregion_get_vbt(i915, NULL);

Crash boom bang without the NULL check.

>  
>  	/*
>  	 * If the OpRegion does not have VBT, look in SPI flash through MMIO or
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index fcbb083318a7..5d07a002edaa 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -27,7 +27,6 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
> -#include <linux/firmware.h>
>  #include <acpi/video.h>
>  
>  #include <drm/drm_edid.h>
> @@ -263,7 +262,6 @@ struct intel_opregion {
>  	struct opregion_asle *asle;
>  	struct opregion_asle_ext *asle_ext;
>  	void *rvda;
> -	void *vbt_firmware;
>  	const void *vbt;
>  	u32 vbt_size;
>  	struct work_struct asle_work;
> @@ -869,46 +867,6 @@ 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->display.opregion;
> -	const struct firmware *fw = NULL;
> -	const char *name = dev_priv->display.params.vbt_firmware;
> -	int ret;
> -
> -	if (!name || !*name)
> -		return -ENOENT;
> -
> -	ret = request_firmware(&fw, name, dev_priv->drm.dev);
> -	if (ret) {
> -		drm_err(&dev_priv->drm,
> -			"Requesting VBT firmware \"%s\" failed (%d)\n",
> -			name, ret);
> -		return ret;
> -	}
> -
> -	if (intel_bios_is_valid_vbt(dev_priv, fw->data, fw->size)) {
> -		opregion->vbt_firmware = kmemdup(fw->data, fw->size, GFP_KERNEL);
> -		if (opregion->vbt_firmware) {
> -			drm_dbg_kms(&dev_priv->drm,
> -				    "Found valid VBT firmware \"%s\"\n", name);
> -			opregion->vbt = opregion->vbt_firmware;
> -			opregion->vbt_size = fw->size;
> -			ret = 0;
> -		} else {
> -			ret = -ENOMEM;
> -		}
> -	} else {
> -		drm_dbg_kms(&dev_priv->drm, "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;
> @@ -1006,9 +964,6 @@ int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  		drm_dbg(&dev_priv->drm, "Mailbox #2 for backlight present\n");
>  	}
>  
> -	if (intel_load_vbt_firmware(dev_priv) == 0)
> -		goto out;
> -
>  	if (dmi_check_system(intel_no_opregion_vbt))
>  		goto out;
>  
> @@ -1312,7 +1267,6 @@ void intel_opregion_cleanup(struct drm_i915_private *i915)
>  	memunmap(opregion->header);
>  	if (opregion->rvda)
>  		memunmap(opregion->rvda);
> -	kfree(opregion->vbt_firmware);
>  	kfree(opregion);
>  	i915->display.opregion = NULL;
>  }
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 2624a4528b21..4cd338ed362d 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -25,6 +25,8 @@ 
  *
  */
 
+#include <linux/firmware.h>
+
 #include <drm/display/drm_dp_helper.h>
 #include <drm/display/drm_dsc_helper.h>
 #include <drm/drm_edid.h>
@@ -2950,6 +2952,47 @@  bool intel_bios_is_valid_vbt(struct drm_i915_private *i915,
 	return vbt;
 }
 
+static struct vbt_header *firmware_get_vbt(struct drm_i915_private *i915,
+					   size_t *size)
+{
+	struct vbt_header *vbt = NULL;
+	const struct firmware *fw = NULL;
+	const char *name = i915->display.params.vbt_firmware;
+	int ret;
+
+	if (!name || !*name)
+		return vbt;
+
+	ret = request_firmware(&fw, name, i915->drm.dev);
+	if (ret) {
+		drm_err(&i915->drm,
+			"Requesting VBT firmware \"%s\" failed (%d)\n",
+			name, ret);
+		return vbt;
+	}
+
+	if (intel_bios_is_valid_vbt(i915, fw->data, fw->size)) {
+		vbt = kmemdup(fw->data, fw->size, GFP_KERNEL);
+		if (vbt) {
+			drm_dbg_kms(&i915->drm,
+				    "Found valid VBT firmware \"%s\"\n", name);
+			*size = fw->size;
+		} else {
+			drm_err(&i915->drm,
+				"Requesting VBT firmware \"%s\" failed (%d)\n",
+				name, ENOMEM);
+			vbt = NULL;
+		}
+	} else {
+		drm_dbg_kms(&i915->drm, "Invalid VBT firmware \"%s\"\n",
+			    name);
+	}
+
+	release_firmware(fw);
+
+	return vbt;
+}
+
 static u32 intel_spi_read(struct intel_uncore *uncore, u32 offset)
 {
 	intel_uncore_write(uncore, PRIMARY_SPI_ADDRESS, offset);
@@ -3100,7 +3143,10 @@  void intel_bios_init(struct drm_i915_private *i915)
 
 	init_vbt_defaults(i915);
 
-	vbt = intel_opregion_get_vbt(i915, NULL);
+	vbt = firmware_get_vbt(i915, &size);
+
+	if (!vbt)
+		vbt = intel_opregion_get_vbt(i915, NULL);
 
 	/*
 	 * If the OpRegion does not have VBT, look in SPI flash through MMIO or
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index fcbb083318a7..5d07a002edaa 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -27,7 +27,6 @@ 
 
 #include <linux/acpi.h>
 #include <linux/dmi.h>
-#include <linux/firmware.h>
 #include <acpi/video.h>
 
 #include <drm/drm_edid.h>
@@ -263,7 +262,6 @@  struct intel_opregion {
 	struct opregion_asle *asle;
 	struct opregion_asle_ext *asle_ext;
 	void *rvda;
-	void *vbt_firmware;
 	const void *vbt;
 	u32 vbt_size;
 	struct work_struct asle_work;
@@ -869,46 +867,6 @@  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->display.opregion;
-	const struct firmware *fw = NULL;
-	const char *name = dev_priv->display.params.vbt_firmware;
-	int ret;
-
-	if (!name || !*name)
-		return -ENOENT;
-
-	ret = request_firmware(&fw, name, dev_priv->drm.dev);
-	if (ret) {
-		drm_err(&dev_priv->drm,
-			"Requesting VBT firmware \"%s\" failed (%d)\n",
-			name, ret);
-		return ret;
-	}
-
-	if (intel_bios_is_valid_vbt(dev_priv, fw->data, fw->size)) {
-		opregion->vbt_firmware = kmemdup(fw->data, fw->size, GFP_KERNEL);
-		if (opregion->vbt_firmware) {
-			drm_dbg_kms(&dev_priv->drm,
-				    "Found valid VBT firmware \"%s\"\n", name);
-			opregion->vbt = opregion->vbt_firmware;
-			opregion->vbt_size = fw->size;
-			ret = 0;
-		} else {
-			ret = -ENOMEM;
-		}
-	} else {
-		drm_dbg_kms(&dev_priv->drm, "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;
@@ -1006,9 +964,6 @@  int intel_opregion_setup(struct drm_i915_private *dev_priv)
 		drm_dbg(&dev_priv->drm, "Mailbox #2 for backlight present\n");
 	}
 
-	if (intel_load_vbt_firmware(dev_priv) == 0)
-		goto out;
-
 	if (dmi_check_system(intel_no_opregion_vbt))
 		goto out;
 
@@ -1312,7 +1267,6 @@  void intel_opregion_cleanup(struct drm_i915_private *i915)
 	memunmap(opregion->header);
 	if (opregion->rvda)
 		memunmap(opregion->rvda);
-	kfree(opregion->vbt_firmware);
 	kfree(opregion);
 	i915->display.opregion = NULL;
 }