[PATCH v3 2/3] drm/vram: Add infrastructure for move_notify()

[Thomas Zimmermann]

Hi

Am 06.09.19 um 11:28 schrieb Daniel Vetter:
> On Fri, Sep 06, 2019 at 10:52:13AM +0200, Thomas Zimmermann wrote:
>> This patch prepares VRAM helpers for lazy unmapping of buffer objects.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>  drivers/gpu/drm/drm_vram_mm_helper.c | 12 ++++++++++++
>>  include/drm/drm_vram_mm_helper.h     |  4 ++++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_vram_mm_helper.c b/drivers/gpu/drm/drm_vram_mm_helper.c
>> index c911781d6728..31984690d5f3 100644
>> --- a/drivers/gpu/drm/drm_vram_mm_helper.c
>> +++ b/drivers/gpu/drm/drm_vram_mm_helper.c
>> @@ -98,6 +98,17 @@ static int bo_driver_verify_access(struct ttm_buffer_object *bo,
>>  	return vmm->funcs->verify_access(bo, filp);
>>  }
>>  
>> +static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>> +				  bool evict,
>> +				  struct ttm_mem_reg *new_mem)
>> +{
>> +	struct drm_vram_mm *vmm = drm_vram_mm_of_bdev(bo->bdev);
>> +
>> +	if (!vmm->funcs || !vmm->funcs->move_notify)
>> +		return;
>> +	vmm->funcs->move_notify(bo, evict, new_mem);
>> +}
>> +
>>  static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
>>  				    struct ttm_mem_reg *mem)
>>  {
>> @@ -140,6 +151,7 @@ static struct ttm_bo_driver bo_driver = {
>>  	.eviction_valuable = ttm_bo_eviction_valuable,
>>  	.evict_flags = bo_driver_evict_flags,
>>  	.verify_access = bo_driver_verify_access,
>> +	.move_notify = bo_driver_move_notify,
>>  	.io_mem_reserve = bo_driver_io_mem_reserve,
>>  	.io_mem_free = bo_driver_io_mem_free,
>>  };
>> diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h
>> index 2aacfb1ccfae..7fb8700f45fe 100644
>> --- a/include/drm/drm_vram_mm_helper.h
>> +++ b/include/drm/drm_vram_mm_helper.h
>> @@ -15,6 +15,8 @@ struct drm_device;
>>  	&ttm_bo_driver.evict_flags
>>   * @verify_access:	Provides an implementation for \
>>  	struct &ttm_bo_driver.verify_access
>> + * @move_notify:	Provides an implementation for
>> + *			struct &ttm_bo_driver.move_notify
>>   *
>>   * These callback function integrate VRAM MM with TTM buffer objects. New
>>   * functions can be added if necessary.
>> @@ -23,6 +25,8 @@ struct drm_vram_mm_funcs {
>>  	void (*evict_flags)(struct ttm_buffer_object *bo,
>>  			    struct ttm_placement *placement);
>>  	int (*verify_access)(struct ttm_buffer_object *bo, struct file *filp);
>> +	void (*move_notify)(struct ttm_buffer_object *bo, bool evict,
>> +			    struct ttm_mem_reg *new_mem);
> 
> Is this indirection really worth it? We have a grand total of 1 driver
> which isn't using gem (vmwgfx), and I don't think that one will ever
> switch over to vram helpers.
> 
> I'd fold that all in. Helpers don't need to be flexible enough to support
> every possible use-case (that's just the job of the core), they can be
> opinionated to get cleaner code for most cases.
> 

The original idea was to make this as flexible as possible; probably
more flexible than necessary. I wouldn't mind merging everything into
one file, but please not in this patch set, can we keep it for now and I
send you a cleanup next?

Best regards
Thomas

> For this case here if you fold this in you can unexport 3 EXPORT_SYMBOLs
> (4 with this patch here), which I think is a neat simplification of the
> complexity exposed to driver writers. Just put the necessary declarations
> into a drm_vram_helper_internal.h so that drivers don't get at it by
> accident (like the other drm*internal.h helpers we have).
> -Daniel
> 
>>  };
>>  
>>  /**
>> -- 
>> 2.23.0
>>
> 

-- 
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/20190906/3d2ebb08/attachment.sig>