[Impeller] add parsing of known GPU models #55196
Conversation
|
From some brief research, we should probably disable all 500 series Adreno's |
jonahwilliams
changed the title
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
jonahwilliams
changed the title
| // | ||
| // Reports "VK_INCOMPLETE" when compiling certain entity shader with | ||
| // vkCreateGraphicsPipelines, which is not a valid return status. | ||
| // See https://github.com/flutter/flutter/issues/155185 . |
There was a problem hiding this comment.
This version of the PR doesn't actually affect that issue, right?
That is, this adds parsing but it doesn't yet change what set of GPUs get treated as known-bad.
There was a problem hiding this comment.
we're still going to try to aquire a device with a similar GPU and see what isn't working first.
|
|
||
| AdrenoGPU GetAdrenoVersion(std::string_view version) { | ||
| /// The format that Adreno names follow is "Adreno (TM) VERSION". | ||
| auto paren_pos = version.find(")"); |
There was a problem hiding this comment.
to be supppper sure; version.find("Adreno (TM) ") - and then just skip.
There was a problem hiding this comment.
(or use a regex like below)
There was a problem hiding this comment.
I changed this to match more of the string.
|
|
||
| MaliGPU GetMaliVersion(std::string_view version) { | ||
| // These names are usually Mali-VERSION or Mali-Version-EXTRA_CRAP. | ||
| auto dash_pos = version.find("-"); |
There was a problem hiding this comment.
Here I would search for 'Mali-' just to be as specific as possible. Are the version strings always at the start of version? You could also be super pedantic and check that it starts at position 0.
Could we also just use regex?
std::regex reg("^Mali-([\w]+)(-[\w]+)?");
std::smatch match;
if (std::regex_search(version, match, re) && match.size() > 1) {
const auto& result = kMaliVersions.find(match::str(1));
if (result == kMaliVersions.end()) {
return MaliGPU::kUnknown;
}
}There was a problem hiding this comment.
There was a problem hiding this comment.
I'm terrified of c++ regex. Though the google c++ style guide says not to use them, that is only because they have a better alternative in RE2 (see https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.md?cl=head )
There was a problem hiding this comment.
I changed this to match more of the string.
jonahwilliams
added
the
autosubmit
…156356) flutter/engine@0c9d3f1...e84e303 2024-10-07 [email protected] [Impeller] add parsing of known GPU models (flutter/engine#55196) 2024-10-07 [email protected] Path clarification in Setting-up-the-Engine-development-environment.md (flutter/engine#55529) 2024-10-07 [email protected] Roll Skia from 89284b1d7eeb to 6afbd6253e66 (2 revisions) (flutter/engine#55702) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
We will need to denylist certain Vulkan drivers. This is easier to do if we have some mechanism of determining which GPU model we're currently using. This doesn't add any new denylisting, it just ads a mechanism to parse the driver name into an enum. I mean, there are only so many SoCs, right?
See also flutter/flutter#155185