* [PATCH 0/6] drm/gud: Use the shadow plane helper
@ 2022-11-22 20:58 Noralf Trønnes
2022-11-22 20:58 ` [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access Noralf Trønnes
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
Hi,
I have started to look at igt for testing and want to use CRC tests. To
implement support for this I need to move away from the simple kms
helper.
When looking around for examples I came across Thomas' nice shadow
helper and thought, yes this is perfect for drm/gud. So I'll switch to
that before I move away from the simple kms helper.
The async framebuffer flushing code path now uses a shadow buffer and
doesn't touch the framebuffer when it shouldn't. I have also taken the
opportunity to inline the synchronous flush code path since this will be
the future default when userspace predominently don't run all displays
in the same rendering loop. A shared rendering loop slows down all
displays to run at the speed of the slowest one.
Noralf.
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
Noralf Trønnes (6):
drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
drm/gud: Fix UBSAN warning
drm/gud: Don't retry a failed framebuffer flush
drm/gud: Split up gud_flush_work()
drm/gud: Prepare buffer for CPU access in gud_flush_work()
drm/gud: Use the shadow plane helper
drivers/gpu/drm/drm_gem_atomic_helper.c | 13 ++-
drivers/gpu/drm/gud/gud_drv.c | 1 +
drivers/gpu/drm/gud/gud_internal.h | 1 +
drivers/gpu/drm/gud/gud_pipe.c | 198 +++++++++++++++-----------------
drivers/gpu/drm/solomon/ssd130x.c | 10 +-
drivers/gpu/drm/tiny/gm12u320.c | 10 +-
drivers/gpu/drm/tiny/ofdrm.c | 10 +-
drivers/gpu/drm/tiny/simpledrm.c | 10 +-
drivers/gpu/drm/udl/udl_modeset.c | 11 +-
9 files changed, 117 insertions(+), 147 deletions(-)
---
base-commit: 7257702951305b1f0259c3468c39fc59d1ad4d8b
change-id: 20221122-gud-shadow-plane-ae37a95d4d8d
Best regards,
--
Noralf Trønnes <noralf@tronnes.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
2022-11-22 20:58 ` [PATCH 2/6] drm/gud: Fix UBSAN warning Noralf Trønnes
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
Complete the shadow fb access functions by also preparing imported buffers
for CPU access. Update the affected drivers that currently use
drm_gem_fb_begin_cpu_access().
Through this change the following SHMEM drivers will now also make sure
their imported buffers are prepared for CPU access: cirrus, hyperv,
mgag200, vkms
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/drm_gem_atomic_helper.c | 13 ++++++++++++-
drivers/gpu/drm/solomon/ssd130x.c | 10 +---------
drivers/gpu/drm/tiny/gm12u320.c | 10 +---------
drivers/gpu/drm/tiny/ofdrm.c | 10 ++--------
drivers/gpu/drm/tiny/simpledrm.c | 10 ++--------
drivers/gpu/drm/udl/udl_modeset.c | 11 ++---------
6 files changed, 20 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e42800718f51..0eef4bb30d25 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -368,6 +368,7 @@ EXPORT_SYMBOL(drm_gem_reset_shadow_plane);
* maps all buffer objects of the plane's framebuffer into kernel address
* space and stores them in struct &drm_shadow_plane_state.map. The first data
* bytes are available in struct &drm_shadow_plane_state.data.
+ * It also prepares imported buffers for CPU access.
*
* See drm_gem_end_shadow_fb_access() for cleanup.
*
@@ -378,11 +379,20 @@ int drm_gem_begin_shadow_fb_access(struct drm_plane *plane, struct drm_plane_sta
{
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
+ int ret;
if (!fb)
return 0;
- return drm_gem_fb_vmap(fb, shadow_plane_state->map, shadow_plane_state->data);
+ ret = drm_gem_fb_vmap(fb, shadow_plane_state->map, shadow_plane_state->data);
+ if (ret)
+ return ret;
+
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret)
+ drm_gem_fb_vunmap(fb, shadow_plane_state->map);
+
+ return ret;
}
EXPORT_SYMBOL(drm_gem_begin_shadow_fb_access);
@@ -404,6 +414,7 @@ void drm_gem_end_shadow_fb_access(struct drm_plane *plane, struct drm_plane_stat
if (!fb)
return;
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
drm_gem_fb_vunmap(fb, shadow_plane_state->map);
}
EXPORT_SYMBOL(drm_gem_end_shadow_fb_access);
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 53464afc2b9a..58a2f0113f24 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -544,7 +544,6 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
struct iosys_map dst;
unsigned int dst_pitch;
- int ret = 0;
u8 *buf = NULL;
/* Align y to display page boundaries */
@@ -556,21 +555,14 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
if (!buf)
return -ENOMEM;
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- goto out_free;
-
iosys_map_set_vaddr(&dst, buf);
drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-
ssd130x_update_rect(ssd130x, buf, rect);
-out_free:
kfree(buf);
- return ret;
+ return 0;
}
static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 130fd07a967d..59aad4b468cc 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -252,7 +252,7 @@ static void gm12u320_32bpp_to_24bpp_packed(u8 *dst, u8 *src, int len)
static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
{
- int block, dst_offset, len, remain, ret, x1, x2, y1, y2;
+ int block, dst_offset, len, remain, x1, x2, y1, y2;
struct drm_framebuffer *fb;
void *vaddr;
u8 *src;
@@ -269,12 +269,6 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
y2 = gm12u320->fb_update.rect.y2;
vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping abstraction properly */
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret) {
- GM12U320_ERR("drm_gem_fb_begin_cpu_access err: %d\n", ret);
- goto put_fb;
- }
-
src = vaddr + y1 * fb->pitches[0] + x1 * 4;
x1 += (GM12U320_REAL_WIDTH - GM12U320_USER_WIDTH) / 2;
@@ -309,8 +303,6 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
src += fb->pitches[0];
}
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-put_fb:
drm_framebuffer_put(fb);
gm12u320->fb_update.fb = NULL;
unlock:
diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
index dc9e4d71b12a..ed3072563db9 100644
--- a/drivers/gpu/drm/tiny/ofdrm.c
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -820,14 +820,10 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
const struct drm_format_info *dst_format = odev->format;
struct drm_atomic_helper_damage_iter iter;
struct drm_rect damage;
- int ret, idx;
-
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- return;
+ int idx;
if (!drm_dev_enter(dev, &idx))
- goto out_drm_gem_fb_end_cpu_access;
+ return;
drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
drm_atomic_for_each_plane_damage(&iter, &damage) {
@@ -843,8 +839,6 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
}
drm_dev_exit(idx);
-out_drm_gem_fb_end_cpu_access:
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
}
static void ofdrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 162eb44dcba8..1c0d9e277dc3 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -481,14 +481,10 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
struct drm_atomic_helper_damage_iter iter;
struct drm_rect damage;
- int ret, idx;
-
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- return;
+ int idx;
if (!drm_dev_enter(dev, &idx))
- goto out_drm_gem_fb_end_cpu_access;
+ return;
drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
drm_atomic_for_each_plane_damage(&iter, &damage) {
@@ -504,8 +500,6 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane
}
drm_dev_exit(idx);
-out_drm_gem_fb_end_cpu_access:
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
}
static void simpledrm_primary_plane_helper_atomic_disable(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
index 4b79d44752c9..022b18aa3f48 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -271,17 +271,13 @@ static void udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_atomic_helper_damage_iter iter;
struct drm_rect damage;
- int ret, idx;
+ int idx;
if (!fb)
return; /* no framebuffer; plane is disabled */
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- return;
-
if (!drm_dev_enter(dev, &idx))
- goto out_drm_gem_fb_end_cpu_access;
+ return;
drm_atomic_helper_damage_iter_init(&iter, old_plane_state, plane_state);
drm_atomic_for_each_plane_damage(&iter, &damage) {
@@ -289,9 +285,6 @@ static void udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
}
drm_dev_exit(idx);
-
-out_drm_gem_fb_end_cpu_access:
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
}
static const struct drm_plane_helper_funcs udl_primary_plane_helper_funcs = {
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/6] drm/gud: Fix UBSAN warning
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
2022-11-22 20:58 ` [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
2022-11-22 20:58 ` [PATCH 3/6] drm/gud: Don't retry a failed framebuffer flush Noralf Trønnes
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
UBSAN complains about invalid value for bool:
[ 101.165172] [drm] Initialized gud 1.0.0 20200422 for 2-3.2:1.0 on minor 1
[ 101.213360] gud 2-3.2:1.0: [drm] fb1: guddrmfb frame buffer device
[ 101.213426] usbcore: registered new interface driver gud
[ 101.989431] ================================================================================
[ 101.989441] UBSAN: invalid-load in /home/pi/linux/include/linux/iosys-map.h:253:9
[ 101.989447] load of value 121 is not a valid value for type '_Bool'
[ 101.989451] CPU: 1 PID: 455 Comm: kworker/1:6 Not tainted 5.18.0-rc5-gud-5.18-rc5 #3
[ 101.989456] Hardware name: Hewlett-Packard HP EliteBook 820 G1/1991, BIOS L71 Ver. 01.44 04/12/2018
[ 101.989459] Workqueue: events_long gud_flush_work [gud]
[ 101.989471] Call Trace:
[ 101.989474] <TASK>
[ 101.989479] dump_stack_lvl+0x49/0x5f
[ 101.989488] dump_stack+0x10/0x12
[ 101.989493] ubsan_epilogue+0x9/0x3b
[ 101.989498] __ubsan_handle_load_invalid_value.cold+0x44/0x49
[ 101.989504] dma_buf_vmap.cold+0x38/0x3d
[ 101.989511] ? find_busiest_group+0x48/0x300
[ 101.989520] drm_gem_shmem_vmap+0x76/0x1b0 [drm_shmem_helper]
[ 101.989528] drm_gem_shmem_object_vmap+0x9/0xb [drm_shmem_helper]
[ 101.989535] drm_gem_vmap+0x26/0x60 [drm]
[ 101.989594] drm_gem_fb_vmap+0x47/0x150 [drm_kms_helper]
[ 101.989630] gud_prep_flush+0xc1/0x710 [gud]
[ 101.989639] ? _raw_spin_lock+0x17/0x40
[ 101.989648] gud_flush_work+0x1e0/0x430 [gud]
[ 101.989653] ? __switch_to+0x11d/0x470
[ 101.989664] process_one_work+0x21f/0x3f0
[ 101.989673] worker_thread+0x200/0x3e0
[ 101.989679] ? rescuer_thread+0x390/0x390
[ 101.989684] kthread+0xfd/0x130
[ 101.989690] ? kthread_complete_and_exit+0x20/0x20
[ 101.989696] ret_from_fork+0x22/0x30
[ 101.989706] </TASK>
[ 101.989708] ================================================================================
The source of this warning is in iosys_map_clear() called from
dma_buf_vmap(). It conditionally sets values based on map->is_iomem. The
iosys_map variables are allocated uninitialized on the stack leading to
->is_iomem having all kinds of values and not only 0/1.
Fix this by zeroing the iosys_map variables.
Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Cc: <stable@vger.kernel.org> # v5.18+
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/gud/gud_pipe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 7c6dc2bcd14a..61f4abaf1811 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -157,8 +157,8 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
{
struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach;
u8 compression = gdrm->compression;
- struct iosys_map map[DRM_FORMAT_MAX_PLANES];
- struct iosys_map map_data[DRM_FORMAT_MAX_PLANES];
+ struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
+ struct iosys_map map_data[DRM_FORMAT_MAX_PLANES] = { };
struct iosys_map dst;
void *vaddr, *buf;
size_t pitch, len;
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/6] drm/gud: Don't retry a failed framebuffer flush
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
2022-11-22 20:58 ` [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access Noralf Trønnes
2022-11-22 20:58 ` [PATCH 2/6] drm/gud: Fix UBSAN warning Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
2022-11-22 20:58 ` [PATCH 4/6] drm/gud: Split up gud_flush_work() Noralf Trønnes
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
If a framebuffer flush fails the driver will do one retry by requeing the
worker. Currently the worker is used even for synchronous flushing, but a
later patch will inline it, so this needs to change. Thinking about how to
solve this I came to the conclusion that this retry mechanism was a fix
for a problem that was only in the mind of the developer (me) and not
something that solved a real problem.
So let's remove this for now and revisit later should it become necessary.
gud_add_damage() has now only one caller so it can be inlined.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/gud/gud_pipe.c | 48 +++++++-----------------------------------
1 file changed, 8 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 61f4abaf1811..ff1358815af5 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -333,37 +333,6 @@ void gud_clear_damage(struct gud_device *gdrm)
gdrm->damage.y2 = 0;
}
-static void gud_add_damage(struct gud_device *gdrm, struct drm_rect *damage)
-{
- gdrm->damage.x1 = min(gdrm->damage.x1, damage->x1);
- gdrm->damage.y1 = min(gdrm->damage.y1, damage->y1);
- gdrm->damage.x2 = max(gdrm->damage.x2, damage->x2);
- gdrm->damage.y2 = max(gdrm->damage.y2, damage->y2);
-}
-
-static void gud_retry_failed_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
- struct drm_rect *damage)
-{
- /*
- * pipe_update waits for the worker when the display mode is going to change.
- * This ensures that the width and height is still the same making it safe to
- * add back the damage.
- */
-
- mutex_lock(&gdrm->damage_lock);
- if (!gdrm->fb) {
- drm_framebuffer_get(fb);
- gdrm->fb = fb;
- }
- gud_add_damage(gdrm, damage);
- mutex_unlock(&gdrm->damage_lock);
-
- /* Retry only once to avoid a possible storm in case of continues errors. */
- if (!gdrm->prev_flush_failed)
- queue_work(system_long_wq, &gdrm->work);
- gdrm->prev_flush_failed = true;
-}
-
void gud_flush_work(struct work_struct *work)
{
struct gud_device *gdrm = container_of(work, struct gud_device, work);
@@ -407,14 +376,10 @@ void gud_flush_work(struct work_struct *work)
ret = gud_flush_rect(gdrm, fb, format, &rect);
if (ret) {
if (ret != -ENODEV && ret != -ECONNRESET &&
- ret != -ESHUTDOWN && ret != -EPROTO) {
- bool prev_flush_failed = gdrm->prev_flush_failed;
-
- gud_retry_failed_flush(gdrm, fb, &damage);
- if (!prev_flush_failed)
- dev_err_ratelimited(fb->dev->dev,
- "Failed to flush framebuffer: error=%d\n", ret);
- }
+ ret != -ESHUTDOWN && ret != -EPROTO)
+ dev_err_ratelimited(fb->dev->dev,
+ "Failed to flush framebuffer: error=%d\n", ret);
+ gdrm->prev_flush_failed = true;
break;
}
@@ -439,7 +404,10 @@ static void gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer
gdrm->fb = fb;
}
- gud_add_damage(gdrm, damage);
+ gdrm->damage.x1 = min(gdrm->damage.x1, damage->x1);
+ gdrm->damage.y1 = min(gdrm->damage.y1, damage->y1);
+ gdrm->damage.x2 = max(gdrm->damage.x2, damage->x2);
+ gdrm->damage.y2 = max(gdrm->damage.y2, damage->y2);
mutex_unlock(&gdrm->damage_lock);
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/6] drm/gud: Split up gud_flush_work()
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
` (2 preceding siblings ...)
2022-11-22 20:58 ` [PATCH 3/6] drm/gud: Don't retry a failed framebuffer flush Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
2022-11-22 20:58 ` [PATCH 5/6] drm/gud: Prepare buffer for CPU access in gud_flush_work() Noralf Trønnes
2022-11-22 20:58 ` [PATCH 6/6] drm/gud: Use the shadow plane helper Noralf Trønnes
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
In preparation for inlining synchronous flushing split out the part of
gud_flush_work() that can be shared by the sync and async code paths.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/gud/gud_pipe.c | 72 +++++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index ff1358815af5..d2af9947494f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -333,15 +333,49 @@ void gud_clear_damage(struct gud_device *gdrm)
gdrm->damage.y2 = 0;
}
+static void gud_flush_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ struct drm_rect *damage)
+{
+ const struct drm_format_info *format;
+ unsigned int i, lines;
+ size_t pitch;
+ int ret;
+
+ format = fb->format;
+ if (format->format == DRM_FORMAT_XRGB8888 && gdrm->xrgb8888_emulation_format)
+ format = gdrm->xrgb8888_emulation_format;
+
+ /* Split update if it's too big */
+ pitch = drm_format_info_min_pitch(format, 0, drm_rect_width(damage));
+ lines = drm_rect_height(damage);
+
+ if (gdrm->bulk_len < lines * pitch)
+ lines = gdrm->bulk_len / pitch;
+
+ for (i = 0; i < DIV_ROUND_UP(drm_rect_height(damage), lines); i++) {
+ struct drm_rect rect = *damage;
+
+ rect.y1 += i * lines;
+ rect.y2 = min_t(u32, rect.y1 + lines, damage->y2);
+
+ ret = gud_flush_rect(gdrm, fb, format, &rect);
+ if (ret) {
+ if (ret != -ENODEV && ret != -ECONNRESET &&
+ ret != -ESHUTDOWN && ret != -EPROTO)
+ dev_err_ratelimited(fb->dev->dev,
+ "Failed to flush framebuffer: error=%d\n", ret);
+ gdrm->prev_flush_failed = true;
+ break;
+ }
+ }
+}
+
void gud_flush_work(struct work_struct *work)
{
struct gud_device *gdrm = container_of(work, struct gud_device, work);
- const struct drm_format_info *format;
struct drm_framebuffer *fb;
struct drm_rect damage;
- unsigned int i, lines;
- int idx, ret = 0;
- size_t pitch;
+ int idx;
if (!drm_dev_enter(&gdrm->drm, &idx))
return;
@@ -356,35 +390,7 @@ void gud_flush_work(struct work_struct *work)
if (!fb)
goto out;
- format = fb->format;
- if (format->format == DRM_FORMAT_XRGB8888 && gdrm->xrgb8888_emulation_format)
- format = gdrm->xrgb8888_emulation_format;
-
- /* Split update if it's too big */
- pitch = drm_format_info_min_pitch(format, 0, drm_rect_width(&damage));
- lines = drm_rect_height(&damage);
-
- if (gdrm->bulk_len < lines * pitch)
- lines = gdrm->bulk_len / pitch;
-
- for (i = 0; i < DIV_ROUND_UP(drm_rect_height(&damage), lines); i++) {
- struct drm_rect rect = damage;
-
- rect.y1 += i * lines;
- rect.y2 = min_t(u32, rect.y1 + lines, damage.y2);
-
- ret = gud_flush_rect(gdrm, fb, format, &rect);
- if (ret) {
- if (ret != -ENODEV && ret != -ECONNRESET &&
- ret != -ESHUTDOWN && ret != -EPROTO)
- dev_err_ratelimited(fb->dev->dev,
- "Failed to flush framebuffer: error=%d\n", ret);
- gdrm->prev_flush_failed = true;
- break;
- }
-
- gdrm->prev_flush_failed = false;
- }
+ gud_flush_damage(gdrm, fb, &damage);
drm_framebuffer_put(fb);
out:
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/6] drm/gud: Prepare buffer for CPU access in gud_flush_work()
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
` (3 preceding siblings ...)
2022-11-22 20:58 ` [PATCH 4/6] drm/gud: Split up gud_flush_work() Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
2022-11-22 20:58 ` [PATCH 6/6] drm/gud: Use the shadow plane helper Noralf Trønnes
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
In preparation for moving to the shadow plane helper prepare the
framebuffer for CPU access as early as possible.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/gud/gud_pipe.c | 67 +++++++++++++++++++++---------------------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index d2af9947494f..dfada6eedc58 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -15,6 +15,7 @@
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem.h>
+#include <drm/drm_gem_atomic_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_print.h>
#include <drm/drm_rect.h>
@@ -152,32 +153,21 @@ static size_t gud_xrgb8888_to_color(u8 *dst, const struct drm_format_info *forma
}
static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ const struct iosys_map *map, bool cached_reads,
const struct drm_format_info *format, struct drm_rect *rect,
struct gud_set_buffer_req *req)
{
- struct dma_buf_attachment *import_attach = fb->obj[0]->import_attach;
u8 compression = gdrm->compression;
- struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
- struct iosys_map map_data[DRM_FORMAT_MAX_PLANES] = { };
struct iosys_map dst;
void *vaddr, *buf;
size_t pitch, len;
- int ret = 0;
pitch = drm_format_info_min_pitch(format, 0, drm_rect_width(rect));
len = pitch * drm_rect_height(rect);
if (len > gdrm->bulk_len)
return -E2BIG;
- ret = drm_gem_fb_vmap(fb, map, map_data);
- if (ret)
- return ret;
-
- vaddr = map_data[0].vaddr;
-
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- goto vunmap;
+ vaddr = map[0].vaddr;
retry:
if (compression)
buf = gdrm->compress_buf;
@@ -192,29 +182,27 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
if (format != fb->format) {
if (format->format == GUD_DRM_FORMAT_R1) {
len = gud_xrgb8888_to_r124(buf, format, vaddr, fb, rect);
- if (!len) {
- ret = -ENOMEM;
- goto end_cpu_access;
- }
+ if (!len)
+ return -ENOMEM;
} else if (format->format == DRM_FORMAT_R8) {
- drm_fb_xrgb8888_to_gray8(&dst, NULL, map_data, fb, rect);
+ drm_fb_xrgb8888_to_gray8(&dst, NULL, map, fb, rect);
} else if (format->format == DRM_FORMAT_RGB332) {
- drm_fb_xrgb8888_to_rgb332(&dst, NULL, map_data, fb, rect);
+ drm_fb_xrgb8888_to_rgb332(&dst, NULL, map, fb, rect);
} else if (format->format == DRM_FORMAT_RGB565) {
- drm_fb_xrgb8888_to_rgb565(&dst, NULL, map_data, fb, rect,
+ drm_fb_xrgb8888_to_rgb565(&dst, NULL, map, fb, rect,
gud_is_big_endian());
} else if (format->format == DRM_FORMAT_RGB888) {
- drm_fb_xrgb8888_to_rgb888(&dst, NULL, map_data, fb, rect);
+ drm_fb_xrgb8888_to_rgb888(&dst, NULL, map, fb, rect);
} else {
len = gud_xrgb8888_to_color(buf, format, vaddr, fb, rect);
}
} else if (gud_is_big_endian() && format->cpp[0] > 1) {
- drm_fb_swab(&dst, NULL, map_data, fb, rect, !import_attach);
- } else if (compression && !import_attach && pitch == fb->pitches[0]) {
+ drm_fb_swab(&dst, NULL, map, fb, rect, cached_reads);
+ } else if (compression && cached_reads && pitch == fb->pitches[0]) {
/* can compress directly from the framebuffer */
buf = vaddr + rect->y1 * pitch;
} else {
- drm_fb_memcpy(&dst, NULL, map_data, fb, rect);
+ drm_fb_memcpy(&dst, NULL, map, fb, rect);
}
memset(req, 0, sizeof(*req));
@@ -237,12 +225,7 @@ static int gud_prep_flush(struct gud_device *gdrm, struct drm_framebuffer *fb,
req->compressed_length = cpu_to_le32(complen);
}
-end_cpu_access:
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-vunmap:
- drm_gem_fb_vunmap(fb, map);
-
- return ret;
+ return 0;
}
struct gud_usb_bulk_context {
@@ -285,6 +268,7 @@ static int gud_usb_bulk(struct gud_device *gdrm, size_t len)
}
static int gud_flush_rect(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ const struct iosys_map *map, bool cached_reads,
const struct drm_format_info *format, struct drm_rect *rect)
{
struct gud_set_buffer_req req;
@@ -293,7 +277,7 @@ static int gud_flush_rect(struct gud_device *gdrm, struct drm_framebuffer *fb,
drm_dbg(&gdrm->drm, "Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
- ret = gud_prep_flush(gdrm, fb, format, rect, &req);
+ ret = gud_prep_flush(gdrm, fb, map, cached_reads, format, rect, &req);
if (ret)
return ret;
@@ -334,6 +318,7 @@ void gud_clear_damage(struct gud_device *gdrm)
}
static void gud_flush_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ const struct iosys_map *map, bool cached_reads,
struct drm_rect *damage)
{
const struct drm_format_info *format;
@@ -358,7 +343,7 @@ static void gud_flush_damage(struct gud_device *gdrm, struct drm_framebuffer *fb
rect.y1 += i * lines;
rect.y2 = min_t(u32, rect.y1 + lines, damage->y2);
- ret = gud_flush_rect(gdrm, fb, format, &rect);
+ ret = gud_flush_rect(gdrm, fb, map, cached_reads, format, &rect);
if (ret) {
if (ret != -ENODEV && ret != -ECONNRESET &&
ret != -ESHUTDOWN && ret != -EPROTO)
@@ -373,9 +358,10 @@ static void gud_flush_damage(struct gud_device *gdrm, struct drm_framebuffer *fb
void gud_flush_work(struct work_struct *work)
{
struct gud_device *gdrm = container_of(work, struct gud_device, work);
+ struct iosys_map gem_map = { }, fb_map = { };
struct drm_framebuffer *fb;
struct drm_rect damage;
- int idx;
+ int idx, ret;
if (!drm_dev_enter(&gdrm->drm, &idx))
return;
@@ -390,8 +376,21 @@ void gud_flush_work(struct work_struct *work)
if (!fb)
goto out;
- gud_flush_damage(gdrm, fb, &damage);
+ ret = drm_gem_fb_vmap(fb, &gem_map, &fb_map);
+ if (ret)
+ goto fb_put;
+ ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+ if (ret)
+ goto vunmap;
+
+ /* Imported buffers are assumed to be WriteCombined with uncached reads */
+ gud_flush_damage(gdrm, fb, &fb_map, !fb->obj[0]->import_attach, &damage);
+
+ drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+vunmap:
+ drm_gem_fb_vunmap(fb, &gem_map);
+fb_put:
drm_framebuffer_put(fb);
out:
drm_dev_exit(idx);
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/6] drm/gud: Use the shadow plane helper
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
` (4 preceding siblings ...)
2022-11-22 20:58 ` [PATCH 5/6] drm/gud: Prepare buffer for CPU access in gud_flush_work() Noralf Trønnes
@ 2022-11-22 20:58 ` Noralf Trønnes
5 siblings, 0 replies; 7+ messages in thread
From: Noralf Trønnes @ 2022-11-22 20:58 UTC (permalink / raw)
To: Thomas Zimmermann, stable, Dave Airlie, dri-devel, Hans de Goede,
Noralf Trønnes, Maxime Ripard, Javier Martinez Canillas
Use the shadow plane helper to take care of preparing the framebuffer for
CPU access. The synchronous flushing is now done inline without the use of
a worker. The async path now uses a shadow buffer to hold framebuffer
changes and it doesn't read the framebuffer behind userspace's back
anymore.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/gud/gud_drv.c | 1 +
drivers/gpu/drm/gud/gud_internal.h | 1 +
drivers/gpu/drm/gud/gud_pipe.c | 69 ++++++++++++++++++++++++--------------
3 files changed, 46 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index d57dab104358..5aac7cda0505 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -365,6 +365,7 @@ static void gud_debugfs_init(struct drm_minor *minor)
static const struct drm_simple_display_pipe_funcs gud_pipe_funcs = {
.check = gud_pipe_check,
.update = gud_pipe_update,
+ DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS
};
static const struct drm_mode_config_funcs gud_mode_config_funcs = {
diff --git a/drivers/gpu/drm/gud/gud_internal.h b/drivers/gpu/drm/gud/gud_internal.h
index e351a1f1420d..0d148a6f27aa 100644
--- a/drivers/gpu/drm/gud/gud_internal.h
+++ b/drivers/gpu/drm/gud/gud_internal.h
@@ -43,6 +43,7 @@ struct gud_device {
struct drm_framebuffer *fb;
struct drm_rect damage;
bool prev_flush_failed;
+ void *shadow_buf;
};
static inline struct gud_device *to_gud_device(struct drm_device *drm)
diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index dfada6eedc58..7686325f7ee7 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -358,10 +358,10 @@ static void gud_flush_damage(struct gud_device *gdrm, struct drm_framebuffer *fb
void gud_flush_work(struct work_struct *work)
{
struct gud_device *gdrm = container_of(work, struct gud_device, work);
- struct iosys_map gem_map = { }, fb_map = { };
+ struct iosys_map shadow_map;
struct drm_framebuffer *fb;
struct drm_rect damage;
- int idx, ret;
+ int idx;
if (!drm_dev_enter(&gdrm->drm, &idx))
return;
@@ -369,6 +369,7 @@ void gud_flush_work(struct work_struct *work)
mutex_lock(&gdrm->damage_lock);
fb = gdrm->fb;
gdrm->fb = NULL;
+ iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
damage = gdrm->damage;
gud_clear_damage(gdrm);
mutex_unlock(&gdrm->damage_lock);
@@ -376,33 +377,33 @@ void gud_flush_work(struct work_struct *work)
if (!fb)
goto out;
- ret = drm_gem_fb_vmap(fb, &gem_map, &fb_map);
- if (ret)
- goto fb_put;
+ gud_flush_damage(gdrm, fb, &shadow_map, true, &damage);
- ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
- if (ret)
- goto vunmap;
-
- /* Imported buffers are assumed to be WriteCombined with uncached reads */
- gud_flush_damage(gdrm, fb, &fb_map, !fb->obj[0]->import_attach, &damage);
-
- drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
-vunmap:
- drm_gem_fb_vunmap(fb, &gem_map);
-fb_put:
drm_framebuffer_put(fb);
out:
drm_dev_exit(idx);
}
-static void gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,
- struct drm_rect *damage)
+static int gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ const struct iosys_map *map, struct drm_rect *damage)
{
struct drm_framebuffer *old_fb = NULL;
+ struct iosys_map shadow_map;
mutex_lock(&gdrm->damage_lock);
+ if (!gdrm->shadow_buf) {
+ gdrm->shadow_buf = vzalloc(fb->pitches[0] * fb->height);
+ if (!gdrm->shadow_buf) {
+ mutex_unlock(&gdrm->damage_lock);
+ return -ENOMEM;
+ }
+ }
+
+ iosys_map_set_vaddr(&shadow_map, gdrm->shadow_buf);
+ iosys_map_incr(&shadow_map, drm_fb_clip_offset(fb->pitches[0], fb->format, damage));
+ drm_fb_memcpy(&shadow_map, fb->pitches, map, fb, damage);
+
if (fb != gdrm->fb) {
old_fb = gdrm->fb;
drm_framebuffer_get(fb);
@@ -420,6 +421,26 @@ static void gud_fb_queue_damage(struct gud_device *gdrm, struct drm_framebuffer
if (old_fb)
drm_framebuffer_put(old_fb);
+
+ return 0;
+}
+
+static void gud_fb_handle_damage(struct gud_device *gdrm, struct drm_framebuffer *fb,
+ const struct iosys_map *map, struct drm_rect *damage)
+{
+ int ret;
+
+ if (gdrm->flags & GUD_DISPLAY_FLAG_FULL_UPDATE)
+ drm_rect_init(damage, 0, 0, fb->width, fb->height);
+
+ if (gud_async_flush) {
+ ret = gud_fb_queue_damage(gdrm, fb, map, damage);
+ if (ret != -ENOMEM)
+ return;
+ }
+
+ /* Imported buffers are assumed to be WriteCombined with uncached reads */
+ gud_flush_damage(gdrm, fb, map, !fb->obj[0]->import_attach, damage);
}
int gud_pipe_check(struct drm_simple_display_pipe *pipe,
@@ -544,6 +565,7 @@ void gud_pipe_update(struct drm_simple_display_pipe *pipe,
struct drm_device *drm = pipe->crtc.dev;
struct gud_device *gdrm = to_gud_device(drm);
struct drm_plane_state *state = pipe->plane.state;
+ struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
struct drm_framebuffer *fb = state->fb;
struct drm_crtc *crtc = &pipe->crtc;
struct drm_rect damage;
@@ -557,6 +579,8 @@ void gud_pipe_update(struct drm_simple_display_pipe *pipe,
gdrm->fb = NULL;
}
gud_clear_damage(gdrm);
+ vfree(gdrm->shadow_buf);
+ gdrm->shadow_buf = NULL;
mutex_unlock(&gdrm->damage_lock);
}
@@ -572,13 +596,8 @@ void gud_pipe_update(struct drm_simple_display_pipe *pipe,
if (crtc->state->active_changed)
gud_usb_set_u8(gdrm, GUD_REQ_SET_DISPLAY_ENABLE, crtc->state->active);
- if (drm_atomic_helper_damage_merged(old_state, state, &damage)) {
- if (gdrm->flags & GUD_DISPLAY_FLAG_FULL_UPDATE)
- drm_rect_init(&damage, 0, 0, fb->width, fb->height);
- gud_fb_queue_damage(gdrm, fb, &damage);
- if (!gud_async_flush)
- flush_work(&gdrm->work);
- }
+ if (drm_atomic_helper_damage_merged(old_state, state, &damage))
+ gud_fb_handle_damage(gdrm, fb, &shadow_plane_state->data[0], &damage);
if (!crtc->state->enable)
gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 0);
--
b4 0.10.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-22 20:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22 20:58 [PATCH 0/6] drm/gud: Use the shadow plane helper Noralf Trønnes
2022-11-22 20:58 ` [PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access Noralf Trønnes
2022-11-22 20:58 ` [PATCH 2/6] drm/gud: Fix UBSAN warning Noralf Trønnes
2022-11-22 20:58 ` [PATCH 3/6] drm/gud: Don't retry a failed framebuffer flush Noralf Trønnes
2022-11-22 20:58 ` [PATCH 4/6] drm/gud: Split up gud_flush_work() Noralf Trønnes
2022-11-22 20:58 ` [PATCH 5/6] drm/gud: Prepare buffer for CPU access in gud_flush_work() Noralf Trønnes
2022-11-22 20:58 ` [PATCH 6/6] drm/gud: Use the shadow plane helper Noralf Trønnes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).