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.
-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)
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
