Re: [Intel-gfx] [PATCH i-g-t v2 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
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 Add support for the OUT_FENCE_PTR property to enable setting out fences for atomic commits. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- 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
Re: [Intel-gfx] [PATCH i-g-t v2 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
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 Add support for the OUT_FENCE_PTR property to enable setting out fences for atomic commits. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- 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. 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? 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
Re: [Intel-gfx] [PATCH i-g-t v2 07/12] lib/igt_kms: Add support for the OUT_FENCE_PTR property
Hi, On Wed, Dec 14, 2016 at 04:05:04AM -0500, Robert Foss wrote: From: Gustavo Padovan Add support for the OUT_FENCE_PTR property to enable setting out fences for atomic commits. Signed-off-by: Gustavo Padovan Signed-off-by: Robert Foss --- 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 }; 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