diff mbox

[hwc,v2,3/6] drm_hwcomposer: Submit in-fence to DRM

Message ID 20170927115841.29134-4-robert.foss@collabora.com (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Foss Sept. 27, 2017, 11:58 a.m. UTC 
Add support for in-fences through the IN_FENCE_FD property. In-fences signal
when their associated buffer may be read by DRM/KMS.

Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

Comments

Sean Paul Sept. 27, 2017, 6:55 p.m. UTC | #1 
On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
> Add support for in-fences through the IN_FENCE_FD property. In-fences signal
> when their associated buffer may be read by DRM/KMS.
>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
>  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index bd670cf..71c0451 100644
> --- a/drmdisplaycompositor.cpp
> +++ b/drmdisplaycompositor.cpp
> @@ -529,6 +529,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>      std::vector<size_t> &source_layers = comp_plane.source_layers();
>
>      int fb_id = -1;
> +    int fence_fd = -1;
>      DrmHwcRect<int> display_frame;
>      DrmHwcRect<float> source_crop;
>      uint64_t rotation = 0;
> @@ -547,30 +548,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>          break;
>        }
>        DrmHwcLayer &layer = layers[source_layers.front()];
> -      if (!test_only && layer.acquire_fence.get() >= 0) {
> -        int acquire_fence = layer.acquire_fence.get();
> -        int total_fence_timeout = 0;
> -        for (int i = 0; i < kAcquireWaitTries; ++i) {
> -          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
> -          total_fence_timeout += fence_timeout;
> -          ret = sync_wait(acquire_fence, fence_timeout);
> -          if (ret)
> -            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
> -                  acquire_fence, i, ret, total_fence_timeout);
> -          else
> -            break;
> -        }
> -        if (ret) {
> -          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
> -          break;
> -        }
> -        layer.acquire_fence.Close();
> -      }
>        if (!layer.buffer) {
>          ALOGE("Expected a valid framebuffer for pset");
>          break;
>        }
>        fb_id = layer.buffer->fb_id;
> +      fence_fd = layer.acquire_fence.get();
>        display_frame = layer.display_frame;
>        source_crop = layer.source_crop;
>        if (layer.blending == DrmHwcBlending::kPreMult)
> @@ -587,7 +570,21 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>          rotation |= 1 << DRM_ROTATE_180;
>        else if (layer.transform & DrmHwcTransform::kRotate270)
>          rotation |= 1 << DRM_ROTATE_270;
> +
> +      if (fence_fd != -1) {

nit: if (fence_fd < 0)

With that fixed:

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> +        int prop_id = plane->in_fence_fd_property().id();
> +        if (prop_id == 0) {
> +                ALOGE("Failed to get IN_FENCE_FD property id");
> +                break;
> +        }
> +        ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd);
> +        if (ret < 0) {
> +          ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret);
> +          break;
> +        }
> +      }
>      }
> +
>      // Disable the plane if there's no framebuffer
>      if (fb_id < 0) {
>        ret = drmModeAtomicAddProperty(pset, plane->id(),
> --
> 2.11.0
>
Robert Foss Sept. 27, 2017, 6:59 p.m. UTC | #2 
Hey Sean,

On Wed, 2017-09-27 at 14:55 -0400, Sean Paul wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.c
> om> wrote:
> > Add support for in-fences through the IN_FENCE_FD property. In-
> > fences signal
> > when their associated buffer may be read by DRM/KMS.
> > 
> > Signed-off-by: Robert Foss <robert.foss@collabora.com>
> > ---
> >  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
> >  1 file changed, 16 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index bd670cf..71c0451 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -529,6 +529,7 @@ int
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition
> > *display_comp,
> >      std::vector<size_t> &source_layers =
> > comp_plane.source_layers();
> > 
> >      int fb_id = -1;
> > +    int fence_fd = -1;
> >      DrmHwcRect<int> display_frame;
> >      DrmHwcRect<float> source_crop;
> >      uint64_t rotation = 0;
> > @@ -547,30 +548,12 @@ int
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition
> > *display_comp,
> >          break;
> >        }
> >        DrmHwcLayer &layer = layers[source_layers.front()];
> > -      if (!test_only && layer.acquire_fence.get() >= 0) {
> > -        int acquire_fence = layer.acquire_fence.get();
> > -        int total_fence_timeout = 0;
> > -        for (int i = 0; i < kAcquireWaitTries; ++i) {
> > -          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
> > -          total_fence_timeout += fence_timeout;
> > -          ret = sync_wait(acquire_fence, fence_timeout);
> > -          if (ret)
> > -            ALOGW("Acquire fence %d wait %d failed (%d). Total
> > time %d",
> > -                  acquire_fence, i, ret, total_fence_timeout);
> > -          else
> > -            break;
> > -        }
> > -        if (ret) {
> > -          ALOGE("Failed to wait for acquire %d/%d", acquire_fence,
> > ret);
> > -          break;
> > -        }
> > -        layer.acquire_fence.Close();
> > -      }
> >        if (!layer.buffer) {
> >          ALOGE("Expected a valid framebuffer for pset");
> >          break;
> >        }
> >        fb_id = layer.buffer->fb_id;
> > +      fence_fd = layer.acquire_fence.get();
> >        display_frame = layer.display_frame;
> >        source_crop = layer.source_crop;
> >        if (layer.blending == DrmHwcBlending::kPreMult)
> > @@ -587,7 +570,21 @@ int
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition
> > *display_comp,
> >          rotation |= 1 << DRM_ROTATE_180;
> >        else if (layer.transform & DrmHwcTransform::kRotate270)
> >          rotation |= 1 << DRM_ROTATE_270;
> > +
> > +      if (fence_fd != -1) {
> 
> nit: if (fence_fd < 0)
> 
> With that fixed:
> 
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Ack.
Submitting a fix in v3 tomorrow.

> 
> > +        int prop_id = plane->in_fence_fd_property().id();
> > +        if (prop_id == 0) {
> > +                ALOGE("Failed to get IN_FENCE_FD property id");
> > +                break;
> > +        }
> > +        ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id,
> > fence_fd);
> > +        if (ret < 0) {
> > +          ALOGE("Failed to add IN_FENCE_FD property to pset: %d",
> > ret);
> > +          break;
> > +        }
> > +      }
> >      }
> > +
> >      // Disable the plane if there's no framebuffer
> >      if (fb_id < 0) {
> >        ret = drmModeAtomicAddProperty(pset, plane->id(),
> > --
> > 2.11.0
> >
Rob Herring (Arm) Feb. 12, 2018, 10:10 p.m. UTC | #3 
On Wed, Sep 27, 2017 at 1:55 PM, Sean Paul <seanpaul@chromium.org> wrote:
> On Wed, Sep 27, 2017 at 7:58 AM, Robert Foss <robert.foss@collabora.com> wrote:
>> Add support for in-fences through the IN_FENCE_FD property. In-fences signal
>> when their associated buffer may be read by DRM/KMS.
>>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>>  drmdisplaycompositor.cpp | 35 ++++++++++++++++-------------------
>>  1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
>> index bd670cf..71c0451 100644
>> --- a/drmdisplaycompositor.cpp
>> +++ b/drmdisplaycompositor.cpp
>> @@ -529,6 +529,7 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>>      std::vector<size_t> &source_layers = comp_plane.source_layers();
>>
>>      int fb_id = -1;
>> +    int fence_fd = -1;
>>      DrmHwcRect<int> display_frame;
>>      DrmHwcRect<float> source_crop;
>>      uint64_t rotation = 0;
>> @@ -547,30 +548,12 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>>          break;
>>        }
>>        DrmHwcLayer &layer = layers[source_layers.front()];
>> -      if (!test_only && layer.acquire_fence.get() >= 0) {
>> -        int acquire_fence = layer.acquire_fence.get();
>> -        int total_fence_timeout = 0;
>> -        for (int i = 0; i < kAcquireWaitTries; ++i) {
>> -          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
>> -          total_fence_timeout += fence_timeout;
>> -          ret = sync_wait(acquire_fence, fence_timeout);
>> -          if (ret)
>> -            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
>> -                  acquire_fence, i, ret, total_fence_timeout);
>> -          else
>> -            break;
>> -        }
>> -        if (ret) {
>> -          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
>> -          break;
>> -        }
>> -        layer.acquire_fence.Close();
>> -      }
>>        if (!layer.buffer) {
>>          ALOGE("Expected a valid framebuffer for pset");
>>          break;
>>        }
>>        fb_id = layer.buffer->fb_id;
>> +      fence_fd = layer.acquire_fence.get();
>>        display_frame = layer.display_frame;
>>        source_crop = layer.source_crop;
>>        if (layer.blending == DrmHwcBlending::kPreMult)
>> @@ -587,7 +570,21 @@ int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
>>          rotation |= 1 << DRM_ROTATE_180;
>>        else if (layer.transform & DrmHwcTransform::kRotate270)
>>          rotation |= 1 << DRM_ROTATE_270;
>> +
>> +      if (fence_fd != -1) {
>
> nit: if (fence_fd < 0)

Your nit is a bug. :) We're checking for if a valid fd. I think this
may be John's issue with needing a glFinish().

A fix is coming.

Rob
diff mbox

Patch

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index bd670cf..71c0451 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -529,6 +529,7 @@  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
     std::vector<size_t> &source_layers = comp_plane.source_layers();
 
     int fb_id = -1;
+    int fence_fd = -1;
     DrmHwcRect<int> display_frame;
     DrmHwcRect<float> source_crop;
     uint64_t rotation = 0;
@@ -547,30 +548,12 @@  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         break;
       }
       DrmHwcLayer &layer = layers[source_layers.front()];
-      if (!test_only && layer.acquire_fence.get() >= 0) {
-        int acquire_fence = layer.acquire_fence.get();
-        int total_fence_timeout = 0;
-        for (int i = 0; i < kAcquireWaitTries; ++i) {
-          int fence_timeout = kAcquireWaitTimeoutMs * (1 << i);
-          total_fence_timeout += fence_timeout;
-          ret = sync_wait(acquire_fence, fence_timeout);
-          if (ret)
-            ALOGW("Acquire fence %d wait %d failed (%d). Total time %d",
-                  acquire_fence, i, ret, total_fence_timeout);
-          else
-            break;
-        }
-        if (ret) {
-          ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
-          break;
-        }
-        layer.acquire_fence.Close();
-      }
       if (!layer.buffer) {
         ALOGE("Expected a valid framebuffer for pset");
         break;
       }
       fb_id = layer.buffer->fb_id;
+      fence_fd = layer.acquire_fence.get();
       display_frame = layer.display_frame;
       source_crop = layer.source_crop;
       if (layer.blending == DrmHwcBlending::kPreMult)
@@ -587,7 +570,21 @@  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
         rotation |= 1 << DRM_ROTATE_180;
       else if (layer.transform & DrmHwcTransform::kRotate270)
         rotation |= 1 << DRM_ROTATE_270;
+
+      if (fence_fd != -1) {
+        int prop_id = plane->in_fence_fd_property().id();
+        if (prop_id == 0) {
+                ALOGE("Failed to get IN_FENCE_FD property id");
+                break;
+        }
+        ret = drmModeAtomicAddProperty(pset, plane->id(), prop_id, fence_fd);
+        if (ret < 0) {
+          ALOGE("Failed to add IN_FENCE_FD property to pset: %d", ret);
+          break;
+        }
+      }
     }
+
     // Disable the plane if there's no framebuffer
     if (fb_id < 0) {
       ret = drmModeAtomicAddProperty(pset, plane->id(),