| Message ID | 20190628122659.31887-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert VRAM helpers to GEM object functions | expand |
On Fri, Jun 28, 2019 at 02:26:56PM +0200, Thomas Zimmermann wrote: > PRIME functionality is now provided via the callback functions in > struct drm_gem_object_funcs. The driver-structure functions are obsolete. > As a side effect of this patch, VRAM-based drivers get basic PRIME > support automatically without having to set any flags or additional > fields. > +static void drm_gem_vram_object_free(struct drm_gem_object *gem) > +static int drm_gem_vram_object_funcs_pin(struct drm_gem_object *gem) > +static void drm_gem_vram_object_funcs_unpin(struct drm_gem_object *gem) > +static void *drm_gem_vram_object_funcs_vmap(struct drm_gem_object *gem) > +static void drm_gem_vram_object_funcs_vunmap(struct drm_gem_object *gem, > + void *vaddr) > +static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { > + .free = drm_gem_vram_object_free, > + .pin = drm_gem_vram_object_funcs_pin, > + .unpin = drm_gem_vram_object_funcs_unpin, > + .vmap = drm_gem_vram_object_funcs_vmap, > + .vunmap = drm_gem_vram_object_funcs_vunmap > +}; Why new functions? Can't you just hook up the existing prime functions? cheers, Gerd
Hi Am 01.07.19 um 08:32 schrieb Gerd Hoffmann: > On Fri, Jun 28, 2019 at 02:26:56PM +0200, Thomas Zimmermann wrote: >> PRIME functionality is now provided via the callback functions in >> struct drm_gem_object_funcs. The driver-structure functions are obsolete. >> As a side effect of this patch, VRAM-based drivers get basic PRIME >> support automatically without having to set any flags or additional >> fields. > >> +static void drm_gem_vram_object_free(struct drm_gem_object *gem) >> +static int drm_gem_vram_object_funcs_pin(struct drm_gem_object *gem) >> +static void drm_gem_vram_object_funcs_unpin(struct drm_gem_object *gem) >> +static void *drm_gem_vram_object_funcs_vmap(struct drm_gem_object *gem) >> +static void drm_gem_vram_object_funcs_vunmap(struct drm_gem_object *gem, >> + void *vaddr) > >> +static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { >> + .free = drm_gem_vram_object_free, >> + .pin = drm_gem_vram_object_funcs_pin, >> + .unpin = drm_gem_vram_object_funcs_unpin, >> + .vmap = drm_gem_vram_object_funcs_vmap, >> + .vunmap = drm_gem_vram_object_funcs_vunmap >> +}; > > Why new functions? Can't you just hook up the existing prime functions? The final patch will remove the existing functions, so drivers won't use them accidentally. Best regards Thomas > > cheers, > Gerd >
On Mon, Jul 01, 2019 at 09:28:59AM +0200, Thomas Zimmermann wrote: > Hi > > Am 01.07.19 um 08:32 schrieb Gerd Hoffmann: > > On Fri, Jun 28, 2019 at 02:26:56PM +0200, Thomas Zimmermann wrote: > >> PRIME functionality is now provided via the callback functions in > >> struct drm_gem_object_funcs. The driver-structure functions are obsolete. > >> As a side effect of this patch, VRAM-based drivers get basic PRIME > >> support automatically without having to set any flags or additional > >> fields. > > > >> +static void drm_gem_vram_object_free(struct drm_gem_object *gem) > >> +static int drm_gem_vram_object_funcs_pin(struct drm_gem_object *gem) > >> +static void drm_gem_vram_object_funcs_unpin(struct drm_gem_object *gem) > >> +static void *drm_gem_vram_object_funcs_vmap(struct drm_gem_object *gem) > >> +static void drm_gem_vram_object_funcs_vunmap(struct drm_gem_object *gem, > >> + void *vaddr) > > > >> +static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { > >> + .free = drm_gem_vram_object_free, > >> + .pin = drm_gem_vram_object_funcs_pin, > >> + .unpin = drm_gem_vram_object_funcs_unpin, > >> + .vmap = drm_gem_vram_object_funcs_vmap, > >> + .vunmap = drm_gem_vram_object_funcs_vunmap > >> +}; > > > > Why new functions? Can't you just hook up the existing prime functions? > > The final patch will remove the existing functions, so drivers won't use > them accidentally. But the new and the old ones are identical, right? So why add/remove? Why not just rename them? I'd also suggest to name them consistently (free has no _funcs, all others have). I'd drop _funcs from all function names. cheers, Gerd
Hi Am 01.07.19 um 10:48 schrieb Gerd Hoffmann: > On Mon, Jul 01, 2019 at 09:28:59AM +0200, Thomas Zimmermann wrote: >> Hi >> >> Am 01.07.19 um 08:32 schrieb Gerd Hoffmann: >>> On Fri, Jun 28, 2019 at 02:26:56PM +0200, Thomas Zimmermann wrote: >>>> PRIME functionality is now provided via the callback functions in >>>> struct drm_gem_object_funcs. The driver-structure functions are obsolete. >>>> As a side effect of this patch, VRAM-based drivers get basic PRIME >>>> support automatically without having to set any flags or additional >>>> fields. >>> >>>> +static void drm_gem_vram_object_free(struct drm_gem_object *gem) >>>> +static int drm_gem_vram_object_funcs_pin(struct drm_gem_object *gem) >>>> +static void drm_gem_vram_object_funcs_unpin(struct drm_gem_object *gem) >>>> +static void *drm_gem_vram_object_funcs_vmap(struct drm_gem_object *gem) >>>> +static void drm_gem_vram_object_funcs_vunmap(struct drm_gem_object *gem, >>>> + void *vaddr) >>> >>>> +static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { >>>> + .free = drm_gem_vram_object_free, >>>> + .pin = drm_gem_vram_object_funcs_pin, >>>> + .unpin = drm_gem_vram_object_funcs_unpin, >>>> + .vmap = drm_gem_vram_object_funcs_vmap, >>>> + .vunmap = drm_gem_vram_object_funcs_vunmap >>>> +}; >>> >>> Why new functions? Can't you just hook up the existing prime functions? >> >> The final patch will remove the existing functions, so drivers won't use >> them accidentally. > > But the new and the old ones are identical, right? So why add/remove? > Why not just rename them? Hmm, OK. Does that somehow make a difference (e.g., easier backporting or maintenance)? > I'd also suggest to name them consistently (free has no _funcs, all > others have). I'd drop _funcs from all function names. This inconsistency is actually an error in the patch. Thanks Best regards Thomas > cheers, > Gerd > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
Hi, > > But the new and the old ones are identical, right? So why add/remove? > > Why not just rename them? > > Hmm, OK. Does that somehow make a difference (e.g., easier backporting > or maintenance)? Easier patch review (it is obvious then you only change the way the functions are hooked up, not the actual code). A bit less code churn. cheers, Gerd
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 4de782ca26b2..dd220795e725 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -14,6 +14,73 @@ * (VRAM). It can be used for framebuffer devices with dedicated memory. */ +/* + * GEM object funcs + */ + +static void drm_gem_vram_object_free(struct drm_gem_object *gem) +{ + struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); + + drm_gem_vram_put(gbo); +} + +static int drm_gem_vram_object_funcs_pin(struct drm_gem_object *gem) +{ + struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); + + /* Fbdev console emulation is the use case of these PRIME + * helpers. This may involve updating a hardware buffer from + * a shadow FB. We pin the buffer to it's current location + * (either video RAM or system memory) to prevent it from + * being relocated during the update operation. If you require + * the buffer to be pinned to VRAM, implement a callback that + * sets the flags accordingly. + */ + return drm_gem_vram_pin(gbo, 0); +} + +static void drm_gem_vram_object_funcs_unpin(struct drm_gem_object *gem) +{ + struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); + + drm_gem_vram_unpin(gbo); +} + +static void *drm_gem_vram_object_funcs_vmap(struct drm_gem_object *gem) +{ + struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); + int ret; + void *base; + + ret = drm_gem_vram_pin(gbo, 0); + if (ret) + return NULL; + base = drm_gem_vram_kmap(gbo, true, NULL); + if (IS_ERR(base)) { + drm_gem_vram_unpin(gbo); + return NULL; + } + return base; +} + +static void drm_gem_vram_object_funcs_vunmap(struct drm_gem_object *gem, + void *vaddr) +{ + struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); + + drm_gem_vram_kunmap(gbo); + drm_gem_vram_unpin(gbo); +} + +static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = { + .free = drm_gem_vram_object_free, + .pin = drm_gem_vram_object_funcs_pin, + .unpin = drm_gem_vram_object_funcs_unpin, + .vmap = drm_gem_vram_object_funcs_vmap, + .vunmap = drm_gem_vram_object_funcs_vunmap +}; + /* * Buffer-objects helpers */ @@ -80,6 +147,9 @@ static int drm_gem_vram_init(struct drm_device *dev, int ret; size_t acc_size; + if (!gbo->gem.funcs) + gbo->gem.funcs = &drm_gem_vram_object_funcs; + ret = drm_gem_object_init(dev, &gbo->gem, size); if (ret) return ret; diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 1a0ea18e7a74..bc8fe9feee3b 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -127,7 +127,8 @@ int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file, .gem_free_object_unlocked = \ drm_gem_vram_driver_gem_free_object_unlocked, \ .dumb_create = drm_gem_vram_driver_dumb_create, \ - .dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset + .dumb_map_offset = drm_gem_vram_driver_dumb_mmap_offset, \ + .gem_prime_mmap = drm_gem_prime_mmap /* * PRIME helpers for struct drm_driver
PRIME functionality is now provided via the callback functions in struct drm_gem_object_funcs. The driver-structure functions are obsolete. As a side effect of this patch, VRAM-based drivers get basic PRIME support automatically without having to set any flags or additional fields. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_gem_vram_helper.c | 70 +++++++++++++++++++++++++++ include/drm/drm_gem_vram_helper.h | 3 +- 2 files changed, 72 insertions(+), 1 deletion(-)