[PATCH v2 1/9] drm/gem-vram: Support pinning buffers to current location

[Thomas Zimmermann]

Hi

Am 12.06.19 um 10:13 schrieb Gerd Hoffmann:
> On Tue, Jun 11, 2019 at 03:03:36PM +0200, Thomas Zimmermann wrote:
>> Pinning a buffer prevents it from being moved to a different memory
>> location. For some operations, such as buffer updates, it is not
>> important where the buffer is located. Setting the pin function's
>> pl_flag argument to 0 will pin the buffer to whereever it is stored.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem_vram_helper.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 42ad80888df7..214f54b8920b 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -224,7 +224,9 @@ EXPORT_SYMBOL(drm_gem_vram_offset);
>>   *
>>   * Pinning a buffer object ensures that it is not evicted from
>>   * a memory region. A pinned buffer object has to be unpinned before
>> - * it can be pinned to another region.
>> + * it can be pinned to another region. If the pl_flag argument is 0,
>> + * the buffer is pinned at its current location (video RAM or system
>> + * memory).
>>   *
>>   * Returns:
>>   * 0 on success, or
>> @@ -242,7 +244,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
>>  	if (gbo->pin_count)
>>  		goto out;
>>  
>> -	drm_gem_vram_placement(gbo, pl_flag);
>> +	if (pl_flag)
>> +		drm_gem_vram_placement(gbo, pl_flag);
>> +
>>  	for (i = 0; i < gbo->placement.num_placement; ++i)
>>  		gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
>>  
>> @@ -691,7 +695,7 @@ int drm_gem_vram_driver_gem_prime_pin(struct drm_gem_object *gem)
>>  {
>>  	struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
>>  
>> -	return drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>> +	return drm_gem_vram_pin(gbo, 0);

The only use case for these Prime helpers is fbdev console emulation. I
have another patch set that replaces the ast and mgag200 consoles with
generic code. During the console updates it temporarily pins the BO via
this Prime funcation, which might move the BO into scarce VRAM
unnecessarily. Can we leave it like this and add a comment explaining
the decision?

Best regards
Thomas

> Not sure this is a good idea here.  If the bo happens to be in sysram
> it can't be displayed any more.
> 
>> -	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
>> +	ret = drm_gem_vram_pin(gbo, 0);
> 
> Likewise.
> 
> cheers,
>   Gerd
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20190612/850e90a4/attachment.sig>