[PATCH 3/5] drm/gem: Handle drm_gem_{begin, end}_cpu_access() in shadow-plane helpers

Thomas Zimmermann tzimmermann at suse.de
Mon Oct 17 11:15:08 UTC 2022


Handle drm_gem_{begin,end}_cpu_access() in shadow-plane helpers during
a commit. The functions synchronizes imported dma-buf objects with the
exporter's state. If this fail (as signalled by an error code), atomic-
commit helpers can now still abort the commit.

Several drivers already reserved CPU access in their atomic update
handlers. Remove the respective calls, as they are no longer required.
All other drivers with shadow planes that never bothered with these
functions now correctly reserve CPU access to the framebuffer memory.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/drm_gem_atomic_helper.c | 26 ++++++++++++++++++++-----
 drivers/gpu/drm/solomon/ssd130x.c       | 10 +---------
 drivers/gpu/drm/tiny/gm12u320.c         | 10 +---------
 drivers/gpu/drm/tiny/ofdrm.c            |  8 +-------
 drivers/gpu/drm/tiny/simpledrm.c        | 10 ++--------
 drivers/gpu/drm/udl/udl_modeset.c       | 11 ++---------
 6 files changed, 28 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e42800718f515..eac47f1668f43 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -364,10 +364,12 @@ EXPORT_SYMBOL(drm_gem_reset_shadow_plane);
  * @plane: the plane
  * @plane_state: the plane state of type struct drm_shadow_plane_state
  *
- * This function implements struct &drm_plane_helper_funcs.begin_fb_access. It
- * 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.
+ * This function implements struct &drm_plane_helper_funcs.begin_fb_access.
+ * It reserves CPU read access to the framebuffer memory by calling
+ * drm_gem_fb_begin_cpu_access(). It further 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.
  *
  * See drm_gem_end_shadow_fb_access() for cleanup.
  *
@@ -378,11 +380,24 @@ 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_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret)
+		return ret;
+
+	ret = drm_gem_fb_vmap(fb, shadow_plane_state->map, shadow_plane_state->data);
+	if (ret)
+		goto drm_gem_fb_end_cpu_access;
+
+	return 0;
+
+drm_gem_fb_end_cpu_access:
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+	return ret;
 }
 EXPORT_SYMBOL(drm_gem_begin_shadow_fb_access);
 
@@ -404,6 +419,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 f2795f90ea693..51e06eb76c613 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 7441d992a5d7a..b721ed6452fbf 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 0e1cc2369afcc..6584485bce951 100644
--- a/drivers/gpu/drm/tiny/ofdrm.c
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -817,12 +817,8 @@ static void ofdrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
 	struct drm_rect damage;
 	int ret, idx;
 
-	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) {
@@ -838,8 +834,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 cbb100753154e..a05da02b96a25 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 4b79d44752c92..022b18aa3f486 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 = {
-- 
2.38.0



More information about the dri-devel mailing list