Bug 91451 - Apple miniDP-to-VGA adapter doesn't work anymore since Kernel 4.1
Summary: Apple miniDP-to-VGA adapter doesn't work anymore since Kernel 4.1
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Intel GFX Bugs mailing list
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-24 12:46 UTC by Florian H.
Modified: 2017-07-24 22:45 UTC (History)
9 users (show)

See Also:
i915 platform:
i915 features: display/DP


Attachments
dmesg output (192.53 KB, text/plain)
2015-07-24 12:46 UTC, Florian H.
no flags Details
[PATCH] drm/dp: Adjust i2c-over-aux retry count based on message size and i2c bus speed (4.71 KB, patch)
2015-08-25 14:59 UTC, Ville Syrjala
no flags Details | Splinter Review

Description Florian H. 2015-07-24 12:46:43 UTC
Created attachment 117341 [details]
dmesg output

Since Kernel 4.1 the Apple miniDP-to-VGA adapter stopped working. If I revert the following commit the dongle works again:

1d002fa720738bcd0bddb9178e9ea0773288e1dd is the first bad commit
commit 1d002fa720738bcd0bddb9178e9ea0773288e1dd
Author: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
Date: Tue Feb 10 18:38:08 2015 +0000

drm/dp: Use large transactions for I2C over AUX

I was able to reproduce this bug with the nightly build:

Linux note 4.2.0-1-drm-intel-nightly #1 SMP PREEMPT Fri Jul 24 13:19:36 CEST 2015 x86_64 GNU/Linux

Apparently there are more user affected by this bug: https://bbs.archlinux.org/viewtopic.php?id=199540

miniDP-to-DVI/HDMI still seems to be working (but I can't test that)
Comment 1 Peter Hanisch 2015-07-28 19:07:46 UTC
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
Comment 2 Florian H. 2015-07-28 19:26:06 UTC
I don't have his E-Mail address, but since you posted a link in the openSUSE bug report I guess he will find
Comment 3 Florian H. 2015-07-28 19:28:07 UTC
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.
Comment 4 Simon Farnsworth 2015-07-29 08:47:43 UTC
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.
Comment 5 Carl Michal 2015-07-29 18:43:53 UTC
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.
Comment 6 Florian H. 2015-07-30 09:00:08 UTC
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.
Comment 7 Chris 2015-08-06 14:59:54 UTC
Hi,
I can confirm:
drm_kms_helper.dp_aux_i2c_transfer_size to 1, 2 or 4 work; 8 doesn't work.
Comment 8 Simon Farnsworth 2015-08-06 15:07:15 UTC
(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.
Comment 9 Chris 2015-08-06 16:33:01 UTC
(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
Comment 10 pierpaolo.busacchi 2015-08-17 21:14:56 UTC
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
Comment 11 Peter Hanisch 2015-08-18 12:34:49 UTC
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.
Comment 12 Simon Farnsworth 2015-08-24 10:08:08 UTC
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.
Comment 13 Ville Syrjala 2015-08-24 13:00:40 UTC
(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.
Comment 14 Simon Farnsworth 2015-08-24 13:29:29 UTC
(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.
Comment 15 Simon Farnsworth 2015-08-25 09:03:06 UTC
(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);
Comment 16 Ville Syrjala 2015-08-25 14:59:34 UTC
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.
Comment 18 Hugo Roy 2015-10-13 21:11:00 UTC
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.
Comment 19 Jani Nikula 2015-10-14 07:39:32 UTC
(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.
Comment 20 pierpaolo.busacchi 2015-12-03 02:10:19 UTC
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
Comment 21 Jani Nikula 2015-12-03 12:12:24 UTC
(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.
Comment 22 Takashi Iwai 2015-12-03 14:39:40 UTC
Will we have stable fixes for 4.1 / 4.2, or aren't they easily backportable?
Comment 23 Jani Nikula 2015-12-03 15:57:24 UTC
(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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.