[PATCH 1/3] drm/i915/bios: remove a redundant NULL pointer check

[PATCH 1/3] drm/i915/bios: remove a redundant NULL pointer check

[Jani Nikula]

We never pass a non-NULL vbt to validate_vbt, and we can safely expect
the callers to not change.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index cbd16c01959a..d39c41cf45a7 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1207,11 +1207,6 @@ static const struct bdb_header *validate_vbt(const void *base, size_t size,
 	size_t offset;
 	const struct bdb_header *bdb;
 
-	if (vbt == NULL) {
-		DRM_DEBUG_DRIVER("VBT signature missing\n");
-		return NULL;
-	}
-
 	offset = _vbt - base;
 	if (offset + sizeof(struct vbt_header) > size) {
 		DRM_DEBUG_DRIVER("VBT header incomplete\n");
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/bios: abstract finding VBT in BIOS to a separate function

[Jani Nikula]

Improve clarity. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index d39c41cf45a7..6e018ba53035 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1235,6 +1235,22 @@ static const struct bdb_header *validate_vbt(const void *base, size_t size,
 	return bdb;
 }
 
+static const struct bdb_header *find_vbt(void *bios, size_t size)
+{
+	const struct bdb_header *bdb = NULL;
+	size_t i;
+
+	/* Scour memory looking for the VBT signature. */
+	for (i = 0; i + 4 < size; i++) {
+		if (memcmp(bios + i, "$VBT", 4) == 0) {
+			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
+			break;
+		}
+	}
+
+	return bdb;
+}
+
 /**
  * intel_parse_bios - find VBT and initialize settings from the BIOS
  * @dev: DRM device
@@ -1263,22 +1279,13 @@ intel_parse_bios(struct drm_device *dev)
 				   dev_priv->opregion.vbt, "OpRegion");
 
 	if (bdb == NULL) {
-		size_t i, size;
+		size_t size;
 
 		bios = pci_map_rom(pdev, &size);
 		if (!bios)
 			return -1;
 
-		/* Scour memory looking for the VBT signature */
-		for (i = 0; i + 4 < size; i++) {
-			if (memcmp(bios + i, "$VBT", 4) == 0) {
-				bdb = validate_vbt(bios, size,
-						   bios + i,
-						   "PCI ROM");
-				break;
-			}
-		}
-
+		bdb = find_vbt(bios, size);
 		if (!bdb) {
 			pci_unmap_rom(pdev, bios);
 			return -1;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/bios: be more explicit about discarding iomem address space

[Jani Nikula]

Add one explicit discard of __iomem address space qualifier in
validate_vbt(), and respect it otherwise. This adds clarity in the code,
and reduces the sparse warnings from the module to just one.

Quoting Daniel, "The vbt really is plain old memory. Except that it's
reserved in the e820 table as something special and hence treated as io
range by the kernel. But it is memory, hence casting away the __iomem is
imo the right approach."

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 6e018ba53035..198fc3c3291b 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1199,15 +1199,22 @@ static const struct dmi_system_id intel_no_opregion_vbt[] = {
 	{ }
 };
 
-static const struct bdb_header *validate_vbt(const void *base, size_t size,
-					     const void *_vbt,
+static const struct bdb_header *validate_vbt(const void __iomem *_base,
+					     size_t size,
+					     const void __iomem *_vbt,
 					     const char *source)
 {
-	const struct vbt_header *vbt = _vbt;
-	size_t offset;
+	/*
+	 * This is the one place where we explicitly discard the address space
+	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
+	 * From now on everything is based on 'base', and treated as regular
+	 * memory.
+	 */
+	const void *base = (const void *) _base;
+	size_t offset = _vbt - _base;
+	const struct vbt_header *vbt = base + offset;
 	const struct bdb_header *bdb;
 
-	offset = _vbt - base;
 	if (offset + sizeof(struct vbt_header) > size) {
 		DRM_DEBUG_DRIVER("VBT header incomplete\n");
 		return NULL;
@@ -1235,14 +1242,14 @@ static const struct bdb_header *validate_vbt(const void *base, size_t size,
 	return bdb;
 }
 
-static const struct bdb_header *find_vbt(void *bios, size_t size)
+static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
 {
 	const struct bdb_header *bdb = NULL;
 	size_t i;
 
 	/* Scour memory looking for the VBT signature. */
 	for (i = 0; i + 4 < size; i++) {
-		if (memcmp(bios + i, "$VBT", 4) == 0) {
+		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
 			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
 			break;
 		}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/bios: be more explicit about discarding iomem address space

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6408
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  272/272              272/272
ILK                                  302/302              302/302
SNB                 -1              315/315              314/315
IVB                                  343/343              343/343
BYT                                  287/287              287/287
BDW                 -1              317/317              316/317
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(2)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
*BDW  igt@gem_flink@basic      PASS(2)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_display.c:#assert_plane[i915]()@WARNING:.* at .* assert_plane
assertion_failure@assertion failure
WARNING:at_drivers/gpu/drm/drm_irq.c:#drm_wait_one_vblank[drm]()@WARNING:.* at .* drm_wait_one_vblank+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/bios: be more explicit about discarding iomem address space

[Daniel Vetter]

On Wed, May 13, 2015 at 03:34:05PM +0300, Jani Nikula wrote:
> Add one explicit discard of __iomem address space qualifier in
> validate_vbt(), and respect it otherwise. This adds clarity in the code,
> and reduces the sparse warnings from the module to just one.
> 
> Quoting Daniel, "The vbt really is plain old memory. Except that it's
> reserved in the e820 table as something special and hence treated as io
> range by the kernel. But it is memory, hence casting away the __iomem is
> imo the right approach."
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Merged entire series, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 6e018ba53035..198fc3c3291b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1199,15 +1199,22 @@ static const struct dmi_system_id intel_no_opregion_vbt[] = {
>  	{ }
>  };
>  
> -static const struct bdb_header *validate_vbt(const void *base, size_t size,
> -					     const void *_vbt,
> +static const struct bdb_header *validate_vbt(const void __iomem *_base,
> +					     size_t size,
> +					     const void __iomem *_vbt,
>  					     const char *source)
>  {
> -	const struct vbt_header *vbt = _vbt;
> -	size_t offset;
> +	/*
> +	 * This is the one place where we explicitly discard the address space
> +	 * (__iomem) of the BIOS/VBT. (And this will cause a sparse complaint.)
> +	 * From now on everything is based on 'base', and treated as regular
> +	 * memory.
> +	 */
> +	const void *base = (const void *) _base;
> +	size_t offset = _vbt - _base;
> +	const struct vbt_header *vbt = base + offset;
>  	const struct bdb_header *bdb;
>  
> -	offset = _vbt - base;
>  	if (offset + sizeof(struct vbt_header) > size) {
>  		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>  		return NULL;
> @@ -1235,14 +1242,14 @@ static const struct bdb_header *validate_vbt(const void *base, size_t size,
>  	return bdb;
>  }
>  
> -static const struct bdb_header *find_vbt(void *bios, size_t size)
> +static const struct bdb_header *find_vbt(void __iomem *bios, size_t size)
>  {
>  	const struct bdb_header *bdb = NULL;
>  	size_t i;
>  
>  	/* Scour memory looking for the VBT signature. */
>  	for (i = 0; i + 4 < size; i++) {
> -		if (memcmp(bios + i, "$VBT", 4) == 0) {
> +		if (ioread32(bios + i) == *((const u32 *) "$VBT")) {
>  			bdb = validate_vbt(bios, size, bios + i, "PCI ROM");
>  			break;
>  		}
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx