[PATCH] drm/dp: Use AUX constants from specification

Patrik Jakobsson patrik.r.jakobsson at gmail.com
Thu Dec 19 07:24:17 PST 2013


On Mon, Dec 16, 2013 at 5:01 PM, Thierry Reding
<thierry.reding at gmail.com> 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 at nvidia.com>

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 at gmail.com>

> ---
>  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
>


More information about the dri-devel mailing list