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

[Deng, Emily]

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Deng,
>Emily
>Sent: Friday, December 21, 2018 5:41 PM
>To: Michel Dänzer <michel at daenzer.net>
>Cc: amd-gfx at lists.freedesktop.org
>Subject: RE: [PATCH] drm/amdgpu/virtual_dce: Need to pin the fb's bo
>
>>-----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?
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.
Which method do you think is better?
>>
>>
>>--
>>Earthling Michel Dänzer               |               http://www.amd.com
>>Libre software enthusiast             |             Mesa and X developer
>_______________________________________________
>amd-gfx mailing list
>amd-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx