On Fri, Dec 16, 2016 at 03:21:45AM -0500, Robert Foss wrote:


On 2016-12-14 11:13 AM, Brian Starkey wrote:
Hi,

On Wed, Dec 14, 2016 at 04:05:04AM -0500, Robert Foss wrote:
From: Gustavo Padovan <gustavo.pado...@collabora.co.uk>

Add support for the OUT_FENCE_PTR property to enable setting out
fences for
atomic commits.

Signed-off-by: Gustavo Padovan <gustavo.pado...@collabora.co.uk>
Signed-off-by: Robert Foss <robert.f...@collabora.com>
---
lib/igt_kms.c | 21 ++++++++++++++++++++-
lib/igt_kms.h |  3 +++
2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8ca49d86..fe1b356d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -175,7 +175,8 @@ const char
*igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
   "DEGAMMA_LUT",
   "GAMMA_LUT",
   "MODE_ID",
-    "ACTIVE"
+    "ACTIVE",
+    "OUT_FENCE_PTR"
};

const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
@@ -2103,6 +2104,10 @@ static void
igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
       igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE,
!!output);
   }

+    if (pipe_obj->out_fence_ptr)
+        igt_atomic_populate_crtc_req(req, pipe_obj,
IGT_CRTC_OUT_FENCE_PTR,
+            (uint64_t)(uintptr_t) pipe_obj->out_fence_ptr);
+
   /*
    *    TODO: Add all crtc level properties here
    */
@@ -2683,6 +2688,20 @@ igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void
*ptr, size_t length)
}

/**
+ * igt_pipe_set_out_fence_ptr:
+ * @pipe: pipe pointer to which background color to be set
+ * @fence_ptr: out fence pointer
+ *
+ * Sets the out fence pointer that will be passed to the kernel in
+ * the atomic ioctl. When the kernel returns the out fence pointer
+ * will contain the fd number of the out fence created by KMS.
+ */
+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr)
+{
+    pipe->out_fence_ptr = fence_ptr;
+}
+
+/**
* igt_crtc_set_background:
* @pipe: pipe pointer to which background color to be set
* @background: background color value in BGR 16bpc
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 9766807c..8eb611c0 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -110,6 +110,7 @@ enum igt_atomic_crtc_properties {
      IGT_CRTC_GAMMA_LUT,
      IGT_CRTC_MODE_ID,
      IGT_CRTC_ACTIVE,
+       IGT_CRTC_OUT_FENCE_PTR,
      IGT_NUM_CRTC_PROPS
};

@@ -316,6 +317,7 @@ struct igt_pipe {

   uint64_t mode_blob;
   bool mode_changed;
+    int64_t *out_fence_ptr;

I prefer the interface that got suggested before - igt_pipe gets an
"int64_t out_fence;" member which tests can query to get the fence
value:

int igt_pipe_get_last_out_fence(igt_pipe_t *pipe);

..and the kernel only ever receives a pointer to pipe->out_fence.

The only reason I can see for a test to want to provide its own fence
pointer is to test invalid pointer values - and I think it's OK for
that test to set the property directly instead of making setting a
custom fence pointer the common case for all users.

If we don't want to get a fence for every commit then I guess there
could be a way for a test to request an out-fence for just the next
commit on a pipe (or the inverse - disable fencing for a particular
commit):

void igt_pipe_request_out_fence(igt_pipe_t *pipe);

-Brian

Now I see what you meant in the v1 discussion, I'll amend the implementation in v3 to be the one mentioned above.


Partly... my main point on v1 was just to make sure the pointer was to
a 64-bit type.

The only question I have is about igt_pipe_get_last_out_fence(), is it really necessary? I don't foresee a function like that ever doing more than just returning a struct member. Is it not a bit redundant?


I see two reasons for it:
 - It means tests only deal with plain ints, which I think Chris was
   fairly adamant about on v1.
 - The getter can set pipe->out_fence to -1 when its called, making
   the ownership of the fd obvious.

In this case I guess before each commit out_fence needs to be checked,
close()'d if it looks valid, and set to -1 too.

Cheers,
Brian


Rob.


};

typedef struct {
@@ -369,6 +371,7 @@ static inline bool
igt_plane_supports_rotation(igt_plane_t *plane)
void igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t
length);
void igt_pipe_set_ctm_matrix(igt_pipe_t *pipe, void *ptr, size_t length);
void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
+void igt_pipe_set_out_fence_ptr(igt_pipe_t *pipe, int64_t *fence_ptr);

void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
void igt_plane_set_fence_fd(igt_plane_t *plane, uint32_t fence_fd);
--
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to