Re: [PATCH 4/7] drm/client: Deprecate struct drm_client_buffer.gem

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:

The client buffer's framebuffer holds a reference and pointer on
each of its GEM buffer objects. Thus the field gem in the client-
buffer struct is not necessary. Deprecated the field and convert
the client-buffer helpers to use the framebuffer's objects.

In drm_client_buffer_delete(), do a possible vunmap before releasing
the framebuffer. Otherwise we'd eventually release the framebuffer
before unmaping its buffer objects.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfale...@redhat.com>

Signed-off-by: Thomas Zimmermann <tzimmerm...@suse.de>
---
  drivers/gpu/drm/drm_client.c | 20 ++++++++++++--------
  include/drm/drm_client.h     |  9 +++------
  2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 73ae63f856a0..29ed5f55b5f9 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -17,6 +17,7 @@
  #include <drm/drm_fourcc.h>
  #include <drm/drm_framebuffer.h>
  #include <drm/drm_gem.h>
+#include <drm/drm_gem_framebuffer_helper.h>
  #include <drm/drm_mode.h>
  #include <drm/drm_print.h>

@@ -174,17 +175,17 @@ EXPORT_SYMBOL(drm_client_release); static void drm_client_buffer_delete(struct drm_client_buffer *buffer)

  {
+       struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
        int ret;

+ drm_gem_vunmap(gem, &buffer->map);

+
        ret = drm_mode_rmfb(buffer->client->dev, buffer->fb->base.id, 
buffer->client->file);
        if (ret)
                drm_err(buffer->client->dev,
                        "Error removing FB:%u (%d)\n", buffer->fb->base.id, 
ret);

- if (buffer->gem) {

-               drm_gem_vunmap(buffer->gem, &buffer->map);
-               drm_gem_object_put(buffer->gem);
-       }
+       drm_gem_object_put(buffer->gem);

kfree(buffer);

  }
@@ -274,7 +275,7 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 
width, u32 height,
  int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
                                 struct iosys_map *map_copy)
  {
-       struct drm_gem_object *gem = buffer->gem;
+       struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
        struct iosys_map *map = &buffer->map;
        int ret;

@@ -303,7 +304,7 @@ EXPORT_SYMBOL(drm_client_buffer_vmap_local);

   */
  void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer)
  {
-       struct drm_gem_object *gem = buffer->gem;
+       struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
        struct iosys_map *map = &buffer->map;

drm_gem_vunmap_locked(gem, map);

@@ -334,9 +335,10 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap_local);
  int drm_client_buffer_vmap(struct drm_client_buffer *buffer,
                           struct iosys_map *map_copy)
  {
+       struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
        int ret;

- ret = drm_gem_vmap(buffer->gem, &buffer->map);

+       ret = drm_gem_vmap(gem, &buffer->map);
        if (ret)
                return ret;
        *map_copy = buffer->map;
@@ -355,7 +357,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
   */
  void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
  {
-       drm_gem_vunmap(buffer->gem, &buffer->map);
+       struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
+
+       drm_gem_vunmap(gem, &buffer->map);
  }
  EXPORT_SYMBOL(drm_client_buffer_vunmap);

diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h

index db0665263a10..5909664d4d43 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -166,12 +166,9 @@ struct drm_client_buffer {
        /**
         * @gem: GEM object backing this buffer
         *
-        * FIXME: The dependency on GEM here isn't required, we could
-        * convert the driver handle to a dma-buf instead and use the
-        * backend-agnostic dma-buf vmap support instead. This would
-        * require that the handle2fd prime ioctl is reworked to pull the
-        * fd_install step out of the driver backend hooks, to make that
-        * final step optional for internal users.
+        * FIXME: The DRM framebuffer holds a reference on its GEM
+        * buffer objects. Do not use this field in new code and
+        * update existing users.
         */
        struct drm_gem_object *gem;