* [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers
@ 2026-05-11 11:54 Thomas Zimmermann
2026-05-11 11:54 ` [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer Thomas Zimmermann
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann
A client buffer holds the DRM framebuffer for an in-kernel DRM
client. Until now, exynos created an internal ad-hoc framebuffer for
its fbdev emulation, while by-passing the regular interfaces used by
user-space compositors.
Convert exynos' fbdev emulation to use client buffers. Replacing the
existing code with a client buffer allows for stream-lining exynos code
and later also the fbdev helpers. The new framebuffer will be registered
against the client's file and will support handles for GEM objects. It
is then just another framebuffer within the DRM ecosystem.
If all driver's fbdev-emulation helpers can be converted to use client
buffers, the emulation's framebuffer handling as a whole can possibly be
moved into shared helpers.
Patch 1 first fixes a long-standing bug.
Patches 2 to 4 convert exynos' fbdev emulation to client buffers. It
still allocates a GEM object buffer tailored towards fbdev emulation,
but size calculations now use common DRM helpers.
Patch 5 cleans up symbol visibility in exynos' fb code.
v3:
- add error checks to geometry calculation
v2:
- clean up exynos fb header (Chen-Yu)
Thomas Zimmermann (5):
drm/exynos: fbdev: Remove offset into screen_buffer
drm/exynos: fbdev: Inline exynos_drm_fbdev_update()
drm/exynos: fbdev: Calculate buffer geometry with format helpers
drm/exynos: fbdev: Use a DRM client buffer
drm/exynos: Make exynos_drm_framebuffer_init() an internal interface
drivers/gpu/drm/exynos/exynos_drm_fb.c | 3 +-
drivers/gpu/drm/exynos/exynos_drm_fb.h | 10 +-
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 111 +++++++++++-----------
3 files changed, 62 insertions(+), 62 deletions(-)
base-commit: 680a4832a8182b13ac5d55ab0c03c54ebc2baadd
--
2.54.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
@ 2026-05-11 11:54 ` Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 2/5] drm/exynos: fbdev: Inline exynos_drm_fbdev_update() Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann, linux-arm-kernel,
stable
The screen_buffer field in struct fb_info contains the kernel address
of the first byte of framebuffer memory. Do not add the display offset.
This offset only describes scrolling during scanout.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 19c8b8343d9c ("drm/exynos: fixed overlay data updating.")
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: <stable@vger.kernel.org> # v3.2+
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 637927818dfe..d283ded266d5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -61,17 +61,13 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
struct fb_info *fbi = helper->info;
struct drm_framebuffer *fb = helper->fb;
unsigned int size = fb->width * fb->height * fb->format->cpp[0];
- unsigned long offset;
fbi->fbops = &exynos_drm_fb_ops;
drm_fb_helper_fill_info(fbi, helper, sizes);
- offset = fbi->var.xoffset * fb->format->cpp[0];
- offset += fbi->var.yoffset * fb->pitches[0];
-
fbi->flags |= FBINFO_VIRTFB;
- fbi->screen_buffer = exynos_gem->kvaddr + offset;
+ fbi->screen_buffer = exynos_gem->kvaddr;
fbi->screen_size = size;
fbi->fix.smem_len = size;
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/5] drm/exynos: fbdev: Inline exynos_drm_fbdev_update()
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
2026-05-11 11:54 ` [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer Thomas Zimmermann
@ 2026-05-11 11:54 ` Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 3/5] drm/exynos: fbdev: Calculate buffer geometry with format helpers Thomas Zimmermann
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann
Inline exynos_drm_fbdev_update() into its only caller. Prepares the
code for using DRM client buffers.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 35 ++++++-----------------
1 file changed, 9 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index d283ded266d5..1c564edd497e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -54,26 +54,6 @@ static const struct fb_ops exynos_drm_fb_ops = {
.fb_destroy = exynos_drm_fb_destroy,
};
-static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
- struct drm_fb_helper_surface_size *sizes,
- struct exynos_drm_gem *exynos_gem)
-{
- struct fb_info *fbi = helper->info;
- struct drm_framebuffer *fb = helper->fb;
- unsigned int size = fb->width * fb->height * fb->format->cpp[0];
-
- fbi->fbops = &exynos_drm_fb_ops;
-
- drm_fb_helper_fill_info(fbi, helper, sizes);
-
- fbi->flags |= FBINFO_VIRTFB;
- fbi->screen_buffer = exynos_gem->kvaddr;
- fbi->screen_size = size;
- fbi->fix.smem_len = size;
-
- return 0;
-}
-
static const struct drm_fb_helper_funcs exynos_drm_fbdev_helper_funcs = {
};
@@ -82,6 +62,7 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
{
struct exynos_drm_gem *exynos_gem;
struct drm_device *dev = helper->dev;
+ struct fb_info *info = helper->info;
struct drm_mode_fb_cmd2 mode_cmd = { 0 };
unsigned long size;
int ret;
@@ -115,15 +96,17 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
}
helper->funcs = &exynos_drm_fbdev_helper_funcs;
- ret = exynos_drm_fbdev_update(helper, sizes, exynos_gem);
- if (ret < 0)
- goto err_destroy_framebuffer;
+ info->fbops = &exynos_drm_fb_ops;
+
+ drm_fb_helper_fill_info(info, helper, sizes);
+
+ info->flags |= FBINFO_VIRTFB;
+ info->screen_buffer = exynos_gem->kvaddr;
+ info->screen_size = size;
+ info->fix.smem_len = size;
return 0;
-err_destroy_framebuffer:
- drm_framebuffer_cleanup(helper->fb);
- helper->fb = NULL;
err_destroy_gem:
exynos_drm_gem_destroy(exynos_gem);
return ret;
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/5] drm/exynos: fbdev: Calculate buffer geometry with format helpers
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
2026-05-11 11:54 ` [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer Thomas Zimmermann
2026-05-11 11:54 ` [PATCH v3 2/5] drm/exynos: fbdev: Inline exynos_drm_fbdev_update() Thomas Zimmermann
@ 2026-05-11 11:54 ` Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 4/5] drm/exynos: fbdev: Use a DRM client buffer Thomas Zimmermann
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann
Replace the geometry and size calculation in exynos' fbdev emulation
with DRM format helpers. This consists of a 4CC lookup from the fbdev
parameters, format lookup, pitch calculation and size calculation.
Then allocate the GEM buffer object for the framebuffer memory from
the calculated size. Mmap provides the allocated buffer to user space,
so align the buffer size to PAGE_SIZE.
Initialize the fields screen_size and fix.smem_len in struct fb_info
from the size of the allocated buffer object. This is the real size
and can differ from the requested size.
v3:
- add more error checks to geometry calculations
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 45 +++++++++++++++--------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 1c564edd497e..9163efd676dd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -60,11 +60,14 @@ static const struct drm_fb_helper_funcs exynos_drm_fbdev_helper_funcs = {
int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
- struct exynos_drm_gem *exynos_gem;
struct drm_device *dev = helper->dev;
struct fb_info *info = helper->info;
+ u32 fourcc, pitch;
+ u64 size;
+ const struct drm_format_info *format;
+ struct exynos_drm_gem *exynos_gem;
+ struct drm_gem_object *obj;
struct drm_mode_fb_cmd2 mode_cmd = { 0 };
- unsigned long size;
int ret;
DRM_DEV_DEBUG_KMS(dev->dev,
@@ -72,23 +75,33 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
sizes->surface_width, sizes->surface_height,
sizes->surface_bpp);
- mode_cmd.width = sizes->surface_width;
- mode_cmd.height = sizes->surface_height;
- mode_cmd.pitches[0] = sizes->surface_width * (sizes->surface_bpp >> 3);
- mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
- sizes->surface_depth);
-
- size = mode_cmd.pitches[0] * mode_cmd.height;
+ fourcc = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
+ if (fourcc == DRM_FORMAT_INVALID)
+ return -EINVAL;
+ format = drm_get_format_info(dev, fourcc, DRM_FORMAT_MOD_LINEAR);
+ if (!format)
+ return -EINVAL;
+ pitch = drm_format_info_min_pitch(format, 0, sizes->surface_width);
+ if (!pitch)
+ return -EINVAL;
+ if (check_mul_overflow(pitch, sizes->surface_height, &size))
+ return -EINVAL;
+ size = ALIGN(size, PAGE_SIZE);
+ if (size < PAGE_SIZE)
+ return -EINVAL;
exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_WC, size, true);
if (IS_ERR(exynos_gem))
return PTR_ERR(exynos_gem);
+ obj = &exynos_gem->base;
+
+ mode_cmd.width = sizes->surface_width;
+ mode_cmd.height = sizes->surface_height;
+ mode_cmd.pixel_format = fourcc;
+ mode_cmd.pitches[0] = pitch;
+ mode_cmd.modifier[0] = DRM_FORMAT_MOD_LINEAR;
- helper->fb =
- exynos_drm_framebuffer_init(dev,
- drm_get_format_info(dev, mode_cmd.pixel_format,
- mode_cmd.modifier[0]),
- &mode_cmd, &exynos_gem, 1);
+ helper->fb = exynos_drm_framebuffer_init(dev, format, &mode_cmd, &exynos_gem, 1);
if (IS_ERR(helper->fb)) {
DRM_DEV_ERROR(dev->dev, "failed to create drm framebuffer.\n");
ret = PTR_ERR(helper->fb);
@@ -102,8 +115,8 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
info->flags |= FBINFO_VIRTFB;
info->screen_buffer = exynos_gem->kvaddr;
- info->screen_size = size;
- info->fix.smem_len = size;
+ info->screen_size = obj->size;
+ info->fix.smem_len = obj->size;
return 0;
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/5] drm/exynos: fbdev: Use a DRM client buffer
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
` (2 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v3 3/5] drm/exynos: fbdev: Calculate buffer geometry with format helpers Thomas Zimmermann
@ 2026-05-11 11:54 ` Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 5/5] drm/exynos: Make exynos_drm_framebuffer_init() an internal interface Thomas Zimmermann
2026-05-16 5:38 ` Claude review: drm/exynos: fbdev: Use client buffers Claude Code Review Bot
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann
Replace the internal DRM framebuffer with a DRM client buffer. The
client buffer allocates the DRM framebuffer on a file and also uses
GEM object handles via the regular ADDFB2 interfaces.
Using client-buffer interfaces unifies framebuffer allocation for
DRM clients in user space and exynos' internal fbdev emulation. It
also simplifies the clean-up side of the fbdev emulation.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 46 ++++++++++++++---------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 9163efd676dd..121c342dd14a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -36,12 +36,10 @@ static int exynos_drm_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
static void exynos_drm_fb_destroy(struct fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
- struct drm_framebuffer *fb = fb_helper->fb;
drm_fb_helper_fini(fb_helper);
- drm_framebuffer_remove(fb);
-
+ drm_client_buffer_delete(fb_helper->buffer);
drm_client_release(&fb_helper->client);
}
@@ -60,14 +58,17 @@ static const struct drm_fb_helper_funcs exynos_drm_fbdev_helper_funcs = {
int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
- struct drm_device *dev = helper->dev;
+ struct drm_client_dev *client = &helper->client;
+ struct drm_device *dev = client->dev;
+ struct drm_file *file = client->file;
struct fb_info *info = helper->info;
u32 fourcc, pitch;
u64 size;
const struct drm_format_info *format;
struct exynos_drm_gem *exynos_gem;
struct drm_gem_object *obj;
- struct drm_mode_fb_cmd2 mode_cmd = { 0 };
+ struct drm_client_buffer *buffer;
+ u32 handle;
int ret;
DRM_DEV_DEBUG_KMS(dev->dev,
@@ -95,19 +96,20 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
return PTR_ERR(exynos_gem);
obj = &exynos_gem->base;
- mode_cmd.width = sizes->surface_width;
- mode_cmd.height = sizes->surface_height;
- mode_cmd.pixel_format = fourcc;
- mode_cmd.pitches[0] = pitch;
- mode_cmd.modifier[0] = DRM_FORMAT_MOD_LINEAR;
-
- helper->fb = exynos_drm_framebuffer_init(dev, format, &mode_cmd, &exynos_gem, 1);
- if (IS_ERR(helper->fb)) {
- DRM_DEV_ERROR(dev->dev, "failed to create drm framebuffer.\n");
- ret = PTR_ERR(helper->fb);
- goto err_destroy_gem;
+ ret = drm_gem_handle_create(file, obj, &handle);
+ if (ret)
+ goto err_drm_gem_object_put;
+
+ buffer = drm_client_buffer_create(client, sizes->surface_width, sizes->surface_height,
+ fourcc, handle, pitch);
+ if (IS_ERR(buffer)) {
+ ret = PTR_ERR(buffer);
+ goto err_drm_gem_handle_delete;
}
+
helper->funcs = &exynos_drm_fbdev_helper_funcs;
+ helper->buffer = buffer;
+ helper->fb = buffer->fb;
info->fbops = &exynos_drm_fb_ops;
@@ -118,9 +120,17 @@ int exynos_drm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
info->screen_size = obj->size;
info->fix.smem_len = obj->size;
+ /* The handle is only needed for creating the framebuffer. */
+ drm_gem_handle_delete(file, handle);
+
+ /* The framebuffer still holds a reference on the GEM object. */
+ drm_gem_object_put(obj);
+
return 0;
-err_destroy_gem:
- exynos_drm_gem_destroy(exynos_gem);
+err_drm_gem_handle_delete:
+ drm_gem_handle_delete(file, handle);
+err_drm_gem_object_put:
+ drm_gem_object_put(obj);
return ret;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 5/5] drm/exynos: Make exynos_drm_framebuffer_init() an internal interface
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
` (3 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v3 4/5] drm/exynos: fbdev: Use a DRM client buffer Thomas Zimmermann
@ 2026-05-11 11:54 ` Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-16 5:38 ` Claude review: drm/exynos: fbdev: Use client buffers Claude Code Review Bot
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-05-11 11:54 UTC (permalink / raw)
To: inki.dae, sw0312.kim, kyungmin.park, m.szyprowski, wens, airlied,
simona
Cc: dri-devel, linux-samsung-soc, Thomas Zimmermann
The only caller of exynos_drm_framebuffer_init() is the helper
exynos_user_fb_create() from the same source file. Declare the
former as static. Tidy up the header's include statements.
v2:
- clean up the includes in the header file (Chen-Yu)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fb.c | 3 ++-
drivers/gpu/drm/exynos/exynos_drm_fb.h | 10 +++-------
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 1 +
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index ab0e0c74ec47..c6a33f550ace 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -22,6 +22,7 @@
#include "exynos_drm_drv.h"
#include "exynos_drm_fb.h"
#include "exynos_drm_fbdev.h"
+#include "exynos_drm_gem.h"
static int check_fb_gem_memory_type(struct drm_device *drm_dev,
struct exynos_drm_gem *exynos_gem)
@@ -55,7 +56,7 @@ static const struct drm_framebuffer_funcs exynos_drm_fb_funcs = {
.create_handle = drm_gem_fb_create_handle,
};
-struct drm_framebuffer *
+static struct drm_framebuffer *
exynos_drm_framebuffer_init(struct drm_device *dev,
const struct drm_format_info *info,
const struct drm_mode_fb_cmd2 *mode_cmd,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.h b/drivers/gpu/drm/exynos/exynos_drm_fb.h
index fdc6cb40cc9c..74300ad9bb51 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.h
@@ -10,14 +10,10 @@
#ifndef _EXYNOS_DRM_FB_H_
#define _EXYNOS_DRM_FB_H_
-#include "exynos_drm_gem.h"
+#include <linux/types.h>
-struct drm_framebuffer *
-exynos_drm_framebuffer_init(struct drm_device *dev,
- const struct drm_format_info *info,
- const struct drm_mode_fb_cmd2 *mode_cmd,
- struct exynos_drm_gem **exynos_gem,
- int count);
+struct drm_device;
+struct drm_framebuffer;
dma_addr_t exynos_drm_fb_dma_addr(struct drm_framebuffer *fb, int index);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 121c342dd14a..8641c9e84d4c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -22,6 +22,7 @@
#include "exynos_drm_drv.h"
#include "exynos_drm_fb.h"
#include "exynos_drm_fbdev.h"
+#include "exynos_drm_gem.h"
#define MAX_CONNECTOR 4
--
2.54.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: fbdev: Use client buffers
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
` (4 preceding siblings ...)
2026-05-11 11:54 ` [PATCH v3 5/5] drm/exynos: Make exynos_drm_framebuffer_init() an internal interface Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
5 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/exynos: fbdev: Use client buffers
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 6
Reviewed: 2026-05-16T15:38:04.172880
---
This is a clean, well-structured 5-patch series from Thomas Zimmermann that converts the Exynos DRM fbdev emulation from using an internal ad-hoc framebuffer to using DRM client buffers. The progression is logical: fix a bug (patch 1), inline the helper (patch 2), modernize geometry calculation (patch 3), switch to client buffers (patch 4), and clean up symbol visibility (patch 5).
The series has Tested-by and Acked-by from Marek Szyprowski (Samsung platform maintainer), and patch 1 correctly carries a Fixes tag with Cc: stable. The code is correct, reference counting is handled properly, and error paths are clean.
**Verdict: Looks good. No blocking issues.**
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: fbdev: Remove offset into screen_buffer
2026-05-11 11:54 ` [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good**
This is a straightforward bugfix. The `screen_buffer` field in `struct fb_info` should point to the start of framebuffer memory, not to an offset within it. The `xoffset`/`yoffset` values describe scrolling during scanout and should not be baked into the base address.
```c
- offset = fbi->var.xoffset * fb->format->cpp[0];
- offset += fbi->var.yoffset * fb->pitches[0];
...
- fbi->screen_buffer = exynos_gem->kvaddr + offset;
+ fbi->screen_buffer = exynos_gem->kvaddr;
```
The Fixes tag and stable Cc are appropriate. The offsets are typically 0 at fbdev setup time anyway (`drm_fb_helper_fill_info` sets `var.xoffset = var.yoffset = 0`), so this was mostly a latent bug rather than a functional regression, but still worth fixing for correctness.
No concerns.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: fbdev: Inline exynos_drm_fbdev_update()
2026-05-11 11:54 ` [PATCH v3 2/5] drm/exynos: fbdev: Inline exynos_drm_fbdev_update() Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good**
`exynos_drm_fbdev_update()` was a single-caller function that always returned 0. Inlining it removes the `err_destroy_framebuffer` error label, which was dead code since the function could never fail.
The inlined assignments are all non-failable:
```c
+ info->fbops = &exynos_drm_fb_ops;
+ drm_fb_helper_fill_info(info, helper, sizes);
+ info->flags |= FBINFO_VIRTFB;
+ info->screen_buffer = exynos_gem->kvaddr;
+ info->screen_size = size;
+ info->fix.smem_len = size;
```
Removing the dead `err_destroy_framebuffer` path is correct — if something failed here (it can't), the fbdev teardown via `fb_destroy` would clean up the framebuffer anyway.
No concerns.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: fbdev: Calculate buffer geometry with format helpers
2026-05-11 11:54 ` [PATCH v3 3/5] drm/exynos: fbdev: Calculate buffer geometry with format helpers Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good, one minor note**
This replaces the hand-rolled geometry calculation with proper DRM format helpers. The v3 error checks are thorough:
```c
fourcc = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
if (fourcc == DRM_FORMAT_INVALID)
return -EINVAL;
format = drm_get_format_info(dev, fourcc, DRM_FORMAT_MOD_LINEAR);
if (!format)
return -EINVAL;
pitch = drm_format_info_min_pitch(format, 0, sizes->surface_width);
if (!pitch)
return -EINVAL;
if (check_mul_overflow(pitch, sizes->surface_height, &size))
return -EINVAL;
size = ALIGN(size, PAGE_SIZE);
if (size < PAGE_SIZE)
return -EINVAL;
```
**Minor note on types:** `drm_format_info_min_pitch()` returns `uint64_t` but `pitch` is declared as `u32`. For fbdev surface sizes this is never a practical issue, and the resulting pitch is consumed by `mode_cmd.pitches[0]` (also `u32`) and `drm_client_buffer_create(..., u32 pitch)`, so `u32` is the right type for the interface contract. The `!pitch` check catches the degenerate case where the return was 0. If the pitch somehow exceeded 32 bits (theoretically impossible for reasonable fbdev sizes), it would silently truncate, but this matches the existing API constraints.
The switch to `obj->size` for `screen_size` and `fix.smem_len` is correct — it reflects the actual allocated size (which may be larger than the requested size after GEM allocation alignment):
```c
+ info->screen_size = obj->size;
+ info->fix.smem_len = obj->size;
```
No blocking concerns.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: fbdev: Use a DRM client buffer
2026-05-11 11:54 ` [PATCH v3 4/5] drm/exynos: fbdev: Use a DRM client buffer Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good**
This is the core change. The old flow created a framebuffer directly via `exynos_drm_framebuffer_init()`. The new flow:
1. Creates a GEM handle via `drm_gem_handle_create()`
2. Creates a client buffer via `drm_client_buffer_create()` (which internally does ADDFB2)
3. Cleans up the handle and drops the initial GEM reference
The reference counting is correct:
```c
/* The handle is only needed for creating the framebuffer. */
drm_gem_handle_delete(file, handle);
/* The framebuffer still holds a reference on the GEM object. */
drm_gem_object_put(obj);
```
After these two lines, the GEM object survives with exactly one reference held by the framebuffer created inside `drm_client_buffer_create()`. The `exynos_gem->kvaddr` read happens before `drm_gem_object_put()`, so we're dereferencing the pointer while still holding our own reference. The stored `kvaddr` value remains valid as long as the framebuffer exists.
The cleanup in `exynos_drm_fb_destroy` is simplified:
```c
- drm_framebuffer_remove(fb);
+ drm_client_buffer_delete(fb_helper->buffer);
```
`drm_client_buffer_delete()` handles framebuffer removal and releasing the GEM reference, which is the right cleanup for the client buffer path.
The error paths correctly unwind in reverse order:
```c
err_drm_gem_handle_delete:
drm_gem_handle_delete(file, handle);
err_drm_gem_object_put:
drm_gem_object_put(obj);
```
Note: the old error path used `exynos_drm_gem_destroy(exynos_gem)` while the new one uses `drm_gem_object_put(obj)`. Since `exynos_drm_gem_create()` returns the object with refcount=1, `drm_gem_object_put` is the correct generic way to release it (it will call the destroy callback when refcount drops to 0). This is a clean improvement.
No concerns.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/exynos: Make exynos_drm_framebuffer_init() an internal interface
2026-05-11 11:54 ` [PATCH v3 5/5] drm/exynos: Make exynos_drm_framebuffer_init() an internal interface Thomas Zimmermann
@ 2026-05-16 5:38 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-05-16 5:38 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Status: Good**
After patch 4 removed the fbdev caller of `exynos_drm_framebuffer_init()`, the only remaining caller is `exynos_user_fb_create()` in the same file. Making it `static` is the right thing to do:
```c
-struct drm_framebuffer *
+static struct drm_framebuffer *
exynos_drm_framebuffer_init(...)
```
The header cleanup is also correct — replacing the `#include "exynos_drm_gem.h"` (which was only needed for the now-removed declaration) with minimal forward declarations and `<linux/types.h>` for `dma_addr_t`:
```c
-#include "exynos_drm_gem.h"
+#include <linux/types.h>
-struct drm_framebuffer *
-exynos_drm_framebuffer_init(...);
+struct drm_device;
+struct drm_framebuffer;
```
The compensating `#include "exynos_drm_gem.h"` additions in `exynos_drm_fb.c` and `exynos_drm_fbdev.c` are correct — both files use `exynos_drm_gem` types and were previously getting the header transitively through `exynos_drm_fb.h`.
No concerns.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-16 5:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 11:54 [PATCH v3 0/5] drm/exynos: fbdev: Use client buffers Thomas Zimmermann
2026-05-11 11:54 ` [PATCH v3 1/5] drm/exynos: fbdev: Remove offset into screen_buffer Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 2/5] drm/exynos: fbdev: Inline exynos_drm_fbdev_update() Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 3/5] drm/exynos: fbdev: Calculate buffer geometry with format helpers Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 4/5] drm/exynos: fbdev: Use a DRM client buffer Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-11 11:54 ` [PATCH v3 5/5] drm/exynos: Make exynos_drm_framebuffer_init() an internal interface Thomas Zimmermann
2026-05-16 5:38 ` Claude review: " Claude Code Review Bot
2026-05-16 5:38 ` Claude review: drm/exynos: fbdev: Use client buffers Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox