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
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))
On Mon, Apr 7, 2014 at 5:37 AM, Jani Nikula jani.nikula@intel.com wrote:
Looks good to me.
Reviewed-by: Alex Deucher alexander.deucher@amd.com
Do you want me to add this to the patch set?
Alex
On Mon, Apr 7, 2014 at 4:35 PM, Alex Deucher alexdeucher@gmail.com wrote:
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
On Mon, 07 Apr 2014, Daniel Vetter daniel@ffwll.ch 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?
BR, Jani.
[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/...
[2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/...
On Tue, Apr 8, 2014 at 8:58 AM, Jani Nikula jani.nikula@intel.com wrote:
Indeed, I've been blinding. Acked for merging through radeon trees together with the other patches as a fixup for 3.15. -Daniel
On Tue, Apr 8, 2014 at 4:03 AM, Daniel Vetter daniel@ffwll.ch wrote:
Christian,
Can you pull this into the radeon 3.15 branch as well?
Thanks,
Alex
Am 08.04.2014 15:04, schrieb Alex Deucher:
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
On Tue, Apr 8, 2014 at 9:09 AM, Christian König deathsimple@vodafone.de wrote:
yes.
Going to send out the pull request to Dave this evening if nobody objects.
Sounds good to me.
Thanks!
Alex
On Tue, Apr 8, 2014 at 4:06 PM, Christian König deathsimple@vodafone.de wrote:
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@lists.freedesktop.org