Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vaPutSurface] do not reuse previous vpp context #944

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 1 addition & 12 deletions media_driver/linux/common/ddi/media_libva.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,7 @@ VAStatus DdiMedia__Initialize (
// return VA_STATUS_ERROR_UNIMPLEMENTED directly.
ctx->vtable->vaPutSurface = NULL;
}
mediaCtx->PutSurfaceContext = VA_INVALID_ID;
output_dri_init(ctx);
#endif

Expand Down Expand Up @@ -3961,21 +3962,9 @@ static VAStatus DdiMedia_PutSurface(

DDI_CHK_LESS((uint32_t)surface, mediaDrvCtx->pSurfaceHeap->uiAllocatedHeapElements, "Invalid surface", VA_STATUS_ERROR_INVALID_SURFACE);

if (nullptr != mediaDrvCtx->pVpCtxHeap->pHeapBase)
{
uint32_t ctxType = DDI_MEDIA_CONTEXT_TYPE_NONE;
vpCtx = DdiMedia_GetContextFromContextID(ctx, (VAContextID)(0 + DDI_MEDIA_VACONTEXTID_OFFSET_VP), &ctxType);
}

#if defined(ANDROID) || !defined(X11_FOUND)
return VA_STATUS_ERROR_UNIMPLEMENTED;
#else
if(nullptr == vpCtx)
{
VAContextID context = VA_INVALID_ID;
VAStatus vaStatus = DdiVp_CreateContext(ctx, 0, 0, 0, 0, 0, 0, &context);
DDI_CHK_RET(vaStatus, "Create VP Context failed");
}
return DdiCodec_PutSurfaceLinuxHW(ctx, surface, draw, srcx, srcy, srcw, srch, destx, desty, destw, desth, cliprects, number_cliprects, flags);
#endif

Expand Down
1 change: 1 addition & 0 deletions media_driver/linux/common/ddi/media_libva_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ struct DDI_MEDIA_CONTEXT
//vpgPutSurfaceLinuxHW acceleration hack
MEDIA_MUTEX_T PutSurfaceRenderMutex;
MEDIA_MUTEX_T PutSurfaceSwapBufferMutex;
VAContextID PutSurfaceContext;
#endif
bool apoMosEnabled;
};
Expand Down
21 changes: 11 additions & 10 deletions media_driver/linux/common/ddi/media_libva_putsurface_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -399,18 +399,19 @@ VAStatus DdiCodec_PutSurfaceLinuxHW(
DdiMediaUtil_MediaPrintFps();
pitch = bufferObject->iPitch;

vpCtx = nullptr;
if (nullptr != mediaCtx->pVpCtxHeap->pHeapBase)
{
vpCtx = (PDDI_VP_CONTEXT)DdiMedia_GetContextFromContextID(ctx, (VAContextID)(0 + DDI_MEDIA_VACONTEXTID_OFFSET_VP), &ctxType);
DDI_CHK_NULL(vpCtx, "Null vpCtx", VA_STATUS_ERROR_INVALID_PARAMETER);
vpHal = vpCtx->pVpHal;
DDI_CHK_NULL(vpHal, "Null vpHal", VA_STATUS_ERROR_INVALID_PARAMETER);
}
else
DdiMediaUtil_LockMutex(&mediaCtx->PutSurfaceRenderMutex);
if (mediaCtx->PutSurfaceContext == VA_INVALID_ID)
{
return VA_STATUS_ERROR_INVALID_CONTEXT;
VAStatus vaStatus = DdiVp_CreateContext(ctx, 0, 0, 0, 0, 0, 0, &mediaCtx->PutSurfaceContext);
DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);
DDI_CHK_RET(vaStatus, "Create VP Context failed");
} else {
DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);
}
vpCtx = (PDDI_VP_CONTEXT)DdiMedia_GetContextFromContextID(ctx, mediaCtx->PutSurfaceContext, &ctxType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move these lines into mutex protection part considering here vphal is used? Thanks.
DdiMediaUtil_LockMutex(&mediaCtx->PutSurfaceRenderMutex);
eStatus = vpHal->Render(&renderParams);
if (MOS_FAILED(eStatus))
{
DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);
mos_bo_unreference(drawable_bo);
return VA_STATUS_ERROR_OPERATION_FAILED;
}

DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Furong,
Thanks for the review. It may not needed.
Above lock just protect us for context creation. Once it created, it only destroy at uninitialized time.
The DdiMedia_GetContextFromContextID will hold a lock, and later operation like "vpHal->Render(&renderParams);" is protected by lock too.
So the entire process is thread safe.

DDI_CHK_NULL(vpCtx, "Null vpCtx", VA_STATUS_ERROR_INVALID_PARAMETER);
vpHal = vpCtx->pVpHal;
DDI_CHK_NULL(vpHal, "Null vpHal", VA_STATUS_ERROR_INVALID_PARAMETER);

// Zero memory
MOS_ZeroMemory(&Surf, sizeof(Surf));
Expand Down