Re: [PATCH v2 12/15] drm/edid: do invalid block filtering in-place

[Ville Syrjälä]

On Wed, Jun 08, 2022 at 10:50:42AM +0300, Jani Nikula wrote:
> Rewrite edid_filter_invalid_blocks() to filter invalid blocks
> in-place. The main motivation is to not rely on passed in information on
> invalid block count or the allocation size, which will be helpful in
> follow-up work on HF-EEODB.
> 
> Signed-off-by: Jani Nikula 

Reviewed-by: Ville Syrjälä 

> ---
>  drivers/gpu/drm/drm_edid.c | 43 --
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4e788c5cbf25..77ec5b0e436d 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_edid_is_valid);
>  
> -static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
> -int invalid_blocks,
> +static struct edid *edid_filter_invalid_blocks(struct edid *edid,
>  size_t *alloc_size)
>  {
> - struct edid *new, *dest_block;
> - int valid_extensions = edid->extensions - invalid_blocks;
> - int i;
> + struct edid *new;
> + int i, valid_blocks = 0;
>  
> - *alloc_size = edid_size_by_blocks(valid_extensions + 1);
> + for (i = 0; i < edid_block_count(edid); i++) {
> + const void *src_block = edid_block_data(edid, i);
>  
> - new = kmalloc(*alloc_size, GFP_KERNEL);
> - if (!new)
> - goto out;
> + if (edid_block_valid(src_block, i == 0)) {
> + void *dst_block = (void *)edid_block_data(edid, 
> valid_blocks);
>  
> - dest_block = new;
> - for (i = 0; i < edid_block_count(edid); i++) {
> - const void *block = edid_block_data(edid, i);
> + memmove(dst_block, src_block, EDID_LENGTH);
> + valid_blocks++;
> + }
> + }
>  
> - if (edid_block_valid(block, i == 0))
> - memcpy(dest_block++, block, EDID_LENGTH);
> + /* We already trusted the base block to be valid here... */
> + if (WARN_ON(!valid_blocks)) {
> + kfree(edid);
> + return NULL;
>   }
>  
> - new->extensions = valid_extensions;
> - new->checksum = edid_block_compute_checksum(new);
> + edid->extensions = valid_blocks - 1;
> + edid->checksum = edid_block_compute_checksum(edid);
>  
> -out:
> - kfree(edid);
> + *alloc_size = edid_size_by_blocks(valid_blocks);
> +
> + new = krealloc(edid, *alloc_size, GFP_KERNEL);
> + if (!new)
> + kfree(edid);
>  
>   return new;
>  }
> @@ -2290,8 +2294,7 @@ static struct edid *_drm_do_get_edid(struct 
> drm_connector *connector,
>   if (invalid_blocks) {
>   connector_bad_edid(connector, edid, edid_block_count(edid));
>  
> - edid = edid_filter_invalid_blocks(edid, invalid_blocks,
> -   _size);
> + edid = edid_filter_invalid_blocks(edid, _size);
>   }
>  
>  ok:
> -- 
> 2.30.2

-- 
Ville Syrjälä
Intel

[PATCH v2 12/15] drm/edid: do invalid block filtering in-place

[Jani Nikula]

Rewrite edid_filter_invalid_blocks() to filter invalid blocks
in-place. The main motivation is to not rely on passed in information on
invalid block count or the allocation size, which will be helpful in
follow-up work on HF-EEODB.

Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/drm_edid.c | 43 --
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4e788c5cbf25..77ec5b0e436d 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2020,33 +2020,37 @@ bool drm_edid_is_valid(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_edid_is_valid);
 
-static struct edid *edid_filter_invalid_blocks(const struct edid *edid,
-  int invalid_blocks,
+static struct edid *edid_filter_invalid_blocks(struct edid *edid,
   size_t *alloc_size)
 {
-   struct edid *new, *dest_block;
-   int valid_extensions = edid->extensions - invalid_blocks;
-   int i;
+   struct edid *new;
+   int i, valid_blocks = 0;
 
-   *alloc_size = edid_size_by_blocks(valid_extensions + 1);
+   for (i = 0; i < edid_block_count(edid); i++) {
+   const void *src_block = edid_block_data(edid, i);
 
-   new = kmalloc(*alloc_size, GFP_KERNEL);
-   if (!new)
-   goto out;
+   if (edid_block_valid(src_block, i == 0)) {
+   void *dst_block = (void *)edid_block_data(edid, 
valid_blocks);
 
-   dest_block = new;
-   for (i = 0; i < edid_block_count(edid); i++) {
-   const void *block = edid_block_data(edid, i);
+   memmove(dst_block, src_block, EDID_LENGTH);
+   valid_blocks++;
+   }
+   }
 
-   if (edid_block_valid(block, i == 0))
-   memcpy(dest_block++, block, EDID_LENGTH);
+   /* We already trusted the base block to be valid here... */
+   if (WARN_ON(!valid_blocks)) {
+   kfree(edid);
+   return NULL;
}
 
-   new->extensions = valid_extensions;
-   new->checksum = edid_block_compute_checksum(new);
+   edid->extensions = valid_blocks - 1;
+   edid->checksum = edid_block_compute_checksum(edid);
 
-out:
-   kfree(edid);
+   *alloc_size = edid_size_by_blocks(valid_blocks);
+
+   new = krealloc(edid, *alloc_size, GFP_KERNEL);
+   if (!new)
+   kfree(edid);
 
return new;
 }
@@ -2290,8 +2294,7 @@ static struct edid *_drm_do_get_edid(struct drm_connector 
*connector,
if (invalid_blocks) {
connector_bad_edid(connector, edid, edid_block_count(edid));
 
-   edid = edid_filter_invalid_blocks(edid, invalid_blocks,
- _size);
+   edid = edid_filter_invalid_blocks(edid, _size);
}
 
 ok:
-- 
2.30.2