[ Adding the dri-devel list ]
On 24/10/17 04:58 PM, Andrey Grodzovsky wrote:
> On 10/24/2017 10:36 AM, S, Shirish wrote:
>> On 10/24/2017 7:48 PM, Andrey Grodzovsky wrote:
>>> On 10/24/2017 09:51 AM, S, Shirish wrote:
>>>> From: Shirish S <shirish.s at amd.com>
>>>>
>>>> On some systems amdgpu_bo_gpu_offset seems to be
>>>> called before the BO is pinned, leading to page faults
>>>> as the GPU address is still subject to change.
>>>>
>>>> This patch fixes the above issue.
>>>>
>>>> Signed-off-by: Shirish S <shirish.s at amd.com>
>>>> ---
>>>> Â drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++--
>>>> Â 1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> index 974ee97..b251b7e 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>>> @@ -1786,8 +1786,11 @@ static int get_fb_info(const struct
>>>> amdgpu_framebuffer *amdgpu_fb,
>>>> Â Â Â Â Â Â Â Â Â return r;
>>>> Â Â Â Â Â }
>>>> Â -Â Â Â if (fb_location)
>>>> -Â Â Â Â Â Â Â *fb_location = amdgpu_bo_gpu_offset(rbo);
>>>> +Â Â Â r = amdgpu_bo_pin(rbo, AMDGPU_GEM_DOMAIN_VRAM, fb_location);
>>>> +Â Â Â if (unlikely(r != 0)) {
>>>> +Â Â Â Â Â Â Â amdgpu_bo_unreserve(rbo);
>>>> +Â Â Â Â Â Â Â return -EINVAL;
>>>> +Â Â Â }
>>>
>>> Where the unpin would happen then ? Today DAL calls pin from
>>> dm_plane_helper_prepare_fb while the unpin from
>>> dm_plane_helper_cleanup_fb.
>>>
>> Good point, am trying to address the issue Michel is facing, if this
>> patch works, we can think of unpinning the bo appropriately.
>> Ideally it should be at the end of amdgpu_dm_atomic_check(). Can you
>> think of any better place?
>
> Sounds right, you have to unpin within atomic_check since UMD might call
> check without commit sometimes.
Not sure it's appropriate to pin BOs from atomic_check.
In general, BOs should be pinned just before the display hardware starts
accessing them, and unpinned just after it stops accessing them.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
