Den 26.07.2017 14.05, skrev Emil Velikov:
Hi Noralf, On 23 July 2017 at 20:16, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:Add a common drm_driver.dumb_map_offset function for GEM backed drivers. Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx> --- drivers/gpu/drm/drm_gem.c | 35 +++++++++++++++++++++++++++++++++++ include/drm/drm_gem.h | 2 ++ 2 files changed, 37 insertions(+) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 5df028a..a8d396b 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -311,6 +311,41 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) EXPORT_SYMBOL(drm_gem_handle_delete); /** + * drm_gem_dumb_map_offset - return the fake mmap offset for a gem object + * @file: drm file-private structure containing the gem object + * @dev: corresponding drm_device + * @handle: gem object handle + * @offset: return location for the fake mmap offset + * + * This implements the &drm_driver.dumb_map_offset kms driver callback for + * drivers which use gem to manage their backing storage. + * + * Returns: + * 0 on success or a negative error code on failure. + */ +int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, + u32 handle, u64 *offset) +{ + struct drm_gem_object *obj; + int ret; + + obj = drm_gem_object_lookup(file, handle); + if (!obj) + return -ENOENT; + + ret = drm_gem_create_mmap_offset(obj);With later patches one goes to reuse this helper instead of drm_gem_cma_dumb_map_offset(). At the same time, the latter does not have the drm_gem_create_mmap_offset() call we see here.
You're rigth, the cma library and a couple of other drivers if I recall correctly, call drm_gem_create_mmap_offset() during object creation. Daniel was of the opinion that this should happen when mmap is called. drm_gem_create_mmap_offset() is idempotent as the docs call it. Calling it a second time is ok, since it does nothing (needs to take a lock though). I haven't removed drm_gem_create_mmap_offset() from the drivers that do it during object creation, trying to keep this from doing to much. The cma library will be changed though when I add the generic fb/gem helper.
Meanwhile some drivers have their own offset function which is virtually identical to the original drm_gem_cma_dumb_map_offset. Yet those are left unchanged. For example in cirrus [1]: There the only code difference seems to be an "upcast" followed immediately by a "downcast". Which should effectively be a noop.
The main reason for that is to try and keep this simple so I'm able to add my shmem/gem library in reasonable time :-) I didn't want to do changes that wasn't straight forward. But yes, the cirrus drivers looks quite straight forward and the functions isn't used by other parts of the driver. I looked at a driver (omap?) that had similar functions, and those where called from other parts of the driver, so I expected the same here I guess. You say "some drivers", can you name them? Some drivers takes locks or do other stuff that made me skip them. Noralf.
That said, I could be loosing my marbles. HTH Emil [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/cirrus/cirrus_main.c#n292
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
