[PATCH] drm/amd/display: assign fb_location only if bo is pinned

[Andrey Grodzovsky]

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.

Andrey

> Regards,
> Shirish S
>> Thanks,
>> Andrey
>>
>>>         if (tiling_flags)
>>>           amdgpu_bo_get_tiling_flags(rbo, tiling_flags);
>>
>