| Message ID | 1396622458-2092-3-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 04, 2014 at 10:40:57AM -0400, Alex Deucher wrote: > We need bare address packets at the start and end of > each i2c over aux transaction to properly reset the connection > between transactions. This mirrors what the existing dp i2c > over aux algo currently does. > > This fixes EDID fetches on certain monitors especially with > dp bridges. > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/drm_dp_helper.c | 53 +++++++++++++++++++++++------------------ > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index f4babed..f0c2850 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -664,12 +664,26 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > int num) > { > struct drm_dp_aux *aux = adapter->algo_data; > - unsigned int i, j; > + unsigned int m, b; > + struct drm_dp_aux_msg msg; > + int err = 0; > + u8 buf = 0; > > - for (i = 0; i < num; i++) { > - struct drm_dp_aux_msg msg; > - int err; > + memset(&msg, 0, sizeof(msg)); > > + for (m = 0; m < num; m++) { > + msg.address = msgs[m].addr; > + msg.request = (msgs[m].flags & I2C_M_RD) ? > + DP_AUX_I2C_READ : > + DP_AUX_I2C_WRITE; > + msg.request |= DP_AUX_I2C_MOT; > + msg.buffer = &buf; Maybe just pass NULL and let buggy implementations explode? > + msg.size = 0; > + err = drm_dp_i2c_do_msg(aux, &msg); > + if (err < 0) { > + printk("error %d in bare address write\n", err); > + break; > + } > /* > * Many hardware implementations support FIFOs larger than a > * single byte, but it has been empirically determined that > @@ -677,31 +691,24 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > * decreased performance. Therefore each message is simply > * transferred byte-by-byte. > */ > - for (j = 0; j < msgs[i].len; j++) { > - memset(&msg, 0, sizeof(msg)); > - msg.address = msgs[i].addr; > - > - msg.request = (msgs[i].flags & I2C_M_RD) ? > - DP_AUX_I2C_READ : > - DP_AUX_I2C_WRITE; > - > - /* > - * All messages except the last one are middle-of- > - * transfer messages. > - */ > - if ((i < num - 1) || (j < msgs[i].len - 1)) > - msg.request |= DP_AUX_I2C_MOT; > - > - msg.buffer = msgs[i].buf + j; > + for (b = 0; b < msgs[m].len; b++) { > + msg.buffer = msgs[m].buf + b; > msg.size = 1; > > err = drm_dp_i2c_do_msg(aux, &msg); > if (err < 0) > - return err; > + break; This will abort the current message, but it'll keep going and try to tranfer the next message. We need to abort the entire thing and proceed to generate the i2c STOP. Also you're now reusing the msg and expecting .transfer() to not clobber any of the fields (apart from msg.reply). Hmm, actually the retry loop in drm_dp_i2c_do_msg() already requires such a contract between the caller and the callee. As we can't make the msg const due to msg.reply, it would be nice if the documentation mentioned this somewhat important detail. > } > } > - > - return num; > + if (err >= 0) > + err = num; > + /* send a bare address packet to close out the connection */ > + msg.request &= ~DP_AUX_I2C_MOT; > + msg.buffer = &buf; Another chance to make buggy drivers explode ;) > + msg.size = 0; > + (void)drm_dp_i2c_do_msg(aux, &msg); > + > + return err; > } > > static const struct i2c_algorithm drm_dp_i2c_algo = { > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Apr 04, 2014 at 10:40:57AM -0400, Alex Deucher wrote: > We need bare address packets at the start and end of > each i2c over aux transaction to properly reset the connection > between transactions. This mirrors what the existing dp i2c > over aux algo currently does. > > This fixes EDID fetches on certain monitors especially with > dp bridges. > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> The i915 dp aux code will work the same way still, but curiously the hw spec doesn't mention anything for this really. I didn't check tegra. -Daniel > --- > drivers/gpu/drm/drm_dp_helper.c | 53 +++++++++++++++++++++++------------------ > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index f4babed..f0c2850 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -664,12 +664,26 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > int num) > { > struct drm_dp_aux *aux = adapter->algo_data; > - unsigned int i, j; > + unsigned int m, b; > + struct drm_dp_aux_msg msg; > + int err = 0; > + u8 buf = 0; > > - for (i = 0; i < num; i++) { > - struct drm_dp_aux_msg msg; > - int err; > + memset(&msg, 0, sizeof(msg)); > > + for (m = 0; m < num; m++) { > + msg.address = msgs[m].addr; > + msg.request = (msgs[m].flags & I2C_M_RD) ? > + DP_AUX_I2C_READ : > + DP_AUX_I2C_WRITE; > + msg.request |= DP_AUX_I2C_MOT; > + msg.buffer = &buf; > + msg.size = 0; > + err = drm_dp_i2c_do_msg(aux, &msg); > + if (err < 0) { > + printk("error %d in bare address write\n", err); > + break; > + } > /* > * Many hardware implementations support FIFOs larger than a > * single byte, but it has been empirically determined that > @@ -677,31 +691,24 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, > * decreased performance. Therefore each message is simply > * transferred byte-by-byte. > */ > - for (j = 0; j < msgs[i].len; j++) { > - memset(&msg, 0, sizeof(msg)); > - msg.address = msgs[i].addr; > - > - msg.request = (msgs[i].flags & I2C_M_RD) ? > - DP_AUX_I2C_READ : > - DP_AUX_I2C_WRITE; > - > - /* > - * All messages except the last one are middle-of- > - * transfer messages. > - */ > - if ((i < num - 1) || (j < msgs[i].len - 1)) > - msg.request |= DP_AUX_I2C_MOT; > - > - msg.buffer = msgs[i].buf + j; > + for (b = 0; b < msgs[m].len; b++) { > + msg.buffer = msgs[m].buf + b; > msg.size = 1; > > err = drm_dp_i2c_do_msg(aux, &msg); > if (err < 0) > - return err; > + break; > } > } > - > - return num; > + if (err >= 0) > + err = num; > + /* send a bare address packet to close out the connection */ > + msg.request &= ~DP_AUX_I2C_MOT; > + msg.buffer = &buf; > + msg.size = 0; > + (void)drm_dp_i2c_do_msg(aux, &msg); > + > + return err; > } > > static const struct i2c_algorithm drm_dp_i2c_algo = { > -- > 1.8.3.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index f4babed..f0c2850 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -664,12 +664,26 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) { struct drm_dp_aux *aux = adapter->algo_data; - unsigned int i, j; + unsigned int m, b; + struct drm_dp_aux_msg msg; + int err = 0; + u8 buf = 0; - for (i = 0; i < num; i++) { - struct drm_dp_aux_msg msg; - int err; + memset(&msg, 0, sizeof(msg)); + for (m = 0; m < num; m++) { + msg.address = msgs[m].addr; + msg.request = (msgs[m].flags & I2C_M_RD) ? + DP_AUX_I2C_READ : + DP_AUX_I2C_WRITE; + msg.request |= DP_AUX_I2C_MOT; + msg.buffer = &buf; + msg.size = 0; + err = drm_dp_i2c_do_msg(aux, &msg); + if (err < 0) { + printk("error %d in bare address write\n", err); + break; + } /* * Many hardware implementations support FIFOs larger than a * single byte, but it has been empirically determined that @@ -677,31 +691,24 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, * decreased performance. Therefore each message is simply * transferred byte-by-byte. */ - for (j = 0; j < msgs[i].len; j++) { - memset(&msg, 0, sizeof(msg)); - msg.address = msgs[i].addr; - - msg.request = (msgs[i].flags & I2C_M_RD) ? - DP_AUX_I2C_READ : - DP_AUX_I2C_WRITE; - - /* - * All messages except the last one are middle-of- - * transfer messages. - */ - if ((i < num - 1) || (j < msgs[i].len - 1)) - msg.request |= DP_AUX_I2C_MOT; - - msg.buffer = msgs[i].buf + j; + for (b = 0; b < msgs[m].len; b++) { + msg.buffer = msgs[m].buf + b; msg.size = 1; err = drm_dp_i2c_do_msg(aux, &msg); if (err < 0) - return err; + break; } } - - return num; + if (err >= 0) + err = num; + /* send a bare address packet to close out the connection */ + msg.request &= ~DP_AUX_I2C_MOT; + msg.buffer = &buf; + msg.size = 0; + (void)drm_dp_i2c_do_msg(aux, &msg); + + return err; } static const struct i2c_algorithm drm_dp_i2c_algo = {
We need bare address packets at the start and end of each i2c over aux transaction to properly reset the connection between transactions. This mirrors what the existing dp i2c over aux algo currently does. This fixes EDID fetches on certain monitors especially with dp bridges. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/drm_dp_helper.c | 53 +++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 23 deletions(-)