[PATCH 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()

[PATCH 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()

[Ian Ray]

Migrate away from custom EDID parsing and validity checks.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ian Ray <ian.ray@ge.com>
---
 .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c   | 57 ++++------------------
 1 file changed, 9 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
index 460db3c..e93083b 100644
--- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
+++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
@@ -65,12 +65,11 @@ struct ge_b850v3_lvds {
 
 static struct ge_b850v3_lvds *ge_b850v3_lvds_ptr;
 
-static u8 *stdp2690_get_edid(struct i2c_client *client)
+static int stdp2690_read_block(void *context, u8 *buf, unsigned int block, size_t len)
 {
+	struct i2c_client *client = context;
 	struct i2c_adapter *adapter = client->adapter;
-	unsigned char start = 0x00;
-	unsigned int total_size;
-	u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+	unsigned char start = block * EDID_LENGTH;
 
 	struct i2c_msg msgs[] = {
 		{
@@ -81,53 +80,15 @@ static u8 *stdp2690_get_edid(struct i2c_client *client)
 		}, {
 			.addr	= client->addr,
 			.flags	= I2C_M_RD,
-			.len	= EDID_LENGTH,
-			.buf	= block,
+			.len	= len,
+			.buf	= buf,
 		}
 	};
 
-	if (!block)
-		return NULL;
+	if (i2c_transfer(adapter, msgs, 2) != 2)
+		return -1;
 
-	if (i2c_transfer(adapter, msgs, 2) != 2) {
-		DRM_ERROR("Unable to read EDID.\n");
-		goto err;
-	}
-
-	if (!drm_edid_block_valid(block, 0, false, NULL)) {
-		DRM_ERROR("Invalid EDID data\n");
-		goto err;
-	}
-
-	total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
-	if (total_size > EDID_LENGTH) {
-		kfree(block);
-		block = kmalloc(total_size, GFP_KERNEL);
-		if (!block)
-			return NULL;
-
-		/* Yes, read the entire buffer, and do not skip the first
-		 * EDID_LENGTH bytes.
-		 */
-		start = 0x00;
-		msgs[1].len = total_size;
-		msgs[1].buf = block;
-
-		if (i2c_transfer(adapter, msgs, 2) != 2) {
-			DRM_ERROR("Unable to read EDID extension blocks.\n");
-			goto err;
-		}
-		if (!drm_edid_block_valid(block, 1, false, NULL)) {
-			DRM_ERROR("Invalid EDID data\n");
-			goto err;
-		}
-	}
-
-	return block;
-
-err:
-	kfree(block);
-	return NULL;
+	return 0;
 }
 
 static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
@@ -137,7 +98,7 @@ static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
 
 	client = ge_b850v3_lvds_ptr->stdp2690_i2c;
 
-	return (struct edid *)stdp2690_get_edid(client);
+	return drm_do_get_edid(connector, stdp2690_read_block, client);
 }
 
 static int ge_b850v3_lvds_get_modes(struct drm_connector *connector)
-- 
2.10.1

[PATCH 2/2] MAINTAINERS: Update entry for megachips-stdpxxxx-ge-b850v3-fw

[Ian Ray]

Replace Martin, who has left GE.

Signed-off-by: Ian Ray <ian.ray@ge.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index bf0f54c..31bb835 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13524,7 +13524,7 @@ F:	drivers/usb/mtu3/
 
 MEGACHIPS STDPXXXX-GE-B850V3-FW LVDS/DP++ BRIDGES
 M:	Peter Senna Tschudin <peter.senna@gmail.com>
-M:	Martin Donnelly <martin.donnelly@ge.com>
+M:	Ian Ray <ian.ray@ge.com>
 M:	Martyn Welch <martyn.welch@collabora.co.uk>
 S:	Maintained
 F:	Documentation/devicetree/bindings/display/bridge/megachips-stdpxxxx-ge-b850v3-fw.txt
-- 
2.10.1

Re: [PATCH 1/2] drm/bridge: megachips-stdpxxxx-ge-b850v3-fw: switch to drm_do_get_edid()

[Jani Nikula]

On Thu, 21 Sep 2023, Ian Ray <ian.ray@ge.com> wrote:
> Migrate away from custom EDID parsing and validity checks.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Ian Ray <ian.ray@ge.com>

So this is v2 of [1]. For future reference, people can get really fussy
about preserving authorship. I don't really mind in this case, because
most of the work was actually following through with it, testing, etc.
But maybe add

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

immediately above my Signed-off-by.

Thanks,
Jani.


[1] https://patchwork.freedesktop.org/patch/msgid/20230901102400.552254-1-jani.nikula@intel.com


> ---
>  .../drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c   | 57 ++++------------------
>  1 file changed, 9 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> index 460db3c..e93083b 100644
> --- a/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> +++ b/drivers/gpu/drm/bridge/megachips-stdpxxxx-ge-b850v3-fw.c
> @@ -65,12 +65,11 @@ struct ge_b850v3_lvds {
>  
>  static struct ge_b850v3_lvds *ge_b850v3_lvds_ptr;
>  
> -static u8 *stdp2690_get_edid(struct i2c_client *client)
> +static int stdp2690_read_block(void *context, u8 *buf, unsigned int block, size_t len)
>  {
> +	struct i2c_client *client = context;
>  	struct i2c_adapter *adapter = client->adapter;
> -	unsigned char start = 0x00;
> -	unsigned int total_size;
> -	u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> +	unsigned char start = block * EDID_LENGTH;
>  
>  	struct i2c_msg msgs[] = {
>  		{
> @@ -81,53 +80,15 @@ static u8 *stdp2690_get_edid(struct i2c_client *client)
>  		}, {
>  			.addr	= client->addr,
>  			.flags	= I2C_M_RD,
> -			.len	= EDID_LENGTH,
> -			.buf	= block,
> +			.len	= len,
> +			.buf	= buf,
>  		}
>  	};
>  
> -	if (!block)
> -		return NULL;
> +	if (i2c_transfer(adapter, msgs, 2) != 2)
> +		return -1;
>  
> -	if (i2c_transfer(adapter, msgs, 2) != 2) {
> -		DRM_ERROR("Unable to read EDID.\n");
> -		goto err;
> -	}
> -
> -	if (!drm_edid_block_valid(block, 0, false, NULL)) {
> -		DRM_ERROR("Invalid EDID data\n");
> -		goto err;
> -	}
> -
> -	total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> -	if (total_size > EDID_LENGTH) {
> -		kfree(block);
> -		block = kmalloc(total_size, GFP_KERNEL);
> -		if (!block)
> -			return NULL;
> -
> -		/* Yes, read the entire buffer, and do not skip the first
> -		 * EDID_LENGTH bytes.
> -		 */
> -		start = 0x00;
> -		msgs[1].len = total_size;
> -		msgs[1].buf = block;
> -
> -		if (i2c_transfer(adapter, msgs, 2) != 2) {
> -			DRM_ERROR("Unable to read EDID extension blocks.\n");
> -			goto err;
> -		}
> -		if (!drm_edid_block_valid(block, 1, false, NULL)) {
> -			DRM_ERROR("Invalid EDID data\n");
> -			goto err;
> -		}
> -	}
> -
> -	return block;
> -
> -err:
> -	kfree(block);
> -	return NULL;
> +	return 0;
>  }
>  
>  static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
> @@ -137,7 +98,7 @@ static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge,
>  
>  	client = ge_b850v3_lvds_ptr->stdp2690_i2c;
>  
> -	return (struct edid *)stdp2690_get_edid(client);
> +	return drm_do_get_edid(connector, stdp2690_read_block, client);
>  }
>  
>  static int ge_b850v3_lvds_get_modes(struct drm_connector *connector)

-- 
Jani Nikula, Intel