[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250117-drm-rework-dpcd-access-v1-1-7fc020e04dbc@linaro.org>
Date: Fri, 17 Jan 2025 10:56:36 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>,
Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
Jani Nikula <jani.nikula@...ux.intel.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org
Subject: [PATCH RFC 1/7] drm/display: dp: change
drm_dp_dpcd_read_link_status() return
drm_dp_dpcd_read_link_status() follows the "return error code or number
of bytes read" protocol, with the code returning less bytes than
requested in case of some errors. However most of the drivers (except
the drm/msm one) interpreted that as "return error code in case of any
error". Move return len check to drm_dp_dpcd_read_link_status() and make
drm/msm/dp follow that protocol too.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
---
drivers/gpu/drm/display/drm_dp_helper.c | 16 +++++++++---
drivers/gpu/drm/msm/dp/dp_ctrl.c | 45 ++++++++++++++++++---------------
drivers/gpu/drm/msm/dp/dp_link.c | 17 ++++++-------
3 files changed, 44 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index da3c8521a7fa7d3c9761377363cdd4b44ab1106e..809c65dcb58983693fb335b88759a66919410114 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -716,14 +716,22 @@ EXPORT_SYMBOL(drm_dp_dpcd_write);
* @aux: DisplayPort AUX channel
* @status: buffer to store the link status in (must be at least 6 bytes)
*
- * Returns the number of bytes transferred on success or a negative error
- * code on failure.
+ * Returns the zero on success or a negative error code on failure.
*/
int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE])
{
- return drm_dp_dpcd_read(aux, DP_LANE0_1_STATUS, status,
- DP_LINK_STATUS_SIZE);
+ int ret;
+
+ ret = drm_dp_dpcd_read(aux, DP_LANE0_1_STATUS, status,
+ DP_LINK_STATUS_SIZE);
+ if (ret < 0)
+ return ret;
+
+ if (ret < DP_LINK_STATUS_SIZE)
+ return -EPROTO;
+
+ return 0;
}
EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index bc2ca8133b790fc049e18ab3b37a629558664dd4..8e4fdc0eae7ce218bdcb1aa03bded2f2a61c4b92 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1100,20 +1100,6 @@ static bool msm_dp_ctrl_train_pattern_set(struct msm_dp_ctrl_private *ctrl,
return ret == 1;
}
-static int msm_dp_ctrl_read_link_status(struct msm_dp_ctrl_private *ctrl,
- u8 *link_status)
-{
- int ret = 0, len;
-
- len = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
- if (len != DP_LINK_STATUS_SIZE) {
- DRM_ERROR("DP link status read failed, err: %d\n", len);
- ret = -EINVAL;
- }
-
- return ret;
-}
-
static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
int *training_step)
{
@@ -1140,9 +1126,11 @@ static int msm_dp_ctrl_link_train_1(struct msm_dp_ctrl_private *ctrl,
for (tries = 0; tries < maximum_retries; tries++) {
drm_dp_link_train_clock_recovery_delay(ctrl->aux, ctrl->panel->dpcd);
- ret = msm_dp_ctrl_read_link_status(ctrl, link_status);
- if (ret)
+ ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ if (ret < 0) {
+ DRM_ERROR("DP link status read failed, err: %d\n", ret);
return ret;
+ }
if (drm_dp_clock_recovery_ok(link_status,
ctrl->link->link_params.num_lanes)) {
@@ -1252,9 +1240,11 @@ static int msm_dp_ctrl_link_train_2(struct msm_dp_ctrl_private *ctrl,
for (tries = 0; tries <= maximum_retries; tries++) {
drm_dp_link_train_channel_eq_delay(ctrl->aux, ctrl->panel->dpcd);
- ret = msm_dp_ctrl_read_link_status(ctrl, link_status);
- if (ret)
+ ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ if (ret) {
+ DRM_ERROR("DP link status read failed, err: %d\n", ret);
return ret;
+ }
if (drm_dp_channel_eq_ok(link_status,
ctrl->link->link_params.num_lanes)) {
@@ -1804,8 +1794,13 @@ static bool msm_dp_ctrl_channel_eq_ok(struct msm_dp_ctrl_private *ctrl)
{
u8 link_status[DP_LINK_STATUS_SIZE];
int num_lanes = ctrl->link->link_params.num_lanes;
+ int ret;
- msm_dp_ctrl_read_link_status(ctrl, link_status);
+ ret = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ if (ret < 0) {
+ DRM_ERROR("DP link status read failed, err: %d\n", ret);
+ return false;
+ }
return drm_dp_channel_eq_ok(link_status, num_lanes);
}
@@ -1863,7 +1858,11 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
if (!msm_dp_catalog_link_is_connected(ctrl->catalog))
break;
- msm_dp_ctrl_read_link_status(ctrl, link_status);
+ rc = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ if (rc < 0) {
+ DRM_ERROR("DP link status read failed, err: %d\n", rc);
+ break;
+ }
rc = msm_dp_ctrl_link_rate_down_shift(ctrl);
if (rc < 0) { /* already in RBR = 1.6G */
@@ -1888,7 +1887,11 @@ int msm_dp_ctrl_on_link(struct msm_dp_ctrl *msm_dp_ctrl)
if (!msm_dp_catalog_link_is_connected(ctrl->catalog))
break;
- msm_dp_ctrl_read_link_status(ctrl, link_status);
+ rc = drm_dp_dpcd_read_link_status(ctrl->aux, link_status);
+ if (rc < 0) {
+ DRM_ERROR("DP link status read failed, err: %d\n", rc);
+ break;
+ }
if (!drm_dp_clock_recovery_ok(link_status,
ctrl->link->link_params.num_lanes))
diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c
index 1a1fbb2d7d4f2afcaace85d97b744d03017d37ce..431ee86a939343f9c7f2de51703f8f76f5580934 100644
--- a/drivers/gpu/drm/msm/dp/dp_link.c
+++ b/drivers/gpu/drm/msm/dp/dp_link.c
@@ -714,21 +714,20 @@ static int msm_dp_link_parse_request(struct msm_dp_link_private *link)
static int msm_dp_link_parse_sink_status_field(struct msm_dp_link_private *link)
{
- int len;
+ int ret;
link->prev_sink_count = link->msm_dp_link.sink_count;
- len = drm_dp_read_sink_count(link->aux);
- if (len < 0) {
+ ret = drm_dp_read_sink_count(link->aux);
+ if (ret < 0) {
DRM_ERROR("DP parse sink count failed\n");
- return len;
+ return ret;
}
- link->msm_dp_link.sink_count = len;
+ link->msm_dp_link.sink_count = ret;
- len = drm_dp_dpcd_read_link_status(link->aux,
- link->link_status);
- if (len < DP_LINK_STATUS_SIZE) {
+ ret = drm_dp_dpcd_read_link_status(link->aux, link->link_status);
+ if (ret < 0) {
DRM_ERROR("DP link status read failed\n");
- return len;
+ return ret;
}
return msm_dp_link_parse_request(link);
--
2.39.5
Powered by blists - more mailing lists