[v5,10/15] drm/bridge: tc358767: Add support for address-only I2C transfers

Submitted by Andrey Smirnov on June 12, 2019, 8:32 a.m.

Details

Message ID 20190612083252.15321-11-andrew.smirnov@gmail.com
State Superseded
Headers show
Series "tc358767 driver improvements" ( rev: 5 ) in DRI devel

Not browsing as part of any series.

Commit Message

Andrey Smirnov June 12, 2019, 8:32 a.m.
Transfer size of zero means a request to do an address-only
transfer. Since the HW support this, we probably shouldn't be just
ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass
through, since it is supported by the HW as well.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/gpu/drm/bridge/tc358767.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 7d0fbb12195b..4bb9b15e1324 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -145,6 +145,8 @@ 
 
 /* AUX channel */
 #define DP0_AUXCFG0		0x0660
+#define DP0_AUXCFG0_BSIZE	GENMASK(11, 8)
+#define DP0_AUXCFG0_ADDR_ONLY	BIT(4)
 #define DP0_AUXCFG1		0x0664
 #define AUX_RX_FILTER_EN		BIT(16)
 
@@ -327,6 +329,18 @@  static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
 	return size;
 }
 
+static u32 tc_auxcfg0(struct drm_dp_aux_msg *msg, size_t size)
+{
+	u32 auxcfg0 = msg->request;
+
+	if (size)
+		auxcfg0 |= FIELD_PREP(DP0_AUXCFG0_BSIZE, size - 1);
+	else
+		auxcfg0 |= DP0_AUXCFG0_ADDR_ONLY;
+
+	return auxcfg0;
+}
+
 static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 			       struct drm_dp_aux_msg *msg)
 {
@@ -336,9 +350,6 @@  static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 	u32 auxstatus;
 	int ret;
 
-	if (size == 0)
-		return 0;
-
 	ret = tc_aux_wait_busy(tc, 100);
 	if (ret)
 		return ret;
@@ -362,8 +373,7 @@  static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 	if (ret)
 		return ret;
 	/* Start transfer */
-	ret = regmap_write(tc->regmap, DP0_AUXCFG0,
-			   ((size - 1) << 8) | request);
+	ret = regmap_write(tc->regmap, DP0_AUXCFG0, tc_auxcfg0(msg, size));
 	if (ret)
 		return ret;
 
@@ -377,8 +387,14 @@  static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 
 	if (auxstatus & AUX_TIMEOUT)
 		return -ETIMEDOUT;
-
-	size = FIELD_GET(AUX_BYTES, auxstatus);
+	/*
+	 * For some reason address-only DP_AUX_I2C_WRITE (MOT), still
+	 * reports 1 byte transferred in its status. To deal we that
+	 * we ignore aux_bytes field if we know that this was an
+	 * address-only transfer
+	 */
+	if (size)
+		size = FIELD_GET(AUX_BYTES, auxstatus);
 	msg->reply = FIELD_GET(AUX_STATUS, auxstatus);
 
 	switch (request) {

Comments

On Wed, Jun 12, 2019 at 5:48 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Hi,
>
> On 12/06/2019 11:32, Andrey Smirnov wrote:
> > Transfer size of zero means a request to do an address-only
> > transfer. Since the HW support this, we probably shouldn't be just
> > ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass
> > through, since it is supported by the HW as well.
>
> I bisected the EDID read issue to this patch...
>

I don't think I've had any problems on my end with this. I'll double
check. It might be the case that yours is the only setup where the
problem can be repro'd, though. We can drop this patch if you don't
have time/would rather not dig into this.

Thanks,
Andrey Smirnov
On Wed, Jun 12, 2019 at 9:22 AM Andrey Smirnov <andrew.smirnov@gmail.com> wrote:
>
> On Wed, Jun 12, 2019 at 5:48 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > Hi,
> >
> > On 12/06/2019 11:32, Andrey Smirnov wrote:
> > > Transfer size of zero means a request to do an address-only
> > > transfer. Since the HW support this, we probably shouldn't be just
> > > ignoring such requests. While at it allow DP_AUX_I2C_MOT flag to pass
> > > through, since it is supported by the HW as well.
> >
> > I bisected the EDID read issue to this patch...
> >
>
> I don't think I've had any problems on my end with this. I'll double
> check. It might be the case that yours is the only setup where the
> problem can be repro'd, though. We can drop this patch if you don't
> have time/would rather not dig into this.
>

Turns out I do have this problem. Just didn't pay enough attention to
notice. Will fix in v6.

Thanks,
Andrey Smirnov