On 2018-12-21 11:15 a.m., Deng, Emily wrote: >> -----Original Message----- >> From: Michel Dänzer <michel@xxxxxxxxxxx> >> Sent: Friday, December 21, 2018 6:08 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: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. > Ok, then I will try, but the code may be ugly. > > [...] > >> But none of that is necessary if dce_virtual_pageflip simply unpins the BO and >> skips unpin_work. > Yes, but it will still have the issue that it won't unpin the bo which is pinned by amdgpu_display_crtc_page_flip_target. I mean dce_virtual_pageflip unpinning precisely the BO pinned by amdgpu_display_crtc_page_flip_target. -- 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
