Re: [PATCH v2 2/5] drm/format-helper: Pass xfrm buffer to format-conversion helpers

[Javier Martinez Canillas <javierm@redhat.com>]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Pass an instance of struct drm_xfrm_buf to DRM's format conversion
> helpers. Update all callers. Drivers will later be able to keep this
> cache across display updates.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: David Lechner <david@lechnology.com>
> ---

[...]

> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 5a80b228d18ca..d11079733bc0e 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -571,6 +571,7 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
>  	struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
>  	unsigned int page_height = ssd130x->device_info->page_height;
>  	struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state);
> +	struct drm_xfrm_buf xfrm = DRM_XFRM_BUF_INIT;

I would prefer if this structure is a field of struct ssd130x_plane_state.

Since ssd130x_primary_plane_helper_atomic_check() zero allocates that, it
will have the same initial values as set by DRM_XFRM_BUF_INIT.

Alternatively you can do a drmm_xfrm_buf_init() + drm_xfrm_buf_reserve()
in ssd130x_primary_plane_helper_atomic_check().

>  	u8 *buf = ssd130x_state->buffer;

and struct drm_xfrm_buf *xfrm = &ssd130x_state->xfrm;

>  	struct iosys_map dst;
>  	unsigned int dst_pitch;
> @@ -587,12 +588,14 @@ static int ssd130x_fb_blit_rect(struct drm_plane_state *state,
>  		return ret;
>  
>  	iosys_map_set_vaddr(&dst, buf);
> -	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect);
> +	drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, &xfrm);
>  
>  	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>  
>  	ssd130x_update_rect(ssd130x, ssd130x_state, rect);
>  
> +	drm_xfrm_buf_release(&xfrm);
> +

and you can release it in ssd130x_primary_plane_destroy_state().

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat