Bug 200045

Ok, actually this is 4.17 issue; on 4.16 hw_i2c=1 fixes the issue completely, but on 4.17 changing backlight in Xorg (using keys) causes a reboot, fbcon works fine though.

Okay, these are actually two bug reports meanwhile:

1) 3e5f06bed72fe72166a6778f630241a893f67799 causes a regression when bit-banging I2C in software and results in keeping the screen black.

2) There is some regression when using the a HW interface for I2C which causes a reboot when changing the brightness.

I'd suggest to work only on 1) here and create a new bug report for 2). I should be able to give you some patches to test in a few minutes.

Thanks for the report and the bisecting!

I did some more research and it turns out that 2) happens if I change brightness level right after X starts, even in 4.15 kernel. If I wait a bit, everything works good, including 4.17.1. Software I2C doesn't reproduce same results, so I'm not sure if I should report 2) because it's stated that it can cause issues, but with different kind of errors:

[drm] hw_i2c forced on, you may experience display detection problems!

hw_i2c doesn't use bit banging, it uses the hw i2c engine.

Sure. But reading the original description above, I think the default use case is to not use hw_i2c. It was just added to try to work around the regression. And that led to another bug. Or am I wrong here?

So, I really posted the patches now. Sorry, I got side-tracked.

They can be found here:

http://patchwork.ozlabs.org/patch/929185/
http://patchwork.ozlabs.org/patch/929186/

I set you on CC as well. As indicated in the cover letter, could you try these patches one by one?

Thanks and kind regards.

(In reply to Wolfram Sang from comment #5)
> Sure. But reading the original description above, I think the default use
> case is to not use hw_i2c. It was just added to try to work around the
> regression. And that led to another bug. Or am I wrong here?

Correct.  I was just clarifying that that option does.

Ok, I tested the patches, but they did not fix the problem. Nothing changed, and the log is empty. I tried to apply them separately and both at the same time. Note that second one seems to depend on the first, so I had to apply it manually.

Thanks for trying to fix the issue!

Created attachment 276551 [details]
print initial states to allow further debug

Thanks for testing the patches, pity it didn't help.

Would you mind testing with the attached patch? It should give some output about the initial states. Let's hope that will lead to something...

Here is the output:

[   23.056329]  (null): initial SCL state 1
[   23.056330]  (null): initial SDA state 1
[   23.056532]  (null): initial SCL state 1
[   23.056534]  (null): initial SDA state 1
[   23.056578]  (null): initial SCL state 0
[   23.056580]  (null): initial SDA state 0
[   23.056617]  (null): initial SCL state 0
[   23.056618]  (null): initial SDA state 0
[   23.056656]  (null): initial SCL state 0
[   23.056657]  (null): initial SDA state 0

I have sent another patch, let's hope it really is this issue:

http://patchwork.ozlabs.org/patch/929798/

Teset this, but it did not help.

Created attachment 276571 [details]
WIP to test

Pity that it didn't work. The thing is that I am quite sure the pre/post_xfer calls are needed. I am running out of ideas now.

The best thing I can come up with is this patch which adds all the stuff altogether. I don't have high hopes, but maybe all 3 improvements make it work. At least, the debug output will be better, it was a bit meaningless before.

If this still doesn't work, could you move the new code block from this patch after the add_adapter() call and try again (with the old block still deleted)?

Thanks for all the testing!

Still nothing changed... This is what I got in both cases:

[   57.656675] Radeon i2c bit bus 0x90: initial SCL state 1
[   57.656678] Radeon i2c bit bus 0x90: initial SDA state 1
[   57.656805] Radeon i2c bit bus 0x91: initial SCL state 1
[   57.656807] Radeon i2c bit bus 0x91: initial SDA state 1
[   57.656921] Radeon i2c bit bus 0x92: initial SCL state 0
[   57.656924] Radeon i2c bit bus 0x92: initial SDA state 1
[   57.660108] Radeon i2c bit bus 0x93: initial SCL state 0
[   57.660111] Radeon i2c bit bus 0x93: initial SDA state 0
[   57.664128] Radeon i2c bit bus 0x14: initial SCL state 0
[   57.664131] Radeon i2c bit bus 0x14: initial SDA state 0

And if I leave only printing code (i.e get the working state):

[   23.992763] Radeon i2c bit bus 0x90: initial SCL state 1
[   23.992766] Radeon i2c bit bus 0x90: initial SDA state 1
[   23.992851] Radeon i2c bit bus 0x91: initial SCL state 1
[   23.992854] Radeon i2c bit bus 0x91: initial SDA state 1
[   23.992938] Radeon i2c bit bus 0x92: initial SCL state 0
[   23.992941] Radeon i2c bit bus 0x92: initial SDA state 0
[   23.993080] Radeon i2c bit bus 0x93: initial SCL state 0
[   23.993082] Radeon i2c bit bus 0x93: initial SDA state 0
[   23.993146] Radeon i2c bit bus 0x14: initial SCL state 0
[   23.993148] Radeon i2c bit bus 0x14: initial SDA state 0

I will just revert this commit. I have no further ideas for remote debugging. I think reverting is the safest solution for now, I can't find any patches since v4.16 which added users to this bitbanging algorithm. So, there can't be anyone depending on the new functionality. And I will try to fix my original issue one layer above at the i2c-gpio level.

Thanks for reporting! Would you reveal your real name for a proper "Reported-by:" tag?

My name is Sergey Larin, I added it to my account. Thanks for your help!