* [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME
@ 2025-04-04 13:23 Thomas Zimmermann
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-04 13:23 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel, Thomas Zimmermann
The pin and unpin callbacks in struct drm_gem_object_funcs are
for PRIME exported dma-bufs. Remove the pin calls in the client
code, drop the unnecessary pin callbacks from gem-vram and inline
drm_gem_pin() into the only remaining caller. Do the equivalent
for drm_gem_unpin().
AFAICT the long-term plan is to switch to dynamic dma-buf imports
and remove pin/unpin from GEM object functions.
Thomas Zimmermann (4):
drm/client: Do not pin in drm_client_buffer_vmap()
drm/gem-vram: Do not set pin and unpin callbacks
drm/gem-vram: Un-export pin helpers
drm/gem: Inline drm_gem_pin() into PRIME helpers
drivers/gpu/drm/drm_client.c | 22 ++------
drivers/gpu/drm/drm_gem.c | 32 -----------
drivers/gpu/drm/drm_gem_vram_helper.c | 79 +--------------------------
drivers/gpu/drm/drm_internal.h | 4 --
drivers/gpu/drm/drm_prime.c | 22 +++++++-
include/drm/drm_gem.h | 3 +-
include/drm/drm_gem_vram_helper.h | 2 -
7 files changed, 29 insertions(+), 135 deletions(-)
--
2.49.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap()
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
@ 2025-04-04 13:23 ` Thomas Zimmermann
2025-04-14 10:02 ` Boris Brezillon
2025-04-14 10:31 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks Thomas Zimmermann
` (3 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-04 13:23 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel, Thomas Zimmermann
Pin and vmap are two distict operations. Do not mix them.
The helper drm_client_buffer_vmap() maps the pages for fbdev-dma
and fbdev-shmem. In both cases, the vmap operation ensures that
the pages are available until the vunmap happens. And as the pages
in DMA or SHMEM areas cannot be moved, there is no reason to call
pin. Hence remove the pin call.
Update drm_client_buffer_vunmap() accordingly.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_client.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index f1de7faf9fb45..154693066a127 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -303,34 +303,23 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap_local);
* Returns:
* 0 on success, or a negative errno code otherwise.
*/
-int
-drm_client_buffer_vmap(struct drm_client_buffer *buffer,
- struct iosys_map *map_copy)
+int drm_client_buffer_vmap(struct drm_client_buffer *buffer,
+ struct iosys_map *map_copy)
{
struct drm_gem_object *gem = buffer->gem;
struct iosys_map *map = &buffer->map;
int ret;
drm_gem_lock(gem);
-
- ret = drm_gem_pin_locked(gem);
- if (ret)
- goto err_drm_gem_pin_locked;
ret = drm_gem_vmap_locked(gem, map);
- if (ret)
- goto err_drm_gem_vmap;
-
drm_gem_unlock(gem);
+ if (ret)
+ return ret;
+
*map_copy = *map;
return 0;
-
-err_drm_gem_vmap:
- drm_gem_unpin_locked(buffer->gem);
-err_drm_gem_pin_locked:
- drm_gem_unlock(gem);
- return ret;
}
EXPORT_SYMBOL(drm_client_buffer_vmap);
@@ -349,7 +338,6 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
drm_gem_lock(gem);
drm_gem_vunmap_locked(gem, map);
- drm_gem_unpin_locked(gem);
drm_gem_unlock(gem);
}
EXPORT_SYMBOL(drm_client_buffer_vunmap);
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
@ 2025-04-04 13:23 ` Thomas Zimmermann
2025-04-14 10:32 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 3/4] drm/gem-vram: Un-export pin helpers Thomas Zimmermann
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-04 13:23 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel, Thomas Zimmermann
Gem-vram helpers do not support PRIME dma-buf sharing. No nothing
will ever call pin/unpin on its buffer objects. Do not set these
callbacks in struct drm_gem_object_funcs.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem_vram_helper.c | 37 ---------------------------
1 file changed, 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 22b1fe9c03b81..c73480d318362 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -689,41 +689,6 @@ EXPORT_SYMBOL(drm_gem_vram_plane_helper_cleanup_fb);
* PRIME helpers
*/
-/**
- * drm_gem_vram_object_pin() - Implements &struct drm_gem_object_funcs.pin
- * @gem: The GEM object to pin
- *
- * Returns:
- * 0 on success, or
- * a negative errno code otherwise.
- */
-static int drm_gem_vram_object_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_locked(gbo, 0);
-}
-
-/**
- * drm_gem_vram_object_unpin() - Implements &struct drm_gem_object_funcs.unpin
- * @gem: The GEM object to unpin
- */
-static void drm_gem_vram_object_unpin(struct drm_gem_object *gem)
-{
- struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem);
-
- drm_gem_vram_unpin_locked(gbo);
-}
-
/**
* drm_gem_vram_object_vmap() -
* Implements &struct drm_gem_object_funcs.vmap
@@ -762,8 +727,6 @@ static void drm_gem_vram_object_vunmap(struct drm_gem_object *gem,
static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = {
.free = drm_gem_vram_object_free,
- .pin = drm_gem_vram_object_pin,
- .unpin = drm_gem_vram_object_unpin,
.vmap = drm_gem_vram_object_vmap,
.vunmap = drm_gem_vram_object_vunmap,
.mmap = drm_gem_ttm_mmap,
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] drm/gem-vram: Un-export pin helpers
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
2025-04-04 13:23 ` [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks Thomas Zimmermann
@ 2025-04-04 13:23 ` Thomas Zimmermann
2025-04-14 10:32 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers Thomas Zimmermann
2025-04-14 8:21 ` [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
4 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-04 13:23 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel, Thomas Zimmermann
There are no external callers of the gem-vram pin helpers. Hence
unexport them.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem_vram_helper.c | 42 ++-------------------------
include/drm/drm_gem_vram_helper.h | 2 --
2 files changed, 2 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index c73480d318362..ead50fef5e7d4 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -88,11 +88,6 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
* drmm_vram_helper_init() is a managed interface that installs a
* clean-up handler to run during the DRM device's release.
*
- * For drawing or scanout operations, rsp. buffer objects have to be pinned
- * in video RAM. Call drm_gem_vram_pin() with &DRM_GEM_VRAM_PL_FLAG_VRAM or
- * &DRM_GEM_VRAM_PL_FLAG_SYSTEM to pin a buffer object in video RAM or system
- * memory. Call drm_gem_vram_unpin() to release the pinned object afterwards.
- *
* A buffer object that is pinned in video RAM has a fixed address within that
* memory region. Call drm_gem_vram_offset() to retrieve this value. Typically
* it's used to program the hardware's scanout engine for framebuffers, set
@@ -299,30 +294,7 @@ static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
return 0;
}
-/**
- * drm_gem_vram_pin() - Pins a GEM VRAM object in a region.
- * @gbo: the GEM VRAM object
- * @pl_flag: a bitmask of possible memory regions
- *
- * Pinning a buffer object ensures that it is not evicted from
- * a memory region. A pinned buffer object has to be unpinned before
- * it can be pinned to another region. If the pl_flag argument is 0,
- * the buffer is pinned at its current location (video RAM or system
- * memory).
- *
- * Small buffer objects, such as cursor images, can lead to memory
- * fragmentation if they are pinned in the middle of video RAM. This
- * is especially a problem on devices with only a small amount of
- * video RAM. Fragmentation can prevent the primary framebuffer from
- * fitting in, even though there's enough memory overall. The modifier
- * DRM_GEM_VRAM_PL_FLAG_TOPDOWN marks the buffer object to be pinned
- * at the high end of the memory region to avoid fragmentation.
- *
- * Returns:
- * 0 on success, or
- * a negative error code otherwise.
- */
-int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
+static int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
{
int ret;
@@ -334,7 +306,6 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
return ret;
}
-EXPORT_SYMBOL(drm_gem_vram_pin);
static void drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
{
@@ -343,15 +314,7 @@ static void drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
ttm_bo_unpin(&gbo->bo);
}
-/**
- * drm_gem_vram_unpin() - Unpins a GEM VRAM object
- * @gbo: the GEM VRAM object
- *
- * Returns:
- * 0 on success, or
- * a negative error code otherwise.
- */
-int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
+static int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
{
int ret;
@@ -364,7 +327,6 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
return 0;
}
-EXPORT_SYMBOL(drm_gem_vram_unpin);
/**
* drm_gem_vram_vmap() - Pins and maps a GEM VRAM object into kernel address
diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
index 00830b49a3ffc..2dd42bed679d1 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -94,8 +94,6 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
unsigned long pg_align);
void drm_gem_vram_put(struct drm_gem_vram_object *gbo);
s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
-int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
-int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
int drm_gem_vram_vmap(struct drm_gem_vram_object *gbo, struct iosys_map *map);
void drm_gem_vram_vunmap(struct drm_gem_vram_object *gbo,
struct iosys_map *map);
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
` (2 preceding siblings ...)
2025-04-04 13:23 ` [PATCH 3/4] drm/gem-vram: Un-export pin helpers Thomas Zimmermann
@ 2025-04-04 13:23 ` Thomas Zimmermann
2025-04-14 10:37 ` Dmitry Osipenko
2025-04-14 8:21 ` [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
4 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-04 13:23 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel, Thomas Zimmermann
Inline drm_gem_pin() into its only caller drm_gem_map_attach()
and update the documentation in the callback's purpose. Do the
equivalent for drm_gem_unpin(). Also add stricter error checking
on the involved locking.
The pin operation in the GEM object functions is a helper for
PRIME-exported buffer objects. Having drm_gem_pin() gives the
impression of a general-purpose interface, which is not the case.
Removing it makes the pin callback a bit harder to misuse.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/drm_gem.c | 32 --------------------------------
drivers/gpu/drm/drm_internal.h | 4 ----
drivers/gpu/drm/drm_prime.c | 22 ++++++++++++++++++++--
include/drm/drm_gem.h | 3 ++-
4 files changed, 22 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1e659d2660f73..a0a3b6baa5690 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1184,38 +1184,6 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
obj->funcs->print_info(p, indent, obj);
}
-int drm_gem_pin_locked(struct drm_gem_object *obj)
-{
- if (obj->funcs->pin)
- return obj->funcs->pin(obj);
-
- return 0;
-}
-
-void drm_gem_unpin_locked(struct drm_gem_object *obj)
-{
- if (obj->funcs->unpin)
- obj->funcs->unpin(obj);
-}
-
-int drm_gem_pin(struct drm_gem_object *obj)
-{
- int ret;
-
- dma_resv_lock(obj->resv, NULL);
- ret = drm_gem_pin_locked(obj);
- dma_resv_unlock(obj->resv);
-
- return ret;
-}
-
-void drm_gem_unpin(struct drm_gem_object *obj)
-{
- dma_resv_lock(obj->resv, NULL);
- drm_gem_unpin_locked(obj);
- dma_resv_unlock(obj->resv);
-}
-
int drm_gem_vmap_locked(struct drm_gem_object *obj, struct iosys_map *map)
{
int ret;
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index e44f28fd81d34..442eb31351ddd 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -175,10 +175,6 @@ void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
const struct drm_gem_object *obj);
-int drm_gem_pin_locked(struct drm_gem_object *obj);
-void drm_gem_unpin_locked(struct drm_gem_object *obj);
-int drm_gem_pin(struct drm_gem_object *obj);
-void drm_gem_unpin(struct drm_gem_object *obj);
int drm_gem_vmap_locked(struct drm_gem_object *obj, struct iosys_map *map);
void drm_gem_vunmap_locked(struct drm_gem_object *obj, struct iosys_map *map);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d828502268b8e..a1852c02f5123 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -599,6 +599,7 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
struct dma_buf_attachment *attach)
{
struct drm_gem_object *obj = dma_buf->priv;
+ int ret;
/*
* drm_gem_map_dma_buf() requires obj->get_sg_table(), but drivers
@@ -608,7 +609,16 @@ int drm_gem_map_attach(struct dma_buf *dma_buf,
!obj->funcs->get_sg_table)
return -ENOSYS;
- return drm_gem_pin(obj);
+ if (!obj->funcs->pin)
+ return 0;
+
+ ret = dma_resv_lock(obj->resv, NULL);
+ if (ret)
+ return ret;
+ ret = obj->funcs->pin(obj);
+ dma_resv_unlock(obj->resv);
+
+ return ret;
}
EXPORT_SYMBOL(drm_gem_map_attach);
@@ -625,8 +635,16 @@ void drm_gem_map_detach(struct dma_buf *dma_buf,
struct dma_buf_attachment *attach)
{
struct drm_gem_object *obj = dma_buf->priv;
+ int ret;
- drm_gem_unpin(obj);
+ if (!obj->funcs->unpin)
+ return;
+
+ ret = dma_resv_lock(obj->resv, NULL);
+ if (drm_WARN_ON(obj->dev, ret))
+ return;
+ obj->funcs->unpin(obj);
+ dma_resv_unlock(obj->resv);
}
EXPORT_SYMBOL(drm_gem_map_detach);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 9b71f7a9f3f8a..498485f4501f9 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -126,7 +126,8 @@ struct drm_gem_object_funcs {
/**
* @pin:
*
- * Pin backing buffer in memory. Used by the drm_gem_map_attach() helper.
+ * Pin backing buffer in memory, such that importers can access it.
+ * Used by the drm_gem_map_attach() helper.
*
* This callback is optional.
*/
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
` (3 preceding siblings ...)
2025-04-04 13:23 ` [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers Thomas Zimmermann
@ 2025-04-14 8:21 ` Thomas Zimmermann
4 siblings, 0 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-04-14 8:21 UTC (permalink / raw)
To: boris.brezillon, dmitry.osipenko, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel
Ping for review
Am 04.04.25 um 15:23 schrieb Thomas Zimmermann:
> The pin and unpin callbacks in struct drm_gem_object_funcs are
> for PRIME exported dma-bufs. Remove the pin calls in the client
> code, drop the unnecessary pin callbacks from gem-vram and inline
> drm_gem_pin() into the only remaining caller. Do the equivalent
> for drm_gem_unpin().
>
> AFAICT the long-term plan is to switch to dynamic dma-buf imports
> and remove pin/unpin from GEM object functions.
>
> Thomas Zimmermann (4):
> drm/client: Do not pin in drm_client_buffer_vmap()
> drm/gem-vram: Do not set pin and unpin callbacks
> drm/gem-vram: Un-export pin helpers
> drm/gem: Inline drm_gem_pin() into PRIME helpers
>
> drivers/gpu/drm/drm_client.c | 22 ++------
> drivers/gpu/drm/drm_gem.c | 32 -----------
> drivers/gpu/drm/drm_gem_vram_helper.c | 79 +--------------------------
> drivers/gpu/drm/drm_internal.h | 4 --
> drivers/gpu/drm/drm_prime.c | 22 +++++++-
> include/drm/drm_gem.h | 3 +-
> include/drm/drm_gem_vram_helper.h | 2 -
> 7 files changed, 29 insertions(+), 135 deletions(-)
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap()
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
@ 2025-04-14 10:02 ` Boris Brezillon
2025-04-14 10:31 ` Dmitry Osipenko
1 sibling, 0 replies; 11+ messages in thread
From: Boris Brezillon @ 2025-04-14 10:02 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: dmitry.osipenko, airlied, simona, mripard, maarten.lankhorst,
dri-devel
On Fri, 4 Apr 2025 15:23:31 +0200
Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Pin and vmap are two distict operations. Do not mix them.
>
> The helper drm_client_buffer_vmap() maps the pages for fbdev-dma
> and fbdev-shmem. In both cases, the vmap operation ensures that
> the pages are available until the vunmap happens. And as the pages
> in DMA or SHMEM areas cannot be moved, there is no reason to call
> pin. Hence remove the pin call.
>
> Update drm_client_buffer_vunmap() accordingly.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/drm_client.c | 22 +++++-----------------
> 1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index f1de7faf9fb45..154693066a127 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -303,34 +303,23 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap_local);
> * Returns:
> * 0 on success, or a negative errno code otherwise.
> */
> -int
> -drm_client_buffer_vmap(struct drm_client_buffer *buffer,
> - struct iosys_map *map_copy)
> +int drm_client_buffer_vmap(struct drm_client_buffer *buffer,
> + struct iosys_map *map_copy)
> {
> struct drm_gem_object *gem = buffer->gem;
> struct iosys_map *map = &buffer->map;
> int ret;
>
> drm_gem_lock(gem);
> -
> - ret = drm_gem_pin_locked(gem);
> - if (ret)
> - goto err_drm_gem_pin_locked;
> ret = drm_gem_vmap_locked(gem, map);
> - if (ret)
> - goto err_drm_gem_vmap;
> -
> drm_gem_unlock(gem);
>
> + if (ret)
> + return ret;
> +
> *map_copy = *map;
>
> return 0;
> -
> -err_drm_gem_vmap:
> - drm_gem_unpin_locked(buffer->gem);
> -err_drm_gem_pin_locked:
> - drm_gem_unlock(gem);
> - return ret;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -349,7 +338,6 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>
> drm_gem_lock(gem);
> drm_gem_vunmap_locked(gem, map);
> - drm_gem_unpin_locked(gem);
> drm_gem_unlock(gem);
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap()
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
2025-04-14 10:02 ` Boris Brezillon
@ 2025-04-14 10:31 ` Dmitry Osipenko
1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-04-14 10:31 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel
On 4/4/25 16:23, Thomas Zimmermann wrote:
> +int drm_client_buffer_vmap(struct drm_client_buffer *buffer,
> + struct iosys_map *map_copy)
> {
> struct drm_gem_object *gem = buffer->gem;
> struct iosys_map *map = &buffer->map;
> int ret;
>
> drm_gem_lock(gem);
> -
> - ret = drm_gem_pin_locked(gem);
> - if (ret)
> - goto err_drm_gem_pin_locked;
> ret = drm_gem_vmap_locked(gem, map);
> - if (ret)
> - goto err_drm_gem_vmap;
> -
> drm_gem_unlock(gem);
The lock+unlock can be replaced with a single drm_gem_vmap_unlocked().
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks
2025-04-04 13:23 ` [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks Thomas Zimmermann
@ 2025-04-14 10:32 ` Dmitry Osipenko
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-04-14 10:32 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel
On 4/4/25 16:23, Thomas Zimmermann wrote:
> Gem-vram helpers do not support PRIME dma-buf sharing. No nothing
> will ever call pin/unpin on its buffer objects. Do not set these
> callbacks in struct drm_gem_object_funcs.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/gem-vram: Un-export pin helpers
2025-04-04 13:23 ` [PATCH 3/4] drm/gem-vram: Un-export pin helpers Thomas Zimmermann
@ 2025-04-14 10:32 ` Dmitry Osipenko
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-04-14 10:32 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel
On 4/4/25 16:23, Thomas Zimmermann wrote:
> There are no external callers of the gem-vram pin helpers. Hence
> unexport them.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers
2025-04-04 13:23 ` [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers Thomas Zimmermann
@ 2025-04-14 10:37 ` Dmitry Osipenko
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2025-04-14 10:37 UTC (permalink / raw)
To: Thomas Zimmermann, boris.brezillon, airlied, simona, mripard,
maarten.lankhorst
Cc: dri-devel
On 4/4/25 16:23, Thomas Zimmermann wrote:
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 9b71f7a9f3f8a..498485f4501f9 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -126,7 +126,8 @@ struct drm_gem_object_funcs {
> /**
> * @pin:
> *
> - * Pin backing buffer in memory. Used by the drm_gem_map_attach() helper.
> + * Pin backing buffer in memory, such that importers can access it.
Saying `such that dma-buf importers` would make it more clear that this
is about dma-bufs, IMO. Otherwise looks good.
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-04-14 10:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-04 13:23 [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
2025-04-04 13:23 ` [PATCH 1/4] drm/client: Do not pin in drm_client_buffer_vmap() Thomas Zimmermann
2025-04-14 10:02 ` Boris Brezillon
2025-04-14 10:31 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 2/4] drm/gem-vram: Do not set pin and unpin callbacks Thomas Zimmermann
2025-04-14 10:32 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 3/4] drm/gem-vram: Un-export pin helpers Thomas Zimmermann
2025-04-14 10:32 ` Dmitry Osipenko
2025-04-04 13:23 ` [PATCH 4/4] drm/gem: Inline drm_gem_pin() into PRIME helpers Thomas Zimmermann
2025-04-14 10:37 ` Dmitry Osipenko
2025-04-14 8:21 ` [PATCH 0/4] drm: Restrict GEM's pin/unpin to PRIME Thomas Zimmermann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.