[i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915

[i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915

[Tomeu Vizoso]

Hi,

following Daniel's comments on a previous series that completely removed
usage of DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, I'm sending this one that
removes it only on codepaths that can be run on drivers other than i915.

It does so by using the already-available crtc indexes instead of
querying for the pipe number (which should return the same value).

Thanks,

Tomeu


Tomeu Vizoso (2):
  lib: Remove superfluous kmstest_connector_config.pipe
  tests/kms_setmode: Remove superfluous call to
    kmstest_get_pipe_from_crtc_id

 lib/igt_kms.c                     | 24 +++++++++++-------------
 lib/igt_kms.h                     |  1 -
 tests/kms_crtc_background_color.c |  2 +-
 tests/kms_flip.c                  | 10 +++++-----
 tests/kms_flip_tiling.c           |  2 +-
 tests/kms_panel_fitting.c         |  2 +-
 tests/kms_plane_scaling.c         |  2 +-
 tests/kms_render.c                |  2 +-
 tests/kms_setmode.c               |  5 +----
 tests/testdisplay.c               |  1 -
 10 files changed, 22 insertions(+), 29 deletions(-)

-- 
2.5.5

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

[i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe

[Tomeu Vizoso]

The field pipe in struct kmstest_connector_config is superfluous because
there's already crtc_idx which should contain always the same value.

By dropping the original assignment to the field pipe, we can also drop
a call to DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID which is problematic when
running the tests on drivers other than i915.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

 lib/igt_kms.c                     | 24 +++++++++++-------------
 lib/igt_kms.h                     |  1 -
 tests/kms_crtc_background_color.c |  2 +-
 tests/kms_flip.c                  | 10 +++++-----
 tests/kms_flip_tiling.c           |  2 +-
 tests/kms_panel_fitting.c         |  2 +-
 tests/kms_plane_scaling.c         |  2 +-
 tests/kms_render.c                |  2 +-
 tests/testdisplay.c               |  1 -
 9 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 8f30c94070d5..40cb317e99b9 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -822,8 +822,6 @@ found:
 	config->encoder = encoder;
 	config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[i]);
 	config->crtc_idx = i;
-	config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
-						     config->crtc->crtc_id);
 
 	drmModeFreeResources(resources);
 
@@ -1167,9 +1165,9 @@ static void igt_output_refresh(igt_output_t *output)
 	}
 
 	LOG(display, "%s: Selecting pipe %s\n", output->name,
-	    kmstest_pipe_name(output->config.pipe));
+	    kmstest_pipe_name(output->config.crtc_idx));
 
-	display->pipes_in_use |= 1 << output->config.pipe;
+	display->pipes_in_use |= 1 << output->config.crtc_idx;
 	igt_atomic_fill_props(display, output, IGT_NUM_CRTC_PROPS, igt_crtc_prop_names,
 		IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
 }
@@ -1540,7 +1538,7 @@ static igt_pipe_t *igt_output_get_driving_pipe(igt_output_t *output)
 		 * The user hasn't specified a pipe to use, take the one
 		 * configured by the last refresh()
 		 */
-		pipe = output->config.pipe;
+		pipe = output->config.crtc_idx;
 	} else {
 		/*
 		 * Otherwise, return the pending pipe (ie the pipe that should
@@ -1628,7 +1626,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output,
 	LOG(display,
 	    "%s: populating plane data: %s.%d, fb %u\n",
 	    igt_output_name(output),
-	    kmstest_pipe_name(output->config.pipe),
+	    kmstest_pipe_name(output->config.crtc_idx),
 	    plane->index,
 	    fb_id);
 
@@ -1711,7 +1709,7 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 		LOG(display,
 		    "%s: SetPlane pipe %s, plane %d, disabling\n",
 		    igt_output_name(output),
-		    kmstest_pipe_name(output->config.pipe),
+		    kmstest_pipe_name(output->config.crtc_idx),
 		    plane->index);
 
 		ret = drmModeSetPlane(display->drm_fd,
@@ -1742,7 +1740,7 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
 		    "%s: 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(output->config.crtc_idx),
 		    plane->index,
 		    fb_id,
 		    src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
@@ -1797,7 +1795,7 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
 			LOG(display,
 			    "%s: SetCursor pipe %s, fb %u %dx%d\n",
 			    igt_output_name(output),
-			    kmstest_pipe_name(output->config.pipe),
+			    kmstest_pipe_name(output->config.crtc_idx),
 			    gem_handle,
 			    cursor->crtc_w, cursor->crtc_h);
 
@@ -1809,7 +1807,7 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
 			LOG(display,
 			    "%s: SetCursor pipe %s, disabling\n",
 			    igt_output_name(output),
-			    kmstest_pipe_name(output->config.pipe));
+			    kmstest_pipe_name(output->config.crtc_idx));
 
 			ret = drmModeSetCursor(display->drm_fd, crtc_id,
 					       0, 0, 0);
@@ -1827,7 +1825,7 @@ static int igt_cursor_commit_legacy(igt_plane_t *cursor,
 		LOG(display,
 		    "%s: MoveCursor pipe %s, (%d, %d)\n",
 		    igt_output_name(output),
-		    kmstest_pipe_name(output->config.pipe),
+		    kmstest_pipe_name(output->config.crtc_idx),
 		    x, y);
 
 		ret = drmModeMoveCursor(display->drm_fd, crtc_id, x, y);
@@ -1874,7 +1872,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(output->config.crtc_idx),
 		    fb_id,
 		    primary->pan_x, primary->pan_y,
 		    mode->hdisplay, mode->vdisplay);
@@ -1890,7 +1888,7 @@ static int igt_primary_plane_commit_legacy(igt_plane_t *primary,
 		LOG(display,
 		    "%s: SetCrtc pipe %s, disabling\n",
 		    igt_output_name(output),
-		    kmstest_pipe_name(output->config.pipe));
+		    kmstest_pipe_name(output->config.crtc_idx));
 
 		ret = drmModeSetCrtc(display->drm_fd,
 				     crtc_id,
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 2c189ed47d04..f2444dd74ffe 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -132,7 +132,6 @@ struct kmstest_connector_config {
 	uint32_t atomic_props_crtc[IGT_NUM_CRTC_PROPS];
 	uint32_t atomic_props_connector[IGT_NUM_CONNECTOR_PROPS];
 	int crtc_idx;
-	int pipe;
 };
 
 /**
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index b496625c1693..57e7d4a1f0ad 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -136,7 +136,7 @@ static void test_crtc_background(data_t *data)
 	for_each_connected_output(display, output) {
 		igt_plane_t *plane;
 
-		pipe = output->config.pipe;
+		pipe = output->config.crtc_idx;
 		igt_output_set_pipe(output, pipe);
 
 		plane = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 3d4454407709..660ec6f1135e 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1042,11 +1042,11 @@ static void connector_find_preferred_mode(uint32_t connector_id, int crtc_idx,
 		return;
 	}
 
-	o->pipe = config.pipe;
+	o->pipe = config.crtc_idx;
 	o->kconnector[0] = config.connector;
 	o->kencoder[0] = config.encoder;
 	o->_crtc[0] = config.crtc->crtc_id;
-	o->_pipe[0] = config.pipe;
+	o->_pipe[0] = config.crtc_idx;
 	o->kmode[0] = config.default_mode;
 	o->mode_valid = 1;
 
@@ -1107,7 +1107,7 @@ static void connector_find_compatible_mode(int crtc_idx0, int crtc_idx1,
 	}
 
 found:
-	o->pipe = config[0].pipe;
+	o->pipe = config[0].crtc_idx;
 	o->fb_width = mode[0]->hdisplay;
 	o->fb_height = mode[0]->vdisplay;
 	o->mode_valid = 1;
@@ -1115,13 +1115,13 @@ found:
 	o->kconnector[0] = config[0].connector;
 	o->kencoder[0] = config[0].encoder;
 	o->_crtc[0] = config[0].crtc->crtc_id;
-	o->_pipe[0] = config[0].pipe;
+	o->_pipe[0] = config[0].crtc_idx;
 	o->kmode[0] = *mode[0];
 
 	o->kconnector[1] = config[1].connector;
 	o->kencoder[1] = config[1].encoder;
 	o->_crtc[1] = config[1].crtc->crtc_id;
-	o->_pipe[1] = config[1].pipe;
+	o->_pipe[1] = config[1].crtc_idx;
 	o->kmode[1] = *mode[1];
 
 	drmModeFreeCrtc(config[0].crtc);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index f58e65be61ee..9056c7ed1116 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -89,7 +89,7 @@ test_flip_tiling(data_t *data, igt_output_t *output, uint64_t tiling[2])
 	igt_crc_t reference_crc, crc;
 	int fb_id, pipe, ret, width;
 
-	pipe = output->config.pipe;
+	pipe = output->config.crtc_idx;
 	pipe_crc = pipe_crc_new(pipe);
 	igt_output_set_pipe(output, pipe);
 
diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
index 829d9cdd0631..5a22e47e3a61 100644
--- a/tests/kms_panel_fitting.c
+++ b/tests/kms_panel_fitting.c
@@ -153,7 +153,7 @@ static void test_panel_fitting(data_t *d)
 		igt_require(output->config.connector->connector_type ==
 			DRM_MODE_CONNECTOR_eDP);
 
-		pipe = output->config.pipe;
+		pipe = output->config.crtc_idx;
 		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..07c581caef12 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -189,7 +189,7 @@ static void test_plane_scaling(data_t *d)
 	for_each_connected_output(display, output) {
 		drmModeModeInfo *mode;
 
-		pipe = output->config.pipe;
+		pipe = output->config.crtc_idx;
 		igt_output_set_pipe(output, pipe);
 
 		mode = igt_output_get_mode(output);
diff --git a/tests/kms_render.c b/tests/kms_render.c
index e0a2b58f6b82..d08b7ce0e72b 100644
--- a/tests/kms_render.c
+++ b/tests/kms_render.c
@@ -109,7 +109,7 @@ static int test_format(const char *test_name,
 		 mode->name, mode->vrefresh, igt_format_str(format));
 	igt_assert_lt(0, ret);
 	ret = asprintf(&cconf_str, "pipe %s, encoder %s, connector %s",
-		       kmstest_pipe_name(cconf->pipe),
+		       kmstest_pipe_name(cconf->crtc_idx),
 		       kmstest_encoder_type_str(cconf->encoder->encoder_type),
 		       kmstest_connector_type_str(cconf->connector->connector_type));
 	igt_assert_lt(0, ret);
diff --git a/tests/testdisplay.c b/tests/testdisplay.c
index 00b47bd06280..98455fbf42b1 100644
--- a/tests/testdisplay.c
+++ b/tests/testdisplay.c
@@ -212,7 +212,6 @@ static void connector_find_preferred_mode(uint32_t connector_id,
 	c->encoder = config.encoder;
 	c->crtc = config.crtc->crtc_id;
 	c->crtc_idx = config.crtc_idx;
-	c->pipe = config.pipe;
 
 	if (mode_num != -1) {
 		igt_assert(mode_num < config.connector->count_modes);
-- 
2.5.5

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

[i-g-t PATCH v1 2/2] tests/kms_setmode: Remove superfluous call to kmstest_get_pipe_from_crtc_id

[Tomeu Vizoso]

connector_config.crtc_idx already contains the pipe ID, so just use that
and drop the call to kmstest_get_pipe_from_crtc_id which is problematic
on drivers other than i915 because it uses
DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
danvet
---

 tests/kms_setmode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 24fb34ca0006..34f27c66d79e 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -89,7 +89,6 @@ struct connector_config {
 struct crtc_config {
 	int crtc_idx;
 	int crtc_id;
-	int pipe_id;
 	int connector_count;
 	struct connector_config *cconfs;
 	struct igt_fb fb_info;
@@ -241,7 +240,7 @@ static void get_crtc_config_str(struct crtc_config *crtc, char *buf,
 
 	pos = snprintf(buf, buf_size,
 		       "CRTC[%d] [Pipe %s] Mode: %s@%dHz Connectors: ",
-		       crtc->crtc_id, kmstest_pipe_name(crtc->pipe_id),
+		       crtc->crtc_id, kmstest_pipe_name(crtc->crtc_idx),
 		       crtc->mode.name, crtc->mode.vrefresh);
 	if (pos > buf_size)
 		return;
@@ -285,8 +284,6 @@ static void setup_crtcs(drmModeRes *resources, struct connector_config *cconf,
 					  resources->crtcs[crtc->crtc_idx]);
 		crtc->crtc_id = drm_crtc->crtc_id;
 		drmModeFreeCrtc(drm_crtc);
-		crtc->pipe_id = kmstest_get_pipe_from_crtc_id(drm_fd,
-							      crtc->crtc_id);
 
 		crtc->connector_count = 1;
 		for (j = i + 1; j < connector_count; j++)
-- 
2.5.5

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

Re: [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915

[Daniel Vetter]

On Tue, Apr 19, 2016 at 01:40:26PM +0200, Tomeu Vizoso wrote:
> Hi,
> 
> following Daniel's comments on a previous series that completely removed
> usage of DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, I'm sending this one that
> removes it only on codepaths that can be run on drivers other than i915.
> 
> It does so by using the already-available crtc indexes instead of
> querying for the pipe number (which should return the same value).

Problem is a bit that KMS ids in general are really hard to tell apart. In
the kernel we started to have proper names for all of them, and let
drivers set those on a per-chip basis even, to make sure they match the hw
specs.

I do think keeping the pipe id around is somewhat useful, since for most
hw it's a valid concept to enumerate crtcs like that.
-Daniel

> 
> Thanks,
> 
> Tomeu
> 
> 
> Tomeu Vizoso (2):
>   lib: Remove superfluous kmstest_connector_config.pipe
>   tests/kms_setmode: Remove superfluous call to
>     kmstest_get_pipe_from_crtc_id
> 
>  lib/igt_kms.c                     | 24 +++++++++++-------------
>  lib/igt_kms.h                     |  1 -
>  tests/kms_crtc_background_color.c |  2 +-
>  tests/kms_flip.c                  | 10 +++++-----
>  tests/kms_flip_tiling.c           |  2 +-
>  tests/kms_panel_fitting.c         |  2 +-
>  tests/kms_plane_scaling.c         |  2 +-
>  tests/kms_render.c                |  2 +-
>  tests/kms_setmode.c               |  5 +----
>  tests/testdisplay.c               |  1 -
>  10 files changed, 22 insertions(+), 29 deletions(-)
> 
> -- 
> 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