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

[Javier Martinez Canillas]

Hello Noralf,

On 11/22/22 21:58, Noralf Trønnes via B4 Submission Endpoint wrote:
> 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
> 

[...]

> @@ -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;

Makes sense to me to have the CPU access prepare here too.

> 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);
> -

The only thing I'm not sure about is that drivers used to call the begin/end
CPU access only during the window while the BO data was accessed but now the
windows will be slightly bigger if that happens in .{begin,end}_fb_access.

If that's not an issue then, I agree with your patch since it simplifies the
logic in the drivers and gets rid of duplicated code.

Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat