* [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
@ 2016-03-07 10:18 Pratik Vishwakarma
2016-03-07 12:50 ` Lionel Landwerlin
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Pratik Vishwakarma @ 2016-03-07 10:18 UTC (permalink / raw)
To: intel-gfx
From: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
Co-Author : Marius Vlad <marius.c.vlad@intel.com>
Co-Author : Pratik Vishwakarma <pratik.vishwakarma@intel.com>
So far we have had only two commit styles, COMMIT_LEGACY
and COMMIT_UNIVERSAL. This patch adds another commit style
COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
v2: (Marius)
i)Set CRTC_ID to zero while disabling plane
ii)Modified the log message in igt_atomic_prepare_plane_commit
https://patchwork.freedesktop.org/patch/71945/
v3: (Marius)Set FB_ID to zero while disabling plane
https://patchwork.freedesktop.org/patch/72179/
v4: (Maarten) Corrected the typo in commit message
https://patchwork.freedesktop.org/patch/72598/
v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
(Marius)
i)Removed unused props from igt_display_init
ii)Removed unused ret. Changed function to void
iii)Declare the variable before checking if we have
DRM_CLIENT_CAP_ATOMIC.
https://patchwork.freedesktop.org/patch/73505/
v6: (Jani) Corrected typo in commit message
https://patchwork.freedesktop.org/patch/74468/
v7: (Vlad & Maarten) Added is_atomic check for DRM_CLIENT_CAP_ATOMIC
in igt_display_init and igt_atomic_commit
Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma@intel.com>
---
lib/igt_kms.c | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
lib/igt_kms.h | 72 ++++++++++++-
2 files changed, 391 insertions(+), 2 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5a61def..1f738e1 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
*
* Returns: an alternate edid block
*/
+static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
+ "SRC_X",
+ "SRC_Y",
+ "SRC_W",
+ "SRC_H",
+ "CRTC_X",
+ "CRTC_Y",
+ "CRTC_W",
+ "CRTC_H",
+ "FB_ID",
+ "CRTC_ID",
+ "type",
+ "rotation"
+};
+
+static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
+ "background_color"
+};
+
+static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
+ "scaling mode",
+ "DPMS"
+};
+
+/*
+ * Retrieve all the properies specified in props_name and store them into
+ * plane->atomic_props_plane.
+ */
+static void
+igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
+ int num_props, const char **prop_names)
+{
+ drmModeObjectPropertiesPtr props;
+ int i, j, fd;
+
+ fd = display->drm_fd;
+
+ props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
+ igt_assert(props);
+
+ for (i = 0; i < props->count_props; i++) {
+ drmModePropertyPtr prop =
+ drmModeGetProperty(fd, props->props[i]);
+
+ for (j = 0; j < num_props; j++) {
+ if (strcmp(prop->name, prop_names[j]) != 0)
+ continue;
+
+ plane->atomic_props_plane[j] = props->props[i];
+ break;
+ }
+
+ drmModeFreeProperty(prop);
+ }
+
+ drmModeFreeObjectProperties(props);
+}
+
+/*
+ * Retrieve all the properies specified in props_name and store them into
+ * config->atomic_props_crtc and config->atomic_props_connector.
+ */
+static void
+igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
+ int num_crtc_props, const char **crtc_prop_names,
+ int num_connector_props, const char **conn_prop_names)
+{
+ drmModeObjectPropertiesPtr props;
+ int i, j, fd;
+
+ fd = display->drm_fd;
+
+ props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
+ igt_assert(props);
+
+ for (i = 0; i < props->count_props; i++) {
+ drmModePropertyPtr prop =
+ drmModeGetProperty(fd, props->props[i]);
+
+ for (j = 0; j < num_crtc_props; j++) {
+ if (strcmp(prop->name, crtc_prop_names[j]) != 0)
+ continue;
+
+ output->config.atomic_props_crtc[j] = props->props[i];
+ break;
+ }
+
+ drmModeFreeProperty(prop);
+ }
+
+ drmModeFreeObjectProperties(props);
+ props = NULL;
+ props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
+ igt_assert(props);
+
+ for (i = 0; i < props->count_props; i++) {
+ drmModePropertyPtr prop =
+ drmModeGetProperty(fd, props->props[i]);
+
+ for (j = 0; j < num_connector_props; j++) {
+ if (strcmp(prop->name, conn_prop_names[j]) != 0)
+ continue;
+
+ output->config.atomic_props_connector[j] = props->props[i];
+ break;
+ }
+
+ drmModeFreeProperty(prop);
+ }
+
+ drmModeFreeObjectProperties(props);
+
+}
+
const unsigned char* igt_kms_get_alt_edid(void)
{
update_edid_csum(alt_edid);
@@ -1002,6 +1116,8 @@ static void igt_output_refresh(igt_output_t *output)
kmstest_pipe_name(output->config.pipe));
display->pipes_in_use |= 1 << output->config.pipe;
+ igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
+ IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
}
static bool
@@ -1071,6 +1187,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
drmModeRes *resources;
drmModePlaneRes *plane_resources;
int i;
+ int ret = 0;
memset(display, 0, sizeof(igt_display_t));
@@ -1088,6 +1205,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
display->n_pipes = resources->count_crtcs;
drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+ ret = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
plane_resources = drmModeGetPlaneResources(display->drm_fd);
igt_assert(plane_resources);
@@ -1144,6 +1262,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
plane->pipe = pipe;
plane->drm_plane = drm_plane;
+ if (ret == 0) {
+ display->is_atomic = true;
+ igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
+ }
get_plane_property(display->drm_fd, drm_plane->plane_id,
"rotation",
@@ -1399,6 +1521,111 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
igt_assert(r == 0); \
}
+
+
+
+/*
+ * Add position and fb changes of a plane to the atomic property set
+ */
+static void
+igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
+ drmModeAtomicReq *req)
+{
+
+ igt_display_t *display = output->display;
+ uint32_t fb_id, crtc_id;
+ uint32_t src_x;
+ uint32_t src_y;
+ uint32_t src_w;
+ uint32_t src_h;
+ int32_t crtc_x;
+ int32_t crtc_y;
+ uint32_t crtc_w;
+ uint32_t crtc_h;
+
+ igt_assert(plane->drm_plane);
+
+ /* it's an error to try an unsupported feature */
+ igt_assert(igt_plane_supports_rotation(plane) ||
+ !plane->rotation_changed);
+
+ fb_id = igt_plane_get_fb_id(plane);
+ crtc_id = output->config.crtc->crtc_id;
+
+ if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
+
+ LOG(display,
+ "%s: populating plane data: pipe %s, plane %d, disabling\n",
+ igt_output_name(output),
+ kmstest_pipe_name(output->config.pipe),
+ plane->index);
+
+ /* populate plane req */
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
+
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
+
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
+
+ } else if (plane->fb_changed || plane->position_changed ||
+ plane->size_changed) {
+
+ src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
+ src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
+ src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
+ src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
+ crtc_x = plane->crtc_x;
+ crtc_y = plane->crtc_y;
+ crtc_w = plane->crtc_w;
+ crtc_h = plane->crtc_h;
+
+ LOG(display,
+ "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
+ "%ux%u dst = (%u, %u) %ux%u\n",
+ igt_output_name(output),
+ kmstest_pipe_name(output->config.pipe),
+ plane->index,
+ fb_id,
+ src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
+ crtc_x, crtc_y, crtc_w, crtc_h);
+
+
+ /* populate plane req */
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
+
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
+
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
+
+ if (plane->rotation_changed)
+ igt_atomic_populate_plane_req(req, plane,
+ IGT_PLANE_ROTATION, plane->rotation);
+
+ }
+
+ plane->fb_changed = false;
+ plane->position_changed = false;
+ plane->size_changed = false;
+
+ return;
+}
+
+
+
/*
* Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
* DRM call to program the plane fails, we'll either fail immediately (for
@@ -1651,7 +1878,7 @@ static int igt_plane_commit(igt_plane_t *plane,
return igt_primary_plane_commit_legacy(plane, output,
fail_on_error);
} else {
- return igt_drm_plane_commit(plane, output, fail_on_error);
+ return igt_drm_plane_commit(plane, output, fail_on_error);
}
}
@@ -1705,6 +1932,90 @@ static int igt_output_commit(igt_output_t *output,
return 0;
}
+
+/*
+ * Add crtc property changes to the atomic property set
+ */
+static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
+{
+
+ igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
+
+ if (pipe_obj->background_changed)
+ igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
+
+ /*
+ * TODO: Add all crtc level properties here
+ */
+
+}
+
+/*
+ * Add connector property changes to the atomic property set
+ */
+static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAtomicReq *req)
+{
+
+ struct kmstest_connector_config *config = &output->config;
+
+ if (config->connector_scaling_mode_changed)
+ igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
+
+ if (config->connector_dpms_changed)
+ igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_DPMS, config->connector_dpms);
+ /*
+ * TODO: Add all other connector level properties here
+ */
+
+}
+
+/*
+ * Commit all the changes of all the planes,crtcs, connectors
+ * atomically using drmModeAtomicCommit()
+ */
+static int igt_atomic_commit(igt_display_t *display)
+{
+
+ int ret = 0;
+ int i = 0;
+ drmModeAtomicReq *req;
+ if (display->is_atomic != true)
+ return -EINVAL;
+ req = drmModeAtomicAlloc();
+ drmModeAtomicSetCursor(req, 0);
+
+ for (i = 0; i < display->n_outputs; i++) {
+ igt_output_t *output = &display->outputs[i];
+ igt_pipe_t *pipe;
+
+ if (!output->valid)
+ continue;
+
+ /*
+ * Add CRTC Properties to the property set
+ */
+ igt_atomic_prepare_crtc_commit(output, req);
+
+ /*
+ * Add Connector Properties to the property set
+ */
+ igt_atomic_prepare_connector_commit(output, req);
+
+
+ pipe = igt_output_get_driving_pipe(output);
+
+ for (i = 0; i < pipe->n_planes; i++) {
+ igt_plane_t *plane = &pipe->planes[i];
+ igt_atomic_prepare_plane_commit(plane, output, req);
+ }
+
+ }
+
+ ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
+ drmModeAtomicFree(req);
+ return ret;
+
+}
/*
* Commit all plane changes across all outputs of the display.
*
@@ -1724,6 +2035,14 @@ static int do_display_commit(igt_display_t *display,
igt_display_refresh(display);
+ if (s == COMMIT_ATOMIC) {
+
+ ret = igt_atomic_commit(display);
+ CHECK_RETURN(ret, fail_on_error);
+ return 0;
+
+ }
+
for (i = 0; i < display->n_outputs; i++) {
igt_output_t *output = &display->outputs[i];
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 5744ed0..d6e2f12 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -106,11 +106,28 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
void kmstest_set_vt_graphics_mode(void);
void kmstest_restore_vt_mode(void);
+enum igt_atomic_crtc_properties {
+ IGT_CRTC_BACKGROUND = 0,
+ IGT_NUM_CRTC_PROPS
+};
+
+enum igt_atomic_connector_properties {
+ IGT_CONNECTOR_SCALING_MODE = 0,
+ IGT_CONNECTOR_DPMS,
+ IGT_NUM_CONNECTOR_PROPS
+};
+
struct kmstest_connector_config {
drmModeCrtc *crtc;
drmModeConnector *connector;
drmModeEncoder *encoder;
drmModeModeInfo default_mode;
+ uint64_t connector_scaling_mode;
+ bool connector_scaling_mode_changed;
+ uint64_t connector_dpms;
+ bool connector_dpms_changed;
+ uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
+ uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
int crtc_idx;
int pipe;
};
@@ -163,9 +180,28 @@ uint32_t kmstest_find_crtc_for_connector(int fd, drmModeRes *res,
enum igt_commit_style {
COMMIT_LEGACY = 0,
COMMIT_UNIVERSAL,
- /* We'll add atomic here eventually. */
+ COMMIT_ATOMIC,
};
+enum igt_atomic_plane_properties {
+ IGT_PLANE_SRC_X = 0,
+ IGT_PLANE_SRC_Y,
+ IGT_PLANE_SRC_W,
+ IGT_PLANE_SRC_H,
+
+ IGT_PLANE_CRTC_X,
+ IGT_PLANE_CRTC_Y,
+ IGT_PLANE_CRTC_W,
+ IGT_PLANE_CRTC_H,
+
+ IGT_PLANE_FB_ID,
+ IGT_PLANE_CRTC_ID,
+ IGT_PLANE_TYPE,
+ IGT_PLANE_ROTATION,
+ IGT_NUM_PLANE_PROPS
+};
+
+
typedef struct igt_display igt_display_t;
typedef struct igt_pipe igt_pipe_t;
typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
@@ -207,6 +243,7 @@ typedef struct {
/* panning offset within the fb */
unsigned int pan_x, pan_y;
igt_rotation_t rotation;
+ uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
} igt_plane_t;
struct igt_pipe {
@@ -242,6 +279,7 @@ struct igt_display {
igt_output_t *outputs;
igt_pipe_t pipes[I915_MAX_PIPES];
bool has_universal_planes;
+ bool is_atomic;
};
void igt_display_init(igt_display_t *display, int drm_fd);
@@ -288,6 +326,38 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
#define IGT_FIXED(i,f) ((i) << 16 | (f))
+/**
+ * igt_atomic_populate_plane_req:
+ * @req: A pointer to drmModeAtomicReq
+ * @plane: A pointer igt_plane_t
+ * @prop: one of igt_atomic_plane_properties
+ * @value: the value to add
+ */
+#define igt_atomic_populate_plane_req(req, plane, prop, value) \
+ igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
+ plane->atomic_props_plane[prop], value))
+
+/**
+ * igt_atomic_populate_crtc_req:
+ * @req: A pointer to drmModeAtomicReq
+ * @output: A pointer igt_output_t
+ * @prop: one of igt_atomic_crtc_properties
+ * @value: the value to add
+ */
+#define igt_atomic_populate_crtc_req(req, output, prop, value) \
+ igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crtc_id,\
+ output->config.atomic_props_crtc[prop], value))
+/**
+ * igt_atomic_populate_connector_req:
+ * @req: A pointer to drmModeAtomicReq
+ * @output: A pointer igt_output_t
+ * @prop: one of igt_atomic_connector_properties
+ * @value: the value to add
+ */
+#define igt_atomic_populate_connector_req(req, output, prop, value) \
+ igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector->connector_id,\
+ output->config.atomic_props_connector[prop], value))
+
void igt_enable_connectors(void);
void igt_reset_connectors(void);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-07 10:18 [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2() Pratik Vishwakarma
@ 2016-03-07 12:50 ` Lionel Landwerlin
2016-03-08 15:56 ` Maarten Lankhorst
2016-03-07 12:50 ` Lionel Landwerlin
2016-03-08 16:58 ` Matthew Auld
2 siblings, 1 reply; 8+ messages in thread
From: Lionel Landwerlin @ 2016-03-07 12:50 UTC (permalink / raw)
To: Pratik Vishwakarma, intel-gfx
Hi Pratik,
I'm really looking forward to get this merged.
Just a few comments on the plane commit part below.
Cheers,
-
Lionel
On 07/03/16 10:18, Pratik Vishwakarma wrote:
> From: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
>
> Co-Author : Marius Vlad <marius.c.vlad@intel.com>
> Co-Author : Pratik Vishwakarma <pratik.vishwakarma@intel.com>
>
> So far we have had only two commit styles, COMMIT_LEGACY
> and COMMIT_UNIVERSAL. This patch adds another commit style
> COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
>
> v2: (Marius)
> i)Set CRTC_ID to zero while disabling plane
> ii)Modified the log message in igt_atomic_prepare_plane_commit
> https://patchwork.freedesktop.org/patch/71945/
>
> v3: (Marius)Set FB_ID to zero while disabling plane
> https://patchwork.freedesktop.org/patch/72179/
>
> v4: (Maarten) Corrected the typo in commit message
> https://patchwork.freedesktop.org/patch/72598/
>
> v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
> (Marius)
> i)Removed unused props from igt_display_init
> ii)Removed unused ret. Changed function to void
> iii)Declare the variable before checking if we have
> DRM_CLIENT_CAP_ATOMIC.
> https://patchwork.freedesktop.org/patch/73505/
>
> v6: (Jani) Corrected typo in commit message
> https://patchwork.freedesktop.org/patch/74468/
>
> v7: (Vlad & Maarten) Added is_atomic check for DRM_CLIENT_CAP_ATOMIC
> in igt_display_init and igt_atomic_commit
>
> Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
> Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma@intel.com>
> ---
> lib/igt_kms.c | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/igt_kms.h | 72 ++++++++++++-
> 2 files changed, 391 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5a61def..1f738e1 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
> *
> * Returns: an alternate edid block
> */
> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> + "SRC_X",
> + "SRC_Y",
> + "SRC_W",
> + "SRC_H",
> + "CRTC_X",
> + "CRTC_Y",
> + "CRTC_W",
> + "CRTC_H",
> + "FB_ID",
> + "CRTC_ID",
> + "type",
> + "rotation"
> +};
> +
> +static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> + "background_color"
> +};
> +
> +static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> + "scaling mode",
> + "DPMS"
> +};
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * plane->atomic_props_plane.
> + */
> +static void
> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> + int num_props, const char **prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_props; j++) {
> + if (strcmp(prop->name, prop_names[j]) != 0)
> + continue;
> +
> + plane->atomic_props_plane[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +}
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * config->atomic_props_crtc and config->atomic_props_connector.
> + */
> +static void
> +igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
> + int num_crtc_props, const char **crtc_prop_names,
> + int num_connector_props, const char **conn_prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_crtc_props; j++) {
> + if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_crtc[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> + props = NULL;
> + props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_connector_props; j++) {
> + if (strcmp(prop->name, conn_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_connector[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +
> +}
> +
> const unsigned char* igt_kms_get_alt_edid(void)
> {
> update_edid_csum(alt_edid);
> @@ -1002,6 +1116,8 @@ static void igt_output_refresh(igt_output_t *output)
> kmstest_pipe_name(output->config.pipe));
>
> display->pipes_in_use |= 1 << output->config.pipe;
> + igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
> + IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
> }
>
> static bool
> @@ -1071,6 +1187,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> drmModeRes *resources;
> drmModePlaneRes *plane_resources;
> int i;
> + int ret = 0;
>
> memset(display, 0, sizeof(igt_display_t));
>
> @@ -1088,6 +1205,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> display->n_pipes = resources->count_crtcs;
>
> drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> + ret = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> @@ -1144,6 +1262,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>
> plane->pipe = pipe;
> plane->drm_plane = drm_plane;
> + if (ret == 0) {
> + display->is_atomic = true;
> + igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> + }
>
> get_plane_property(display->drm_fd, drm_plane->plane_id,
> "rotation",
> @@ -1399,6 +1521,111 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> igt_assert(r == 0); \
> }
>
> +
> +
> +
> +/*
> + * Add position and fb changes of a plane to the atomic property set
> + */
> +static void
> +igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> + drmModeAtomicReq *req)
> +{
> +
> + igt_display_t *display = output->display;
> + uint32_t fb_id, crtc_id;
> + uint32_t src_x;
> + uint32_t src_y;
> + uint32_t src_w;
> + uint32_t src_h;
> + int32_t crtc_x;
> + int32_t crtc_y;
> + uint32_t crtc_w;
> + uint32_t crtc_h;
> +
> + igt_assert(plane->drm_plane);
> +
> + /* it's an error to try an unsupported feature */
> + igt_assert(igt_plane_supports_rotation(plane) ||
> + !plane->rotation_changed);
> +
> + fb_id = igt_plane_get_fb_id(plane);
> + crtc_id = output->config.crtc->crtc_id;
> +
> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> +
> + LOG(display,
> + "%s: populating plane data: pipe %s, plane %d, disabling\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index);
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
Why resetting all these properties? It seems unnecessary.
If there is a reason for this, you might want to synchronize the fields
of igt_plane_t.
> +
> + } else if (plane->fb_changed || plane->position_changed ||
> + plane->size_changed) {
> +
> + src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> + src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> + src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> + src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> + crtc_x = plane->crtc_x;
> + crtc_y = plane->crtc_y;
> + crtc_w = plane->crtc_w;
> + crtc_h = plane->crtc_h;
> +
> + LOG(display,
> + "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> + "%ux%u dst = (%u, %u) %ux%u\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index,
> + fb_id,
> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> + crtc_x, crtc_y, crtc_w, crtc_h);
> +
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> +
> + if (plane->rotation_changed)
> + igt_atomic_populate_plane_req(req, plane,
> + IGT_PLANE_ROTATION, plane->rotation);
What happens if only the rotation has changed?
This condition should be outside the if (plane->fb_changed ||
plane->position_changed || plane->size_changed).
> +
> + }
> +
> + plane->fb_changed = false;
> + plane->position_changed = false;
> + plane->size_changed = false;
Overall I think this function would be a lot clearer if you just had a
test for each of the *_changed properties and just submitted the
associated properties in each case.
> +
> + return;
> +}
> +
> +
> +
> /*
> * Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
> * DRM call to program the plane fails, we'll either fail immediately (for
> @@ -1651,7 +1878,7 @@ static int igt_plane_commit(igt_plane_t *plane,
> return igt_primary_plane_commit_legacy(plane, output,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, output, fail_on_error);
> + return igt_drm_plane_commit(plane, output, fail_on_error);
> }
> }
>
> @@ -1705,6 +1932,90 @@ static int igt_output_commit(igt_output_t *output,
> return 0;
> }
>
> +
> +/*
> + * Add crtc property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
> +
> + if (pipe_obj->background_changed)
> + igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
> +
> + /*
> + * TODO: Add all crtc level properties here
> + */
> +
> +}
> +
> +/*
> + * Add connector property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + struct kmstest_connector_config *config = &output->config;
> +
> + if (config->connector_scaling_mode_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
> +
> + if (config->connector_dpms_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_DPMS, config->connector_dpms);
> + /*
> + * TODO: Add all other connector level properties here
> + */
> +
> +}
> +
> +/*
> + * Commit all the changes of all the planes,crtcs, connectors
> + * atomically using drmModeAtomicCommit()
> + */
> +static int igt_atomic_commit(igt_display_t *display)
> +{
> +
> + int ret = 0;
> + int i = 0;
> + drmModeAtomicReq *req;
> + if (display->is_atomic != true)
> + return -EINVAL;
> + req = drmModeAtomicAlloc();
> + drmModeAtomicSetCursor(req, 0);
> +
> + for (i = 0; i < display->n_outputs; i++) {
> + igt_output_t *output = &display->outputs[i];
> + igt_pipe_t *pipe;
> +
> + if (!output->valid)
> + continue;
> +
> + /*
> + * Add CRTC Properties to the property set
> + */
> + igt_atomic_prepare_crtc_commit(output, req);
> +
> + /*
> + * Add Connector Properties to the property set
> + */
> + igt_atomic_prepare_connector_commit(output, req);
> +
> +
> + pipe = igt_output_get_driving_pipe(output);
> +
> + for (i = 0; i < pipe->n_planes; i++) {
> + igt_plane_t *plane = &pipe->planes[i];
> + igt_atomic_prepare_plane_commit(plane, output, req);
> + }
> +
> + }
> +
> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> + drmModeAtomicFree(req);
> + return ret;
> +
> +}
> /*
> * Commit all plane changes across all outputs of the display.
> *
> @@ -1724,6 +2035,14 @@ static int do_display_commit(igt_display_t *display,
>
> igt_display_refresh(display);
>
> + if (s == COMMIT_ATOMIC) {
> +
> + ret = igt_atomic_commit(display);
> + CHECK_RETURN(ret, fail_on_error);
> + return 0;
> +
> + }
> +
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *output = &display->outputs[i];
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5744ed0..d6e2f12 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,11 +106,28 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
> void kmstest_set_vt_graphics_mode(void);
> void kmstest_restore_vt_mode(void);
>
> +enum igt_atomic_crtc_properties {
> + IGT_CRTC_BACKGROUND = 0,
> + IGT_NUM_CRTC_PROPS
> +};
> +
> +enum igt_atomic_connector_properties {
> + IGT_CONNECTOR_SCALING_MODE = 0,
> + IGT_CONNECTOR_DPMS,
> + IGT_NUM_CONNECTOR_PROPS
> +};
> +
> struct kmstest_connector_config {
> drmModeCrtc *crtc;
> drmModeConnector *connector;
> drmModeEncoder *encoder;
> drmModeModeInfo default_mode;
> + uint64_t connector_scaling_mode;
> + bool connector_scaling_mode_changed;
> + uint64_t connector_dpms;
> + bool connector_dpms_changed;
> + uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> + uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
> int crtc_idx;
> int pipe;
> };
> @@ -163,9 +180,28 @@ uint32_t kmstest_find_crtc_for_connector(int fd, drmModeRes *res,
> enum igt_commit_style {
> COMMIT_LEGACY = 0,
> COMMIT_UNIVERSAL,
> - /* We'll add atomic here eventually. */
> + COMMIT_ATOMIC,
> };
>
> +enum igt_atomic_plane_properties {
> + IGT_PLANE_SRC_X = 0,
> + IGT_PLANE_SRC_Y,
> + IGT_PLANE_SRC_W,
> + IGT_PLANE_SRC_H,
> +
> + IGT_PLANE_CRTC_X,
> + IGT_PLANE_CRTC_Y,
> + IGT_PLANE_CRTC_W,
> + IGT_PLANE_CRTC_H,
> +
> + IGT_PLANE_FB_ID,
> + IGT_PLANE_CRTC_ID,
> + IGT_PLANE_TYPE,
> + IGT_PLANE_ROTATION,
> + IGT_NUM_PLANE_PROPS
> +};
> +
> +
> typedef struct igt_display igt_display_t;
> typedef struct igt_pipe igt_pipe_t;
> typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
> @@ -207,6 +243,7 @@ typedef struct {
> /* panning offset within the fb */
> unsigned int pan_x, pan_y;
> igt_rotation_t rotation;
> + uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
> struct igt_pipe {
> @@ -242,6 +279,7 @@ struct igt_display {
> igt_output_t *outputs;
> igt_pipe_t pipes[I915_MAX_PIPES];
> bool has_universal_planes;
> + bool is_atomic;
> };
>
> void igt_display_init(igt_display_t *display, int drm_fd);
> @@ -288,6 +326,38 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> #define IGT_FIXED(i,f) ((i) << 16 | (f))
>
> +/**
> + * igt_atomic_populate_plane_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @plane: A pointer igt_plane_t
> + * @prop: one of igt_atomic_plane_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_plane_req(req, plane, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
> + plane->atomic_props_plane[prop], value))
> +
> +/**
> + * igt_atomic_populate_crtc_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_crtc_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_crtc_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crtc_id,\
> + output->config.atomic_props_crtc[prop], value))
> +/**
> + * igt_atomic_populate_connector_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_connector_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_connector_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector->connector_id,\
> + output->config.atomic_props_connector[prop], value))
> +
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-07 10:18 [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2() Pratik Vishwakarma
2016-03-07 12:50 ` Lionel Landwerlin
@ 2016-03-07 12:50 ` Lionel Landwerlin
2016-03-08 16:58 ` Matthew Auld
2 siblings, 0 replies; 8+ messages in thread
From: Lionel Landwerlin @ 2016-03-07 12:50 UTC (permalink / raw)
To: Pratik Vishwakarma, intel-gfx
Hi Pratik,
I'm really looking forward to get this merged.
Just a few comments on the plane commit part below.
Cheers,
-
Lionel
On 07/03/16 10:18, Pratik Vishwakarma wrote:
> From: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
>
> Co-Author : Marius Vlad <marius.c.vlad@intel.com>
> Co-Author : Pratik Vishwakarma <pratik.vishwakarma@intel.com>
>
> So far we have had only two commit styles, COMMIT_LEGACY
> and COMMIT_UNIVERSAL. This patch adds another commit style
> COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
>
> v2: (Marius)
> i)Set CRTC_ID to zero while disabling plane
> ii)Modified the log message in igt_atomic_prepare_plane_commit
> https://patchwork.freedesktop.org/patch/71945/
>
> v3: (Marius)Set FB_ID to zero while disabling plane
> https://patchwork.freedesktop.org/patch/72179/
>
> v4: (Maarten) Corrected the typo in commit message
> https://patchwork.freedesktop.org/patch/72598/
>
> v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
> (Marius)
> i)Removed unused props from igt_display_init
> ii)Removed unused ret. Changed function to void
> iii)Declare the variable before checking if we have
> DRM_CLIENT_CAP_ATOMIC.
> https://patchwork.freedesktop.org/patch/73505/
>
> v6: (Jani) Corrected typo in commit message
> https://patchwork.freedesktop.org/patch/74468/
>
> v7: (Vlad & Maarten) Added is_atomic check for DRM_CLIENT_CAP_ATOMIC
> in igt_display_init and igt_atomic_commit
>
> Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
> Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma@intel.com>
> ---
> lib/igt_kms.c | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/igt_kms.h | 72 ++++++++++++-
> 2 files changed, 391 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5a61def..1f738e1 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
> *
> * Returns: an alternate edid block
> */
> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> + "SRC_X",
> + "SRC_Y",
> + "SRC_W",
> + "SRC_H",
> + "CRTC_X",
> + "CRTC_Y",
> + "CRTC_W",
> + "CRTC_H",
> + "FB_ID",
> + "CRTC_ID",
> + "type",
> + "rotation"
> +};
> +
> +static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> + "background_color"
> +};
> +
> +static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> + "scaling mode",
> + "DPMS"
> +};
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * plane->atomic_props_plane.
> + */
> +static void
> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> + int num_props, const char **prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_props; j++) {
> + if (strcmp(prop->name, prop_names[j]) != 0)
> + continue;
> +
> + plane->atomic_props_plane[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +}
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * config->atomic_props_crtc and config->atomic_props_connector.
> + */
> +static void
> +igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
> + int num_crtc_props, const char **crtc_prop_names,
> + int num_connector_props, const char **conn_prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_crtc_props; j++) {
> + if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_crtc[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> + props = NULL;
> + props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_connector_props; j++) {
> + if (strcmp(prop->name, conn_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_connector[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +
> +}
> +
> const unsigned char* igt_kms_get_alt_edid(void)
> {
> update_edid_csum(alt_edid);
> @@ -1002,6 +1116,8 @@ static void igt_output_refresh(igt_output_t *output)
> kmstest_pipe_name(output->config.pipe));
>
> display->pipes_in_use |= 1 << output->config.pipe;
> + igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
> + IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
> }
>
> static bool
> @@ -1071,6 +1187,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> drmModeRes *resources;
> drmModePlaneRes *plane_resources;
> int i;
> + int ret = 0;
>
> memset(display, 0, sizeof(igt_display_t));
>
> @@ -1088,6 +1205,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> display->n_pipes = resources->count_crtcs;
>
> drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> + ret = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> @@ -1144,6 +1262,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>
> plane->pipe = pipe;
> plane->drm_plane = drm_plane;
> + if (ret == 0) {
> + display->is_atomic = true;
> + igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> + }
>
> get_plane_property(display->drm_fd, drm_plane->plane_id,
> "rotation",
> @@ -1399,6 +1521,111 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> igt_assert(r == 0); \
> }
>
> +
> +
> +
> +/*
> + * Add position and fb changes of a plane to the atomic property set
> + */
> +static void
> +igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> + drmModeAtomicReq *req)
> +{
> +
> + igt_display_t *display = output->display;
> + uint32_t fb_id, crtc_id;
> + uint32_t src_x;
> + uint32_t src_y;
> + uint32_t src_w;
> + uint32_t src_h;
> + int32_t crtc_x;
> + int32_t crtc_y;
> + uint32_t crtc_w;
> + uint32_t crtc_h;
> +
> + igt_assert(plane->drm_plane);
> +
> + /* it's an error to try an unsupported feature */
> + igt_assert(igt_plane_supports_rotation(plane) ||
> + !plane->rotation_changed);
> +
> + fb_id = igt_plane_get_fb_id(plane);
> + crtc_id = output->config.crtc->crtc_id;
> +
> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> +
> + LOG(display,
> + "%s: populating plane data: pipe %s, plane %d, disabling\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index);
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
Why resetting all these properties? It seems unnecessary.
If there is a reason for this, you might want to synchronize the fields
of igt_plane_t.
> +
> + } else if (plane->fb_changed || plane->position_changed ||
> + plane->size_changed) {
> +
> + src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> + src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> + src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> + src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> + crtc_x = plane->crtc_x;
> + crtc_y = plane->crtc_y;
> + crtc_w = plane->crtc_w;
> + crtc_h = plane->crtc_h;
> +
> + LOG(display,
> + "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> + "%ux%u dst = (%u, %u) %ux%u\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index,
> + fb_id,
> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> + crtc_x, crtc_y, crtc_w, crtc_h);
> +
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> +
> + if (plane->rotation_changed)
> + igt_atomic_populate_plane_req(req, plane,
> + IGT_PLANE_ROTATION, plane->rotation);
What happens if only the rotation has changed?
This condition should be outside the if (plane->fb_changed ||
plane->position_changed || plane->size_changed).
> +
> + }
> +
> + plane->fb_changed = false;
> + plane->position_changed = false;
> + plane->size_changed = false;
Overall I think this function would be a lot clearer if you just had a
test for each of the *_changed properties and just submitted the
associated properties in each case.
> +
> + return;
> +}
> +
> +
> +
> /*
> * Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
> * DRM call to program the plane fails, we'll either fail immediately (for
> @@ -1651,7 +1878,7 @@ static int igt_plane_commit(igt_plane_t *plane,
> return igt_primary_plane_commit_legacy(plane, output,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, output, fail_on_error);
> + return igt_drm_plane_commit(plane, output, fail_on_error);
> }
> }
>
> @@ -1705,6 +1932,90 @@ static int igt_output_commit(igt_output_t *output,
> return 0;
> }
>
> +
> +/*
> + * Add crtc property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
> +
> + if (pipe_obj->background_changed)
> + igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
> +
> + /*
> + * TODO: Add all crtc level properties here
> + */
> +
> +}
> +
> +/*
> + * Add connector property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + struct kmstest_connector_config *config = &output->config;
> +
> + if (config->connector_scaling_mode_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
> +
> + if (config->connector_dpms_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_DPMS, config->connector_dpms);
> + /*
> + * TODO: Add all other connector level properties here
> + */
> +
> +}
> +
> +/*
> + * Commit all the changes of all the planes,crtcs, connectors
> + * atomically using drmModeAtomicCommit()
> + */
> +static int igt_atomic_commit(igt_display_t *display)
> +{
> +
> + int ret = 0;
> + int i = 0;
> + drmModeAtomicReq *req;
> + if (display->is_atomic != true)
> + return -EINVAL;
> + req = drmModeAtomicAlloc();
> + drmModeAtomicSetCursor(req, 0);
> +
> + for (i = 0; i < display->n_outputs; i++) {
> + igt_output_t *output = &display->outputs[i];
> + igt_pipe_t *pipe;
> +
> + if (!output->valid)
> + continue;
> +
> + /*
> + * Add CRTC Properties to the property set
> + */
> + igt_atomic_prepare_crtc_commit(output, req);
> +
> + /*
> + * Add Connector Properties to the property set
> + */
> + igt_atomic_prepare_connector_commit(output, req);
> +
> +
> + pipe = igt_output_get_driving_pipe(output);
> +
> + for (i = 0; i < pipe->n_planes; i++) {
> + igt_plane_t *plane = &pipe->planes[i];
> + igt_atomic_prepare_plane_commit(plane, output, req);
> + }
> +
> + }
> +
> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> + drmModeAtomicFree(req);
> + return ret;
> +
> +}
> /*
> * Commit all plane changes across all outputs of the display.
> *
> @@ -1724,6 +2035,14 @@ static int do_display_commit(igt_display_t *display,
>
> igt_display_refresh(display);
>
> + if (s == COMMIT_ATOMIC) {
> +
> + ret = igt_atomic_commit(display);
> + CHECK_RETURN(ret, fail_on_error);
> + return 0;
> +
> + }
> +
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *output = &display->outputs[i];
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5744ed0..d6e2f12 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,11 +106,28 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
> void kmstest_set_vt_graphics_mode(void);
> void kmstest_restore_vt_mode(void);
>
> +enum igt_atomic_crtc_properties {
> + IGT_CRTC_BACKGROUND = 0,
> + IGT_NUM_CRTC_PROPS
> +};
> +
> +enum igt_atomic_connector_properties {
> + IGT_CONNECTOR_SCALING_MODE = 0,
> + IGT_CONNECTOR_DPMS,
> + IGT_NUM_CONNECTOR_PROPS
> +};
> +
> struct kmstest_connector_config {
> drmModeCrtc *crtc;
> drmModeConnector *connector;
> drmModeEncoder *encoder;
> drmModeModeInfo default_mode;
> + uint64_t connector_scaling_mode;
> + bool connector_scaling_mode_changed;
> + uint64_t connector_dpms;
> + bool connector_dpms_changed;
> + uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> + uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
> int crtc_idx;
> int pipe;
> };
> @@ -163,9 +180,28 @@ uint32_t kmstest_find_crtc_for_connector(int fd, drmModeRes *res,
> enum igt_commit_style {
> COMMIT_LEGACY = 0,
> COMMIT_UNIVERSAL,
> - /* We'll add atomic here eventually. */
> + COMMIT_ATOMIC,
> };
>
> +enum igt_atomic_plane_properties {
> + IGT_PLANE_SRC_X = 0,
> + IGT_PLANE_SRC_Y,
> + IGT_PLANE_SRC_W,
> + IGT_PLANE_SRC_H,
> +
> + IGT_PLANE_CRTC_X,
> + IGT_PLANE_CRTC_Y,
> + IGT_PLANE_CRTC_W,
> + IGT_PLANE_CRTC_H,
> +
> + IGT_PLANE_FB_ID,
> + IGT_PLANE_CRTC_ID,
> + IGT_PLANE_TYPE,
> + IGT_PLANE_ROTATION,
> + IGT_NUM_PLANE_PROPS
> +};
> +
> +
> typedef struct igt_display igt_display_t;
> typedef struct igt_pipe igt_pipe_t;
> typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
> @@ -207,6 +243,7 @@ typedef struct {
> /* panning offset within the fb */
> unsigned int pan_x, pan_y;
> igt_rotation_t rotation;
> + uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
> struct igt_pipe {
> @@ -242,6 +279,7 @@ struct igt_display {
> igt_output_t *outputs;
> igt_pipe_t pipes[I915_MAX_PIPES];
> bool has_universal_planes;
> + bool is_atomic;
> };
>
> void igt_display_init(igt_display_t *display, int drm_fd);
> @@ -288,6 +326,38 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> #define IGT_FIXED(i,f) ((i) << 16 | (f))
>
> +/**
> + * igt_atomic_populate_plane_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @plane: A pointer igt_plane_t
> + * @prop: one of igt_atomic_plane_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_plane_req(req, plane, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
> + plane->atomic_props_plane[prop], value))
> +
> +/**
> + * igt_atomic_populate_crtc_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_crtc_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_crtc_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crtc_id,\
> + output->config.atomic_props_crtc[prop], value))
> +/**
> + * igt_atomic_populate_connector_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_connector_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_connector_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector->connector_id,\
> + output->config.atomic_props_connector[prop], value))
> +
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-07 12:50 ` Lionel Landwerlin
@ 2016-03-08 15:56 ` Maarten Lankhorst
2016-03-08 16:11 ` Lionel Landwerlin
0 siblings, 1 reply; 8+ messages in thread
From: Maarten Lankhorst @ 2016-03-08 15:56 UTC (permalink / raw)
To: Lionel Landwerlin, Pratik Vishwakarma, intel-gfx
Op 07-03-16 om 13:50 schreef Lionel Landwerlin:
> Hi Pratik,
>
> I'm really looking forward to get this merged.
> Just a few comments on the plane commit part below.
Would the following diff to the patch satisfy you? It would clean up things a lot.
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 1f738e14f52f..454ff7552b4a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1534,14 +1534,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
igt_display_t *display = output->display;
uint32_t fb_id, crtc_id;
- uint32_t src_x;
- uint32_t src_y;
- uint32_t src_w;
- uint32_t src_h;
- int32_t crtc_x;
- int32_t crtc_y;
- uint32_t crtc_w;
- uint32_t crtc_h;
igt_assert(plane->drm_plane);
@@ -1552,54 +1544,30 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
fb_id = igt_plane_get_fb_id(plane);
crtc_id = output->config.crtc->crtc_id;
- if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
-
- LOG(display,
- "%s: populating plane data: pipe %s, plane %d, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
- plane->index);
-
- /* populate plane req */
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
-
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
-
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
-
- } else if (plane->fb_changed || plane->position_changed ||
- plane->size_changed) {
-
- src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
- src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
- src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
- src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
- crtc_x = plane->crtc_x;
- crtc_y = plane->crtc_y;
- crtc_w = plane->crtc_w;
- crtc_h = plane->crtc_h;
-
- LOG(display,
- "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
- "%ux%u dst = (%u, %u) %ux%u\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
- plane->index,
- fb_id,
- src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
- crtc_x, crtc_y, crtc_w, crtc_h);
-
-
- /* populate plane req */
- igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
+ LOG(display,
+ "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
+ "%ux%u dst = (%u, %u) %ux%u\n",
+ igt_output_name(output),
+ kmstest_pipe_name(output->config.pipe),
+ plane->index,
+ fb_id,
+ src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
+ crtc_x, crtc_y, crtc_w, crtc_h);
+
+ if (plane->fb_changed) {
+ igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
+ }
+
+ if (plane->position_changed || plane->size_changed) {
+ uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
+ uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
+ uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
+ uint32_t src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
+ int32_t crtc_x = plane->crtc_x;
+ int32_t crtc_y = plane->crtc_y;
+ uint32_t crtc_w = plane->crtc_w;
+ uint32_t crtc_h = plane->crtc_h;
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
@@ -1610,18 +1578,15 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
-
- if (plane->rotation_changed)
- igt_atomic_populate_plane_req(req, plane,
- IGT_PLANE_ROTATION, plane->rotation);
-
}
+ if (plane->rotation_changed)
+ igt_atomic_populate_plane_req(req, plane,
+ IGT_PLANE_ROTATION, plane->rotation);
+
plane->fb_changed = false;
plane->position_changed = false;
plane->size_changed = false;
-
- return;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-08 15:56 ` Maarten Lankhorst
@ 2016-03-08 16:11 ` Lionel Landwerlin
2016-03-09 14:25 ` Mayuresh Gharpure
0 siblings, 1 reply; 8+ messages in thread
From: Lionel Landwerlin @ 2016-03-08 16:11 UTC (permalink / raw)
To: Maarten Lankhorst, Pratik Vishwakarma, intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 5488 bytes --]
Hi Maarten,
Yeah that would be much simpler I think.
Damien also looked at this patch over my shoulder and had additional
comments.
So more or less proxying :
- Not sure why we're exposing the new enums and the
igt_atomic_popuplate_* macros in igt_kms.h.
After all we're not using them anywhere outside igt_kms.c.
- We have a couple of properties ids stored in igt_kms.h
(background_property & rotation_property), it would make sense to get
rid of those and use the new created arrays instead.
Cheers,
-
Lionel
On 08/03/16 15:56, Maarten Lankhorst wrote:
> Op 07-03-16 om 13:50 schreef Lionel Landwerlin:
>> Hi Pratik,
>>
>> I'm really looking forward to get this merged.
>> Just a few comments on the plane commit part below.
> Would the following diff to the patch satisfy you? It would clean up things a lot.
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 1f738e14f52f..454ff7552b4a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1534,14 +1534,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
>
> igt_display_t *display = output->display;
> uint32_t fb_id, crtc_id;
> - uint32_t src_x;
> - uint32_t src_y;
> - uint32_t src_w;
> - uint32_t src_h;
> - int32_t crtc_x;
> - int32_t crtc_y;
> - uint32_t crtc_w;
> - uint32_t crtc_h;
>
> igt_assert(plane->drm_plane);
>
> @@ -1552,54 +1544,30 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> fb_id = igt_plane_get_fb_id(plane);
> crtc_id = output->config.crtc->crtc_id;
>
> - if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> -
> - LOG(display,
> - "%s: populating plane data: pipe %s, plane %d, disabling\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> - plane->index);
> -
> - /* populate plane req */
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
> -
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> -
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
> -
> - } else if (plane->fb_changed || plane->position_changed ||
> - plane->size_changed) {
> -
> - src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> - src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> - src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> - src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> - crtc_x = plane->crtc_x;
> - crtc_y = plane->crtc_y;
> - crtc_w = plane->crtc_w;
> - crtc_h = plane->crtc_h;
> -
> - LOG(display,
> - "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> - "%ux%u dst = (%u, %u) %ux%u\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> - plane->index,
> - fb_id,
> - src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> - crtc_x, crtc_y, crtc_w, crtc_h);
> -
> -
> - /* populate plane req */
> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> + LOG(display,
> + "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> + "%ux%u dst = (%u, %u) %ux%u\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index,
> + fb_id,
> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> + crtc_x, crtc_y, crtc_w, crtc_h);
> +
> + if (plane->fb_changed) {
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> + }
> +
> + if (plane->position_changed || plane->size_changed) {
> + uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> + uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> + uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> + uint32_t src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> + int32_t crtc_x = plane->crtc_x;
> + int32_t crtc_y = plane->crtc_y;
> + uint32_t crtc_w = plane->crtc_w;
> + uint32_t crtc_h = plane->crtc_h;
>
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> @@ -1610,18 +1578,15 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> -
> - if (plane->rotation_changed)
> - igt_atomic_populate_plane_req(req, plane,
> - IGT_PLANE_ROTATION, plane->rotation);
> -
> }
>
> + if (plane->rotation_changed)
> + igt_atomic_populate_plane_req(req, plane,
> + IGT_PLANE_ROTATION, plane->rotation);
> +
> plane->fb_changed = false;
> plane->position_changed = false;
> plane->size_changed = false;
I missed that, but I think we need rotation_changed = false; here too.
> -
> - return;
> }
>
>
>
[-- Attachment #1.2: Type: text/html, Size: 6444 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-07 10:18 [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2() Pratik Vishwakarma
2016-03-07 12:50 ` Lionel Landwerlin
2016-03-07 12:50 ` Lionel Landwerlin
@ 2016-03-08 16:58 ` Matthew Auld
2 siblings, 0 replies; 8+ messages in thread
From: Matthew Auld @ 2016-03-08 16:58 UTC (permalink / raw)
To: Pratik Vishwakarma; +Cc: intel-gfx
I think it would be more idiomatic to replace:
+ for (i = 0; i < display->n_outputs; i++) {
+ igt_output_t *output = &display->outputs[i];
with:
+ for_each_connected_output(display, output)
And:
+ for (i = 0; i < pipe->n_planes; i++) {
+ igt_plane_t *plane = &pipe->planes[i];
with:
+ for_each_plane_on_pipe(display, pipe, plane)
On 7 March 2016 at 10:18, Pratik Vishwakarma
<pratik.vishwakarma@intel.com> wrote:
> From: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
>
> Co-Author : Marius Vlad <marius.c.vlad@intel.com>
> Co-Author : Pratik Vishwakarma <pratik.vishwakarma@intel.com>
>
> So far we have had only two commit styles, COMMIT_LEGACY
> and COMMIT_UNIVERSAL. This patch adds another commit style
> COMMIT_ATOMIC which makes use of drmModeAtomicCommit()
>
> v2: (Marius)
> i)Set CRTC_ID to zero while disabling plane
> ii)Modified the log message in igt_atomic_prepare_plane_commit
> https://patchwork.freedesktop.org/patch/71945/
>
> v3: (Marius)Set FB_ID to zero while disabling plane
> https://patchwork.freedesktop.org/patch/72179/
>
> v4: (Maarten) Corrected the typo in commit message
> https://patchwork.freedesktop.org/patch/72598/
>
> v5: Added check for DRM_CLIENT_CAP_ATOMIC in igt_display_init
> (Marius)
> i)Removed unused props from igt_display_init
> ii)Removed unused ret. Changed function to void
> iii)Declare the variable before checking if we have
> DRM_CLIENT_CAP_ATOMIC.
> https://patchwork.freedesktop.org/patch/73505/
>
> v6: (Jani) Corrected typo in commit message
> https://patchwork.freedesktop.org/patch/74468/
>
> v7: (Vlad & Maarten) Added is_atomic check for DRM_CLIENT_CAP_ATOMIC
> in igt_display_init and igt_atomic_commit
>
> Signed-off-by: Mayuresh Gharpure <mayuresh.s.gharpure@intel.com>
> Signed-off-by: Pratik Vishwakarma <pratik.vishwakarma@intel.com>
> ---
> lib/igt_kms.c | 321 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/igt_kms.h | 72 ++++++++++++-
> 2 files changed, 391 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 5a61def..1f738e1 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -145,6 +145,120 @@ const unsigned char* igt_kms_get_base_edid(void)
> *
> * Returns: an alternate edid block
> */
> +static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> + "SRC_X",
> + "SRC_Y",
> + "SRC_W",
> + "SRC_H",
> + "CRTC_X",
> + "CRTC_Y",
> + "CRTC_W",
> + "CRTC_H",
> + "FB_ID",
> + "CRTC_ID",
> + "type",
> + "rotation"
> +};
> +
> +static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> + "background_color"
> +};
> +
> +static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
> + "scaling mode",
> + "DPMS"
> +};
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * plane->atomic_props_plane.
> + */
> +static void
> +igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
> + int num_props, const char **prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, DRM_MODE_OBJECT_PLANE);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_props; j++) {
> + if (strcmp(prop->name, prop_names[j]) != 0)
> + continue;
> +
> + plane->atomic_props_plane[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +}
> +
> +/*
> + * Retrieve all the properies specified in props_name and store them into
> + * config->atomic_props_crtc and config->atomic_props_connector.
> + */
> +static void
> +igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
> + int num_crtc_props, const char **crtc_prop_names,
> + int num_connector_props, const char **conn_prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_crtc_props; j++) {
> + if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_crtc[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> + props = NULL;
> + props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
> + igt_assert(props);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop =
> + drmModeGetProperty(fd, props->props[i]);
> +
> + for (j = 0; j < num_connector_props; j++) {
> + if (strcmp(prop->name, conn_prop_names[j]) != 0)
> + continue;
> +
> + output->config.atomic_props_connector[j] = props->props[i];
> + break;
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> +
> +}
> +
> const unsigned char* igt_kms_get_alt_edid(void)
> {
> update_edid_csum(alt_edid);
> @@ -1002,6 +1116,8 @@ static void igt_output_refresh(igt_output_t *output)
> kmstest_pipe_name(output->config.pipe));
>
> display->pipes_in_use |= 1 << output->config.pipe;
> + igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
> + IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
> }
>
> static bool
> @@ -1071,6 +1187,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> drmModeRes *resources;
> drmModePlaneRes *plane_resources;
> int i;
> + int ret = 0;
>
> memset(display, 0, sizeof(igt_display_t));
>
> @@ -1088,6 +1205,7 @@ void igt_display_init(igt_display_t *display, int drm_fd)
> display->n_pipes = resources->count_crtcs;
>
> drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> + ret = drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
> plane_resources = drmModeGetPlaneResources(display->drm_fd);
> igt_assert(plane_resources);
>
> @@ -1144,6 +1262,10 @@ void igt_display_init(igt_display_t *display, int drm_fd)
>
> plane->pipe = pipe;
> plane->drm_plane = drm_plane;
> + if (ret == 0) {
> + display->is_atomic = true;
> + igt_atomic_fill_plane_props(display, plane, IGT_NUM_PLANE_PROPS, igt_plane_prop_names);
> + }
>
> get_plane_property(display->drm_fd, drm_plane->plane_id,
> "rotation",
> @@ -1399,6 +1521,111 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> igt_assert(r == 0); \
> }
>
> +
> +
> +
> +/*
> + * Add position and fb changes of a plane to the atomic property set
> + */
> +static void
> +igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
> + drmModeAtomicReq *req)
> +{
> +
> + igt_display_t *display = output->display;
> + uint32_t fb_id, crtc_id;
> + uint32_t src_x;
> + uint32_t src_y;
> + uint32_t src_w;
> + uint32_t src_h;
> + int32_t crtc_x;
> + int32_t crtc_y;
> + uint32_t crtc_w;
> + uint32_t crtc_h;
> +
> + igt_assert(plane->drm_plane);
> +
> + /* it's an error to try an unsupported feature */
> + igt_assert(igt_plane_supports_rotation(plane) ||
> + !plane->rotation_changed);
> +
> + fb_id = igt_plane_get_fb_id(plane);
> + crtc_id = output->config.crtc->crtc_id;
> +
> + if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> +
> + LOG(display,
> + "%s: populating plane data: pipe %s, plane %d, disabling\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index);
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
> +
> + } else if (plane->fb_changed || plane->position_changed ||
> + plane->size_changed) {
> +
> + src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
> + src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
> + src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
> + src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
> + crtc_x = plane->crtc_x;
> + crtc_y = plane->crtc_y;
> + crtc_w = plane->crtc_w;
> + crtc_h = plane->crtc_h;
> +
> + LOG(display,
> + "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
> + "%ux%u dst = (%u, %u) %ux%u\n",
> + igt_output_name(output),
> + kmstest_pipe_name(output->config.pipe),
> + plane->index,
> + fb_id,
> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> + crtc_x, crtc_y, crtc_w, crtc_h);
> +
> +
> + /* populate plane req */
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, src_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, src_h);
> +
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, crtc_x);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
> +
> + if (plane->rotation_changed)
> + igt_atomic_populate_plane_req(req, plane,
> + IGT_PLANE_ROTATION, plane->rotation);
> +
> + }
> +
> + plane->fb_changed = false;
> + plane->position_changed = false;
> + plane->size_changed = false;
> +
> + return;
> +}
> +
> +
> +
> /*
> * Commit position and fb changes to a DRM plane via the SetPlane ioctl; if the
> * DRM call to program the plane fails, we'll either fail immediately (for
> @@ -1651,7 +1878,7 @@ static int igt_plane_commit(igt_plane_t *plane,
> return igt_primary_plane_commit_legacy(plane, output,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, output, fail_on_error);
> + return igt_drm_plane_commit(plane, output, fail_on_error);
> }
> }
>
> @@ -1705,6 +1932,90 @@ static int igt_output_commit(igt_output_t *output,
> return 0;
> }
>
> +
> +/*
> + * Add crtc property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
> +
> + if (pipe_obj->background_changed)
> + igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
> +
> + /*
> + * TODO: Add all crtc level properties here
> + */
> +
> +}
> +
> +/*
> + * Add connector property changes to the atomic property set
> + */
> +static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAtomicReq *req)
> +{
> +
> + struct kmstest_connector_config *config = &output->config;
> +
> + if (config->connector_scaling_mode_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_SCALING_MODE, config->connector_scaling_mode);
> +
> + if (config->connector_dpms_changed)
> + igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_DPMS, config->connector_dpms);
> + /*
> + * TODO: Add all other connector level properties here
> + */
> +
> +}
> +
> +/*
> + * Commit all the changes of all the planes,crtcs, connectors
> + * atomically using drmModeAtomicCommit()
> + */
> +static int igt_atomic_commit(igt_display_t *display)
> +{
> +
> + int ret = 0;
> + int i = 0;
> + drmModeAtomicReq *req;
> + if (display->is_atomic != true)
> + return -EINVAL;
> + req = drmModeAtomicAlloc();
> + drmModeAtomicSetCursor(req, 0);
> +
> + for (i = 0; i < display->n_outputs; i++) {
> + igt_output_t *output = &display->outputs[i];
> + igt_pipe_t *pipe;
> +
> + if (!output->valid)
> + continue;
> +
> + /*
> + * Add CRTC Properties to the property set
> + */
> + igt_atomic_prepare_crtc_commit(output, req);
> +
> + /*
> + * Add Connector Properties to the property set
> + */
> + igt_atomic_prepare_connector_commit(output, req);
> +
> +
> + pipe = igt_output_get_driving_pipe(output);
> +
> + for (i = 0; i < pipe->n_planes; i++) {
> + igt_plane_t *plane = &pipe->planes[i];
> + igt_atomic_prepare_plane_commit(plane, output, req);
> + }
> +
> + }
> +
> + ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> + drmModeAtomicFree(req);
> + return ret;
> +
> +}
> /*
> * Commit all plane changes across all outputs of the display.
> *
> @@ -1724,6 +2035,14 @@ static int do_display_commit(igt_display_t *display,
>
> igt_display_refresh(display);
>
> + if (s == COMMIT_ATOMIC) {
> +
> + ret = igt_atomic_commit(display);
> + CHECK_RETURN(ret, fail_on_error);
> + return 0;
> +
> + }
> +
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *output = &display->outputs[i];
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 5744ed0..d6e2f12 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -106,11 +106,28 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id);
> void kmstest_set_vt_graphics_mode(void);
> void kmstest_restore_vt_mode(void);
>
> +enum igt_atomic_crtc_properties {
> + IGT_CRTC_BACKGROUND = 0,
> + IGT_NUM_CRTC_PROPS
> +};
> +
> +enum igt_atomic_connector_properties {
> + IGT_CONNECTOR_SCALING_MODE = 0,
> + IGT_CONNECTOR_DPMS,
> + IGT_NUM_CONNECTOR_PROPS
> +};
> +
> struct kmstest_connector_config {
> drmModeCrtc *crtc;
> drmModeConnector *connector;
> drmModeEncoder *encoder;
> drmModeModeInfo default_mode;
> + uint64_t connector_scaling_mode;
> + bool connector_scaling_mode_changed;
> + uint64_t connector_dpms;
> + bool connector_dpms_changed;
> + uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> + uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
> int crtc_idx;
> int pipe;
> };
> @@ -163,9 +180,28 @@ uint32_t kmstest_find_crtc_for_connector(int fd, drmModeRes *res,
> enum igt_commit_style {
> COMMIT_LEGACY = 0,
> COMMIT_UNIVERSAL,
> - /* We'll add atomic here eventually. */
> + COMMIT_ATOMIC,
> };
>
> +enum igt_atomic_plane_properties {
> + IGT_PLANE_SRC_X = 0,
> + IGT_PLANE_SRC_Y,
> + IGT_PLANE_SRC_W,
> + IGT_PLANE_SRC_H,
> +
> + IGT_PLANE_CRTC_X,
> + IGT_PLANE_CRTC_Y,
> + IGT_PLANE_CRTC_W,
> + IGT_PLANE_CRTC_H,
> +
> + IGT_PLANE_FB_ID,
> + IGT_PLANE_CRTC_ID,
> + IGT_PLANE_TYPE,
> + IGT_PLANE_ROTATION,
> + IGT_NUM_PLANE_PROPS
> +};
> +
> +
> typedef struct igt_display igt_display_t;
> typedef struct igt_pipe igt_pipe_t;
> typedef uint32_t igt_fixed_t; /* 16.16 fixed point */
> @@ -207,6 +243,7 @@ typedef struct {
> /* panning offset within the fb */
> unsigned int pan_x, pan_y;
> igt_rotation_t rotation;
> + uint32_t atomic_props_plane[IGT_NUM_PLANE_PROPS];
> } igt_plane_t;
>
> struct igt_pipe {
> @@ -242,6 +279,7 @@ struct igt_display {
> igt_output_t *outputs;
> igt_pipe_t pipes[I915_MAX_PIPES];
> bool has_universal_planes;
> + bool is_atomic;
> };
>
> void igt_display_init(igt_display_t *display, int drm_fd);
> @@ -288,6 +326,38 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> #define IGT_FIXED(i,f) ((i) << 16 | (f))
>
> +/**
> + * igt_atomic_populate_plane_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @plane: A pointer igt_plane_t
> + * @prop: one of igt_atomic_plane_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_plane_req(req, plane, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, plane->drm_plane->plane_id,\
> + plane->atomic_props_plane[prop], value))
> +
> +/**
> + * igt_atomic_populate_crtc_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_crtc_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_crtc_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.crtc->crtc_id,\
> + output->config.atomic_props_crtc[prop], value))
> +/**
> + * igt_atomic_populate_connector_req:
> + * @req: A pointer to drmModeAtomicReq
> + * @output: A pointer igt_output_t
> + * @prop: one of igt_atomic_connector_properties
> + * @value: the value to add
> + */
> +#define igt_atomic_populate_connector_req(req, output, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, output->config.connector->connector_id,\
> + output->config.atomic_props_connector[prop], value))
> +
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-08 16:11 ` Lionel Landwerlin
@ 2016-03-09 14:25 ` Mayuresh Gharpure
2016-03-09 16:14 ` Lionel Landwerlin
0 siblings, 1 reply; 8+ messages in thread
From: Mayuresh Gharpure @ 2016-03-09 14:25 UTC (permalink / raw)
To: Lionel Landwerlin, Maarten Lankhorst, Pratik Vishwakarma,
intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6462 bytes --]
Hi Lionel,
I've incorporated your comments using the diff patch submitted by
Maarten and submitted the changes
https://patchwork.freedesktop.org/series/4274/
Also, please find my comments inline
Regards,
Mayuresh
On 3/8/2016 9:41 PM, Lionel Landwerlin wrote:
> Hi Maarten,
>
> Yeah that would be much simpler I think.
>
> Damien also looked at this patch over my shoulder and had additional
> comments.
> So more or less proxying :
>
> - Not sure why we're exposing the new enums and the
> igt_atomic_popuplate_* macros in igt_kms.h.
> After all we're not using them anywhere outside igt_kms.c.
Macros are just to improve readability. In case some new feature needs
to be added, we can just add an enum and use the macro to populate it in
call to IOCTL.
These were added in initial implementation by Marius.
>
> - We have a couple of properties ids stored in igt_kms.h
> (background_property & rotation_property), it would make sense to get
> rid of those and use the new created arrays instead.
These are used by the COMMIT_LEGACY and COMMIT_UNIVERSAL style calls. So
keeping it as is, as changing this may need a change in many existing tests
>
> Cheers,
>
> -
> Lionel
>
> On 08/03/16 15:56, Maarten Lankhorst wrote:
>> Op 07-03-16 om 13:50 schreef Lionel Landwerlin:
>>> Hi Pratik,
>>>
>>> I'm really looking forward to get this merged.
>>> Just a few comments on the plane commit part below.
>> Would the following diff to the patch satisfy you? It would clean up things a lot.
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 1f738e14f52f..454ff7552b4a 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1534,14 +1534,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
>>
>> igt_display_t *display = output->display;
>> uint32_t fb_id, crtc_id;
>> - uint32_t src_x;
>> - uint32_t src_y;
>> - uint32_t src_w;
>> - uint32_t src_h;
>> - int32_t crtc_x;
>> - int32_t crtc_y;
>> - uint32_t crtc_w;
>> - uint32_t crtc_h;
>>
>> igt_assert(plane->drm_plane);
>>
>> @@ -1552,54 +1544,30 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
>> fb_id = igt_plane_get_fb_id(plane);
>> crtc_id = output->config.crtc->crtc_id;
>>
>> - if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
>> -
>> - LOG(display,
>> - "%s: populating plane data: pipe %s, plane %d, disabling\n",
>> - igt_output_name(output),
>> - kmstest_pipe_name(output->config.pipe),
>> - plane->index);
>> -
>> - /* populate plane req */
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 0);
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, 0);
>> -
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
>> -
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
>> -
>> - } else if (plane->fb_changed || plane->position_changed ||
>> - plane->size_changed) {
>> -
>> - src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
>> - src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
>> - src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
>> - src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
>> - crtc_x = plane->crtc_x;
>> - crtc_y = plane->crtc_y;
>> - crtc_w = plane->crtc_w;
>> - crtc_h = plane->crtc_h;
>> -
>> - LOG(display,
>> - "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
>> - "%ux%u dst = (%u, %u) %ux%u\n",
>> - igt_output_name(output),
>> - kmstest_pipe_name(output->config.pipe),
>> - plane->index,
>> - fb_id,
>> - src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
>> - crtc_x, crtc_y, crtc_w, crtc_h);
>> -
>> -
>> - /* populate plane req */
>> - igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, crtc_id);
>> + LOG(display,
>> + "%s: populating plane data: %s.%d, fb %u, src = (%d, %d) "
>> + "%ux%u dst = (%u, %u) %ux%u\n",
>> + igt_output_name(output),
>> + kmstest_pipe_name(output->config.pipe),
>> + plane->index,
>> + fb_id,
>> + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
>> + crtc_x, crtc_y, crtc_w, crtc_h);
>> +
>> + if (plane->fb_changed) {
>> + igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id);
>> + }
>> +
>> + if (plane->position_changed || plane->size_changed) {
>> + uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */
>> + uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */
>> + uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
>> + uint32_t src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h */
>> + int32_t crtc_x = plane->crtc_x;
>> + int32_t crtc_y = plane->crtc_y;
>> + uint32_t crtc_w = plane->crtc_w;
>> + uint32_t crtc_h = plane->crtc_h;
>>
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, src_x);
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, src_y);
>> @@ -1610,18 +1578,15 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, crtc_y);
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, crtc_w);
>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, crtc_h);
>> -
>> - if (plane->rotation_changed)
>> - igt_atomic_populate_plane_req(req, plane,
>> - IGT_PLANE_ROTATION, plane->rotation);
>> -
>> }
>>
>> + if (plane->rotation_changed)
>> + igt_atomic_populate_plane_req(req, plane,
>> + IGT_PLANE_ROTATION, plane->rotation);
>> +
>> plane->fb_changed = false;
>> plane->position_changed = false;
>> plane->size_changed = false;
>
> I missed that, but I think we need rotation_changed = false; here too.
>
>> -
>> - return;
>> }
>>
>>
>>
>
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[-- Attachment #1.2: Type: text/html, Size: 8292 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2()
2016-03-09 14:25 ` Mayuresh Gharpure
@ 2016-03-09 16:14 ` Lionel Landwerlin
0 siblings, 0 replies; 8+ messages in thread
From: Lionel Landwerlin @ 2016-03-09 16:14 UTC (permalink / raw)
To: Mayuresh Gharpure, Maarten Lankhorst, Pratik Vishwakarma,
intel-gfx
Thanks,
It seems the igt_atomic_prepare_plane_commit() function still needs to
reset the plane->rotation_changed field (it was at the bottom of my
previous email, sorry for that).
The way supported property ids are listed from the driver isn't
different in the atomic and non atomic cases.
The igt_atomic_fill_props and igt_atomic_fill_plane_props functions are
not atomic specific.
What I was suggesting is to have a unified way of listing the
properties regardless of whether we're committing things atomically or
not and just have the commit function do a special case for atomic.
That way it becomes easier to add new properties.
-
Lionel
On Wed, 2016-03-09 at 19:55 +0530, Mayuresh Gharpure wrote:
> Hi Lionel,
>
> I've incorporated your comments using the diff patch submitted by
> Maarten and submitted the changes
>
> https://patchwork.freedesktop.org/series/4274/
>
> Also, please find my comments inline
>
> Regards,
> Mayuresh
>
> On 3/8/2016 9:41 PM, Lionel Landwerlin wrote:
> > Hi Maarten,
> >
> > Yeah that would be much simpler I think.
> >
> > Damien also looked at this patch over my shoulder and had
> > additional comments.
> > So more or less proxying :
> >
> > - Not sure why we're exposing the new enums and the
> > igt_atomic_popuplate_* macros in igt_kms.h.
> > After all we're not using them anywhere outside igt_kms.c.
> Macros are just to improve readability. In case some new feature
> needs to be added, we can just add an enum and use the macro to
> populate it in call to IOCTL.
> These were added in initial implementation by Marius.
>
> >
> > - We have a couple of properties ids stored in igt_kms.h
> > (background_property & rotation_property), it would make sense to
> > get rid of those and use the new created arrays instead.
> These are used by the COMMIT_LEGACY and COMMIT_UNIVERSAL style
> calls. So keeping it as is, as changing this may need a change in
> many existing tests
> >
> > Cheers,
> >
> > -
> > Lionel
> >
> > On 08/03/16 15:56, Maarten Lankhorst wrote:
> > > Op 07-03-16 om 13:50 schreef Lionel Landwerlin:
> > > > Hi Pratik,
> > > >
> > > > I'm really looking forward to get this merged.
> > > > Just a few comments on the plane commit part below.
> > > Would the following diff to the patch satisfy you? It would clean
> > > up things a lot.
> > >
> > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > index 1f738e14f52f..454ff7552b4a 100644
> > > --- a/lib/igt_kms.c
> > > +++ b/lib/igt_kms.c
> > > @@ -1534,14 +1534,6 @@
> > > igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t
> > > *output,
> > >
> > > igt_display_t *display = output->display;
> > > uint32_t fb_id, crtc_id;
> > > - uint32_t src_x;
> > > - uint32_t src_y;
> > > - uint32_t src_w;
> > > - uint32_t src_h;
> > > - int32_t crtc_x;
> > > - int32_t crtc_y;
> > > - uint32_t crtc_w;
> > > - uint32_t crtc_h;
> > >
> > > igt_assert(plane->drm_plane);
> > >
> > > @@ -1552,54 +1544,30 @@
> > > igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t
> > > *output,
> > > fb_id = igt_plane_get_fb_id(plane);
> > > crtc_id = output->config.crtc->crtc_id;
> > >
> > > - if ((plane->fb_changed || plane->size_changed) && fb_id
> > > == 0) {
> > > -
> > > - LOG(display,
> > > - "%s: populating plane data: pipe %s, plane
> > > %d, disabling\n",
> > > - igt_output_name(output),
> > > - kmstest_pipe_name(output->config.pipe),
> > > - plane->index);
> > > -
> > > - /* populate plane req */
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_ID, 0);
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_FB_ID, 0);
> > > -
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_X, IGT_FIXED(0, 0));
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_Y, IGT_FIXED(0, 0));
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_W, IGT_FIXED(0, 0));
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_H, IGT_FIXED(0, 0));
> > > -
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_X, 0);
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_Y, 0);
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_W, 0);
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_H, 0);
> > > -
> > > - } else if (plane->fb_changed || plane->position_changed
> > > ||
> > > - plane->size_changed) {
> > > -
> > > - src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x
> > > */
> > > - src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y
> > > */
> > > - src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w
> > > */
> > > - src_h = IGT_FIXED(plane->fb->src_h, 0); /* src_h
> > > */
> > > - crtc_x = plane->crtc_x;
> > > - crtc_y = plane->crtc_y;
> > > - crtc_w = plane->crtc_w;
> > > - crtc_h = plane->crtc_h;
> > > -
> > > - LOG(display,
> > > - "%s: populating plane data: %s.%d, fb %u,
> > > src = (%d, %d) "
> > > - "%ux%u dst = (%u, %u) %ux%u\n",
> > > - igt_output_name(output),
> > > - kmstest_pipe_name(output->config.pipe),
> > > - plane->index,
> > > - fb_id,
> > > - src_x >> 16, src_y >> 16, src_w >> 16, src_h
> > > >> 16,
> > > - crtc_x, crtc_y, crtc_w, crtc_h);
> > > -
> > > -
> > > - /* populate plane req */
> > > - igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_ID, crtc_id);
> > > + LOG(display,
> > > + "%s: populating plane data: %s.%d, fb %u, src = (%d,
> > > %d) "
> > > + "%ux%u dst = (%u, %u) %ux%u\n",
> > > + igt_output_name(output),
> > > + kmstest_pipe_name(output->config.pipe),
> > > + plane->index,
> > > + fb_id,
> > > + src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> > > + crtc_x, crtc_y, crtc_w, crtc_h);
> > > +
> > > + if (plane->fb_changed) {
> > > + igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_ID, fb_id ? crtc_id : 0);
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_FB_ID, fb_id);
> > > + }
> > > +
> > > + if (plane->position_changed || plane->size_changed) {
> > > + uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0);
> > > /* src_x */
> > > + uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0);
> > > /* src_y */
> > > + uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0);
> > > /* src_w */
> > > + uint32_t src_h = IGT_FIXED(plane->fb->src_h, 0);
> > > /* src_h */
> > > + int32_t crtc_x = plane->crtc_x;
> > > + int32_t crtc_y = plane->crtc_y;
> > > + uint32_t crtc_w = plane->crtc_w;
> > > + uint32_t crtc_h = plane->crtc_h;
> > >
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_X, src_x);
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_SRC_Y, src_y);
> > > @@ -1610,18 +1578,15 @@
> > > igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t
> > > *output,
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_Y, crtc_y);
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_W, crtc_w);
> > > igt_atomic_populate_plane_req(req, plane,
> > > IGT_PLANE_CRTC_H, crtc_h);
> > > -
> > > - if (plane->rotation_changed)
> > > - igt_atomic_populate_plane_req(req,
> > > plane,
> > > - IGT_PLANE_ROTATION, plane-
> > > >rotation);
> > > -
> > > }
> > >
> > > + if (plane->rotation_changed)
> > > + igt_atomic_populate_plane_req(req, plane,
> > > + IGT_PLANE_ROTATION, plane->rotation);
> > > +
> > > plane->fb_changed = false;
> > > plane->position_changed = false;
> > > plane->size_changed = false;
> >
> > I missed that, but I think we need rotation_changed = false; here
> > too.
> >
> > > -
> > > - return;
> > > }
> > >
> > >
> > >
> >
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-09 16:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-07 10:18 [PATCH i-g-t v7] lib/igt_kms: Add COMMIT_ATOMIC to igt_display_commit2() Pratik Vishwakarma
2016-03-07 12:50 ` Lionel Landwerlin
2016-03-08 15:56 ` Maarten Lankhorst
2016-03-08 16:11 ` Lionel Landwerlin
2016-03-09 14:25 ` Mayuresh Gharpure
2016-03-09 16:14 ` Lionel Landwerlin
2016-03-07 12:50 ` Lionel Landwerlin
2016-03-08 16:58 ` Matthew Auld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox