>-----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. > >> 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. 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. Best wishes Emily Deng > > >-- >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
