From: Sean Paul seanpaul@chromium.org
When I started re-applying these I realized they hadn't hit the list.
Sean
Rob Herring (2): Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()" Revert "drm/panfrost: Use drm_gem_map_offset()"
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++------- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++++++++++++++-- include/drm/drm_gem.h | 4 ++-- 5 files changed, 23 insertions(+), 14 deletions(-)
From: Rob Herring robh@kernel.org
This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598.
Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer case, so revert the name change.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run --- drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++------- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- include/drm/drm_gem.h | 4 ++-- 4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c index b55cfc9e8772..d18a740fe0f1 100644 --- a/drivers/gpu/drm/drm_dumb_buffers.c +++ b/drivers/gpu/drm/drm_dumb_buffers.c @@ -48,7 +48,7 @@ * To support dumb objects drivers must implement the &drm_driver.dumb_create * operation. &drm_driver.dumb_destroy defaults to drm_gem_dumb_destroy() if * not set and &drm_driver.dumb_map_offset defaults to - * drm_gem_map_offset(). See the callbacks for further details. + * drm_gem_dumb_map_offset(). See the callbacks for further details. * * Note that dumb objects may not be used for gpu acceleration, as has been * attempted on some ARM embedded platforms. Such drivers really must have @@ -127,7 +127,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev, args->handle, &args->offset); else - return drm_gem_map_offset(file_priv, dev, args->handle, + return drm_gem_dumb_map_offset(file_priv, dev, args->handle, &args->offset); }
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 8cbfd60e09c0..afc38cece3f5 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -298,7 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) EXPORT_SYMBOL(drm_gem_handle_delete);
/** - * drm_gem_map_offset - return the fake mmap offset for a gem object + * 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 @@ -307,14 +307,10 @@ EXPORT_SYMBOL(drm_gem_handle_delete); * This implements the &drm_driver.dumb_map_offset kms driver callback for * drivers which use gem to manage their backing storage. * - * It can also be used by drivers using GEM BO implementations which - * have same restriction that imported objects cannot be mapped. The - * shmem backend is one example. - * * Returns: * 0 on success or a negative error code on failure. */ -int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, +int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset) { struct drm_gem_object *obj; @@ -340,7 +336,7 @@ int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
return ret; } -EXPORT_SYMBOL_GPL(drm_gem_map_offset); +EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset);
/** * drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index bf0ad8e5a02b..d734d9d51762 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -273,7 +273,8 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void *data, { struct drm_exynos_gem_map *args = data;
- return drm_gem_map_offset(file_priv, dev, args->handle, &args->offset); + return drm_gem_dumb_map_offset(file_priv, dev, args->handle, + &args->offset); }
struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp, diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 0d6445fa9541..ae693c0666cd 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -401,8 +401,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array, int drm_gem_fence_array_add_implicit(struct xarray *fence_array, struct drm_gem_object *obj, bool write); -int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, - u32 handle, u64 *offset); +int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, + u32 handle, u64 *offset); int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle);
On Wed, Aug 07, 2019 at 10:52:47AM -0400, Sean Paul wrote:
From: Rob Herring robh@kernel.org
This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598.
Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer case, so revert the name change.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run
This part of the series seems unecessary to revert?
iow I still like this, and Im trying to sell Gerd Hoffmann on it for some of his ttm refactoring ... revert of the revert of the revert of the revert? Probably better if Gerd cherry-picks into his series (if my suggestion works out) and maybe references this entire chain for entertainment purposes :-) -Daniel
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++------- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- include/drm/drm_gem.h | 4 ++-- 4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c index b55cfc9e8772..d18a740fe0f1 100644 --- a/drivers/gpu/drm/drm_dumb_buffers.c +++ b/drivers/gpu/drm/drm_dumb_buffers.c @@ -48,7 +48,7 @@
- To support dumb objects drivers must implement the &drm_driver.dumb_create
- operation. &drm_driver.dumb_destroy defaults to drm_gem_dumb_destroy() if
- not set and &drm_driver.dumb_map_offset defaults to
- drm_gem_map_offset(). See the callbacks for further details.
- drm_gem_dumb_map_offset(). See the callbacks for further details.
- Note that dumb objects may not be used for gpu acceleration, as has been
- attempted on some ARM embedded platforms. Such drivers really must have
@@ -127,7 +127,7 @@ int drm_mode_mmap_dumb_ioctl(struct drm_device *dev, args->handle, &args->offset); else
return drm_gem_map_offset(file_priv, dev, args->handle,
return drm_gem_dumb_map_offset(file_priv, dev, args->handle, &args->offset);}
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 8cbfd60e09c0..afc38cece3f5 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -298,7 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) EXPORT_SYMBOL(drm_gem_handle_delete);
/**
- drm_gem_map_offset - return the fake mmap offset for a gem object
- 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
@@ -307,14 +307,10 @@ EXPORT_SYMBOL(drm_gem_handle_delete);
- This implements the &drm_driver.dumb_map_offset kms driver callback for
- drivers which use gem to manage their backing storage.
- It can also be used by drivers using GEM BO implementations which
- have same restriction that imported objects cannot be mapped. The
- shmem backend is one example.
*/
- Returns:
- 0 on success or a negative error code on failure.
-int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev, +int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset) { struct drm_gem_object *obj; @@ -340,7 +336,7 @@ int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
return ret; } -EXPORT_SYMBOL_GPL(drm_gem_map_offset); +EXPORT_SYMBOL_GPL(drm_gem_dumb_map_offset);
/**
- drm_gem_dumb_destroy - dumb fb callback helper for gem based drivers
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index bf0ad8e5a02b..d734d9d51762 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -273,7 +273,8 @@ int exynos_drm_gem_map_ioctl(struct drm_device *dev, void *data, { struct drm_exynos_gem_map *args = data;
- return drm_gem_map_offset(file_priv, dev, args->handle, &args->offset);
- return drm_gem_dumb_map_offset(file_priv, dev, args->handle,
&args->offset);}
struct exynos_drm_gem *exynos_drm_gem_get(struct drm_file *filp, diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 0d6445fa9541..ae693c0666cd 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -401,8 +401,8 @@ int drm_gem_fence_array_add(struct xarray *fence_array, int drm_gem_fence_array_add_implicit(struct xarray *fence_array, struct drm_gem_object *obj, bool write); -int drm_gem_map_offset(struct drm_file *file, struct drm_device *dev,
u32 handle, u64 *offset);+int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
u32 handle, u64 *offset);int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev, uint32_t handle); -- Sean Paul, Software Engineer, Google / Chromium OS
On Wed, Aug 7, 2019 at 3:01 PM Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Aug 07, 2019 at 10:52:47AM -0400, Sean Paul wrote:
From: Rob Herring robh@kernel.org
This reverts commit 220df83a5394fbf7c1486ba7848794b7b351d598.
Turns out drm_gem_dumb_map_offset really only worked for the dumb buffer case, so revert the name change.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run
This part of the series seems unecessary to revert?
Already committed, so let me go revert that right now for you. I can only hope that's enough to get my commit rights revoked too. :)
iow I still like this, and Im trying to sell Gerd Hoffmann on it for some of his ttm refactoring ... revert of the revert of the revert of the revert? Probably better if Gerd cherry-picks into his series (if my suggestion works out) and maybe references this entire chain for entertainment purposes :-) -Daniel
From: Rob Herring robh@kernel.org
This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f.
Turns out we need mmap to work on imported BOs even if the current code is buggy.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run --- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index b2e325e270b7..b187daa4da85 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -291,14 +291,26 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_panfrost_mmap_bo *args = data; + struct drm_gem_object *gem_obj; + int ret;
if (args->flags != 0) { DRM_INFO("unknown mmap_bo flags: %d\n", args->flags); return -EINVAL; }
- return drm_gem_map_offset(file_priv, dev, args->handle, - &args->offset); + gem_obj = drm_gem_object_lookup(file_priv, args->handle); + if (!gem_obj) { + DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle); + return -ENOENT; + } + + ret = drm_gem_create_mmap_offset(gem_obj); + if (ret == 0) + args->offset = drm_vma_node_offset_addr(&gem_obj->vma_node); + drm_gem_object_put_unlocked(gem_obj); + + return ret; }
static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data,
On Wed, 7 Aug 2019 at 15:53, Sean Paul sean@poorly.run wrote:
From: Rob Herring robh@kernel.org
This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f.
Turns out we need mmap to work on imported BOs even if the current code is buggy.
Personally I would have mentioned a use case where imported BOs are used.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run
Regardless of the above nitpick, with the patch order fixed the series is: Reviewed-by: Emil Velikov emil.velikov@collabora.com
... in case you haven't picked it already.
-Emil
On Wed, Aug 07, 2019 at 04:59:51PM +0100, Emil Velikov wrote:
On Wed, 7 Aug 2019 at 15:53, Sean Paul sean@poorly.run wrote:
From: Rob Herring robh@kernel.org
This reverts commit 583bbf46133c726bae277e8f4e32bfba2a528c7f.
Turns out we need mmap to work on imported BOs even if the current code is buggy.
Personally I would have mentioned a use case where imported BOs are used.
Signed-off-by: Rob Herring robh@kernel.org Signed-off-by: Sean Paul sean@poorly.run
Regardless of the above nitpick, with the patch order fixed the series is: Reviewed-by: Emil Velikov emil.velikov@collabora.com
... in case you haven't picked it already.
Yeah a follow-up patch to add a comment here about why exactly this went all kaboom, plus which userspace (since panfrost is moving fast) would be real nice here.
Atm we need to hope someone does a git blame on this before the break this again, which seems a bit hopeful ... -Daniel
On Wed, Aug 07, 2019 at 10:52:46AM -0400, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
When I started re-applying these I realized they hadn't hit the list.
Sean
Rob Herring (2): Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()" Revert "drm/panfrost: Use drm_gem_map_offset()"
Note I have the order messed up, so will swap it when I apply (and properly compile test in between :/)
Sean
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++------- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++++++++++++++-- include/drm/drm_gem.h | 4 ++-- 5 files changed, 23 insertions(+), 14 deletions(-)
-- Sean Paul, Software Engineer, Google / Chromium OS
On Wed, Aug 07, 2019 at 10:52:46AM -0400, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
When I started re-applying these I realized they hadn't hit the list.
Applied to drm-misc-next
Sean
Sean
Rob Herring (2): Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()" Revert "drm/panfrost: Use drm_gem_map_offset()"
drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++------- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++++++++++++++-- include/drm/drm_gem.h | 4 ++-- 5 files changed, 23 insertions(+), 14 deletions(-)
-- Sean Paul, Software Engineer, Google / Chromium OS
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Emil Velikov -
Rob Herring -
Sean Paul