* [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT.
@ 2016-07-06 9:55 Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx Maarten Lankhorst
` (14 more replies)
0 siblings, 15 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
With legacy modesets if the primary plane had a null fb it was disabled,
and code would often do the following:
for_each_pipe(...)
for_each_output(...) {
igt_output_set_pipe(output, pipe);
igt_display_commit();
if (!output->valid)
bail;
cleanup;
igt_output_set_pipe(output, PIPE_ANY);
igt_display_commit();
}
A straightforward replacement of igt_display_commit with
igt_display_commit2(COMMIT_ATOMIC) will not work with this
series, since in this case it can perform a modeset without
primary framebuffer, and invalid configurations get rejected.
Best solution is to do the following to get atomic behavior,
with fallback to legacy:
bool valid = false;
for_each_pipe_with_valid_output(display, pipe, output) {
valid = true;
igt_output_set_pipe(output, pipe);
/* perform more setup here for planes, etc */
igt_display_commit2(display, display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
/* tests + cleanup */
igt_output_set_pipe(output, PIPE_NONE);
}
igt_require_f(valid, "no valid crtc/connector combinations found\n");
If the test requires a fixed pipe, use for_each_valid_output_on_pipe instead.
XXX: Make a igt_require_valid_output() for test fixtures?
Unfortunately it made me learn that that setting connector properties with
the atomic api doesn't work yet, needs more tests and changes in the kernel.
But we'll get there, converting encoder api in the kernel to use the atomic
state is a first step.
Changes since first series:
- Silence compiler warning about unused variable 'encoder'.
- Update pan members patch to fix ordering.
- Clean up more tests that rely on outputs with unassigned pipes having
filled in a default pipe.
- Add kms_atomic_transition
- Add atomic test for fastset panel fitting changes, to ensure no modeset's done.
Removing the call to intel_update_pipe_config in the kernel caused fifo underruns
and hw state validator failures, so I think this test works as intended. :-)
Maarten Lankhorst (15):
igt_kms: Remove kmstest_connector_config.crtc_idx
igt_kms: Find optimal encoder only after selecting pipe
kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid
config.
igt_kms: Make PIPE_ANY a alias for PIPE_NONE
tests/kms: Clean up more users of unassigned pipes.
igt_kms: Change PIPE_ANY behavior to mean unassigned
igt_kms: Handle atomic pipe properties better.
igt_kms: Remove pan members from igt_plane, v2.
igt_kms: Clear all _changed members centrally
igt_kms: Add modeset support to atomic commits.
tests: Add kms_rmfb test.
tests: Add kms_atomic_transition
igt_kms: Add more apis for panel fitting test.
igt_kms: Allow disabling previous override mode
kms_panel_fitting: Add tests for fastboot.
lib/igt_kms.c | 698 +++++++++++++++++++++++---------------
lib/igt_kms.h | 51 ++-
tests/Makefile.sources | 2 +
tests/kms_atomic_transition.c | 189 +++++++++++
tests/kms_crtc_background_color.c | 3 +-
tests/kms_flip_tiling.c | 50 +--
tests/kms_panel_fitting.c | 108 +++++-
tests/kms_plane.c | 8 +-
tests/kms_plane_scaling.c | 7 +-
tests/kms_psr_sink_crc.c | 5 +-
tests/kms_rmfb.c | 171 ++++++++++
tests/testdisplay.c | 4 +-
12 files changed, 954 insertions(+), 342 deletions(-)
create mode 100644 tests/kms_atomic_transition.c
create mode 100644 tests/kms_rmfb.c
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-13 12:13 ` Daniel Vetter
2016-07-06 9:55 ` [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe Maarten Lankhorst
` (13 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This is the same as using config.pipe because the order of crtcs will
never change.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 4 +---
lib/igt_kms.h | 1 -
tests/testdisplay.c | 4 +---
3 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8f9ac2da43ff..c16e40ea273b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -854,9 +854,7 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
config->connector = connector;
config->encoder = found;
config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[pipe]);
- config->crtc_idx = pipe;
- config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
- config->crtc->crtc_id);
+ config->pipe = pipe;
drmModeFreeResources(resources);
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 829615d70874..4882075430c8 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -118,7 +118,6 @@ struct kmstest_connector_config {
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;
unsigned valid_crtc_idx_mask;
};
diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 45280e4cad82..a974f42be9f1 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -112,7 +112,6 @@ struct connector {
drmModeEncoder *encoder;
drmModeConnector *connector;
int crtc;
- int crtc_idx;
int pipe;
};
@@ -211,7 +210,6 @@ static void connector_find_preferred_mode(uint32_t connector_id,
c->connector = config.connector;
c->encoder = config.encoder;
c->crtc = config.crtc->crtc_id;
- c->crtc_idx = config.crtc_idx;
c->pipe = config.pipe;
if (mode_num != -1) {
@@ -497,7 +495,7 @@ int update_display(bool probe)
if (test_preferred_mode || force_mode ||
specified_mode_num != -1)
- crtc_idx_mask &= ~(1 << connector->crtc_idx);
+ crtc_idx_mask &= ~(1 << connector->pipe);
}
}
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-15 11:14 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config Maarten Lankhorst
` (12 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This will allow us to find a matching encoder based on a pipe only.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 97 +++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 61 insertions(+), 36 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c16e40ea273b..ce8aa2455348 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -761,6 +761,59 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
return true;
}
+static void
+_kmstest_connector_config_crtc_mask(int drm_fd,
+ drmModeConnector *connector,
+ struct kmstest_connector_config *config)
+{
+ int i;
+
+ config->valid_crtc_idx_mask = 0;
+
+ /* Now get a compatible encoder */
+ for (i = 0; i < connector->count_encoders; i++) {
+ drmModeEncoder *encoder = drmModeGetEncoder(drm_fd,
+ connector->encoders[i]);
+
+ if (!encoder) {
+ igt_warn("could not get encoder %d: %s\n",
+ connector->encoders[i],
+ strerror(errno));
+
+ continue;
+ }
+
+ config->valid_crtc_idx_mask |= encoder->possible_crtcs;
+ drmModeFreeEncoder(encoder);
+ }
+}
+
+static drmModeEncoder *
+_kmstest_connector_config_find_encoder(int drm_fd, drmModeConnector *connector, enum pipe pipe)
+{
+ int i;
+
+ for (i = 0; i < connector->count_encoders; i++) {
+ drmModeEncoder *encoder = drmModeGetEncoder(drm_fd, connector->encoders[i]);
+
+ if (!encoder) {
+ igt_warn("could not get encoder %d: %s\n",
+ connector->encoders[i],
+ strerror(errno));
+
+ continue;
+ }
+
+ if (encoder->possible_crtcs & (1 << pipe))
+ return encoder;
+
+ drmModeFreeEncoder(encoder);
+ }
+
+ igt_assert(false);
+ return NULL;
+}
+
/**
* _kmstest_connector_config:
* @drm_fd: DRM fd
@@ -779,8 +832,6 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
{
drmModeRes *resources;
drmModeConnector *connector;
- drmModeEncoder *encoder, *found = NULL;
- int i, j, pipe;
resources = drmModeGetResources(drm_fd);
if (!resources) {
@@ -816,51 +867,25 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
* In both cases find the first compatible encoder and skip the CRTC
* if there is non such.
*/
- config->valid_crtc_idx_mask = 0;
- for (i = 0; i < resources->count_crtcs; i++) {
- if (!resources->crtcs[i])
- continue;
+ _kmstest_connector_config_crtc_mask(drm_fd, connector, config);
- /* Now get a compatible encoder */
- for (j = 0; j < connector->count_encoders; j++) {
- encoder = drmModeGetEncoder(drm_fd,
- connector->encoders[j]);
-
- if (!encoder) {
- igt_warn("could not get encoder %d: %s\n",
- resources->encoders[j],
- strerror(errno));
-
- continue;
- }
-
- config->valid_crtc_idx_mask |= encoder->possible_crtcs;
-
- if (!found && (crtc_idx_mask & encoder->possible_crtcs & (1 << i))) {
- found = encoder;
- pipe = i;
- } else
- drmModeFreeEncoder(encoder);
- }
- }
-
- if (!found)
+ crtc_idx_mask &= config->valid_crtc_idx_mask;
+ if (!crtc_idx_mask)
goto err3;
+ config->pipe = ffs(crtc_idx_mask) - 1;
+
if (!kmstest_get_connector_default_mode(drm_fd, connector,
&config->default_mode))
- goto err4;
+ goto err3;
+ config->encoder = _kmstest_connector_config_find_encoder(drm_fd, connector, config->pipe);
config->connector = connector;
- config->encoder = found;
- config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[pipe]);
- config->pipe = pipe;
+ config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[config->pipe]);
drmModeFreeResources(resources);
return true;
-err4:
- drmModeFreeEncoder(found);
err3:
drmModeFreeConnector(connector);
err2:
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-15 11:15 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 04/15] igt_kms: Make PIPE_ANY a alias for PIPE_NONE Maarten Lankhorst
` (11 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This is the only time PIPE_ANY was used to mean something other than
unassign this output from a pipe. Without this PIPE_ANY can be aliased
to PIPE_NONE.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
tests/kms_psr_sink_crc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index b18e426303e3..d7bce3bb7855 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -103,15 +103,16 @@ static void setup_output(data_t *data)
{
igt_display_t *display = &data->display;
igt_output_t *output;
+ enum pipe pipe;
- for_each_connected_output(display, output) {
+ for_each_pipe_with_valid_output(display, pipe, output) {
drmModeConnectorPtr c = output->config.connector;
if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
c->connection != DRM_MODE_CONNECTED)
continue;
- igt_output_set_pipe(output, PIPE_ANY);
+ igt_output_set_pipe(output, pipe);
data->crtc_id = output->config.crtc->crtc_id;
data->output = output;
data->mode = igt_output_get_mode(output);
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 04/15] igt_kms: Make PIPE_ANY a alias for PIPE_NONE
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (2 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes Maarten Lankhorst
` (10 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4882075430c8..dc6be5e53b74 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -40,7 +40,8 @@
/* Low-level helpers with kmstest_ prefix */
enum pipe {
- PIPE_ANY = -1,
+ PIPE_NONE = -1,
+ PIPE_ANY = PIPE_NONE,
PIPE_A = 0,
PIPE_B,
PIPE_C,
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (3 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 04/15] igt_kms: Make PIPE_ANY a alias for PIPE_NONE Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-20 12:56 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned Maarten Lankhorst
` (9 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
Use for_each_pipe_with_valid_output instead.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
tests/kms_crtc_background_color.c | 3 +--
tests/kms_flip_tiling.c | 50 +++++++++++++++++++++++----------------
tests/kms_panel_fitting.c | 5 ++--
tests/kms_plane_scaling.c | 5 ++--
4 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index b496625c1693..b97c1142df6e 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -133,10 +133,9 @@ static void test_crtc_background(data_t *data)
igt_require(data->display.has_universal_planes);
- for_each_connected_output(display, output) {
+ for_each_pipe_with_valid_output(display, pipe, output) {
igt_plane_t *plane;
- pipe = output->config.pipe;
igt_output_set_pipe(output, pipe);
plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index f58e65be61ee..cd2f510f1d45 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -80,16 +80,15 @@ static void wait_for_pageflip(int fd)
}
static void
-test_flip_tiling(data_t *data, igt_output_t *output, uint64_t tiling[2])
+test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t tiling[2])
{
drmModeModeInfo *mode;
igt_plane_t *primary;
struct igt_fb fb[2];
igt_pipe_crc_t *pipe_crc;
igt_crc_t reference_crc, crc;
- int fb_id, pipe, ret, width;
+ int fb_id, ret, width;
- pipe = output->config.pipe;
pipe_crc = pipe_crc_new(pipe);
igt_output_set_pipe(output, pipe);
@@ -184,31 +183,34 @@ igt_main
igt_subtest_f("flip-changes-tiling") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
LOCAL_DRM_FORMAT_MOD_NONE };
+ enum pipe pipe;
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-changes-tiling-Y") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
LOCAL_DRM_FORMAT_MOD_NONE };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-changes-tiling-Yf") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
LOCAL_DRM_FORMAT_MOD_NONE };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
/*
@@ -222,31 +224,34 @@ igt_main
igt_subtest_f("flip-X-tiled") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
LOCAL_I915_FORMAT_MOD_X_TILED };
+ enum pipe pipe;
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-Y-tiled") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
LOCAL_I915_FORMAT_MOD_Y_TILED };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-Yf-tiled") {
uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
LOCAL_I915_FORMAT_MOD_Yf_TILED };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
/*
@@ -260,31 +265,34 @@ igt_main
igt_subtest_f("flip-to-X-tiled") {
uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
LOCAL_I915_FORMAT_MOD_X_TILED };
+ enum pipe pipe;
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-to-Y-tiled") {
uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
LOCAL_I915_FORMAT_MOD_Y_TILED };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_subtest_f("flip-to-Yf-tiled") {
uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
LOCAL_I915_FORMAT_MOD_Yf_TILED };
+ enum pipe pipe;
igt_require_fb_modifiers(data.drm_fd);
igt_require(data.gen >= 9);
- for_each_connected_output(&data.display, output)
- test_flip_tiling(&data, output, tiling);
+ for_each_pipe_with_valid_output(&data.display, pipe, output)
+ test_flip_tiling(&data, pipe, output, tiling);
}
igt_fixture {
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index b796c6812679..7c501fcdd3fb 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -87,7 +87,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
if (s == COMMIT_LEGACY) {
int ret;
ret = drmModeSetCrtc(data->drm_fd,
- output->config.crtc->crtc_id,
+ plane->pipe->crtc_id,
data->fb_id1,
plane->pan_x, plane->pan_y,
&output->id,
@@ -137,7 +137,7 @@ static void test_panel_fitting(data_t *d)
enum pipe pipe;
int valid_tests = 0;
- for_each_connected_output(display, output) {
+ for_each_pipe_with_valid_output(display, pipe, output) {
drmModeModeInfo *mode, native_mode;
bool scaling_mode_set;
@@ -153,7 +153,6 @@ static void test_panel_fitting(data_t *d)
if (!scaling_mode_set)
continue;
- pipe = output->config.pipe;
igt_output_set_pipe(output, pipe);
mode = igt_output_get_mode(output);
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index ad5404d90bfa..39bb5e113411 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -96,7 +96,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
if (s == COMMIT_LEGACY) {
int ret;
ret = drmModeSetCrtc(data->drm_fd,
- output->config.crtc->crtc_id,
+ plane->pipe->crtc_id,
data->fb_id1,
plane->pan_x, plane->pan_y,
&output->id,
@@ -186,10 +186,9 @@ static void test_plane_scaling(data_t *d)
igt_require(d->display.has_universal_planes);
igt_require(d->num_scalers);
- for_each_connected_output(display, output) {
+ for_each_pipe_with_valid_output(display, pipe, output) {
drmModeModeInfo *mode;
- pipe = output->config.pipe;
igt_output_set_pipe(output, pipe);
mode = igt_output_get_mode(output);
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (4 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-21 9:23 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better Maarten Lankhorst
` (8 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
None of the tests requires that a output bound to PIPE_ANY is assigned,
so don't do it. Fix the display commit to iterate over crtc's instead
of outputs to properly disable pipes without outputs.
This also means that output->valid is only set after connecting a
output to a pipe, so no longer depend on it in for_each_connected_output
and similar macros.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 246 ++++++++++++++++++++++++++++------------------------------
lib/igt_kms.h | 18 ++++-
2 files changed, 135 insertions(+), 129 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index ce8aa2455348..88cae7d51787 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -869,18 +869,20 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
*/
_kmstest_connector_config_crtc_mask(drm_fd, connector, config);
+ if (!kmstest_get_connector_default_mode(drm_fd, connector,
+ &config->default_mode))
+ goto err3;
+
+ config->connector = connector;
+
crtc_idx_mask &= config->valid_crtc_idx_mask;
if (!crtc_idx_mask)
- goto err3;
+ /* Keep config->connector */
+ goto err2;
config->pipe = ffs(crtc_idx_mask) - 1;
- if (!kmstest_get_connector_default_mode(drm_fd, connector,
- &config->default_mode))
- goto err3;
-
config->encoder = _kmstest_connector_config_find_encoder(drm_fd, connector, config->pipe);
- config->connector = connector;
config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[config->pipe]);
drmModeFreeResources(resources);
@@ -940,8 +942,13 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t connector_id,
void kmstest_free_connector_config(struct kmstest_connector_config *config)
{
drmModeFreeCrtc(config->crtc);
+ config->crtc = NULL;
+
drmModeFreeEncoder(config->encoder);
+ config->encoder = NULL;
+
drmModeFreeConnector(config->connector);
+ config->connector = NULL;
}
/**
@@ -1197,8 +1204,7 @@ static void igt_output_refresh(igt_output_t *output)
/* we mask out the pipes already in use */
crtc_idx_mask = output->pending_crtc_idx_mask & ~display->pipes_in_use;
- if (output->valid)
- kmstest_free_connector_config(&output->config);
+ kmstest_free_connector_config(&output->config);
ret = kmstest_get_connector_config(display->drm_fd,
output->id,
@@ -1209,19 +1215,19 @@ static void igt_output_refresh(igt_output_t *output)
else
output->valid = false;
- if (!output->valid)
- return;
-
- if (output->use_override_mode)
- output->config.default_mode = output->override_mode;
-
- if (!output->name) {
+ if (!output->name && output->config.connector) {
drmModeConnector *c = output->config.connector;
igt_assert_neq(asprintf(&output->name, "%s-%d", kmstest_connector_type_str(c->connector_type), c->connector_type_id),
-1);
}
+ if (!output->valid)
+ return;
+
+ if (output->use_override_mode)
+ output->config.default_mode = output->override_mode;
+
LOG(display, "%s: Selecting pipe %s\n", output->name,
kmstest_pipe_name(output->config.pipe));
@@ -1259,10 +1265,10 @@ get_crtc_property(int drm_fd, uint32_t crtc_id, const char *name,
}
static void
-igt_crtc_set_property(igt_output_t *output, uint32_t prop_id, uint64_t value)
+igt_crtc_set_property(igt_pipe_t *pipe, uint32_t prop_id, uint64_t value)
{
- drmModeObjectSetProperty(output->display->drm_fd,
- output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id, value);
+ drmModeObjectSetProperty(pipe->display->drm_fd,
+ pipe->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id, value);
}
/*
@@ -1325,15 +1331,37 @@ void igt_display_init(igt_display_t *display, int drm_fd)
int p = IGT_PLANE_2;
int j, type;
uint8_t n_planes = 0;
+ uint64_t prop_value;
pipe->crtc_id = resources->crtcs[i];
pipe->display = display;
pipe->pipe = i;
+ get_crtc_property(display->drm_fd, pipe->crtc_id,
+ "background_color",
+ &pipe->background_property,
+ &prop_value,
+ NULL);
+ pipe->background = (uint32_t)prop_value;
+ get_crtc_property(display->drm_fd, pipe->crtc_id,
+ "DEGAMMA_LUT",
+ &pipe->degamma_property,
+ NULL,
+ NULL);
+ get_crtc_property(display->drm_fd, pipe->crtc_id,
+ "CTM",
+ &pipe->ctm_property,
+ NULL,
+ NULL);
+ get_crtc_property(display->drm_fd, pipe->crtc_id,
+ "GAMMA_LUT",
+ &pipe->gamma_property,
+ NULL,
+ NULL);
+
/* add the planes that can be used with that pipe */
for (j = 0; j < plane_resources->count_planes; j++) {
drmModePlane *drm_plane;
- uint64_t prop_value;
drm_plane = drmModeGetPlane(display->drm_fd,
plane_resources->planes[j]);
@@ -1440,47 +1468,17 @@ void igt_display_init(igt_display_t *display, int drm_fd)
igt_assert(display->outputs);
for (i = 0; i < display->n_outputs; i++) {
- int j;
igt_output_t *output = &display->outputs[i];
/*
- * We're free to select any pipe to drive that output until
- * a constraint is set with igt_output_set_pipe().
+ * We don't assign each output a pipe unless
+ * a pipe is set with igt_output_set_pipe().
*/
- output->pending_crtc_idx_mask = -1UL;
+ output->pending_crtc_idx_mask = 0;
output->id = resources->connectors[i];
output->display = display;
igt_output_refresh(output);
-
- for (j = 0; j < display->n_pipes; j++) {
- uint64_t prop_value;
- igt_pipe_t *pipe = &display->pipes[j];
-
- if (output->config.crtc) {
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "background_color",
- &pipe->background_property,
- &prop_value,
- NULL);
- pipe->background = (uint32_t)prop_value;
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "DEGAMMA_LUT",
- &pipe->degamma_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "CTM",
- &pipe->ctm_property,
- NULL,
- NULL);
- get_crtc_property(display->drm_fd, output->config.crtc->crtc_id,
- "GAMMA_LUT",
- &pipe->gamma_property,
- NULL,
- NULL);
- }
- }
}
drmModeFreePlaneResources(plane_resources);
@@ -1545,7 +1543,7 @@ static void igt_display_refresh(igt_display_t *display)
for (i = 0; i < display->n_outputs; i++) {
igt_output_t *a = &display->outputs[i];
- if (a->pending_crtc_idx_mask == -1UL)
+ if (!a->pending_crtc_idx_mask)
continue;
for (j = 0; j < display->n_outputs; j++) {
@@ -1554,9 +1552,6 @@ static void igt_display_refresh(igt_display_t *display)
if (i == j)
continue;
- if (b->pending_crtc_idx_mask == -1UL)
- continue;
-
igt_assert_f(a->pending_crtc_idx_mask !=
b->pending_crtc_idx_mask,
"%s and %s are both trying to use pipe %s\n",
@@ -1565,25 +1560,9 @@ static void igt_display_refresh(igt_display_t *display)
}
}
- /*
- * The pipe allocation has to be done in two phases:
- * - first, try to satisfy the outputs where a pipe has been specified
- * - then, allocate the outputs with PIPE_ANY
- */
for (i = 0; i < display->n_outputs; i++) {
igt_output_t *output = &display->outputs[i];
- if (output->pending_crtc_idx_mask == -1UL)
- continue;
-
- igt_output_refresh(output);
- }
- for (i = 0; i < display->n_outputs; i++) {
- igt_output_t *output = &display->outputs[i];
-
- if (output->pending_crtc_idx_mask != -1UL)
- continue;
-
igt_output_refresh(output);
}
}
@@ -1593,12 +1572,11 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
igt_display_t *display = output->display;
enum pipe pipe;
- if (output->pending_crtc_idx_mask == -1UL) {
+ if (!output->pending_crtc_idx_mask) {
/*
- * The user hasn't specified a pipe to use, take the one
- * configured by the last refresh()
+ * The user hasn't specified a pipe to use, return none.
*/
- pipe = output->config.pipe;
+ return NULL;
} else {
/*
* Otherwise, return the pending pipe (ie the pipe that should
@@ -1628,6 +1606,21 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t *pipe, enum igt_plane plane)
return &pipe->planes[idx];
}
+static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe)
+{
+ igt_display_t *display = pipe->display;
+ int i;
+
+ for (i = 0; i < display->n_outputs; i++) {
+ igt_output_t *output = &display->outputs[i];
+
+ if (output->pending_crtc_idx_mask == (1 << pipe->pipe))
+ return output;
+ }
+
+ return NULL;
+}
+
bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
uint32_t *prop_id, uint64_t *value,
drmModePropertyPtr *prop)
@@ -1741,10 +1734,10 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
* tests that expect a specific error code).
*/
static int igt_drm_plane_commit(igt_plane_t *plane,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
- igt_display_t *display = output->display;
+ igt_display_t *display = pipe->display;
uint32_t fb_id, crtc_id;
int ret;
uint32_t src_x;
@@ -1763,13 +1756,12 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
!plane->rotation_changed);
fb_id = igt_plane_get_fb_id(plane);
- crtc_id = output->config.crtc->crtc_id;
+ crtc_id = pipe->crtc_id;
if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
LOG(display,
- "%s: SetPlane pipe %s, plane %d, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "SetPlane pipe %s, plane %d, disabling\n",
+ kmstest_pipe_name(pipe->pipe),
plane->index);
ret = drmModeSetPlane(display->drm_fd,
@@ -1797,10 +1789,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
crtc_h = plane->crtc_h;
LOG(display,
- "%s: SetPlane %s.%d, fb %u, src = (%d, %d) "
+ "SetPlane %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),
+ kmstest_pipe_name(pipe->pipe),
plane->index,
fb_id,
src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
@@ -1841,11 +1832,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
* code).
*/
static int igt_cursor_commit_legacy(igt_plane_t *cursor,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
- igt_display_t *display = output->display;
- uint32_t crtc_id = output->config.crtc->crtc_id;
+ igt_display_t *display = pipe->display;
+ uint32_t crtc_id = pipe->crtc_id;
int ret;
if (cursor->fb_changed) {
@@ -1853,9 +1844,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
if (gem_handle) {
LOG(display,
- "%s: SetCursor pipe %s, fb %u %dx%d\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "SetCursor pipe %s, fb %u %dx%d\n",
+ kmstest_pipe_name(pipe->pipe),
gem_handle,
cursor->crtc_w, cursor->crtc_h);
@@ -1865,9 +1855,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
cursor->crtc_h);
} else {
LOG(display,
- "%s: SetCursor pipe %s, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe));
+ "SetCursor pipe %s, disabling\n",
+ kmstest_pipe_name(pipe->pipe));
ret = drmModeSetCursor(display->drm_fd, crtc_id,
0, 0, 0);
@@ -1883,9 +1872,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
int y = cursor->crtc_y;
LOG(display,
- "%s: MoveCursor pipe %s, (%d, %d)\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "MoveCursor pipe %s, (%d, %d)\n",
+ kmstest_pipe_name(pipe->pipe),
x, y);
ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
@@ -1902,10 +1890,11 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
* (setmode).
*/
static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
- igt_output_t *output,
+ igt_pipe_t *pipe,
bool fail_on_error)
{
struct igt_display *display = primary->pipe->display;
+ igt_output_t *output = igt_pipe_get_output(pipe);
drmModeModeInfo *mode;
uint32_t fb_id, crtc_id;
int ret;
@@ -1920,7 +1909,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
!primary->size_changed && !primary->panning_changed)
return 0;
- crtc_id = output->config.crtc->crtc_id;
+ crtc_id = pipe->crtc_id;
fb_id = igt_plane_get_fb_id(primary);
if (fb_id)
mode = igt_output_get_mode(output);
@@ -1932,7 +1921,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
"%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
"mode %dx%d\n",
igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ kmstest_pipe_name(pipe->pipe),
fb_id,
primary->pan_x, primary->pan_y,
mode->hdisplay, mode->vdisplay);
@@ -1946,9 +1935,8 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
mode);
} else {
LOG(display,
- "%s: SetCrtc pipe %s, disabling\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe));
+ "SetCrtc pipe %s, disabling\n",
+ kmstest_pipe_name(pipe->pipe));
ret = drmModeSetCrtc(display->drm_fd,
crtc_id,
@@ -1976,17 +1964,17 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
* which API is used to do the programming.
*/
static int igt_plane_commit(igt_plane_t *plane,
- igt_output_t *output,
+ igt_pipe_t *pipe,
enum igt_commit_style s,
bool fail_on_error)
{
if (plane->is_cursor && s == COMMIT_LEGACY) {
- return igt_cursor_commit_legacy(plane, output, fail_on_error);
+ return igt_cursor_commit_legacy(plane, pipe, fail_on_error);
} else if (plane->is_primary && s == COMMIT_LEGACY) {
- return igt_primary_plane_commit_legacy(plane, output,
+ return igt_primary_plane_commit_legacy(plane, pipe,
fail_on_error);
} else {
- return igt_drm_plane_commit(plane, output, fail_on_error);
+ return igt_drm_plane_commit(plane, pipe, fail_on_error);
}
}
@@ -2000,31 +1988,28 @@ static int igt_plane_commit(igt_plane_t *plane,
* further programming will take place, which may result in some changes
* taking effect and others not taking effect.
*/
-static int igt_output_commit(igt_output_t *output,
- enum igt_commit_style s,
- bool fail_on_error)
+static int igt_pipe_commit(igt_pipe_t *pipe,
+ enum igt_commit_style s,
+ bool fail_on_error)
{
- igt_display_t *display = output->display;
- igt_pipe_t *pipe;
+ igt_display_t *display = pipe->display;
int i;
int ret;
bool need_wait_for_vblank = false;
- pipe = igt_output_get_driving_pipe(output);
-
if (pipe->background_changed) {
- igt_crtc_set_property(output, pipe->background_property,
+ igt_crtc_set_property(pipe, pipe->background_property,
pipe->background);
pipe->background_changed = false;
}
if (pipe->color_mgmt_changed) {
- igt_crtc_set_property(output, pipe->degamma_property,
+ igt_crtc_set_property(pipe, pipe->degamma_property,
pipe->degamma_blob);
- igt_crtc_set_property(output, pipe->ctm_property,
+ igt_crtc_set_property(pipe, pipe->ctm_property,
pipe->ctm_blob);
- igt_crtc_set_property(output, pipe->gamma_property,
+ igt_crtc_set_property(pipe, pipe->gamma_property,
pipe->gamma_blob);
pipe->color_mgmt_changed = false;
@@ -2036,7 +2021,7 @@ static int igt_output_commit(igt_output_t *output,
if (plane->fb_changed || plane->position_changed || plane->size_changed)
need_wait_for_vblank = true;
- ret = igt_plane_commit(plane, output, s, fail_on_error);
+ ret = igt_plane_commit(plane, pipe, s, fail_on_error);
CHECK_RETURN(ret, fail_on_error);
}
@@ -2060,6 +2045,9 @@ static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicRe
igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
+ if (!pipe_obj)
+ return;
+
if (pipe_obj->background_changed)
igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
@@ -2125,6 +2113,8 @@ static int igt_atomic_commit(igt_display_t *display)
pipe_obj = igt_output_get_driving_pipe(output);
+ if (!pipe_obj)
+ continue;
pipe = pipe_obj->pipe;
@@ -2167,14 +2157,14 @@ static int do_display_commit(igt_display_t *display,
}
- for (i = 0; i < display->n_outputs; i++) {
- igt_output_t *output = &display->outputs[i];
+ for_each_pipe(display, i) {
+ igt_pipe_t *pipe_obj = &display->pipes[i];
+ igt_output_t *output = igt_pipe_get_output(pipe_obj);
- if (!output->valid)
- continue;
+ if (output && output->valid)
+ valid_outs++;
- valid_outs++;
- ret = igt_output_commit(output, s, fail_on_error);
+ ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
CHECK_RETURN(ret, fail_on_error);
}
@@ -2282,9 +2272,9 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
{
igt_display_t *display = output->display;
- if (pipe == PIPE_ANY) {
+ if (pipe == PIPE_NONE) {
LOG(display, "%s: set_pipe(any)\n", igt_output_name(output));
- output->pending_crtc_idx_mask = -1UL;
+ output->pending_crtc_idx_mask = 0;
} else {
LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
kmstest_pipe_name(pipe));
@@ -2297,6 +2287,8 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane)
igt_pipe_t *pipe;
pipe = igt_output_get_driving_pipe(output);
+ igt_assert(pipe);
+
return igt_pipe_get_plane(pipe, plane);
}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index dc6be5e53b74..3531dc20b6e0 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -334,17 +334,31 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
+static inline bool igt_output_is_connected(igt_output_t *output)
+{
+ /* Something went wrong during probe? */
+ if (!output->config.connector)
+ return false;
+
+ if (output->config.connector->connection == DRM_MODE_CONNECTED)
+ return true;
+
+ return false;
+}
+
static inline bool igt_pipe_connector_valid(enum pipe pipe,
igt_output_t *output)
{
- return output->valid && (output->config.valid_crtc_idx_mask & (1 << pipe));
+ return igt_output_is_connected(output) &&
+ (output->config.valid_crtc_idx_mask & (1 << pipe));
}
#define for_each_if(condition) if (!(condition)) {} else
#define for_each_connected_output(display, output) \
for (int i__ = 0; i__ < (display)->n_outputs; i__++) \
- for_each_if (((output = &(display)->outputs[i__]), output->valid))
+ for_each_if (((output = &(display)->outputs[i__]), \
+ igt_output_is_connected(output)))
#define for_each_pipe(display, pipe) \
for (pipe = 0; pipe < igt_display_get_n_pipes(display); pipe++) \
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (5 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-21 10:07 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2 Maarten Lankhorst
` (7 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
Move properties to the pipe, they don't belong in the output
and make atomic commit use the pipes for crtc properties.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 98 +++++++++++++++++++++++++++++------------------------------
lib/igt_kms.h | 12 +++++---
2 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 88cae7d51787..8c7598a88a69 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -213,8 +213,7 @@ igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
* 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,
+igt_atomic_fill_connector_props(igt_display_t *display, igt_output_t *output,
int num_connector_props, const char **conn_prop_names)
{
drmModeObjectPropertiesPtr props;
@@ -222,18 +221,18 @@ igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
fd = display->drm_fd;
- props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC);
+ 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_crtc_props; j++) {
- if (strcmp(prop->name, crtc_prop_names[j]) != 0)
+ for (j = 0; j < num_connector_props; j++) {
+ if (strcmp(prop->name, conn_prop_names[j]) != 0)
continue;
- output->config.atomic_props_crtc[j] = props->props[i];
+ output->config.atomic_props_connector[j] = props->props[i];
break;
}
@@ -241,19 +240,29 @@ igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
}
drmModeFreeObjectProperties(props);
- props = NULL;
- props = drmModeObjectGetProperties(fd, output->config.connector->connector_id, DRM_MODE_OBJECT_CONNECTOR);
+}
+
+static void
+igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
+ int num_crtc_props, const char **crtc_prop_names)
+{
+ drmModeObjectPropertiesPtr props;
+ int i, j, fd;
+
+ fd = display->drm_fd;
+
+ props = drmModeObjectGetProperties(fd, pipe->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_connector_props; j++) {
- if (strcmp(prop->name, conn_prop_names[j]) != 0)
+ for (j = 0; j < num_crtc_props; j++) {
+ if (strcmp(prop->name, crtc_prop_names[j]) != 0)
continue;
- output->config.atomic_props_connector[j] = props->props[i];
+ pipe->atomic_props_crtc[j] = props->props[i];
break;
}
@@ -261,7 +270,6 @@ igt_atomic_fill_props(igt_display_t *display, igt_output_t *output,
}
drmModeFreeObjectProperties(props);
-
}
const unsigned char* igt_kms_get_alt_edid(void)
@@ -1222,6 +1230,9 @@ static void igt_output_refresh(igt_output_t *output)
-1);
}
+ igt_atomic_fill_connector_props(display, output,
+ IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
+
if (!output->valid)
return;
@@ -1232,8 +1243,6 @@ 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
@@ -1359,6 +1368,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
NULL,
NULL);
+ igt_atomic_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names);
+
/* add the planes that can be used with that pipe */
for (j = 0; j < plane_resources->count_planes; j++) {
drmModePlane *drm_plane;
@@ -1660,11 +1671,10 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t *plane)
* 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,
+igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
drmModeAtomicReq *req)
{
-
- igt_display_t *display = output->display;
+ igt_display_t *display = pipe->display;
uint32_t fb_id, crtc_id;
igt_assert(plane->drm_plane);
@@ -1674,12 +1684,11 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
!plane->rotation_changed);
fb_id = igt_plane_get_fb_id(plane);
- crtc_id = output->config.crtc->crtc_id;
+ crtc_id = pipe->crtc_id;
LOG(display,
- "%s: populating plane data: %s.%d, fb %u\n",
- igt_output_name(output),
- kmstest_pipe_name(output->config.pipe),
+ "populating plane data: %s.%d, fb %u\n",
+ kmstest_pipe_name(pipe->pipe),
plane->index,
fb_id);
@@ -2040,21 +2049,15 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
/*
* Add crtc property changes to the atomic property set
*/
-static void igt_atomic_prepare_crtc_commit(igt_output_t *output, drmModeAtomicReq *req)
+static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicReq *req)
{
-
- igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
-
- if (!pipe_obj)
- return;
-
if (pipe_obj->background_changed)
- igt_atomic_populate_crtc_req(req, output, IGT_CRTC_BACKGROUND, pipe_obj->background);
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_BACKGROUND, pipe_obj->background);
if (pipe_obj->color_mgmt_changed) {
- igt_atomic_populate_crtc_req(req, output, IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
- igt_atomic_populate_crtc_req(req, output, IGT_CRTC_CTM, pipe_obj->ctm_blob);
- igt_atomic_populate_crtc_req(req, output, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_CTM, pipe_obj->ctm_blob);
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
}
/*
@@ -2088,42 +2091,37 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
static int igt_atomic_commit(igt_display_t *display)
{
- int ret = 0;
+ int ret = 0, i;
+ enum pipe pipe;
drmModeAtomicReq *req;
igt_output_t *output;
+
if (display->is_atomic != 1)
return -1;
req = drmModeAtomicAlloc();
drmModeAtomicSetCursor(req, 0);
- for_each_connected_output(display, output) {
- igt_pipe_t *pipe_obj;
+ for_each_pipe(display, pipe) {
+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
igt_plane_t *plane;
- enum pipe pipe;
/*
* 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_obj = igt_output_get_driving_pipe(output);
- if (!pipe_obj)
- continue;
-
- pipe = pipe_obj->pipe;
+ igt_atomic_prepare_crtc_commit(pipe_obj, req);
for_each_plane_on_pipe(display, pipe, plane) {
- igt_atomic_prepare_plane_commit(plane, output, req);
+ igt_atomic_prepare_plane_commit(plane, pipe_obj, req);
}
}
+ for (i = 0; i < display->n_outputs; i++) {
+ output = &display->outputs[i];
+
+ igt_atomic_prepare_connector_commit(output, req);
+ }
+
ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
drmModeAtomicFree(req);
return ret;
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 3531dc20b6e0..b9e8d344ed8c 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -117,7 +117,6 @@ struct kmstest_connector_config {
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 pipe;
unsigned valid_crtc_idx_mask;
@@ -257,6 +256,9 @@ struct igt_pipe {
bool enabled;
int n_planes;
igt_plane_t planes[IGT_MAX_PLANES];
+
+ uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
+
uint64_t background; /* Background color MSB BGR 16bpc LSB */
uint32_t background_changed : 1;
uint32_t background_property;
@@ -395,13 +397,13 @@ static inline bool igt_pipe_connector_valid(enum pipe pipe,
/**
* igt_atomic_populate_crtc_req:
* @req: A pointer to drmModeAtomicReq
- * @output: A pointer igt_output_t
+ * @pipe: A pointer igt_pipe_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))
+#define igt_atomic_populate_crtc_req(req, pipe, prop, value) \
+ igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe->crtc_id,\
+ pipe->atomic_props_crtc[prop], value))
/**
* igt_atomic_populate_connector_req:
* @req: A pointer to drmModeAtomicReq
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (6 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-21 11:42 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally Maarten Lankhorst
` (6 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
They're duplicates with src_x/y, so just use those.
Changes since v1:
- Fix order of parameters in calls to igt_fb_set_position.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 24 ++++--------------------
lib/igt_kms.h | 4 ----
tests/kms_panel_fitting.c | 2 +-
tests/kms_plane.c | 8 +++-----
tests/kms_plane_scaling.c | 2 +-
5 files changed, 9 insertions(+), 31 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8c7598a88a69..9fc5559a5df4 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1915,7 +1915,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
igt_assert(!primary->rotation_changed);
if (!primary->fb_changed && !primary->position_changed &&
- !primary->size_changed && !primary->panning_changed)
+ !primary->size_changed)
return 0;
crtc_id = pipe->crtc_id;
@@ -1927,18 +1927,18 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
if (fb_id) {
LOG(display,
- "%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
+ "%s: SetCrtc pipe %s, fb %u, src (%d, %d), "
"mode %dx%d\n",
igt_output_name(output),
kmstest_pipe_name(pipe->pipe),
fb_id,
- primary->pan_x, primary->pan_y,
+ primary->src_x, primary->src_x,
mode->hdisplay, mode->vdisplay);
ret = drmModeSetCrtc(display->drm_fd,
crtc_id,
fb_id,
- primary->pan_x, primary->pan_y,
+ primary->src_x, primary->src_x,
&output->id,
1,
mode);
@@ -1962,7 +1962,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
primary->fb_changed = false;
primary->position_changed = false;
primary->size_changed = false;
- primary->panning_changed = false;
return 0;
}
@@ -2408,21 +2407,6 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t *plane,
plane->fb_changed = true;
}
-void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
-{
- igt_pipe_t *pipe = plane->pipe;
- igt_display_t *display = pipe->display;
-
- LOG(display, "%s.%d: plane_set_panning(%d,%d)\n",
- kmstest_pipe_name(pipe->pipe),
- plane->index, x, y);
-
- plane->pan_x = x;
- plane->pan_y = y;
-
- plane->panning_changed = true;
-}
-
static const char *rotation_name(igt_rotation_t rotation)
{
switch (rotation) {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index b9e8d344ed8c..4d18c01042c9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -220,7 +220,6 @@ typedef struct {
/* state tracking */
unsigned int fb_changed : 1;
unsigned int position_changed : 1;
- unsigned int panning_changed : 1;
unsigned int rotation_changed : 1;
unsigned int size_changed : 1;
/*
@@ -244,8 +243,6 @@ typedef struct {
uint32_t src_w;
uint32_t src_h;
- /* 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;
@@ -326,7 +323,6 @@ void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr, size_t length);
void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
void igt_plane_set_position(igt_plane_t *plane, int x, int y);
void igt_plane_set_size(igt_plane_t *plane, int w, int h);
-void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index 7c501fcdd3fb..da09da32d704 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -89,7 +89,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
ret = drmModeSetCrtc(data->drm_fd,
plane->pipe->crtc_id,
data->fb_id1,
- plane->pan_x, plane->pan_y,
+ plane->src_x, plane->src_y,
&output->id,
1,
mode);
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 60b8d71d52d4..7c01fe91b263 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -321,11 +321,9 @@ test_plane_panning_with_output(data_t *data,
igt_plane_set_fb(primary, &primary_fb);
if (flags & TEST_PANNING_TOP_LEFT)
- igt_plane_set_panning(primary, 0, 0);
+ igt_fb_set_position(&primary_fb, primary, 0, 0);
else
- igt_plane_set_panning(primary, mode->hdisplay, mode->vdisplay);
-
- igt_plane_set_position(primary, 0, 0);
+ igt_fb_set_position(&primary_fb, primary, mode->hdisplay, mode->vdisplay);
igt_display_commit(&data->display);
@@ -343,7 +341,7 @@ test_plane_panning_with_output(data_t *data,
/* reset states to neutral values, assumed by other tests */
igt_output_set_pipe(output, PIPE_ANY);
- igt_plane_set_panning(primary, 0, 0);
+ igt_fb_set_position(&primary_fb, primary, 0, 0);
test_fini(data);
}
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 39bb5e113411..4546ce548515 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -98,7 +98,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
ret = drmModeSetCrtc(data->drm_fd,
plane->pipe->crtc_id,
data->fb_id1,
- plane->pan_x, plane->pan_y,
+ plane->src_x, plane->src_y,
&output->id,
1,
mode);
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (7 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2 Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-21 12:13 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 10/15] igt_kms: Add modeset support to atomic commits Maarten Lankhorst
` (5 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This will make it easier to support TEST_ONLY commits.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 84 ++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 48 insertions(+), 36 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 9fc5559a5df4..d0d0dcff8193 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1727,11 +1727,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
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;
- plane->rotation_changed = false;
}
@@ -1819,15 +1814,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
CHECK_RETURN(ret, fail_on_error);
}
- plane->fb_changed = false;
- plane->position_changed = false;
- plane->size_changed = false;
-
if (plane->rotation_changed) {
ret = igt_plane_set_property(plane, plane->rotation_property,
plane->rotation);
- plane->rotation_changed = false;
CHECK_RETURN(ret, fail_on_error);
}
@@ -1872,8 +1862,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
}
CHECK_RETURN(ret, fail_on_error);
-
- cursor->fb_changed = false;
}
if (cursor->position_changed) {
@@ -1887,8 +1875,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
CHECK_RETURN(ret, fail_on_error);
-
- cursor->position_changed = false;
}
return 0;
@@ -1915,7 +1901,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
igt_assert(!primary->rotation_changed);
if (!primary->fb_changed && !primary->position_changed &&
- !primary->size_changed)
+ !primary->size_changed)
return 0;
crtc_id = pipe->crtc_id;
@@ -1959,9 +1945,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
CHECK_RETURN(ret, fail_on_error);
primary->pipe->enabled = (fb_id != 0);
- primary->fb_changed = false;
- primary->position_changed = false;
- primary->size_changed = false;
return 0;
}
@@ -1982,7 +1965,7 @@ static int igt_plane_commit(igt_plane_t *plane,
return igt_primary_plane_commit_legacy(plane, pipe,
fail_on_error);
} else {
- return igt_drm_plane_commit(plane, pipe, fail_on_error);
+ return igt_drm_plane_commit(plane, pipe, fail_on_error);
}
}
@@ -2008,8 +1991,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
if (pipe->background_changed) {
igt_crtc_set_property(pipe, pipe->background_property,
pipe->background);
-
- pipe->background_changed = false;
}
if (pipe->color_mgmt_changed) {
@@ -2019,8 +2000,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
pipe->ctm_blob);
igt_crtc_set_property(pipe, pipe->gamma_property,
pipe->gamma_blob);
-
- pipe->color_mgmt_changed = false;
}
for (i = 0; i < pipe->n_planes; i++) {
@@ -2140,35 +2119,68 @@ static int do_display_commit(igt_display_t *display,
bool fail_on_error)
{
int i, ret;
- int valid_outs = 0;
-
+ enum pipe pipe;
LOG_INDENT(display, "commit");
igt_display_refresh(display);
if (s == COMMIT_ATOMIC) {
-
ret = igt_atomic_commit(display);
+
CHECK_RETURN(ret, fail_on_error);
- return 0;
+ } else {
+ int valid_outs = 0;
- }
+ for_each_pipe(display, pipe) {
+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
+ igt_output_t *output = igt_pipe_get_output(pipe_obj);
- for_each_pipe(display, i) {
- igt_pipe_t *pipe_obj = &display->pipes[i];
- igt_output_t *output = igt_pipe_get_output(pipe_obj);
+ if (output && output->valid)
+ valid_outs++;
- if (output && output->valid)
- valid_outs++;
+ ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
+ CHECK_RETURN(ret, fail_on_error);
+ }
- ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
CHECK_RETURN(ret, fail_on_error);
+
+ if (valid_outs == 0) {
+ LOG_UNINDENT(display);
+
+ return -1;
+ }
}
LOG_UNINDENT(display);
- if (valid_outs == 0)
- return -1;
+ if (ret)
+ return ret;
+
+ for_each_pipe(display, pipe) {
+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
+ igt_plane_t *plane;
+
+ if (s == COMMIT_ATOMIC) {
+ pipe_obj->color_mgmt_changed = false;
+ pipe_obj->background_changed = false;
+ }
+
+ for_each_plane_on_pipe(display, pipe, plane) {
+ plane->fb_changed = false;
+ plane->position_changed = false;
+ plane->size_changed = false;
+
+ if (s != COMMIT_LEGACY || !(plane->is_primary || plane->is_cursor))
+ plane->rotation_changed = false;
+ }
+ }
+
+ for (i = 0; i < display->n_outputs && s == COMMIT_ATOMIC; i++) {
+ igt_output_t *output = &display->outputs[i];
+
+ output->config.connector_dpms_changed = false;
+ output->config.connector_scaling_mode_changed = false;
+ }
igt_debug_wait_for_keypress("modeset");
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 10/15] igt_kms: Add modeset support to atomic commits.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (8 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 11/15] tests: Add kms_rmfb test Maarten Lankhorst
` (4 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
Add the correct properties, add pipe_changed and mode_changed and go!
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++------------
lib/igt_kms.h | 10 +++--
2 files changed, 105 insertions(+), 29 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d0d0dcff8193..17865c263f35 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -167,11 +167,13 @@ static const char *igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
"DEGAMMA_LUT",
"CTM",
"GAMMA_LUT",
+ "MODE_ID",
+ "ACTIVE"
};
static const char *igt_connector_prop_names[IGT_NUM_CONNECTOR_PROPS] = {
"scaling mode",
- "DPMS"
+ "CRTC_ID"
};
/*
@@ -312,6 +314,9 @@ const char *kmstest_pipe_name(enum pipe pipe)
{
const char *str[] = { "A", "B", "C" };
+ if (pipe == PIPE_NONE)
+ return "None";
+
if (pipe > 2)
return "invalid";
@@ -841,6 +846,8 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
drmModeRes *resources;
drmModeConnector *connector;
+ config->pipe = PIPE_NONE;
+
resources = drmModeGetResources(drm_fd);
if (!resources) {
igt_warn("drmModeGetResources failed");
@@ -1230,8 +1237,9 @@ static void igt_output_refresh(igt_output_t *output)
-1);
}
- igt_atomic_fill_connector_props(display, output,
- IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
+ if (output->config.connector)
+ igt_atomic_fill_connector_props(display, output,
+ IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
if (!output->valid)
return;
@@ -1465,8 +1473,13 @@ void igt_display_init(igt_display_t *display, int drm_fd)
pipe->n_planes = n_planes;
+ for_each_plane_on_pipe(display, i, plane)
+ plane->fb_changed = true;
+
/* make sure we don't overflow the plane array */
igt_assert_lte(pipe->n_planes, IGT_MAX_PLANES);
+
+ pipe->mode_changed = true;
}
/*
@@ -1490,6 +1503,8 @@ void igt_display_init(igt_display_t *display, int drm_fd)
output->display = display;
igt_output_refresh(output);
+
+ output->config.pipe_changed = true;
}
drmModeFreePlaneResources(plane_resources);
@@ -2023,6 +2038,22 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
return 0;
}
+static void
+igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length)
+{
+ igt_display_t *display = pipe->display;
+ uint32_t blob_id = 0;
+
+ if (*blob != 0)
+ igt_assert(drmModeDestroyPropertyBlob(display->drm_fd,
+ *blob) == 0);
+
+ if (length > 0)
+ igt_assert(drmModeCreatePropertyBlob(display->drm_fd,
+ ptr, length, &blob_id) == 0);
+
+ *blob = blob_id;
+}
/*
* Add crtc property changes to the atomic property set
@@ -2038,6 +2069,28 @@ static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj, drmModeAtomicRe
igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
}
+ if (pipe_obj->mode_changed) {
+ igt_output_t *output = igt_pipe_get_output(pipe_obj);
+
+ if (!output) {
+ igt_pipe_replace_blob(pipe_obj, &pipe_obj->mode_blob, NULL, 0);
+
+ LOG(pipe_obj->display, "%s: Setting NULL mode\n",
+ kmstest_pipe_name(pipe_obj->pipe));
+ } else {
+ drmModeModeInfo *mode = igt_output_get_mode(output);
+
+ igt_pipe_replace_blob(pipe_obj, &pipe_obj->mode_blob, mode, sizeof(*mode));
+
+ LOG(pipe_obj->display, "%s: Setting mode %s from %s\n",
+ kmstest_pipe_name(pipe_obj->pipe),
+ mode->name, igt_output_name(output));
+ }
+
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_MODE_ID, pipe_obj->mode_blob);
+ igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_ACTIVE, !!output);
+ }
+
/*
* TODO: Add all crtc level properties here
*/
@@ -2054,8 +2107,14 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
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);
+ if (config->pipe_changed) {
+ uint32_t crtc_id = 0;
+
+ if (output->config.pipe != PIPE_NONE)
+ crtc_id = output->config.crtc->crtc_id;
+
+ igt_atomic_populate_connector_req(req, output, IGT_CONNECTOR_CRTC_ID, crtc_id);
+ }
/*
* TODO: Add all other connector level properties here
*/
@@ -2097,10 +2156,17 @@ static int igt_atomic_commit(igt_display_t *display)
for (i = 0; i < display->n_outputs; i++) {
output = &display->outputs[i];
+ if (!output->config.connector)
+ continue;
+
+ LOG(display, "%s: preparing atomic, pipe: %s\n",
+ igt_output_name(output),
+ kmstest_pipe_name(output->config.pipe));
+
igt_atomic_prepare_connector_commit(output, req);
}
- ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
+ ret = drmModeAtomicCommit(display->drm_fd, req, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
drmModeAtomicFree(req);
return ret;
@@ -2165,6 +2231,9 @@ static int do_display_commit(igt_display_t *display,
pipe_obj->background_changed = false;
}
+ if (s != COMMIT_UNIVERSAL)
+ pipe_obj->mode_changed = false;
+
for_each_plane_on_pipe(display, pipe, plane) {
plane->fb_changed = false;
plane->position_changed = false;
@@ -2175,11 +2244,14 @@ static int do_display_commit(igt_display_t *display,
}
}
- for (i = 0; i < display->n_outputs && s == COMMIT_ATOMIC; i++) {
+ for (i = 0; i < display->n_outputs; i++) {
igt_output_t *output = &display->outputs[i];
- output->config.connector_dpms_changed = false;
- output->config.connector_scaling_mode_changed = false;
+ if (s != COMMIT_UNIVERSAL)
+ output->config.pipe_changed = false;
+
+ if (s == COMMIT_ATOMIC)
+ output->config.connector_scaling_mode_changed = false;
}
igt_debug_wait_for_keypress("modeset");
@@ -2273,13 +2345,25 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output)
*/
void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
{
+ igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
+
output->override_mode = *mode;
output->use_override_mode = true;
+
+ if (pipe)
+ pipe->mode_changed = true;
}
void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
{
igt_display_t *display = output->display;
+ igt_pipe_t *old_pipe;
+
+ if (output->pending_crtc_idx_mask) {
+ old_pipe = igt_output_get_driving_pipe(output);
+
+ old_pipe->mode_changed = true;
+ }
if (pipe == PIPE_NONE) {
LOG(display, "%s: set_pipe(any)\n", igt_output_name(output));
@@ -2288,7 +2372,12 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
kmstest_pipe_name(pipe));
output->pending_crtc_idx_mask = 1 << pipe;
+
+ display->pipes[pipe].mode_changed = true;
}
+
+ if (pipe != output->config.pipe)
+ output->config.pipe_changed = true;
}
igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane)
@@ -2449,23 +2538,6 @@ void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation)
plane->rotation_changed = true;
}
-static void
-igt_pipe_replace_blob(igt_pipe_t *pipe, uint64_t *blob, void *ptr, size_t length)
-{
- igt_display_t *display = pipe->display;
- uint32_t blob_id = 0;
-
- if (*blob != 0)
- igt_assert(drmModeDestroyPropertyBlob(display->drm_fd,
- *blob) == 0);
-
- if (length > 0)
- igt_assert(drmModeCreatePropertyBlob(display->drm_fd,
- ptr, length, &blob_id) == 0);
-
- *blob = blob_id;
-}
-
void
igt_pipe_set_degamma_lut(igt_pipe_t *pipe, void *ptr, size_t length)
{
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4d18c01042c9..39e3b9fa1972 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -99,12 +99,14 @@ enum igt_atomic_crtc_properties {
IGT_CRTC_CTM,
IGT_CRTC_DEGAMMA_LUT,
IGT_CRTC_GAMMA_LUT,
+ IGT_CRTC_MODE_ID,
+ IGT_CRTC_ACTIVE,
IGT_NUM_CRTC_PROPS
};
enum igt_atomic_connector_properties {
IGT_CONNECTOR_SCALING_MODE = 0,
- IGT_CONNECTOR_DPMS,
+ IGT_CONNECTOR_CRTC_ID,
IGT_NUM_CONNECTOR_PROPS
};
@@ -115,8 +117,7 @@ struct kmstest_connector_config {
drmModeModeInfo default_mode;
uint64_t connector_scaling_mode;
bool connector_scaling_mode_changed;
- uint64_t connector_dpms;
- bool connector_dpms_changed;
+ bool pipe_changed;
uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
int pipe;
unsigned valid_crtc_idx_mask;
@@ -269,6 +270,9 @@ struct igt_pipe {
uint32_t color_mgmt_changed : 1;
uint32_t crtc_id;
+
+ uint64_t mode_blob;
+ bool mode_changed;
};
typedef struct {
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 11/15] tests: Add kms_rmfb test.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (9 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 10/15] igt_kms: Add modeset support to atomic commits Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 12/15] tests: Add kms_atomic_transition Maarten Lankhorst
` (3 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This tests rmfb and last close behavior. In those cases the framebuffers
should be removed from the crtc.
If atomic modeset is supported, we do it all in one nice atomic commit too.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
tests/Makefile.sources | 1 +
tests/kms_rmfb.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 172 insertions(+)
create mode 100644 tests/kms_rmfb.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 80e9a35330dd..2ab25eac7764 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -103,6 +103,7 @@ TESTS_progs_M = \
kms_plane \
kms_psr_sink_crc \
kms_render \
+ kms_rmfb \
kms_rotation_crc \
kms_setmode \
kms_universal_plane \
diff --git a/tests/kms_rmfb.c b/tests/kms_rmfb.c
new file mode 100644
index 000000000000..17a3065a52bb
--- /dev/null
+++ b/tests/kms_rmfb.c
@@ -0,0 +1,171 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "drmtest.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#ifndef DRM_CAP_CURSOR_WIDTH
+#define DRM_CAP_CURSOR_WIDTH 0x8
+#endif
+#ifndef DRM_CAP_CURSOR_HEIGHT
+#define DRM_CAP_CURSOR_HEIGHT 0x9
+#endif
+
+struct rmfb_data {
+ int drm_fd;
+ igt_display_t display;
+};
+
+/*
+ * 1. Set primary plane to a known fb.
+ * 2. Make sure getcrtc returns the correct fb id.
+ * 3. Call rmfb on the fb.
+ * 4. Make sure getcrtc returns 0 fb id.
+ *
+ * RMFB is supposed to free the framebuffers from any and all planes,
+ * so test this and make sure it works.
+ */
+static void
+test_rmfb(struct rmfb_data *data, igt_output_t *output, enum pipe pipe, bool reopen)
+{
+ struct igt_fb fb, argb_fb;
+ drmModeModeInfo *mode;
+ igt_plane_t *plane;
+ drmModeCrtc *crtc;
+ uint64_t cursor_width, cursor_height;
+
+ igt_output_set_pipe(output, pipe);
+
+ mode = igt_output_get_mode(output);
+
+ igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
+
+ igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &argb_fb);
+
+ do_or_die(drmGetCap(data->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
+ if (cursor_width > mode->hdisplay)
+ cursor_width = mode->hdisplay;
+
+ do_or_die(drmGetCap(data->drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height));
+ if (cursor_height > mode->vdisplay)
+ cursor_height = mode->vdisplay;
+
+ /*
+ * Make sure these buffers are suited for display use
+ * because most of the modeset operations must be fast
+ * later on.
+ */
+ for_each_plane_on_pipe(&data->display, pipe, plane) {
+ if (plane->is_cursor) {
+ igt_plane_set_fb(plane, &argb_fb);
+ igt_fb_set_size(&argb_fb, plane, cursor_width, cursor_height);
+ igt_plane_set_size(plane, cursor_width, cursor_height);
+ } else {
+ igt_plane_set_fb(plane, &fb);
+ }
+ }
+
+ igt_display_commit2(&data->display, data->display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+
+ crtc = drmModeGetCrtc(data->drm_fd, output->config.crtc->crtc_id);
+
+ igt_assert_eq(crtc->buffer_id, fb.fb_id);
+
+ drmModeFreeCrtc(crtc);
+
+ if (reopen) {
+ close(data->drm_fd);
+
+ data->drm_fd = drm_open_driver_master(DRIVER_ANY);
+ drmSetClientCap(data->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+ drmSetClientCap(data->drm_fd, DRM_CLIENT_CAP_ATOMIC, 1);
+
+ data->display.pipes[pipe].mode_blob = 0;
+ } else {
+ igt_remove_fb(data->drm_fd, &fb);
+ igt_remove_fb(data->drm_fd, &argb_fb);
+ }
+
+ crtc = drmModeGetCrtc(data->drm_fd, output->config.crtc->crtc_id);
+
+ igt_assert_eq(crtc->buffer_id, 0);
+
+ drmModeFreeCrtc(crtc);
+
+ for_each_plane_on_pipe(&data->display, pipe, plane) {
+ drmModePlanePtr planeres = drmModeGetPlane(data->drm_fd, plane->drm_plane->plane_id);
+
+ igt_assert_eq(planeres->fb_id, 0);
+
+ drmModeFreePlane(planeres);
+ }
+
+ igt_output_set_pipe(output, PIPE_ANY);
+}
+
+static void
+run_rmfb_test(struct rmfb_data *data, bool reopen)
+{
+ igt_output_t *output;
+ int valid_tests = 0;
+ enum pipe pipe;
+
+ for_each_pipe_with_valid_output(&data->display, pipe, output) {
+ test_rmfb(data, output, pipe, reopen);
+ valid_tests = 1;
+ }
+
+ igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+igt_main
+{
+ struct rmfb_data data = {};
+
+ igt_skip_on_simulation();
+
+ igt_fixture {
+ data.drm_fd = drm_open_driver_master(DRIVER_ANY);
+
+ kmstest_set_vt_graphics_mode();
+
+ igt_display_init(&data.display, data.drm_fd);
+ }
+
+ igt_subtest_f("rmfb-ioctl")
+ run_rmfb_test(&data, false);
+
+ igt_subtest_f("close-fd")
+ run_rmfb_test(&data, true);
+
+ igt_fixture {
+ igt_display_fini(&data.display);
+ }
+}
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 12/15] tests: Add kms_atomic_transition
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (10 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 11/15] tests: Add kms_rmfb test Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 13/15] igt_kms: Add more apis for panel fitting test Maarten Lankhorst
` (2 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This is meant as a stress test, to ensure that all combinations of
atomic transitions work correctly. This could be useful for other
drivers too, so I kept it generic. For i915 this will mainly be a
stress test on watermark calculations.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
tests/Makefile.sources | 1 +
tests/kms_atomic_transition.c | 189 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 190 insertions(+)
create mode 100644 tests/kms_atomic_transition.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 2ab25eac7764..6e86c723f78e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -84,6 +84,7 @@ TESTS_progs_M = \
gvt_basic \
kms_addfb_basic \
kms_atomic \
+ kms_atomic_transition \
kms_chv_cursor_fail \
kms_cursor_crc \
kms_cursor_legacy \
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
new file mode 100644
index 000000000000..29d89802ce40
--- /dev/null
+++ b/tests/kms_atomic_transition.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "drmtest.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#ifndef DRM_CAP_CURSOR_WIDTH
+#define DRM_CAP_CURSOR_WIDTH 0x8
+#endif
+#ifndef DRM_CAP_CURSOR_HEIGHT
+#define DRM_CAP_CURSOR_HEIGHT 0x9
+#endif
+
+static void
+wm_setup_plane(igt_display_t *display, enum pipe pipe, uint32_t mask,
+ struct igt_fb *fb, struct igt_fb *argb_fb,
+ uint64_t cursor_width, uint64_t cursor_height)
+{
+ igt_plane_t *plane;
+
+ /*
+ * Make sure these buffers are suited for display use
+ * because most of the modeset operations must be fast
+ * later on.
+ */
+ for_each_plane_on_pipe(display, pipe, plane) {
+ if (!((1 << plane->index) & mask)) {
+ igt_plane_set_fb(plane, NULL);
+ continue;
+ }
+
+ if (plane->is_primary)
+ igt_plane_set_fb(plane, fb);
+ else
+ igt_plane_set_fb(plane, argb_fb);
+
+ if (plane->is_cursor) {
+ igt_fb_set_size(argb_fb, plane, cursor_width, cursor_height);
+ igt_plane_set_size(plane, cursor_width, cursor_height);
+ }
+ }
+}
+
+/*
+ * 1. Set primary plane to a known fb.
+ * 2. Make sure getcrtc returns the correct fb id.
+ * 3. Call rmfb on the fb.
+ * 4. Make sure getcrtc returns 0 fb id.
+ *
+ * RMFB is supposed to free the framebuffers from any and all planes,
+ * so test this and make sure it works.
+ */
+static void
+run_transition_test(igt_display_t *display, enum pipe pipe, igt_output_t *output, bool modeset)
+{
+ struct igt_fb fb, argb_fb;
+ drmModeModeInfo *mode;
+ igt_plane_t *plane;
+ uint64_t cursor_width, cursor_height;
+ uint32_t iter_max = 1 << display->pipes[pipe].n_planes, i, j;
+
+ mode = igt_output_get_mode(output);
+
+ igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
+
+ igt_create_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &argb_fb);
+
+ do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_WIDTH, &cursor_width));
+ if (cursor_width > mode->hdisplay)
+ cursor_width = mode->hdisplay;
+
+ do_or_die(drmGetCap(display->drm_fd, DRM_CAP_CURSOR_HEIGHT, &cursor_height));
+ if (cursor_height > mode->vdisplay)
+ cursor_height = mode->vdisplay;
+
+ if (modeset) {
+ igt_output_set_pipe(output, PIPE_NONE);
+
+ wm_setup_plane(display, pipe, 0, NULL, NULL,
+ cursor_width, cursor_height);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ }
+
+ for (i = 0; i < iter_max; i++) {
+ igt_output_set_pipe(output, pipe);
+
+ wm_setup_plane(display, pipe, i, &fb, &argb_fb,
+ cursor_width, cursor_height);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ if (modeset) {
+ igt_output_set_pipe(output, PIPE_NONE);
+
+ wm_setup_plane(display, pipe, 0, NULL, NULL,
+ cursor_width, cursor_height);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ } else {
+ /* i -> i+1 will be done when i increases, can be skipped here */
+ for (j = iter_max - 1; j > i + 1; j--) {
+ wm_setup_plane(display, pipe, j, &fb, &argb_fb,
+ cursor_width, cursor_height);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ wm_setup_plane(display, pipe, i, &fb, &argb_fb,
+ cursor_width, cursor_height);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ }
+ }
+ }
+
+ igt_output_set_pipe(output, PIPE_NONE);
+
+ for_each_plane_on_pipe(display, pipe, plane)
+ igt_plane_set_fb(plane, NULL);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ igt_remove_fb(display->drm_fd, &fb);
+ igt_remove_fb(display->drm_fd, &argb_fb);
+}
+
+igt_main
+{
+ igt_display_t display;
+ igt_output_t *output;
+ enum pipe pipe;
+
+ igt_skip_on_simulation();
+
+ igt_fixture {
+ int valid_outputs = 0;
+
+ display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+
+ kmstest_set_vt_graphics_mode();
+
+ igt_display_init(&display, display.drm_fd);
+
+ igt_require(display.is_atomic);
+
+ for_each_pipe_with_valid_output(&display, pipe, output)
+ valid_outputs++;
+
+ igt_require_f(valid_outputs, "no valid crtc/connector combinations found\n");
+ }
+
+ igt_subtest_f("plane-all-transition")
+ for_each_pipe_with_valid_output(&display, pipe, output)
+ run_transition_test(&display, pipe, output, false);
+
+ igt_subtest_f("plane-modeset-transition")
+ for_each_pipe_with_valid_output(&display, pipe, output)
+ run_transition_test(&display, pipe, output, true);
+
+ igt_fixture {
+ igt_display_fini(&display);
+ }
+}
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 13/15] igt_kms: Add more apis for panel fitting test.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (11 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 12/15] tests: Add kms_atomic_transition Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 14/15] igt_kms: Allow disabling previous override mode Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 15/15] kms_panel_fitting: Add tests for fastboot Maarten Lankhorst
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
igt_output_set_scaling_mode is a simple way to update scaling mode,
igt_display_(try_)commit_atomic will allow you to call drmAtomicCommit
with custom flags, like TEST_ONLY, EVENT, NONBLOCKING or without
ALLOW_MODESET.
This is useful when you only want to do any of those things, for events
it can be useful to set the user_data pointer too, so export that to
users.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
lib/igt_kms.c | 113 +++++++++++++++++++++++++++++++++++++++++++---------------
lib/igt_kms.h | 3 ++
2 files changed, 87 insertions(+), 29 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 17865c263f35..681f2bb41b07 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -867,7 +867,9 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
goto err3;
if (!connector->count_modes) {
- igt_warn("connector %d has no modes\n", connector_id);
+ igt_warn("connector %d/%s-%d has no modes\n", connector_id,
+ kmstest_connector_type_str(connector->connector_type),
+ connector->connector_type_id);
goto err3;
}
@@ -2125,7 +2127,7 @@ static void igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
* Commit all the changes of all the planes,crtcs, connectors
* atomically using drmModeAtomicCommit()
*/
-static int igt_atomic_commit(igt_display_t *display)
+static int igt_atomic_commit(igt_display_t *display, uint32_t flags, void *user_data)
{
int ret = 0, i;
@@ -2166,11 +2168,51 @@ static int igt_atomic_commit(igt_display_t *display)
igt_atomic_prepare_connector_commit(output, req);
}
- ret = drmModeAtomicCommit(display->drm_fd, req, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+ ret = drmModeAtomicCommit(display->drm_fd, req, flags, user_data);
drmModeAtomicFree(req);
return ret;
}
+
+static void
+display_commit_changed(igt_display_t *display, enum igt_commit_style s)
+{
+ int i;
+ enum pipe pipe;
+
+ for_each_pipe(display, pipe) {
+ igt_pipe_t *pipe_obj = &display->pipes[pipe];
+ igt_plane_t *plane;
+
+ if (s == COMMIT_ATOMIC) {
+ pipe_obj->color_mgmt_changed = false;
+ pipe_obj->background_changed = false;
+ }
+
+ if (s != COMMIT_UNIVERSAL)
+ pipe_obj->mode_changed = false;
+
+ for_each_plane_on_pipe(display, pipe, plane) {
+ plane->fb_changed = false;
+ plane->position_changed = false;
+ plane->size_changed = false;
+
+ if (s != COMMIT_LEGACY || !(plane->is_primary || plane->is_cursor))
+ plane->rotation_changed = false;
+ }
+ }
+
+ for (i = 0; i < display->n_outputs; i++) {
+ igt_output_t *output = &display->outputs[i];
+
+ if (s != COMMIT_UNIVERSAL)
+ output->config.pipe_changed = false;
+
+ if (s == COMMIT_ATOMIC)
+ output->config.connector_scaling_mode_changed = false;
+ }
+}
+
/*
* Commit all plane changes across all outputs of the display.
*
@@ -2184,14 +2226,14 @@ static int do_display_commit(igt_display_t *display,
enum igt_commit_style s,
bool fail_on_error)
{
- int i, ret;
+ int ret;
enum pipe pipe;
LOG_INDENT(display, "commit");
igt_display_refresh(display);
if (s == COMMIT_ATOMIC) {
- ret = igt_atomic_commit(display);
+ ret = igt_atomic_commit(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
CHECK_RETURN(ret, fail_on_error);
} else {
@@ -2222,43 +2264,42 @@ static int do_display_commit(igt_display_t *display,
if (ret)
return ret;
- for_each_pipe(display, pipe) {
- igt_pipe_t *pipe_obj = &display->pipes[pipe];
- igt_plane_t *plane;
+ display_commit_changed(display, s);
- if (s == COMMIT_ATOMIC) {
- pipe_obj->color_mgmt_changed = false;
- pipe_obj->background_changed = false;
- }
+ igt_debug_wait_for_keypress("modeset");
- if (s != COMMIT_UNIVERSAL)
- pipe_obj->mode_changed = false;
+ return 0;
+}
- for_each_plane_on_pipe(display, pipe, plane) {
- plane->fb_changed = false;
- plane->position_changed = false;
- plane->size_changed = false;
+int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data)
+{
+ int ret;
- if (s != COMMIT_LEGACY || !(plane->is_primary || plane->is_cursor))
- plane->rotation_changed = false;
- }
- }
+ LOG_INDENT(display, "commit");
- for (i = 0; i < display->n_outputs; i++) {
- igt_output_t *output = &display->outputs[i];
+ igt_display_refresh(display);
- if (s != COMMIT_UNIVERSAL)
- output->config.pipe_changed = false;
+ ret = igt_atomic_commit(display, flags, user_data);
- if (s == COMMIT_ATOMIC)
- output->config.connector_scaling_mode_changed = false;
- }
+ LOG_UNINDENT(display);
+
+ if (ret || (flags & DRM_MODE_ATOMIC_TEST_ONLY))
+ return ret;
+
+ display_commit_changed(display, COMMIT_ATOMIC);
igt_debug_wait_for_keypress("modeset");
return 0;
}
+void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data)
+{
+ int ret = igt_display_try_commit_atomic(display, flags, user_data);
+
+ igt_assert_eq(ret, 0);
+}
+
/**
* igt_display_commit2:
* @display: DRM device handle
@@ -2380,6 +2421,15 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe pipe)
output->config.pipe_changed = true;
}
+void igt_output_set_scaling_mode(igt_output_t *output, uint64_t scaling_mode)
+{
+ output->config.connector_scaling_mode_changed = true;
+
+ output->config.connector_scaling_mode = scaling_mode;
+
+ igt_require(output->config.atomic_props_connector[IGT_CONNECTOR_SCALING_MODE]);
+}
+
igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane)
{
igt_pipe_t *pipe;
@@ -2411,6 +2461,11 @@ void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb)
plane->src_w = fb->width;
plane->src_h = fb->height;
} else {
+ plane->src_x = 0;
+ plane->src_y = 0;
+ plane->src_w = 0;
+ plane->src_h = 0;
+
plane->crtc_w = 0;
plane->crtc_h = 0;
}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 39e3b9fa1972..bc7825a3f06d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -303,6 +303,8 @@ void igt_display_init(igt_display_t *display, int drm_fd);
void igt_display_fini(igt_display_t *display);
int igt_display_commit2(igt_display_t *display, enum igt_commit_style s);
int igt_display_commit(igt_display_t *display);
+int igt_display_try_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data);
+void igt_display_commit_atomic(igt_display_t *display, uint32_t flags, void *user_data);
int igt_display_try_commit2(igt_display_t *display, enum igt_commit_style s);
int igt_display_get_n_pipes(igt_display_t *display);
@@ -310,6 +312,7 @@ const char *igt_output_name(igt_output_t *output);
drmModeModeInfo *igt_output_get_mode(igt_output_t *output);
void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode);
void igt_output_set_pipe(igt_output_t *output, enum pipe pipe);
+void igt_output_set_scaling_mode(igt_output_t *output, uint64_t scaling_mode);
igt_plane_t *igt_output_get_plane(igt_output_t *output, enum igt_plane plane);
bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
uint32_t *prop_id, uint64_t *value,
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 14/15] igt_kms: Allow disabling previous override mode
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (12 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 13/15] igt_kms: Add more apis for panel fitting test Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 15/15] kms_panel_fitting: Add tests for fastboot Maarten Lankhorst
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
---
lib/igt_kms.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 681f2bb41b07..263f32883beb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2378,7 +2378,7 @@ drmModeModeInfo *igt_output_get_mode(igt_output_t *output)
/**
* igt_output_override_mode:
* @output: Output of which the mode will be overridden
- * @mode: New mode
+ * @mode: New mode, or NULL to disable override.
*
* Overrides the output's mode with @mode, so that it is used instead of the
* mode obtained with get connectors. Note that the mode is used without
@@ -2388,8 +2388,14 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
{
igt_pipe_t *pipe = igt_output_get_driving_pipe(output);
- output->override_mode = *mode;
- output->use_override_mode = true;
+ if (mode)
+ output->override_mode = *mode;
+ else /* restore default_mode, may have been overwritten in igt_output_refresh */
+ kmstest_get_connector_default_mode(output->display->drm_fd,
+ output->config.connector,
+ &output->config.default_mode);
+
+ output->use_override_mode = !!mode;
if (pipe)
pipe->mode_changed = true;
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH i-g-t v2 15/15] kms_panel_fitting: Add tests for fastboot.
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
` (13 preceding siblings ...)
2016-07-06 9:55 ` [PATCH i-g-t v2 14/15] igt_kms: Allow disabling previous override mode Maarten Lankhorst
@ 2016-07-06 9:55 ` Maarten Lankhorst
14 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-06 9:55 UTC (permalink / raw)
To: intel-gfx
This is a small testcase to ensure fastboot works as intended.
When I removed the pipe updates in intel_update_pipe_config
the test started failing, so I think it's useful to keep.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
tests/kms_panel_fitting.c | 101 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 94 insertions(+), 7 deletions(-)
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index da09da32d704..acc51ec5cc88 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -24,7 +24,7 @@
#include "igt.h"
#include <math.h>
-
+#include <sys/stat.h>
IGT_TEST_DESCRIPTION("Test display panel fitting");
@@ -214,16 +214,103 @@ static void test_panel_fitting(data_t *d)
igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
}
-igt_simple_main
+static void
+test_panel_fitting_fastset(igt_display_t *display, const enum pipe pipe, igt_output_t *output)
+{
+ igt_plane_t *primary, *sprite;
+ drmModeModeInfo mode;
+ igt_crc_t topleft, cur_crc;
+ igt_pipe_crc_t *pipe_crc;
+
+ struct igt_fb black, red;
+
+ igt_assert(kmstest_get_connector_default_mode(display->drm_fd, output->config.connector, &mode));
+
+ igt_output_override_mode(output, &mode);
+ igt_output_set_pipe(output, pipe);
+
+ primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+ sprite = igt_output_get_plane(output, IGT_PLANE_2);
+
+ igt_create_color_fb(display->drm_fd, mode.hdisplay, mode.vdisplay,
+ DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+ 0.f, 0.f, 0.f, &black);
+
+ igt_create_color_fb(display->drm_fd, 640, 480,
+ DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+ 1.f, 0.f, 0.f, &red);
+
+ igt_plane_set_fb(primary, &black);
+ igt_plane_set_fb(sprite, &red);
+
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ pipe_crc = igt_pipe_crc_new(pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+ igt_pipe_crc_collect_crc(pipe_crc, &topleft);
+
+ mode.hdisplay = 640;
+ mode.vdisplay = 480;
+ igt_output_override_mode(output, &mode);
+
+ igt_plane_set_fb(sprite, NULL);
+ igt_plane_set_fb(primary, &red);
+
+ /* Don't pass ALLOW_MODESET with overridden mode, force fastset. */
+ igt_display_commit_atomic(display, 0, NULL);
+
+ igt_pipe_crc_collect_crc(pipe_crc, &cur_crc);
+
+ igt_assert(!igt_crc_equal(&topleft, &cur_crc));
+
+ igt_plane_set_fb(primary, NULL);
+ igt_output_set_pipe(output, PIPE_NONE);
+ igt_output_override_mode(output, NULL);
+
+ igt_pipe_crc_free(pipe_crc);
+}
+
+static void test_atomic_fastset(igt_display_t *display)
+{
+ igt_output_t *output;
+ enum pipe pipe;
+ int valid_tests = 0;
+ struct stat sb;
+
+ /* Until this is force enabled, force modeset evasion. */
+ if (stat("/sys/module/i915/parameters/fastboot", &sb) == 0)
+ igt_set_module_param_int("fastboot", 1);
+
+ igt_require_pipe_crc();
+ igt_require(display->is_atomic);
+ igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5);
+
+ for_each_pipe_with_valid_output(display, pipe, output) {
+ if (!output->config.atomic_props_connector[IGT_CONNECTOR_SCALING_MODE])
+ continue;
+
+ test_panel_fitting_fastset(display, pipe, output);
+ valid_tests++;
+ }
+ igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
+}
+
+igt_main
{
data_t data = {};
- igt_skip_on_simulation();
+ igt_fixture {
+ igt_skip_on_simulation();
+
+ data.drm_fd = drm_open_driver(DRIVER_ANY);
+ igt_display_init(&data.display, data.drm_fd);
+ }
- data.drm_fd = drm_open_driver(DRIVER_ANY);
- igt_display_init(&data.display, data.drm_fd);
+ igt_subtest("legacy")
+ test_panel_fitting(&data);
- test_panel_fitting(&data);
+ igt_subtest("atomic-fastset")
+ test_atomic_fastset(&data.display);
- igt_display_fini(&data.display);
+ igt_fixture
+ igt_display_fini(&data.display);
}
--
2.5.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx
2016-07-06 9:55 ` [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx Maarten Lankhorst
@ 2016-07-13 12:13 ` Daniel Vetter
2016-07-19 12:52 ` Maarten Lankhorst
0 siblings, 1 reply; 32+ messages in thread
From: Daniel Vetter @ 2016-07-13 12:13 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Wed, Jul 06, 2016 at 11:55:41AM +0200, Maarten Lankhorst wrote:
> This is the same as using config.pipe because the order of crtcs will
> never change.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
In the interest of generic igt, I'm somewhat inclined to instead nuke
crtc->pipe (it's an Intelism) instead of crtc->idx. I also thought there's
some work from Robert Foss (still uncommented) to reorganize this.
-Daniel
> ---
> lib/igt_kms.c | 4 +---
> lib/igt_kms.h | 1 -
> tests/testdisplay.c | 4 +---
> 3 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 8f9ac2da43ff..c16e40ea273b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -854,9 +854,7 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t connector_id,
> config->connector = connector;
> config->encoder = found;
> config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[pipe]);
> - config->crtc_idx = pipe;
> - config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
> - config->crtc->crtc_id);
> + config->pipe = pipe;
>
> drmModeFreeResources(resources);
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 829615d70874..4882075430c8 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -118,7 +118,6 @@ struct kmstest_connector_config {
> 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;
> unsigned valid_crtc_idx_mask;
> };
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index 45280e4cad82..a974f42be9f1 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -112,7 +112,6 @@ struct connector {
> drmModeEncoder *encoder;
> drmModeConnector *connector;
> int crtc;
> - int crtc_idx;
> int pipe;
> };
>
> @@ -211,7 +210,6 @@ static void connector_find_preferred_mode(uint32_t connector_id,
> c->connector = config.connector;
> c->encoder = config.encoder;
> c->crtc = config.crtc->crtc_id;
> - c->crtc_idx = config.crtc_idx;
> c->pipe = config.pipe;
>
> if (mode_num != -1) {
> @@ -497,7 +495,7 @@ int update_display(bool probe)
>
> if (test_preferred_mode || force_mode ||
> specified_mode_num != -1)
> - crtc_idx_mask &= ~(1 << connector->crtc_idx);
> + crtc_idx_mask &= ~(1 << connector->pipe);
>
> }
> }
> --
> 2.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe
2016-07-06 9:55 ` [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe Maarten Lankhorst
@ 2016-07-15 11:14 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-15 11:14 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> This will allow us to find a matching encoder based on a pipe only.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> lib/igt_kms.c | 97 +++++++++++++++++++++++++++++++++++++---------------------
> -
> 1 file changed, 61 insertions(+), 36 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index c16e40ea273b..ce8aa2455348 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -761,6 +761,59 @@ bool kmstest_get_connector_default_mode(int drm_fd,
> drmModeConnector *connector,
> return true;
> }
>
> +static void
> +_kmstest_connector_config_crtc_mask(int drm_fd,
> + drmModeConnector *connector,
> + struct kmstest_connector_config *config)
> +{
> + int i;
> +
> + config->valid_crtc_idx_mask = 0;
> +
> + /* Now get a compatible encoder */
> + for (i = 0; i < connector->count_encoders; i++) {
> + drmModeEncoder *encoder = drmModeGetEncoder(drm_fd,
> + connector-
> >encoders[i]);
> +
> + if (!encoder) {
> + igt_warn("could not get encoder %d: %s\n",
> + connector->encoders[i],
> + strerror(errno));
> +
> + continue;
> + }
> +
> + config->valid_crtc_idx_mask |= encoder->possible_crtcs;
> + drmModeFreeEncoder(encoder);
> + }
> +}
> +
> +static drmModeEncoder *
> +_kmstest_connector_config_find_encoder(int drm_fd, drmModeConnector
> *connector, enum pipe pipe)
> +{
> + int i;
> +
> + for (i = 0; i < connector->count_encoders; i++) {
> + drmModeEncoder *encoder = drmModeGetEncoder(drm_fd,
> connector->encoders[i]);
> +
> + if (!encoder) {
> + igt_warn("could not get encoder %d: %s\n",
> + connector->encoders[i],
> + strerror(errno));
> +
> + continue;
> + }
> +
> + if (encoder->possible_crtcs & (1 << pipe))
> + return encoder;
> +
> + drmModeFreeEncoder(encoder);
> + }
> +
> + igt_assert(false);
> + return NULL;
> +}
> +
> /**
> * _kmstest_connector_config:
> * @drm_fd: DRM fd
> @@ -779,8 +832,6 @@ static bool _kmstest_connector_config(int drm_fd, uint32_t
> connector_id,
> {
> drmModeRes *resources;
> drmModeConnector *connector;
> - drmModeEncoder *encoder, *found = NULL;
> - int i, j, pipe;
>
> resources = drmModeGetResources(drm_fd);
> if (!resources) {
> @@ -816,51 +867,25 @@ static bool _kmstest_connector_config(int drm_fd,
> uint32_t connector_id,
> * In both cases find the first compatible encoder and skip the CRTC
> * if there is non such.
> */
> - config->valid_crtc_idx_mask = 0;
> - for (i = 0; i < resources->count_crtcs; i++) {
> - if (!resources->crtcs[i])
> - continue;
This patch looses the NULL check above, but if I understand correctly this can't
happen anyway, since we would have bailed on a GetResources failure. So,
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> + _kmstest_connector_config_crtc_mask(drm_fd, connector, config);
>
> - /* Now get a compatible encoder */
> - for (j = 0; j < connector->count_encoders; j++) {
> - encoder = drmModeGetEncoder(drm_fd,
> - connector->encoders[j]);
> -
> - if (!encoder) {
> - igt_warn("could not get encoder %d: %s\n",
> - resources->encoders[j],
> - strerror(errno));
> -
> - continue;
> - }
> -
> - config->valid_crtc_idx_mask |= encoder-
> >possible_crtcs;
> -
> - if (!found && (crtc_idx_mask & encoder-
> >possible_crtcs & (1 << i))) {
> - found = encoder;
> - pipe = i;
> - } else
> - drmModeFreeEncoder(encoder);
> - }
> - }
> -
> - if (!found)
> + crtc_idx_mask &= config->valid_crtc_idx_mask;
> + if (!crtc_idx_mask)
> goto err3;
>
> + config->pipe = ffs(crtc_idx_mask) - 1;
> +
> if (!kmstest_get_connector_default_mode(drm_fd, connector,
> &config->default_mode))
> - goto err4;
> + goto err3;
>
> + config->encoder = _kmstest_connector_config_find_encoder(drm_fd,
> connector, config->pipe);
> config->connector = connector;
> - config->encoder = found;
> - config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[pipe]);
> - config->pipe = pipe;
> + config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[config-
> >pipe]);
>
> drmModeFreeResources(resources);
>
> return true;
> -err4:
> - drmModeFreeEncoder(found);
> err3:
> drmModeFreeConnector(connector);
> err2:
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config.
2016-07-06 9:55 ` [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config Maarten Lankhorst
@ 2016-07-15 11:15 ` Ander Conselvan De Oliveira
2016-07-19 13:58 ` Ander Conselvan De Oliveira
0 siblings, 1 reply; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-15 11:15 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> This is the only time PIPE_ANY was used to mean something other than
> unassign this output from a pipe. Without this PIPE_ANY can be aliased
> to PIPE_NONE.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> ---
> tests/kms_psr_sink_crc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index b18e426303e3..d7bce3bb7855 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -103,15 +103,16 @@ static void setup_output(data_t *data)
> {
> igt_display_t *display = &data->display;
> igt_output_t *output;
> + enum pipe pipe;
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
> drmModeConnectorPtr c = output->config.connector;
>
> if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> c->connection != DRM_MODE_CONNECTED)
> continue;
>
> - igt_output_set_pipe(output, PIPE_ANY);
> + igt_output_set_pipe(output, pipe);
> data->crtc_id = output->config.crtc->crtc_id;
> data->output = output;
> data->mode = igt_output_get_mode(output);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx
2016-07-13 12:13 ` Daniel Vetter
@ 2016-07-19 12:52 ` Maarten Lankhorst
0 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-19 12:52 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
Hey,
Op 13-07-16 om 14:13 schreef Daniel Vetter:
> On Wed, Jul 06, 2016 at 11:55:41AM +0200, Maarten Lankhorst wrote:
>> This is the same as using config.pipe because the order of crtcs will
>> never change.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> In the interest of generic igt, I'm somewhat inclined to instead nuke
> crtc->pipe (it's an Intelism) instead of crtc->idx. I also thought there's
> some work from Robert Foss (still uncommented) to reorganize this.
config->pipe is mostly used as argument to kmstest_pipe_name, so I think this commit is fine.
it's named display->pipes in the tests, not display->crtcs.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config.
2016-07-15 11:15 ` Ander Conselvan De Oliveira
@ 2016-07-19 13:58 ` Ander Conselvan De Oliveira
2016-07-20 7:53 ` Maarten Lankhorst
0 siblings, 1 reply; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-19 13:58 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Fri, 2016-07-15 at 14:15 +0300, Ander Conselvan De Oliveira wrote:
> On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> >
> > This is the only time PIPE_ANY was used to mean something other than
> > unassign this output from a pipe. Without this PIPE_ANY can be aliased
> > to PIPE_NONE.
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Actually, kms_sink_crc_basic needs the same treatment.
Ander
>
> >
> > ---
> > tests/kms_psr_sink_crc.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index b18e426303e3..d7bce3bb7855 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -103,15 +103,16 @@ static void setup_output(data_t *data)
> > {
> > igt_display_t *display = &data->display;
> > igt_output_t *output;
> > + enum pipe pipe;
> >
> > - for_each_connected_output(display, output) {
> > + for_each_pipe_with_valid_output(display, pipe, output) {
> > drmModeConnectorPtr c = output->config.connector;
> >
> > if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> > c->connection != DRM_MODE_CONNECTED)
> > continue;
> >
> > - igt_output_set_pipe(output, PIPE_ANY);
> > + igt_output_set_pipe(output, pipe);
> > data->crtc_id = output->config.crtc->crtc_id;
> > data->output = output;
> > data->mode = igt_output_get_mode(output);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config.
2016-07-19 13:58 ` Ander Conselvan De Oliveira
@ 2016-07-20 7:53 ` Maarten Lankhorst
2016-07-20 12:17 ` Ander Conselvan De Oliveira
0 siblings, 1 reply; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-20 7:53 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
Op 19-07-16 om 15:58 schreef Ander Conselvan De Oliveira:
> On Fri, 2016-07-15 at 14:15 +0300, Ander Conselvan De Oliveira wrote:
>> On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
>>> This is the only time PIPE_ANY was used to mean something other than
>>> unassign this output from a pipe. Without this PIPE_ANY can be aliased
>>> to PIPE_NONE.
>>>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Actually, kms_sink_crc_basic needs the same treatment.
Indeed, how does this fixup look to the patch? Will probably send kms_sink_crc_basic out as separate patch.
The DRM_MODE_CONNECTED check is implied by for_each_connected_output and for_each_pipe_with_valid_output,
it can be killed.
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index d7bce3bb7855..8aafedb03614 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -108,8 +108,7 @@ static void setup_output(data_t *data)
for_each_pipe_with_valid_output(display, pipe, output) {
drmModeConnectorPtr c = output->config.connector;
- if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
- c->connection != DRM_MODE_CONNECTED)
+ if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
continue;
igt_output_set_pipe(output, pipe);
And for kms_sink_crc_basic:
diff --git a/tests/kms_sink_crc_basic.c b/tests/kms_sink_crc_basic.c
index 9fac958f2142..c332eb1e39c2 100644
--- a/tests/kms_sink_crc_basic.c
+++ b/tests/kms_sink_crc_basic.c
@@ -112,15 +112,15 @@ static void run_test(data_t *data)
igt_display_t *display = &data->display;
igt_output_t *output;
drmModeModeInfo *mode;
+ enum pipe pipe;
- for_each_connected_output(display, output) {
+ for_each_pipe_with_valid_output(display, pipe, output) {
drmModeConnectorPtr c = output->config.connector;
- if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
- c->connection != DRM_MODE_CONNECTED)
+ if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
continue;
- igt_output_set_pipe(output, PIPE_ANY);
+ igt_output_set_pipe(output, pipe);
mode = igt_output_get_mode(output);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config.
2016-07-20 7:53 ` Maarten Lankhorst
@ 2016-07-20 12:17 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-20 12:17 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-20 at 09:53 +0200, Maarten Lankhorst wrote:
> Op 19-07-16 om 15:58 schreef Ander Conselvan De Oliveira:
> >
> > On Fri, 2016-07-15 at 14:15 +0300, Ander Conselvan De Oliveira wrote:
> > >
> > > On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> > > >
> > > > This is the only time PIPE_ANY was used to mean something other than
> > > > unassign this output from a pipe. Without this PIPE_ANY can be aliased
> > > > to PIPE_NONE.
> > > >
> > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> > Actually, kms_sink_crc_basic needs the same treatment.
> Indeed, how does this fixup look to the patch? Will probably send
> kms_sink_crc_basic out as separate patch.
> The DRM_MODE_CONNECTED check is implied by for_each_connected_output and
> for_each_pipe_with_valid_output,
> it can be killed.
True, kmstest_get_connector_config() already tests for that, so it is enough to
check output->valid.
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index d7bce3bb7855..8aafedb03614 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -108,8 +108,7 @@ static void setup_output(data_t *data)
> for_each_pipe_with_valid_output(display, pipe, output) {
> drmModeConnectorPtr c = output->config.connector;
>
> - if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> - c->connection != DRM_MODE_CONNECTED)
> + if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> continue;
>
> igt_output_set_pipe(output, pipe);
>
> And for kms_sink_crc_basic:
>
> diff --git a/tests/kms_sink_crc_basic.c b/tests/kms_sink_crc_basic.c
> index 9fac958f2142..c332eb1e39c2 100644
> --- a/tests/kms_sink_crc_basic.c
> +++ b/tests/kms_sink_crc_basic.c
> @@ -112,15 +112,15 @@ static void run_test(data_t *data)
> igt_display_t *display = &data->display;
> igt_output_t *output;
> drmModeModeInfo *mode;
> + enum pipe pipe;
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
> drmModeConnectorPtr c = output->config.connector;
>
> - if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
> - c->connection != DRM_MODE_CONNECTED)
> + if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> continue;
>
> - igt_output_set_pipe(output, PIPE_ANY);
> + igt_output_set_pipe(output, pipe);
>
> mode = igt_output_get_mode(output);
Both look good. Feel free to slap my R-b on it.
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes.
2016-07-06 9:55 ` [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes Maarten Lankhorst
@ 2016-07-20 12:56 ` Ander Conselvan De Oliveira
2016-07-21 9:21 ` Maarten Lankhorst
2016-07-25 13:04 ` Maarten Lankhorst
0 siblings, 2 replies; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-20 12:56 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> Use for_each_pipe_with_valid_output instead.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> tests/kms_crtc_background_color.c | 3 +--
> tests/kms_flip_tiling.c | 50 +++++++++++++++++++++++---------------
> -
> tests/kms_panel_fitting.c | 5 ++--
> tests/kms_plane_scaling.c | 5 ++--
> 4 files changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/tests/kms_crtc_background_color.c
> b/tests/kms_crtc_background_color.c
> index b496625c1693..b97c1142df6e 100644
> --- a/tests/kms_crtc_background_color.c
> +++ b/tests/kms_crtc_background_color.c
> @@ -133,10 +133,9 @@ static void test_crtc_background(data_t *data)
>
> igt_require(data->display.has_universal_planes);
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
> igt_plane_t *plane;
>
> - pipe = output->config.pipe;
> igt_output_set_pipe(output, pipe);
Won't this cause the test to be executed more times? I.e., every output paired
with every supported pipe vs. every output only with the pipe it is currently
configured to use.
None of the changed tests are part of BAT, so I'm not sure we care about a
potential increase in execution time, though. But maybe add a note in the commit
message for future reference?
Ander
>
> plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
> diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
> index f58e65be61ee..cd2f510f1d45 100644
> --- a/tests/kms_flip_tiling.c
> +++ b/tests/kms_flip_tiling.c
> @@ -80,16 +80,15 @@ static void wait_for_pageflip(int fd)
> }
>
> static void
> -test_flip_tiling(data_t *data, igt_output_t *output, uint64_t tiling[2])
> +test_flip_tiling(data_t *data, enum pipe pipe, igt_output_t *output, uint64_t
> tiling[2])
> {
> drmModeModeInfo *mode;
> igt_plane_t *primary;
> struct igt_fb fb[2];
> igt_pipe_crc_t *pipe_crc;
> igt_crc_t reference_crc, crc;
> - int fb_id, pipe, ret, width;
> + int fb_id, ret, width;
>
> - pipe = output->config.pipe;
> pipe_crc = pipe_crc_new(pipe);
> igt_output_set_pipe(output, pipe);
>
> @@ -184,31 +183,34 @@ igt_main
> igt_subtest_f("flip-changes-tiling") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
> LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-changes-tiling-Y") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
> LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-changes-tiling-Yf") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
> LOCAL_DRM_FORMAT_MOD_NONE };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> /*
> @@ -222,31 +224,34 @@ igt_main
> igt_subtest_f("flip-X-tiled") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_X_TILED,
> LOCAL_I915_FORMAT_MOD_X_TILED };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-Y-tiled") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Y_TILED,
> LOCAL_I915_FORMAT_MOD_Y_TILED };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-Yf-tiled") {
> uint64_t tiling[2] = { LOCAL_I915_FORMAT_MOD_Yf_TILED,
> LOCAL_I915_FORMAT_MOD_Yf_TILED };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> /*
> @@ -260,31 +265,34 @@ igt_main
> igt_subtest_f("flip-to-X-tiled") {
> uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
> LOCAL_I915_FORMAT_MOD_X_TILED };
> + enum pipe pipe;
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-to-Y-tiled") {
> uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
> LOCAL_I915_FORMAT_MOD_Y_TILED };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_subtest_f("flip-to-Yf-tiled") {
> uint64_t tiling[2] = { LOCAL_DRM_FORMAT_MOD_NONE,
> LOCAL_I915_FORMAT_MOD_Yf_TILED };
> + enum pipe pipe;
>
> igt_require_fb_modifiers(data.drm_fd);
> igt_require(data.gen >= 9);
>
> - for_each_connected_output(&data.display, output)
> - test_flip_tiling(&data, output, tiling);
> + for_each_pipe_with_valid_output(&data.display, pipe, output)
> + test_flip_tiling(&data, pipe, output, tiling);
> }
>
> igt_fixture {
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index b796c6812679..7c501fcdd3fb 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -87,7 +87,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
> if (s == COMMIT_LEGACY) {
> int ret;
> ret = drmModeSetCrtc(data->drm_fd,
> - output->config.crtc->crtc_id,
> + plane->pipe->crtc_id,
> data->fb_id1,
> plane->pan_x, plane->pan_y,
> &output->id,
> @@ -137,7 +137,7 @@ static void test_panel_fitting(data_t *d)
> enum pipe pipe;
> int valid_tests = 0;
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
> drmModeModeInfo *mode, native_mode;
> bool scaling_mode_set;
>
> @@ -153,7 +153,6 @@ static void test_panel_fitting(data_t *d)
> if (!scaling_mode_set)
> continue;
>
> - pipe = output->config.pipe;
> igt_output_set_pipe(output, pipe);
>
> mode = igt_output_get_mode(output);
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index ad5404d90bfa..39bb5e113411 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -96,7 +96,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
> if (s == COMMIT_LEGACY) {
> int ret;
> ret = drmModeSetCrtc(data->drm_fd,
> - output->config.crtc->crtc_id,
> + plane->pipe->crtc_id,
> data->fb_id1,
> plane->pan_x, plane->pan_y,
> &output->id,
> @@ -186,10 +186,9 @@ static void test_plane_scaling(data_t *d)
> igt_require(d->display.has_universal_planes);
> igt_require(d->num_scalers);
>
> - for_each_connected_output(display, output) {
> + for_each_pipe_with_valid_output(display, pipe, output) {
> drmModeModeInfo *mode;
>
> - pipe = output->config.pipe;
> igt_output_set_pipe(output, pipe);
>
> mode = igt_output_get_mode(output);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes.
2016-07-20 12:56 ` Ander Conselvan De Oliveira
@ 2016-07-21 9:21 ` Maarten Lankhorst
2016-07-25 13:04 ` Maarten Lankhorst
1 sibling, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-21 9:21 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
Op 20-07-16 om 14:56 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
>> Use for_each_pipe_with_valid_output instead.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> tests/kms_crtc_background_color.c | 3 +--
>> tests/kms_flip_tiling.c | 50 +++++++++++++++++++++++---------------
>> -
>> tests/kms_panel_fitting.c | 5 ++--
>> tests/kms_plane_scaling.c | 5 ++--
>> 4 files changed, 34 insertions(+), 29 deletions(-)
>>
>> diff --git a/tests/kms_crtc_background_color.c
>> b/tests/kms_crtc_background_color.c
>> index b496625c1693..b97c1142df6e 100644
>> --- a/tests/kms_crtc_background_color.c
>> +++ b/tests/kms_crtc_background_color.c
>> @@ -133,10 +133,9 @@ static void test_crtc_background(data_t *data)
>>
>> igt_require(data->display.has_universal_planes);
>>
>> - for_each_connected_output(display, output) {
>> + for_each_pipe_with_valid_output(display, pipe, output) {
>> igt_plane_t *plane;
>>
>> - pipe = output->config.pipe;
>> igt_output_set_pipe(output, pipe);
> Won't this cause the test to be executed more times? I.e., every output paired
> with every supported pipe vs. every output only with the pipe it is currently
> configured to use.
>
> None of the changed tests are part of BAT, so I'm not sure we care about a
> potential increase in execution time, though. But maybe add a note in the commit
> message for future reference?
It will, but it will also increase test coverage by running the test on each crtc. If we only cared for the
test to run with a single combination, we should have just added break at the end.
Good idea to put it in the commit message though, would be useful.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned
2016-07-06 9:55 ` [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned Maarten Lankhorst
@ 2016-07-21 9:23 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-21 9:23 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> None of the tests requires that a output bound to PIPE_ANY is assigned,
> so don't do it. Fix the display commit to iterate over crtc's instead
> of outputs to properly disable pipes without outputs.
>
> This also means that output->valid is only set after connecting a
> output to a pipe, so no longer depend on it in for_each_connected_output
> and similar macros.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> lib/igt_kms.c | 246 ++++++++++++++++++++++++++++-----------------------------
> -
> lib/igt_kms.h | 18 ++++-
> 2 files changed, 135 insertions(+), 129 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index ce8aa2455348..88cae7d51787 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -869,18 +869,20 @@ static bool _kmstest_connector_config(int drm_fd,
> uint32_t connector_id,
> */
> _kmstest_connector_config_crtc_mask(drm_fd, connector, config);
>
> + if (!kmstest_get_connector_default_mode(drm_fd, connector,
> + &config->default_mode))
> + goto err3;
> +
> + config->connector = connector;
> +
> crtc_idx_mask &= config->valid_crtc_idx_mask;
> if (!crtc_idx_mask)
> - goto err3;
> + /* Keep config->connector */
> + goto err2;
>
> config->pipe = ffs(crtc_idx_mask) - 1;
>
> - if (!kmstest_get_connector_default_mode(drm_fd, connector,
> - &config->default_mode))
> - goto err3;
> -
> config->encoder = _kmstest_connector_config_find_encoder(drm_fd,
> connector, config->pipe);
> - config->connector = connector;
> config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[config-
> >pipe]);
>
> drmModeFreeResources(resources);
> @@ -940,8 +942,13 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t
> connector_id,
> void kmstest_free_connector_config(struct kmstest_connector_config *config)
> {
> drmModeFreeCrtc(config->crtc);
> + config->crtc = NULL;
> +
> drmModeFreeEncoder(config->encoder);
> + config->encoder = NULL;
> +
> drmModeFreeConnector(config->connector);
> + config->connector = NULL;
> }
>
> /**
> @@ -1197,8 +1204,7 @@ static void igt_output_refresh(igt_output_t *output)
> /* we mask out the pipes already in use */
> crtc_idx_mask = output->pending_crtc_idx_mask & ~display-
> >pipes_in_use;
>
> - if (output->valid)
> - kmstest_free_connector_config(&output->config);
> + kmstest_free_connector_config(&output->config);
>
> ret = kmstest_get_connector_config(display->drm_fd,
> output->id,
> @@ -1209,19 +1215,19 @@ static void igt_output_refresh(igt_output_t *output)
> else
> output->valid = false;
>
> - if (!output->valid)
> - return;
> -
> - if (output->use_override_mode)
> - output->config.default_mode = output->override_mode;
> -
> - if (!output->name) {
> + if (!output->name && output->config.connector) {
> drmModeConnector *c = output->config.connector;
>
> igt_assert_neq(asprintf(&output->name, "%s-%d",
> kmstest_connector_type_str(c->connector_type), c->connector_type_id),
> -1);
> }
>
> + if (!output->valid)
> + return;
> +
> + if (output->use_override_mode)
> + output->config.default_mode = output->override_mode;
> +
> LOG(display, "%s: Selecting pipe %s\n", output->name,
> kmstest_pipe_name(output->config.pipe));
>
> @@ -1259,10 +1265,10 @@ get_crtc_property(int drm_fd, uint32_t crtc_id, const
> char *name,
> }
>
> static void
> -igt_crtc_set_property(igt_output_t *output, uint32_t prop_id, uint64_t value)
> +igt_crtc_set_property(igt_pipe_t *pipe, uint32_t prop_id, uint64_t value)
> {
> - drmModeObjectSetProperty(output->display->drm_fd,
> - output->config.crtc->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id,
> value);
> + drmModeObjectSetProperty(pipe->display->drm_fd,
> + pipe->crtc_id, DRM_MODE_OBJECT_CRTC, prop_id, value);
> }
>
> /*
> @@ -1325,15 +1331,37 @@ void igt_display_init(igt_display_t *display, int
> drm_fd)
> int p = IGT_PLANE_2;
> int j, type;
> uint8_t n_planes = 0;
> + uint64_t prop_value;
>
> pipe->crtc_id = resources->crtcs[i];
> pipe->display = display;
> pipe->pipe = i;
>
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "background_color",
> + &pipe->background_property,
> + &prop_value,
> + NULL);
> + pipe->background = (uint32_t)prop_value;
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "DEGAMMA_LUT",
> + &pipe->degamma_property,
> + NULL,
> + NULL);
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "CTM",
> + &pipe->ctm_property,
> + NULL,
> + NULL);
> + get_crtc_property(display->drm_fd, pipe->crtc_id,
> + "GAMMA_LUT",
> + &pipe->gamma_property,
> + NULL,
> + NULL);
> +
I think it might be worth splitting the drm property related changes to a
separate patch. They seem fairly self contained and this is a somewhat long
patch.
> /* add the planes that can be used with that pipe */
> for (j = 0; j < plane_resources->count_planes; j++) {
> drmModePlane *drm_plane;
> - uint64_t prop_value;
>
> drm_plane = drmModeGetPlane(display->drm_fd,
> plane_resources-
> >planes[j]);
> @@ -1440,47 +1468,17 @@ void igt_display_init(igt_display_t *display, int
> drm_fd)
> igt_assert(display->outputs);
>
> for (i = 0; i < display->n_outputs; i++) {
> - int j;
> igt_output_t *output = &display->outputs[i];
>
> /*
> - * We're free to select any pipe to drive that output until
> - * a constraint is set with igt_output_set_pipe().
> + * We don't assign each output a pipe unless
> + * a pipe is set with igt_output_set_pipe().
> */
> - output->pending_crtc_idx_mask = -1UL;
> + output->pending_crtc_idx_mask = 0;
> output->id = resources->connectors[i];
> output->display = display;
>
> igt_output_refresh(output);
> -
> - for (j = 0; j < display->n_pipes; j++) {
> - uint64_t prop_value;
> - igt_pipe_t *pipe = &display->pipes[j];
> -
> - if (output->config.crtc) {
> - get_crtc_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> - "background_color",
> - &pipe-
> >background_property,
> - &prop_value,
> - NULL);
> - pipe->background = (uint32_t)prop_value;
> - get_crtc_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> - "DEGAMMA_LUT",
> - &pipe->degamma_property,
> - NULL,
> - NULL);
> - get_crtc_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> - "CTM",
> - &pipe->ctm_property,
> - NULL,
> - NULL);
> - get_crtc_property(display->drm_fd, output-
> >config.crtc->crtc_id,
> - "GAMMA_LUT",
> - &pipe->gamma_property,
> - NULL,
> - NULL);
> - }
> - }
> }
>
> drmModeFreePlaneResources(plane_resources);
> @@ -1545,7 +1543,7 @@ static void igt_display_refresh(igt_display_t *display)
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *a = &display->outputs[i];
>
> - if (a->pending_crtc_idx_mask == -1UL)
> + if (!a->pending_crtc_idx_mask)
> continue;
>
> for (j = 0; j < display->n_outputs; j++) {
> @@ -1554,9 +1552,6 @@ static void igt_display_refresh(igt_display_t *display)
> if (i == j)
> continue;
>
> - if (b->pending_crtc_idx_mask == -1UL)
> - continue;
> -
> igt_assert_f(a->pending_crtc_idx_mask !=
> b->pending_crtc_idx_mask,
> "%s and %s are both trying to use pipe
> %s\n",
> @@ -1565,25 +1560,9 @@ static void igt_display_refresh(igt_display_t *display)
> }
> }
>
> - /*
> - * The pipe allocation has to be done in two phases:
> - * - first, try to satisfy the outputs where a pipe has been
> specified
> - * - then, allocate the outputs with PIPE_ANY
> - */
> for (i = 0; i < display->n_outputs; i++) {
> igt_output_t *output = &display->outputs[i];
>
> - if (output->pending_crtc_idx_mask == -1UL)
> - continue;
> -
> - igt_output_refresh(output);
> - }
> - for (i = 0; i < display->n_outputs; i++) {
> - igt_output_t *output = &display->outputs[i];
> -
> - if (output->pending_crtc_idx_mask != -1UL)
> - continue;
> -
> igt_output_refresh(output);
> }
> }
> @@ -1593,12 +1572,11 @@ static igt_pipe_t
> *igt_output_get_driving_pipe(igt_output_t *output)
> igt_display_t *display = output->display;
> enum pipe pipe;
>
> - if (output->pending_crtc_idx_mask == -1UL) {
> + if (!output->pending_crtc_idx_mask) {
> /*
> - * The user hasn't specified a pipe to use, take the one
> - * configured by the last refresh()
> + * The user hasn't specified a pipe to use, return none.
> */
> - pipe = output->config.pipe;
> + return NULL;
> } else {
> /*
> * Otherwise, return the pending pipe (ie the pipe that
> should
> @@ -1628,6 +1606,21 @@ static igt_plane_t *igt_pipe_get_plane(igt_pipe_t
> *pipe, enum igt_plane plane)
> return &pipe->planes[idx];
> }
>
> +static igt_output_t *igt_pipe_get_output(igt_pipe_t *pipe)
> +{
> + igt_display_t *display = pipe->display;
> + int i;
> +
> + for (i = 0; i < display->n_outputs; i++) {
> + igt_output_t *output = &display->outputs[i];
> +
> + if (output->pending_crtc_idx_mask == (1 << pipe->pipe))
> + return output;
> + }
> +
> + return NULL;
> +}
> +
> bool igt_pipe_get_property(igt_pipe_t *pipe, const char *name,
> uint32_t *prop_id, uint64_t *value,
> drmModePropertyPtr *prop)
> @@ -1741,10 +1734,10 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane,
> igt_output_t *output,
> * tests that expect a specific error code).
> */
> static int igt_drm_plane_commit(igt_plane_t *plane,
> - igt_output_t *output,
> + igt_pipe_t *pipe,
> bool fail_on_error)
> {
> - igt_display_t *display = output->display;
> + igt_display_t *display = pipe->display;
> uint32_t fb_id, crtc_id;
> int ret;
> uint32_t src_x;
> @@ -1763,13 +1756,12 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
> !plane->rotation_changed);
>
> fb_id = igt_plane_get_fb_id(plane);
> - crtc_id = output->config.crtc->crtc_id;
> + crtc_id = pipe->crtc_id;
>
> if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
> LOG(display,
> - "%s: SetPlane pipe %s, plane %d, disabling\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> + "SetPlane pipe %s, plane %d, disabling\n",
> + kmstest_pipe_name(pipe->pipe),
> plane->index);
>
> ret = drmModeSetPlane(display->drm_fd,
> @@ -1797,10 +1789,9 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
> crtc_h = plane->crtc_h;
>
> LOG(display,
> - "%s: SetPlane %s.%d, fb %u, src = (%d, %d) "
> + "SetPlane %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),
> + kmstest_pipe_name(pipe->pipe),
> plane->index,
> fb_id,
> src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
> @@ -1841,11 +1832,11 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
> * code).
> */
> static int igt_cursor_commit_legacy(igt_plane_t *cursor,
> - igt_output_t *output,
> + igt_pipe_t *pipe,
> bool fail_on_error)
> {
> - igt_display_t *display = output->display;
> - uint32_t crtc_id = output->config.crtc->crtc_id;
> + igt_display_t *display = pipe->display;
> + uint32_t crtc_id = pipe->crtc_id;
> int ret;
>
> if (cursor->fb_changed) {
> @@ -1853,9 +1844,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
>
> if (gem_handle) {
> LOG(display,
> - "%s: SetCursor pipe %s, fb %u %dx%d\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> + "SetCursor pipe %s, fb %u %dx%d\n",
> + kmstest_pipe_name(pipe->pipe),
> gem_handle,
> cursor->crtc_w, cursor->crtc_h);
>
> @@ -1865,9 +1855,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
> cursor->crtc_h);
> } else {
> LOG(display,
> - "%s: SetCursor pipe %s, disabling\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe));
> + "SetCursor pipe %s, disabling\n",
> + kmstest_pipe_name(pipe->pipe));
>
> ret = drmModeSetCursor(display->drm_fd, crtc_id,
> 0, 0, 0);
> @@ -1883,9 +1872,8 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
> int y = cursor->crtc_y;
>
> LOG(display,
> - "%s: MoveCursor pipe %s, (%d, %d)\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> + "MoveCursor pipe %s, (%d, %d)\n",
> + kmstest_pipe_name(pipe->pipe),
> x, y);
>
> ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
> @@ -1902,10 +1890,11 @@ static int igt_cursor_commit_legacy(igt_plane_t
> *cursor,
> * (setmode).
> */
> static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
> - igt_output_t *output,
> + igt_pipe_t *pipe,
> bool fail_on_error)
> {
> struct igt_display *display = primary->pipe->display;
> + igt_output_t *output = igt_pipe_get_output(pipe);
> drmModeModeInfo *mode;
> uint32_t fb_id, crtc_id;
> int ret;
> @@ -1920,7 +1909,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> !primary->size_changed && !primary->panning_changed)
> return 0;
>
> - crtc_id = output->config.crtc->crtc_id;
> + crtc_id = pipe->crtc_id;
> fb_id = igt_plane_get_fb_id(primary);
> if (fb_id)
> mode = igt_output_get_mode(output);
> @@ -1932,7 +1921,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> "%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
> "mode %dx%d\n",
> igt_output_name(output),
Here you kept the output name in the log while it was removed in all other LOG()
instances. Should it be removed from here too for consistency?
Looks good otherwise.
Ander
> - kmstest_pipe_name(output->config.pipe),
> + kmstest_pipe_name(pipe->pipe),
> fb_id,
> primary->pan_x, primary->pan_y,
> mode->hdisplay, mode->vdisplay);
> @@ -1946,9 +1935,8 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> mode);
> } else {
> LOG(display,
> - "%s: SetCrtc pipe %s, disabling\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe));
> + "SetCrtc pipe %s, disabling\n",
> + kmstest_pipe_name(pipe->pipe));
>
> ret = drmModeSetCrtc(display->drm_fd,
> crtc_id,
> @@ -1976,17 +1964,17 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> * which API is used to do the programming.
> */
> static int igt_plane_commit(igt_plane_t *plane,
> - igt_output_t *output,
> + igt_pipe_t *pipe,
> enum igt_commit_style s,
> bool fail_on_error)
> {
> if (plane->is_cursor && s == COMMIT_LEGACY) {
> - return igt_cursor_commit_legacy(plane, output,
> fail_on_error);
> + return igt_cursor_commit_legacy(plane, pipe, fail_on_error);
> } else if (plane->is_primary && s == COMMIT_LEGACY) {
> - return igt_primary_plane_commit_legacy(plane, output,
> + return igt_primary_plane_commit_legacy(plane, pipe,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, output,
> fail_on_error);
> + return igt_drm_plane_commit(plane, pipe,
> fail_on_error);
> }
> }
>
> @@ -2000,31 +1988,28 @@ static int igt_plane_commit(igt_plane_t *plane,
> * further programming will take place, which may result in some changes
> * taking effect and others not taking effect.
> */
> -static int igt_output_commit(igt_output_t *output,
> - enum igt_commit_style s,
> - bool fail_on_error)
> +static int igt_pipe_commit(igt_pipe_t *pipe,
> + enum igt_commit_style s,
> + bool fail_on_error)
> {
> - igt_display_t *display = output->display;
> - igt_pipe_t *pipe;
> + igt_display_t *display = pipe->display;
> int i;
> int ret;
> bool need_wait_for_vblank = false;
>
> - pipe = igt_output_get_driving_pipe(output);
> -
> if (pipe->background_changed) {
> - igt_crtc_set_property(output, pipe->background_property,
> + igt_crtc_set_property(pipe, pipe->background_property,
> pipe->background);
>
> pipe->background_changed = false;
> }
>
> if (pipe->color_mgmt_changed) {
> - igt_crtc_set_property(output, pipe->degamma_property,
> + igt_crtc_set_property(pipe, pipe->degamma_property,
> pipe->degamma_blob);
> - igt_crtc_set_property(output, pipe->ctm_property,
> + igt_crtc_set_property(pipe, pipe->ctm_property,
> pipe->ctm_blob);
> - igt_crtc_set_property(output, pipe->gamma_property,
> + igt_crtc_set_property(pipe, pipe->gamma_property,
> pipe->gamma_blob);
>
> pipe->color_mgmt_changed = false;
> @@ -2036,7 +2021,7 @@ static int igt_output_commit(igt_output_t *output,
> if (plane->fb_changed || plane->position_changed || plane-
> >size_changed)
> need_wait_for_vblank = true;
>
> - ret = igt_plane_commit(plane, output, s, fail_on_error);
> + ret = igt_plane_commit(plane, pipe, s, fail_on_error);
> CHECK_RETURN(ret, fail_on_error);
> }
>
> @@ -2060,6 +2045,9 @@ static void igt_atomic_prepare_crtc_commit(igt_output_t
> *output, drmModeAtomicRe
>
> igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
>
> + if (!pipe_obj)
> + return;
> +
> if (pipe_obj->background_changed)
> igt_atomic_populate_crtc_req(req, output,
> IGT_CRTC_BACKGROUND, pipe_obj->background);
>
> @@ -2125,6 +2113,8 @@ static int igt_atomic_commit(igt_display_t *display)
>
>
> pipe_obj = igt_output_get_driving_pipe(output);
> + if (!pipe_obj)
> + continue;
>
> pipe = pipe_obj->pipe;
>
> @@ -2167,14 +2157,14 @@ static int do_display_commit(igt_display_t *display,
>
> }
>
> - for (i = 0; i < display->n_outputs; i++) {
> - igt_output_t *output = &display->outputs[i];
> + for_each_pipe(display, i) {
> + igt_pipe_t *pipe_obj = &display->pipes[i];
> + igt_output_t *output = igt_pipe_get_output(pipe_obj);
>
> - if (!output->valid)
> - continue;
> + if (output && output->valid)
> + valid_outs++;
>
> - valid_outs++;
> - ret = igt_output_commit(output, s, fail_on_error);
> + ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
> CHECK_RETURN(ret, fail_on_error);
> }
>
> @@ -2282,9 +2272,9 @@ void igt_output_set_pipe(igt_output_t *output, enum pipe
> pipe)
> {
> igt_display_t *display = output->display;
>
> - if (pipe == PIPE_ANY) {
> + if (pipe == PIPE_NONE) {
> LOG(display, "%s: set_pipe(any)\n", igt_output_name(output));
> - output->pending_crtc_idx_mask = -1UL;
> + output->pending_crtc_idx_mask = 0;
> } else {
> LOG(display, "%s: set_pipe(%s)\n", igt_output_name(output),
> kmstest_pipe_name(pipe));
> @@ -2297,6 +2287,8 @@ igt_plane_t *igt_output_get_plane(igt_output_t *output,
> enum igt_plane plane)
> igt_pipe_t *pipe;
>
> pipe = igt_output_get_driving_pipe(output);
> + igt_assert(pipe);
> +
> return igt_pipe_get_plane(pipe, plane);
> }
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index dc6be5e53b74..3531dc20b6e0 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -334,17 +334,31 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t
> *plane,
>
> void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
>
> +static inline bool igt_output_is_connected(igt_output_t *output)
> +{
> + /* Something went wrong during probe? */
> + if (!output->config.connector)
> + return false;
> +
> + if (output->config.connector->connection == DRM_MODE_CONNECTED)
> + return true;
> +
> + return false;
> +}
> +
> static inline bool igt_pipe_connector_valid(enum pipe pipe,
> igt_output_t *output)
> {
> - return output->valid && (output->config.valid_crtc_idx_mask & (1 <<
> pipe));
> + return igt_output_is_connected(output) &&
> + (output->config.valid_crtc_idx_mask & (1 << pipe));
> }
>
> #define for_each_if(condition) if (!(condition)) {} else
>
> #define for_each_connected_output(display, output) \
> for (int i__ = 0; i__ < (display)->n_outputs; i__++) \
> - for_each_if (((output = &(display)->outputs[i__]), output-
> >valid))
> + for_each_if (((output = &(display)->outputs[i__]), \
> + igt_output_is_connected(output)))
>
> #define for_each_pipe(display, pipe) \
> for (pipe = 0; pipe < igt_display_get_n_pipes(display); pipe++)
> \
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better.
2016-07-06 9:55 ` [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better Maarten Lankhorst
@ 2016-07-21 10:07 ` Ander Conselvan De Oliveira
0 siblings, 0 replies; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-21 10:07 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> Move properties to the pipe, they don't belong in the output
> and make atomic commit use the pipes for crtc properties.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> ---
> lib/igt_kms.c | 98 +++++++++++++++++++++++++++++-----------------------------
> -
> lib/igt_kms.h | 12 +++++---
> 2 files changed, 55 insertions(+), 55 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 88cae7d51787..8c7598a88a69 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -213,8 +213,7 @@ igt_atomic_fill_plane_props(igt_display_t *display,
> igt_plane_t *plane,
> * 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,
> +igt_atomic_fill_connector_props(igt_display_t *display, igt_output_t *output,
> int num_connector_props, const char
> **conn_prop_names)
> {
> drmModeObjectPropertiesPtr props;
> @@ -222,18 +221,18 @@ igt_atomic_fill_props(igt_display_t *display,
> igt_output_t *output,
>
> fd = display->drm_fd;
>
> - props = drmModeObjectGetProperties(fd, output->config.crtc->crtc_id,
> DRM_MODE_OBJECT_CRTC);
> + 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_crtc_props; j++) {
> - if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> + for (j = 0; j < num_connector_props; j++) {
> + if (strcmp(prop->name, conn_prop_names[j]) != 0)
> continue;
>
> - output->config.atomic_props_crtc[j] = props-
> >props[i];
> + output->config.atomic_props_connector[j] = props-
> >props[i];
> break;
> }
>
> @@ -241,19 +240,29 @@ igt_atomic_fill_props(igt_display_t *display,
> igt_output_t *output,
> }
>
> drmModeFreeObjectProperties(props);
> - props = NULL;
> - props = drmModeObjectGetProperties(fd, output->config.connector-
> >connector_id, DRM_MODE_OBJECT_CONNECTOR);
> +}
> +
> +static void
> +igt_atomic_fill_pipe_props(igt_display_t *display, igt_pipe_t *pipe,
> + int num_crtc_props, const char **crtc_prop_names)
> +{
> + drmModeObjectPropertiesPtr props;
> + int i, j, fd;
> +
> + fd = display->drm_fd;
> +
> + props = drmModeObjectGetProperties(fd, pipe->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_connector_props; j++) {
> - if (strcmp(prop->name, conn_prop_names[j]) != 0)
> + for (j = 0; j < num_crtc_props; j++) {
> + if (strcmp(prop->name, crtc_prop_names[j]) != 0)
> continue;
>
> - output->config.atomic_props_connector[j] = props-
> >props[i];
> + pipe->atomic_props_crtc[j] = props->props[i];
> break;
> }
>
> @@ -261,7 +270,6 @@ igt_atomic_fill_props(igt_display_t *display, igt_output_t
> *output,
> }
>
> drmModeFreeObjectProperties(props);
> -
> }
>
> const unsigned char* igt_kms_get_alt_edid(void)
> @@ -1222,6 +1230,9 @@ static void igt_output_refresh(igt_output_t *output)
> -1);
> }
>
> + igt_atomic_fill_connector_props(display, output,
> + IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
> +
> if (!output->valid)
> return;
>
> @@ -1232,8 +1243,6 @@ 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
> @@ -1359,6 +1368,8 @@ void igt_display_init(igt_display_t *display, int
> drm_fd)
> NULL,
> NULL);
>
> + igt_atomic_fill_pipe_props(display, pipe, IGT_NUM_CRTC_PROPS,
> igt_crtc_prop_names);
> +
> /* add the planes that can be used with that pipe */
> for (j = 0; j < plane_resources->count_planes; j++) {
> drmModePlane *drm_plane;
> @@ -1660,11 +1671,10 @@ static uint32_t
> igt_plane_get_fb_gem_handle(igt_plane_t *plane)
> * 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,
> +igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_pipe_t *pipe,
> drmModeAtomicReq *req)
> {
> -
> - igt_display_t *display = output->display;
> + igt_display_t *display = pipe->display;
> uint32_t fb_id, crtc_id;
>
> igt_assert(plane->drm_plane);
> @@ -1674,12 +1684,11 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane,
> igt_output_t *output,
> !plane->rotation_changed);
>
> fb_id = igt_plane_get_fb_id(plane);
> - crtc_id = output->config.crtc->crtc_id;
> + crtc_id = pipe->crtc_id;
>
> LOG(display,
> - "%s: populating plane data: %s.%d, fb %u\n",
> - igt_output_name(output),
> - kmstest_pipe_name(output->config.pipe),
> + "populating plane data: %s.%d, fb %u\n",
> + kmstest_pipe_name(pipe->pipe),
> plane->index,
> fb_id);
>
> @@ -2040,21 +2049,15 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
> /*
> * Add crtc property changes to the atomic property set
> */
> -static void igt_atomic_prepare_crtc_commit(igt_output_t *output,
> drmModeAtomicReq *req)
> +static void igt_atomic_prepare_crtc_commit(igt_pipe_t *pipe_obj,
> drmModeAtomicReq *req)
> {
> -
> - igt_pipe_t *pipe_obj = igt_output_get_driving_pipe(output);
> -
> - if (!pipe_obj)
> - return;
> -
> if (pipe_obj->background_changed)
> - igt_atomic_populate_crtc_req(req, output,
> IGT_CRTC_BACKGROUND, pipe_obj->background);
> + igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_BACKGROUND, pipe_obj->background);
>
> if (pipe_obj->color_mgmt_changed) {
> - igt_atomic_populate_crtc_req(req, output,
> IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
> - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_CTM,
> pipe_obj->ctm_blob);
> - igt_atomic_populate_crtc_req(req, output, IGT_CRTC_GAMMA_LUT,
> pipe_obj->gamma_blob);
> + igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_DEGAMMA_LUT, pipe_obj->degamma_blob);
> + igt_atomic_populate_crtc_req(req, pipe_obj, IGT_CRTC_CTM,
> pipe_obj->ctm_blob);
> + igt_atomic_populate_crtc_req(req, pipe_obj,
> IGT_CRTC_GAMMA_LUT, pipe_obj->gamma_blob);
> }
>
> /*
> @@ -2088,42 +2091,37 @@ static void
> igt_atomic_prepare_connector_commit(igt_output_t *output, drmModeAto
> static int igt_atomic_commit(igt_display_t *display)
> {
>
> - int ret = 0;
> + int ret = 0, i;
> + enum pipe pipe;
> drmModeAtomicReq *req;
> igt_output_t *output;
> +
> if (display->is_atomic != 1)
> return -1;
> req = drmModeAtomicAlloc();
> drmModeAtomicSetCursor(req, 0);
>
> - for_each_connected_output(display, output) {
> - igt_pipe_t *pipe_obj;
> + for_each_pipe(display, pipe) {
> + igt_pipe_t *pipe_obj = &display->pipes[pipe];
> igt_plane_t *plane;
> - enum pipe pipe;
>
> /*
> * 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_obj = igt_output_get_driving_pipe(output);
> - if (!pipe_obj)
> - continue;
> -
> - pipe = pipe_obj->pipe;
> + igt_atomic_prepare_crtc_commit(pipe_obj, req);
>
> for_each_plane_on_pipe(display, pipe, plane) {
> - igt_atomic_prepare_plane_commit(plane, output, req);
> + igt_atomic_prepare_plane_commit(plane, pipe_obj,
> req);
> }
>
> }
>
> + for (i = 0; i < display->n_outputs; i++) {
> + output = &display->outputs[i];
> +
> + igt_atomic_prepare_connector_commit(output, req);
> + }
> +
> ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
> drmModeAtomicFree(req);
> return ret;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 3531dc20b6e0..b9e8d344ed8c 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -117,7 +117,6 @@ struct kmstest_connector_config {
> 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 pipe;
> unsigned valid_crtc_idx_mask;
> @@ -257,6 +256,9 @@ struct igt_pipe {
> bool enabled;
> int n_planes;
> igt_plane_t planes[IGT_MAX_PLANES];
> +
> + uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
> +
> uint64_t background; /* Background color MSB BGR 16bpc LSB */
> uint32_t background_changed : 1;
> uint32_t background_property;
> @@ -395,13 +397,13 @@ static inline bool igt_pipe_connector_valid(enum pipe
> pipe,
> /**
> * igt_atomic_populate_crtc_req:
> * @req: A pointer to drmModeAtomicReq
> - * @output: A pointer igt_output_t
> + * @pipe: A pointer igt_pipe_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))
> +#define igt_atomic_populate_crtc_req(req, pipe, prop, value) \
> + igt_assert_lt(0, drmModeAtomicAddProperty(req, pipe->crtc_id,\
> + pipe-
> >atomic_props_crtc[prop], value))
> /**
> * igt_atomic_populate_connector_req:
> * @req: A pointer to drmModeAtomicReq
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2.
2016-07-06 9:55 ` [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2 Maarten Lankhorst
@ 2016-07-21 11:42 ` Ander Conselvan De Oliveira
2016-07-21 12:37 ` Maarten Lankhorst
0 siblings, 1 reply; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-21 11:42 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> They're duplicates with src_x/y, so just use those.
>
> Changes since v1:
> - Fix order of parameters in calls to igt_fb_set_position.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> lib/igt_kms.c | 24 ++++--------------------
> lib/igt_kms.h | 4 ----
> tests/kms_panel_fitting.c | 2 +-
> tests/kms_plane.c | 8 +++-----
> tests/kms_plane_scaling.c | 2 +-
> 5 files changed, 9 insertions(+), 31 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 8c7598a88a69..9fc5559a5df4 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1915,7 +1915,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> igt_assert(!primary->rotation_changed);
>
> if (!primary->fb_changed && !primary->position_changed &&
> - !primary->size_changed && !primary->panning_changed)
> + !primary->size_changed)
> return 0;
>
> crtc_id = pipe->crtc_id;
> @@ -1927,18 +1927,18 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
>
> if (fb_id) {
> LOG(display,
> - "%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
> + "%s: SetCrtc pipe %s, fb %u, src (%d, %d), "
> "mode %dx%d\n",
> igt_output_name(output),
> kmstest_pipe_name(pipe->pipe),
> fb_id,
> - primary->pan_x, primary->pan_y,
> + primary->src_x, primary->src_x,
> mode->hdisplay, mode->vdisplay);
>
> ret = drmModeSetCrtc(display->drm_fd,
> crtc_id,
> fb_id,
> - primary->pan_x, primary->pan_y,
> + primary->src_x, primary->src_x,
> &output->id,
> 1,
> mode);
> @@ -1962,7 +1962,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> primary->fb_changed = false;
> primary->position_changed = false;
> primary->size_changed = false;
> - primary->panning_changed = false;
>
> return 0;
> }
> @@ -2408,21 +2407,6 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t
> *plane,
> plane->fb_changed = true;
> }
>
> -void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
> -{
> - igt_pipe_t *pipe = plane->pipe;
> - igt_display_t *display = pipe->display;
> -
> - LOG(display, "%s.%d: plane_set_panning(%d,%d)\n",
> - kmstest_pipe_name(pipe->pipe),
> - plane->index, x, y);
> -
> - plane->pan_x = x;
> - plane->pan_y = y;
> -
> - plane->panning_changed = true;
> -}
> -
> static const char *rotation_name(igt_rotation_t rotation)
> {
> switch (rotation) {
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index b9e8d344ed8c..4d18c01042c9 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -220,7 +220,6 @@ typedef struct {
> /* state tracking */
> unsigned int fb_changed : 1;
> unsigned int position_changed : 1;
> - unsigned int panning_changed : 1;
> unsigned int rotation_changed : 1;
> unsigned int size_changed : 1;
> /*
> @@ -244,8 +243,6 @@ typedef struct {
> uint32_t src_w;
> uint32_t src_h;
>
> - /* 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;
> @@ -326,7 +323,6 @@ void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
> size_t length);
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
> void igt_plane_set_position(igt_plane_t *plane, int x, int y);
> void igt_plane_set_size(igt_plane_t *plane, int w, int h);
> -void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
> void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
> void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index 7c501fcdd3fb..da09da32d704 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -89,7 +89,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
> ret = drmModeSetCrtc(data->drm_fd,
> plane->pipe->crtc_id,
> data->fb_id1,
> - plane->pan_x, plane->pan_y,
> + plane->src_x, plane->src_y,
> &output->id,
> 1,
> mode);
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 60b8d71d52d4..7c01fe91b263 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -321,11 +321,9 @@ test_plane_panning_with_output(data_t *data,
> igt_plane_set_fb(primary, &primary_fb);
>
> if (flags & TEST_PANNING_TOP_LEFT)
> - igt_plane_set_panning(primary, 0, 0);
> + igt_fb_set_position(&primary_fb, primary, 0, 0);
While at it you might want to rename igt_fb_set_position() to
igt_plane_set_position() or something. The name is very misleading as it doens't
depend on the fb and only sets properties on the plane.
But anyway,
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> else
> - igt_plane_set_panning(primary, mode->hdisplay, mode-
> >vdisplay);
> -
> - igt_plane_set_position(primary, 0, 0);
> + igt_fb_set_position(&primary_fb, primary, mode->hdisplay,
> mode->vdisplay);
>
> igt_display_commit(&data->display);
>
> @@ -343,7 +341,7 @@ test_plane_panning_with_output(data_t *data,
>
> /* reset states to neutral values, assumed by other tests */
> igt_output_set_pipe(output, PIPE_ANY);
> - igt_plane_set_panning(primary, 0, 0);
> + igt_fb_set_position(&primary_fb, primary, 0, 0);
>
> test_fini(data);
> }
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 39bb5e113411..4546ce548515 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -98,7 +98,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> enum pipe pipe,
> ret = drmModeSetCrtc(data->drm_fd,
> plane->pipe->crtc_id,
> data->fb_id1,
> - plane->pan_x, plane->pan_y,
> + plane->src_x, plane->src_y,
> &output->id,
> 1,
> mode);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally
2016-07-06 9:55 ` [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally Maarten Lankhorst
@ 2016-07-21 12:13 ` Ander Conselvan De Oliveira
2016-07-21 12:43 ` Maarten Lankhorst
0 siblings, 1 reply; 32+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-07-21 12:13 UTC (permalink / raw)
To: Maarten Lankhorst, intel-gfx
On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
> This will make it easier to support TEST_ONLY commits.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> lib/igt_kms.c | 84 ++++++++++++++++++++++++++++++++++----------------------
> ---
> 1 file changed, 48 insertions(+), 36 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 9fc5559a5df4..d0d0dcff8193 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1727,11 +1727,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane,
> igt_pipe_t *pipe,
> 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;
> - plane->rotation_changed = false;
> }
>
>
> @@ -1819,15 +1814,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
> CHECK_RETURN(ret, fail_on_error);
> }
>
> - plane->fb_changed = false;
> - plane->position_changed = false;
> - plane->size_changed = false;
> -
> if (plane->rotation_changed) {
> ret = igt_plane_set_property(plane, plane->rotation_property,
> plane->rotation);
>
> - plane->rotation_changed = false;
> CHECK_RETURN(ret, fail_on_error);
> }
>
> @@ -1872,8 +1862,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
> }
>
> CHECK_RETURN(ret, fail_on_error);
> -
> - cursor->fb_changed = false;
> }
>
> if (cursor->position_changed) {
> @@ -1887,8 +1875,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
>
> ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
> CHECK_RETURN(ret, fail_on_error);
> -
> - cursor->position_changed = false;
> }
>
> return 0;
> @@ -1915,7 +1901,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> igt_assert(!primary->rotation_changed);
>
> if (!primary->fb_changed && !primary->position_changed &&
> - !primary->size_changed)
> + !primary->size_changed)
> return 0;
>
> crtc_id = pipe->crtc_id;
> @@ -1959,9 +1945,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
> *primary,
> CHECK_RETURN(ret, fail_on_error);
>
> primary->pipe->enabled = (fb_id != 0);
> - primary->fb_changed = false;
> - primary->position_changed = false;
> - primary->size_changed = false;
>
> return 0;
> }
> @@ -1982,7 +1965,7 @@ static int igt_plane_commit(igt_plane_t *plane,
> return igt_primary_plane_commit_legacy(plane, pipe,
> fail_on_error);
> } else {
> - return igt_drm_plane_commit(plane, pipe,
> fail_on_error);
> + return igt_drm_plane_commit(plane, pipe, fail_on_error);
> }
> }
>
> @@ -2008,8 +1991,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
> if (pipe->background_changed) {
> igt_crtc_set_property(pipe, pipe->background_property,
> pipe->background);
> -
> - pipe->background_changed = false;
> }
>
> if (pipe->color_mgmt_changed) {
> @@ -2019,8 +2000,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
> pipe->ctm_blob);
> igt_crtc_set_property(pipe, pipe->gamma_property,
> pipe->gamma_blob);
> -
> - pipe->color_mgmt_changed = false;
> }
>
> for (i = 0; i < pipe->n_planes; i++) {
> @@ -2140,35 +2119,68 @@ static int do_display_commit(igt_display_t *display,
> bool fail_on_error)
> {
> int i, ret;
> - int valid_outs = 0;
> -
> + enum pipe pipe;
> LOG_INDENT(display, "commit");
>
> igt_display_refresh(display);
>
> if (s == COMMIT_ATOMIC) {
> -
> ret = igt_atomic_commit(display);
> +
> CHECK_RETURN(ret, fail_on_error);
> - return 0;
> + } else {
> + int valid_outs = 0;
>
> - }
> + for_each_pipe(display, pipe) {
> + igt_pipe_t *pipe_obj = &display->pipes[pipe];
> + igt_output_t *output = igt_pipe_get_output(pipe_obj);
>
> - for_each_pipe(display, i) {
> - igt_pipe_t *pipe_obj = &display->pipes[i];
> - igt_output_t *output = igt_pipe_get_output(pipe_obj);
> + if (output && output->valid)
> + valid_outs++;
>
> - if (output && output->valid)
> - valid_outs++;
> + ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
> + CHECK_RETURN(ret, fail_on_error);
> + }
>
> - ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
> CHECK_RETURN(ret, fail_on_error);
> +
> + if (valid_outs == 0) {
> + LOG_UNINDENT(display);
> +
> + return -1;
> + }
> }
>
> LOG_UNINDENT(display);
>
> - if (valid_outs == 0)
> - return -1;
> + if (ret)
> + return ret;
> +
> + for_each_pipe(display, pipe) {
> + igt_pipe_t *pipe_obj = &display->pipes[pipe];
> + igt_plane_t *plane;
> +
> + if (s == COMMIT_ATOMIC) {
> + pipe_obj->color_mgmt_changed = false;
> + pipe_obj->background_changed = false;
> + }
If this is supposed to match the previous behavior, shouldn't the condition be
"!= COMMIT_ATOMIC". Both flags are set from igt_pipe_commit() which is not
called in the atomic case. However, the flags aren't set to false in
igt_atomic_prepare_crtc_commit(). That actually sounds like a bug?
> +
> + for_each_plane_on_pipe(display, pipe, plane) {
> + plane->fb_changed = false;
> + plane->position_changed = false;
> + plane->size_changed = false;
> +
> + if (s != COMMIT_LEGACY || !(plane->is_primary ||
> plane->is_cursor))
> + plane->rotation_changed = false;
Can't plane->rotation_changed be set unconditionally here? The legacy primary
plane commit has an assert for rotation_changed == false and perhaps the cursor
version should have the same.
> + }
> + }
> +
> + for (i = 0; i < display->n_outputs && s == COMMIT_ATOMIC; i++) {
> + igt_output_t *output = &display->outputs[i];
> +
> + output->config.connector_dpms_changed = false;
> + output->config.connector_scaling_mode_changed = false;
> + }
>
> igt_debug_wait_for_keypress("modeset");
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2.
2016-07-21 11:42 ` Ander Conselvan De Oliveira
@ 2016-07-21 12:37 ` Maarten Lankhorst
0 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-21 12:37 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
Op 21-07-16 om 13:42 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
>> They're duplicates with src_x/y, so just use those.
>>
>> Changes since v1:
>> - Fix order of parameters in calls to igt_fb_set_position.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> lib/igt_kms.c | 24 ++++--------------------
>> lib/igt_kms.h | 4 ----
>> tests/kms_panel_fitting.c | 2 +-
>> tests/kms_plane.c | 8 +++-----
>> tests/kms_plane_scaling.c | 2 +-
>> 5 files changed, 9 insertions(+), 31 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 8c7598a88a69..9fc5559a5df4 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1915,7 +1915,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
>> *primary,
>> igt_assert(!primary->rotation_changed);
>>
>> if (!primary->fb_changed && !primary->position_changed &&
>> - !primary->size_changed && !primary->panning_changed)
>> + !primary->size_changed)
>> return 0;
>>
>> crtc_id = pipe->crtc_id;
>> @@ -1927,18 +1927,18 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
>> *primary,
>>
>> if (fb_id) {
>> LOG(display,
>> - "%s: SetCrtc pipe %s, fb %u, panning (%d, %d), "
>> + "%s: SetCrtc pipe %s, fb %u, src (%d, %d), "
>> "mode %dx%d\n",
>> igt_output_name(output),
>> kmstest_pipe_name(pipe->pipe),
>> fb_id,
>> - primary->pan_x, primary->pan_y,
>> + primary->src_x, primary->src_x,
>> mode->hdisplay, mode->vdisplay);
>>
>> ret = drmModeSetCrtc(display->drm_fd,
>> crtc_id,
>> fb_id,
>> - primary->pan_x, primary->pan_y,
>> + primary->src_x, primary->src_x,
>> &output->id,
>> 1,
>> mode);
>> @@ -1962,7 +1962,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
>> *primary,
>> primary->fb_changed = false;
>> primary->position_changed = false;
>> primary->size_changed = false;
>> - primary->panning_changed = false;
>>
>> return 0;
>> }
>> @@ -2408,21 +2407,6 @@ void igt_fb_set_size(struct igt_fb *fb, igt_plane_t
>> *plane,
>> plane->fb_changed = true;
>> }
>>
>> -void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
>> -{
>> - igt_pipe_t *pipe = plane->pipe;
>> - igt_display_t *display = pipe->display;
>> -
>> - LOG(display, "%s.%d: plane_set_panning(%d,%d)\n",
>> - kmstest_pipe_name(pipe->pipe),
>> - plane->index, x, y);
>> -
>> - plane->pan_x = x;
>> - plane->pan_y = y;
>> -
>> - plane->panning_changed = true;
>> -}
>> -
>> static const char *rotation_name(igt_rotation_t rotation)
>> {
>> switch (rotation) {
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index b9e8d344ed8c..4d18c01042c9 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -220,7 +220,6 @@ typedef struct {
>> /* state tracking */
>> unsigned int fb_changed : 1;
>> unsigned int position_changed : 1;
>> - unsigned int panning_changed : 1;
>> unsigned int rotation_changed : 1;
>> unsigned int size_changed : 1;
>> /*
>> @@ -244,8 +243,6 @@ typedef struct {
>> uint32_t src_w;
>> uint32_t src_h;
>>
>> - /* 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;
>> @@ -326,7 +323,6 @@ void igt_pipe_set_gamma_lut(igt_pipe_t *pipe, void *ptr,
>> size_t length);
>> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
>> void igt_plane_set_position(igt_plane_t *plane, int x, int y);
>> void igt_plane_set_size(igt_plane_t *plane, int w, int h);
>> -void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
>> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>> void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background);
>> void igt_fb_set_position(struct igt_fb *fb, igt_plane_t *plane,
>> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
>> index 7c501fcdd3fb..da09da32d704 100644
>> --- a/tests/kms_panel_fitting.c
>> +++ b/tests/kms_panel_fitting.c
>> @@ -89,7 +89,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
>> enum pipe pipe,
>> ret = drmModeSetCrtc(data->drm_fd,
>> plane->pipe->crtc_id,
>> data->fb_id1,
>> - plane->pan_x, plane->pan_y,
>> + plane->src_x, plane->src_y,
>> &output->id,
>> 1,
>> mode);
>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>> index 60b8d71d52d4..7c01fe91b263 100644
>> --- a/tests/kms_plane.c
>> +++ b/tests/kms_plane.c
>> @@ -321,11 +321,9 @@ test_plane_panning_with_output(data_t *data,
>> igt_plane_set_fb(primary, &primary_fb);
>>
>> if (flags & TEST_PANNING_TOP_LEFT)
>> - igt_plane_set_panning(primary, 0, 0);
>> + igt_fb_set_position(&primary_fb, primary, 0, 0);
> While at it you might want to rename igt_fb_set_position() to
> igt_plane_set_position() or something. The name is very misleading as it doens't
> depend on the fb and only sets properties on the plane.
That one already exists for dst, but yeah it should be renamed to igt_plane_set_src_position,
the fb argument is unused anyway..
Will probably just commit some patches to do so later on.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally
2016-07-21 12:13 ` Ander Conselvan De Oliveira
@ 2016-07-21 12:43 ` Maarten Lankhorst
0 siblings, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-21 12:43 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
Op 21-07-16 om 14:13 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-07-06 at 11:55 +0200, Maarten Lankhorst wrote:
>> This will make it easier to support TEST_ONLY commits.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> lib/igt_kms.c | 84 ++++++++++++++++++++++++++++++++++----------------------
>> ---
>> 1 file changed, 48 insertions(+), 36 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 9fc5559a5df4..d0d0dcff8193 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1727,11 +1727,6 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane,
>> igt_pipe_t *pipe,
>> 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;
>> - plane->rotation_changed = false;
>> }
>>
>>
>> @@ -1819,15 +1814,10 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
>> CHECK_RETURN(ret, fail_on_error);
>> }
>>
>> - plane->fb_changed = false;
>> - plane->position_changed = false;
>> - plane->size_changed = false;
>> -
>> if (plane->rotation_changed) {
>> ret = igt_plane_set_property(plane, plane->rotation_property,
>> plane->rotation);
>>
>> - plane->rotation_changed = false;
>> CHECK_RETURN(ret, fail_on_error);
>> }
>>
>> @@ -1872,8 +1862,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
>> }
>>
>> CHECK_RETURN(ret, fail_on_error);
>> -
>> - cursor->fb_changed = false;
>> }
>>
>> if (cursor->position_changed) {
>> @@ -1887,8 +1875,6 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
>>
>> ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
>> CHECK_RETURN(ret, fail_on_error);
>> -
>> - cursor->position_changed = false;
>> }
>>
>> return 0;
>> @@ -1915,7 +1901,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
>> *primary,
>> igt_assert(!primary->rotation_changed);
>>
>> if (!primary->fb_changed && !primary->position_changed &&
>> - !primary->size_changed)
>> + !primary->size_changed)
>> return 0;
>>
>> crtc_id = pipe->crtc_id;
>> @@ -1959,9 +1945,6 @@ static int igt_primary_plane_commit_legacy(igt_plane_t
>> *primary,
>> CHECK_RETURN(ret, fail_on_error);
>>
>> primary->pipe->enabled = (fb_id != 0);
>> - primary->fb_changed = false;
>> - primary->position_changed = false;
>> - primary->size_changed = false;
>>
>> return 0;
>> }
>> @@ -1982,7 +1965,7 @@ static int igt_plane_commit(igt_plane_t *plane,
>> return igt_primary_plane_commit_legacy(plane, pipe,
>> fail_on_error);
>> } else {
>> - return igt_drm_plane_commit(plane, pipe,
>> fail_on_error);
>> + return igt_drm_plane_commit(plane, pipe, fail_on_error);
>> }
>> }
>>
>> @@ -2008,8 +1991,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>> if (pipe->background_changed) {
>> igt_crtc_set_property(pipe, pipe->background_property,
>> pipe->background);
>> -
>> - pipe->background_changed = false;
>> }
>>
>> if (pipe->color_mgmt_changed) {
>> @@ -2019,8 +2000,6 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>> pipe->ctm_blob);
>> igt_crtc_set_property(pipe, pipe->gamma_property,
>> pipe->gamma_blob);
>> -
>> - pipe->color_mgmt_changed = false;
>> }
>>
>> for (i = 0; i < pipe->n_planes; i++) {
>> @@ -2140,35 +2119,68 @@ static int do_display_commit(igt_display_t *display,
>> bool fail_on_error)
>> {
>> int i, ret;
>> - int valid_outs = 0;
>> -
>> + enum pipe pipe;
>> LOG_INDENT(display, "commit");
>>
>> igt_display_refresh(display);
>>
>> if (s == COMMIT_ATOMIC) {
>> -
>> ret = igt_atomic_commit(display);
>> +
>> CHECK_RETURN(ret, fail_on_error);
>> - return 0;
>> + } else {
>> + int valid_outs = 0;
>>
>> - }
>> + for_each_pipe(display, pipe) {
>> + igt_pipe_t *pipe_obj = &display->pipes[pipe];
>> + igt_output_t *output = igt_pipe_get_output(pipe_obj);
>>
>> - for_each_pipe(display, i) {
>> - igt_pipe_t *pipe_obj = &display->pipes[i];
>> - igt_output_t *output = igt_pipe_get_output(pipe_obj);
>> + if (output && output->valid)
>> + valid_outs++;
>>
>> - if (output && output->valid)
>> - valid_outs++;
>> + ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
>> + CHECK_RETURN(ret, fail_on_error);
>> + }
>>
>> - ret = igt_pipe_commit(pipe_obj, s, fail_on_error);
>> CHECK_RETURN(ret, fail_on_error);
>> +
>> + if (valid_outs == 0) {
>> + LOG_UNINDENT(display);
>> +
>> + return -1;
>> + }
>> }
>>
>> LOG_UNINDENT(display);
>>
>> - if (valid_outs == 0)
>> - return -1;
>> + if (ret)
>> + return ret;
>> +
>> + for_each_pipe(display, pipe) {
>> + igt_pipe_t *pipe_obj = &display->pipes[pipe];
>> + igt_plane_t *plane;
>> +
>> + if (s == COMMIT_ATOMIC) {
>> + pipe_obj->color_mgmt_changed = false;
>> + pipe_obj->background_changed = false;
>> + }
> If this is supposed to match the previous behavior, shouldn't the condition be
> "!= COMMIT_ATOMIC". Both flags are set from igt_pipe_commit() which is not
> called in the atomic case. However, the flags aren't set to false in
> igt_atomic_prepare_crtc_commit(). That actually sounds like a bug?
I think you're right, real fix is to just update those atomically too. :)
>> +
>> + for_each_plane_on_pipe(display, pipe, plane) {
>> + plane->fb_changed = false;
>> + plane->position_changed = false;
>> + plane->size_changed = false;
>> +
>> + if (s != COMMIT_LEGACY || !(plane->is_primary ||
>> plane->is_cursor))
>> + plane->rotation_changed = false;
> Can't plane->rotation_changed be set unconditionally here? The legacy primary
> plane commit has an assert for rotation_changed == false and perhaps the cursor
> version should have the same.
This is specifically to exclude s == COMMIT_LEGACY && (plane->is_primary || plane->is_cursor),
we don't update rotation_changed in this case.
Maybe I should just add an assert to the legacy update functions instead.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes.
2016-07-20 12:56 ` Ander Conselvan De Oliveira
2016-07-21 9:21 ` Maarten Lankhorst
@ 2016-07-25 13:04 ` Maarten Lankhorst
1 sibling, 0 replies; 32+ messages in thread
From: Maarten Lankhorst @ 2016-07-25 13:04 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, intel-gfx
How about this?
tests/kms: Clean up more users of unassigned pipes.
Use for_each_pipe_with_valid_output instead.
This may increase test time slightly on the affected tests, because now
outputs will be tested on every pipe instead of the first pipe. This
will increase test coverage to all usable pipes though, so it shouldn't
be an issue.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-07-25 13:04 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-06 9:55 [PATCH i-g-t v2 00/15] Add support for atomic modeset to IGT Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 01/15] igt_kms: Remove kmstest_connector_config.crtc_idx Maarten Lankhorst
2016-07-13 12:13 ` Daniel Vetter
2016-07-19 12:52 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 02/15] igt_kms: Find optimal encoder only after selecting pipe Maarten Lankhorst
2016-07-15 11:14 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 03/15] kms_psr_sink_crc: Use for_each_pipe_with_valid_output to find a valid config Maarten Lankhorst
2016-07-15 11:15 ` Ander Conselvan De Oliveira
2016-07-19 13:58 ` Ander Conselvan De Oliveira
2016-07-20 7:53 ` Maarten Lankhorst
2016-07-20 12:17 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 04/15] igt_kms: Make PIPE_ANY a alias for PIPE_NONE Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 05/15] tests/kms: Clean up more users of unassigned pipes Maarten Lankhorst
2016-07-20 12:56 ` Ander Conselvan De Oliveira
2016-07-21 9:21 ` Maarten Lankhorst
2016-07-25 13:04 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 06/15] igt_kms: Change PIPE_ANY behavior to mean unassigned Maarten Lankhorst
2016-07-21 9:23 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 07/15] igt_kms: Handle atomic pipe properties better Maarten Lankhorst
2016-07-21 10:07 ` Ander Conselvan De Oliveira
2016-07-06 9:55 ` [PATCH i-g-t v2 08/15] igt_kms: Remove pan members from igt_plane, v2 Maarten Lankhorst
2016-07-21 11:42 ` Ander Conselvan De Oliveira
2016-07-21 12:37 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 09/15] igt_kms: Clear all _changed members centrally Maarten Lankhorst
2016-07-21 12:13 ` Ander Conselvan De Oliveira
2016-07-21 12:43 ` Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 10/15] igt_kms: Add modeset support to atomic commits Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 11/15] tests: Add kms_rmfb test Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 12/15] tests: Add kms_atomic_transition Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 13/15] igt_kms: Add more apis for panel fitting test Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 14/15] igt_kms: Allow disabling previous override mode Maarten Lankhorst
2016-07-06 9:55 ` [PATCH i-g-t v2 15/15] kms_panel_fitting: Add tests for fastboot Maarten Lankhorst
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.