Messages in this thread | | | Subject | Re: [PATCH 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures | From | Stephen Boyd <> | Date | Thu, 29 Oct 2020 13:24:18 -0700 |
| |
Quoting Doug Anderson (2020-10-29 09:22:55) > Hi, > > On Wed, Oct 28, 2020 at 6:12 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. > > I don't think you update the "reply" field for SHORT, right? You just > return a different size?
Correct.
> > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index 6b6e98ca2881..19737bc01b8f 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -909,10 +910,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; > > IIUC, your digging through the code showed that in order to fully > handle the "SHORT" case you also needed to add support for > "DP_AUX_I2C_WRITE_STATUS_UPDATE", right?
Oh yeah. If a short reply happens and it is aux over i2c then drm_dp_i2c_msg_write_status_update() is called and DP_AUX_I2C_WRITE_STATUS_UPDATE is set and then we try a transfer again. We need to handle that type of request in this ti_sn_aux_transfer() function.
> > Even without handling "DP_AUX_I2C_WRITE_STATUS_UPDATE" though, this > patch seems to be an improvement and I'd support landing it. > > Oh, I guess one other thing: I think this is all from code inspection, > right? You didn't manage to reproduce anything that would tickle one > of these code paths? Might be worth mentioning, even if "after the > cut"? >
Yes, just code inspection. I can add that detail to the commit text.
| |