Bug 91451

I have the same problem on openSUSE, and I can confirm that the external monitor attached to a 2014 MacBook Air via an Apple miniDP->VGA adapter is not recognised anymore after updating to 4.1 yesterday. I have also tested this with 4.2-rc3, but the problem persists. The external monitor works as expected with 4.0.x kernels.

Also, could you please add Takashi Iwai to the CC list as per his post in the associated bug report for openSUSE here: https://bugzilla.opensuse.org/show_bug.cgi?id=939607#c2

I don't have his E-Mail address, but since you posted a link in the openSUSE bug report I guess he will find

I don't have his E-Mail address, but since you posted a link in the openSUSE bug report I guess he will find this bug report too. I didn't know I was able to put people in the CC list so I just added Simon Farnsworth who commited the above patch, I hope that is ok though.

Can you test a boot with "drm_kms_helper.dp_aux_i2c_transfer_size=1" on the kernel command line? That will reduce the transfer size back to 1 byte per AUX read, but leave the rest of my changes in place.

If that works, I'd be interested in knowing whether transfer_size of 2, 4, or 8 works for you - Windows always uses 16 on Intel Haswell platforms. You have Broadwell, which I haven't had a chance to test.

I see the same on a Dell XPS 13 9343 Broadwell system with an Apple dongle.

Setting drm_kms_helper.dp_aux_i2c_transfer_size to 1, 2 or 4 the dongle works, 8 doesn't work.

Even with the default setting, I was able to:
xrandr --addmode DP1 1920x1080
xrandr --output DP1 --mode 1920x1080

and have the dongle "work" - though xinerama was kind of messed up - applications didn't realize that there were two separate displays.

I also have a DP to HDMI/DVI dongle that works fine with the default setting.

I left my dongle at work so I can't do the test until tomorrow, but I have the same setup as Carl so I guess the outcome would be the same (hopefully). I will try it out tomorrow anyway.

Hi,
I can confirm:
drm_kms_helper.dp_aux_i2c_transfer_size to 1, 2 or 4 work; 8 doesn't work.

(In reply to Chris from comment #7)
> Hi,
> I can confirm:
> drm_kms_helper.dp_aux_i2c_transfer_size to 1, 2 or 4 work; 8 doesn't work.

Are you also on a Broadwell GPU? If so, this definitely needs help from Intel, as I don't have easy access to Broadwell right now for testing.

(In reply to Simon Farnsworth from comment #8)
> (In reply to Chris from comment #7)
> > Hi,
> > I can confirm:
> > drm_kms_helper.dp_aux_i2c_transfer_size to 1, 2 or 4 work; 8 doesn't work.
> 
> Are you also on a Broadwell GPU? If so, this definitely needs help from
> Intel, as I don't have easy access to Broadwell right now for testing.

I'm on a MacBook Pro 8,1 Sandy Bridge with Intel HD Graphics 3000

I have the same exact problem with my dell L502x. With the kernel 4.1.0 the external monitor doesn't work while using a dp->vga adapter. 

Also I have a Sandy Bridge with Intel HD Graphics 3000

drm_kms_helper.dp_aux_i2c_transfer_size with sizes 1, 2 & 4 works for me on a Haswell (i7-4650U) cpu with Intel HD Graphics 5000, while size 8 does not work and leads again to the initial bug whereas the external monitor/dongle are not recognised correctly.

So, a suggestion if anyone's got time to work on this:

drm_kms_helper.dp_aux_i2c_transfer_size sets the number of bytes of I2C data we try to transfer in a single DP AUX transaction; the maximum is 16, the minimum is 1 byte. I have devices that need 16 bytes per transaction, otherwise they fail; it looks like the Apple adapter fails if you try to transfer more than 4 bytes per transaction.

drm_dp_i2c_xfer currently only reduces the transfer size if the DP device returns a partial success (e.g. 4 bytes of I2C data returned when we asked for 16 will cause us to try with 4 bytes per transaction in future). With a bit of care, it could also learn that when the I2C transaction reaches the retry limit and the only response to DP AUX transactions was DP_AUX_NATIVE_REPLY_DEFER, it's time to reduce the transaction size. Note that you do expect some DP_AUX_NATIVE_REPLY_DEFERs in normal operation - they're how the device tells you that it can't reply yet.

(In reply to Simon Farnsworth from comment #12)
> So, a suggestion if anyone's got time to work on this:
> 
> drm_kms_helper.dp_aux_i2c_transfer_size sets the number of bytes of I2C data
> we try to transfer in a single DP AUX transaction; the maximum is 16, the
> minimum is 1 byte. I have devices that need 16 bytes per transaction,
> otherwise they fail; it looks like the Apple adapter fails if you try to
> transfer more than 4 bytes per transaction.
> 
> drm_dp_i2c_xfer currently only reduces the transfer size if the DP device
> returns a partial success (e.g. 4 bytes of I2C data returned when we asked
> for 16 will cause us to try with 4 bytes per transaction in future). With a
> bit of care, it could also learn that when the I2C transaction reaches the
> retry limit and the only response to DP AUX transactions was
> DP_AUX_NATIVE_REPLY_DEFER, it's time to reduce the transaction size. Note
> that you do expect some DP_AUX_NATIVE_REPLY_DEFERs in normal operation -
> they're how the device tells you that it can't reply yet.

I don't think we should need to reduce the transfer size. What we should do is figure out how long the transfer might reasonably take, and adjust our timeouts accordingly.

(In reply to Ville Syrjala from comment #13)
> (In reply to Simon Farnsworth from comment #12)
> > So, a suggestion if anyone's got time to work on this:
> > 
> > drm_kms_helper.dp_aux_i2c_transfer_size sets the number of bytes of I2C data
> > we try to transfer in a single DP AUX transaction; the maximum is 16, the
> > minimum is 1 byte. I have devices that need 16 bytes per transaction,
> > otherwise they fail; it looks like the Apple adapter fails if you try to
> > transfer more than 4 bytes per transaction.
> > 
> > drm_dp_i2c_xfer currently only reduces the transfer size if the DP device
> > returns a partial success (e.g. 4 bytes of I2C data returned when we asked
> > for 16 will cause us to try with 4 bytes per transaction in future). With a
> > bit of care, it could also learn that when the I2C transaction reaches the
> > retry limit and the only response to DP AUX transactions was
> > DP_AUX_NATIVE_REPLY_DEFER, it's time to reduce the transaction size. Note
> > that you do expect some DP_AUX_NATIVE_REPLY_DEFERs in normal operation -
> > they're how the device tells you that it can't reply yet.
> 
> I don't think we should need to reduce the transfer size. What we should do
> is figure out how long the transfer might reasonably take, and adjust our
> timeouts accordingly.

If anyone with an affected adapter wants to test this, the two things to play with, both in drm_dp_i2c_do_msg:

 1. Try changing the 7 in "for (retry = 0; retry < 7; retry++) {" to 70, and see if that helps - it *will* make things slower, by allowing 70 attempts before a transaction fails (at 0.5 ms per attempt, so a delay of 35 ms per failed transaction instead of 3.5 ms), but it may fix things.

 2. Try changing the "500, 600" in "usleep_range(500, 600);" in the case DP_AUX_NATIVE_REPLY_DEFER: block to "5000, 6000"; this makes us wait 5 ms between attempts instead of 0.5 ms. Again, this will slow things down massively,  especially if you increase the retry count to 70.

(In reply to Ville Syrjala from comment #13)
> (In reply to Simon Farnsworth from comment #12)
> > So, a suggestion if anyone's got time to work on this:
> > 
> > drm_kms_helper.dp_aux_i2c_transfer_size sets the number of bytes of I2C data
> > we try to transfer in a single DP AUX transaction; the maximum is 16, the
> > minimum is 1 byte. I have devices that need 16 bytes per transaction,
> > otherwise they fail; it looks like the Apple adapter fails if you try to
> > transfer more than 4 bytes per transaction.
> > 
> > drm_dp_i2c_xfer currently only reduces the transfer size if the DP device
> > returns a partial success (e.g. 4 bytes of I2C data returned when we asked
> > for 16 will cause us to try with 4 bytes per transaction in future). With a
> > bit of care, it could also learn that when the I2C transaction reaches the
> > retry limit and the only response to DP AUX transactions was
> > DP_AUX_NATIVE_REPLY_DEFER, it's time to reduce the transaction size. Note
> > that you do expect some DP_AUX_NATIVE_REPLY_DEFERs in normal operation -
> > they're how the device tells you that it can't reply yet.
> 
> I don't think we should need to reduce the transfer size. What we should do
> is figure out how long the transfer might reasonably take, and adjust our
> timeouts accordingly.

I have an e-mail report saying that the following patch appears to help; Ville, can you take over on this?

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 80a02a4..a17444f 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -440,7 +440,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
         * is required to retry at least seven times upon receiving AUX_DEFER
         * before giving up the AUX transaction.
         */
-       for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
+       for (retry = 0, defer_i2c = 0; retry < (70 + defer_i2c); retry++) {
                mutex_lock(&aux->hw_mutex);
                ret = aux->transfer(aux, msg);
                mutex_unlock(&aux->hw_mutex);

Created attachment 117909 [details] [review]
[PATCH] drm/dp: Adjust i2c-over-aux retry count based on message size and i2c bus speed

This patch should help. For now it assumes a 10 kHz i2c bus speed, but in the future we could optimize that a bit based on DPCD.

Please test.

Please test the series:

http://mid.gmane.org/1440618907-11169-1-git-send-email-ville.syrjala@linux.intel.com

http://mid.gmane.org/1440618907-11169-2-git-send-email-ville.syrjala@linux.intel.com

http://mid.gmane.org/1440618907-11169-3-git-send-email-ville.syrjala@linux.intel.com

Linux 4.2 is still affected. 

Also, I couldn't see anything in the three test links posted above, the Gmane page was blank except for the menus and black borders.

(In reply to Hugo Roy from comment #18)
> Linux 4.2 is still affected. 
> 
> Also, I couldn't see anything in the three test links posted above, the
> Gmane page was blank except for the menus and black borders.

Works for me. But the patches were merged and are in v4.3-rc1 and later. Please retest with that.

Can confirm. Using (3) different mdp-to-vga adapters
- on 4.1.13 my external monitor is not recognized and does not work, xrandr returns some garbage output and/or freeze the whole system until i unplug the monitor
- on 4.2.6 monitor is recognized correctly but the resolutions supported are all wrong (not even the same scale, 4:3 vs 16:9), no way to make it use the correct ones (like lost 1-2 hours testing everything I could)
- on 4.3.0 all works flawlessly and without any problem, no need to configure anything: as soon as the monitor is plugged in it uses the correct (max) resolution

(In reply to pierpaolo.busacchi from comment #20)
> Can confirm. Using (3) different mdp-to-vga adapters
> - on 4.1.13 my external monitor is not recognized and does not work, xrandr
> returns some garbage output and/or freeze the whole system until i unplug
> the monitor
> - on 4.2.6 monitor is recognized correctly but the resolutions supported are
> all wrong (not even the same scale, 4:3 vs 16:9), no way to make it use the
> correct ones (like lost 1-2 hours testing everything I could)
> - on 4.3.0 all works flawlessly and without any problem, no need to
> configure anything: as soon as the monitor is plugged in it uses the correct
> (max) resolution

Awesome, thanks for the follow-up. Closing.

Will we have stable fixes for 4.1 / 4.2, or aren't they easily backportable?

(In reply to Takashi Iwai from comment #22)
> Will we have stable fixes for 4.1 / 4.2, or aren't they easily backportable?

I think we had quite a pile of fixes on DP aux and i2c-over-aux both in the drm helpers and in our driver code. Some of them were probably bigger than stable kernel limits. And I can't be sure whether a specific individual commit fixed things, or whether it was some combination of them all. We also had other bugs than this one.

What I do know is that nobody on our side is working on identifying and doing the actual backports.