[RESEND] drm: tda998x: Use drm_do_get_edid()

Commit Message

Laurent Pinchart Jan. 16, 2015, 4:37 p.m. UTC

Replace the internal EDID read implementation by a call to the new EDID
read core function.

Signed-off-by: Laurent Pinchart <[e-mail]>
Reviewed-by: Rob Clark <[e-mail]>
Tested-by: Jean-Francois Moine <[e-mail]>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 86 ++++++++-------------------------------
 1 file changed, 18 insertions(+), 68 deletions(-)

Comments

Russell King - ARM Linux Jan. 16, 2015, 5:47 p.m. UTC | #1

On Fri, Jan 16, 2015 at 06:37:43PM +0200, Laurent Pinchart wrote:
> Replace the internal EDID read implementation by a call to the new EDID
> read core function.
> 
> Signed-off-by: Laurent Pinchart <[e-mail]>
> Reviewed-by: Rob Clark <[e-mail]>
> Tested-by: Jean-Francois Moine <[e-mail]>

Thanks, committed, and updated the summary line to:

"drm/i2c: tda998x: use drm_do_get_edid()"

to match the style used in the past.

Jean-Francois Moine Jan. 17, 2015, 8:07 a.m. UTC | #2

On Fri, 16 Jan 2015 17:47:34 +0000
Russell King - ARM Linux <[e-mail]> wrote:

> Thanks, committed, and updated the summary line to:
> 
> "drm/i2c: tda998x: use drm_do_get_edid()"
> 
> to match the style used in the past.

You also committed 2 fixes of mine on the tda998x, but I could not
retrieve them in linux-next nor elsewhere.

I will soon resubmit a new patch of the tda998x codec. I could base it
on linux-next or Mark's ASoC reference, but basing it on your tda998x
version would avoid you to merge...

Laurent Pinchart Feb. 26, 2015, 7:52 a.m. UTC | #3

Hi Russell,

On Friday 16 January 2015 17:47:34 Russell King - ARM Linux wrote:
> On Fri, Jan 16, 2015 at 06:37:43PM +0200, Laurent Pinchart wrote:
> > Replace the internal EDID read implementation by a call to the new EDID
> > read core function.
> > 
> > Signed-off-by: Laurent Pinchart
> > <[e-mail]>
> > Reviewed-by: Rob Clark <[e-mail]>
> > Tested-by: Jean-Francois Moine <[e-mail]>
> 
> Thanks, committed, and updated the summary line to:
> 
> "drm/i2c: tda998x: use drm_do_get_edid()"
> 
> to match the style used in the past.

I can't see the patch in Dave's -next branch, has it been lost somewhere ?

Russell King - ARM Linux Feb. 26, 2015, 9:20 a.m. UTC | #4

On Thu, Feb 26, 2015 at 09:52:02AM +0200, Laurent Pinchart wrote:
> Hi Russell,
> 
> On Friday 16 January 2015 17:47:34 Russell King - ARM Linux wrote:
> > On Fri, Jan 16, 2015 at 06:37:43PM +0200, Laurent Pinchart wrote:
> > > Replace the internal EDID read implementation by a call to the new EDID
> > > read core function.
> > > 
> > > Signed-off-by: Laurent Pinchart
> > > <[e-mail]>
> > > Reviewed-by: Rob Clark <[e-mail]>
> > > Tested-by: Jean-Francois Moine <[e-mail]>
> > 
> > Thanks, committed, and updated the summary line to:
> > 
> > "drm/i2c: tda998x: use drm_do_get_edid()"
> > 
> > to match the style used in the past.
> 
> I can't see the patch in Dave's -next branch, has it been lost somewhere ?

I thought I had sent a pull request before the last merge window, but
it seems I didn't.  That's the problem when you end up carrying too many
patches... tracking what's been sent and to whom is a real pain.  The
only time I get to verify what's been sent is after it's appeared in
mainline (so after a merge window).

Laurent Pinchart Feb. 26, 2015, 6:55 p.m. UTC | #5

Hi Russell,

On Thursday 26 February 2015 09:20:08 Russell King - ARM Linux wrote:
> On Thu, Feb 26, 2015 at 09:52:02AM +0200, Laurent Pinchart wrote:
> > On Friday 16 January 2015 17:47:34 Russell King - ARM Linux wrote:
> >> On Fri, Jan 16, 2015 at 06:37:43PM +0200, Laurent Pinchart wrote:
> >>> Replace the internal EDID read implementation by a call to the new
> >>> EDID
> >>> read core function.
> >>> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <[e-mail]>
> >>> Reviewed-by: Rob Clark <[e-mail]>
> >>> Tested-by: Jean-Francois Moine <[e-mail]>
> >> 
> >> Thanks, committed, and updated the summary line to:
> >> 
> >> "drm/i2c: tda998x: use drm_do_get_edid()"
> >> 
> >> to match the style used in the past.
> > 
> > I can't see the patch in Dave's -next branch, has it been lost somewhere ?
> 
> I thought I had sent a pull request before the last merge window, but
> it seems I didn't.  That's the problem when you end up carrying too many
> patches... tracking what's been sent and to whom is a real pain.  The
> only time I get to verify what's been sent is after it's appeared in
> mainline (so after a merge window).

No worries. Looks like I'm part of a distributed patch tracker system then ;-)

Patch

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index d4762799351d..a267a78a0d0a 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1011,8 +1011,9 @@  tda998x_encoder_detect(struct tda998x_priv *priv)
 			connector_status_disconnected;
 }
 
-static int read_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
+static int read_edid_block(void *data, u8 *buf, unsigned int blk, size_t length)
 {
+	struct tda998x_priv *priv = data;
 	uint8_t offset, segptr;
 	int ret, i;
 
@@ -1056,8 +1057,8 @@  static int read_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
 		return -ETIMEDOUT;
 	}
 
-	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, EDID_LENGTH);
-	if (ret != EDID_LENGTH) {
+	ret = reg_read_range(priv, REG_EDID_DATA_0, buf, length);
+	if (ret != length) {
 		dev_err(&priv->hdmi->dev, "failed to read edid block %d: %d\n",
 			blk, ret);
 		return ret;
@@ -1066,82 +1067,31 @@  static int read_edid_block(struct tda998x_priv *priv, uint8_t *buf, int blk)
 	return 0;
 }
 
-static uint8_t *do_get_edid(struct tda998x_priv *priv)
+static int
+tda998x_encoder_get_modes(struct tda998x_priv *priv,
+			  struct drm_connector *connector)
 {
-	int j, valid_extensions = 0;
-	uint8_t *block, *new;
-	bool print_bad_edid = drm_debug & DRM_UT_KMS;
-
-	if ((block = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
-		return NULL;
+	struct edid *edid;
+	int n;
 
 	if (priv->rev == TDA19988)
 		reg_clear(priv, REG_TX4, TX4_PD_RAM);
 
-	/* base block fetch */
-	if (read_edid_block(priv, block, 0))
-		goto fail;
-
-	if (!drm_edid_block_valid(block, 0, print_bad_edid))
-		goto fail;
-
-	/* if there's no extensions, we're done */
-	if (block[0x7e] == 0)
-		goto done;
-
-	new = krealloc(block, (block[0x7e] + 1) * EDID_LENGTH, GFP_KERNEL);
-	if (!new)
-		goto fail;
-	block = new;
-
-	for (j = 1; j <= block[0x7e]; j++) {
-		uint8_t *ext_block = block + (valid_extensions + 1) * EDID_LENGTH;
-		if (read_edid_block(priv, ext_block, j))
-			goto fail;
-
-		if (!drm_edid_block_valid(ext_block, j, print_bad_edid))
-			goto fail;
+	edid = drm_do_get_edid(connector, read_edid_block, priv);
 
-		valid_extensions++;
-	}
-
-	if (valid_extensions != block[0x7e]) {
-		block[EDID_LENGTH-1] += block[0x7e] - valid_extensions;
-		block[0x7e] = valid_extensions;
-		new = krealloc(block, (valid_extensions + 1) * EDID_LENGTH, GFP_KERNEL);
-		if (!new)
-			goto fail;
-		block = new;
-	}
-
-done:
 	if (priv->rev == TDA19988)
 		reg_set(priv, REG_TX4, TX4_PD_RAM);
 
-	return block;
-
-fail:
-	if (priv->rev == TDA19988)
-		reg_set(priv, REG_TX4, TX4_PD_RAM);
-	dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
-	kfree(block);
-	return NULL;
-}
-
-static int
-tda998x_encoder_get_modes(struct tda998x_priv *priv,
-			  struct drm_connector *connector)
-{
-	struct edid *edid = (struct edid *)do_get_edid(priv);
-	int n = 0;
-
-	if (edid) {
-		drm_mode_connector_update_edid_property(connector, edid);
-		n = drm_add_edid_modes(connector, edid);
-		priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
-		kfree(edid);
+	if (!edid) {
+		dev_warn(&priv->hdmi->dev, "failed to read EDID\n");
+		return 0;
 	}
 
+	drm_mode_connector_update_edid_property(connector, edid);
+	n = drm_add_edid_modes(connector, edid);
+	priv->is_hdmi_sink = drm_detect_hdmi_monitor(edid);
+	kfree(edid);
+
 	return n;
 }