[i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC

public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed

* [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915
@ 2016-04-19 11:40 Tomeu Vizoso
  2016-04-19 11:40 ` [i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe Tomeu Vizoso
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Tomeu Vizoso @ 2016-04-19 11:40 UTC (permalink / raw)
  To: Intel GFX discussion
  Cc: Daniel Stone, Tomeu Vizoso, Micah Fedke, Gustavo Padovan,
	Emil Velikov

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe
  2016-04-19 11:40 [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Tomeu Vizoso
@ 2016-04-19 11:40 ` Tomeu Vizoso
  2016-04-19 11:40 ` [i-g-t PATCH v1 2/2] tests/kms_setmode: Remove superfluous call to kmstest_get_pipe_from_crtc_id Tomeu Vizoso
  2016-04-20 14:14 ` [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Daniel Vetter
  2 siblings, 0 replies; 4+ messages in thread
From: Tomeu Vizoso @ 2016-04-19 11:40 UTC (permalink / raw)
  To: Intel GFX discussion
  Cc: Daniel Stone, Tomeu Vizoso, Micah Fedke, Gustavo Padovan,
	Emil Velikov

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [i-g-t PATCH v1 2/2] tests/kms_setmode: Remove superfluous call to kmstest_get_pipe_from_crtc_id
  2016-04-19 11:40 [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Tomeu Vizoso
  2016-04-19 11:40 ` [i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe Tomeu Vizoso
@ 2016-04-19 11:40 ` Tomeu Vizoso
  2016-04-20 14:14 ` [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Daniel Vetter
  2 siblings, 0 replies; 4+ messages in thread
From: Tomeu Vizoso @ 2016-04-19 11:40 UTC (permalink / raw)
  To: Intel GFX discussion
  Cc: Daniel Stone, Tomeu Vizoso, Micah Fedke, Gustavo Padovan,
	Emil Velikov

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915
  2016-04-19 11:40 [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Tomeu Vizoso
  2016-04-19 11:40 ` [i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe Tomeu Vizoso
  2016-04-19 11:40 ` [i-g-t PATCH v1 2/2] tests/kms_setmode: Remove superfluous call to kmstest_get_pipe_from_crtc_id Tomeu Vizoso
@ 2016-04-20 14:14 ` Daniel Vetter
  2 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2016-04-20 14:14 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Micah Fedke, Intel GFX discussion, Gustavo Padovan, Daniel Stone,
	Emil Velikov

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-04-20 14:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-19 11:40 [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Tomeu Vizoso
2016-04-19 11:40 ` [i-g-t PATCH v1 1/2] lib: Remove superfluous kmstest_connector_config.pipe Tomeu Vizoso
2016-04-19 11:40 ` [i-g-t PATCH v1 2/2] tests/kms_setmode: Remove superfluous call to kmstest_get_pipe_from_crtc_id Tomeu Vizoso
2016-04-20 14:14 ` [i-g-t PATCH v1 0/2] Avoid calling DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID on !i915 Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox