[PATCH 10/16] drm/udl: Use damage iterator

[Javier Martinez Canillas]

On 9/19/22 15:04, Thomas Zimmermann wrote:
> Use a damage iterator to process damage areas individually. Merging
> damage areas can resul tin large updates of unchanged framebuffer

result in

> regions. As USB is rather slow, it's better to process damage areas
> individually and hence minimize USB-transfered data.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---

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

but I've a comment below.

>  
>  /*
> @@ -301,16 +291,26 @@ static void udl_primary_plane_helper_atomic_update(struct drm_plane *plane,
>  	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
>  	struct drm_framebuffer *fb = plane_state->fb;
>  	struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> -	struct drm_rect rect;
> -	int idx;
> +	struct drm_atomic_helper_damage_iter iter;
> +	struct drm_rect damage;
> +	int ret, idx;
>  
> -	if (!drm_dev_enter(dev, &idx))
> +	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
> +	if (ret)
>  		return;
>

This is an unrelated change. The sync was made in udl_handle_damage() before
and you are moving to udl_primary_plane_helper_atomic_update() in this patch.

I don't have a strong opinion and I see that there are drivers that do once
before iterating over the damage areas and others do the sync for each damage
area update. But it would be good to mention that this change is done too and
provided some justification.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat