diff mbox

drm/tegra: dp: Support address-only I2C-over-AUX transactions

Message ID 1396859864-25955-1-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding April 7, 2014, 8:37 a.m. UTC 
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(-)

Comments

Christian König April 8, 2014, 2:06 p.m. UTC | #1 
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
Thierry Reding April 8, 2014, 8:47 p.m. UTC | #2 
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
diff mbox

Patch

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