All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions
@ 2014-04-07  8:37 Thierry Reding
  2014-04-07  9:37 ` [PATCH] drm/i915: support address only i2c-over-aux transactions Jani Nikula
  2014-04-08 14:06 ` [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Christian König
  0 siblings, 2 replies; 11+ messages in thread
From: Thierry Reding @ 2014-04-07  8:37 UTC (permalink / raw)
  To: Alex Deucher; +Cc: dri-devel

From: Thierry Reding <treding@nvidia.com>

Certain types of I2C-over-AUX transactions require that only the address
is transferred. Detect this by looking at the AUX message's size and set
the address-only bit appropriately.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Hi Alex,

This patch is required to make eDP work properly on Tegra with your I2C-
over-AUX patch series applied. Would you consider carrying this in your
series so that bisectability can be maintained? It would need to be
applied prior to the core change (patch 2/4 in the latest series) to do
so.

Thanks,
Thierry

 drivers/gpu/drm/tegra/dpaux.c | 44 ++++++++++++++++++++++++++++++-------------
 drivers/gpu/drm/tegra/dpaux.h |  1 +
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index d536ed381fbd..005c19bd92df 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -99,55 +99,73 @@ static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer,
 static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux,
 				    struct drm_dp_aux_msg *msg)
 {
-	unsigned long value = DPAUX_DP_AUXCTL_TRANSACTREQ;
 	unsigned long timeout = msecs_to_jiffies(250);
 	struct tegra_dpaux *dpaux = to_dpaux(aux);
 	unsigned long status;
 	ssize_t ret = 0;
+	u32 value;
 
-	if (msg->size < 1 || msg->size > 16)
+	/* Tegra has 4x4 byte DP AUX transmit and receive FIFOs. */
+	if (msg->size > 16)
 		return -EINVAL;
 
-	tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR);
+	/*
+	 * Allow zero-sized messages only for I2C, in which case they specify
+	 * address-only transactions.
+	 */
+	if (msg->size < 1) {
+		switch (msg->request & ~DP_AUX_I2C_MOT) {
+		case DP_AUX_I2C_WRITE:
+		case DP_AUX_I2C_READ:
+			value = DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+	} else {
+		/* For non-zero-sized messages, set the CMDLEN field. */
+		value = DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1);
+	}
 
 	switch (msg->request & ~DP_AUX_I2C_MOT) {
 	case DP_AUX_I2C_WRITE:
 		if (msg->request & DP_AUX_I2C_MOT)
-			value = DPAUX_DP_AUXCTL_CMD_MOT_WR;
+			value |= DPAUX_DP_AUXCTL_CMD_MOT_WR;
 		else
-			value = DPAUX_DP_AUXCTL_CMD_I2C_WR;
+			value |= DPAUX_DP_AUXCTL_CMD_I2C_WR;
 
 		break;
 
 	case DP_AUX_I2C_READ:
 		if (msg->request & DP_AUX_I2C_MOT)
-			value = DPAUX_DP_AUXCTL_CMD_MOT_RD;
+			value |= DPAUX_DP_AUXCTL_CMD_MOT_RD;
 		else
-			value = DPAUX_DP_AUXCTL_CMD_I2C_RD;
+			value |= DPAUX_DP_AUXCTL_CMD_I2C_RD;
 
 		break;
 
 	case DP_AUX_I2C_STATUS:
 		if (msg->request & DP_AUX_I2C_MOT)
-			value = DPAUX_DP_AUXCTL_CMD_MOT_RQ;
+			value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ;
 		else
-			value = DPAUX_DP_AUXCTL_CMD_I2C_RQ;
+			value |= DPAUX_DP_AUXCTL_CMD_I2C_RQ;
 
 		break;
 
 	case DP_AUX_NATIVE_WRITE:
-		value = DPAUX_DP_AUXCTL_CMD_AUX_WR;
+		value |= DPAUX_DP_AUXCTL_CMD_AUX_WR;
 		break;
 
 	case DP_AUX_NATIVE_READ:
-		value = DPAUX_DP_AUXCTL_CMD_AUX_RD;
+		value |= DPAUX_DP_AUXCTL_CMD_AUX_RD;
 		break;
 
 	default:
 		return -EINVAL;
 	}
 
-	value |= DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1);
+	tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR);
 	tegra_dpaux_writel(dpaux, value, DPAUX_DP_AUXCTL);
 
 	if ((msg->request & DP_AUX_I2C_READ) == 0) {
@@ -198,7 +216,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux,
 		break;
 	}
 
-	if (msg->reply == DP_AUX_NATIVE_REPLY_ACK) {
+	if ((msg->size > 0) && (msg->reply == DP_AUX_NATIVE_REPLY_ACK)) {
 		if (msg->request & DP_AUX_I2C_READ) {
 			size_t count = value & DPAUX_DP_AUXSTAT_REPLY_MASK;
 
diff --git a/drivers/gpu/drm/tegra/dpaux.h b/drivers/gpu/drm/tegra/dpaux.h
index 4f5bf10fdff9..806e245ca787 100644
--- a/drivers/gpu/drm/tegra/dpaux.h
+++ b/drivers/gpu/drm/tegra/dpaux.h
@@ -32,6 +32,7 @@
 #define DPAUX_DP_AUXCTL_CMD_I2C_RQ (2 << 12)
 #define DPAUX_DP_AUXCTL_CMD_I2C_RD (1 << 12)
 #define DPAUX_DP_AUXCTL_CMD_I2C_WR (0 << 12)
+#define DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY (1 << 8)
 #define DPAUX_DP_AUXCTL_CMDLEN(x) ((x) & 0xff)
 
 #define DPAUX_DP_AUXSTAT 0x31
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-07  8:37 [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Thierry Reding
@ 2014-04-07  9:37 ` Jani Nikula
  2014-04-07 14:35   ` Alex Deucher
  2014-04-08 14:06 ` [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Christian König
  1 sibling, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2014-04-07  9:37 UTC (permalink / raw)
  To: dri-devel, intel-gfx, Alex Deucher; +Cc: jani.nikula, Thierry Reding

To support bare address requests used by the drm dp helpers.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Hi Alex, similar to Thierry's patch for i915.

BR,
Jani.
---
 drivers/gpu/drm/i915/intel_dp.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e48d47ced57b..b435d07fbc08 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -575,7 +575,8 @@ out:
 	return ret;
 }
 
-#define HEADER_SIZE	4
+#define BARE_ADDRESS_SIZE	3
+#define HEADER_SIZE		(BARE_ADDRESS_SIZE + 1)
 static ssize_t
 intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 {
@@ -592,7 +593,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 	switch (msg->request & ~DP_AUX_I2C_MOT) {
 	case DP_AUX_NATIVE_WRITE:
 	case DP_AUX_I2C_WRITE:
-		txsize = HEADER_SIZE + msg->size;
+		txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE;
 		rxsize = 1;
 
 		if (WARN_ON(txsize > 20))
@@ -611,7 +612,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 
 	case DP_AUX_NATIVE_READ:
 	case DP_AUX_I2C_READ:
-		txsize = HEADER_SIZE;
+		txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
 		rxsize = msg->size + 1;
 
 		if (WARN_ON(rxsize > 20))
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-07  9:37 ` [PATCH] drm/i915: support address only i2c-over-aux transactions Jani Nikula
@ 2014-04-07 14:35   ` Alex Deucher
  2014-04-07 20:00     ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Deucher @ 2014-04-07 14:35 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Intel Graphics Development, Thierry Reding, Maling list - DRI developers

On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> To support bare address requests used by the drm dp helpers.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> Hi Alex, similar to Thierry's patch for i915.
>

Looks good to me.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Do you want me to add this to the patch set?

Alex

> BR,
> Jani.
> ---
>  drivers/gpu/drm/i915/intel_dp.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e48d47ced57b..b435d07fbc08 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -575,7 +575,8 @@ out:
>         return ret;
>  }
>
> -#define HEADER_SIZE    4
> +#define BARE_ADDRESS_SIZE      3
> +#define HEADER_SIZE            (BARE_ADDRESS_SIZE + 1)
>  static ssize_t
>  intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  {
> @@ -592,7 +593,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>         switch (msg->request & ~DP_AUX_I2C_MOT) {
>         case DP_AUX_NATIVE_WRITE:
>         case DP_AUX_I2C_WRITE:
> -               txsize = HEADER_SIZE + msg->size;
> +               txsize = msg->size ? HEADER_SIZE + msg->size : BARE_ADDRESS_SIZE;
>                 rxsize = 1;
>
>                 if (WARN_ON(txsize > 20))
> @@ -611,7 +612,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>
>         case DP_AUX_NATIVE_READ:
>         case DP_AUX_I2C_READ:
> -               txsize = HEADER_SIZE;
> +               txsize = msg->size ? HEADER_SIZE : BARE_ADDRESS_SIZE;
>                 rxsize = msg->size + 1;
>
>                 if (WARN_ON(rxsize > 20))
> --
> 1.7.9.5
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-07 14:35   ` Alex Deucher
@ 2014-04-07 20:00     ` Daniel Vetter
  2014-04-08  6:58       ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-04-07 20:00 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Jani Nikula, Intel Graphics Development, Thierry Reding,
	Maling list - DRI developers

On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> To support bare address requests used by the drm dp helpers.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> Hi Alex, similar to Thierry's patch for i915.
>>
>
> Looks good to me.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Do you want me to add this to the patch set?

Afaict we've done an unconditional msg_bytes = send_bytes + 4; in the
i915 driver before the conversion to the dp aux helper, which is why
I've slapped an r-b onto your patch without asking for a i915
adjustement. Otoh we have a pile of bugs for dp dongles still. Imo my
preferred approach would be to get the helper + radeon stuff in and
then hawk the i915 patch to a bunch of bug reporter and see whether it
sticks. Ofc if we already know that we need it it would be best to
merge it in one pull together with all your other patches.

Jani?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-07 20:00     ` Daniel Vetter
@ 2014-04-08  6:58       ` Jani Nikula
  2014-04-08  8:03         ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2014-04-08  6:58 UTC (permalink / raw)
  To: Daniel Vetter, Alex Deucher
  Cc: Intel Graphics Development, Maling list - DRI developers

On Mon, 07 Apr 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>>> To support bare address requests used by the drm dp helpers.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>
>>> ---
>>>
>>> Hi Alex, similar to Thierry's patch for i915.
>>>
>>
>> Looks good to me.
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Do you want me to add this to the patch set?
>
> Afaict we've done an unconditional msg_bytes = send_bytes + 4; in the
> i915 driver before the conversion to the dp aux helper, which is why
> I've slapped an r-b onto your patch without asking for a i915
> adjustement. Otoh we have a pile of bugs for dp dongles still. Imo my
> preferred approach would be to get the helper + radeon stuff in and
> then hawk the i915 patch to a bunch of bug reporter and see whether it
> sticks. Ofc if we already know that we need it it would be best to
> merge it in one pull together with all your other patches.
>
> Jani?

Before the conversion to dp aux helpers there was a switch case [1] that
ended up in msg_bytes = 3 for address only start/stop messages
(MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
but without my patch, we'd send the four byte header but with 0 - 1 =
0xff in txbuf[3]. I'm pretty sure breakage would follow.

Thus I'd like to have my patch in before the dp aux helpers use
msg->size == 0 for address only messages.

Daniel, convinced yet?

BR,
Jani.


[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/i915/intel_dp.c?id=v3.14#n654

[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_dp_helper.c?id=v3.14#n59


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-08  6:58       ` Jani Nikula
@ 2014-04-08  8:03         ` Daniel Vetter
  2014-04-08 13:04           ` [Intel-gfx] " Alex Deucher
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2014-04-08  8:03 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Alex Deucher, Intel Graphics Development, Maling list - DRI developers

On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
> Before the conversion to dp aux helpers there was a switch case [1] that
> ended up in msg_bytes = 3 for address only start/stop messages
> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
> but without my patch, we'd send the four byte header but with 0 - 1 =
> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>
> Thus I'd like to have my patch in before the dp aux helpers use
> msg->size == 0 for address only messages.
>
> Daniel, convinced yet?

Indeed, I've been blinding. Acked for merging through radeon trees
together with the other patches as a fixup for 3.15.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-08  8:03         ` Daniel Vetter
@ 2014-04-08 13:04           ` Alex Deucher
  2014-04-08 13:09             ` Christian König
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Deucher @ 2014-04-08 13:04 UTC (permalink / raw)
  To: Daniel Vetter, Christian König
  Cc: Jani Nikula, Intel Graphics Development, Maling list - DRI developers

On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>> Before the conversion to dp aux helpers there was a switch case [1] that
>> ended up in msg_bytes = 3 for address only start/stop messages
>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>> but without my patch, we'd send the four byte header but with 0 - 1 =
>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>
>> Thus I'd like to have my patch in before the dp aux helpers use
>> msg->size == 0 for address only messages.
>>
>> Daniel, convinced yet?
>
> Indeed, I've been blinding. Acked for merging through radeon trees
> together with the other patches as a fixup for 3.15.

Christian,

Can you pull this into the radeon 3.15 branch as well?

Thanks,

Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-08 13:04           ` [Intel-gfx] " Alex Deucher
@ 2014-04-08 13:09             ` Christian König
  2014-04-08 13:11               ` Alex Deucher
  0 siblings, 1 reply; 11+ messages in thread
From: Christian König @ 2014-04-08 13:09 UTC (permalink / raw)
  To: Alex Deucher, Daniel Vetter
  Cc: Jani Nikula, Intel Graphics Development, Maling list - DRI developers

Am 08.04.2014 15:04, schrieb Alex Deucher:
> On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com> wrote:
>>> Before the conversion to dp aux helpers there was a switch case [1] that
>>> ended up in msg_bytes = 3 for address only start/stop messages
>>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>>> but without my patch, we'd send the four byte header but with 0 - 1 =
>>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>>
>>> Thus I'd like to have my patch in before the dp aux helpers use
>>> msg->size == 0 for address only messages.
>>>
>>> Daniel, convinced yet?
>> Indeed, I've been blinding. Acked for merging through radeon trees
>> together with the other patches as a fixup for 3.15.
> Christian,
>
> Can you pull this into the radeon 3.15 branch as well?

Sure, directly before your other patches I would assume.

Going to send out the pull request to Dave this evening if nobody objects.

Christian.

>
> Thanks,
>
> Alex

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: support address only i2c-over-aux transactions
  2014-04-08 13:09             ` Christian König
@ 2014-04-08 13:11               ` Alex Deucher
  0 siblings, 0 replies; 11+ messages in thread
From: Alex Deucher @ 2014-04-08 13:11 UTC (permalink / raw)
  To: Christian König
  Cc: Jani Nikula, Intel Graphics Development, Maling list - DRI developers

On Tue, Apr 8, 2014 at 9:09 AM, Christian König <deathsimple@vodafone.de> wrote:
> Am 08.04.2014 15:04, schrieb Alex Deucher:
>
>> On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>
>>> On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula <jani.nikula@intel.com>
>>> wrote:
>>>>
>>>> Before the conversion to dp aux helpers there was a switch case [1] that
>>>> ended up in msg_bytes = 3 for address only start/stop messages
>>>> (MODE_I2C_START or MODE_I2C_STOP bit set [2]). With Alex's series in,
>>>> but without my patch, we'd send the four byte header but with 0 - 1 =
>>>> 0xff in txbuf[3]. I'm pretty sure breakage would follow.
>>>>
>>>> Thus I'd like to have my patch in before the dp aux helpers use
>>>> msg->size == 0 for address only messages.
>>>>
>>>> Daniel, convinced yet?
>>>
>>> Indeed, I've been blinding. Acked for merging through radeon trees
>>> together with the other patches as a fixup for 3.15.
>>
>> Christian,
>>
>> Can you pull this into the radeon 3.15 branch as well?
>
>
> Sure, directly before your other patches I would assume.

yes.

>
> Going to send out the pull request to Dave this evening if nobody objects.
>

Sounds good to me.

Thanks!

Alex

> Christian.
>
>>
>> Thanks,
>>
>> Alex
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions
  2014-04-07  8:37 [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Thierry Reding
  2014-04-07  9:37 ` [PATCH] drm/i915: support address only i2c-over-aux transactions Jani Nikula
@ 2014-04-08 14:06 ` Christian König
  2014-04-08 20:47   ` Thierry Reding
  1 sibling, 1 reply; 11+ messages in thread
From: Christian König @ 2014-04-08 14:06 UTC (permalink / raw)
  To: Thierry Reding, Alex Deucher; +Cc: dri-devel

Am 07.04.2014 10:37, schrieb Thierry Reding:
> From: Thierry Reding <treding@nvidia.com>
>
> Certain types of I2C-over-AUX transactions require that only the address
> is transferred. Detect this by looking at the AUX message's size and set
> the address-only bit appropriately.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Hi Alex,
>
> This patch is required to make eDP work properly on Tegra with your I2C-
> over-AUX patch series applied. Would you consider carrying this in your
> series so that bisectability can be maintained? It would need to be
> applied prior to the core change (patch 2/4 in the latest series) to do
> so.

Hi Thierry,

do you already have this patch in drm-next or should I send it do Dave 
together with the pull request for the dp aux helper changes?

Regards,
Christian.

>
> Thanks,
> Thierry
>
>   drivers/gpu/drm/tegra/dpaux.c | 44 ++++++++++++++++++++++++++++++-------------
>   drivers/gpu/drm/tegra/dpaux.h |  1 +
>   2 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index d536ed381fbd..005c19bd92df 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -99,55 +99,73 @@ static void tegra_dpaux_read_fifo(struct tegra_dpaux *dpaux, u8 *buffer,
>   static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux,
>   				    struct drm_dp_aux_msg *msg)
>   {
> -	unsigned long value = DPAUX_DP_AUXCTL_TRANSACTREQ;
>   	unsigned long timeout = msecs_to_jiffies(250);
>   	struct tegra_dpaux *dpaux = to_dpaux(aux);
>   	unsigned long status;
>   	ssize_t ret = 0;
> +	u32 value;
>   
> -	if (msg->size < 1 || msg->size > 16)
> +	/* Tegra has 4x4 byte DP AUX transmit and receive FIFOs. */
> +	if (msg->size > 16)
>   		return -EINVAL;
>   
> -	tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR);
> +	/*
> +	 * Allow zero-sized messages only for I2C, in which case they specify
> +	 * address-only transactions.
> +	 */
> +	if (msg->size < 1) {
> +		switch (msg->request & ~DP_AUX_I2C_MOT) {
> +		case DP_AUX_I2C_WRITE:
> +		case DP_AUX_I2C_READ:
> +			value = DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY;
> +			break;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	} else {
> +		/* For non-zero-sized messages, set the CMDLEN field. */
> +		value = DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1);
> +	}
>   
>   	switch (msg->request & ~DP_AUX_I2C_MOT) {
>   	case DP_AUX_I2C_WRITE:
>   		if (msg->request & DP_AUX_I2C_MOT)
> -			value = DPAUX_DP_AUXCTL_CMD_MOT_WR;
> +			value |= DPAUX_DP_AUXCTL_CMD_MOT_WR;
>   		else
> -			value = DPAUX_DP_AUXCTL_CMD_I2C_WR;
> +			value |= DPAUX_DP_AUXCTL_CMD_I2C_WR;
>   
>   		break;
>   
>   	case DP_AUX_I2C_READ:
>   		if (msg->request & DP_AUX_I2C_MOT)
> -			value = DPAUX_DP_AUXCTL_CMD_MOT_RD;
> +			value |= DPAUX_DP_AUXCTL_CMD_MOT_RD;
>   		else
> -			value = DPAUX_DP_AUXCTL_CMD_I2C_RD;
> +			value |= DPAUX_DP_AUXCTL_CMD_I2C_RD;
>   
>   		break;
>   
>   	case DP_AUX_I2C_STATUS:
>   		if (msg->request & DP_AUX_I2C_MOT)
> -			value = DPAUX_DP_AUXCTL_CMD_MOT_RQ;
> +			value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ;
>   		else
> -			value = DPAUX_DP_AUXCTL_CMD_I2C_RQ;
> +			value |= DPAUX_DP_AUXCTL_CMD_I2C_RQ;
>   
>   		break;
>   
>   	case DP_AUX_NATIVE_WRITE:
> -		value = DPAUX_DP_AUXCTL_CMD_AUX_WR;
> +		value |= DPAUX_DP_AUXCTL_CMD_AUX_WR;
>   		break;
>   
>   	case DP_AUX_NATIVE_READ:
> -		value = DPAUX_DP_AUXCTL_CMD_AUX_RD;
> +		value |= DPAUX_DP_AUXCTL_CMD_AUX_RD;
>   		break;
>   
>   	default:
>   		return -EINVAL;
>   	}
>   
> -	value |= DPAUX_DP_AUXCTL_CMDLEN(msg->size - 1);
> +	tegra_dpaux_writel(dpaux, msg->address, DPAUX_DP_AUXADDR);
>   	tegra_dpaux_writel(dpaux, value, DPAUX_DP_AUXCTL);
>   
>   	if ((msg->request & DP_AUX_I2C_READ) == 0) {
> @@ -198,7 +216,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux,
>   		break;
>   	}
>   
> -	if (msg->reply == DP_AUX_NATIVE_REPLY_ACK) {
> +	if ((msg->size > 0) && (msg->reply == DP_AUX_NATIVE_REPLY_ACK)) {
>   		if (msg->request & DP_AUX_I2C_READ) {
>   			size_t count = value & DPAUX_DP_AUXSTAT_REPLY_MASK;
>   
> diff --git a/drivers/gpu/drm/tegra/dpaux.h b/drivers/gpu/drm/tegra/dpaux.h
> index 4f5bf10fdff9..806e245ca787 100644
> --- a/drivers/gpu/drm/tegra/dpaux.h
> +++ b/drivers/gpu/drm/tegra/dpaux.h
> @@ -32,6 +32,7 @@
>   #define DPAUX_DP_AUXCTL_CMD_I2C_RQ (2 << 12)
>   #define DPAUX_DP_AUXCTL_CMD_I2C_RD (1 << 12)
>   #define DPAUX_DP_AUXCTL_CMD_I2C_WR (0 << 12)
> +#define DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY (1 << 8)
>   #define DPAUX_DP_AUXCTL_CMDLEN(x) ((x) & 0xff)
>   
>   #define DPAUX_DP_AUXSTAT 0x31

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions
  2014-04-08 14:06 ` [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Christian König
@ 2014-04-08 20:47   ` Thierry Reding
  0 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2014-04-08 20:47 UTC (permalink / raw)
  To: Christian König; +Cc: dri-devel

On Tue, Apr 8, 2014 at 4:06 PM, Christian König <deathsimple@vodafone.de> wrote:
> Am 07.04.2014 10:37, schrieb Thierry Reding:
>
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Certain types of I2C-over-AUX transactions require that only the address
>> is transferred. Detect this by looking at the AUX message's size and set
>> the address-only bit appropriately.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>> Hi Alex,
>>
>> This patch is required to make eDP work properly on Tegra with your I2C-
>> over-AUX patch series applied. Would you consider carrying this in your
>> series so that bisectability can be maintained? It would need to be
>> applied prior to the core change (patch 2/4 in the latest series) to do
>> so.
>
>
> Hi Thierry,
>
> do you already have this patch in drm-next or should I send it do Dave
> together with the pull request for the dp aux helper changes?

The patch is not in drm-next. To preserve bisectability it should be part of
Alex' patch set and it needs to be applied before the patch that makes use
of the bare address transactions (patch 2/4 in the series).

Thierry
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-04-08 20:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-07  8:37 [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Thierry Reding
2014-04-07  9:37 ` [PATCH] drm/i915: support address only i2c-over-aux transactions Jani Nikula
2014-04-07 14:35   ` Alex Deucher
2014-04-07 20:00     ` Daniel Vetter
2014-04-08  6:58       ` Jani Nikula
2014-04-08  8:03         ` Daniel Vetter
2014-04-08 13:04           ` [Intel-gfx] " Alex Deucher
2014-04-08 13:09             ` Christian König
2014-04-08 13:11               ` Alex Deucher
2014-04-08 14:06 ` [PATCH] drm/tegra: dp: Support address-only I2C-over-AUX transactions Christian König
2014-04-08 20:47   ` Thierry Reding

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.