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@xxxxxxx> > --- Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> 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
