| Message ID | 20201030011738.2028313-5-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: ti-sn65dsi86: Support EDID reading | expand |
Hi, On Thu, Oct 29, 2020 at 6:17 PM Stephen Boyd <swboyd@chromium.org> wrote: > > We should be setting the drm_dp_aux_msg::reply field if a NACK or a > SHORT reply happens. Update the error bit handling logic in > ti_sn_aux_transfer() to handle these cases and notify upper layers that > such errors have happened. This helps the retry logic understand that a > timeout has happened, or to shorten the read length if the panel isn't > able to handle the longest read possible. > > Note: I don't have any hardware that exhibits these code paths so this > is written based on reading the datasheet for this bridge and inspecting > the code and how this is called. > > Changes in v2: > - Handle WRITE_STATUS_UPDATE properly > > Cc: Douglas Anderson <dianders@chromium.org> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Jernej Skrabec <jernej.skrabec@siol.net> > Cc: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++--- > 1 file changed, 32 insertions(+), 4 deletions(-) This looks right to me, now. Hopefully if/when someone ends up with hardware that exercises these codepaths they'll at least be in a better state and maybe they will all just work! :-) Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index f86934fd6cc8..984ea41deca8 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -873,10 +873,16 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux, return -EINVAL; switch (request) { + case DP_AUX_I2C_WRITE_STATUS_UPDATE: + /* WRITE_STATUS_UPDATE only matters for request_val */ + request &= ~DP_AUX_I2C_WRITE_STATUS_UPDATE; + fallthrough; case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: case DP_AUX_NATIVE_READ: case DP_AUX_I2C_READ: + /* Assume it's good */ + msg->reply = 0; break; default: return -EINVAL; @@ -909,10 +915,32 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux, ret = regmap_read(pdata->regmap, SN_AUX_CMD_STATUS_REG, &val); if (ret) return ret; - else if ((val & AUX_IRQ_STATUS_NAT_I2C_FAIL) - || (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) - || (val & AUX_IRQ_STATUS_AUX_SHORT)) - return -ENXIO; + + if (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) { + /* + * The hardware tried the message seven times per the DP spec + * but it hit a timeout. We ignore defers here because they're + * handled in hardware. + */ + return -ETIMEDOUT; + } + if (val & AUX_IRQ_STATUS_AUX_SHORT) { + ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, &len); + if (ret) + return ret; + } else if (val & AUX_IRQ_STATUS_NAT_I2C_FAIL) { + switch (request) { + case DP_AUX_I2C_WRITE: + case DP_AUX_I2C_READ: + msg->reply |= DP_AUX_I2C_REPLY_NACK; + break; + case DP_AUX_NATIVE_READ: + case DP_AUX_NATIVE_WRITE: + msg->reply |= DP_AUX_NATIVE_REPLY_NACK; + break; + } + return 0; + } if (request == DP_AUX_NATIVE_WRITE || request == DP_AUX_I2C_WRITE || len == 0)
We should be setting the drm_dp_aux_msg::reply field if a NACK or a SHORT reply happens. Update the error bit handling logic in ti_sn_aux_transfer() to handle these cases and notify upper layers that such errors have happened. This helps the retry logic understand that a timeout has happened, or to shorten the read length if the panel isn't able to handle the longest read possible. Note: I don't have any hardware that exhibits these code paths so this is written based on reading the datasheet for this bridge and inspecting the code and how this is called. Changes in v2: - Handle WRITE_STATUS_UPDATE properly Cc: Douglas Anderson <dianders@chromium.org> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Jernej Skrabec <jernej.skrabec@siol.net> Cc: Sean Paul <seanpaul@chromium.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 36 ++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-)