[PATCH 4/7] drm/radeon: Pin buffers while they are vmap'ed
Thomas Zimmermann
tzimmermann at suse.de
Tue Nov 24 09:16:18 UTC 2020
Hi Christian
Am 16.11.20 um 12:28 schrieb Christian König:
> Am 13.11.20 um 08:59 schrieb Thomas Zimmermann:
>> Hi Christian
>>
>> Am 12.11.20 um 18:16 schrieb Christian König:
>>> Am 12.11.20 um 14:21 schrieb Thomas Zimmermann:
>>>> In order to avoid eviction of vmap'ed buffers, pin them in their GEM
>>>> object's vmap implementation. Unpin them in the vunmap implementation.
>>>> This is needed to make generic fbdev support work reliably. Without,
>>>> the buffer object could be evicted while fbdev flushed its shadow
>>>> buffer.
>>>>
>>>> In difference to the PRIME pin/unpin functions, the vmap code does not
>>>> modify the BOs prime_shared_count, so a vmap-pinned BO does not
>>>> count as
>>>> shared.
>>>>
>>>> The actual pin location is not important as the vmap call returns
>>>> information on how to access the buffer. Callers that require a
>>>> specific location should explicitly pin the BO before vmapping it.
>>> Well is the buffer supposed to be scanned out?
>> No, not by the fbdev helper.
>
> Ok in this case that should work.
>
>>> If yes then the pin location is actually rather important since the
>>> hardware can only scan out from VRAM.
>> For relocatable BOs, fbdev uses a shadow buffer that makes all any
>> relocation transparent to userspace. It flushes the shadow fb into the
>> BO's memory if there are updates. The code is in
>> drm_fb_helper_dirty_work(). [1] During the flush operation, the vmap
>> call now pins the BO to wherever it is. The actual location does not
>> matter. It's vunmap'ed immediately afterwards.
>
> The problem is what happens when it is prepared for scanout, but can't
> be moved to VRAM because it is vmapped?
>
> When the shadow is never scanned out that isn't a problem, but we need
> to keep that in mind.
>
I'd like ask for your suggestions before sending an update for this patch.
After the discussion about locking in fbdev, [1] I intended to replace
the pin call with code that acquires the reservation lock.
First I wanted to put this into drm_gem_ttm_vmap/vunmap(), but then
wondered why ttm_bo_vmap() doe not acquire the lock internally? I'd
expect that vmap/vunmap are close together and do not overlap for the
same BO. Otherwise, acquiring the reservation lock would require another
ref-counting variable or per-driver code.
Best regards
Thomas
[1] https://patchwork.freedesktop.org/patch/401088/?series=83918&rev=1
> Regards,
> Christian.
>
>>
>> For dma-buf sharing, the regular procedure of pin + vmap still apply.
>> This should always move the BO into GTT-managed memory.
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fgpu%2Fdrm%2Fdrm_fb_helper.c%23n432&data=04%7C01%7Cchristian.koenig%40amd.com%7C31b890664ca7429fc45808d887aa0842%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408511650629569%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RLauuAuXkcl0rXwWWJ%2FrKP%2BsCr2wAzU1ejGV1bnQ80w%3D&reserved=0
>>
>>
>>> Regards,
>>> Christian.
>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>>> ---
>>>> drivers/gpu/drm/radeon/radeon_gem.c | 51
>>>> +++++++++++++++++++++++++++--
>>>> 1 file changed, 49 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
>>>> b/drivers/gpu/drm/radeon/radeon_gem.c
>>>> index d2876ce3bc9e..eaf7fc9a7b07 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_gem.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
>>>> @@ -226,6 +226,53 @@ static int radeon_gem_handle_lockup(struct
>>>> radeon_device *rdev, int r)
>>>> return r;
>>>> }
>>>> +static int radeon_gem_object_vmap(struct drm_gem_object *obj,
>>>> struct dma_buf_map *map)
>>>> +{
>>>> + static const uint32_t any_domain = RADEON_GEM_DOMAIN_VRAM |
>>>> + RADEON_GEM_DOMAIN_GTT |
>>>> + RADEON_GEM_DOMAIN_CPU;
>>>> +
>>>> + struct radeon_bo *bo = gem_to_radeon_bo(obj);
>>>> + int ret;
>>>> +
>>>> + ret = radeon_bo_reserve(bo, false);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + /* pin buffer at its current location */
>>>> + ret = radeon_bo_pin(bo, any_domain, NULL);
>>>> + if (ret)
>>>> + goto err_radeon_bo_unreserve;
>>>> +
>>>> + ret = drm_gem_ttm_vmap(obj, map);
>>>> + if (ret)
>>>> + goto err_radeon_bo_unpin;
>>>> +
>>>> + radeon_bo_unreserve(bo);
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_radeon_bo_unpin:
>>>> + radeon_bo_unpin(bo);
>>>> +err_radeon_bo_unreserve:
>>>> + radeon_bo_unreserve(bo);
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void radeon_gem_object_vunmap(struct drm_gem_object *obj,
>>>> struct dma_buf_map *map)
>>>> +{
>>>> + struct radeon_bo *bo = gem_to_radeon_bo(obj);
>>>> + int ret;
>>>> +
>>>> + ret = radeon_bo_reserve(bo, false);
>>>> + if (ret)
>>>> + return;
>>>> +
>>>> + drm_gem_ttm_vunmap(obj, map);
>>>> + radeon_bo_unpin(bo);
>>>> + radeon_bo_unreserve(bo);
>>>> +}
>>>> +
>>>> static const struct drm_gem_object_funcs radeon_gem_object_funcs = {
>>>> .free = radeon_gem_object_free,
>>>> .open = radeon_gem_object_open,
>>>> @@ -234,8 +281,8 @@ static const struct drm_gem_object_funcs
>>>> radeon_gem_object_funcs = {
>>>> .pin = radeon_gem_prime_pin,
>>>> .unpin = radeon_gem_prime_unpin,
>>>> .get_sg_table = radeon_gem_prime_get_sg_table,
>>>> - .vmap = drm_gem_ttm_vmap,
>>>> - .vunmap = drm_gem_ttm_vunmap,
>>>> + .vmap = radeon_gem_object_vmap,
>>>> + .vunmap = radeon_gem_object_vunmap,
>>>> };
>>>> /*
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=04%7C01%7Cchristian.koenig%40amd.com%7C31b890664ca7429fc45808d887aa0842%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637408511650629569%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=h1U9Po83K7webxsiKpn3ZGFz9Fcg6SRkxtrXWZ1%2B%2FEc%3D&reserved=0
>>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x680DC11D530B7A23.asc
Type: application/pgp-keys
Size: 7435 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20201124/0cb426ce/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20201124/0cb426ce/attachment.sig>
More information about the amd-gfx
mailing list