| Message ID | 1525421338-1021-1-git-send-email-hl@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > >
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);