Bug 100954 - i915 i2c-dev failures with recent chips and docking stations
Summary: i915 i2c-dev failures with recent chips and docking stations
Status: REOPENED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Intel GFX Bugs mailing list
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard: Triaged, ReadyForDev
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-06 05:52 UTC by Sanford Rockowitz
Modified: 2019-06-18 14:52 UTC (History)
11 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sanford Rockowitz 2017-05-06 05:52:43 UTC
I am the author of ddcutil (http://www.ddcutil.com), a Linux utility that
manages monitor settings using DDC/CI. I am seeing a pattern of user
error reports in which I2C communication is not working when a system
with a recent Intel chip, using the i915 driver, is plugged into a
docking station. 

ddcutil looks for displays by examining all non-SMBus /dev/i2c devices
on the system.  If checks for the presence of slave address x50 and
x37.  If they exist it tries to read the EDID on x50 and a Virtual
Control Panel feature value on x37.  Looking at one of the user logs, I
see that a couple /dev/i2c-n devices have udev sysattr name DPMST.  When
ddcutil probes those /dev/i2c devices, slave addresses x50 and x37
appear active, but reading the EDID fails. 

The issue was discussed in this thread (https://lists.freedesktop.org/archives/intel-gfx/2017-May/127350.html) on the intel-gfx mailing list.

Comments from Jani Nikula: 

The issues are related to DP MST (Multi-Stream Transport) that the docks
use nowadays. The single DP link is divided to several logical links to
sink devices. The I2C communication needs to be encapsulated to remote
I2C-over-AUX reads and writes, with the logical DP MST addresses, to
route them to the correct sinks. In kernel, this is handled by the MST
topology manager.

...

I think it's more likely the issue is in core drm DP MST code, and
nobody ever tried the I2C interface for them. The core I2C code should
be completely ignorant about DP MST, or even DP for that matter.

...

You'll want the DP MST I2C code fixed. Well, at least it's my *guess*
that's where the problem is.
Comment 1 Ricardo 2017-05-09 20:54:43 UTC
DP and DP MST has seen many changes lately that could address your issue. I recommend using the latest intel drm-tip to test, but in order to address the issue and debug. 
Can you also include the logs, here is documentation to help you 
https://01.org/linuxgraphics/documentation/bugs-and-debugging

Also can you add the hardware information of the system use to capture logs.

Once that information is provided can you change the status of the bug to REOPEN
Comment 2 Sanford Rockowitz 2017-05-10 07:58:50 UTC
Ricardo, 

Unfortunately, I don't see the problem on any of my own laptops, which are older. So for me to install the latest intel drm and produce a driver diagnostic log is pointless.  However, I've received enough bug reports from people who have tried to use ddcutil through a docking station that I'm pretty sure there's a problem.  

From the original post on the intel-gfx list:

> I'm not sure how best to go about reporting these issues.  The usual bug
> reporting mechanism asks for detailed information about a specific
> system, which I don't have.   What I do have is the ability to see a
> pattern.

> Because I2C communication is so fragile (dependencies on hardware
> quirks, drivers, etc), ddcutil (which is written in C) has an extensive
> subsystem devoted to remotely diagnosing user configurations.  It
> already reports DDC (i.e. I2C) communication errors, and has interfaces
> to udev, libdrm, and x11 libraries.  If there is some i915 specific code
> I could add to refine the diagnosis, I would be happy to add that. 
> (Note: ddcutil is entirely a user space program, using the i2c-dev
> interface.)

> Suggestions on how to proceed appreciated.

That list thread led to the suggestion that I post a bug report despite the unavailability of detail.  Hopefully I've categorized the bug properly, and this report will at least serve as a marker that there's a problem. 

Sanford
Comment 3 Ricardo 2017-06-21 17:33:45 UTC
hello again,
Like I comment before there has been many changes and fixes done to DP. I would suggest to your users that they try a recent kernel 4.12  or newer. 
and also suggest them to file a bug and we will try to work on identifying the issue.

I'm closing this as NOT A BUG, but again we are here to help but in order to do so please help us with more information about the issue in specific.
Comment 4 Tomas Janousek 2017-11-24 17:36:19 UTC
This is definitely still reproducible with kernel 4.13, ThinkPad 25 and TP Ultra Dock. Which log exactly would help diagnose the issue further?
Comment 5 Ville Syrjala 2017-11-24 17:46:17 UTC
(In reply to Tomas Janousek from comment #4)
> This is definitely still reproducible with kernel 4.13, ThinkPad 25 and TP
> Ultra Dock. Which log exactly would help diagnose the issue further?

I don't think remote i2c writes are really implemented in the DP MST code. Yeah, looks like the code only does reads, and even that implementation might be somewhat dubious.
Comment 6 Ville Syrjala 2018-02-22 15:25:55 UTC
I took at stab at implementing remote DPCD read/write [1] at some point. While that does not help with the remote i2c directly, it's basically the same sort of problem in the code. So something similar would have to be done for remote i2c. Note that my remote DPCD thing didn't actually work, so someone would have to fix it up as well.

[1] git://github.com/vsyrjala/linux.git dp_mst_port_aux_dev
Comment 7 Jani Saarinen 2018-03-29 07:10:50 UTC
First of all. Sorry about spam.
This is mass update for our bugs. 

Sorry if you feel this annoying but with this trying to understand if bug still valid or not.
If bug investigation still in progress, please ignore this and I apologize!

If you think this is not anymore valid, please comment to the bug that can be closed.
If you haven't tested with our latest pre-upstream tree(drm-tip), can you do that also to see if issue is valid there still and if you cannot see issue there, please comment to the bug.
Comment 8 Jani Saarinen 2018-04-23 09:55:26 UTC
Please test also latest drm-tip as there has been MST changes lately.
Comment 9 Lakshmi 2018-09-07 15:35:55 UTC
Sorry for the delay...

I assume this issue has been fixed.
Closing now. Feel free to reopen if you still have the issue. 

When you reopen ensure that issue is with the latest drmtip. (https://cgit.freedesktop.org/drm-tip)

Attach the full dmesg from boot with kernel parameters drm.debug=0x1e log_buf_len=4M.
Comment 10 Mariusz Mazur 2018-10-14 22:04:58 UTC
Thinkpad X1 Carbon 6th gen, lenovo's thundrbolt dock, kernel 4.18.13-300.fc29.x86_64.

Results:
– built-in hdmi port on the laptop – ddc/ci works fine.
– dp/hdmi/vga ports on the dock – no ddc/ci at all.

It appears that at present a multi monitor setup via a thunderbolt dock is doable only if one's ok with manually changing brightness on each monitor. Unless there's a separate case for dealing with this particular problem (haven't found one), this case should stay opened.
Comment 11 Lakshmi 2018-10-16 11:26:45 UTC
Mariusz, Thanks for reporting back.

(In reply to Mariusz Mazur from comment #10)
> Thinkpad X1 Carbon 6th gen, lenovo's thundrbolt dock, kernel
> 4.18.13-300.fc29.x86_64.
> 
can you produce this with latest kernel (drm-tip branch)? This looks like a bit older than the latest kernel.
(https://cgit.freedesktop.org/drm-tip).
If problem exists with latest drm-tip, set kernel parameters drm.debug=0x1e log_buf_len=4M and reboot.
Try to reproduce the issue and attach the dmesg log. This way we see more information about the bug.

This information is needed for debugging this issue.
Comment 12 Mariusz Mazur 2018-10-16 13:49:17 UTC
Will do, though this will take until next week at the earliest.
Comment 13 Ville Syrjala 2018-10-16 13:52:36 UTC
We still don't have remote i2c writes. No point in wasting time testing this.
Comment 14 Mariusz Mazur 2018-10-21 21:34:45 UTC
@Ville, is the lack of remote i2c writes also responsible for my usb-c to hdmi adapter not allowing ddc/ci?
Comment 15 Ville Syrjala 2018-10-22 10:29:10 UTC
(In reply to Mariusz Mazur from comment #14)
> @Ville, is the lack of remote i2c writes also responsible for my usb-c to
> hdmi adapter not allowing ddc/ci?

Only if it is an MST device. Otherwise you should at least be able to try it. It may not work in practice though on account of crappy DDC/CI implementation in the monitor. At least one monitor where I tried it didn't like the default 100 kHz bus speed and so bunch of things randomly failed until I lowered the bus speed significantly.
Comment 16 Sanford Rockowitz 2018-10-22 15:14:12 UTC
Ville, 

How are you lowering the bus speed?  Is there a publicly usable i915 option that I can add to the ddcutil documentation, or was it a private driver mod? 

Sanford
Comment 17 Ville Syrjala 2018-10-22 15:27:29 UTC
(In reply to Sanford Rockowitz from comment #16)
> Ville, 
> 
> How are you lowering the bus speed?  Is there a publicly usable i915 option
> that I can add to the ddcutil documentation, or was it a private driver mod? 
> 
> Sanford

git://github.com/vsyrjala/linux.git i2c_bus_speed

Never tried to upstream that. And I don't recall if I actually tested that specific branch, but I do remember testing something similar.
Comment 18 Sanford Rockowitz 2018-10-22 21:29:43 UTC
Here's a vote for getting some variant of the I2C speed change for DDC devices into the i915 driver, preferably with a module option allowing users to specify the speed to be used.  

Relatedly, it looks like the following change allows for adjusting the DDC/CI speed for DisplayPort connected monitors, irrespective of video driver.  Am I correct on that?

https://www.systutorials.com/linux-kernels/600766/drm-dp-add-dp_aux_i2c_speed_khz-module-param-to-set-the-assume-i2c-bus-speed-linux-4-3/

You're so right about marginal DDC/CI implementations.  For my personal rogues' gallery see: http://www.ddcutil.com/monitor_notes/
Comment 19 Mariusz Mazur 2018-10-23 09:38:04 UTC
@Ville, how would I easily verify whether mst/non–mst codepaths are used by the drivers? My usb dongle is a single hdmi output one, so it'd makes sense for it to be non–mst, though I can't be 100% certain. I know the monitor has no issues with ddc/ci on a dedicated hdmi port, so if the dongle is non–mst, that'd probably warrant some separate debugging/a case of it's own.
Comment 20 Ville Syrjala 2018-10-23 09:51:44 UTC
(In reply to Sanford Rockowitz from comment #18)
> Here's a vote for getting some variant of the I2C speed change for DDC
> devices into the i915 driver, preferably with a module option allowing users
> to specify the speed to be used.  
> 
> Relatedly, it looks like the following change allows for adjusting the
> DDC/CI speed for DisplayPort connected monitors, irrespective of video
> driver.  Am I correct on that?
> 
> https://www.systutorials.com/linux-kernels/600766/drm-dp-add-
> dp_aux_i2c_speed_khz-module-param-to-set-the-assume-i2c-bus-speed-linux-4-3/
> 
> You're so right about marginal DDC/CI implementations.  For my personal
> rogues' gallery see: http://www.ddcutil.com/monitor_notes/

dp_aux_i2c_speed_khz only allows you to set the assumed i2c bus speed (for timeouts and such). It doesn't actually try to configure it via the DP_I2C_SPEED_CONTROL_STATUS. And I haven't seen many devices that even support DP_I2C_SPEED_CONTROL_STATUS.

In general we don't like to add modparams. Having them usually means users randomly setting them instead of reporting the bugs. Ideally we'd come up with some kind of scheme that just works. Perhaps some kind of "retry with slower speed" type of thing. Unfortunately that would only work for i2c protocol level errors. If it's just the payload that's corrupted we can't detect it. That would have to be done by whoever initiated the transfer. So some kind of new i2c adapter function that could be called to reduce the speed perhaps, and I suppose we'd need a similar ioctl for /dev/i2c. An alternative would be a new function/ioctl to explicitly set the bus speed. Something like that would allow you to maintain a i2c bus speed quirk list for ddc/ci at least.
Comment 21 Sanford Rockowitz 2018-10-23 12:43:59 UTC
Some observations.

1) What I see on dirty monitors is payload corruption, detected by the DDC/CI checksum.  There are commonly duplicate bytes in the packet.

2) If there were an ioctl to control bus speed, ddcutil could change that before retrying a write/read sequence (used to read feature values and the monitor capabilities string).  It would have be a bit clever about writes without a subsequent read (used to set a feature value) as there's no way to detect data corruption in that case.

3) If the i915 driver were to unilaterally slow down all I2C transactions for slave address x37 it would eliminate the possibility of per-monitor adjustments, but I don't believe it would have any perceived affect on performance.  The DDC/CI protocol mandates that the host wait from 40ms to 200ms between writes and reads, depending on operation. ddcutil spends over 90% of its elapsed time in sleeps mandated by the DDC/CI protocol.
Comment 22 Lakshmi 2018-12-18 05:54:57 UTC
Ville, how to proceed further?
Comment 23 nicolo 2019-01-24 23:51:48 UTC
I also have the same problem, using a Lenovo Ultra Dock and Thinkpad T460s, running i915 and kernel 4.20.3. ddcutil works if plugged directly (say via hdmi), but not through the dock (neither via hdmi nor dvi; tested with multiple monitors). Let me know if I can be of any help.
Comment 24 Mariusz Mazur 2019-02-06 15:13:27 UTC
2019 the year of the DP multi monitor brightness control, I want to believe!
Comment 25 James Ausmus 2019-06-18 14:40:40 UTC
Ville - are we still at the same state regarding remote i2c writes, or would it make sense to retest on drm-tip now?
Comment 26 Ville Syrjala 2019-06-18 14:52:14 UTC
(In reply to James Ausmus from comment #25)
> Ville - are we still at the same state regarding remote i2c writes, or would
> it make sense to retest on drm-tip now?

No has volunteered to do the work -> nothing has changed.


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.