[RESEND 1/2] drm/i915/gvt: stop using drm_edid_block_valid()

[RESEND 1/2] drm/i915/gvt: stop using drm_edid_block_valid()

[Jani Nikula]

We'll want to stop drm_edid_block_valid() usage. KVMGT is the last
user. Replace with drm_edid_valid(), which unfortunately requires an
allocated drm_edid. However, on the plus side, this would be required to
handle the TODO comment about EDID extension block support.

Acked-by: Zhi Wang <zhiwang@kernel.rog>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.wang.linux@gmail.com>
Cc: intel-gvt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 38830818c120..ca0fb126b02d 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -425,6 +425,18 @@ static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
 	.release = intel_vgpu_reg_release_opregion,
 };
 
+static bool edid_valid(const void *edid, size_t size)
+{
+	const struct drm_edid *drm_edid;
+	bool is_valid;
+
+	drm_edid = drm_edid_alloc(edid, size);
+	is_valid = drm_edid_valid(drm_edid);
+	drm_edid_free(drm_edid);
+
+	return is_valid;
+}
+
 static int handle_edid_regs(struct intel_vgpu *vgpu,
 			struct vfio_edid_region *region, char *buf,
 			size_t count, u16 offset, bool is_write)
@@ -443,11 +455,7 @@ static int handle_edid_regs(struct intel_vgpu *vgpu,
 		switch (offset) {
 		case offsetof(struct vfio_region_gfx_edid, link_state):
 			if (data == VFIO_DEVICE_GFX_LINK_STATE_UP) {
-				if (!drm_edid_block_valid(
-					(u8 *)region->edid_blob,
-					0,
-					true,
-					NULL)) {
+				if (!edid_valid(region->edid_blob, EDID_SIZE)) {
 					gvt_vgpu_err("invalid EDID blob\n");
 					return -EINVAL;
 				}
-- 
2.39.2

[RESEND 2/2] drm/edid: make drm_edid_block_valid() static

[Jani Nikula]

drm_edid_block_valid() is no longer used outside of drm_edid.c. Make it
static.

Acked-by: Zhi Wang <zhiwang@kernel.rog>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.wang.linux@gmail.com>
Cc: intel-gvt-dev@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_edid.c | 17 ++++-------------
 include/drm/drm_edid.h     |  2 --
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f68a41eeb1fa..13b3fd351b16 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1966,22 +1966,14 @@ static void edid_block_dump(const char *level, const void *block, int block_num)
 		       block, EDID_LENGTH, false);
 }
 
-/**
- * drm_edid_block_valid - Sanity check the EDID block (base or extension)
- * @_block: pointer to raw EDID block
- * @block_num: type of block to validate (0 for base, extension otherwise)
- * @print_bad_edid: if true, dump bad EDID blocks to the console
- * @edid_corrupt: if true, the header or checksum is invalid
- *
+/*
  * Validate a base or extension EDID block and optionally dump bad blocks to
  * the console.
- *
- * Return: True if the block is valid, false otherwise.
  */
-bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
-			  bool *edid_corrupt)
+static bool drm_edid_block_valid(void *_block, int block_num, bool print_bad_edid,
+				 bool *edid_corrupt)
 {
-	struct edid *block = (struct edid *)_block;
+	struct edid *block = _block;
 	enum edid_block_status status;
 	bool is_base_block = block_num == 0;
 	bool valid;
@@ -2024,7 +2016,6 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
 
 	return valid;
 }
-EXPORT_SYMBOL(drm_edid_block_valid);
 
 /**
  * drm_edid_is_valid - sanity check EDID data
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 6bdfa254a1c1..eaac5e665892 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -440,8 +440,6 @@ int drm_add_modes_noedid(struct drm_connector *connector,
 			 int hdisplay, int vdisplay);
 
 int drm_edid_header_is_valid(const void *edid);
-bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
-			  bool *edid_corrupt);
 bool drm_edid_is_valid(struct edid *edid);
 void drm_edid_get_monitor_name(const struct edid *edid, char *name,
 			       int buflen);
-- 
2.39.2

Re: [RESEND 1/2] drm/i915/gvt: stop using drm_edid_block_valid()

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

On 2024.08.12 17:28:48 +0300, Jani Nikula wrote:
> We'll want to stop drm_edid_block_valid() usage. KVMGT is the last
> user. Replace with drm_edid_valid(), which unfortunately requires an
> allocated drm_edid. However, on the plus side, this would be required to
> handle the TODO comment about EDID extension block support.
> 
> Acked-by: Zhi Wang <zhiwang@kernel.rog>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>

Looks fine to me. Feel free to merge.

Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com>

> ---
> 
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.wang.linux@gmail.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 38830818c120..ca0fb126b02d 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -425,6 +425,18 @@ static const struct intel_vgpu_regops intel_vgpu_regops_opregion = {
>  	.release = intel_vgpu_reg_release_opregion,
>  };
>  
> +static bool edid_valid(const void *edid, size_t size)
> +{
> +	const struct drm_edid *drm_edid;
> +	bool is_valid;
> +
> +	drm_edid = drm_edid_alloc(edid, size);
> +	is_valid = drm_edid_valid(drm_edid);
> +	drm_edid_free(drm_edid);
> +
> +	return is_valid;
> +}
> +
>  static int handle_edid_regs(struct intel_vgpu *vgpu,
>  			struct vfio_edid_region *region, char *buf,
>  			size_t count, u16 offset, bool is_write)
> @@ -443,11 +455,7 @@ static int handle_edid_regs(struct intel_vgpu *vgpu,
>  		switch (offset) {
>  		case offsetof(struct vfio_region_gfx_edid, link_state):
>  			if (data == VFIO_DEVICE_GFX_LINK_STATE_UP) {
> -				if (!drm_edid_block_valid(
> -					(u8 *)region->edid_blob,
> -					0,
> -					true,
> -					NULL)) {
> +				if (!edid_valid(region->edid_blob, EDID_SIZE)) {
>  					gvt_vgpu_err("invalid EDID blob\n");
>  					return -EINVAL;
>  				}
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [RESEND 2/2] drm/edid: make drm_edid_block_valid() static

[Jani Nikula]

On Mon, 12 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote:
> drm_edid_block_valid() is no longer used outside of drm_edid.c. Make it
> static.
>
> Acked-by: Zhi Wang <zhiwang@kernel.rog>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Both pushed to drm-misc-next with Sima's R-b on patch 2.

BR,
Jani.


>
> ---
>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.wang.linux@gmail.com>
> Cc: intel-gvt-dev@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/drm_edid.c | 17 ++++-------------
>  include/drm/drm_edid.h     |  2 --
>  2 files changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index f68a41eeb1fa..13b3fd351b16 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1966,22 +1966,14 @@ static void edid_block_dump(const char *level, const void *block, int block_num)
>  		       block, EDID_LENGTH, false);
>  }
>  
> -/**
> - * drm_edid_block_valid - Sanity check the EDID block (base or extension)
> - * @_block: pointer to raw EDID block
> - * @block_num: type of block to validate (0 for base, extension otherwise)
> - * @print_bad_edid: if true, dump bad EDID blocks to the console
> - * @edid_corrupt: if true, the header or checksum is invalid
> - *
> +/*
>   * Validate a base or extension EDID block and optionally dump bad blocks to
>   * the console.
> - *
> - * Return: True if the block is valid, false otherwise.
>   */
> -bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
> -			  bool *edid_corrupt)
> +static bool drm_edid_block_valid(void *_block, int block_num, bool print_bad_edid,
> +				 bool *edid_corrupt)
>  {
> -	struct edid *block = (struct edid *)_block;
> +	struct edid *block = _block;
>  	enum edid_block_status status;
>  	bool is_base_block = block_num == 0;
>  	bool valid;
> @@ -2024,7 +2016,6 @@ bool drm_edid_block_valid(u8 *_block, int block_num, bool print_bad_edid,
>  
>  	return valid;
>  }
> -EXPORT_SYMBOL(drm_edid_block_valid);
>  
>  /**
>   * drm_edid_is_valid - sanity check EDID data
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 6bdfa254a1c1..eaac5e665892 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -440,8 +440,6 @@ int drm_add_modes_noedid(struct drm_connector *connector,
>  			 int hdisplay, int vdisplay);
>  
>  int drm_edid_header_is_valid(const void *edid);
> -bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid,
> -			  bool *edid_corrupt);
>  bool drm_edid_is_valid(struct edid *edid);
>  void drm_edid_get_monitor_name(const struct edid *edid, char *name,
>  			       int buflen);

-- 
Jani Nikula, Intel