Re: [PATCH 21/26] drm: use new iterator in drm_gem_plane_helper_prepare_fb v2

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 17.09.21 um 16:55 schrieb Daniel Vetter:
> On Fri, Sep 17, 2021 at 02:35:08PM +0200, Christian König wrote:
>> Makes the handling a bit more complex, but avoids the use of
>> dma_resv_get_excl_unlocked().
>>
>> v2: add missing rcu_read_lock()/unlock()
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/drm_gem_atomic_helper.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
>> index e570398abd78..d8f9c6432544 100644
>> --- a/drivers/gpu/drm/drm_gem_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
>> @@ -143,6 +143,7 @@
>>    */
>>   int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state)
>>   {
>> +	struct dma_resv_iter cursor;
>>   	struct drm_gem_object *obj;
>>   	struct dma_fence *fence;
>>   
>> @@ -150,9 +151,18 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_st
>>   		return 0;
>>   
>>   	obj = drm_gem_fb_get_obj(state->fb, 0);
>> -	fence = dma_resv_get_excl_unlocked(obj->resv);
>> -	drm_atomic_set_fence_for_plane(state, fence);
>> +	rcu_read_lock();
>> +	dma_resv_iter_begin(&cursor, obj->resv, false);
>> +	dma_resv_for_each_fence_unlocked(&cursor, fence) {
>> +		rcu_read_unlock();
>> +		/* TODO: We only use the first write fence here */
>> +		drm_atomic_set_fence_for_plane(state, fence);
> Yeah I wonder whether we should/need to collate them all together. But I
> guesss whomever hits that first with their funny multi-plane yuv or
> whatever gets to do that. Or I'm not clear on what exactly your TODO here
> means?

Yeah, exactly that. Basically we have use cases where where we have more 
than one fence to wait for.

The TODO is here because adding that to the atomic helper is just not my 
construction site at the moment.

Regards,
Christian.

>
>> +		return 0;
>> +	}
>> +	dma_resv_iter_end(&cursor);
>> +	rcu_read_unlock();
> Imo we should do full dma_resv_lock here. atomic helpers are designed to
> allow this, and it simplifies things. Also it really doesn't matter for
> atomic, we should be able to do 60fps*a few planes easily :-)
> -Daniel
>
>>   
>> +	drm_atomic_set_fence_for_plane(state, NULL);
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
>> -- 
>> 2.25.1
>>