[PATCH] drm/amdgpu/virtual_dce: Need to pin the fb's bo

[Deng, Emily]

>-----Original Message-----
>From: Michel Dänzer <michel at daenzer.net>
>Sent: Friday, December 21, 2018 5:37 PM
>To: Deng, Emily <Emily.Deng at amd.com>
>Cc: amd-gfx at lists.freedesktop.org
>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 at daenzer.net>
>>> Sent: Friday, December 21, 2018 5:28 PM
>>> To: Deng, Emily <Emily.Deng at amd.com>
>>> Cc: amd-gfx at lists.freedesktop.org
>>> 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 at lists.freedesktop.org> On Behalf Of
>>>>> Deng, Emily
>>>>>> From: Michel Dänzer <michel at daenzer.net> On 2018-12-21 9:45 a.m.,
>>>>>> Deng, Emily wrote:
>>>>>>>> From: Michel Dänzer <michel at daenzer.net> 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?
>
>
>--
>Earthling Michel Dänzer               |               http://www.amd.com
>Libre software enthusiast             |             Mesa and X developer