[1/4] drm/rockchip: add transfer function for cdn-dp
diff mbox

Message ID 1525421338-1021-1-git-send-email-hl@rock-chips.com
State New
Headers show

Commit Message

huang lin May 4, 2018, 8:08 a.m. UTC
From: Chris Zhong <zyw@rock-chips.com>

We may support training outside firmware, so we need support
dpcd read/write to get the message or do some setting with
display.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Lin Huang <hl@rock-chips.com>
---
 drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
 drivers/gpu/drm/rockchip/cdn-dp-core.h |  1 +
 drivers/gpu/drm/rockchip/cdn-dp-reg.c  | 66 +++++++++++++++++++++++++++++-----
 drivers/gpu/drm/rockchip/cdn-dp-reg.h  | 14 ++++++--
 4 files changed, 119 insertions(+), 17 deletions(-)

Comments

Enric Balletbo Serra May 7, 2018, 11:27 a.m. UTC | #1
Hi Lin,

I am interested in these patches, could you cc me on newer versions? Thanks.

Some comments below.

2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> From: Chris Zhong <zyw@rock-chips.com>
>
> We may support training outside firmware, so we need support
> dpcd read/write to get the message or do some setting with
> display.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
>  drivers/gpu/drm/rockchip/cdn-dp-core.h |  1 +
>  drivers/gpu/drm/rockchip/cdn-dp-reg.c  | 66 +++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/rockchip/cdn-dp-reg.h  | 14 ++++++--
>  4 files changed, 119 insertions(+), 17 deletions(-)
>

In general, for this patch and all the other patches in the series I
saw that checkpatch spits out some warnings, could you fix these and
ideally run checkpatch with the --strict --subjective option?

> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index c6fbdcd..268c190 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
>         u8 value;
>
>         *sink_count = 0;
> -       ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
> -       if (ret)
> +       ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
> +       if (ret < 0)
>                 return ret;
>
>         *sink_count = DP_GET_SINK_COUNT(value);
> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
>         if (!cdn_dp_check_sink_connection(dp))
>                 return -ENODEV;
>
> -       ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
> -                              DP_RECEIVER_CAP_SIZE);
> -       if (ret) {
> +       ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> +                              sizeof(dp->dpcd));
> +       if (ret < 0) {
>                 DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>                 return ret;
>         }
> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>         if (!port || !dp->link.rate || !dp->link.num_lanes)
>                 return false;
>
> -       if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
> -                            DP_LINK_STATUS_SIZE)) {
> +       if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> +           DP_LINK_STATUS_SIZE) {
>                 DRM_ERROR("Failed to get link status\n");
>                 return false;
>         }
> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
>         return NOTIFY_DONE;
>  }
>
> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
> +                                  struct drm_dp_aux_msg *msg)
> +{
> +       struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
> +       int ret;
> +       u8 status;
> +
> +       switch (msg->request & ~DP_AUX_I2C_MOT) {
> +       case DP_AUX_NATIVE_WRITE:
> +       case DP_AUX_I2C_WRITE:
> +       case DP_AUX_I2C_WRITE_STATUS_UPDATE:
> +               ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
> +                                       msg->size);
> +               break;
> +       case DP_AUX_NATIVE_READ:
> +       case DP_AUX_I2C_READ:
> +               ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
> +                                      msg->size);
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       status = cdn_dp_get_aux_status(dp);
> +       if (status == AUX_STAUS_ACK)
> +               msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> +       else if (status == AUX_STAUS_NACK)
> +               msg->reply = DP_AUX_NATIVE_REPLY_NACK;
> +       else if (status == AUX_STAUS_DEFER)
> +               msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> +

I think that you would mean STATUS instead of STAUS on these defines.

What happens if the status is AUX_STATUS_SINK_ERROR or AUX_STATUS_BUS_ERROR?

> +       return ret;
> +}
> +
>  static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>  {
>         struct cdn_dp_device *dp = dev_get_drvdata(dev);
> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>         dp->active = false;
>         dp->active_port = -1;
>         dp->fw_loaded = false;
> +       dp->aux.name = "DP-AUX";
> +       dp->aux.transfer = cdn_dp_aux_transfer;
> +       dp->aux.dev = dev;
> +
> +       ret = drm_dp_aux_register(&dp->aux);
> +       if (ret)
> +               return ret;
>
>         INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index f57e296..46159b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -78,6 +78,7 @@ struct cdn_dp_device {
>         struct platform_device *audio_pdev;
>         struct work_struct event_work;
>         struct edid *edid;
> +       struct drm_dp_aux aux;
>
>         struct mutex lock;
>         bool connected;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index eb3042c..b2f532a 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
>                                    sizeof(field), field);
>  }
>
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> + */

Returns the number of bytes or -ETIMEDOUT, no other negative errors
can be returned, right?

I am not English native but the last phrase sounds incorrect to me,
I'd rephrase it: (open to suggestions)

 -ETIMEDOUT if fails to receive the mailbox message.

> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>  {
>         u8 msg[5], reg[5];
>         int ret;
> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>                 goto err_dpcd_read;
>
>         ret = cdn_dp_mailbox_read_receive(dp, data, len);
> +       if (!ret)
> +               return len;
>
>  err_dpcd_read:
> +       DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
>         return ret;
>  }
>
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> +#define CDN_AUX_HEADER_SIZE    5
> +#define CDN_AUX_MSG_SIZE       20
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;

Same as above. Sounds incorrect to me.

> + * -EINVAL is return if get the wrong data size after message send.

Same here.

> + */
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>  {
> -       u8 msg[6], reg[5];
> +       u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
> +       u8 reg[CDN_AUX_HEADER_SIZE];
>         int ret;
>
> -       msg[0] = 0;
> -       msg[1] = 1;
> +       if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
> +               return -EINVAL;
> +
> +       msg[0] = (len >> 8) & 0xff;
> +       msg[1] = len & 0xff;
>         msg[2] = (addr >> 16) & 0xff;
>         msg[3] = (addr >> 8) & 0xff;
>         msg[4] = addr & 0xff;
> -       msg[5] = value;
> +
> +       memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
> +
>         ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
> -                                 sizeof(msg), msg);
> +                                 CDN_AUX_HEADER_SIZE + len, msg);
>         if (ret)
>                 goto err_dpcd_write;
>
> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>         if (ret)
>                 goto err_dpcd_write;
>
> -       if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
> +       if ((len != (reg[0] << 8 | reg[1])) ||
> +           (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
>                 ret = -EINVAL;
> +       } else {
> +               return len;
> +       }
>
>  err_dpcd_write:
>         if (ret)
> @@ -286,6 +310,32 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>         return ret;
>  }
>
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
> +{
> +       u8 status;
> +       int ret;
> +
> +       ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
> +                                 0, NULL);
> +       if (ret)
> +               goto err_get_hpd;
> +
> +       ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
> +                                             DPTX_GET_LAST_AUX_STAUS, sizeof(status));
> +       if (ret)
> +               goto err_get_hpd;
> +
> +       ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
> +       if (ret)
> +               goto err_get_hpd;
> +
> +       return status;
> +
> +err_get_hpd:
> +       DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
> +       return ret;
> +}
> +
>  int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
>                          u32 i_size, const u32 *d_mem, u32 d_size)
>  {
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index c4bbb4a83..aedf2dc 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -328,6 +328,13 @@
>  #define GENERAL_BUS_SETTINGS            0x03
>  #define GENERAL_TEST_ACCESS             0x04
>
> +/* AUX status*/
> +#define AUX_STAUS_ACK                  0
> +#define AUX_STAUS_NACK                 1
> +#define AUX_STAUS_DEFER                        2
> +#define AUX_STAUS_SINK_ERROR           3
> +#define AUX_STAUS_BUS_ERROR            4
> +

For the five defines, s/STAUS/STATUS/

>  #define DPTX_SET_POWER_MNG                     0x00
>  #define DPTX_SET_HOST_CAPABILITIES             0x01
>  #define DPTX_GET_EDID                          0x02
> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
>  int cdn_dp_event_config(struct cdn_dp_device *dp);
>  u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>  int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> +                         u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> +                        u8 *data, u16 len);
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
>  int cdn_dp_get_edid_block(void *dp, u8 *edid,
>                           unsigned int block, size_t length);
>  int cdn_dp_train_link(struct cdn_dp_device *dp);
> --
> 2.7.4
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
huang lin May 9, 2018, 2:59 a.m. UTC | #2
Hi Enric,


On Monday, May 07, 2018 07:27 PM, Enric Balletbo Serra wrote:
> Hi Lin,
>
> I am interested in these patches, could you cc me on newer versions? Thanks.
>
> Some comments below.
Sure, will cc to you next version.
>
> 2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
>> From: Chris Zhong <zyw@rock-chips.com>
>>
>> We may support training outside firmware, so we need support
>> dpcd read/write to get the message or do some setting with
>> display.
>>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
>>   drivers/gpu/drm/rockchip/cdn-dp-core.h |  1 +
>>   drivers/gpu/drm/rockchip/cdn-dp-reg.c  | 66 +++++++++++++++++++++++++++++-----
>>   drivers/gpu/drm/rockchip/cdn-dp-reg.h  | 14 ++++++--
>>   4 files changed, 119 insertions(+), 17 deletions(-)
>>
> In general, for this patch and all the other patches in the series I
> saw that checkpatch spits out some warnings, could you fix these and
> ideally run checkpatch with the --strict --subjective option?
Okay.
>
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> index c6fbdcd..268c190 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
>> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
>>          u8 value;
>>
>>          *sink_count = 0;
>> -       ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
>> -       if (ret)
>> +       ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
>> +       if (ret < 0)
>>                  return ret;
>>
>>          *sink_count = DP_GET_SINK_COUNT(value);
>> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
>>          if (!cdn_dp_check_sink_connection(dp))
>>                  return -ENODEV;
>>
>> -       ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
>> -                              DP_RECEIVER_CAP_SIZE);
>> -       if (ret) {
>> +       ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
>> +                              sizeof(dp->dpcd));
>> +       if (ret < 0) {
>>                  DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
>>                  return ret;
>>          }
>> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
>>          if (!port || !dp->link.rate || !dp->link.num_lanes)
>>                  return false;
>>
>> -       if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
>> -                            DP_LINK_STATUS_SIZE)) {
>> +       if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
>> +           DP_LINK_STATUS_SIZE) {
>>                  DRM_ERROR("Failed to get link status\n");
>>                  return false;
>>          }
>> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
>>          return NOTIFY_DONE;
>>   }
>>
>> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
>> +                                  struct drm_dp_aux_msg *msg)
>> +{
>> +       struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
>> +       int ret;
>> +       u8 status;
>> +
>> +       switch (msg->request & ~DP_AUX_I2C_MOT) {
>> +       case DP_AUX_NATIVE_WRITE:
>> +       case DP_AUX_I2C_WRITE:
>> +       case DP_AUX_I2C_WRITE_STATUS_UPDATE:
>> +               ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
>> +                                       msg->size);
>> +               break;
>> +       case DP_AUX_NATIVE_READ:
>> +       case DP_AUX_I2C_READ:
>> +               ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
>> +                                      msg->size);
>> +               break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +
>> +       status = cdn_dp_get_aux_status(dp);
>> +       if (status == AUX_STAUS_ACK)
>> +               msg->reply = DP_AUX_NATIVE_REPLY_ACK;
>> +       else if (status == AUX_STAUS_NACK)
>> +               msg->reply = DP_AUX_NATIVE_REPLY_NACK;
>> +       else if (status == AUX_STAUS_DEFER)
>> +               msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
>> +
> I think that you would mean STATUS instead of STAUS on these defines.
>
> What happens if the status is AUX_STATUS_SINK_ERROR or AUX_STATUS_BUS_ERROR?
drm_dp_i2c_do_msg() will mark it as invalid i2c relay and return error.
>
>> +       return ret;
>> +}
>> +
>>   static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>   {
>>          struct cdn_dp_device *dp = dev_get_drvdata(dev);
>> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
>>          dp->active = false;
>>          dp->active_port = -1;
>>          dp->fw_loaded = false;
>> +       dp->aux.name = "DP-AUX";
>> +       dp->aux.transfer = cdn_dp_aux_transfer;
>> +       dp->aux.dev = dev;
>> +
>> +       ret = drm_dp_aux_register(&dp->aux);
>> +       if (ret)
>> +               return ret;
>>
>>          INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>>
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> index f57e296..46159b2 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
>> @@ -78,6 +78,7 @@ struct cdn_dp_device {
>>          struct platform_device *audio_pdev;
>>          struct work_struct event_work;
>>          struct edid *edid;
>> +       struct drm_dp_aux aux;
>>
>>          struct mutex lock;
>>          bool connected;
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> index eb3042c..b2f532a 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
>> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
>>                                     sizeof(field), field);
>>   }
>>
>> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>> +/*
>> + * Returns the number of bytes transferred on success, or a negative error
>> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
>> + */
> Returns the number of bytes or -ETIMEDOUT, no other negative errors
> can be returned, right?
>
> I am not English native but the last phrase sounds incorrect to me,
> I'd rephrase it: (open to suggestions)
>
>   -ETIMEDOUT if fails to receive the mailbox message.
>
>> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>>   {
>>          u8 msg[5], reg[5];
>>          int ret;
>> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>>                  goto err_dpcd_read;
>>
>>          ret = cdn_dp_mailbox_read_receive(dp, data, len);
>> +       if (!ret)
>> +               return len;
>>
>>   err_dpcd_read:
>> +       DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
>>          return ret;
>>   }
>>
>> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>> +#define CDN_AUX_HEADER_SIZE    5
>> +#define CDN_AUX_MSG_SIZE       20
>> +/*
>> + * Returns the number of bytes transferred on success, or a negative error
>> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> Same as above. Sounds incorrect to me.
>
>> + * -EINVAL is return if get the wrong data size after message send.
> Same here.
>
>> + */
>> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
>>   {
>> -       u8 msg[6], reg[5];
>> +       u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
>> +       u8 reg[CDN_AUX_HEADER_SIZE];
>>          int ret;
>>
>> -       msg[0] = 0;
>> -       msg[1] = 1;
>> +       if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
>> +               return -EINVAL;
>> +
>> +       msg[0] = (len >> 8) & 0xff;
>> +       msg[1] = len & 0xff;
>>          msg[2] = (addr >> 16) & 0xff;
>>          msg[3] = (addr >> 8) & 0xff;
>>          msg[4] = addr & 0xff;
>> -       msg[5] = value;
>> +
>> +       memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
>> +
>>          ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
>> -                                 sizeof(msg), msg);
>> +                                 CDN_AUX_HEADER_SIZE + len, msg);
>>          if (ret)
>>                  goto err_dpcd_write;
>>
>> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>>          if (ret)
>>                  goto err_dpcd_write;
>>
>> -       if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
>> +       if ((len != (reg[0] << 8 | reg[1])) ||
>> +           (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
>>                  ret = -EINVAL;
>> +       } else {
>> +               return len;
>> +       }
>>
>>   err_dpcd_write:
>>          if (ret)
>> @@ -286,6 +310,32 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
>>          return ret;
>>   }
>>
>> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
>> +{
>> +       u8 status;
>> +       int ret;
>> +
>> +       ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
>> +                                 0, NULL);
>> +       if (ret)
>> +               goto err_get_hpd;
>> +
>> +       ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
>> +                                             DPTX_GET_LAST_AUX_STAUS, sizeof(status));
>> +       if (ret)
>> +               goto err_get_hpd;
>> +
>> +       ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
>> +       if (ret)
>> +               goto err_get_hpd;
>> +
>> +       return status;
>> +
>> +err_get_hpd:
>> +       DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
>> +       return ret;
>> +}
>> +
>>   int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
>>                           u32 i_size, const u32 *d_mem, u32 d_size)
>>   {
>> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> index c4bbb4a83..aedf2dc 100644
>> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
>> @@ -328,6 +328,13 @@
>>   #define GENERAL_BUS_SETTINGS            0x03
>>   #define GENERAL_TEST_ACCESS             0x04
>>
>> +/* AUX status*/
>> +#define AUX_STAUS_ACK                  0
>> +#define AUX_STAUS_NACK                 1
>> +#define AUX_STAUS_DEFER                        2
>> +#define AUX_STAUS_SINK_ERROR           3
>> +#define AUX_STAUS_BUS_ERROR            4
>> +
> For the five defines, s/STAUS/STATUS/
will fix it.
>
>>   #define DPTX_SET_POWER_MNG                     0x00
>>   #define DPTX_SET_HOST_CAPABILITIES             0x01
>>   #define DPTX_GET_EDID                          0x02
>> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
>>   int cdn_dp_event_config(struct cdn_dp_device *dp);
>>   u32 cdn_dp_get_event(struct cdn_dp_device *dp);
>>   int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
>> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
>> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
>> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
>> +                         u8 *data, u16 len);
>> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
>> +                        u8 *data, u16 len);
>> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
>>   int cdn_dp_get_edid_block(void *dp, u8 *edid,
>>                            unsigned int block, size_t length);
>>   int cdn_dp_train_link(struct cdn_dp_device *dp);
>> --
>> 2.7.4
>>
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index c6fbdcd..268c190 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -176,8 +176,8 @@  static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
 	u8 value;
 
 	*sink_count = 0;
-	ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
-	if (ret)
+	ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
+	if (ret < 0)
 		return ret;
 
 	*sink_count = DP_GET_SINK_COUNT(value);
@@ -374,9 +374,9 @@  static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
 	if (!cdn_dp_check_sink_connection(dp))
 		return -ENODEV;
 
-	ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
-			       DP_RECEIVER_CAP_SIZE);
-	if (ret) {
+	ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
+			       sizeof(dp->dpcd));
+	if (ret < 0) {
 		DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
 		return ret;
 	}
@@ -582,8 +582,8 @@  static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
 	if (!port || !dp->link.rate || !dp->link.num_lanes)
 		return false;
 
-	if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
-			     DP_LINK_STATUS_SIZE)) {
+	if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
+	    DP_LINK_STATUS_SIZE) {
 		DRM_ERROR("Failed to get link status\n");
 		return false;
 	}
@@ -1012,6 +1012,40 @@  static int cdn_dp_pd_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
+				   struct drm_dp_aux_msg *msg)
+{
+	struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
+	int ret;
+	u8 status;
+
+	switch (msg->request & ~DP_AUX_I2C_MOT) {
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+	case DP_AUX_I2C_WRITE_STATUS_UPDATE:
+		ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
+					msg->size);
+		break;
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
+				       msg->size);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	status = cdn_dp_get_aux_status(dp);
+	if (status == AUX_STAUS_ACK)
+		msg->reply = DP_AUX_NATIVE_REPLY_ACK;
+	else if (status == AUX_STAUS_NACK)
+		msg->reply = DP_AUX_NATIVE_REPLY_NACK;
+	else if (status == AUX_STAUS_DEFER)
+		msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
+
+	return ret;
+}
+
 static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 {
 	struct cdn_dp_device *dp = dev_get_drvdata(dev);
@@ -1030,6 +1064,13 @@  static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
 	dp->active = false;
 	dp->active_port = -1;
 	dp->fw_loaded = false;
+	dp->aux.name = "DP-AUX";
+	dp->aux.transfer = cdn_dp_aux_transfer;
+	dp->aux.dev = dev;
+
+	ret = drm_dp_aux_register(&dp->aux);
+	if (ret)
+		return ret;
 
 	INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
 
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
index f57e296..46159b2 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
@@ -78,6 +78,7 @@  struct cdn_dp_device {
 	struct platform_device *audio_pdev;
 	struct work_struct event_work;
 	struct edid *edid;
+	struct drm_dp_aux aux;
 
 	struct mutex lock;
 	bool connected;
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
index eb3042c..b2f532a 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
@@ -221,7 +221,11 @@  static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
 				   sizeof(field), field);
 }
 
-int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
+/*
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
+ */
+ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
 {
 	u8 msg[5], reg[5];
 	int ret;
@@ -247,24 +251,40 @@  int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
 		goto err_dpcd_read;
 
 	ret = cdn_dp_mailbox_read_receive(dp, data, len);
+	if (!ret)
+		return len;
 
 err_dpcd_read:
+	DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
 	return ret;
 }
 
-int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
+#define CDN_AUX_HEADER_SIZE	5
+#define CDN_AUX_MSG_SIZE	20
+/*
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
+ * -EINVAL is return if get the wrong data size after message send.
+ */
+ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
 {
-	u8 msg[6], reg[5];
+	u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
+	u8 reg[CDN_AUX_HEADER_SIZE];
 	int ret;
 
-	msg[0] = 0;
-	msg[1] = 1;
+	if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
+		return -EINVAL;
+
+	msg[0] = (len >> 8) & 0xff;
+	msg[1] = len & 0xff;
 	msg[2] = (addr >> 16) & 0xff;
 	msg[3] = (addr >> 8) & 0xff;
 	msg[4] = addr & 0xff;
-	msg[5] = value;
+
+	memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
+
 	ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
-				  sizeof(msg), msg);
+				  CDN_AUX_HEADER_SIZE + len, msg);
 	if (ret)
 		goto err_dpcd_write;
 
@@ -277,8 +297,12 @@  int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
 	if (ret)
 		goto err_dpcd_write;
 
-	if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
+	if ((len != (reg[0] << 8 | reg[1])) ||
+	    (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
 		ret = -EINVAL;
+	} else {
+		return len;
+	}
 
 err_dpcd_write:
 	if (ret)
@@ -286,6 +310,32 @@  int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
 	return ret;
 }
 
+int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
+{
+	u8 status;
+	int ret;
+
+	ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
+				  0, NULL);
+	if (ret)
+		goto err_get_hpd;
+
+	ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
+					      DPTX_GET_LAST_AUX_STAUS, sizeof(status));
+	if (ret)
+		goto err_get_hpd;
+
+	ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
+	if (ret)
+		goto err_get_hpd;
+
+	return status;
+
+err_get_hpd:
+	DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
+	return ret;
+}
+
 int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
 			 u32 i_size, const u32 *d_mem, u32 d_size)
 {
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
index c4bbb4a83..aedf2dc 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
+++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
@@ -328,6 +328,13 @@ 
 #define GENERAL_BUS_SETTINGS            0x03
 #define GENERAL_TEST_ACCESS             0x04
 
+/* AUX status*/
+#define AUX_STAUS_ACK			0
+#define AUX_STAUS_NACK			1
+#define AUX_STAUS_DEFER			2
+#define AUX_STAUS_SINK_ERROR		3
+#define AUX_STAUS_BUS_ERROR		4
+
 #define DPTX_SET_POWER_MNG			0x00
 #define DPTX_SET_HOST_CAPABILITIES		0x01
 #define DPTX_GET_EDID				0x02
@@ -469,8 +476,11 @@  int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
 int cdn_dp_event_config(struct cdn_dp_device *dp);
 u32 cdn_dp_get_event(struct cdn_dp_device *dp);
 int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
-int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
-int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
+ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
+			  u8 *data, u16 len);
+ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
+			 u8 *data, u16 len);
+int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
 int cdn_dp_get_edid_block(void *dp, u8 *edid,
 			  unsigned int block, size_t length);
 int cdn_dp_train_link(struct cdn_dp_device *dp);