New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 709252 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocked on:
issue 709642


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

glimmer: tablet mode does not work, on m58 and m59

Project Member Reported by bleung@chromium.org, Apr 6 2017

Issue description

Chrome Version: 58.0.3029.55
Chrome OS Version: R58-9334.36.0
Chrome OS Platform: Glimmer MP

Please specify Cr-* of the system to which this bug/feature applies (add
the label below).

Steps To Reproduce:
(1) Log into guest mode
(2) Flip Glimmer around to Stand/Tent/Tablet mode
(3)

Expected Result:
UI reacts appropriately, goes into tablet UI

Actual Result:
Stays in "clamshell" ui. Keyboard and mouse still functional.

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)
Always

What is the impact to the user, and is there a workaround? If so, what is
it?
Tablet mode, tent mode, and stand mode not working.

Please provide any additional information below. Attach a screen shot or
log if possible.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.


 
Cc: bleung@google.com
Logs from my system attached.
debug-logs_20170406-153941.tgz
67.6 KB Download
Cc: -bleung@google.com rajatja@chromium.org
Labels: Kernel-4.4
I also wonder if this has something to do with the uprevved kernel. Glimmer now has v4.4.

Comment 3 by rajatja@google.com, Apr 6 2017

Cc: jonr...@chromium.org
OK, let me take a look. There were some changes that went into chrome that changed the way tablet ui is detected. But I don't think any of them made into R58. Is that a correct understanding, Jon? 
I don't believe that any of those patches were merged into R58, IIRC 59.0.3041.0 was the first build with them.

Do we have a /var/log/ui/ui.LATEST that is longer than the one in #1? It seems short. However it isn't listing any accelerometer errors.

Comment 5 by rajatja@google.com, Apr 7 2017

Cc: gwen...@google.com snanda@chromium.org
OK, I confirmed that this is because of the kernel upgrade to 4.4. Chrome does not see the accelerometer anymore. Gwendal mentioned that may be because the EC-Kernel API changed in 3.18, so we might need to backport stuff into baytrail EC code :-(
Would it be possible to move the 3.18 accelerometer support forward to 4.4?

Changing the EC is a pretty heavy procedure.
Cc: girard@chromium.org
+girard@ FYI

The last time there was a change to the accelerometer format it was during the 3.14-3.18. At the time we did not want to make the change to reading in the Chrome side. This was because we were afraid of having to maintain 'n' different file format parsings within Chrome.

We did end up adding it, due to time constraints, and at the time a longer term plan to get the 3.14 devices updated to 3.18. So that we could remove the legacy path.

I would be against adding a third set of file parsing to the Chrome side. As then we would end up having to maintain all formats. 
Cc: zork@chromium.org
I may be confusing things here, but if we have support for the 3.18 API in Chrome, and we enable the 3.18 API on 4.4, it should be somewhat trivial from the Chrome point of view?


If 4.4 can have the 3.18 API enabled, then it should be trivial from the Chrome code side.

I had not understood that as an option.
OK, this does seem more serious than it looks, on further discussions with Gwendal. Summary:

On R57: we have 3 baytrails on 4.4 kernel. 1 of them supports "tent mode" so is broken currently.

On R58/R59: All baytrails are on 4.4 kernel. Any model that expects to get into tablet mode is broken.

Reason: The API / protocol / features w.r.t. sensors between kernel <--> EC underwent big changes somewhere in 3.14/3.18 time frame. The new kernel does not support the old EC. I'm not sure if accelerometer is the only thing broken or if there is anything else. Do any baytrail devices have any other sensors than accelerometer? Since other than this issue the system is otherwise behaving very well, passed all our autotests, and also passed our testing team's testing - I'd think EC - Kernel communication should be OK.

Our options:

1) Back port the needed items into baytrail EC. This is quite big (also requiring us to qualify new EC image?)

2) Forward port the old EC API into new kernel. Talked to Gwendal, he seemed to indicate this is also big. However more importantly, he also has doubts if the baytrail EC in its current form will not be able to support ARC++ (w.r.t sensors) - although I couldn't understand very well why. 

3) Forward port bits & pieces into new kernel. Still trying to figure out complexity. This could help us support tablet mode - but ARC++ may yet not be supported. Fallout: Games & other apps that require accelerometer will be impacted. But do we care given baytrails have mostly gone to EDU?

I guess we need some PM inputs here, also I think it may be too late to pull back to older kernel at this point.

Thanks,

Rajat
Cc: yoshiat@google.com
Cc: omrilio@chromium.org
+omrilio@ for PM input on touchview.
It seems like accelerometer data should be managed by some system-level service and exposed to ash and Chrome in some consistent way (and tests to catch early when formats change), but that seems like a bigger task than we want to take on here.

Barring that, the sanest approach seems to be to add support for the new format in Chrome temporarily (though I know jonross@ objected to that in #7)

I'm far more concerned that we completely broke touchview on a device in 57 and didn't catch it until now.  That implies a gaping hole in our testing.

Who can own forging a consensus and driving a solution here?
Albert : 

The issue is that the newer 4.4 kernel no longer supports the legacy protocol 0 of the EC interface for accessing the sensors that kernel 3.10 did. There's nothing for Chrome to do in this situation because the kernel doesn't expose any sensors on Glimmer using kernel v4.4.

We just had a meeting of kernel/firmware people and Yoshi will summarize our gameplan.
Ah! Got it. I misunderstood the problem.
We just had a meeting on this, and the engineering folks are going to do some more analysis on the best way out of this situation before we have a complete plan.

The initial response here is going to be to skip 58 for Glimmer, which will buy sufficient time to put in a work around, we are too late in the cycle to risk doing much else. 

The high level options are:

1. Revert Glimmer back to 3.10.
2. Bring forward the 3.10 protocol (v0) into 4.4. 
3. Backport necessary CLs into Glimmer firmware (ec) to support the 4.4 protocol (v1 or v2). 

I don't think any of these will involve changes on the Chrome side. 

Note that Clapper is already live with this on 57, so we don't need to skip 58 at this time for it, Clapper is only a semi-convertible (270deg hinge) so the touch view use case is limited there.
Cc: amstan@chromium.org
Components: OS>Firmware>EC
Owner: gwendal@chromium.org
This will block stable for 58 for Glimmer.

Comment 21 by rajatja@google.com, Apr 26 2017

Didn't we decide to skip M58 release for glimmer for this reason?
Right, it will block 58 from going to Glimmer which results in that skipping, sorry for the awkward words.

Any updates?
Blockedon: 709642
Used a wrong bug for the CLs (709642). Change is in ToT and R59.
Anything left to do here? FWIW, I've verified this on ToT on clapper. 
Looking in glimmer overlay, I don't see ARC enabled. On the machine, the play icon is not showing. Adding it now.
Can this be closed?
Labels: -M-59
If it is fixed in 59 we can just drop the 59 label. 

We are skipping 58 for glimmer and this bug is the reminder for that.
Status: Fixed (was: Available)
Closing as it sounds like this shipped in 59.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-58; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-58 label, otherwise remove Merge-TBD label. Thanks.

Comment 32 by yoshi@chromium.org, Dec 18 2017

Cc: -yoshiat@google.com
Project Member

Comment 33 by sheriffbot@chromium.org, Feb 12 2018

Labels: -Merge-TBD

Sign in to add a comment