[PATCH 0/7] drm/client: Simply client-buffer interface and implementation

[PATCH 0/7] drm/client: Simply client-buffer interface and implementation

[Thomas Zimmermann]

Streamline the client-buffer code and remove the drm_client_framebuffer_
functions. Makes the code easier to work with.

Also prepares for importing non-dumb-buffer objects. Most of the in-
kernel clients will use dumb-buffer allocation. In the case of the
per-driver fbdev_probe code, the client buffer can also wrap an object
that comes from a different memory region; for example gma500's stolen
memory area. This functionality is required to move forward with the
various per-driver implementations of fbdev_probe within the overall
DRM-client framework.

Tested with efidrm and virtio-gpu on Virt I/O hardware.

Thomas Zimmermann (7):
  drm/client: Remove pitch from struct drm_client_buffer
  drm/client: Move dumb-buffer handling to
    drm_client_framebuffer_create()
  drm/client: Inline drm_client_buffer_addfb() and _rmfb()
  drm/client: Deprecate struct drm_client_buffer.gem
  drm/client: Remove drm_client_framebuffer_delete()
  drm/client: Create client buffers with drm_client_buffer_create_dumb()
  drm/client: Flush client buffers with drm_client_buffer_sync()

 drivers/gpu/drm/clients/drm_log.c |  10 +-
 drivers/gpu/drm/drm_client.c      | 192 ++++++++++++++----------------
 drivers/gpu/drm/drm_fbdev_dma.c   |   8 +-
 drivers/gpu/drm/drm_fbdev_shmem.c |   6 +-
 drivers/gpu/drm/drm_fbdev_ttm.c   |  10 +-
 include/drm/drm_client.h          |  20 +---
 6 files changed, 113 insertions(+), 133 deletions(-)

-- 
2.51.0

[PATCH 1/7] drm/client: Remove pitch from struct drm_client_buffer

[Thomas Zimmermann]

Only the client-buffer setup uses the pitch field from struct
drm_client_buffer. Remove the field and pass the value among setup
helpers.

Clients that need the pitch should rather look at the framebuffer's
pitches[0] directly.

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

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 3fa38d4ac70b..5fa8a1628563 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -184,7 +184,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
 
 static struct drm_client_buffer *
 drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
-			 u32 format, u32 *handle)
+			 u32 format, u32 *handle, u32 *pitch)
 {
 	const struct drm_format_info *info = drm_format_info(format);
 	struct drm_mode_create_dumb dumb_args = { };
@@ -212,9 +212,9 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
 		goto err_delete;
 	}
 
-	buffer->pitch = dumb_args.pitch;
 	buffer->gem = obj;
 	*handle = dumb_args.handle;
+	*pitch = dumb_args.pitch;
 
 	return buffer;
 
@@ -349,7 +349,7 @@ static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
 
 static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
 				   u32 width, u32 height, u32 format,
-				   u32 handle)
+				   u32 handle, u32 pitch)
 {
 	struct drm_client_dev *client = buffer->client;
 	struct drm_mode_fb_cmd2 fb_req = { };
@@ -359,7 +359,7 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
 	fb_req.height = height;
 	fb_req.pixel_format = format;
 	fb_req.handles[0] = handle;
-	fb_req.pitches[0] = buffer->pitch;
+	fb_req.pitches[0] = pitch;
 
 	ret = drm_mode_addfb2(client->dev, &fb_req, client->file);
 	if (ret)
@@ -395,15 +395,15 @@ struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
 {
 	struct drm_client_buffer *buffer;
-	u32 handle;
+	u32 handle, pitch;
 	int ret;
 
 	buffer = drm_client_buffer_create(client, width, height, format,
-					  &handle);
+					  &handle, &pitch);
 	if (IS_ERR(buffer))
 		return buffer;
 
-	ret = drm_client_buffer_addfb(buffer, width, height, format, handle);
+	ret = drm_client_buffer_addfb(buffer, width, height, format, handle, pitch);
 
 	/*
 	 * The handle is only needed for creating the framebuffer, destroy it
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 3556928d3938..db0665263a10 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -163,11 +163,6 @@ struct drm_client_buffer {
 	 */
 	struct drm_client_dev *client;
 
-	/**
-	 * @pitch: Buffer pitch
-	 */
-	u32 pitch;
-
 	/**
 	 * @gem: GEM object backing this buffer
 	 *
-- 
2.51.0

[PATCH 2/7] drm/client: Move dumb-buffer handling to drm_client_framebuffer_create()

[Thomas Zimmermann]

Dumb-buffer creation within the client code is asymetrically balanced
across drm_client_buffer_create() and drm_client_framebuffer_create().
Put all dumb-buffer code into drm_client_framebuffer_create() and leave
client-buffer initialization to drm_client_buffer_create(). Clarifies
responsibility between these functions.

Apart form the architectural improvements, drm_client_buffer_create()
can now be exported if needed by clients. The client will be able to
initialize buffers that have been created from other interfaces than
dumb buffers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_client.c | 56 +++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 5fa8a1628563..9bf2edfb7b64 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -184,11 +184,8 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
 
 static struct drm_client_buffer *
 drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
-			 u32 format, u32 *handle, u32 *pitch)
+			 u32 format, u32 handle, u32 pitch)
 {
-	const struct drm_format_info *info = drm_format_info(format);
-	struct drm_mode_create_dumb dumb_args = { };
-	struct drm_device *dev = client->dev;
 	struct drm_client_buffer *buffer;
 	struct drm_gem_object *obj;
 	int ret;
@@ -199,28 +196,18 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
 
 	buffer->client = client;
 
-	dumb_args.width = width;
-	dumb_args.height = height;
-	dumb_args.bpp = drm_format_info_bpp(info, 0);
-	ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
-	if (ret)
-		goto err_delete;
-
-	obj = drm_gem_object_lookup(client->file, dumb_args.handle);
+	obj = drm_gem_object_lookup(client->file, handle);
 	if (!obj)  {
 		ret = -ENOENT;
 		goto err_delete;
 	}
 
 	buffer->gem = obj;
-	*handle = dumb_args.handle;
-	*pitch = dumb_args.pitch;
 
 	return buffer;
 
 err_delete:
-	drm_client_buffer_delete(buffer);
-
+	kfree(buffer);
 	return ERR_PTR(ret);
 }
 
@@ -394,16 +381,30 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
 struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
 {
+	const struct drm_format_info *info = drm_format_info(format);
+	struct drm_device *dev = client->dev;
+	struct drm_mode_create_dumb dumb_args = { };
 	struct drm_client_buffer *buffer;
-	u32 handle, pitch;
 	int ret;
 
+	dumb_args.width = width;
+	dumb_args.height = height;
+	dumb_args.bpp = drm_format_info_bpp(info, 0);
+	ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
+	if (ret)
+		return ERR_PTR(ret);
+
 	buffer = drm_client_buffer_create(client, width, height, format,
-					  &handle, &pitch);
-	if (IS_ERR(buffer))
-		return buffer;
+					  dumb_args.handle, dumb_args.pitch);
+	if (IS_ERR(buffer)) {
+		ret = PTR_ERR(buffer);
+		goto err_drm_mode_destroy_dumb;
+	}
 
-	ret = drm_client_buffer_addfb(buffer, width, height, format, handle, pitch);
+	ret = drm_client_buffer_addfb(buffer, width, height, format,
+				      dumb_args.handle, dumb_args.pitch);
+	if (ret)
+		goto err_drm_client_buffer_delete;
 
 	/*
 	 * The handle is only needed for creating the framebuffer, destroy it
@@ -411,14 +412,15 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
 	 * object as DMA-buf. The framebuffer and our buffer structure are still
 	 * holding references to the GEM object to prevent its destruction.
 	 */
-	drm_mode_destroy_dumb(client->dev, handle, client->file);
-
-	if (ret) {
-		drm_client_buffer_delete(buffer);
-		return ERR_PTR(ret);
-	}
+	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
 
 	return buffer;
+
+err_drm_client_buffer_delete:
+	drm_client_buffer_delete(buffer);
+err_drm_mode_destroy_dumb:
+	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL(drm_client_framebuffer_create);
 
-- 
2.51.0

[PATCH 3/7] drm/client: Inline drm_client_buffer_addfb() and _rmfb()

[Thomas Zimmermann]

Creating and deleting a client buffer always creates and deletes
the underlying DRM framebuffer. Inline the helper functions into
their callers.

With the _addfb code being inlined into drm_client_buffer_create(),
clean up the function's error rollback to release the framebuffer's
handle and GEM buffer object as needed.

Move the _rmfb code into drm_client_buffer_delete() rather than its
current location in drm_client_framebuffer_delete(). The former is
now the inverse of drm_client_buffer_create(). Makes no difference
for cleaning up. Also prepares for the removal of
drm_client_framebuffer_delete().

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_client.c | 93 ++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 9bf2edfb7b64..73ae63f856a0 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -174,6 +174,13 @@ EXPORT_SYMBOL(drm_client_release);
 
 static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
 {
+	int ret;
+
+	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);
@@ -186,8 +193,21 @@ static struct drm_client_buffer *
 drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
 			 u32 format, u32 handle, u32 pitch)
 {
+	struct drm_mode_fb_cmd2 fb_req = {
+		.width = width,
+		.height = height,
+		.pixel_format = format,
+		.handles = {
+			handle,
+		},
+		.pitches = {
+			pitch,
+		},
+	};
+	struct drm_device *dev = client->dev;
 	struct drm_client_buffer *buffer;
 	struct drm_gem_object *obj;
+	struct drm_framebuffer *fb;
 	int ret;
 
 	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
@@ -202,10 +222,30 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
 		goto err_delete;
 	}
 
+	ret = drm_mode_addfb2(dev, &fb_req, client->file);
+	if (ret)
+		goto err_drm_gem_object_put;
+
+	fb = drm_framebuffer_lookup(dev, client->file, fb_req.fb_id);
+	if (drm_WARN_ON(dev, !fb)) {
+		ret = -ENOENT;
+		goto err_drm_mode_rmfb;
+	}
+
+	/* drop the reference we picked up in framebuffer lookup */
+	drm_framebuffer_put(fb);
+
+	strscpy(fb->comm, client->name, TASK_COMM_LEN);
+
 	buffer->gem = obj;
+	buffer->fb = fb;
 
 	return buffer;
 
+err_drm_mode_rmfb:
+	drm_mode_rmfb(dev, fb_req.fb_id, client->file);
+err_drm_gem_object_put:
+	drm_gem_object_put(obj);
 err_delete:
 	kfree(buffer);
 	return ERR_PTR(ret);
@@ -319,51 +359,6 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
 }
 EXPORT_SYMBOL(drm_client_buffer_vunmap);
 
-static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
-{
-	int ret;
-
-	if (!buffer->fb)
-		return;
-
-	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);
-
-	buffer->fb = NULL;
-}
-
-static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
-				   u32 width, u32 height, u32 format,
-				   u32 handle, u32 pitch)
-{
-	struct drm_client_dev *client = buffer->client;
-	struct drm_mode_fb_cmd2 fb_req = { };
-	int ret;
-
-	fb_req.width = width;
-	fb_req.height = height;
-	fb_req.pixel_format = format;
-	fb_req.handles[0] = handle;
-	fb_req.pitches[0] = pitch;
-
-	ret = drm_mode_addfb2(client->dev, &fb_req, client->file);
-	if (ret)
-		return ret;
-
-	buffer->fb = drm_framebuffer_lookup(client->dev, buffer->client->file, fb_req.fb_id);
-	if (WARN_ON(!buffer->fb))
-		return -ENOENT;
-
-	/* drop the reference we picked up in framebuffer lookup */
-	drm_framebuffer_put(buffer->fb);
-
-	strscpy(buffer->fb->comm, client->name, TASK_COMM_LEN);
-
-	return 0;
-}
-
 /**
  * drm_client_framebuffer_create - Create a client framebuffer
  * @client: DRM client
@@ -401,11 +396,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
 		goto err_drm_mode_destroy_dumb;
 	}
 
-	ret = drm_client_buffer_addfb(buffer, width, height, format,
-				      dumb_args.handle, dumb_args.pitch);
-	if (ret)
-		goto err_drm_client_buffer_delete;
-
 	/*
 	 * The handle is only needed for creating the framebuffer, destroy it
 	 * again to solve a circular dependency should anybody export the GEM
@@ -416,8 +406,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
 
 	return buffer;
 
-err_drm_client_buffer_delete:
-	drm_client_buffer_delete(buffer);
 err_drm_mode_destroy_dumb:
 	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
 	return ERR_PTR(ret);
@@ -433,7 +421,6 @@ void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
 	if (!buffer)
 		return;
 
-	drm_client_buffer_rmfb(buffer);
 	drm_client_buffer_delete(buffer);
 }
 EXPORT_SYMBOL(drm_client_framebuffer_delete);
-- 
2.51.0

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

[Thomas Zimmermann]

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.

Signed-off-by: Thomas Zimmermann <tzimmermann@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;
 
-- 
2.51.0

[PATCH 5/7] drm/client: Remove drm_client_framebuffer_delete()

[Thomas Zimmermann]

Release client buffers with drm_client_buffer_delete() instead of
drm_client_framebuffer_delete(). The latter is just a tiny wrapper
around the former.

Move the test for !buffer into drm_client_buffer_delete(), although
all callers appear to always have a valid pointer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/clients/drm_log.c |  4 ++--
 drivers/gpu/drm/drm_client.c      | 25 ++++++++++---------------
 drivers/gpu/drm/drm_fbdev_dma.c   |  6 +++---
 drivers/gpu/drm/drm_fbdev_shmem.c |  4 ++--
 drivers/gpu/drm/drm_fbdev_ttm.c   |  8 ++++----
 include/drm/drm_client.h          |  2 +-
 6 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index fd8556dd58ed..2d748ab318fe 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -272,7 +272,7 @@ static void drm_log_init_client(struct drm_log *dlog)
 
 err_failed_commit:
 	for (i = 0; i < n_modeset; i++)
-		drm_client_framebuffer_delete(dlog->scanout[i].buffer);
+		drm_client_buffer_delete(dlog->scanout[i].buffer);
 
 err_nomodeset:
 	kfree(dlog->scanout);
@@ -286,7 +286,7 @@ static void drm_log_free_scanout(struct drm_client_dev *client)
 
 	if (dlog->n_scanout) {
 		for (i = 0; i < dlog->n_scanout; i++)
-			drm_client_framebuffer_delete(dlog->scanout[i].buffer);
+			drm_client_buffer_delete(dlog->scanout[i].buffer);
 		dlog->n_scanout = 0;
 		kfree(dlog->scanout);
 		dlog->scanout = NULL;
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 29ed5f55b5f9..d4c424ff44a9 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -173,11 +173,18 @@ void drm_client_release(struct drm_client_dev *client)
 }
 EXPORT_SYMBOL(drm_client_release);
 
-static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
+/**
+ * drm_client_buffer_delete - Delete a client buffer
+ * @buffer: DRM client buffer
+ */
+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;
 
+	if (!buffer)
+		return;
+
 	drm_gem_vunmap(gem, &buffer->map);
 
 	ret = drm_mode_rmfb(buffer->client->dev, buffer->fb->base.id, buffer->client->file);
@@ -189,6 +196,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
 
 	kfree(buffer);
 }
+EXPORT_SYMBOL(drm_client_buffer_delete);
 
 static struct drm_client_buffer *
 drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
@@ -372,7 +380,7 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap);
  *
  * This function creates a &drm_client_buffer which consists of a
  * &drm_framebuffer backed by a dumb buffer.
- * Call drm_client_framebuffer_delete() to free the buffer.
+ * Call drm_client_buffer_delete() to free the buffer.
  *
  * Returns:
  * Pointer to a client buffer or an error pointer on failure.
@@ -416,19 +424,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
 }
 EXPORT_SYMBOL(drm_client_framebuffer_create);
 
-/**
- * drm_client_framebuffer_delete - Delete a client framebuffer
- * @buffer: DRM client buffer (can be NULL)
- */
-void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
-{
-	if (!buffer)
-		return;
-
-	drm_client_buffer_delete(buffer);
-}
-EXPORT_SYMBOL(drm_client_framebuffer_delete);
-
 /**
  * drm_client_framebuffer_flush - Manually flush client framebuffer
  * @buffer: DRM client buffer (can be NULL)
diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index 8bd626ef16c7..17fef288e86b 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -55,7 +55,7 @@ static void drm_fbdev_dma_fb_destroy(struct fb_info *info)
 	drm_fb_helper_fini(fb_helper);
 
 	drm_client_buffer_vunmap(fb_helper->buffer);
-	drm_client_framebuffer_delete(fb_helper->buffer);
+	drm_client_buffer_delete(fb_helper->buffer);
 	drm_client_release(&fb_helper->client);
 	drm_fb_helper_unprepare(fb_helper);
 	kfree(fb_helper);
@@ -90,7 +90,7 @@ static void drm_fbdev_dma_shadowed_fb_destroy(struct fb_info *info)
 	vfree(shadow);
 
 	drm_client_buffer_vunmap(fb_helper->buffer);
-	drm_client_framebuffer_delete(fb_helper->buffer);
+	drm_client_buffer_delete(fb_helper->buffer);
 	drm_client_release(&fb_helper->client);
 	drm_fb_helper_unprepare(fb_helper);
 	kfree(fb_helper);
@@ -328,7 +328,7 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 	fb_helper->buffer = NULL;
 	drm_client_buffer_vunmap(buffer);
 err_drm_client_buffer_delete:
-	drm_client_framebuffer_delete(buffer);
+	drm_client_buffer_delete(buffer);
 	return ret;
 }
 EXPORT_SYMBOL(drm_fbdev_dma_driver_fbdev_probe);
diff --git a/drivers/gpu/drm/drm_fbdev_shmem.c b/drivers/gpu/drm/drm_fbdev_shmem.c
index 1e827bf8b815..f7966b8c51cd 100644
--- a/drivers/gpu/drm/drm_fbdev_shmem.c
+++ b/drivers/gpu/drm/drm_fbdev_shmem.c
@@ -63,7 +63,7 @@ static void drm_fbdev_shmem_fb_destroy(struct fb_info *info)
 	drm_fb_helper_fini(fb_helper);
 
 	drm_client_buffer_vunmap(fb_helper->buffer);
-	drm_client_framebuffer_delete(fb_helper->buffer);
+	drm_client_buffer_delete(fb_helper->buffer);
 	drm_client_release(&fb_helper->client);
 	drm_fb_helper_unprepare(fb_helper);
 	kfree(fb_helper);
@@ -206,7 +206,7 @@ int drm_fbdev_shmem_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 	fb_helper->buffer = NULL;
 	drm_client_buffer_vunmap(buffer);
 err_drm_client_buffer_delete:
-	drm_client_framebuffer_delete(buffer);
+	drm_client_buffer_delete(buffer);
 	return ret;
 }
 EXPORT_SYMBOL(drm_fbdev_shmem_driver_fbdev_probe);
diff --git a/drivers/gpu/drm/drm_fbdev_ttm.c b/drivers/gpu/drm/drm_fbdev_ttm.c
index 85feb55bba11..54f9a46b96c2 100644
--- a/drivers/gpu/drm/drm_fbdev_ttm.c
+++ b/drivers/gpu/drm/drm_fbdev_ttm.c
@@ -50,7 +50,7 @@ static void drm_fbdev_ttm_fb_destroy(struct fb_info *info)
 	fb_deferred_io_cleanup(info);
 	drm_fb_helper_fini(fb_helper);
 	vfree(shadow);
-	drm_client_framebuffer_delete(fb_helper->buffer);
+	drm_client_buffer_delete(fb_helper->buffer);
 
 	drm_client_release(&fb_helper->client);
 	drm_fb_helper_unprepare(fb_helper);
@@ -202,7 +202,7 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 	screen_buffer = vzalloc(screen_size);
 	if (!screen_buffer) {
 		ret = -ENOMEM;
-		goto err_drm_client_framebuffer_delete;
+		goto err_drm_client_buffer_delete;
 	}
 
 	info = drm_fb_helper_alloc_info(fb_helper);
@@ -235,10 +235,10 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 	drm_fb_helper_release_info(fb_helper);
 err_vfree:
 	vfree(screen_buffer);
-err_drm_client_framebuffer_delete:
+err_drm_client_buffer_delete:
 	fb_helper->fb = NULL;
 	fb_helper->buffer = NULL;
-	drm_client_framebuffer_delete(buffer);
+	drm_client_buffer_delete(buffer);
 	return ret;
 }
 EXPORT_SYMBOL(drm_fbdev_ttm_driver_fbdev_probe);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 5909664d4d43..6019f2712448 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -185,7 +185,7 @@ struct drm_client_buffer {
 
 struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
-void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
+void drm_client_buffer_delete(struct drm_client_buffer *buffer);
 int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
 int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
 				 struct iosys_map *map_copy);
-- 
2.51.0

[PATCH 6/7] drm/client: Create client buffers with drm_client_buffer_create_dumb()

[Thomas Zimmermann]

Rename drm_client_framebuffer_create() to drm_client_buffer_create_dump()
and adapt callers. The new name reflects the function's purpose. Using
dumb buffers is the easiest way for creating a GEM buffer in a drivers-
independent way.

There's also drm_client_buffer_create(), which creates the client buffer
from a preexisting buffer object. This helper can be exported for drivers
that create their own GEM buffer object.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/clients/drm_log.c | 2 +-
 drivers/gpu/drm/drm_client.c      | 6 +++---
 drivers/gpu/drm/drm_fbdev_dma.c   | 2 +-
 drivers/gpu/drm/drm_fbdev_shmem.c | 2 +-
 drivers/gpu/drm/drm_fbdev_ttm.c   | 2 +-
 include/drm/drm_client.h          | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index 2d748ab318fe..b8fb9ee9fcf0 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -204,7 +204,7 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
 	if (format == DRM_FORMAT_INVALID)
 		return -EINVAL;
 
-	scanout->buffer = drm_client_framebuffer_create(client, width, height, format);
+	scanout->buffer = drm_client_buffer_create_dumb(client, width, height, format);
 	if (IS_ERR(scanout->buffer)) {
 		drm_warn(client->dev, "drm_log can't create framebuffer %d %d %p4cc\n",
 			 width, height, &format);
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index d4c424ff44a9..9105a0b5f468 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -372,7 +372,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
 EXPORT_SYMBOL(drm_client_buffer_vunmap);
 
 /**
- * drm_client_framebuffer_create - Create a client framebuffer
+ * drm_client_buffer_create_dumb - Create a client buffer backed by a dumb buffer
  * @client: DRM client
  * @width: Framebuffer width
  * @height: Framebuffer height
@@ -386,7 +386,7 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap);
  * Pointer to a client buffer or an error pointer on failure.
  */
 struct drm_client_buffer *
-drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
+drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 height, u32 format)
 {
 	const struct drm_format_info *info = drm_format_info(format);
 	struct drm_device *dev = client->dev;
@@ -422,7 +422,7 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
 	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
 	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL(drm_client_framebuffer_create);
+EXPORT_SYMBOL(drm_client_buffer_create_dumb);
 
 /**
  * drm_client_framebuffer_flush - Manually flush client framebuffer
diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index 17fef288e86b..7d4e557d6a5d 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -285,7 +285,7 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 
 	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp,
 					     sizes->surface_depth);
-	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
+	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
diff --git a/drivers/gpu/drm/drm_fbdev_shmem.c b/drivers/gpu/drm/drm_fbdev_shmem.c
index f7966b8c51cd..b9e48a0bd39f 100644
--- a/drivers/gpu/drm/drm_fbdev_shmem.c
+++ b/drivers/gpu/drm/drm_fbdev_shmem.c
@@ -149,7 +149,7 @@ int drm_fbdev_shmem_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 		    sizes->surface_bpp);
 
 	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, sizes->surface_depth);
-	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
+	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
diff --git a/drivers/gpu/drm/drm_fbdev_ttm.c b/drivers/gpu/drm/drm_fbdev_ttm.c
index 54f9a46b96c2..b1188692399a 100644
--- a/drivers/gpu/drm/drm_fbdev_ttm.c
+++ b/drivers/gpu/drm/drm_fbdev_ttm.c
@@ -189,7 +189,7 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
 
 	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp,
 					     sizes->surface_depth);
-	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
+	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))
 		return PTR_ERR(buffer);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 6019f2712448..e4df404a9645 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -184,7 +184,7 @@ struct drm_client_buffer {
 };
 
 struct drm_client_buffer *
-drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
+drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 height, u32 format);
 void drm_client_buffer_delete(struct drm_client_buffer *buffer);
 int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
 int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
-- 
2.51.0

[PATCH 7/7] drm/client: Flush client buffers with drm_client_buffer_sync()

[Thomas Zimmermann]

Rename drm_client_framebuffer_flush() to drm_cient_buffer_flush() and
adapt its callers. The old name was left over from previous naming
conventions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/clients/drm_log.c | 4 ++--
 drivers/gpu/drm/drm_client.c      | 8 ++++----
 include/drm/drm_client.h          | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index b8fb9ee9fcf0..3d4174c0d344 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -100,7 +100,7 @@ static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 line)
 		return;
 	iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]);
 	drm_client_buffer_vunmap_local(scanout->buffer);
-	drm_client_framebuffer_flush(scanout->buffer, &r);
+	drm_client_buffer_flush(scanout->buffer, &r);
 }
 
 static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
@@ -133,7 +133,7 @@ static void drm_log_draw_line(struct drm_log_scanout *scanout, const char *s,
 	if (scanout->line >= scanout->rows)
 		scanout->line = 0;
 	drm_client_buffer_vunmap_local(scanout->buffer);
-	drm_client_framebuffer_flush(scanout->buffer, &r);
+	drm_client_buffer_flush(scanout->buffer, &r);
 }
 
 static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 9105a0b5f468..1f1e73d3b026 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -425,8 +425,8 @@ drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 heig
 EXPORT_SYMBOL(drm_client_buffer_create_dumb);
 
 /**
- * drm_client_framebuffer_flush - Manually flush client framebuffer
- * @buffer: DRM client buffer (can be NULL)
+ * drm_client_buffer_flush - Manually flush client buffer
+ * @buffer: DRM client buffer
  * @rect: Damage rectangle (if NULL flushes all)
  *
  * This calls &drm_framebuffer_funcs->dirty (if present) to flush buffer changes
@@ -435,7 +435,7 @@ EXPORT_SYMBOL(drm_client_buffer_create_dumb);
  * Returns:
  * Zero on success or negative error code on failure.
  */
-int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect)
+int drm_client_buffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect)
 {
 	if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
 		return 0;
@@ -455,4 +455,4 @@ int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_re
 	return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
 					0, 0, NULL, 0);
 }
-EXPORT_SYMBOL(drm_client_framebuffer_flush);
+EXPORT_SYMBOL(drm_client_buffer_flush);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index e4df404a9645..eab4a51f8e79 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -186,7 +186,7 @@ struct drm_client_buffer {
 struct drm_client_buffer *
 drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 height, u32 format);
 void drm_client_buffer_delete(struct drm_client_buffer *buffer);
-int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
+int drm_client_buffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
 int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
 				 struct iosys_map *map_copy);
 void drm_client_buffer_vunmap_local(struct drm_client_buffer *buffer);
-- 
2.51.0

Re: [PATCH 1/7] drm/client: Remove pitch from struct drm_client_buffer

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:
> Only the client-buffer setup uses the pitch field from struct
> drm_client_buffer. Remove the field and pass the value among setup
> helpers.
> 
> Clients that need the pitch should rather look at the framebuffer's
> pitches[0] directly.

Thanks, it looks good to me.

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

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/drm_client.c | 14 +++++++-------
>   include/drm/drm_client.h     |  5 -----
>   2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 3fa38d4ac70b..5fa8a1628563 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -184,7 +184,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>   
>   static struct drm_client_buffer *
>   drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
> -			 u32 format, u32 *handle)
> +			 u32 format, u32 *handle, u32 *pitch)
>   {
>   	const struct drm_format_info *info = drm_format_info(format);
>   	struct drm_mode_create_dumb dumb_args = { };
> @@ -212,9 +212,9 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
>   		goto err_delete;
>   	}
>   
> -	buffer->pitch = dumb_args.pitch;
>   	buffer->gem = obj;
>   	*handle = dumb_args.handle;
> +	*pitch = dumb_args.pitch;
>   
>   	return buffer;
>   
> @@ -349,7 +349,7 @@ static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
>   
>   static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
>   				   u32 width, u32 height, u32 format,
> -				   u32 handle)
> +				   u32 handle, u32 pitch)
>   {
>   	struct drm_client_dev *client = buffer->client;
>   	struct drm_mode_fb_cmd2 fb_req = { };
> @@ -359,7 +359,7 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
>   	fb_req.height = height;
>   	fb_req.pixel_format = format;
>   	fb_req.handles[0] = handle;
> -	fb_req.pitches[0] = buffer->pitch;
> +	fb_req.pitches[0] = pitch;
>   
>   	ret = drm_mode_addfb2(client->dev, &fb_req, client->file);
>   	if (ret)
> @@ -395,15 +395,15 @@ struct drm_client_buffer *
>   drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
>   {
>   	struct drm_client_buffer *buffer;
> -	u32 handle;
> +	u32 handle, pitch;
>   	int ret;
>   
>   	buffer = drm_client_buffer_create(client, width, height, format,
> -					  &handle);
> +					  &handle, &pitch);
>   	if (IS_ERR(buffer))
>   		return buffer;
>   
> -	ret = drm_client_buffer_addfb(buffer, width, height, format, handle);
> +	ret = drm_client_buffer_addfb(buffer, width, height, format, handle, pitch);
>   
>   	/*
>   	 * The handle is only needed for creating the framebuffer, destroy it
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 3556928d3938..db0665263a10 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -163,11 +163,6 @@ struct drm_client_buffer {
>   	 */
>   	struct drm_client_dev *client;
>   
> -	/**
> -	 * @pitch: Buffer pitch
> -	 */
> -	u32 pitch;
> -
>   	/**
>   	 * @gem: GEM object backing this buffer
>   	 *

Re: [PATCH 2/7] drm/client: Move dumb-buffer handling to drm_client_framebuffer_create()

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:
> Dumb-buffer creation within the client code is asymetrically balanced
> across drm_client_buffer_create() and drm_client_framebuffer_create().
> Put all dumb-buffer code into drm_client_framebuffer_create() and leave
> client-buffer initialization to drm_client_buffer_create(). Clarifies
> responsibility between these functions.
> 
> Apart form the architectural improvements, drm_client_buffer_create()
> can now be exported if needed by clients. The client will be able to
> initialize buffers that have been created from other interfaces than
> dumb buffers.

Thanks, it looks good to me.

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

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/drm_client.c | 56 +++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 5fa8a1628563..9bf2edfb7b64 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -184,11 +184,8 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>   
>   static struct drm_client_buffer *
>   drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
> -			 u32 format, u32 *handle, u32 *pitch)
> +			 u32 format, u32 handle, u32 pitch)
>   {
> -	const struct drm_format_info *info = drm_format_info(format);
> -	struct drm_mode_create_dumb dumb_args = { };
> -	struct drm_device *dev = client->dev;
>   	struct drm_client_buffer *buffer;
>   	struct drm_gem_object *obj;
>   	int ret;
> @@ -199,28 +196,18 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
>   
>   	buffer->client = client;
>   
> -	dumb_args.width = width;
> -	dumb_args.height = height;
> -	dumb_args.bpp = drm_format_info_bpp(info, 0);
> -	ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> -	if (ret)
> -		goto err_delete;
> -
> -	obj = drm_gem_object_lookup(client->file, dumb_args.handle);
> +	obj = drm_gem_object_lookup(client->file, handle);
>   	if (!obj)  {
>   		ret = -ENOENT;
>   		goto err_delete;
>   	}
>   
>   	buffer->gem = obj;
> -	*handle = dumb_args.handle;
> -	*pitch = dumb_args.pitch;
>   
>   	return buffer;
>   
>   err_delete:
> -	drm_client_buffer_delete(buffer);
> -
> +	kfree(buffer);
>   	return ERR_PTR(ret);
>   }
>   
> @@ -394,16 +381,30 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
>   struct drm_client_buffer *
>   drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
>   {
> +	const struct drm_format_info *info = drm_format_info(format);
> +	struct drm_device *dev = client->dev;
> +	struct drm_mode_create_dumb dumb_args = { };
>   	struct drm_client_buffer *buffer;
> -	u32 handle, pitch;
>   	int ret;
>   
> +	dumb_args.width = width;
> +	dumb_args.height = height;
> +	dumb_args.bpp = drm_format_info_bpp(info, 0);
> +	ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
>   	buffer = drm_client_buffer_create(client, width, height, format,
> -					  &handle, &pitch);
> -	if (IS_ERR(buffer))
> -		return buffer;
> +					  dumb_args.handle, dumb_args.pitch);
> +	if (IS_ERR(buffer)) {
> +		ret = PTR_ERR(buffer);
> +		goto err_drm_mode_destroy_dumb;
> +	}
>   
> -	ret = drm_client_buffer_addfb(buffer, width, height, format, handle, pitch);
> +	ret = drm_client_buffer_addfb(buffer, width, height, format,
> +				      dumb_args.handle, dumb_args.pitch);
> +	if (ret)
> +		goto err_drm_client_buffer_delete;
>   
>   	/*
>   	 * The handle is only needed for creating the framebuffer, destroy it
> @@ -411,14 +412,15 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
>   	 * object as DMA-buf. The framebuffer and our buffer structure are still
>   	 * holding references to the GEM object to prevent its destruction.
>   	 */
> -	drm_mode_destroy_dumb(client->dev, handle, client->file);
> -
> -	if (ret) {
> -		drm_client_buffer_delete(buffer);
> -		return ERR_PTR(ret);
> -	}
> +	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
>   
>   	return buffer;
> +
> +err_drm_client_buffer_delete:
> +	drm_client_buffer_delete(buffer);
> +err_drm_mode_destroy_dumb:
> +	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
> +	return ERR_PTR(ret);
>   }
>   EXPORT_SYMBOL(drm_client_framebuffer_create);
>   

Re: [PATCH 3/7] drm/client: Inline drm_client_buffer_addfb() and _rmfb()

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:
> Creating and deleting a client buffer always creates and deletes
> the underlying DRM framebuffer. Inline the helper functions into
> their callers.
> 
> With the _addfb code being inlined into drm_client_buffer_create(),
> clean up the function's error rollback to release the framebuffer's
> handle and GEM buffer object as needed.
> 
> Move the _rmfb code into drm_client_buffer_delete() rather than its
> current location in drm_client_framebuffer_delete(). The former is
> now the inverse of drm_client_buffer_create(). Makes no difference
> for cleaning up. Also prepares for the removal of
> drm_client_framebuffer_delete().

Thanks, it looks good to me.

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


> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/drm_client.c | 93 ++++++++++++++++--------------------
>   1 file changed, 40 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 9bf2edfb7b64..73ae63f856a0 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -174,6 +174,13 @@ EXPORT_SYMBOL(drm_client_release);
>   
>   static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>   {
> +	int ret;
> +
> +	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);
> @@ -186,8 +193,21 @@ static struct drm_client_buffer *
>   drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
>   			 u32 format, u32 handle, u32 pitch)
>   {
> +	struct drm_mode_fb_cmd2 fb_req = {
> +		.width = width,
> +		.height = height,
> +		.pixel_format = format,
> +		.handles = {
> +			handle,
> +		},
> +		.pitches = {
> +			pitch,
> +		},
> +	};
> +	struct drm_device *dev = client->dev;
>   	struct drm_client_buffer *buffer;
>   	struct drm_gem_object *obj;
> +	struct drm_framebuffer *fb;
>   	int ret;
>   
>   	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> @@ -202,10 +222,30 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
>   		goto err_delete;
>   	}
>   
> +	ret = drm_mode_addfb2(dev, &fb_req, client->file);
> +	if (ret)
> +		goto err_drm_gem_object_put;
> +
> +	fb = drm_framebuffer_lookup(dev, client->file, fb_req.fb_id);
> +	if (drm_WARN_ON(dev, !fb)) {
> +		ret = -ENOENT;
> +		goto err_drm_mode_rmfb;
> +	}
> +
> +	/* drop the reference we picked up in framebuffer lookup */
> +	drm_framebuffer_put(fb);
> +
> +	strscpy(fb->comm, client->name, TASK_COMM_LEN);
> +
>   	buffer->gem = obj;
> +	buffer->fb = fb;
>   
>   	return buffer;
>   
> +err_drm_mode_rmfb:
> +	drm_mode_rmfb(dev, fb_req.fb_id, client->file);
> +err_drm_gem_object_put:
> +	drm_gem_object_put(obj);
>   err_delete:
>   	kfree(buffer);
>   	return ERR_PTR(ret);
> @@ -319,51 +359,6 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>   }
>   EXPORT_SYMBOL(drm_client_buffer_vunmap);
>   
> -static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
> -{
> -	int ret;
> -
> -	if (!buffer->fb)
> -		return;
> -
> -	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);
> -
> -	buffer->fb = NULL;
> -}
> -
> -static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
> -				   u32 width, u32 height, u32 format,
> -				   u32 handle, u32 pitch)
> -{
> -	struct drm_client_dev *client = buffer->client;
> -	struct drm_mode_fb_cmd2 fb_req = { };
> -	int ret;
> -
> -	fb_req.width = width;
> -	fb_req.height = height;
> -	fb_req.pixel_format = format;
> -	fb_req.handles[0] = handle;
> -	fb_req.pitches[0] = pitch;
> -
> -	ret = drm_mode_addfb2(client->dev, &fb_req, client->file);
> -	if (ret)
> -		return ret;
> -
> -	buffer->fb = drm_framebuffer_lookup(client->dev, buffer->client->file, fb_req.fb_id);
> -	if (WARN_ON(!buffer->fb))
> -		return -ENOENT;
> -
> -	/* drop the reference we picked up in framebuffer lookup */
> -	drm_framebuffer_put(buffer->fb);
> -
> -	strscpy(buffer->fb->comm, client->name, TASK_COMM_LEN);
> -
> -	return 0;
> -}
> -
>   /**
>    * drm_client_framebuffer_create - Create a client framebuffer
>    * @client: DRM client
> @@ -401,11 +396,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
>   		goto err_drm_mode_destroy_dumb;
>   	}
>   
> -	ret = drm_client_buffer_addfb(buffer, width, height, format,
> -				      dumb_args.handle, dumb_args.pitch);
> -	if (ret)
> -		goto err_drm_client_buffer_delete;
> -
>   	/*
>   	 * The handle is only needed for creating the framebuffer, destroy it
>   	 * again to solve a circular dependency should anybody export the GEM
> @@ -416,8 +406,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
>   
>   	return buffer;
>   
> -err_drm_client_buffer_delete:
> -	drm_client_buffer_delete(buffer);
>   err_drm_mode_destroy_dumb:
>   	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
>   	return ERR_PTR(ret);
> @@ -433,7 +421,6 @@ void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
>   	if (!buffer)
>   		return;
>   
> -	drm_client_buffer_rmfb(buffer);
>   	drm_client_buffer_delete(buffer);
>   }
>   EXPORT_SYMBOL(drm_client_framebuffer_delete);

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 <jfalempe@redhat.com>


> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@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;
>   

Re: [PATCH 5/7] drm/client: Remove drm_client_framebuffer_delete()

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:
> Release client buffers with drm_client_buffer_delete() instead of
> drm_client_framebuffer_delete(). The latter is just a tiny wrapper
> around the former.
> 
> Move the test for !buffer into drm_client_buffer_delete(), although
> all callers appear to always have a valid pointer.


A minor issue below, with that fixed:

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


> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/clients/drm_log.c |  4 ++--
>   drivers/gpu/drm/drm_client.c      | 25 ++++++++++---------------
>   drivers/gpu/drm/drm_fbdev_dma.c   |  6 +++---
>   drivers/gpu/drm/drm_fbdev_shmem.c |  4 ++--
>   drivers/gpu/drm/drm_fbdev_ttm.c   |  8 ++++----
>   include/drm/drm_client.h          |  2 +-
>   6 files changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index fd8556dd58ed..2d748ab318fe 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -272,7 +272,7 @@ static void drm_log_init_client(struct drm_log *dlog)
>   
>   err_failed_commit:
>   	for (i = 0; i < n_modeset; i++)
> -		drm_client_framebuffer_delete(dlog->scanout[i].buffer);
> +		drm_client_buffer_delete(dlog->scanout[i].buffer);
>   
>   err_nomodeset:
>   	kfree(dlog->scanout);
> @@ -286,7 +286,7 @@ static void drm_log_free_scanout(struct drm_client_dev *client)
>   
>   	if (dlog->n_scanout) {
>   		for (i = 0; i < dlog->n_scanout; i++)
> -			drm_client_framebuffer_delete(dlog->scanout[i].buffer);
> +			drm_client_buffer_delete(dlog->scanout[i].buffer);
>   		dlog->n_scanout = 0;
>   		kfree(dlog->scanout);
>   		dlog->scanout = NULL;
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 29ed5f55b5f9..d4c424ff44a9 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -173,11 +173,18 @@ void drm_client_release(struct drm_client_dev *client)
>   }
>   EXPORT_SYMBOL(drm_client_release);
>   
> -static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
> +/**
> + * drm_client_buffer_delete - Delete a client buffer
> + * @buffer: DRM client buffer
> + */
> +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;
>   
> +	if (!buffer)
> +		return;
> +

It's too late to check, as you already used buffer->fb in the previous line.

>   	drm_gem_vunmap(gem, &buffer->map);
>   
>   	ret = drm_mode_rmfb(buffer->client->dev, buffer->fb->base.id, buffer->client->file);
> @@ -189,6 +196,7 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>   
>   	kfree(buffer);
>   }
> +EXPORT_SYMBOL(drm_client_buffer_delete);
>   
>   static struct drm_client_buffer *
>   drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
> @@ -372,7 +380,7 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap);
>    *
>    * This function creates a &drm_client_buffer which consists of a
>    * &drm_framebuffer backed by a dumb buffer.
> - * Call drm_client_framebuffer_delete() to free the buffer.
> + * Call drm_client_buffer_delete() to free the buffer.
>    *
>    * Returns:
>    * Pointer to a client buffer or an error pointer on failure.
> @@ -416,19 +424,6 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
>   }
>   EXPORT_SYMBOL(drm_client_framebuffer_create);
>   
> -/**
> - * drm_client_framebuffer_delete - Delete a client framebuffer
> - * @buffer: DRM client buffer (can be NULL)
> - */
> -void drm_client_framebuffer_delete(struct drm_client_buffer *buffer)
> -{
> -	if (!buffer)
> -		return;
> -
> -	drm_client_buffer_delete(buffer);
> -}
> -EXPORT_SYMBOL(drm_client_framebuffer_delete);
> -
>   /**
>    * drm_client_framebuffer_flush - Manually flush client framebuffer
>    * @buffer: DRM client buffer (can be NULL)
> diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
> index 8bd626ef16c7..17fef288e86b 100644
> --- a/drivers/gpu/drm/drm_fbdev_dma.c
> +++ b/drivers/gpu/drm/drm_fbdev_dma.c
> @@ -55,7 +55,7 @@ static void drm_fbdev_dma_fb_destroy(struct fb_info *info)
>   	drm_fb_helper_fini(fb_helper);
>   
>   	drm_client_buffer_vunmap(fb_helper->buffer);
> -	drm_client_framebuffer_delete(fb_helper->buffer);
> +	drm_client_buffer_delete(fb_helper->buffer);
>   	drm_client_release(&fb_helper->client);
>   	drm_fb_helper_unprepare(fb_helper);
>   	kfree(fb_helper);
> @@ -90,7 +90,7 @@ static void drm_fbdev_dma_shadowed_fb_destroy(struct fb_info *info)
>   	vfree(shadow);
>   
>   	drm_client_buffer_vunmap(fb_helper->buffer);
> -	drm_client_framebuffer_delete(fb_helper->buffer);
> +	drm_client_buffer_delete(fb_helper->buffer);
>   	drm_client_release(&fb_helper->client);
>   	drm_fb_helper_unprepare(fb_helper);
>   	kfree(fb_helper);
> @@ -328,7 +328,7 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   	fb_helper->buffer = NULL;
>   	drm_client_buffer_vunmap(buffer);
>   err_drm_client_buffer_delete:
> -	drm_client_framebuffer_delete(buffer);
> +	drm_client_buffer_delete(buffer);
>   	return ret;
>   }
>   EXPORT_SYMBOL(drm_fbdev_dma_driver_fbdev_probe);
> diff --git a/drivers/gpu/drm/drm_fbdev_shmem.c b/drivers/gpu/drm/drm_fbdev_shmem.c
> index 1e827bf8b815..f7966b8c51cd 100644
> --- a/drivers/gpu/drm/drm_fbdev_shmem.c
> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
> @@ -63,7 +63,7 @@ static void drm_fbdev_shmem_fb_destroy(struct fb_info *info)
>   	drm_fb_helper_fini(fb_helper);
>   
>   	drm_client_buffer_vunmap(fb_helper->buffer);
> -	drm_client_framebuffer_delete(fb_helper->buffer);
> +	drm_client_buffer_delete(fb_helper->buffer);
>   	drm_client_release(&fb_helper->client);
>   	drm_fb_helper_unprepare(fb_helper);
>   	kfree(fb_helper);
> @@ -206,7 +206,7 @@ int drm_fbdev_shmem_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   	fb_helper->buffer = NULL;
>   	drm_client_buffer_vunmap(buffer);
>   err_drm_client_buffer_delete:
> -	drm_client_framebuffer_delete(buffer);
> +	drm_client_buffer_delete(buffer);
>   	return ret;
>   }
>   EXPORT_SYMBOL(drm_fbdev_shmem_driver_fbdev_probe);
> diff --git a/drivers/gpu/drm/drm_fbdev_ttm.c b/drivers/gpu/drm/drm_fbdev_ttm.c
> index 85feb55bba11..54f9a46b96c2 100644
> --- a/drivers/gpu/drm/drm_fbdev_ttm.c
> +++ b/drivers/gpu/drm/drm_fbdev_ttm.c
> @@ -50,7 +50,7 @@ static void drm_fbdev_ttm_fb_destroy(struct fb_info *info)
>   	fb_deferred_io_cleanup(info);
>   	drm_fb_helper_fini(fb_helper);
>   	vfree(shadow);
> -	drm_client_framebuffer_delete(fb_helper->buffer);
> +	drm_client_buffer_delete(fb_helper->buffer);
>   
>   	drm_client_release(&fb_helper->client);
>   	drm_fb_helper_unprepare(fb_helper);
> @@ -202,7 +202,7 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   	screen_buffer = vzalloc(screen_size);
>   	if (!screen_buffer) {
>   		ret = -ENOMEM;
> -		goto err_drm_client_framebuffer_delete;
> +		goto err_drm_client_buffer_delete;
>   	}
>   
>   	info = drm_fb_helper_alloc_info(fb_helper);
> @@ -235,10 +235,10 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   	drm_fb_helper_release_info(fb_helper);
>   err_vfree:
>   	vfree(screen_buffer);
> -err_drm_client_framebuffer_delete:
> +err_drm_client_buffer_delete:
>   	fb_helper->fb = NULL;
>   	fb_helper->buffer = NULL;
> -	drm_client_framebuffer_delete(buffer);
> +	drm_client_buffer_delete(buffer);
>   	return ret;
>   }
>   EXPORT_SYMBOL(drm_fbdev_ttm_driver_fbdev_probe);
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 5909664d4d43..6019f2712448 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -185,7 +185,7 @@ struct drm_client_buffer {
>   
>   struct drm_client_buffer *
>   drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
> -void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> +void drm_client_buffer_delete(struct drm_client_buffer *buffer);
>   int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
>   int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
>   				 struct iosys_map *map_copy);

Re: [PATCH 6/7] drm/client: Create client buffers with drm_client_buffer_create_dumb()

[Jocelyn Falempe]

On 20/10/2025 17:04, Thomas Zimmermann wrote:
> Rename drm_client_framebuffer_create() to drm_client_buffer_create_dump()
> and adapt callers. The new name reflects the function's purpose. Using
> dumb buffers is the easiest way for creating a GEM buffer in a drivers-
> independent way.
> 
> There's also drm_client_buffer_create(), which creates the client buffer
> from a preexisting buffer object. This helper can be exported for drivers
> that create their own GEM buffer object.

Thanks, it looks good to me.

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


> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/clients/drm_log.c | 2 +-
>   drivers/gpu/drm/drm_client.c      | 6 +++---
>   drivers/gpu/drm/drm_fbdev_dma.c   | 2 +-
>   drivers/gpu/drm/drm_fbdev_shmem.c | 2 +-
>   drivers/gpu/drm/drm_fbdev_ttm.c   | 2 +-
>   include/drm/drm_client.h          | 2 +-
>   6 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index 2d748ab318fe..b8fb9ee9fcf0 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -204,7 +204,7 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
>   	if (format == DRM_FORMAT_INVALID)
>   		return -EINVAL;
>   
> -	scanout->buffer = drm_client_framebuffer_create(client, width, height, format);
> +	scanout->buffer = drm_client_buffer_create_dumb(client, width, height, format);
>   	if (IS_ERR(scanout->buffer)) {
>   		drm_warn(client->dev, "drm_log can't create framebuffer %d %d %p4cc\n",
>   			 width, height, &format);
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index d4c424ff44a9..9105a0b5f468 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -372,7 +372,7 @@ void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
>   EXPORT_SYMBOL(drm_client_buffer_vunmap);
>   
>   /**
> - * drm_client_framebuffer_create - Create a client framebuffer
> + * drm_client_buffer_create_dumb - Create a client buffer backed by a dumb buffer
>    * @client: DRM client
>    * @width: Framebuffer width
>    * @height: Framebuffer height
> @@ -386,7 +386,7 @@ EXPORT_SYMBOL(drm_client_buffer_vunmap);
>    * Pointer to a client buffer or an error pointer on failure.
>    */
>   struct drm_client_buffer *
> -drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
> +drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 height, u32 format)
>   {
>   	const struct drm_format_info *info = drm_format_info(format);
>   	struct drm_device *dev = client->dev;
> @@ -422,7 +422,7 @@ drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 heig
>   	drm_mode_destroy_dumb(client->dev, dumb_args.handle, client->file);
>   	return ERR_PTR(ret);
>   }
> -EXPORT_SYMBOL(drm_client_framebuffer_create);
> +EXPORT_SYMBOL(drm_client_buffer_create_dumb);
>   
>   /**
>    * drm_client_framebuffer_flush - Manually flush client framebuffer
> diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
> index 17fef288e86b..7d4e557d6a5d 100644
> --- a/drivers/gpu/drm/drm_fbdev_dma.c
> +++ b/drivers/gpu/drm/drm_fbdev_dma.c
> @@ -285,7 +285,7 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   
>   	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp,
>   					     sizes->surface_depth);
> -	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
> +	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
>   					       sizes->surface_height, format);
>   	if (IS_ERR(buffer))
>   		return PTR_ERR(buffer);
> diff --git a/drivers/gpu/drm/drm_fbdev_shmem.c b/drivers/gpu/drm/drm_fbdev_shmem.c
> index f7966b8c51cd..b9e48a0bd39f 100644
> --- a/drivers/gpu/drm/drm_fbdev_shmem.c
> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
> @@ -149,7 +149,7 @@ int drm_fbdev_shmem_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   		    sizes->surface_bpp);
>   
>   	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp, sizes->surface_depth);
> -	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
> +	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
>   					       sizes->surface_height, format);
>   	if (IS_ERR(buffer))
>   		return PTR_ERR(buffer);
> diff --git a/drivers/gpu/drm/drm_fbdev_ttm.c b/drivers/gpu/drm/drm_fbdev_ttm.c
> index 54f9a46b96c2..b1188692399a 100644
> --- a/drivers/gpu/drm/drm_fbdev_ttm.c
> +++ b/drivers/gpu/drm/drm_fbdev_ttm.c
> @@ -189,7 +189,7 @@ int drm_fbdev_ttm_driver_fbdev_probe(struct drm_fb_helper *fb_helper,
>   
>   	format = drm_driver_legacy_fb_format(dev, sizes->surface_bpp,
>   					     sizes->surface_depth);
> -	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
> +	buffer = drm_client_buffer_create_dumb(client, sizes->surface_width,
>   					       sizes->surface_height, format);
>   	if (IS_ERR(buffer))
>   		return PTR_ERR(buffer);
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 6019f2712448..e4df404a9645 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -184,7 +184,7 @@ struct drm_client_buffer {
>   };
>   
>   struct drm_client_buffer *
> -drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
> +drm_client_buffer_create_dumb(struct drm_client_dev *client, u32 width, u32 height, u32 format);
>   void drm_client_buffer_delete(struct drm_client_buffer *buffer);
>   int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct drm_rect *rect);
>   int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,

Re: [PATCH 5/7] drm/client: Remove drm_client_framebuffer_delete()

[Dan Carpenter]

Hi Thomas,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-client-Remove-pitch-from-struct-drm_client_buffer/20251020-231709
base:   git://anongit.freedesktop.org/drm/drm drm-next
patch link:    https://lore.kernel.org/r/20251020151402.53013-6-tzimmermann%40suse.de
patch subject: [PATCH 5/7] drm/client: Remove drm_client_framebuffer_delete()
config: openrisc-randconfig-r072-20251021 (https://download.01.org/0day-ci/archive/20251022/202510220056.J4uIVWxn-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 12.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202510220056.J4uIVWxn-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/drm_client.c:185 drm_client_buffer_delete() warn: variable dereferenced before check 'buffer' (see line 182)

vim +/buffer +185 drivers/gpu/drm/drm_client.c

449148a7300b8b Thomas Zimmermann 2025-10-20  180  void drm_client_buffer_delete(struct drm_client_buffer *buffer)
c76f0f7cb546b6 Noralf Trønnes    2018-07-03  181  {
95d6cf3a3cf1a1 Thomas Zimmermann 2025-10-20 @182  	struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
                                                                                                        ^^^^^^^^^^
buffer is dereferenced here

82e2a75e5a640c Thomas Zimmermann 2025-10-20  183  	int ret;
82e2a75e5a640c Thomas Zimmermann 2025-10-20  184  
449148a7300b8b Thomas Zimmermann 2025-10-20 @185  	if (!buffer)
                                                            ^^^^^^^
checked too late

449148a7300b8b Thomas Zimmermann 2025-10-20  186  		return;
449148a7300b8b Thomas Zimmermann 2025-10-20  187  
95d6cf3a3cf1a1 Thomas Zimmermann 2025-10-20  188  	drm_gem_vunmap(gem, &buffer->map);
95d6cf3a3cf1a1 Thomas Zimmermann 2025-10-20  189  
82e2a75e5a640c Thomas Zimmermann 2025-10-20  190  	ret = drm_mode_rmfb(buffer->client->dev, buffer->fb->base.id, buffer->client->file);
82e2a75e5a640c Thomas Zimmermann 2025-10-20  191  	if (ret)
82e2a75e5a640c Thomas Zimmermann 2025-10-20  192  		drm_err(buffer->client->dev,
82e2a75e5a640c Thomas Zimmermann 2025-10-20  193  			"Error removing FB:%u (%d)\n", buffer->fb->base.id, ret);
82e2a75e5a640c Thomas Zimmermann 2025-10-20  194  
be6ee102341bc4 Emil Velikov      2020-05-15  195  	drm_gem_object_put(buffer->gem);
c76f0f7cb546b6 Noralf Trønnes    2018-07-03  196  
c76f0f7cb546b6 Noralf Trønnes    2018-07-03  197  	kfree(buffer);
c76f0f7cb546b6 Noralf Trønnes    2018-07-03  198  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-misc/drm-misc-next drm-tip/drm-tip linus/master v6.18-rc2 next-20251024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/drm-client-Remove-pitch-from-struct-drm_client_buffer/20251020-231709
base:   git://anongit.freedesktop.org/drm/drm drm-next
patch link:    https://lore.kernel.org/r/20251020151402.53013-5-tzimmermann%40suse.de
patch subject: [PATCH 4/7] drm/client: Deprecate struct drm_client_buffer.gem
config: arm-multi_v5_defconfig (https://download.01.org/0day-ci/archive/20251026/202510261717.a4f1joxt-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251026/202510261717.a4f1joxt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510261717.a4f1joxt-lkp@intel.com/

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: drivers/gpu/drm/drm_client.o: in function `drm_client_buffer_vmap_local':
>> drivers/gpu/drm/drm_client.c:278:(.text+0x214): undefined reference to `drm_gem_fb_get_obj'
   arm-linux-gnueabi-ld: drivers/gpu/drm/drm_client.o: in function `drm_client_buffer_vunmap_local':
   drivers/gpu/drm/drm_client.c:307:(.text+0x280): undefined reference to `drm_gem_fb_get_obj'
   arm-linux-gnueabi-ld: drivers/gpu/drm/drm_client.o: in function `drm_client_buffer_vmap':
   drivers/gpu/drm/drm_client.c:338:(.text+0x2b4): undefined reference to `drm_gem_fb_get_obj'
   arm-linux-gnueabi-ld: drivers/gpu/drm/drm_client.o: in function `drm_client_buffer_vunmap':
   drivers/gpu/drm/drm_client.c:360:(.text+0x2ec): undefined reference to `drm_gem_fb_get_obj'
   arm-linux-gnueabi-ld: drivers/gpu/drm/drm_client.o: in function `drm_client_buffer_delete':
   drivers/gpu/drm/drm_client.c:178:(.text+0x6ec): undefined reference to `drm_gem_fb_get_obj'


vim +278 drivers/gpu/drm/drm_client.c

   254	
   255	/**
   256	 * drm_client_buffer_vmap_local - Map DRM client buffer into address space
   257	 * @buffer: DRM client buffer
   258	 * @map_copy: Returns the mapped memory's address
   259	 *
   260	 * This function maps a client buffer into kernel address space. If the
   261	 * buffer is already mapped, it returns the existing mapping's address.
   262	 *
   263	 * Client buffer mappings are not ref'counted. Each call to
   264	 * drm_client_buffer_vmap_local() should be closely followed by a call to
   265	 * drm_client_buffer_vunmap_local(). See drm_client_buffer_vmap() for
   266	 * long-term mappings.
   267	 *
   268	 * The returned address is a copy of the internal value. In contrast to
   269	 * other vmap interfaces, you don't need it for the client's vunmap
   270	 * function. So you can modify it at will during blit and draw operations.
   271	 *
   272	 * Returns:
   273	 *	0 on success, or a negative errno code otherwise.
   274	 */
   275	int drm_client_buffer_vmap_local(struct drm_client_buffer *buffer,
   276					 struct iosys_map *map_copy)
   277	{
 > 278		struct drm_gem_object *gem = drm_gem_fb_get_obj(buffer->fb, 0);
   279		struct iosys_map *map = &buffer->map;
   280		int ret;
   281	
   282		drm_gem_lock(gem);
   283	
   284		ret = drm_gem_vmap_locked(gem, map);
   285		if (ret)
   286			goto err_drm_gem_vmap_unlocked;
   287		*map_copy = *map;
   288	
   289		return 0;
   290	
   291	err_drm_gem_vmap_unlocked:
   292		drm_gem_unlock(gem);
   293		return ret;
   294	}
   295	EXPORT_SYMBOL(drm_client_buffer_vmap_local);
   296	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki