[PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

The unaligned ioread32() will make us read byte by byte looking for the
vbt. We could just as well have done a ioread8() + a shift and avoid the
extra confusion on how we are looking for "$VBT".

However when using ACPI it's guaranteed the VBT is 4-byte aligned
per spec, so we can probably assume it here as well.

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

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index 3ab73f8db8dd..328bc6b6157f 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1802,27 +1802,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
 	void __iomem *p = NULL, __iomem *oprom;
 	struct vbt_header *vbt;
 	u16 vbt_size;
-	size_t i, size;
+	size_t 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++) {
-		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
-			continue;
-
-		p = oprom + i;
-		size -= i;
-		break;
-	}
-
-	if (!p)
-		goto err_unmap_oprom;
+	for (p = oprom; size >= 4; p += 4, size -= 4)
+		if (ioread32(p) == *((const u32 *)"$VBT"))
+			break;
 
 	if (sizeof(struct vbt_header) > size) {
-		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		if (size >= 4)
+			DRM_DEBUG_DRIVER("VBT header incomplete\n");
 		goto err_unmap_oprom;
 	}
 
-- 
2.24.0

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

[PATCH 1/3] drm/i915/bios: do not discard address space

[Lucas De Marchi]

When we map the VBT through pci_map_rom() we may not be allowed
to simply discard the address space and go on reading the memory.
That doesn't work on my test system, but by dumping the rom via
sysfs I can can get the correct vbt. So change our find_vbt() to do
the same as done by pci_read_rom(), i.e. use memcpy_fromio().

v2: the just the minimal changes by not bothering with the unaligned io
reads: this can be done on top (from Ville and Jani)

v3: drop const in function return since now we are copying the vbt,
rather than just finding it

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 51 +++++++++++++++++------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index f6a9a5ccb556..8bdfc1d55040 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1896,28 +1896,52 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
 	return vbt;
 }
 
-static const struct vbt_header *find_vbt(void __iomem *oprom, size_t size)
+static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
 {
+	void __iomem *p = NULL;
+	struct vbt_header *vbt;
+	u16 vbt_size;
 	size_t i;
 
 	/* Scour memory looking for the VBT signature. */
 	for (i = 0; i + 4 < size; i++) {
-		void *vbt;
-
 		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
 			continue;
 
-		/*
-		 * This is the one place where we explicitly discard the address
-		 * space (__iomem) of the BIOS/VBT.
-		 */
-		vbt = (void __force *)oprom + i;
-		if (intel_bios_is_valid_vbt(vbt, size - i))
-			return vbt;
-
+		p = oprom + i;
+		size -= i;
 		break;
 	}
 
+	if (!p)
+		return NULL;
+
+	if (sizeof(struct vbt_header) > size) {
+		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		return NULL;
+	}
+
+	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;
+	}
+
+	/* The rest will be validated by intel_bios_is_valid_vbt() */
+	vbt = kmalloc(vbt_size, GFP_KERNEL);
+	if (!vbt)
+		return NULL;
+
+	memcpy_fromio(vbt, p, vbt_size);
+
+	if (!intel_bios_is_valid_vbt(vbt, vbt_size))
+		goto err_free_vbt;
+
+	return vbt;
+
+err_free_vbt:
+	kfree(vbt);
+
 	return NULL;
 }
 
@@ -1953,7 +1977,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
 		if (!oprom)
 			goto out;
 
-		vbt = find_vbt(oprom, size);
+		vbt = copy_vbt(oprom, size);
 		if (!vbt)
 			goto out;
 
@@ -1990,6 +2014,9 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
 
 	if (oprom)
 		pci_unmap_rom(pdev, oprom);
+
+	if (vbt != dev_priv->opregion.vbt)
+		kfree(vbt);
 }
 
 /**
-- 
2.24.0

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

[Intel-gfx] [PATCH 1/3] drm/i915/bios: do not discard address space

[Lucas De Marchi]

When we map the VBT through pci_map_rom() we may not be allowed
to simply discard the address space and go on reading the memory.
That doesn't work on my test system, but by dumping the rom via
sysfs I can can get the correct vbt. So change our find_vbt() to do
the same as done by pci_read_rom(), i.e. use memcpy_fromio().

v2: the just the minimal changes by not bothering with the unaligned io
reads: this can be done on top (from Ville and Jani)

v3: drop const in function return since now we are copying the vbt,
rather than just finding it

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 51 +++++++++++++++++------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index f6a9a5ccb556..8bdfc1d55040 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1896,28 +1896,52 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
 	return vbt;
 }
 
-static const struct vbt_header *find_vbt(void __iomem *oprom, size_t size)
+static struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
 {
+	void __iomem *p = NULL;
+	struct vbt_header *vbt;
+	u16 vbt_size;
 	size_t i;
 
 	/* Scour memory looking for the VBT signature. */
 	for (i = 0; i + 4 < size; i++) {
-		void *vbt;
-
 		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
 			continue;
 
-		/*
-		 * This is the one place where we explicitly discard the address
-		 * space (__iomem) of the BIOS/VBT.
-		 */
-		vbt = (void __force *)oprom + i;
-		if (intel_bios_is_valid_vbt(vbt, size - i))
-			return vbt;
-
+		p = oprom + i;
+		size -= i;
 		break;
 	}
 
+	if (!p)
+		return NULL;
+
+	if (sizeof(struct vbt_header) > size) {
+		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		return NULL;
+	}
+
+	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;
+	}
+
+	/* The rest will be validated by intel_bios_is_valid_vbt() */
+	vbt = kmalloc(vbt_size, GFP_KERNEL);
+	if (!vbt)
+		return NULL;
+
+	memcpy_fromio(vbt, p, vbt_size);
+
+	if (!intel_bios_is_valid_vbt(vbt, vbt_size))
+		goto err_free_vbt;
+
+	return vbt;
+
+err_free_vbt:
+	kfree(vbt);
+
 	return NULL;
 }
 
@@ -1953,7 +1977,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
 		if (!oprom)
 			goto out;
 
-		vbt = find_vbt(oprom, size);
+		vbt = copy_vbt(oprom, size);
 		if (!vbt)
 			goto out;
 
@@ -1990,6 +2014,9 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
 
 	if (oprom)
 		pci_unmap_rom(pdev, oprom);
+
+	if (vbt != dev_priv->opregion.vbt)
+		kfree(vbt);
 }
 
 /**
-- 
2.24.0

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

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

[Lucas De Marchi]

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);
 
@@ -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

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

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

[Lucas De Marchi]

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);
 
@@ -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

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

[PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

The unaligned ioread32() will make us read byte by byte looking for the
vbt. We could just as well have done a ioread8() + a shift and avoid the
extra confusion on how we are looking for "$VBT".

However when using ACPI it's guaranteed the VBT is 4-byte aligned
per spec, so we can probably assume it here as well.

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

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index aa9b182efee5..6bf57b1ad056 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
 	void __iomem *p = NULL, *oprom;
 	struct vbt_header *vbt;
 	u16 vbt_size;
-	size_t i, size;
+	size_t 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++) {
-		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
-			continue;
-
-		p = oprom + i;
-		size -= i;
-		break;
-	}
-
-	if (!p)
-		goto err_unmap_oprom;
+	for (p = oprom; size >= 4; p += 4, size -= 4)
+		if (ioread32(p) == *((const u32 *)"$VBT"))
+			break;
 
 	if (sizeof(struct vbt_header) > size) {
-		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		if (size >= 4)
+			DRM_DEBUG_DRIVER("VBT header incomplete\n");
 		goto err_unmap_oprom;
 	}
 
-- 
2.24.0

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

[Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

The unaligned ioread32() will make us read byte by byte looking for the
vbt. We could just as well have done a ioread8() + a shift and avoid the
extra confusion on how we are looking for "$VBT".

However when using ACPI it's guaranteed the VBT is 4-byte aligned
per spec, so we can probably assume it here as well.

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

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index aa9b182efee5..6bf57b1ad056 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
 	void __iomem *p = NULL, *oprom;
 	struct vbt_header *vbt;
 	u16 vbt_size;
-	size_t i, size;
+	size_t 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++) {
-		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
-			continue;
-
-		p = oprom + i;
-		size -= i;
-		break;
-	}
-
-	if (!p)
-		goto err_unmap_oprom;
+	for (p = oprom; size >= 4; p += 4, size -= 4)
+		if (ioread32(p) == *((const u32 *)"$VBT"))
+			break;
 
 	if (sizeof(struct vbt_header) > size) {
-		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		if (size >= 4)
+			DRM_DEBUG_DRIVER("VBT header incomplete\n");
 		goto err_unmap_oprom;
 	}
 
-- 
2.24.0

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

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/bios: do not discard address space

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/bios: do not discard address space
URL   : https://patchwork.freedesktop.org/series/69790/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7394 -> Patchwork_15364
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html

Known issues
------------

  Here are the changes found in Patchwork_15364 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][1] -> [FAIL][2] ([fdo#108511])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [PASS][3] -> [INCOMPLETE][4] ([fdo#111700])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@gem_exec_gttfill@basic:
    - {fi-tgl-u}:         [INCOMPLETE][5] ([fdo#111593]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-tgl-u/igt@gem_exec_gttfill@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-tgl-u/igt@gem_exec_gttfill@basic.html

  * igt@i915_selftest@live_execlists:
    - fi-whl-u:           [INCOMPLETE][7] ([fdo#112066]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-whl-u/igt@i915_selftest@live_execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-whl-u/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_uncore:
    - fi-bxt-dsi:         [INCOMPLETE][9] ([fdo#103927]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-bxt-dsi/igt@i915_selftest@live_uncore.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-bxt-dsi/igt@i915_selftest@live_uncore.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][11] ([fdo#102614]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [fdo#111700]: https://bugs.freedesktop.org/show_bug.cgi?id=111700
  [fdo#112066]: https://bugs.freedesktop.org/show_bug.cgi?id=112066
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298


Participating hosts (51 -> 45)
------------------------------

  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7394 -> Patchwork_15364

  CI-20190529: 20190529
  CI_DRM_7394: fdf3c3d9ba80a629caf1f76952ce619dc3dc8500 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5299: 65fed6a79adea14f7bef6d55530da47d7731d370 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15364: 16b3402752ceca5f3778fcf5e251ae299541f6dc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

16b3402752ce drm/i915/i915: assume vbt is 4-byte aligned into oprom
c61f7b7ae446 drm/i915/bios: fold pci rom map/unmap into copy function
570222ac56a9 drm/i915/bios: do not discard address space

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/bios: do not discard address space

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/bios: do not discard address space
URL   : https://patchwork.freedesktop.org/series/69790/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7394 -> Patchwork_15364
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html

Known issues
------------

  Here are the changes found in Patchwork_15364 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][1] -> [FAIL][2] ([fdo#108511])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [PASS][3] -> [INCOMPLETE][4] ([fdo#111700])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@gem_exec_gttfill@basic:
    - {fi-tgl-u}:         [INCOMPLETE][5] ([fdo#111593]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-tgl-u/igt@gem_exec_gttfill@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-tgl-u/igt@gem_exec_gttfill@basic.html

  * igt@i915_selftest@live_execlists:
    - fi-whl-u:           [INCOMPLETE][7] ([fdo#112066]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-whl-u/igt@i915_selftest@live_execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-whl-u/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_uncore:
    - fi-bxt-dsi:         [INCOMPLETE][9] ([fdo#103927]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-bxt-dsi/igt@i915_selftest@live_uncore.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-bxt-dsi/igt@i915_selftest@live_uncore.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][11] ([fdo#102614]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [fdo#111700]: https://bugs.freedesktop.org/show_bug.cgi?id=111700
  [fdo#112066]: https://bugs.freedesktop.org/show_bug.cgi?id=112066
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298


Participating hosts (51 -> 45)
------------------------------

  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7394 -> Patchwork_15364

  CI-20190529: 20190529
  CI_DRM_7394: fdf3c3d9ba80a629caf1f76952ce619dc3dc8500 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5299: 65fed6a79adea14f7bef6d55530da47d7731d370 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15364: 16b3402752ceca5f3778fcf5e251ae299541f6dc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

16b3402752ce drm/i915/i915: assume vbt is 4-byte aligned into oprom
c61f7b7ae446 drm/i915/bios: fold pci rom map/unmap into copy function
570222ac56a9 drm/i915/bios: do not discard address space

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

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);
>  }
>  
>  /**

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

Re: [Intel-gfx] [PATCH 2/3] drm/i915/bios: fold pci rom map/unmap into copy function

[Jani Nikula]

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);
>  }
>  
>  /**

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

Re: [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Jani Nikula]

On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> The unaligned ioread32() will make us read byte by byte looking for the
> vbt. We could just as well have done a ioread8() + a shift and avoid the
> extra confusion on how we are looking for "$VBT".
>
> However when using ACPI it's guaranteed the VBT is 4-byte aligned
> per spec, so we can probably assume it here as well.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index aa9b182efee5..6bf57b1ad056 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>  	void __iomem *p = NULL, *oprom;
>  	struct vbt_header *vbt;
>  	u16 vbt_size;
> -	size_t i, size;
> +	size_t 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++) {
> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
> -			continue;
> -
> -		p = oprom + i;
> -		size -= i;
> -		break;
> -	}
> -
> -	if (!p)
> -		goto err_unmap_oprom;
> +	for (p = oprom; size >= 4; p += 4, size -= 4)
> +		if (ioread32(p) == *((const u32 *)"$VBT"))
> +			break;

I think the original is easier to read. You only really need to change
"i++" to "i += 4" and be done with it.

BR,
Jani.

>  
>  	if (sizeof(struct vbt_header) > size) {
> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> +		if (size >= 4)
> +			DRM_DEBUG_DRIVER("VBT header incomplete\n");
>  		goto err_unmap_oprom;
>  	}

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Jani Nikula]

On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> The unaligned ioread32() will make us read byte by byte looking for the
> vbt. We could just as well have done a ioread8() + a shift and avoid the
> extra confusion on how we are looking for "$VBT".
>
> However when using ACPI it's guaranteed the VBT is 4-byte aligned
> per spec, so we can probably assume it here as well.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index aa9b182efee5..6bf57b1ad056 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>  	void __iomem *p = NULL, *oprom;
>  	struct vbt_header *vbt;
>  	u16 vbt_size;
> -	size_t i, size;
> +	size_t 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++) {
> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
> -			continue;
> -
> -		p = oprom + i;
> -		size -= i;
> -		break;
> -	}
> -
> -	if (!p)
> -		goto err_unmap_oprom;
> +	for (p = oprom; size >= 4; p += 4, size -= 4)
> +		if (ioread32(p) == *((const u32 *)"$VBT"))
> +			break;

I think the original is easier to read. You only really need to change
"i++" to "i += 4" and be done with it.

BR,
Jani.

>  
>  	if (sizeof(struct vbt_header) > size) {
> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
> +		if (size >= 4)
> +			DRM_DEBUG_DRIVER("VBT header incomplete\n");
>  		goto err_unmap_oprom;
>  	}

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

Re: [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
>On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> The unaligned ioread32() will make us read byte by byte looking for the
>> vbt. We could just as well have done a ioread8() + a shift and avoid the
>> extra confusion on how we are looking for "$VBT".
>>
>> However when using ACPI it's guaranteed the VBT is 4-byte aligned
>> per spec, so we can probably assume it here as well.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>>  1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index aa9b182efee5..6bf57b1ad056 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>>  	void __iomem *p = NULL, *oprom;
>>  	struct vbt_header *vbt;
>>  	u16 vbt_size;
>> -	size_t i, size;
>> +	size_t 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++) {
>> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
>> -			continue;
>> -
>> -		p = oprom + i;
>> -		size -= i;
>> -		break;
>> -	}
>> -
>> -	if (!p)
>> -		goto err_unmap_oprom;
>> +	for (p = oprom; size >= 4; p += 4, size -= 4)
>> +		if (ioread32(p) == *((const u32 *)"$VBT"))
>> +			break;
>
>I think the original is easier to read. You only really need to change
>"i++" to "i += 4" and be done with it.

I really liked this version much more... shorter and with only one control
variable rather than keeping the control in 3 different places (i, size
and p).

Lucas De Marchi

>
>BR,
>Jani.
>
>>
>>  	if (sizeof(struct vbt_header) > size) {
>> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>> +		if (size >= 4)
>> +			DRM_DEBUG_DRIVER("VBT header incomplete\n");
>>  		goto err_unmap_oprom;
>>  	}
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
>On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> The unaligned ioread32() will make us read byte by byte looking for the
>> vbt. We could just as well have done a ioread8() + a shift and avoid the
>> extra confusion on how we are looking for "$VBT".
>>
>> However when using ACPI it's guaranteed the VBT is 4-byte aligned
>> per spec, so we can probably assume it here as well.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>>  1 file changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index aa9b182efee5..6bf57b1ad056 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>>  	void __iomem *p = NULL, *oprom;
>>  	struct vbt_header *vbt;
>>  	u16 vbt_size;
>> -	size_t i, size;
>> +	size_t 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++) {
>> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
>> -			continue;
>> -
>> -		p = oprom + i;
>> -		size -= i;
>> -		break;
>> -	}
>> -
>> -	if (!p)
>> -		goto err_unmap_oprom;
>> +	for (p = oprom; size >= 4; p += 4, size -= 4)
>> +		if (ioread32(p) == *((const u32 *)"$VBT"))
>> +			break;
>
>I think the original is easier to read. You only really need to change
>"i++" to "i += 4" and be done with it.

I really liked this version much more... shorter and with only one control
variable rather than keeping the control in 3 different places (i, size
and p).

Lucas De Marchi

>
>BR,
>Jani.
>
>>
>>  	if (sizeof(struct vbt_header) > size) {
>> -		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>> +		if (size >= 4)
>> +			DRM_DEBUG_DRIVER("VBT header incomplete\n");
>>  		goto err_unmap_oprom;
>>  	}
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915/bios: do not discard address space

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/bios: do not discard address space
URL   : https://patchwork.freedesktop.org/series/69790/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7394_full -> Patchwork_15364_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15364_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15364_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15364_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-random:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb9/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html

  
Known issues
------------

  Here are the changes found in Patchwork_15364_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-tglb:         [PASS][3] -> [INCOMPLETE][4] ([fdo#111747])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb4/igt@gem_busy@close-race.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb6/igt@gem_busy@close-race.html

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#109276] / [fdo#112080]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_ctx_persistence@vcs1-queued.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb5/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_switch@vcs1-heavy-queue:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#112080]) +10 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_ctx_switch@vcs1-heavy-queue.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_ctx_switch@vcs1-heavy-queue.html

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109276]) +16 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112146]) +9 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb4/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_sync@basic-store-all:
    - shard-tglb:         [PASS][13] -> [INCOMPLETE][14] ([fdo#111880])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb8/igt@gem_sync@basic-store-all.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb6/igt@gem_sync@basic-store-all.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-snb:          [PASS][15] -> [DMESG-WARN][16] ([fdo#111870]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@i915_selftest@live_hangcheck:
    - shard-hsw:          [PASS][17] -> [DMESG-FAIL][18] ([fdo#111991])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw4/igt@i915_selftest@live_hangcheck.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw5/igt@i915_selftest@live_hangcheck.html

  * igt@i915_suspend@debugfs-reader:
    - shard-tglb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#111832] / [fdo#111850]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@i915_suspend@debugfs-reader.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb4/igt@i915_suspend@debugfs-reader.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#103184] / [fdo#103232] / [fdo#108472]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl2/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl6/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#103184] / [fdo#103232] / [fdo#108145])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#105363])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103167]) +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt:
    - shard-tglb:         [PASS][29] -> [FAIL][30] ([fdo#103167])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-tglb:         [PASS][31] -> [INCOMPLETE][32] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb9/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [PASS][33] -> [DMESG-WARN][34] ([fdo#108566]) +6 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][35] -> [DMESG-WARN][36] ([fdo#108566]) +3 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][37] -> [FAIL][38] ([fdo#108145])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][39] -> [FAIL][40] ([fdo#103166])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][41] -> [SKIP][42] ([fdo#109441]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-kbl:          [PASS][43] -> [INCOMPLETE][44] ([fdo#103665])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl1/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl1/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          [PASS][45] -> [INCOMPLETE][46] ([fdo#104108])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@perf@gen8-unprivileged-single-ctx-counters:
    - shard-skl:          [PASS][47] -> [INCOMPLETE][48] ([fdo#111747])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl6/igt@perf@gen8-unprivileged-single-ctx-counters.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl5/igt@perf@gen8-unprivileged-single-ctx-counters.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][49] ([fdo#109276] / [fdo#112080]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb8/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb4/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_ctx_switch@queue-heavy:
    - shard-tglb:         [INCOMPLETE][51] ([fdo#111747]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@gem_ctx_switch@queue-heavy.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb5/igt@gem_ctx_switch@queue-heavy.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][53] ([fdo#110854]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_balancer@smoke.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_create@forked:
    - shard-tglb:         [INCOMPLETE][55] ([fdo#108838] / [fdo#111747]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@gem_exec_create@forked.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@gem_exec_create@forked.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [SKIP][57] ([fdo#112080]) -> [PASS][58] +15 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_parallel@vcs1-fds.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         [SKIP][59] ([fdo#109276]) -> [PASS][60] +16 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_schedule@preempt-contexts-bsd2.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_schedule@preempt-contexts-bsd2.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [SKIP][61] ([fdo#112146]) -> [PASS][62] +7 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][63] ([fdo#104108]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl10/igt@gem_softpin@noreloc-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl8/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-hsw:          [DMESG-WARN][65] ([fdo#111870]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw4/igt@gem_userptr_blits@dmabuf-sync.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw7/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [DMESG-WARN][67] ([fdo#111870]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb2/igt@gem_userptr_blits@sync-unmap-cycles.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb6/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-tglb:         [INCOMPLETE][69] ([fdo#111832] / [fdo#111850]) -> [PASS][70] +2 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb2/igt@i915_pm_backlight@fade_with_suspend.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_selftest@live_execlists:
    - shard-kbl:          [INCOMPLETE][71] ([fdo#103665] / [fdo#112259]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl6/igt@i915_selftest@live_execlists.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl1/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - shard-tglb:         [INCOMPLETE][73] ([fdo#111831]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@i915_selftest@live_gem_contexts.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][75] ([fdo#108566]) -> [PASS][76] +4 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@2x-flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][77] ([fdo#103540]) -> [PASS][78] +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [DMESG-WARN][79] ([fdo#108566]) -> [PASS][80] +3 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-apl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][81] ([fdo#103167]) -> [PASS][82] +6 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-tglb:         [FAIL][83] ([fdo#103167]) -> [PASS][84] +3 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][85] ([fdo#109642] / [fdo#111068]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@kms_psr2_su@frontbuffer.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][87] ([fdo#109441]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@kms_psr@psr2_no_drrs.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][89] ([fdo#99912]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl4/igt@kms_setmode@basic.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl2/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-d-ts-continuation-suspend:
    - shard-tglb:         [INCOMPLETE][91] ([fdo#111850]) -> [PASS][92] +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb2/igt@kms_vblank@pipe-d-ts-continuation-suspend.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@kms_vblank@pipe-d-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [SKIP][93] ([fdo#109276] / [fdo#112080]) -> [FAIL][94] ([fdo#111329]) +1 similar issue
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_eio@kms:
    - shard-snb:          [DMESG-WARN][95] ([fdo#111781] / [fdo#112001]) -> [DMESG-WARN][96] ([fdo#111781])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb1/igt@gem_eio@kms.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb4/igt@gem_eio@kms.html

  * igt@gem_exec_schedule@deep-bsd1:
    - shard-tglb:         [FAIL][97] ([fdo#111646]) -> [INCOMPLETE][98] ([fdo#111671])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb5/igt@gem_exec_schedule@deep-bsd1.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb3/igt@gem_exec_schedule@deep-bsd1.html

  * igt@kms_atomic_transition@6x-modeset-transitions-nonblocking:
    - shard-tglb:         [SKIP][99] ([fdo#112016 ] / [fdo#112021 ]) -> [SKIP][100] ([fdo#112021 ])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb9/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?i

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915/bios: do not discard address space

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/bios: do not discard address space
URL   : https://patchwork.freedesktop.org/series/69790/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7394_full -> Patchwork_15364_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15364_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15364_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15364_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-random:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb9/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb5/igt@kms_cursor_crc@pipe-a-cursor-64x64-random.html

  
Known issues
------------

  Here are the changes found in Patchwork_15364_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-tglb:         [PASS][3] -> [INCOMPLETE][4] ([fdo#111747])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb4/igt@gem_busy@close-race.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb6/igt@gem_busy@close-race.html

  * igt@gem_ctx_persistence@vcs1-queued:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#109276] / [fdo#112080]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_ctx_persistence@vcs1-queued.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb5/igt@gem_ctx_persistence@vcs1-queued.html

  * igt@gem_ctx_switch@vcs1-heavy-queue:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#112080]) +10 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_ctx_switch@vcs1-heavy-queue.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_ctx_switch@vcs1-heavy-queue.html

  * igt@gem_exec_schedule@fifo-bsd1:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#109276]) +16 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@gem_exec_schedule@fifo-bsd1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_exec_schedule@fifo-bsd1.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#112146]) +9 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb4/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_sync@basic-store-all:
    - shard-tglb:         [PASS][13] -> [INCOMPLETE][14] ([fdo#111880])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb8/igt@gem_sync@basic-store-all.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb6/igt@gem_sync@basic-store-all.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-snb:          [PASS][15] -> [DMESG-WARN][16] ([fdo#111870]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@i915_selftest@live_hangcheck:
    - shard-hsw:          [PASS][17] -> [DMESG-FAIL][18] ([fdo#111991])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw4/igt@i915_selftest@live_hangcheck.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw5/igt@i915_selftest@live_hangcheck.html

  * igt@i915_suspend@debugfs-reader:
    - shard-tglb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#111832] / [fdo#111850]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@i915_suspend@debugfs-reader.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb4/igt@i915_suspend@debugfs-reader.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#103184] / [fdo#103232] / [fdo#108472]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl2/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl6/igt@kms_draw_crc@draw-method-xrgb2101010-pwrite-untiled.html

  * igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#103184] / [fdo#103232] / [fdo#108145])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl6/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#105363])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl3/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103167]) +3 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt:
    - shard-tglb:         [PASS][29] -> [FAIL][30] ([fdo#103167])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-suspend:
    - shard-tglb:         [PASS][31] -> [INCOMPLETE][32] ([fdo#111832] / [fdo#111850] / [fdo#111884])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb9/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [PASS][33] -> [DMESG-WARN][34] ([fdo#108566]) +6 similar issues
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][35] -> [DMESG-WARN][36] ([fdo#108566]) +3 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][37] -> [FAIL][38] ([fdo#108145])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][39] -> [FAIL][40] ([fdo#103166])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][41] -> [SKIP][42] ([fdo#109441]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-kbl:          [PASS][43] -> [INCOMPLETE][44] ([fdo#103665])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl1/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl1/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          [PASS][45] -> [INCOMPLETE][46] ([fdo#104108])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@perf@gen8-unprivileged-single-ctx-counters:
    - shard-skl:          [PASS][47] -> [INCOMPLETE][48] ([fdo#111747])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl6/igt@perf@gen8-unprivileged-single-ctx-counters.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl5/igt@perf@gen8-unprivileged-single-ctx-counters.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs1-dirty-create:
    - shard-iclb:         [SKIP][49] ([fdo#109276] / [fdo#112080]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb8/igt@gem_ctx_isolation@vcs1-dirty-create.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb4/igt@gem_ctx_isolation@vcs1-dirty-create.html

  * igt@gem_ctx_switch@queue-heavy:
    - shard-tglb:         [INCOMPLETE][51] ([fdo#111747]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@gem_ctx_switch@queue-heavy.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb5/igt@gem_ctx_switch@queue-heavy.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [SKIP][53] ([fdo#110854]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_balancer@smoke.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_create@forked:
    - shard-tglb:         [INCOMPLETE][55] ([fdo#108838] / [fdo#111747]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@gem_exec_create@forked.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@gem_exec_create@forked.html

  * igt@gem_exec_parallel@vcs1-fds:
    - shard-iclb:         [SKIP][57] ([fdo#112080]) -> [PASS][58] +15 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_parallel@vcs1-fds.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_parallel@vcs1-fds.html

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         [SKIP][59] ([fdo#109276]) -> [PASS][60] +16 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_exec_schedule@preempt-contexts-bsd2.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@gem_exec_schedule@preempt-contexts-bsd2.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [SKIP][61] ([fdo#112146]) -> [PASS][62] +7 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb2/igt@gem_exec_schedule@reorder-wide-bsd.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb3/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][63] ([fdo#104108]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-skl10/igt@gem_softpin@noreloc-s3.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-skl8/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-hsw:          [DMESG-WARN][65] ([fdo#111870]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw4/igt@gem_userptr_blits@dmabuf-sync.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw7/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [DMESG-WARN][67] ([fdo#111870]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb2/igt@gem_userptr_blits@sync-unmap-cycles.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb6/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-tglb:         [INCOMPLETE][69] ([fdo#111832] / [fdo#111850]) -> [PASS][70] +2 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb2/igt@i915_pm_backlight@fade_with_suspend.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb8/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_selftest@live_execlists:
    - shard-kbl:          [INCOMPLETE][71] ([fdo#103665] / [fdo#112259]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl6/igt@i915_selftest@live_execlists.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl1/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_gem_contexts:
    - shard-tglb:         [INCOMPLETE][73] ([fdo#111831]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb6/igt@i915_selftest@live_gem_contexts.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][75] ([fdo#108566]) -> [PASS][76] +4 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@2x-flip-vs-suspend:
    - shard-hsw:          [INCOMPLETE][77] ([fdo#103540]) -> [PASS][78] +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [DMESG-WARN][79] ([fdo#108566]) -> [PASS][80] +3 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-apl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][81] ([fdo#103167]) -> [PASS][82] +6 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-tglb:         [FAIL][83] ([fdo#103167]) -> [PASS][84] +3 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][85] ([fdo#109642] / [fdo#111068]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@kms_psr2_su@frontbuffer.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][87] ([fdo#109441]) -> [PASS][88]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@kms_psr@psr2_no_drrs.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][89] ([fdo#99912]) -> [PASS][90]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-kbl4/igt@kms_setmode@basic.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-kbl2/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-d-ts-continuation-suspend:
    - shard-tglb:         [INCOMPLETE][91] ([fdo#111850]) -> [PASS][92] +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb2/igt@kms_vblank@pipe-d-ts-continuation-suspend.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb7/igt@kms_vblank@pipe-d-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [SKIP][93] ([fdo#109276] / [fdo#112080]) -> [FAIL][94] ([fdo#111329]) +1 similar issue
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-iclb6/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_eio@kms:
    - shard-snb:          [DMESG-WARN][95] ([fdo#111781] / [fdo#112001]) -> [DMESG-WARN][96] ([fdo#111781])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-snb1/igt@gem_eio@kms.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-snb4/igt@gem_eio@kms.html

  * igt@gem_exec_schedule@deep-bsd1:
    - shard-tglb:         [FAIL][97] ([fdo#111646]) -> [INCOMPLETE][98] ([fdo#111671])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb5/igt@gem_exec_schedule@deep-bsd1.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb3/igt@gem_exec_schedule@deep-bsd1.html

  * igt@kms_atomic_transition@6x-modeset-transitions-nonblocking:
    - shard-tglb:         [SKIP][99] ([fdo#112016 ] / [fdo#112021 ]) -> [SKIP][100] ([fdo#112021 ])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7394/shard-tglb7/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/shard-tglb9/igt@kms_atomic_transition@6x-modeset-transitions-nonblocking.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?i

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15364/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] drm/i915/bios: fold pci rom map/unmap into copy function

[Ville Syrjälä]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Ville Syrjälä]

On Thu, Nov 21, 2019 at 10:54:29AM -0800, Lucas De Marchi wrote:
> On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
> >On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >> The unaligned ioread32() will make us read byte by byte looking for the
> >> vbt. We could just as well have done a ioread8() + a shift and avoid the
> >> extra confusion on how we are looking for "$VBT".
> >>
> >> However when using ACPI it's guaranteed the VBT is 4-byte aligned
> >> per spec, so we can probably assume it here as well.
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
> >>  1 file changed, 6 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index aa9b182efee5..6bf57b1ad056 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
> >>  	void __iomem *p = NULL, *oprom;
> >>  	struct vbt_header *vbt;
> >>  	u16 vbt_size;
> >> -	size_t i, size;
> >> +	size_t 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++) {
> >> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
> >> -			continue;
> >> -
> >> -		p = oprom + i;
> >> -		size -= i;
> >> -		break;
> >> -	}
> >> -
> >> -	if (!p)
> >> -		goto err_unmap_oprom;
> >> +	for (p = oprom; size >= 4; p += 4, size -= 4)
> >> +		if (ioread32(p) == *((const u32 *)"$VBT"))
> >> +			break;
> >
> >I think the original is easier to read. You only really need to change
> >"i++" to "i += 4" and be done with it.
> 
> I really liked this version much more... shorter and with only one control
> variable rather than keeping the control in 3 different places (i, size
> and p).

I think I'm with Jani here. Generally not a huge fan of pointer
arithmetic, and having just one variable modified by the loop is
more customary so usually doesn't require me to read more than
once. This thing I had to read a few times to make sure I
understood what it's doing.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Ville Syrjälä]

On Thu, Nov 21, 2019 at 10:54:29AM -0800, Lucas De Marchi wrote:
> On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
> >On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >> The unaligned ioread32() will make us read byte by byte looking for the
> >> vbt. We could just as well have done a ioread8() + a shift and avoid the
> >> extra confusion on how we are looking for "$VBT".
> >>
> >> However when using ACPI it's guaranteed the VBT is 4-byte aligned
> >> per spec, so we can probably assume it here as well.
> >>
> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
> >>  1 file changed, 6 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index aa9b182efee5..6bf57b1ad056 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
> >>  	void __iomem *p = NULL, *oprom;
> >>  	struct vbt_header *vbt;
> >>  	u16 vbt_size;
> >> -	size_t i, size;
> >> +	size_t 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++) {
> >> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
> >> -			continue;
> >> -
> >> -		p = oprom + i;
> >> -		size -= i;
> >> -		break;
> >> -	}
> >> -
> >> -	if (!p)
> >> -		goto err_unmap_oprom;
> >> +	for (p = oprom; size >= 4; p += 4, size -= 4)
> >> +		if (ioread32(p) == *((const u32 *)"$VBT"))
> >> +			break;
> >
> >I think the original is easier to read. You only really need to change
> >"i++" to "i += 4" and be done with it.
> 
> I really liked this version much more... shorter and with only one control
> variable rather than keeping the control in 3 different places (i, size
> and p).

I think I'm with Jani here. Generally not a huge fan of pointer
arithmetic, and having just one variable modified by the loop is
more customary so usually doesn't require me to read more than
once. This thing I had to read a few times to make sure I
understood what it's doing.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

On Fri, Nov 22, 2019 at 03:55:32PM +0200, Ville Syrjälä wrote:
>On Thu, Nov 21, 2019 at 10:54:29AM -0800, Lucas De Marchi wrote:
>> On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
>> >On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> >> The unaligned ioread32() will make us read byte by byte looking for the
>> >> vbt. We could just as well have done a ioread8() + a shift and avoid the
>> >> extra confusion on how we are looking for "$VBT".
>> >>
>> >> However when using ACPI it's guaranteed the VBT is 4-byte aligned
>> >> per spec, so we can probably assume it here as well.
>> >>
>> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>> >>  1 file changed, 6 insertions(+), 13 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> index aa9b182efee5..6bf57b1ad056 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>> >>  	void __iomem *p = NULL, *oprom;
>> >>  	struct vbt_header *vbt;
>> >>  	u16 vbt_size;
>> >> -	size_t i, size;
>> >> +	size_t 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++) {
>> >> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
>> >> -			continue;
>> >> -
>> >> -		p = oprom + i;
>> >> -		size -= i;
>> >> -		break;
>> >> -	}
>> >> -
>> >> -	if (!p)
>> >> -		goto err_unmap_oprom;
>> >> +	for (p = oprom; size >= 4; p += 4, size -= 4)
>> >> +		if (ioread32(p) == *((const u32 *)"$VBT"))
>> >> +			break;
>> >
>> >I think the original is easier to read. You only really need to change
>> >"i++" to "i += 4" and be done with it.
>>
>> I really liked this version much more... shorter and with only one control
>> variable rather than keeping the control in 3 different places (i, size
>> and p).
>
>I think I'm with Jani here. Generally not a huge fan of pointer
>arithmetic, and having just one variable modified by the loop is
>more customary so usually doesn't require me to read more than

we were previously modifying 3: i, p and size. And additionally using
oprom.... versus we only care about p inside the loop, which points to
whatever we just read... and we we keep updates on size to control the
stop condition.

>once. This thing I had to read a few times to make sure I
>understood what it's doing.

Ok, 2 against 1. I will respin this.

Lucas De Marchi

>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915/i915: assume vbt is 4-byte aligned into oprom

[Lucas De Marchi]

On Fri, Nov 22, 2019 at 03:55:32PM +0200, Ville Syrjälä wrote:
>On Thu, Nov 21, 2019 at 10:54:29AM -0800, Lucas De Marchi wrote:
>> On Thu, Nov 21, 2019 at 03:09:03PM +0200, Jani Nikula wrote:
>> >On Wed, 20 Nov 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> >> The unaligned ioread32() will make us read byte by byte looking for the
>> >> vbt. We could just as well have done a ioread8() + a shift and avoid the
>> >> extra confusion on how we are looking for "$VBT".
>> >>
>> >> However when using ACPI it's guaranteed the VBT is 4-byte aligned
>> >> per spec, so we can probably assume it here as well.
>> >>
>> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_bios.c | 19 ++++++-------------
>> >>  1 file changed, 6 insertions(+), 13 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> index aa9b182efee5..6bf57b1ad056 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> @@ -1902,27 +1902,20 @@ static struct vbt_header *oprom_get_vbt(struct drm_i915_private *dev_priv)
>> >>  	void __iomem *p = NULL, *oprom;
>> >>  	struct vbt_header *vbt;
>> >>  	u16 vbt_size;
>> >> -	size_t i, size;
>> >> +	size_t 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++) {
>> >> -		if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
>> >> -			continue;
>> >> -
>> >> -		p = oprom + i;
>> >> -		size -= i;
>> >> -		break;
>> >> -	}
>> >> -
>> >> -	if (!p)
>> >> -		goto err_unmap_oprom;
>> >> +	for (p = oprom; size >= 4; p += 4, size -= 4)
>> >> +		if (ioread32(p) == *((const u32 *)"$VBT"))
>> >> +			break;
>> >
>> >I think the original is easier to read. You only really need to change
>> >"i++" to "i += 4" and be done with it.
>>
>> I really liked this version much more... shorter and with only one control
>> variable rather than keeping the control in 3 different places (i, size
>> and p).
>
>I think I'm with Jani here. Generally not a huge fan of pointer
>arithmetic, and having just one variable modified by the loop is
>more customary so usually doesn't require me to read more than

we were previously modifying 3: i, p and size. And additionally using
oprom.... versus we only care about p inside the loop, which points to
whatever we just read... and we we keep updates on size to control the
stop condition.

>once. This thing I had to read a few times to make sure I
>understood what it's doing.

Ok, 2 against 1. I will respin this.

Lucas De Marchi

>
>-- 
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx