gfxstream: export a ColorBuffer's memory regardless of ExternalBlob - #178
Open
jvle wants to merge 1 commit into
Open
gfxstream: export a ColorBuffer's memory regardless of ExternalBlob#178jvle wants to merge 1 commit into
jvle wants to merge 1 commit into
Conversation
ExternalBlob was gating two unrelated things: whether host-visible
memory travels as a descriptor vs. a mapping, and whether a
ColorBuffer's memory gets exported at all. With the flag off (needed
for SystemBlob on fatcat), vkGetBlobInternal never registered a
ColorBuffer's descriptor, and VirtioGpuResource::Create only knew how
to look up a mapping -- so the scanout blob had neither, and the guest
saw "no external blob mapping" / "unknown resource" for every frame.
Widen vkGetBlobInternal's export branch to also fire when the memory
backs a ColorBuffer (info->boundColorBuffer), independent of the flag.
Make VirtioGpuResource::Create probe for a registered descriptor
before falling back to a mapping, instead of picking one or the other
by the flag. The blob_id==0 ring path and the guest-handle sub-branch
are untouched -- those are real transport choices, not scanout.
Bug: 545345381
Test: fatcat, ExternalBlob:disabled -- Weston reaches its GL renderer,
desktop and app windows composite correctly via AHB-FLIP, zero
GFXSTREAM blob/resource errors in the boot log
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the point @gurchetansingh raised on the ARSP review:
ExternalBlobwasdoing two unrelated jobs — picking the map_blob transport (descriptor vs. host-visible
mapping) for ordinary memory, and gating whether a ColorBuffer's memory gets exported
for scanout at all. Those need to be independent.
The bug
With
ExternalBloboff (we need it off soSystemBlobcan carry host-visible memory —VulkanAllocateHostVisibleAsUdmabuffails on newer hosts, and plainExternalBlobexports as an AHardwareBuffer handle that crosvm's
resource_map_blob()rejects),vkGetBlobInternalnever registered a ColorBuffer's descriptor, andVirtioGpuResource::Create's blob path only knew how to look up a mapping. Thescanout blob had neither, so every frame failed:
Fix
vkGetBlobInternal: export+register the descriptor whenever the memory backs aColorBuffer (
info->boundColorBuffer), independent of the flag.VirtioGpuResource::Create: probe for a registered descriptor first, then fall backto a mapping, instead of picking one or the other by the flag.
The
blob_id==0ring path and the guest-handle sub-branch are untouched — those arereal transport choices, not scanout.
Tested on Intel PTL (fatcat) both with
ExternalBlob:disabled(the case this fixes —Weston reaches its GL renderer, desktop composites via AHB-FLIP, no blob/resource
errors) and with the shipping
SystemBlob:enabled;ExternalBlob:enabledconfig (noregression).