Am 05.10.21 um 12:47 schrieb Tvrtko Ursulin:
On 05/10/2021 11:27, Christian König wrote:
Am 05.10.21 um 09:53 schrieb Tvrtko Ursulin:
On 01/10/2021 11:06, Christian König wrote:
Makes the handling a bit more complex, but avoids the use of
dma_resv_get_excl_unlocked().
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/drm_gem_atomic_helper.c | 13 +++++++++++--
1 file changed, 11 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..21ed930042b8 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,17 @@ 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);
+ dma_resv_iter_begin(&cursor, obj->resv, false);
+ dma_resv_for_each_fence_unlocked(&cursor, fence) {
+ dma_fence_get(fence);
+ dma_resv_iter_end(&cursor);
+ /* TODO: We only use the first write fence here */
What is the TODO? NB instead?
The drm atomic API can unfortunately handle only one fence and we can
certainly have more than that.
+ drm_atomic_set_fence_for_plane(state, fence);
+ return 0;
+ }
+ dma_resv_iter_end(&cursor);
+ drm_atomic_set_fence_for_plane(state, NULL);
dma_resv_iter_begin(&cursor, obj->resv, false);
dma_resv_for_each_fence_unlocked(&cursor, fence) {
dma_fence_get(fence);
break;
}
dma_resv_iter_end(&cursor);
drm_atomic_set_fence_for_plane(state, fence);
Does this work?
Yeah that should work as well.
But overall I am not sure this is nicer. Is the goal to remove
dma_resv_get_excl_unlocked as API it just does not happen in this
series?
Yes, the only user left is the i915_gem_object_last_write_engine()
function and that one you already removed in i915.
To me the above feels clumsier than dma_resv_get_excl_unlocked and you
can even view it as open coding that helper. So don't know, someone
else can have a casting vote.
I guess if support for more than one fence is coming soon(-ish) do drm
atomic api then I could be convinced the iterator here makes sense today.
Well Daniel noted that the drm atomic API needs some more work here
because we don't handle different fences for different planes correctly
either.
We could as well postpone this change and fix the atomic functions first.
Regards,
Christian.
Regards,
Tvrtko
Regards,
Christian.
Regards,
Tvrtko
return 0;
}
EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);