Re: [PATCH 24/28] drm: use new iterator in drm_gem_plane_helper_prepare_fb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);






[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux