[PATCH 1/6] drm/gem: shadow_fb_access: Prepare imported buffers for CPU access

Thomas Zimmermann tzimmermann at suse.de
Wed Nov 23 08:39:17 UTC 2022


(cc: danvet)

Hi

Am 22.11.22 um 21:58 schrieb Noralf Trønnes via B4 Submission Endpoint:
> From: Noralf Trønnes <noralf at tronnes.org>
> 
> 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

I had a similar patch recently, but Daniel shot it down. AFAIR 
begin_cpu_access *somehow* interferes with *something* and that can 
leads to *problems.*  Sorry that's the best I remember. Daniel should 
know. :D

Best regards
Thomas

> 
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Javier Martinez Canillas <javierm at redhat.com>
> Cc: Hans de Goede <hdegoede at redhat.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Signed-off-by: Noralf Trønnes <noralf at 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 = {
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20221123/0c8bdda2/attachment-0001.sig>


More information about the dri-devel mailing list