On 2018-12-21 10:55 a.m., Deng, Emily wrote: >> -----Original Message----- >> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Deng, >> Emily >> Sent: Friday, December 21, 2018 5:41 PM >> To: Michel Dänzer <michel@xxxxxxxxxxx> >> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx >> Subject: RE: [PATCH] drm/amdgpu/virtual_dce: Need to pin the fb's bo >> >>> -----Original Message----- >>> From: Michel Dänzer <michel@xxxxxxxxxxx> >>> Sent: Friday, December 21, 2018 5:37 PM >>> To: Deng, Emily <Emily.Deng@xxxxxxx> >>> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx >>> Subject: Re: [PATCH] drm/amdgpu/virtual_dce: Need to pin the fb's bo >>> >>> On 2018-12-21 10:32 a.m., Deng, Emily wrote: >>>>> -----Original Message----- >>>>> From: Michel Dänzer <michel@xxxxxxxxxxx> >>>>> Sent: Friday, December 21, 2018 5:28 PM >>>>> To: Deng, Emily <Emily.Deng@xxxxxxx> >>>>> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx >>>>> Subject: Re: [PATCH] drm/amdgpu/virtual_dce: Need to pin the fb's bo >>>>> >>>>> On 2018-12-21 10:17 a.m., Deng, Emily wrote: >>>>>>> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of >>>>>>> Deng, Emily >>>>>>>> From: Michel Dänzer <michel@xxxxxxxxxxx> On 2018-12-21 9:45 a.m., >>>>>>>> Deng, Emily wrote: >>>>>>>>>> From: Michel Dänzer <michel@xxxxxxxxxxx> On 2018-12-21 8:26 >>>>>>>>>> a.m., Emily Deng wrote: >>>>>>>>>>> When the bo is used to set mode, the bo need to be pinned. >>>>>>>>>> >>>>>>>>>> On second thought, why does the BO need to be pinned? When >>>>>>>>>> using the display hardware, the BO needs to be pinned to >>>>>>>>>> prevent it from being moved while the hardware is scanning out >>>>>>>>>> from it, but that shouldn't be >>>>>>>> necessary here. >>>>>>>>> The pin here is used for scan out the buffer by remote display app. >>>>>>>> >>>>>>>> I still don't understand why pinning is needed. What mechanism >>>>>>>> does the remote display app use to access the BO contents? >>>>>>> Sorry, I am not familiar with the remote display app. Maybe it >>>>>>> will use drm ioctl function to get the current crtc's fb's >>>>>>> information, and get the content in the fb's buffer object by mmap >>>>>>> or translate the bo to an OpenGL texture for next rendering. Maybe >>>>>>> don't need to pin the bo here, as the use has no different with other >> normal bos. >>>>>>> So please ignore the patch, and will send another patch to remove >>>>>>> the unpin >>>>> the fb's bo code. >>>>>> It seems to be hard to remove all the pin for virtual_dce, as it >>>>>> uses some >>>>> common code in amdgpu_display.c. >>>>> >>>>> Because of amdgpu_display_unpin_work_func? That might be as simple >>>>> as replacing >>>>> >>>>> schedule_work(&works->unpin_work); >>>>> >>>>> with >>>>> >>>>> kfree(works->shared); >>>>> kfree(works); >>>>> >>>>> in dce_virtual_pageflip. >>>> But the amdgpu_display_crtc_page_flip_target will pin the new_bo, >>>> then we don't need to unpin it? >>> >>> Ah, right, but then dce_virtual_pageflip could just unpin it? Not >>> ideal, but better than leaving it pinned unnecessarily. >> Yes, it is not a good idea to leave it pinned. Then will need lots of "if >> (amdgpu_virtual_display)", don't know whether it could be accept? Should rather be if (adev->enable_virtual_display). If you want to never pin, it's probably worth giving this a shot and seeing how bad it gets. BTW, amdgpu_ttm_alloc_gart can probably also be skipped for virtual display, maybe more. > Another method is let the logical stay no change, just use if (!amdgpu_virtual_display) > before WARN_ON_ONCE of amdgpu_bo_unpin to remove the virtual_dce's call trace. That would be ugly IMHO. But none of that is necessary if dce_virtual_pageflip simply unpins the BO and skips unpin_work. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
