diff mbox series

[2/3] drm/i915/bios: fold pci rom map/unmap into copy function

Message ID 20191120234608.17775-2-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915/bios: do not discard address space | expand

Commit Message

Lucas De Marchi Nov. 20, 2019, 11:46 p.m. UTC 
We don't need to keep the pci rom mapped during the entire
intel_bios_init() anymore. Move it to the previous copy_vbt() function
and rename it to oprom_get_vbt() since now it's responsible to to all
operations related to get the vbt from the oprom.

v2: fix double __iomem attribute detected by sparse

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 40 +++++++++++------------
 1 file changed, 19 insertions(+), 21 deletions(-)

Comments

Jani Nikula Nov. 21, 2019, 1:02 p.m. UTC | #1 
On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> We don't need to keep the pci rom mapped during the entire
> intel_bios_init() anymore. Move it to the previous copy_vbt() function
> and rename it to oprom_get_vbt() since now it's responsible to to all
> operations related to get the vbt from the oprom.
>
> v2: fix double __iomem attribute detected by sparse
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 40 +++++++++++------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 8bdfc1d55040..aa9b182efee5 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1896,12 +1896,17 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
>  	return vbt;
>  }
>  
> -static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
> +static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>  {
> -	void __iomem *p = NULL;
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
> +	void __iomem *p = NULL, *oprom;
>  	struct vbt_header *vbt;
>  	u16 vbt_size;
> -	size_t i;
> +	size_t i, size;
> +
> +	oprom = pci_map_rom(pdev, &size);
> +	if (!oprom)
> +		return NULL;
>  
>  	/* Scour memory looking for the VBT signature. */
>  	for (i = 0; i + 4 < size; i++) {
> @@ -1914,23 +1919,23 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>  	}
>  
>  	if (!p)
> -		return NULL;
> +		goto err_unmap_oprom;
>  
>  	if (sizeof(struct vbt_header) > size) {
>  		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> -		return NULL;
> +		goto err_unmap_oprom;
>  	}
>  
>  	vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size));
>  	if (vbt_size > size) {
>  		DRM_DEBUG_DRIVER("VBT incomplete (vbt_size overflows)\n");
> -		return NULL;
> +		goto err_unmap_oprom;
>  	}
>  
>  	/* The rest will be validated by intel_bios_is_valid_vbt() */
>  	vbt = kmalloc(vbt_size, GFP_KERNEL);
>  	if (!vbt)
> -		return NULL;
> +		goto err_unmap_oprom;
>  
>  	memcpy_fromio(vbt, p, vbt_size);
>  
> @@ -1941,6 +1946,8 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>  
>  err_free_vbt:
>  	kfree(vbt);
> +err_unmap_oprom:
> +	pci_unmap_rom(pdev, oprom);
>  
>  	return NULL;
>  }
> @@ -1955,10 +1962,9 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>   */
>  void intel_bios_init(struct drm_i915_private *dev_priv)
>  {
> -	struct pci_dev *pdev = dev_priv->drm.pdev;
>  	const struct vbt_header *vbt = dev_priv->opregion.vbt;
> +	struct vbt_header *oprom_vbt = NULL;
>  	const struct bdb_header *bdb;
> -	u8 __iomem *oprom = NULL;
>  
>  	INIT_LIST_HEAD(&dev_priv->vbt.display_devices);
>  
> @@ -1971,15 +1977,11 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  
>  	/* If the OpRegion does not have VBT, look in PCI ROM. */
>  	if (!vbt) {
> -		size_t size;
> -
> -		oprom = pci_map_rom(pdev, &size);
> -		if (!oprom)
> +		oprom_vbt = oprom_get_vbt(dev_priv);
> +		if (!oprom_vbt)
>  			goto out;
>  
> -		vbt = copy_vbt(oprom, size);
> -		if (!vbt)
> -			goto out;
> +		vbt = oprom_vbt;
>  
>  		DRM_DEBUG_KMS("Found valid VBT in PCI ROM\n");
>  	}
> @@ -2012,11 +2014,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  		init_vbt_missing_defaults(dev_priv);
>  	}
>  
> -	if (oprom)
> -		pci_unmap_rom(pdev, oprom);
> -
> -	if (vbt != dev_priv->opregion.vbt)
> -		kfree(vbt);
> +	kfree(oprom_vbt);
>  }
>  
>  /**
Ville Syrjälä Nov. 22, 2019, 1:49 p.m. UTC | #2 
On Wed, Nov 20, 2019 at 03:46:07PM -0800, Lucas De Marchi wrote:
> We don't need to keep the pci rom mapped during the entire
> intel_bios_init() anymore. Move it to the previous copy_vbt() function
> and rename it to oprom_get_vbt() since now it's responsible to to all
> operations related to get the vbt from the oprom.
> 
> v2: fix double __iomem attribute detected by sparse
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 40 +++++++++++------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 8bdfc1d55040..aa9b182efee5 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1896,12 +1896,17 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
>  	return vbt;
>  }
>  
> -static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
> +static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>  {
> -	void __iomem *p = NULL;
> +	struct pci_dev *pdev = dev_priv->drm.pdev;
> +	void __iomem *p = NULL, *oprom;
>  	struct vbt_header *vbt;
>  	u16 vbt_size;
> -	size_t i;
> +	size_t i, size;
> +
> +	oprom = pci_map_rom(pdev, &size);
> +	if (!oprom)
> +		return NULL;
>  
>  	/* Scour memory looking for the VBT signature. */
>  	for (i = 0; i + 4 < size; i++) {
> @@ -1914,23 +1919,23 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>  	}
>  
>  	if (!p)
> -		return NULL;
> +		goto err_unmap_oprom;
>  
>  	if (sizeof(struct vbt_header) > size) {
>  		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> -		return NULL;
> +		goto err_unmap_oprom;
>  	}
>  
>  	vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size));
>  	if (vbt_size > size) {
>  		DRM_DEBUG_DRIVER("VBT incomplete (vbt_size overflows)\n");
> -		return NULL;
> +		goto err_unmap_oprom;
>  	}
>  
>  	/* The rest will be validated by intel_bios_is_valid_vbt() */
>  	vbt = kmalloc(vbt_size, GFP_KERNEL);
>  	if (!vbt)
> -		return NULL;
> +		goto err_unmap_oprom;
>  
>  	memcpy_fromio(vbt, p, vbt_size);
>  

Where is the unmap for the non-error path?

> @@ -1941,6 +1946,8 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>  
>  err_free_vbt:
>  	kfree(vbt);
> +err_unmap_oprom:
> +	pci_unmap_rom(pdev, oprom);
>  
>  	return NULL;
>  }
> @@ -1955,10 +1962,9 @@ static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
>   */
>  void intel_bios_init(struct drm_i915_private *dev_priv)
>  {
> -	struct pci_dev *pdev = dev_priv->drm.pdev;
>  	const struct vbt_header *vbt = dev_priv->opregion.vbt;
> +	struct vbt_header *oprom_vbt = NULL;
>  	const struct bdb_header *bdb;
> -	u8 __iomem *oprom = NULL;
>  
>  	INIT_LIST_HEAD(&dev_priv->vbt.display_devices);
>  
> @@ -1971,15 +1977,11 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  
>  	/* If the OpRegion does not have VBT, look in PCI ROM. */
>  	if (!vbt) {
> -		size_t size;
> -
> -		oprom = pci_map_rom(pdev, &size);
> -		if (!oprom)
> +		oprom_vbt = oprom_get_vbt(dev_priv);
> +		if (!oprom_vbt)
>  			goto out;
>  
> -		vbt = copy_vbt(oprom, size);
> -		if (!vbt)
> -			goto out;
> +		vbt = oprom_vbt;
>  
>  		DRM_DEBUG_KMS("Found valid VBT in PCI ROM\n");
>  	}
> @@ -2012,11 +2014,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  		init_vbt_missing_defaults(dev_priv);
>  	}
>  
> -	if (oprom)
> -		pci_unmap_rom(pdev, oprom);
> -
> -	if (vbt != dev_priv->opregion.vbt)
> -		kfree(vbt);
> +	kfree(oprom_vbt);
>  }
>  
>  /**
> -- 
> 2.24.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 8bdfc1d55040..aa9b182efee5 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1896,12 +1896,17 @@  bool intel_bios_is_valid_vbt(const void *buf, size_t size)
 	return vbt;
 }
 
-static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
+static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
 {
-	void __iomem *p = NULL;
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	void __iomem *p = NULL, *oprom;
 	struct vbt_header *vbt;
 	u16 vbt_size;
-	size_t i;
+	size_t i, size;
+
+	oprom = pci_map_rom(pdev, &size);
+	if (!oprom)
+		return NULL;
 
 	/* Scour memory looking for the VBT signature. */
 	for (i = 0; i + 4 < size; i++) {
@@ -1914,23 +1919,23 @@  static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
 	}
 
 	if (!p)
-		return NULL;
+		goto err_unmap_oprom;
 
 	if (sizeof(struct vbt_header) > size) {
 		DRM_DEBUG_DRIVER("VBT header incomplete\n");
-		return NULL;
+		goto err_unmap_oprom;
 	}
 
 	vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size));
 	if (vbt_size > size) {
 		DRM_DEBUG_DRIVER("VBT incomplete (vbt_size overflows)\n");
-		return NULL;
+		goto err_unmap_oprom;
 	}
 
 	/* The rest will be validated by intel_bios_is_valid_vbt() */
 	vbt = kmalloc(vbt_size, GFP_KERNEL);
 	if (!vbt)
-		return NULL;
+		goto err_unmap_oprom;
 
 	memcpy_fromio(vbt, p, vbt_size);
 
@@ -1941,6 +1946,8 @@  static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
 
 err_free_vbt:
 	kfree(vbt);
+err_unmap_oprom:
+	pci_unmap_rom(pdev, oprom);
 
 	return NULL;
 }
@@ -1955,10 +1962,9 @@  static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
  */
 void intel_bios_init(struct drm_i915_private *dev_priv)
 {
-	struct pci_dev *pdev = dev_priv->drm.pdev;
 	const struct vbt_header *vbt = dev_priv->opregion.vbt;
+	struct vbt_header *oprom_vbt = NULL;
 	const struct bdb_header *bdb;
-	u8 __iomem *oprom = NULL;
 
 	INIT_LIST_HEAD(&dev_priv->vbt.display_devices);
 
@@ -1971,15 +1977,11 @@  void intel_bios_init(struct drm_i915_private *dev_priv)
 
 	/* If the OpRegion does not have VBT, look in PCI ROM. */
 	if (!vbt) {
-		size_t size;
-
-		oprom = pci_map_rom(pdev, &size);
-		if (!oprom)
+		oprom_vbt = oprom_get_vbt(dev_priv);
+		if (!oprom_vbt)
 			goto out;
 
-		vbt = copy_vbt(oprom, size);
-		if (!vbt)
-			goto out;
+		vbt = oprom_vbt;
 
 		DRM_DEBUG_KMS("Found valid VBT in PCI ROM\n");
 	}
@@ -2012,11 +2014,7 @@  void intel_bios_init(struct drm_i915_private *dev_priv)
 		init_vbt_missing_defaults(dev_priv);
 	}
 
-	if (oprom)
-		pci_unmap_rom(pdev, oprom);
-
-	if (vbt != dev_priv->opregion.vbt)
-		kfree(vbt);
+	kfree(oprom_vbt);
 }
 
 /**