All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Yang <michael.yang@imgtec.com>
To: sumit.semwal@linaro.org
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	gustavo@padovan.org, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, michael.yang@imgtec.com,
	gregkh@linuxfoundation.org
Subject: [PATCH] sync_file: Return reasonable timestamp when merging signaled fences
Date: Thu, 9 May 2019 12:34:11 +0800	[thread overview]
Message-ID: <1557376451-20164-1-git-send-email-michael.yang@imgtec.com> (raw)
In-Reply-To: <1554710495-6646-1-git-send-email-michael.yang@imgtec.com>

If all the sync points were signaled in both fences a and b,
there was only one sync point in merged fence which is a_fence[0].
The Fence structure in android framework might be confused about
timestamp if there were any sync points which were signaled after
a_fence[0]. It might be more reasonable to use timestamp of last signaled
sync point to represent the merged fence.
The issue can be found from EGL extension ANDROID_get_frame_timestamps.
Sometimes the return value of EGL_READS_DONE_TIME_ANDROID is head of
the return value of EGL_RENDERING_COMPLETE_TIME_ANDROID.
That means display/composition had been completed before rendering
was completed that is incorrect.

Some discussion can be found at:
https://android-review.googlesource.com/c/kernel/common/+/907009

Signed-off-by: Michael Yang <michael.yang@imgtec.com>
---
Hi,
I didn't get response since I previously sent this a month ago.
Could someone have a chance to look at it please?
Thanks.
 drivers/dma-buf/sync_file.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 4f6305c..d46bfe1 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -274,8 +274,29 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
 	for (; i_b < b_num_fences; i_b++)
 		add_fence(fences, &i, b_fences[i_b]);
 
-	if (i == 0)
-		fences[i++] = dma_fence_get(a_fences[0]);
+	/* If all the sync pts were signaled, then adding the sync_pt who
+	 * was the last signaled to the fence.
+	 */
+	if (i == 0) {
+		struct dma_fence *last_signaled_sync_pt = a_fences[0];
+		int iter;
+
+		for (iter = 1; iter < a_num_fences; iter++) {
+			if (ktime_compare(last_signaled_sync_pt->timestamp,
+				a_fences[iter]->timestamp) < 0) {
+				last_signaled_sync_pt = a_fences[iter];
+			}
+		}
+
+		for (iter = 0; iter < b_num_fences; iter++) {
+			if (ktime_compare(last_signaled_sync_pt->timestamp,
+				b_fences[iter]->timestamp) < 0) {
+				last_signaled_sync_pt = b_fences[iter];
+			}
+		}
+
+		fences[i++] = dma_fence_get(last_signaled_sync_pt);
+	}
 
 	if (num_fences > i) {
 		nfences = krealloc(fences, i * sizeof(*fences),
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Michael Yang <michael.yang@imgtec.com>
To: <sumit.semwal@linaro.org>
Cc: <dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<gustavo@padovan.org>, <linux-media@vger.kernel.org>,
	<linaro-mm-sig@lists.linaro.org>, <michael.yang@imgtec.com>,
	<gregkh@linuxfoundation.org>
Subject: [PATCH] sync_file: Return reasonable timestamp when merging signaled fences
Date: Thu, 9 May 2019 12:34:11 +0800	[thread overview]
Message-ID: <1557376451-20164-1-git-send-email-michael.yang@imgtec.com> (raw)
In-Reply-To: <1554710495-6646-1-git-send-email-michael.yang@imgtec.com>

If all the sync points were signaled in both fences a and b,
there was only one sync point in merged fence which is a_fence[0].
The Fence structure in android framework might be confused about
timestamp if there were any sync points which were signaled after
a_fence[0]. It might be more reasonable to use timestamp of last signaled
sync point to represent the merged fence.
The issue can be found from EGL extension ANDROID_get_frame_timestamps.
Sometimes the return value of EGL_READS_DONE_TIME_ANDROID is head of
the return value of EGL_RENDERING_COMPLETE_TIME_ANDROID.
That means display/composition had been completed before rendering
was completed that is incorrect.

Some discussion can be found at:
https://android-review.googlesource.com/c/kernel/common/+/907009

Signed-off-by: Michael Yang <michael.yang@imgtec.com>
---
Hi,
I didn't get response since I previously sent this a month ago.
Could someone have a chance to look at it please?
Thanks.
 drivers/dma-buf/sync_file.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 4f6305c..d46bfe1 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -274,8 +274,29 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
 	for (; i_b < b_num_fences; i_b++)
 		add_fence(fences, &i, b_fences[i_b]);
 
-	if (i == 0)
-		fences[i++] = dma_fence_get(a_fences[0]);
+	/* If all the sync pts were signaled, then adding the sync_pt who
+	 * was the last signaled to the fence.
+	 */
+	if (i == 0) {
+		struct dma_fence *last_signaled_sync_pt = a_fences[0];
+		int iter;
+
+		for (iter = 1; iter < a_num_fences; iter++) {
+			if (ktime_compare(last_signaled_sync_pt->timestamp,
+				a_fences[iter]->timestamp) < 0) {
+				last_signaled_sync_pt = a_fences[iter];
+			}
+		}
+
+		for (iter = 0; iter < b_num_fences; iter++) {
+			if (ktime_compare(last_signaled_sync_pt->timestamp,
+				b_fences[iter]->timestamp) < 0) {
+				last_signaled_sync_pt = b_fences[iter];
+			}
+		}
+
+		fences[i++] = dma_fence_get(last_signaled_sync_pt);
+	}
 
 	if (num_fences > i) {
 		nfences = krealloc(fences, i * sizeof(*fences),
-- 
2.7.4


  reply	other threads:[~2019-05-09  4:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08  8:01 [PATCH] sync_file: Return reasonable timestamp when merging signaled fences Michael Yang
2019-04-08  8:01 ` Michael Yang
2019-05-09  4:34 ` Michael Yang [this message]
2019-05-09  4:34   ` Michael Yang
2019-05-09 11:46   ` Chris Wilson
2019-05-09 11:46     ` Chris Wilson
2019-05-14  7:55     ` [EXTERNAL] " Michael Yang
2019-05-14  7:55       ` Michael Yang
2019-05-14  8:06       ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1557376451-20164-1-git-send-email-michael.yang@imgtec.com \
    --to=michael.yang@imgtec.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavo@padovan.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sumit.semwal@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.