Resolves Inconsistent HWDRM Error Codes From AMD GPUs [chromium/src : main]
Dale Curtis (Gerrit)
Dale Curtis voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dale Curtis
| Code-Review | +1 |
1 comment
gpu::GPUInfo gpu_info;This may be expensive to collect?
Related details
- Sangbaek Park
- Tochukwu Ibe-Ekeocha
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaohan Wang (Gerrit)
Xiaohan Wang added 1 comment![]( "Open in Gerrit")
Related details
- Sangbaek Park
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sangbaek Park (Gerrit)
Sangbaek Park added 1 comment![]( "Open in Gerrit")
gpu::GPUInfo gpu_info;This may be expensive to collect?
Maybe this saves some?
```
#include "content/public/browser/gpu_data_manager.h"
const gpu::GPUInfo& gpu_info = content::GpuDataManager::GetInstance()->GetGPUInfo();
```
Related details
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tochukwu Ibe-Ekeocha (Gerrit)
Tochukwu Ibe-Ekeocha added 1 comment![]( "Open in Gerrit")
Sangbaek ParkThis may be expensive to collect?
Maybe this saves some?
```
#include "content/public/browser/gpu_data_manager.h"
const gpu::GPUInfo& gpu_info = content::GpuDataManager::GetInstance()->GetGPUInfo();
```
Looks like in order to use GpuDataManager I would need to add a dependency on the `/content` layer into `/media`. Currently no such dependency exists. After testing `gpu::CollectBasicGraphicsInfo` a couple times using `TimeDelta`, it seems to be taking `<= ~4ms` to get the info. Keeping in mind that this only runs when we are in an error state, do you think this cost outweighs is significant enough to consinder adding the `/content` layer as a dependency to `/media`?
Related details
- Dale Curtis
- Sangbaek Park
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 3 comments![]( "Open in Gerrit")
if (!d3d11_device) {DCHECK instead since you check it below.
gpu::GPUInfo gpu_info;Sangbaek ParkThis may be expensive to collect?
Tochukwu Ibe-EkeochaMaybe this saves some?
```
#include "content/public/browser/gpu_data_manager.h"
const gpu::GPUInfo& gpu_info = content::GpuDataManager::GetInstance()->GetGPUInfo();
```
Looks like in order to use GpuDataManager I would need to add a dependency on the `/content` layer into `/media`. Currently no such dependency exists. After testing `gpu::CollectBasicGraphicsInfo` a couple times using `TimeDelta`, it seems to be taking `<= ~4ms` to get the info. Keeping in mind that this only runs when we are in an error state, do you think this cost outweighs is significant enough to consinder adding the `/content` layer as a dependency to `/media`?
//media can't depend on //content since that'd be a layering violation.
if (d3d11_device) {Have you seen this fail? The impl in CollectBasicGraphicsInfo() looks similar, so I'd assume it would fail the same way if this does.
Related details
- Sangbaek Park
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tochukwu Ibe-Ekeocha (Gerrit)
Tochukwu Ibe-Ekeocha added 1 comment![]( "Open in Gerrit")
if (d3d11_device) {Have you seen this fail? The impl in CollectBasicGraphicsInfo() looks similar, so I'd assume it would fail the same way if this does.
The idea of using CollectBasicGraphicsInfo as a fall back is to try and account for any cases where the dxgi_device_manager_ may not be properly initialized when `MediaFoundationRenderer::OnError` is called like if this error occurs during initialization or audio-only playback. Not 100% sure that these are valid edge cases, but I figured better safe than sorry.
Related details
- Dale Curtis
- Sangbaek Park
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tochukwu Ibe-Ekeocha (Gerrit)
Tochukwu Ibe-Ekeocha added 1 comment![]( "Open in Gerrit")
DCHECK instead since you check it below.
changed
Related details
- Dale Curtis
- Sangbaek Park
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 2 comments![]( "Open in Gerrit")
constexpr uint32_t KAmdVendorId = 0x1002;`kAmdVendorId`
if (d3d11_device) {Tochukwu Ibe-EkeochaHave you seen this fail? The impl in CollectBasicGraphicsInfo() looks similar, so I'd assume it would fail the same way if this does.
The idea of using CollectBasicGraphicsInfo as a fall back is to try and account for any cases where the dxgi_device_manager_ may not be properly initialized when `MediaFoundationRenderer::OnError` is called like if this error occurs during initialization or audio-only playback. Not 100% sure that these are valid edge cases, but I figured better safe than sorry.
Is it possible to get this display related error code during audio only playback?
If so this seems fine, I just meant CollectBasicGraphicsInfo() basically does what GetVendorIdFromD3D11Device() does in terms of asking the DXGI device, so if GetVendorIdFromD3D11Device() fails or a DXGI device is in a bad state, I'm not sure it'll work any better.
I'd probably just ignore this case for now unless you really think it can happen, but defer to your expertise.
Related details
- Sangbaek Park
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |