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

[Thomas Zimmermann]

Hi

Am 05.10.22 um 00:28 schrieb 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.

OK, I'll do that. Very briefly; it's for minimizing the overheads of the 
possible locking and synchronization, and to maybe it keeps possible 
writers out while we copy from the buffer.

Best regards
Thomas

> 

-- 
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/20221005/a04b136e/attachment.sig>