On Mon, Dec 16, 2013 at 5:01 PM, Thierry Reding
<thierry.reding@xxxxxxxxx> wrote:
> The current values seem to be defined in a format that's specific to the
> i915, gma500 and radeon drivers. To make this more generally useful, use
> the values as defined in the specification.
>
> While at it, prefix the constants with DP_ for improved namespacing.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
Sorry if I'm late to the party. Seems good to me.
Perhaps all that shifting is a little unintuitive but I don't wanna
nitpick this.
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@xxxxxxxxx>
> ---
> drivers/gpu/drm/gma500/cdv_intel_dp.c | 37 ++++++++++++++++++-----------------
> drivers/gpu/drm/i915/intel_dp.c | 37 ++++++++++++++++++-----------------
> drivers/gpu/drm/radeon/atombios_dp.c | 36 ++++++++++++++++++----------------
> include/drm/drm_dp_helper.h | 32 +++++++++++++++---------------
> 4 files changed, 73 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> index f88a1815d87c..6a7c2481d4ab 100644
> --- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
> +++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
> @@ -483,7 +483,7 @@ cdv_intel_dp_aux_native_write(struct gma_encoder *encoder,
>
> if (send_bytes > 16)
> return -1;
> - msg[0] = AUX_NATIVE_WRITE << 4;
> + msg[0] = DP_AUX_NATIVE_WRITE << 4;
> msg[1] = address >> 8;
> msg[2] = address & 0xff;
> msg[3] = send_bytes - 1;
> @@ -493,9 +493,10 @@ cdv_intel_dp_aux_native_write(struct gma_encoder *encoder,
> ret = cdv_intel_dp_aux_ch(encoder, msg, msg_bytes, &ack, 1);
> if (ret < 0)
> return ret;
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> + ack >>= 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
> break;
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(100);
> else
> return -EIO;
> @@ -523,7 +524,7 @@ cdv_intel_dp_aux_native_read(struct gma_encoder *encoder,
> uint8_t ack;
> int ret;
>
> - msg[0] = AUX_NATIVE_READ << 4;
> + msg[0] = DP_AUX_NATIVE_READ << 4;
> msg[1] = address >> 8;
> msg[2] = address & 0xff;
> msg[3] = recv_bytes - 1;
> @@ -538,12 +539,12 @@ cdv_intel_dp_aux_native_read(struct gma_encoder *encoder,
> return -EPROTO;
> if (ret < 0)
> return ret;
> - ack = reply[0];
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK) {
> + ack = reply[0] >> 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
> memcpy(recv, reply + 1, ret - 1);
> return ret - 1;
> }
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(100);
> else
> return -EIO;
> @@ -569,12 +570,12 @@ cdv_intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
>
> /* Set up the command byte */
> if (mode & MODE_I2C_READ)
> - msg[0] = AUX_I2C_READ << 4;
> + msg[0] = DP_AUX_I2C_READ << 4;
> else
> - msg[0] = AUX_I2C_WRITE << 4;
> + msg[0] = DP_AUX_I2C_WRITE << 4;
>
> if (!(mode & MODE_I2C_STOP))
> - msg[0] |= AUX_I2C_MOT << 4;
> + msg[0] |= DP_AUX_I2C_MOT << 4;
>
> msg[1] = address >> 8;
> msg[2] = address;
> @@ -606,16 +607,16 @@ cdv_intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> return ret;
> }
>
> - switch (reply[0] & AUX_NATIVE_REPLY_MASK) {
> - case AUX_NATIVE_REPLY_ACK:
> + switch ((reply[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
> + case DP_AUX_NATIVE_REPLY_ACK:
> /* I2C-over-AUX Reply field is only valid
> * when paired with AUX ACK.
> */
> break;
> - case AUX_NATIVE_REPLY_NACK:
> + case DP_AUX_NATIVE_REPLY_NACK:
> DRM_DEBUG_KMS("aux_ch native nack\n");
> return -EREMOTEIO;
> - case AUX_NATIVE_REPLY_DEFER:
> + case DP_AUX_NATIVE_REPLY_DEFER:
> udelay(100);
> continue;
> default:
> @@ -624,16 +625,16 @@ cdv_intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> return -EREMOTEIO;
> }
>
> - switch (reply[0] & AUX_I2C_REPLY_MASK) {
> - case AUX_I2C_REPLY_ACK:
> + switch ((reply[0] >> 4) & DP_AUX_I2C_REPLY_MASK) {
> + case DP_AUX_I2C_REPLY_ACK:
> if (mode == MODE_I2C_READ) {
> *read_byte = reply[1];
> }
> return reply_bytes - 1;
> - case AUX_I2C_REPLY_NACK:
> + case DP_AUX_I2C_REPLY_NACK:
> DRM_DEBUG_KMS("aux_i2c nack\n");
> return -EREMOTEIO;
> - case AUX_I2C_REPLY_DEFER:
> + case DP_AUX_I2C_REPLY_DEFER:
> DRM_DEBUG_KMS("aux_i2c defer\n");
> udelay(100);
> break;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9b40113f4fa1..7df5085973e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -542,7 +542,7 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
> return -E2BIG;
>
> intel_dp_check_edp(intel_dp);
> - msg[0] = AUX_NATIVE_WRITE << 4;
> + msg[0] = DP_AUX_NATIVE_WRITE << 4;
> msg[1] = address >> 8;
> msg[2] = address & 0xff;
> msg[3] = send_bytes - 1;
> @@ -552,9 +552,10 @@ intel_dp_aux_native_write(struct intel_dp *intel_dp,
> ret = intel_dp_aux_ch(intel_dp, msg, msg_bytes, &ack, 1);
> if (ret < 0)
> return ret;
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> + ack >>= 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
> break;
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(100);
> else
> return -EIO;
> @@ -586,7 +587,7 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
> return -E2BIG;
>
> intel_dp_check_edp(intel_dp);
> - msg[0] = AUX_NATIVE_READ << 4;
> + msg[0] = DP_AUX_NATIVE_READ << 4;
> msg[1] = address >> 8;
> msg[2] = address & 0xff;
> msg[3] = recv_bytes - 1;
> @@ -601,12 +602,12 @@ intel_dp_aux_native_read(struct intel_dp *intel_dp,
> return -EPROTO;
> if (ret < 0)
> return ret;
> - ack = reply[0];
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK) {
> + ack = reply[0] >> 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK) {
> memcpy(recv, reply + 1, ret - 1);
> return ret - 1;
> }
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(100);
> else
> return -EIO;
> @@ -633,12 +634,12 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> intel_dp_check_edp(intel_dp);
> /* Set up the command byte */
> if (mode & MODE_I2C_READ)
> - msg[0] = AUX_I2C_READ << 4;
> + msg[0] = DP_AUX_I2C_READ << 4;
> else
> - msg[0] = AUX_I2C_WRITE << 4;
> + msg[0] = DP_AUX_I2C_WRITE << 4;
>
> if (!(mode & MODE_I2C_STOP))
> - msg[0] |= AUX_I2C_MOT << 4;
> + msg[0] |= DP_AUX_I2C_MOT << 4;
>
> msg[1] = address >> 8;
> msg[2] = address;
> @@ -675,17 +676,17 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> goto out;
> }
>
> - switch (reply[0] & AUX_NATIVE_REPLY_MASK) {
> - case AUX_NATIVE_REPLY_ACK:
> + switch ((reply[0] >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
> + case DP_AUX_NATIVE_REPLY_ACK:
> /* I2C-over-AUX Reply field is only valid
> * when paired with AUX ACK.
> */
> break;
> - case AUX_NATIVE_REPLY_NACK:
> + case DP_AUX_NATIVE_REPLY_NACK:
> DRM_DEBUG_KMS("aux_ch native nack\n");
> ret = -EREMOTEIO;
> goto out;
> - case AUX_NATIVE_REPLY_DEFER:
> + case DP_AUX_NATIVE_REPLY_DEFER:
> /*
> * For now, just give more slack to branch devices. We
> * could check the DPCD for I2C bit rate capabilities,
> @@ -706,18 +707,18 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> goto out;
> }
>
> - switch (reply[0] & AUX_I2C_REPLY_MASK) {
> - case AUX_I2C_REPLY_ACK:
> + switch ((reply[0] >> 4) & DP_AUX_I2C_REPLY_MASK) {
> + case DP_AUX_I2C_REPLY_ACK:
> if (mode == MODE_I2C_READ) {
> *read_byte = reply[1];
> }
> ret = reply_bytes - 1;
> goto out;
> - case AUX_I2C_REPLY_NACK:
> + case DP_AUX_I2C_REPLY_NACK:
> DRM_DEBUG_KMS("aux_i2c nack\n");
> ret = -EREMOTEIO;
> goto out;
> - case AUX_I2C_REPLY_DEFER:
> + case DP_AUX_I2C_REPLY_DEFER:
> DRM_DEBUG_KMS("aux_i2c defer\n");
> udelay(100);
> break;
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c
> index fb3ae07a1469..37289f67f965 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -157,7 +157,7 @@ static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
>
> msg[0] = address;
> msg[1] = address >> 8;
> - msg[2] = AUX_NATIVE_WRITE << 4;
> + msg[2] = DP_AUX_NATIVE_WRITE << 4;
> msg[3] = (msg_bytes << 4) | (send_bytes - 1);
> memcpy(&msg[4], send, send_bytes);
>
> @@ -168,9 +168,10 @@ static int radeon_dp_aux_native_write(struct radeon_connector *radeon_connector,
> continue;
> else if (ret < 0)
> return ret;
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> + ack >>= 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
> return send_bytes;
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(400);
> else
> return -EIO;
> @@ -191,7 +192,7 @@ static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
>
> msg[0] = address;
> msg[1] = address >> 8;
> - msg[2] = AUX_NATIVE_READ << 4;
> + msg[2] = DP_AUX_NATIVE_READ << 4;
> msg[3] = (msg_bytes << 4) | (recv_bytes - 1);
>
> for (retry = 0; retry < 4; retry++) {
> @@ -201,9 +202,10 @@ static int radeon_dp_aux_native_read(struct radeon_connector *radeon_connector,
> continue;
> else if (ret < 0)
> return ret;
> - if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_ACK)
> + ack >>= 4;
> + if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_ACK)
> return ret;
> - else if ((ack & AUX_NATIVE_REPLY_MASK) == AUX_NATIVE_REPLY_DEFER)
> + else if ((ack & DP_AUX_NATIVE_REPLY_MASK) == DP_AUX_NATIVE_REPLY_DEFER)
> udelay(400);
> else if (ret == 0)
> return -EPROTO;
> @@ -246,12 +248,12 @@ int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
>
> /* Set up the command byte */
> if (mode & MODE_I2C_READ)
> - msg[2] = AUX_I2C_READ << 4;
> + msg[2] = DP_AUX_I2C_READ << 4;
> else
> - msg[2] = AUX_I2C_WRITE << 4;
> + msg[2] = DP_AUX_I2C_WRITE << 4;
>
> if (!(mode & MODE_I2C_STOP))
> - msg[2] |= AUX_I2C_MOT << 4;
> + msg[2] |= DP_AUX_I2C_MOT << 4;
>
> msg[0] = address;
> msg[1] = address >> 8;
> @@ -282,16 +284,16 @@ int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> return ret;
> }
>
> - switch (ack & AUX_NATIVE_REPLY_MASK) {
> - case AUX_NATIVE_REPLY_ACK:
> + switch ((ack >> 4) & DP_AUX_NATIVE_REPLY_MASK) {
> + case DP_AUX_NATIVE_REPLY_ACK:
> /* I2C-over-AUX Reply field is only valid
> * when paired with AUX ACK.
> */
> break;
> - case AUX_NATIVE_REPLY_NACK:
> + case DP_AUX_NATIVE_REPLY_NACK:
> DRM_DEBUG_KMS("aux_ch native nack\n");
> return -EREMOTEIO;
> - case AUX_NATIVE_REPLY_DEFER:
> + case DP_AUX_NATIVE_REPLY_DEFER:
> DRM_DEBUG_KMS("aux_ch native defer\n");
> udelay(400);
> continue;
> @@ -300,15 +302,15 @@ int radeon_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
> return -EREMOTEIO;
> }
>
> - switch (ack & AUX_I2C_REPLY_MASK) {
> - case AUX_I2C_REPLY_ACK:
> + switch ((ack >> 4) & DP_AUX_I2C_REPLY_MASK) {
> + case DP_AUX_I2C_REPLY_ACK:
> if (mode == MODE_I2C_READ)
> *read_byte = reply[0];
> return ret;
> - case AUX_I2C_REPLY_NACK:
> + case DP_AUX_I2C_REPLY_NACK:
> DRM_DEBUG_KMS("aux_i2c nack\n");
> return -EREMOTEIO;
> - case AUX_I2C_REPLY_DEFER:
> + case DP_AUX_I2C_REPLY_DEFER:
> DRM_DEBUG_KMS("aux_i2c defer\n");
> udelay(400);
> break;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 8b0f6c44251e..b5bf8de7afaa 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -41,22 +41,22 @@
> * 1.2 formally includes both eDP and DPI definitions.
> */
>
> -#define AUX_NATIVE_WRITE 0x8
> -#define AUX_NATIVE_READ 0x9
> -#define AUX_I2C_WRITE 0x0
> -#define AUX_I2C_READ 0x1
> -#define AUX_I2C_STATUS 0x2
> -#define AUX_I2C_MOT 0x4
> -
> -#define AUX_NATIVE_REPLY_ACK (0x0 << 4)
> -#define AUX_NATIVE_REPLY_NACK (0x1 << 4)
> -#define AUX_NATIVE_REPLY_DEFER (0x2 << 4)
> -#define AUX_NATIVE_REPLY_MASK (0x3 << 4)
> -
> -#define AUX_I2C_REPLY_ACK (0x0 << 6)
> -#define AUX_I2C_REPLY_NACK (0x1 << 6)
> -#define AUX_I2C_REPLY_DEFER (0x2 << 6)
> -#define AUX_I2C_REPLY_MASK (0x3 << 6)
> +#define DP_AUX_I2C_WRITE 0x0
> +#define DP_AUX_I2C_READ 0x1
> +#define DP_AUX_I2C_STATUS 0x2
> +#define DP_AUX_I2C_MOT 0x4
> +#define DP_AUX_NATIVE_WRITE 0x8
> +#define DP_AUX_NATIVE_READ 0x9
> +
> +#define DP_AUX_NATIVE_REPLY_ACK (0x0 << 0)
> +#define DP_AUX_NATIVE_REPLY_NACK (0x1 << 0)
> +#define DP_AUX_NATIVE_REPLY_DEFER (0x2 << 0)
> +#define DP_AUX_NATIVE_REPLY_MASK (0x3 << 0)
> +
> +#define DP_AUX_I2C_REPLY_ACK (0x0 << 2)
> +#define DP_AUX_I2C_REPLY_NACK (0x1 << 2)
> +#define DP_AUX_I2C_REPLY_DEFER (0x2 << 2)
> +#define DP_AUX_I2C_REPLY_MASK (0x3 << 2)
>
> /* AUX CH addresses */
> /* DPCD */
> --
> 1.8.4.2
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
