Quoting Doug Anderson (2020-10-29 09:22:55) > Hi, > > On Wed, Oct 28, 2020 at 6:12 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> 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. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel