Add Snapdragon 835 GPU to Windows whitelist

RESOLVED FIXED in Firefox 59

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jonathan.kunkee+bugzilla, Assigned: jonathan.kunkee+bugzilla)

Tracking

58 Branch
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Edge/16.16299

Steps to reproduce:

I am a Microsoft engineer working to get Firefox running well under x86 emulation on ARM64 Windows devices.

Install Firefox.
Navigate to any content that can be hardware accelerated, like shadertoy.com.
Note FPS.




Actual results:

FPS is below 5. Hardware acceleration is not enabled because the Snapdragon 835 GPU is not whitelisted on Windows.


Expected results:

FPS should be above 15--or at least hardware acceleration should be enabled.
(Assignee)

Comment 1

a year ago
I am preparing a patch to add the needed entry.
Component: Untriaged → Graphics
Product: Firefox → Core
(Assignee)

Comment 2

a year ago
A little more detail is in order:

I am using
https://www.shadertoy.com/view/XsfXDI
as a basic functional test of WebGL hardware acceleration.

It renders at 0.8 FPS without hardware acceleration.
It renders at ~9.3 FPS with hardware acceleration.

Hardware acceleration was enabled by adding Qualcomm's vendor ID to the vendor whitelist in widget\windows\GfxInfo.cpp's GfxInfo::GetFeatureStatusImpl.

I have a problem, though: GfxInfo::Init assumes that all device string entities are numerical, when entire categories are not. In this case, though, the DISPLAY_DEVICEW.DeviceID does not have all numerical values:

ACPI\VEN_QCOM&DEV_007C&SUBSYS_CLS08998&REV_007C

Further, this string format is optional. ACPI devices that omit _HRV (as is legal) show up as

ACPI\QCOM007C

so ParseIDFromDeviceID returns 0xC instead of QCOM or a meaningful number. My current fix adds 0xC as Qualcomm's VendorID instead of something correct. Reauthoring the device string parsing logic is about as invasive as converting it to pure DXGI calls, so I plan on filing a separate bug about that and am considering putting a special case in ParseIDFromDeviceID for Qualcomm for now.

I'm prepping a patch for MozReview with this.

Thoughts? Should this discussion go on a mailing list instead?
(Assignee)

Comment 3

a year ago
I made a small associative error while rewriting parts of that last comment:

VEN_QCOM => VendorID=0xC
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
Comment on attachment 8940392 [details]
BUG 1428174 - Add Qualcomm to Windows GPU Whitelist

https://reviewboard.mozilla.org/r/210666/#review216924
Attachment #8940392 - Flags: review?(jmuizelaar) → review+
Keywords: checkin-needed
Assignee: nobody → jonathan.kunkee+bugzilla
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s bd65a029558b015d5c1b588a75e69bb02c7f6d20 -d 17bf5819b42f: rebasing 442800:bd65a029558b "BUG 1428174 - Add Qualcomm to Windows GPU Whitelist r=jrmuizel" (tip)
merging widget/GfxDriverInfo.cpp
merging widget/GfxDriverInfo.h
merging widget/windows/GfxInfo.cpp
warning: conflicts while merging widget/GfxDriverInfo.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging widget/GfxDriverInfo.h! (edit, then use 'hg resolve --mark')
warning: conflicts while merging widget/windows/GfxInfo.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
The code base has changed and the patch has conflicts now, please fix those and push the updated version to review board. Thank you.
Flags: needinfo?(jonathan.kunkee+bugzilla)
Keywords: checkin-needed
Comment hidden (mozreview-request)
(Assignee)

Comment 9

a year ago
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #7)
> The code base has changed and the patch has conflicts now, please fix those
> and push the updated version to review board. Thank you.

It seems that Parallels needed the same type of work at the same time.

An updated version has been pushed.
Flags: needinfo?(jonathan.kunkee+bugzilla)
Keywords: checkin-needed

Comment 10

a year ago
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c1f3acb9f5cc
Add Qualcomm to Windows GPU Whitelist r=jrmuizel
Keywords: checkin-needed

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c1f3acb9f5cc
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.