Hi
Am 06.09.19 um 15:05 schrieb Daniel Vetter:
> On Fri, Sep 6, 2019 at 12:24 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
>>
>> 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@xxxxxxx>
>>>> ---
>>>> 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?
>
> Oh sure, I just wondered about why this flexibility exists and
> realized there's not really a user for it. And pondering this more I
> didn't come up with a use-case where it might reasonably be needed,
> and you don't want to roll your own complete, and couldn't come up
> with anything. Aside from the locking question on patch 1 I think this
> looks like a really tidy solution for the fbdev mapping issue, happy
> to ack once we've figured out the locking thing.
Great. There's a v4 of the patch set that should resolve the locking
problem.
Best regards
Thomas
> -Daniel
>
>>
>> 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)
>>
>
>
--
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)
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
