From efbcec2fb053d1b2b7a7a7f92db2fea71dc0cfcb Mon Sep 17 00:00:00 2001 From: Rajkumar Subbiah Date: Thu, 6 Apr 2023 13:27:36 -0700 Subject: [PATCH] disp: msm: dp: fix aux error handling If an aux transaction fails at its lowest level, there is a builtin retry mechanism before erroring out. Currently an error message is printed after each failed attempt even though the aux transaction might succeed on retry. This change switches the alert level to warning on these attempts and makes sure an error message is printed if the transfer errors out after retries. Change-Id: I47fb27fe0aa15eb5e2400c4338f9b9c59439983f Signed-off-by: Rajkumar Subbiah --- msm/dp/dp_aux.c | 22 +++++++++++++++---- msm/dp/dp_debug.h | 4 ++++ msm/dp/dp_link.c | 56 ++++++++++++++++++++++++++++------------------- msm/dp/dp_panel.c | 18 ++++++++++----- 4 files changed, 68 insertions(+), 32 deletions(-) diff --git a/msm/dp/dp_aux.c b/msm/dp/dp_aux.c index a5d900d4ba..de21279687 100644 --- a/msm/dp/dp_aux.c +++ b/msm/dp/dp_aux.c @@ -35,6 +35,14 @@ DP_WARN_V(fmt, ##__VA_ARGS__); \ } while (0) +#define DP_AUX_WARN_RATELIMITED(dp_aux, fmt, ...) \ + do { \ + if (dp_aux) \ + ipc_log_string(dp_aux->ipc_log_context, "[w][%-4d]"fmt,\ + current->pid, ##__VA_ARGS__); \ + DP_WARN_RATELIMITED_V(fmt, ##__VA_ARGS__); \ + } while (0) + #define DP_AUX_ERR(dp_aux, fmt, ...) \ do { \ if (dp_aux) \ @@ -210,26 +218,32 @@ static int dp_aux_cmd_fifo_tx(struct dp_aux_private *aux, u32 ret = 0, len = 0, timeout; int const aux_timeout_ms = HZ/4; struct dp_aux *dp_aux = &aux->dp_aux; + char prefix[64]; + + snprintf(prefix, sizeof(prefix), "%s %s %4xh(%2zu): ", + (msg->request & DP_AUX_I2C_MOT) ? "I2C" : "NAT", + (msg->request & DP_AUX_I2C_READ) ? "RD" : "WR", + msg->address, msg->size); reinit_completion(&aux->comp); len = dp_aux_write(aux, msg); if (len == 0) { - DP_AUX_ERR(dp_aux, "DP AUX write failed\n"); + DP_AUX_ERR(dp_aux, "DP AUX write failed: %s\n", prefix); return -EINVAL; } timeout = wait_for_completion_timeout(&aux->comp, aux_timeout_ms); if (!timeout) { - DP_AUX_ERR(dp_aux, "aux %s timeout\n", (aux->read ? "read" : "write")); + DP_AUX_WARN_RATELIMITED(dp_aux, "aux timeout during [%s]\n", prefix); return -ETIMEDOUT; } if (aux->aux_error_num == DP_AUX_ERR_NONE) { ret = len; } else { - DP_AUX_ERR_RATELIMITED(dp_aux, "aux err: %s\n", - dp_aux_get_error(aux->aux_error_num)); + DP_AUX_WARN_RATELIMITED(dp_aux, "aux err [%s] during [%s]\n", + dp_aux_get_error(aux->aux_error_num), prefix); ret = -EINVAL; } diff --git a/msm/dp/dp_debug.h b/msm/dp/dp_debug.h index 32dc7026ad..1a5816f588 100644 --- a/msm/dp/dp_debug.h +++ b/msm/dp/dp_debug.h @@ -69,6 +69,10 @@ pr_warn("[drm:%s][msm-dp-warn][%-4d]"fmt, __func__, \ current->pid, ##__VA_ARGS__) +#define DP_WARN_RATELIMITED_V(fmt, ...) \ + pr_warn_ratelimited("[drm:%s][msm-dp-warn][%-4d]"fmt, __func__, \ + current->pid, ##__VA_ARGS__) + #define DP_ERR_V(fmt, ...) \ pr_err("[drm:%s][msm-dp-err][%-4d]"fmt, __func__, \ current->pid, ##__VA_ARGS__) diff --git a/msm/dp/dp_link.c b/msm/dp/dp_link.c index 426fe8f585..13cbc4e2e6 100644 --- a/msm/dp/dp_link.c +++ b/msm/dp/dp_link.c @@ -1512,13 +1512,15 @@ static u32 dp_link_get_test_bits_depth(struct dp_link *dp_link, u32 bpp) int dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link) { u8 values[3]; - int err; + int ret; memset(link, 0, sizeof(*link)); - err = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values)); - if (err < 0) - return err; + ret = drm_dp_dpcd_read(aux, DP_DPCD_REV, values, sizeof(values)); + if (ret < 3) { + DP_ERR("failed to probe link, ret:%d\n", ret); + ret = -EIO; + } link->revision = values[0]; link->rate = drm_dp_bw_code_to_link_rate(values[1]); @@ -1540,22 +1542,26 @@ int dp_link_probe(struct drm_dp_aux *aux, struct drm_dp_link *link) int dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link) { u8 value; - int err; + int ret; /* DP_SET_POWER register is only available on DPCD v1.1 and later */ if (link->revision < 0x11) return 0; - err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value); - if (err < 0) - return err; + ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value); + if (ret != 1) { + DP_ERR("failed to read sink power when powering up, ret:%d\n", ret); + return -EIO; + } value &= ~DP_SET_POWER_MASK; value |= DP_SET_POWER_D0; - err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value); - if (err < 0) - return err; + ret = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value); + if (ret != 1) { + DP_ERR("failed to power up[0x%x] sink, ret:%d\n", value, ret); + return -EIO; + } /* * According to the DP 1.1 specification, a "Sink Device must exit the @@ -1577,22 +1583,26 @@ int dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link) int dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link) { u8 value; - int err; + int ret; /* DP_SET_POWER register is only available on DPCD v1.1 and later */ if (link->revision < 0x11) return 0; - err = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value); - if (err < 0) - return err; + ret = drm_dp_dpcd_readb(aux, DP_SET_POWER, &value); + if (ret != 1) { + DP_ERR("failed to read sink power when powering down, ret:%d\n", ret); + return -EIO; + } value &= ~DP_SET_POWER_MASK; value |= DP_SET_POWER_D3; - err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value); - if (err < 0) - return err; + ret = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value); + if (ret != 1) { + DP_ERR("failed to power down[0x%x] sink, ret:%d\n", value, ret); + return -EIO; + } return 0; } @@ -1607,7 +1617,7 @@ int dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link) int dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link) { u8 values[2]; - int err; + int ret; values[0] = drm_dp_link_rate_to_bw_code(link->rate); values[1] = link->num_lanes; @@ -1615,9 +1625,11 @@ int dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link) if (link->capabilities & DP_LINK_CAP_ENHANCED_FRAMING) values[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN; - err = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values)); - if (err < 0) - return err; + ret = drm_dp_dpcd_write(aux, DP_LINK_BW_SET, values, sizeof(values)); + if (ret != 2) { + DP_ERR("failed to configure link, ret:%d\n", ret); + return -EIO; + } return 0; } diff --git a/msm/dp/dp_panel.c b/msm/dp/dp_panel.c index 1e491bec52..3046ab0839 100644 --- a/msm/dp/dp_panel.c +++ b/msm/dp/dp_panel.c @@ -3091,8 +3091,10 @@ int dp_panel_get_sink_crc(struct dp_panel *dp_panel, u16 *crc) * per component (RGB or CrYCb). */ rc = drm_dp_dpcd_read(drm_aux, DP_TEST_CRC_R_CR, crc_bytes, 6); - if (rc < 0) - return rc; + if (rc != 6) { + DP_ERR("failed to read sink CRC, ret:%d\n", rc); + return -EIO; + } rc = 0; crc[0] = crc_bytes[0] | crc_bytes[1] << 8; @@ -3115,12 +3117,16 @@ int dp_panel_sink_crc_enable(struct dp_panel *dp_panel, bool enable) if (dp_panel->link_info.capabilities & DP_LINK_CAP_CRC) { ret = drm_dp_dpcd_readb(drm_aux, DP_TEST_SINK, &buf); - if (ret < 0) - return ret; + if (ret != 1) { + DP_ERR("failed to read CRC cap, ret:%d\n", ret); + return -EIO; + } ret = drm_dp_dpcd_writeb(drm_aux, DP_TEST_SINK, buf | DP_TEST_SINK_START); - if (ret < 0) - return ret; + if (ret != 1) { + DP_ERR("failed to enable Sink CRC, ret:%d\n", ret); + return -EIO; + } drm_dp_dpcd_readb(drm_aux, DP_TEST_SINK, &buf); }