* [PATCH i-g-t 0/7] Misc fixes and cleanup
@ 2017-04-25 16:45 Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t RESEND 1/7] lib/igt_kms: Fix erroneous assert Brian Starkey
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
Hi,
This is not really a series but a set of miscellaneous fixes and
cleanup.
It should be mostly noncontroversial, with the exception of perhaps
the last two patches.
Patch 6 splits all of the igt_crc code out of igt_debugfs into a
separate file. The goal here was to make it easier to add new
CRC-related code without falling afoul of churn within igt_debugfs (of
which there's been a fair amount while I've been carrying these
patches). Eventually, the intention is to add support for igt_crc via
writeback connectors here, but I understand if this patch should stay
out until then.
Patch 7 allows "video=" on the kernel command-line to override a
connector's default mode. I don't know if that's widely useful, but we
like to use it on our FPGA platforms.
All comments/feedback welcome,
Thanks,
Brian
Brian Starkey (7):
lib/igt_kms: Fix erroneous assert
lib/igt_kms: Fix override_mode handling
lib: Stop igt_get_all_cairo_formats memory leak
lib/igt_debugfs: Remove igt_debugfs_t
lib/igt_debugfs: Only use valid values in igt_crc_to_str()
igt: lib/igt_crc: Split out CRC functionality
lib/igt_kms: Use kernel command line mode if specified
lib/Makefile.sources | 2 +
lib/igt_chamelium.h | 1 +
lib/igt_crc.c | 563 +++++++++++++++++++++++++++++++++++++
lib/igt_crc.h | 125 ++++++++
lib/igt_debugfs.c | 551 ------------------------------------
lib/igt_debugfs.h | 81 ------
lib/igt_fb.c | 4 +-
lib/igt_fb.h | 2 +-
lib/igt_kms.c | 157 ++++++++++-
tests/chamelium.c | 1 +
tests/kms_atomic.c | 3 +-
tests/kms_atomic_transition.c | 1 +
tests/kms_ccs.c | 1 +
tests/kms_chv_cursor_fail.c | 1 +
tests/kms_crtc_background_color.c | 1 +
tests/kms_cursor_crc.c | 1 +
tests/kms_cursor_legacy.c | 1 +
tests/kms_draw_crc.c | 1 +
tests/kms_fbc_crc.c | 1 +
tests/kms_flip_tiling.c | 1 +
tests/kms_frontbuffer_tracking.c | 1 +
tests/kms_mmap_write_crc.c | 1 +
tests/kms_mmio_vs_cs_flip.c | 1 +
tests/kms_pipe_color.c | 1 +
tests/kms_pipe_crc_basic.c | 1 +
tests/kms_plane.c | 1 +
tests/kms_plane_lowres.c | 1 +
tests/kms_plane_multiple.c | 1 +
tests/kms_plane_scaling.c | 1 +
tests/kms_pwrite_crc.c | 1 +
tests/kms_render.c | 4 +-
tests/kms_rotation_crc.c | 1 +
tests/kms_universal_plane.c | 1 +
tools/intel_display_crc.c | 1 +
34 files changed, 867 insertions(+), 649 deletions(-)
create mode 100644 lib/igt_crc.c
create mode 100644 lib/igt_crc.h
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH i-g-t RESEND 1/7] lib/igt_kms: Fix erroneous assert
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 2/7] lib/igt_kms: Fix override_mode handling Brian Starkey
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
In trying to fix igt_display_init() for devices without cursors, I
actually made matters worse. Fix the assert.
Fixes: 545aa3398223 lib/igt_kms: Remove redundant cursor code
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_kms.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 9bfa0e1d0695..5837a9f112a6 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1789,11 +1789,11 @@ void igt_display_init(igt_display_t *display, int drm_fd)
*/
igt_assert_eq(pipe->plane_primary, 0);
- /*
- * There should be no gaps. If there is, something happened
- * which we can't handle (e.g. all planes are cursors).
- */
- igt_assert_eq(p, last_plane);
+ /* Check that we filled every slot exactly once */
+ if (display->has_cursor_plane)
+ igt_assert_eq(p, last_plane);
+ else
+ igt_assert_eq(p, n_planes);
pipe->n_planes = n_planes;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 2/7] lib/igt_kms: Fix override_mode handling
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t RESEND 1/7] lib/igt_kms: Fix erroneous assert Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak Brian Starkey
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
igt_display_commit isn't refreshing all outputs anymore, which means
that an override mode may never get picked up.
Instead of forcing a reprobe to handle copying the override_mode into
default_mode, just change igt_output_get_mode() to return the
override_mode if it's been set, and remove the old code which would
directly overwrite default_mode.
This should be more robust, as igt_output_get_mode() will always return
the correct mode, without needing the output to be reprobed.
This change means that output->config.default_mode always contains the
"non-overridden" default mode.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_kms.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5837a9f112a6..474aa005b9fa 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1575,9 +1575,6 @@ static void igt_output_refresh(igt_output_t *output)
igt_atomic_fill_connector_props(display, output,
IGT_NUM_CONNECTOR_PROPS, igt_connector_prop_names);
- if (output->use_override_mode)
- output->config.default_mode = output->override_mode;
-
if (output->config.pipe == PIPE_NONE)
return;
@@ -2816,7 +2813,10 @@ const char *igt_output_name(igt_output_t *output)
drmModeModeInfo *igt_output_get_mode(igt_output_t *output)
{
- return &output->config.default_mode;
+ if (output->use_override_mode)
+ return &output->override_mode;
+ else
+ return &output->config.default_mode;
}
/**
@@ -2834,10 +2834,6 @@ void igt_output_override_mode(igt_output_t *output, drmModeModeInfo *mode)
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;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t RESEND 1/7] lib/igt_kms: Fix erroneous assert Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 2/7] lib/igt_kms: Fix override_mode handling Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 17:43 ` Gabriel Krisman Bertazi
2017-04-25 16:45 ` [PATCH i-g-t 4/7] lib/igt_debugfs: Remove igt_debugfs_t Brian Starkey
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
igt_get_all_cairo_formats() allocates the format list on the heap, but
returns it in a const pointer. Change this so that callers can free the
array without a warning, and free the array in all callers.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_fb.c | 4 +++-
lib/igt_fb.h | 2 +-
tests/kms_atomic.c | 3 ++-
tests/kms_render.c | 4 +++-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index d2b7e9e36606..b958c970973b 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1277,8 +1277,10 @@ const char *igt_format_str(uint32_t drm_format)
*
* This functions returns an array of all the drm fourcc codes supported by
* cairo and this library.
+ *
+ * The array should be freed by the caller.
*/
-void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count)
+void igt_get_all_cairo_formats(uint32_t **formats, int *format_count)
{
static uint32_t *drm_formats;
static int n_formats;
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index 4a680cefb16d..e124910367a3 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -154,7 +154,7 @@ int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align,
uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth);
uint32_t igt_drm_format_to_bpp(uint32_t drm_format);
const char *igt_format_str(uint32_t drm_format);
-void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count);
+void igt_get_all_cairo_formats(uint32_t **formats, int *format_count);
#endif /* __IGT_FB_H__ */
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index 6375fede7179..9c03f6e21ebb 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -807,7 +807,7 @@ static void atomic_state_free(struct kms_atomic_state *state)
static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane)
{
drmModePlanePtr plane_kms;
- const uint32_t *igt_formats;
+ uint32_t *igt_formats;
uint32_t ret = 0;
int num_igt_formats;
int i;
@@ -827,6 +827,7 @@ static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane)
}
}
+ free(igt_formats);
drmModeFreePlane(plane_kms);
return ret;
}
diff --git a/tests/kms_render.c b/tests/kms_render.c
index fd33dfb7cafe..bc2ffc750c67 100644
--- a/tests/kms_render.c
+++ b/tests/kms_render.c
@@ -176,7 +176,7 @@ static void test_connector(const char *test_name,
struct kmstest_connector_config *cconf,
enum test_flags flags)
{
- const uint32_t *formats;
+ uint32_t *formats;
int format_count;
int i;
@@ -193,6 +193,8 @@ static void test_connector(const char *test_name,
cconf, &cconf->connector->modes[0],
formats[i], flags);
}
+
+ free(formats);
}
static int run_test(const char *test_name, enum test_flags flags)
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 4/7] lib/igt_debugfs: Remove igt_debugfs_t
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
` (2 preceding siblings ...)
2017-04-25 16:45 ` [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 5/7] lib/igt_debugfs: Only use valid values in igt_crc_to_str() Brian Starkey
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
It's not used anymore, so remove it.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_debugfs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 7584be5f9cc1..f5ed3dafee8a 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -84,11 +84,6 @@
* General debugfs helpers
*/
-typedef struct {
- char root[128];
- char dri_path[128];
-} igt_debugfs_t;
-
static bool is_mountpoint(const char *path)
{
char buf[strlen(path) + 4];
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 5/7] lib/igt_debugfs: Only use valid values in igt_crc_to_str()
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
` (3 preceding siblings ...)
2017-04-25 16:45 ` [PATCH i-g-t 4/7] lib/igt_debugfs: Remove igt_debugfs_t Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 6/7] igt: lib/igt_crc: Split out CRC functionality Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified Brian Starkey
6 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
Not all elements in the crc array may be valid, so only use the valid
ones to generate the string.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_debugfs.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index f5ed3dafee8a..80f25c61597c 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -312,10 +312,11 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b)
*/
char *igt_crc_to_string(igt_crc_t *crc)
{
- char buf[128];
+ int i;
+ char buf[128] = { 0 };
- sprintf(buf, "%08x %08x %08x %08x %08x", crc->crc[0],
- crc->crc[1], crc->crc[2], crc->crc[3], crc->crc[4]);
+ for (i = 0; i < crc->n_words; i++)
+ sprintf(buf + strlen(buf), "%08x ", crc->crc[i]);
return strdup(buf);
}
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 6/7] igt: lib/igt_crc: Split out CRC functionality
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
` (4 preceding siblings ...)
2017-04-25 16:45 ` [PATCH i-g-t 5/7] lib/igt_debugfs: Only use valid values in igt_crc_to_str() Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified Brian Starkey
6 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
Separate out the CRC code for better compartmentalisation. Should ease
the addition of more/different CRC sources in the future.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/Makefile.sources | 2 +
lib/igt_chamelium.h | 1 +
lib/igt_crc.c | 563 +++++++++++++++++++++++++++++++++++++
lib/igt_crc.h | 125 ++++++++
lib/igt_debugfs.c | 547 -----------------------------------
lib/igt_debugfs.h | 81 ------
tests/chamelium.c | 1 +
tests/kms_atomic_transition.c | 1 +
tests/kms_ccs.c | 1 +
tests/kms_chv_cursor_fail.c | 1 +
tests/kms_crtc_background_color.c | 1 +
tests/kms_cursor_crc.c | 1 +
tests/kms_cursor_legacy.c | 1 +
tests/kms_draw_crc.c | 1 +
tests/kms_fbc_crc.c | 1 +
tests/kms_flip_tiling.c | 1 +
tests/kms_frontbuffer_tracking.c | 1 +
tests/kms_mmap_write_crc.c | 1 +
tests/kms_mmio_vs_cs_flip.c | 1 +
tests/kms_pipe_color.c | 1 +
tests/kms_pipe_crc_basic.c | 1 +
tests/kms_plane.c | 1 +
tests/kms_plane_lowres.c | 1 +
tests/kms_plane_multiple.c | 1 +
tests/kms_plane_scaling.c | 1 +
tests/kms_pwrite_crc.c | 1 +
tests/kms_rotation_crc.c | 1 +
tests/kms_universal_plane.c | 1 +
tools/intel_display_crc.c | 1 +
29 files changed, 714 insertions(+), 628 deletions(-)
create mode 100644 lib/igt_crc.c
create mode 100644 lib/igt_crc.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 6348487f1107..927ff8cb155a 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -11,6 +11,8 @@ lib_source_list = \
igt_debugfs.h \
igt_aux.c \
igt_aux.h \
+ igt_crc.c \
+ igt_crc.h \
igt_edid_template.h \
igt_gt.c \
igt_gt.h \
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index f421d8372876..ad6ca7739e4b 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -28,6 +28,7 @@
#include "config.h"
#include "igt.h"
+#include "igt_crc.h"
#include <stdbool.h>
struct chamelium;
diff --git a/lib/igt_crc.c b/lib/igt_crc.c
new file mode 100644
index 000000000000..91a0b5a8e04d
--- /dev/null
+++ b/lib/igt_crc.c
@@ -0,0 +1,563 @@
+/*
+ * Copyright © 2013 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 <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include "igt_aux.h"
+#include "igt_crc.h"
+#include "igt_core.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+
+/**
+ * igt_assert_crc_equal:
+ * @a: first pipe CRC value
+ * @b: second pipe CRC value
+ *
+ * Compares two CRC values and fails the testcase if they don't match with
+ * igt_fail(). Note that due to CRC collisions CRC based testcase can only
+ * assert that CRCs match, never that they are different. Otherwise there might
+ * be random testcase failures when different screen contents end up with the
+ * same CRC by chance.
+ */
+void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b)
+{
+ int i;
+
+ for (i = 0; i < a->n_words; i++)
+ igt_assert_eq_u32(a->crc[i], b->crc[i]);
+}
+
+/**
+ * igt_crc_to_string:
+ * @crc: pipe CRC value to print
+ *
+ * This formats @crc into a string buffer which is owned by igt_crc_to_string().
+ * The next call will override the buffer again, which makes this multithreading
+ * unsafe.
+ *
+ * This should only ever be used for diagnostic debug output.
+ */
+char *igt_crc_to_string(igt_crc_t *crc)
+{
+ int i;
+ char buf[128] = { 0 };
+
+ for (i = 0; i < crc->n_words; i++)
+ sprintf(buf + strlen(buf), "%08x ", crc->crc[i]);
+
+ return strdup(buf);
+}
+
+#define MAX_CRC_ENTRIES 10
+#define MAX_LINE_LEN (10 + 11 * MAX_CRC_ENTRIES + 1)
+
+/* (6 fields, 8 chars each, space separated (5) + '\n') */
+#define LEGACY_LINE_LEN (6 * 8 + 5 + 1)
+
+struct _igt_pipe_crc {
+ int fd;
+ int dir;
+ int ctl_fd;
+ int crc_fd;
+ int flags;
+ bool is_legacy;
+
+ enum pipe pipe;
+ enum intel_pipe_crc_source source;
+};
+
+static const char *pipe_crc_sources[] = {
+ "none",
+ "plane1",
+ "plane2",
+ "pf",
+ "pipe",
+ "TV",
+ "DP-B",
+ "DP-C",
+ "DP-D",
+ "auto"
+};
+
+static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
+{
+ return pipe_crc_sources[source];
+}
+
+static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
+{
+ char buf[64];
+
+ /* Stop first just to make sure we don't have lingering state left. */
+ igt_pipe_crc_stop(pipe_crc);
+
+ if (pipe_crc->is_legacy)
+ sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
+ pipe_crc_source_name(pipe_crc->source));
+ else
+ sprintf(buf, "%s", pipe_crc_source_name(pipe_crc->source));
+
+ igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
+
+ if (!pipe_crc->is_legacy) {
+ int err;
+
+ sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
+ err = 0;
+
+ pipe_crc->crc_fd = openat(pipe_crc->dir, buf, pipe_crc->flags);
+ if (pipe_crc->crc_fd < 0)
+ err = -errno;
+
+ if (err == -EINVAL)
+ return false;
+
+ igt_assert_eq(err, 0);
+ }
+
+ errno = 0;
+ return true;
+}
+
+static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
+{
+ char buf[32];
+
+ sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
+ igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
+}
+
+static void igt_pipe_crc_reset(int drm_fd)
+{
+ struct dirent *dirent;
+ const char *cmd = "none";
+ bool done = false;
+ DIR *dir;
+ int fdir;
+ int fd;
+
+ fdir = igt_debugfs_dir(drm_fd);
+ if (fdir < 0)
+ return;
+
+ dir = fdopendir(fdir);
+ if (!dir) {
+ close(fdir);
+ return;
+ }
+
+ while ((dirent = readdir(dir))) {
+ char buf[128];
+
+ if (strcmp(dirent->d_name, "crtc-") != 0)
+ continue;
+
+ sprintf(buf, "%s/crc/control", dirent->d_name);
+ fd = openat(fdir, buf, O_WRONLY);
+ if (fd < 0)
+ continue;
+
+ igt_assert_eq(write(fd, cmd, strlen(cmd)), strlen(cmd));
+ close(fd);
+
+ done = true;
+ }
+ closedir(dir);
+
+ if (!done) {
+ fd = openat(fdir, "i915_display_crtc_ctl", O_WRONLY);
+ if (fd != -1) {
+ igt_pipe_crc_pipe_off(fd, PIPE_A);
+ igt_pipe_crc_pipe_off(fd, PIPE_B);
+ igt_pipe_crc_pipe_off(fd, PIPE_C);
+
+ close(fd);
+ }
+ }
+
+ close(fdir);
+}
+
+static void pipe_crc_exit_handler(int sig)
+{
+ struct dirent *dirent;
+ char buf[128];
+ DIR *dir;
+ int fd;
+
+ dir = opendir("/dev/dri");
+ if (!dir)
+ return;
+
+ /*
+ * Try to reset CRC capture for all DRM devices, this is only needed
+ * for the legacy CRC ABI and can be completely removed once the
+ * legacy codepaths are removed.
+ */
+ while ((dirent = readdir(dir))) {
+ if (strncmp(dirent->d_name, "card", 4) != 0)
+ continue;
+
+ sprintf(buf, "/dev/dri/%s", dirent->d_name);
+ fd = open(buf, O_WRONLY);
+
+ igt_pipe_crc_reset(fd);
+
+ close(fd);
+ }
+ closedir(dir);
+}
+
+/**
+ * igt_require_pipe_crc:
+ *
+ * Convenience helper to check whether pipe CRC capturing is supported by the
+ * kernel. Uses igt_skip to automatically skip the test/subtest if this isn't
+ * the case.
+ */
+void igt_require_pipe_crc(int fd)
+{
+ const char *cmd = "pipe A none";
+ int ctl, written;
+
+ ctl = igt_debugfs_open(fd, "crtc-0/crc/control", O_RDONLY);
+ if (ctl < 0) {
+ ctl = igt_debugfs_open(fd, "i915_display_crc_ctl", O_WRONLY);
+ igt_require_f(ctl,
+ "No display_crc_ctl found, kernel too old\n");
+
+ written = write(ctl, cmd, strlen(cmd));
+ igt_require_f(written < 0,
+ "CRCs not supported on this platform\n");
+ }
+ close(ctl);
+}
+
+static igt_pipe_crc_t *
+pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source, int flags)
+{
+ igt_pipe_crc_t *pipe_crc;
+ char buf[128];
+ int debugfs;
+
+ debugfs = igt_debugfs_dir(fd);
+ igt_assert(debugfs != -1);
+
+ igt_install_exit_handler(pipe_crc_exit_handler);
+
+ pipe_crc = calloc(1, sizeof(struct _igt_pipe_crc));
+
+ sprintf(buf, "crtc-%d/crc/control", pipe);
+ pipe_crc->ctl_fd = openat(debugfs, buf, O_WRONLY);
+ if (pipe_crc->ctl_fd == -1) {
+ pipe_crc->ctl_fd = openat(debugfs,
+ "i915_display_crc_ctl", O_WRONLY);
+ igt_assert(pipe_crc->ctl_fd != -1);
+ pipe_crc->is_legacy = true;
+ }
+
+ if (pipe_crc->is_legacy) {
+ sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
+ pipe_crc->crc_fd = openat(debugfs, buf, flags);
+ igt_assert(pipe_crc->crc_fd != -1);
+ igt_debug("Using legacy frame CRC ABI\n");
+ } else {
+ pipe_crc->crc_fd = -1;
+ igt_debug("Using generic frame CRC ABI\n");
+ }
+
+ pipe_crc->fd = fd;
+ pipe_crc->dir = debugfs;
+ pipe_crc->pipe = pipe;
+ pipe_crc->source = source;
+ pipe_crc->flags = flags;
+
+ return pipe_crc;
+}
+
+/**
+ * igt_pipe_crc_new:
+ * @pipe: display pipe to use as source
+ * @source: CRC tap point to use as source
+ *
+ * This sets up a new pipe CRC capture object for the given @pipe and @source
+ * in blocking mode.
+ *
+ * Returns: A pipe CRC object for the given @pipe and @source. The library
+ * assumes that the source is always available since recent kernels support at
+ * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
+ */
+igt_pipe_crc_t *
+igt_pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source)
+{
+ return pipe_crc_new(fd, pipe, source, O_RDONLY);
+}
+
+/**
+ * igt_pipe_crc_new_nonblock:
+ * @pipe: display pipe to use as source
+ * @source: CRC tap point to use as source
+ *
+ * This sets up a new pipe CRC capture object for the given @pipe and @source
+ * in nonblocking mode.
+ *
+ * Returns: A pipe CRC object for the given @pipe and @source. The library
+ * assumes that the source is always available since recent kernels support at
+ * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
+ */
+igt_pipe_crc_t *
+igt_pipe_crc_new_nonblock(int fd, enum pipe pipe, enum intel_pipe_crc_source source)
+{
+ return pipe_crc_new(fd, pipe, source, O_RDONLY | O_NONBLOCK);
+}
+
+/**
+ * igt_pipe_crc_free:
+ * @pipe_crc: pipe CRC object
+ *
+ * Frees all resources associated with @pipe_crc.
+ */
+void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
+{
+ if (!pipe_crc)
+ return;
+
+ close(pipe_crc->ctl_fd);
+ close(pipe_crc->crc_fd);
+ close(pipe_crc->dir);
+ free(pipe_crc);
+}
+
+static bool pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc,
+ const char *line)
+{
+ int n, i;
+ const char *buf;
+
+ if (pipe_crc->is_legacy) {
+ crc->has_valid_frame = true;
+ crc->n_words = 5;
+ n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame,
+ &crc->crc[0], &crc->crc[1], &crc->crc[2],
+ &crc->crc[3], &crc->crc[4]);
+ return n == 6;
+ }
+
+ if (strncmp(line, "XXXXXXXXXX", 10) == 0)
+ crc->has_valid_frame = false;
+ else {
+ crc->has_valid_frame = true;
+ crc->frame = strtoul(line, NULL, 16);
+ }
+
+ buf = line + 10;
+ for (i = 0; *buf != '\n'; i++, buf += 11)
+ crc->crc[i] = strtoul(buf, NULL, 16);
+
+ crc->n_words = i;
+
+ return true;
+}
+
+static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
+{
+ ssize_t bytes_read;
+ char buf[MAX_LINE_LEN + 1];
+ size_t read_len;
+
+ if (pipe_crc->is_legacy)
+ read_len = LEGACY_LINE_LEN;
+ else
+ read_len = MAX_LINE_LEN;
+
+ igt_set_timeout(5, "CRC reading");
+ bytes_read = read(pipe_crc->crc_fd, &buf, read_len);
+ igt_reset_timeout();
+
+ if (bytes_read < 0 && errno == EAGAIN)
+ igt_assert(pipe_crc->flags & O_NONBLOCK);
+
+ if (bytes_read < 0)
+ bytes_read = 0;
+
+ buf[bytes_read] = '\0';
+
+ if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, buf))
+ return -EINVAL;
+
+ return bytes_read;
+}
+
+static void read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
+{
+ while (read_crc(pipe_crc, out) == 0)
+ usleep(1000);
+}
+
+/**
+ * igt_pipe_crc_start:
+ * @pipe_crc: pipe CRC object
+ *
+ * Starts the CRC capture process on @pipe_crc.
+ */
+void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
+{
+ igt_crc_t crc;
+
+ igt_assert(igt_pipe_crc_do_start(pipe_crc));
+
+ if (pipe_crc->is_legacy) {
+ /*
+ * For some no yet identified reason, the first CRC is
+ * bonkers. So let's just wait for the next vblank and read
+ * out the buggy result.
+ *
+ * On CHV sometimes the second CRC is bonkers as well, so
+ * don't trust that one either.
+ */
+ read_one_crc(pipe_crc, &crc);
+ read_one_crc(pipe_crc, &crc);
+ }
+}
+
+/**
+ * igt_pipe_crc_stop:
+ * @pipe_crc: pipe CRC object
+ *
+ * Stops the CRC capture process on @pipe_crc.
+ */
+void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
+{
+ char buf[32];
+
+ if (pipe_crc->is_legacy) {
+ sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
+ igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)),
+ strlen(buf));
+ } else {
+ close(pipe_crc->crc_fd);
+ pipe_crc->crc_fd = -1;
+ }
+}
+
+/**
+ * igt_pipe_crc_get_crcs:
+ * @pipe_crc: pipe CRC object
+ * @n_crcs: number of CRCs to capture
+ * @out_crcs: buffer pointer for the captured CRC values
+ *
+ * Read up to @n_crcs from @pipe_crc. This function does not block, and will
+ * return early if not enough CRCs can be captured, if @pipe_crc has been
+ * opened using igt_pipe_crc_new_nonblock(). It will block until @n_crcs are
+ * retrieved if @pipe_crc has been opened using igt_pipe_crc_new(). @out_crcs is
+ * alloced by this function and must be released with free() by the caller.
+ *
+ * Callers must start and stop the capturing themselves by calling
+ * igt_pipe_crc_start() and igt_pipe_crc_stop(). For one-shot CRC collecting
+ * look at igt_pipe_crc_collect_crc().
+ *
+ * Returns:
+ * The number of CRCs captured. Should be equal to @n_crcs in blocking mode, but
+ * can be less (even zero) in non-blocking mode.
+ */
+int
+igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
+ igt_crc_t **out_crcs)
+{
+ igt_crc_t *crcs;
+ int n = 0;
+
+ crcs = calloc(n_crcs, sizeof(igt_crc_t));
+
+ do {
+ igt_crc_t *crc = &crcs[n];
+ int ret;
+
+ ret = read_crc(pipe_crc, crc);
+ if (ret < 0)
+ continue;
+ if (ret == 0)
+ break;
+
+ n++;
+ } while (n < n_crcs);
+
+ *out_crcs = crcs;
+ return n;
+}
+
+static void crc_sanity_checks(igt_crc_t *crc)
+{
+ int i;
+ bool all_zero = true;
+
+ for (i = 0; i < crc->n_words; i++) {
+ igt_warn_on_f(crc->crc[i] == 0xffffffff,
+ "Suspicious CRC: it looks like the CRC "
+ "read back was from a register in a powered "
+ "down well\n");
+ if (crc->crc[i])
+ all_zero = false;
+ }
+
+ igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
+}
+
+/**
+ * igt_pipe_crc_collect_crc:
+ * @pipe_crc: pipe CRC object
+ * @out_crc: buffer for the captured CRC values
+ *
+ * Read a single CRC from @pipe_crc. This function blocks until the CRC is
+ * retrieved, irrespective of whether @pipe_crc has been opened with
+ * igt_pipe_crc_new() or igt_pipe_crc_new_nonblock(). @out_crc must be
+ * allocated by the caller.
+ *
+ * This function takes care of the pipe_crc book-keeping, it will start/stop
+ * the collection of the CRC.
+ *
+ * This function also calls the interactive debug with the "crc" domain, so you
+ * can make use of this feature to actually see the screen that is being CRC'd.
+ *
+ * For continuous CRC collection look at igt_pipe_crc_start(),
+ * igt_pipe_crc_get_crcs() and igt_pipe_crc_stop().
+ */
+void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc)
+{
+ igt_debug_wait_for_keypress("crc");
+
+ igt_pipe_crc_start(pipe_crc);
+ read_one_crc(pipe_crc, out_crc);
+ igt_pipe_crc_stop(pipe_crc);
+
+ crc_sanity_checks(out_crc);
+}
+
diff --git a/lib/igt_crc.h b/lib/igt_crc.h
new file mode 100644
index 000000000000..b0623baf4108
--- /dev/null
+++ b/lib/igt_crc.h
@@ -0,0 +1,125 @@
+/*
+ * Copyright © 2013 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.
+ *
+ * # Pipe CRC Support
+ *
+ * This library wraps up the kernel's support for capturing pipe CRCs into a
+ * neat and tidy package. For the detailed usage see all the functions which
+ * work on #igt_pipe_crc_t. This is supported on all platforms and outputs.
+ *
+ * Actually using pipe CRCs to write modeset tests is a bit tricky though, so
+ * there is no way to directly check a CRC: Both the details of the plane
+ * blending, color correction and other hardware and how exactly the CRC is
+ * computed at each tap point vary by hardware generation and are not disclosed.
+ *
+ * The only way to use #igt_crc_t CRCs therefore is to compare CRCs among each
+ * another either for equality or difference. Otherwise CRCs must be treated as
+ * completely opaque values. Note that not even CRCs from different pipes or tap
+ * points on the same platform can be compared. Hence only use
+ * igt_assert_crc_equal() to inspect CRC values captured by the same
+ * #igt_pipe_crc_t object.
+ */
+
+#ifndef __IGT_CRC_H__
+#define __IGT_CRC_H__
+
+#include <stdbool.h>
+#include <stdint.h>
+
+enum pipe;
+
+/**
+ * igt_pipe_crc_t:
+ *
+ * Pipe CRC support structure. Needs to be allocated and set up with
+ * igt_pipe_crc_new() for a specific pipe and pipe CRC source value.
+ */
+typedef struct _igt_pipe_crc igt_pipe_crc_t;
+
+#define DRM_MAX_CRC_NR 10
+/**
+ * igt_crc_t:
+ * @frame: frame number of the capture CRC
+ * @n_words: internal field, don't access
+ * @crc: internal field, don't access
+ *
+ * Pipe CRC value. All other members than @frame are private and should not be
+ * inspected by testcases.
+ */
+typedef struct {
+ uint32_t frame;
+ bool has_valid_frame;
+ int n_words;
+ uint32_t crc[DRM_MAX_CRC_NR];
+} igt_crc_t;
+
+/**
+ * intel_pipe_crc_source:
+ * @INTEL_PIPE_CRC_SOURCE_NONE: No source
+ * @INTEL_PIPE_CRC_SOURCE_PLANE1: Plane 1
+ * @INTEL_PIPE_CRC_SOURCE_PLANE2: Plane 2
+ * @INTEL_PIPE_CRC_SOURCE_PF: Panel Filter
+ * @INTEL_PIPE_CRC_SOURCE_PIPE: Pipe
+ * @INTEL_PIPE_CRC_SOURCE_TV: TV
+ * @INTEL_PIPE_CRC_SOURCE_DP_B: DisplayPort B
+ * @INTEL_PIPE_CRC_SOURCE_DP_C: DisplayPort C
+ * @INTEL_PIPE_CRC_SOURCE_DP_D: DisplayPort D
+ * @INTEL_PIPE_CRC_SOURCE_AUTO: Automatic source selection
+ * @INTEL_PIPE_CRC_SOURCE_MAX: Number of available sources
+ *
+ * Enumeration of all supported pipe CRC sources. Not all platforms and all
+ * outputs support all of them. Generic tests should just use
+ * INTEL_PIPE_CRC_SOURCE_AUTO. It should always map to an end-of-pipe CRC
+ * suitable for checking planes, cursor, color correction and any other
+ * output-agnostic features.
+ */
+enum intel_pipe_crc_source {
+ INTEL_PIPE_CRC_SOURCE_NONE,
+ INTEL_PIPE_CRC_SOURCE_PLANE1,
+ INTEL_PIPE_CRC_SOURCE_PLANE2,
+ INTEL_PIPE_CRC_SOURCE_PF,
+ INTEL_PIPE_CRC_SOURCE_PIPE,
+ INTEL_PIPE_CRC_SOURCE_TV,
+ INTEL_PIPE_CRC_SOURCE_DP_B,
+ INTEL_PIPE_CRC_SOURCE_DP_C,
+ INTEL_PIPE_CRC_SOURCE_DP_D,
+ INTEL_PIPE_CRC_SOURCE_AUTO,
+ INTEL_PIPE_CRC_SOURCE_MAX,
+};
+
+void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
+char *igt_crc_to_string(igt_crc_t *crc);
+
+void igt_require_pipe_crc(int fd);
+igt_pipe_crc_t *
+igt_pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source);
+igt_pipe_crc_t *
+igt_pipe_crc_new_nonblock(int fd, enum pipe pipe, enum intel_pipe_crc_source source);
+void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
+void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
+void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
+__attribute__((warn_unused_result))
+int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
+ igt_crc_t **out_crcs);
+void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
+
+#endif /* __IGT_CRC_H__ */
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 80f25c61597c..e08b7ae846bf 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -51,24 +51,6 @@
* basic functions to access debugfs files with e.g. igt_debugfs_open() it also
* provides higher-level wrappers for some debugfs features.
*
- * # Pipe CRC Support
- *
- * This library wraps up the kernel's support for capturing pipe CRCs into a
- * neat and tidy package. For the detailed usage see all the functions which
- * work on #igt_pipe_crc_t. This is supported on all platforms and outputs.
- *
- * Actually using pipe CRCs to write modeset tests is a bit tricky though, so
- * there is no way to directly check a CRC: Both the details of the plane
- * blending, color correction and other hardware and how exactly the CRC is
- * computed at each tap point vary by hardware generation and are not disclosed.
- *
- * The only way to use #igt_crc_t CRCs therefore is to compare CRCs among each
- * another either for equality or difference. Otherwise CRCs must be treated as
- * completely opaque values. Note that not even CRCs from different pipes or tap
- * points on the same platform can be compared. Hence only use
- * igt_assert_crc_equal() to inspect CRC values captured by the same
- * #igt_pipe_crc_t object.
- *
* # Other debugfs interface wrappers
*
* This covers the miscellaneous debugfs interface wrappers:
@@ -277,235 +259,6 @@ bool igt_debugfs_search(int device, const char *filename, const char *substring)
return matched;
}
-/*
- * Pipe CRC
- */
-
-/**
- * igt_assert_crc_equal:
- * @a: first pipe CRC value
- * @b: second pipe CRC value
- *
- * Compares two CRC values and fails the testcase if they don't match with
- * igt_fail(). Note that due to CRC collisions CRC based testcase can only
- * assert that CRCs match, never that they are different. Otherwise there might
- * be random testcase failures when different screen contents end up with the
- * same CRC by chance.
- */
-void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b)
-{
- int i;
-
- for (i = 0; i < a->n_words; i++)
- igt_assert_eq_u32(a->crc[i], b->crc[i]);
-}
-
-/**
- * igt_crc_to_string:
- * @crc: pipe CRC value to print
- *
- * This formats @crc into a string buffer which is owned by igt_crc_to_string().
- * The next call will override the buffer again, which makes this multithreading
- * unsafe.
- *
- * This should only ever be used for diagnostic debug output.
- */
-char *igt_crc_to_string(igt_crc_t *crc)
-{
- int i;
- char buf[128] = { 0 };
-
- for (i = 0; i < crc->n_words; i++)
- sprintf(buf + strlen(buf), "%08x ", crc->crc[i]);
-
- return strdup(buf);
-}
-
-#define MAX_CRC_ENTRIES 10
-#define MAX_LINE_LEN (10 + 11 * MAX_CRC_ENTRIES + 1)
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define LEGACY_LINE_LEN (6 * 8 + 5 + 1)
-
-struct _igt_pipe_crc {
- int fd;
- int dir;
- int ctl_fd;
- int crc_fd;
- int flags;
- bool is_legacy;
-
- enum pipe pipe;
- enum intel_pipe_crc_source source;
-};
-
-static const char *pipe_crc_sources[] = {
- "none",
- "plane1",
- "plane2",
- "pf",
- "pipe",
- "TV",
- "DP-B",
- "DP-C",
- "DP-D",
- "auto"
-};
-
-static const char *pipe_crc_source_name(enum intel_pipe_crc_source source)
-{
- return pipe_crc_sources[source];
-}
-
-static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
-{
- char buf[64];
-
- /* Stop first just to make sure we don't have lingering state left. */
- igt_pipe_crc_stop(pipe_crc);
-
- if (pipe_crc->is_legacy)
- sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
- pipe_crc_source_name(pipe_crc->source));
- else
- sprintf(buf, "%s", pipe_crc_source_name(pipe_crc->source));
-
- igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
-
- if (!pipe_crc->is_legacy) {
- int err;
-
- sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
- err = 0;
-
- pipe_crc->crc_fd = openat(pipe_crc->dir, buf, pipe_crc->flags);
- if (pipe_crc->crc_fd < 0)
- err = -errno;
-
- if (err == -EINVAL)
- return false;
-
- igt_assert_eq(err, 0);
- }
-
- errno = 0;
- return true;
-}
-
-static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
-{
- char buf[32];
-
- sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
- igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
-}
-
-static void igt_pipe_crc_reset(int drm_fd)
-{
- struct dirent *dirent;
- const char *cmd = "none";
- bool done = false;
- DIR *dir;
- int fdir;
- int fd;
-
- fdir = igt_debugfs_dir(drm_fd);
- if (fdir < 0)
- return;
-
- dir = fdopendir(fdir);
- if (!dir) {
- close(fdir);
- return;
- }
-
- while ((dirent = readdir(dir))) {
- char buf[128];
-
- if (strcmp(dirent->d_name, "crtc-") != 0)
- continue;
-
- sprintf(buf, "%s/crc/control", dirent->d_name);
- fd = openat(fdir, buf, O_WRONLY);
- if (fd < 0)
- continue;
-
- igt_assert_eq(write(fd, cmd, strlen(cmd)), strlen(cmd));
- close(fd);
-
- done = true;
- }
- closedir(dir);
-
- if (!done) {
- fd = openat(fdir, "i915_display_crtc_ctl", O_WRONLY);
- if (fd != -1) {
- igt_pipe_crc_pipe_off(fd, PIPE_A);
- igt_pipe_crc_pipe_off(fd, PIPE_B);
- igt_pipe_crc_pipe_off(fd, PIPE_C);
-
- close(fd);
- }
- }
-
- close(fdir);
-}
-
-static void pipe_crc_exit_handler(int sig)
-{
- struct dirent *dirent;
- char buf[128];
- DIR *dir;
- int fd;
-
- dir = opendir("/dev/dri");
- if (!dir)
- return;
-
- /*
- * Try to reset CRC capture for all DRM devices, this is only needed
- * for the legacy CRC ABI and can be completely removed once the
- * legacy codepaths are removed.
- */
- while ((dirent = readdir(dir))) {
- if (strncmp(dirent->d_name, "card", 4) != 0)
- continue;
-
- sprintf(buf, "/dev/dri/%s", dirent->d_name);
- fd = open(buf, O_WRONLY);
-
- igt_pipe_crc_reset(fd);
-
- close(fd);
- }
- closedir(dir);
-}
-
-/**
- * igt_require_pipe_crc:
- *
- * Convenience helper to check whether pipe CRC capturing is supported by the
- * kernel. Uses igt_skip to automatically skip the test/subtest if this isn't
- * the case.
- */
-void igt_require_pipe_crc(int fd)
-{
- const char *cmd = "pipe A none";
- int ctl, written;
-
- ctl = igt_debugfs_open(fd, "crtc-0/crc/control", O_RDONLY);
- if (ctl < 0) {
- ctl = igt_debugfs_open(fd, "i915_display_crc_ctl", O_WRONLY);
- igt_require_f(ctl,
- "No display_crc_ctl found, kernel too old\n");
-
- written = write(ctl, cmd, strlen(cmd));
- igt_require_f(written < 0,
- "CRCs not supported on this platform\n");
- }
- close(ctl);
-}
-
static void igt_hpd_storm_exit_handler(int sig)
{
int fd = drm_open_driver_master(DRIVER_INTEL);
@@ -627,306 +380,6 @@ void igt_require_hpd_storm_ctl(int drm_fd)
close(fd);
}
-static igt_pipe_crc_t *
-pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source, int flags)
-{
- igt_pipe_crc_t *pipe_crc;
- char buf[128];
- int debugfs;
-
- debugfs = igt_debugfs_dir(fd);
- igt_assert(debugfs != -1);
-
- igt_install_exit_handler(pipe_crc_exit_handler);
-
- pipe_crc = calloc(1, sizeof(struct _igt_pipe_crc));
-
- sprintf(buf, "crtc-%d/crc/control", pipe);
- pipe_crc->ctl_fd = openat(debugfs, buf, O_WRONLY);
- if (pipe_crc->ctl_fd == -1) {
- pipe_crc->ctl_fd = openat(debugfs,
- "i915_display_crc_ctl", O_WRONLY);
- igt_assert(pipe_crc->ctl_fd != -1);
- pipe_crc->is_legacy = true;
- }
-
- if (pipe_crc->is_legacy) {
- sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
- pipe_crc->crc_fd = openat(debugfs, buf, flags);
- igt_assert(pipe_crc->crc_fd != -1);
- igt_debug("Using legacy frame CRC ABI\n");
- } else {
- pipe_crc->crc_fd = -1;
- igt_debug("Using generic frame CRC ABI\n");
- }
-
- pipe_crc->fd = fd;
- pipe_crc->dir = debugfs;
- pipe_crc->pipe = pipe;
- pipe_crc->source = source;
- pipe_crc->flags = flags;
-
- return pipe_crc;
-}
-
-/**
- * igt_pipe_crc_new:
- * @pipe: display pipe to use as source
- * @source: CRC tap point to use as source
- *
- * This sets up a new pipe CRC capture object for the given @pipe and @source
- * in blocking mode.
- *
- * Returns: A pipe CRC object for the given @pipe and @source. The library
- * assumes that the source is always available since recent kernels support at
- * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
- */
-igt_pipe_crc_t *
-igt_pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source)
-{
- return pipe_crc_new(fd, pipe, source, O_RDONLY);
-}
-
-/**
- * igt_pipe_crc_new_nonblock:
- * @pipe: display pipe to use as source
- * @source: CRC tap point to use as source
- *
- * This sets up a new pipe CRC capture object for the given @pipe and @source
- * in nonblocking mode.
- *
- * Returns: A pipe CRC object for the given @pipe and @source. The library
- * assumes that the source is always available since recent kernels support at
- * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
- */
-igt_pipe_crc_t *
-igt_pipe_crc_new_nonblock(int fd, enum pipe pipe, enum intel_pipe_crc_source source)
-{
- return pipe_crc_new(fd, pipe, source, O_RDONLY | O_NONBLOCK);
-}
-
-/**
- * igt_pipe_crc_free:
- * @pipe_crc: pipe CRC object
- *
- * Frees all resources associated with @pipe_crc.
- */
-void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
-{
- if (!pipe_crc)
- return;
-
- close(pipe_crc->ctl_fd);
- close(pipe_crc->crc_fd);
- close(pipe_crc->dir);
- free(pipe_crc);
-}
-
-static bool pipe_crc_init_from_string(igt_pipe_crc_t *pipe_crc, igt_crc_t *crc,
- const char *line)
-{
- int n, i;
- const char *buf;
-
- if (pipe_crc->is_legacy) {
- crc->has_valid_frame = true;
- crc->n_words = 5;
- n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame,
- &crc->crc[0], &crc->crc[1], &crc->crc[2],
- &crc->crc[3], &crc->crc[4]);
- return n == 6;
- }
-
- if (strncmp(line, "XXXXXXXXXX", 10) == 0)
- crc->has_valid_frame = false;
- else {
- crc->has_valid_frame = true;
- crc->frame = strtoul(line, NULL, 16);
- }
-
- buf = line + 10;
- for (i = 0; *buf != '\n'; i++, buf += 11)
- crc->crc[i] = strtoul(buf, NULL, 16);
-
- crc->n_words = i;
-
- return true;
-}
-
-static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
-{
- ssize_t bytes_read;
- char buf[MAX_LINE_LEN + 1];
- size_t read_len;
-
- if (pipe_crc->is_legacy)
- read_len = LEGACY_LINE_LEN;
- else
- read_len = MAX_LINE_LEN;
-
- igt_set_timeout(5, "CRC reading");
- bytes_read = read(pipe_crc->crc_fd, &buf, read_len);
- igt_reset_timeout();
-
- if (bytes_read < 0 && errno == EAGAIN)
- igt_assert(pipe_crc->flags & O_NONBLOCK);
-
- if (bytes_read < 0)
- bytes_read = 0;
-
- buf[bytes_read] = '\0';
-
- if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, buf))
- return -EINVAL;
-
- return bytes_read;
-}
-
-static void read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
-{
- while (read_crc(pipe_crc, out) == 0)
- usleep(1000);
-}
-
-/**
- * igt_pipe_crc_start:
- * @pipe_crc: pipe CRC object
- *
- * Starts the CRC capture process on @pipe_crc.
- */
-void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
-{
- igt_crc_t crc;
-
- igt_assert(igt_pipe_crc_do_start(pipe_crc));
-
- if (pipe_crc->is_legacy) {
- /*
- * For some no yet identified reason, the first CRC is
- * bonkers. So let's just wait for the next vblank and read
- * out the buggy result.
- *
- * On CHV sometimes the second CRC is bonkers as well, so
- * don't trust that one either.
- */
- read_one_crc(pipe_crc, &crc);
- read_one_crc(pipe_crc, &crc);
- }
-}
-
-/**
- * igt_pipe_crc_stop:
- * @pipe_crc: pipe CRC object
- *
- * Stops the CRC capture process on @pipe_crc.
- */
-void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
-{
- char buf[32];
-
- if (pipe_crc->is_legacy) {
- sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
- igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)),
- strlen(buf));
- } else {
- close(pipe_crc->crc_fd);
- pipe_crc->crc_fd = -1;
- }
-}
-
-/**
- * igt_pipe_crc_get_crcs:
- * @pipe_crc: pipe CRC object
- * @n_crcs: number of CRCs to capture
- * @out_crcs: buffer pointer for the captured CRC values
- *
- * Read up to @n_crcs from @pipe_crc. This function does not block, and will
- * return early if not enough CRCs can be captured, if @pipe_crc has been
- * opened using igt_pipe_crc_new_nonblock(). It will block until @n_crcs are
- * retrieved if @pipe_crc has been opened using igt_pipe_crc_new(). @out_crcs is
- * alloced by this function and must be released with free() by the caller.
- *
- * Callers must start and stop the capturing themselves by calling
- * igt_pipe_crc_start() and igt_pipe_crc_stop(). For one-shot CRC collecting
- * look at igt_pipe_crc_collect_crc().
- *
- * Returns:
- * The number of CRCs captured. Should be equal to @n_crcs in blocking mode, but
- * can be less (even zero) in non-blocking mode.
- */
-int
-igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
- igt_crc_t **out_crcs)
-{
- igt_crc_t *crcs;
- int n = 0;
-
- crcs = calloc(n_crcs, sizeof(igt_crc_t));
-
- do {
- igt_crc_t *crc = &crcs[n];
- int ret;
-
- ret = read_crc(pipe_crc, crc);
- if (ret < 0)
- continue;
- if (ret == 0)
- break;
-
- n++;
- } while (n < n_crcs);
-
- *out_crcs = crcs;
- return n;
-}
-
-static void crc_sanity_checks(igt_crc_t *crc)
-{
- int i;
- bool all_zero = true;
-
- for (i = 0; i < crc->n_words; i++) {
- igt_warn_on_f(crc->crc[i] == 0xffffffff,
- "Suspicious CRC: it looks like the CRC "
- "read back was from a register in a powered "
- "down well\n");
- if (crc->crc[i])
- all_zero = false;
- }
-
- igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
-}
-
-/**
- * igt_pipe_crc_collect_crc:
- * @pipe_crc: pipe CRC object
- * @out_crc: buffer for the captured CRC values
- *
- * Read a single CRC from @pipe_crc. This function blocks until the CRC is
- * retrieved, irrespective of whether @pipe_crc has been opened with
- * igt_pipe_crc_new() or igt_pipe_crc_new_nonblock(). @out_crc must be
- * allocated by the caller.
- *
- * This function takes care of the pipe_crc book-keeping, it will start/stop
- * the collection of the CRC.
- *
- * This function also calls the interactive debug with the "crc" domain, so you
- * can make use of this feature to actually see the screen that is being CRC'd.
- *
- * For continuous CRC collection look at igt_pipe_crc_start(),
- * igt_pipe_crc_get_crcs() and igt_pipe_crc_stop().
- */
-void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc)
-{
- igt_debug_wait_for_keypress("crc");
-
- igt_pipe_crc_start(pipe_crc);
- read_one_crc(pipe_crc, out_crc);
- igt_pipe_crc_stop(pipe_crc);
-
- crc_sanity_checks(out_crc);
-}
-
/*
* Drop caches
*/
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 7b846a836d02..01a5ee070a7c 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -29,8 +29,6 @@
#include <stdint.h>
#include <stdio.h>
-enum pipe;
-
const char *igt_debugfs_mount(void);
int igt_debugfs_dir(int device);
@@ -50,85 +48,6 @@ bool igt_debugfs_search(int fd, const char *filename, const char *substring);
#define igt_debugfs_read(fd, filename, buf) \
__igt_debugfs_read(fd, (filename), (buf), sizeof(buf))
-/*
- * Pipe CRC
- */
-
-/**
- * igt_pipe_crc_t:
- *
- * Pipe CRC support structure. Needs to be allocated and set up with
- * igt_pipe_crc_new() for a specific pipe and pipe CRC source value.
- */
-typedef struct _igt_pipe_crc igt_pipe_crc_t;
-
-#define DRM_MAX_CRC_NR 10
-/**
- * igt_crc_t:
- * @frame: frame number of the capture CRC
- * @n_words: internal field, don't access
- * @crc: internal field, don't access
- *
- * Pipe CRC value. All other members than @frame are private and should not be
- * inspected by testcases.
- */
-typedef struct {
- uint32_t frame;
- bool has_valid_frame;
- int n_words;
- uint32_t crc[DRM_MAX_CRC_NR];
-} igt_crc_t;
-
-/**
- * intel_pipe_crc_source:
- * @INTEL_PIPE_CRC_SOURCE_NONE: No source
- * @INTEL_PIPE_CRC_SOURCE_PLANE1: Plane 1
- * @INTEL_PIPE_CRC_SOURCE_PLANE2: Plane 2
- * @INTEL_PIPE_CRC_SOURCE_PF: Panel Filter
- * @INTEL_PIPE_CRC_SOURCE_PIPE: Pipe
- * @INTEL_PIPE_CRC_SOURCE_TV: TV
- * @INTEL_PIPE_CRC_SOURCE_DP_B: DisplayPort B
- * @INTEL_PIPE_CRC_SOURCE_DP_C: DisplayPort C
- * @INTEL_PIPE_CRC_SOURCE_DP_D: DisplayPort D
- * @INTEL_PIPE_CRC_SOURCE_AUTO: Automatic source selection
- * @INTEL_PIPE_CRC_SOURCE_MAX: Number of available sources
- *
- * Enumeration of all supported pipe CRC sources. Not all platforms and all
- * outputs support all of them. Generic tests should just use
- * INTEL_PIPE_CRC_SOURCE_AUTO. It should always map to an end-of-pipe CRC
- * suitable for checking planes, cursor, color correction and any other
- * output-agnostic features.
- */
-enum intel_pipe_crc_source {
- INTEL_PIPE_CRC_SOURCE_NONE,
- INTEL_PIPE_CRC_SOURCE_PLANE1,
- INTEL_PIPE_CRC_SOURCE_PLANE2,
- INTEL_PIPE_CRC_SOURCE_PF,
- INTEL_PIPE_CRC_SOURCE_PIPE,
- INTEL_PIPE_CRC_SOURCE_TV,
- INTEL_PIPE_CRC_SOURCE_DP_B,
- INTEL_PIPE_CRC_SOURCE_DP_C,
- INTEL_PIPE_CRC_SOURCE_DP_D,
- INTEL_PIPE_CRC_SOURCE_AUTO,
- INTEL_PIPE_CRC_SOURCE_MAX,
-};
-
-void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b);
-char *igt_crc_to_string(igt_crc_t *crc);
-
-void igt_require_pipe_crc(int fd);
-igt_pipe_crc_t *
-igt_pipe_crc_new(int fd, enum pipe pipe, enum intel_pipe_crc_source source);
-igt_pipe_crc_t *
-igt_pipe_crc_new_nonblock(int fd, enum pipe pipe, enum intel_pipe_crc_source source);
-void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
-void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
-void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
-__attribute__((warn_unused_result))
-int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
- igt_crc_t **out_crcs);
-void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
-
void igt_hpd_storm_set_threshold(int fd, unsigned int threshold);
void igt_hpd_storm_reset(int fd);
bool igt_hpd_storm_detected(int fd);
diff --git a/tests/chamelium.c b/tests/chamelium.c
index dc6311614abb..ec7821d54dfd 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -26,6 +26,7 @@
#include "config.h"
#include "igt.h"
+#include "igt_crc.h"
#include <fcntl.h>
#include <string.h>
diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
index 80592259266a..4185d4cfcf51 100644
--- a/tests/kms_atomic_transition.c
+++ b/tests/kms_atomic_transition.c
@@ -22,6 +22,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include "drmtest.h"
#include "sw_sync.h"
#include <errno.h>
diff --git a/tests/kms_ccs.c b/tests/kms_ccs.c
index d8291520c8a6..bfde3fdf19e3 100644
--- a/tests/kms_ccs.c
+++ b/tests/kms_ccs.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
IGT_TEST_DESCRIPTION("Test render compression (RC), in which the main surface "
"is complemented by a color control surface (CCS) that "
diff --git a/tests/kms_chv_cursor_fail.c b/tests/kms_chv_cursor_fail.c
index 3e74df1142da..b02958bd314f 100644
--- a/tests/kms_chv_cursor_fail.c
+++ b/tests/kms_chv_cursor_fail.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index e12e163449f8..3bcabcaca0bf 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <math.h>
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 206f85268c9e..d572e758082f 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
index 8a8c71b5a9fb..ea89d0ac0458 100644
--- a/tests/kms_cursor_legacy.c
+++ b/tests/kms_cursor_legacy.c
@@ -27,6 +27,7 @@
#include <sys/poll.h>
#include "igt.h"
+#include "igt_crc.h"
#include "igt_rand.h"
#include "igt_stats.h"
diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index c57d3a35b6b8..8cb4e147fa5e 100644
--- a/tests/kms_draw_crc.c
+++ b/tests/kms_draw_crc.c
@@ -25,6 +25,7 @@
/* This program tests whether the igt_draw library actually works. */
#include "igt.h"
+#include "igt_crc.h"
#define MAX_CONNECTORS 32
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 7964e052862b..10656b891466 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index 5aae29a8d56b..8e1a2fb476c9 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -25,6 +25,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 7cea4de42c5e..36119a3286fa 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -25,6 +25,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include "igt_sysfs.h"
#include <sys/types.h>
#include <sys/stat.h>
diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
index e5f089f6b78f..79efa792c58f 100644
--- a/tests/kms_mmap_write_crc.c
+++ b/tests/kms_mmap_write_crc.c
@@ -31,6 +31,7 @@
#include <string.h>
#include "drmtest.h"
+#include "igt_crc.h"
#include "igt_debugfs.h"
#include "igt_kms.h"
#include "intel_chipset.h"
diff --git a/tests/kms_mmio_vs_cs_flip.c b/tests/kms_mmio_vs_cs_flip.c
index fa947d9cd7f4..ee1d202ad7db 100644
--- a/tests/kms_mmio_vs_cs_flip.c
+++ b/tests/kms_mmio_vs_cs_flip.c
@@ -22,6 +22,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tests/kms_pipe_color.c b/tests/kms_pipe_color.c
index fd58ac8118c2..fa0758c124a2 100644
--- a/tests/kms_pipe_color.c
+++ b/tests/kms_pipe_color.c
@@ -28,6 +28,7 @@
#include "drm.h"
#include "drmtest.h"
#include "igt.h"
+#include "igt_crc.h"
IGT_TEST_DESCRIPTION("Test Color Features at Pipe level");
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index f49b434857e6..fd1c3ba1423c 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include "igt_sysfs.h"
#include <errno.h>
#include <stdbool.h>
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index e1bd467bb97d..0afc4d5bdbea 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -25,6 +25,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 6f1596015d76..031e1d1680b3 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include "drmtest.h"
#include <errno.h>
#include <stdbool.h>
diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index 93dce6b02340..6f7a85b6c4ae 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include "drmtest.h"
#include <errno.h>
#include <stdbool.h>
diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
index 1457894aaa10..d554f27fc1ef 100644
--- a/tests/kms_plane_scaling.c
+++ b/tests/kms_plane_scaling.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <math.h>
diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
index ee895db63fe2..f1e6f023b273 100644
--- a/tests/kms_pwrite_crc.c
+++ b/tests/kms_pwrite_crc.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 8306cf26a8b5..0029428697be 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -23,6 +23,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <math.h>
#define MAX_FENCES 32
diff --git a/tests/kms_universal_plane.c b/tests/kms_universal_plane.c
index 5459a9a3742b..dfa47914a319 100644
--- a/tests/kms_universal_plane.c
+++ b/tests/kms_universal_plane.c
@@ -22,6 +22,7 @@
*/
#include "igt.h"
+#include "igt_crc.h"
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
diff --git a/tools/intel_display_crc.c b/tools/intel_display_crc.c
index d1b28ea77371..104f432f3fd8 100644
--- a/tools/intel_display_crc.c
+++ b/tools/intel_display_crc.c
@@ -30,6 +30,7 @@
#include <unistd.h>
#include "igt_core.h"
+#include "igt_crc.h"
#include "igt_debugfs.h"
#include "igt_kms.h"
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
` (5 preceding siblings ...)
2017-04-25 16:45 ` [PATCH i-g-t 6/7] igt: lib/igt_crc: Split out CRC functionality Brian Starkey
@ 2017-04-25 16:45 ` Brian Starkey
2017-04-25 16:58 ` Ville Syrjälä
6 siblings, 1 reply; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 16:45 UTC (permalink / raw)
To: intel-gfx; +Cc: liviu.dudau
If "video=" is specified on the kernel command-line, use it to override
the default mode in kmstest_get_connector_default_mode.
If a mode override was provided on the command-line, it was probably for
good reason so we should honor it.
Signed-off-by: Brian Starkey <brian.starkey@arm.com>
---
lib/igt_kms.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 135 insertions(+)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 474aa005b9fa..97f80a46354d 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
igt_assert(ret != -1);
}
+/*
+ * Extract xres, yres, refresh and interlaced from a string of the form:
+ * <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
+ * xres and yres must be specified, refresh is optional and will be set to
+ * -1 if not present. interlaced defaults to false.
+ */
+static int parse_cmdline_mode(char *video_str, unsigned int *xres,
+ unsigned int *yres,
+ int *refresh, bool *interlaced)
+{
+ int match, len = strlen(video_str);
+ char *token = strtok(video_str, "@");
+
+ if (!token)
+ return -1;
+
+ *interlaced = false;
+ *refresh = -1;
+
+ match = sscanf(token, "%dx%d", xres, yres);
+ if (match != 2)
+ return -1;
+
+ if (strlen(token) < len - 1) {
+ token += strlen(token) + 1;
+
+ match = sscanf(token, "%d", refresh);
+ if (match != 1 || (*refresh < 0))
+ return -1;
+
+ if (strchr(token, 'i'))
+ *interlaced = true;
+ }
+
+ return 0;
+}
+
+static const drmModeModeInfo *
+connector_match_cmdline_mode(const drmModeConnector *connector,
+ unsigned int xres, unsigned int yres, int refresh,
+ bool interlaced)
+{
+ const drmModeModeInfo *mode = NULL;
+ int i;
+
+ for (i = 0; i < connector->count_modes; i++) {
+ mode = &connector->modes[i];
+ if (mode->hdisplay == xres &&
+ mode->vdisplay == yres &&
+ (refresh < 0 || refresh == mode->vrefresh) &&
+ interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
+ return mode;
+ }
+
+ return NULL;
+}
+
+static const drmModeModeInfo *
+kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
+{
+ char c, *str = NULL, *cursor, *conn_name = NULL;
+ const drmModeModeInfo *mode = NULL;
+ unsigned int size = 0;
+ FILE *fp = NULL;
+
+ fp = fopen("/proc/cmdline", "r");
+ if (!fp)
+ return NULL;
+
+ /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
+ while (fread(&c, 1, 1, fp))
+ size++;
+ rewind(fp);
+
+ str = calloc(1, size + 1);
+ if (!str)
+ goto done;
+
+ if (fread(str, 1, size, fp) != size)
+ goto done;
+
+ if (!asprintf(&conn_name, "%s-%d:",
+ kmstest_connector_type_str(connector->connector_type),
+ connector->connector_type_id))
+ goto done;
+
+ cursor = str;
+ while ((cursor = strstr(cursor, "video="))) {
+ unsigned int xres, yres;
+ bool interlaced;
+ int refresh;
+
+ cursor += strlen("video=");
+ cursor = strtok(cursor, " \n");
+ if (!cursor)
+ break;
+
+ /* Strip the name if it matches ours */
+ if (!strncmp(cursor, conn_name, strlen(conn_name)))
+ cursor += strlen(conn_name);
+
+ /*
+ * Consider this "video=" specification only if it has no
+ * name. If the name matched, we would have already stripped it
+ * above
+ */
+ if (!strstr(cursor, ":") &&
+ !parse_cmdline_mode(cursor, &xres, &yres, &refresh, &interlaced)) {
+ mode = connector_match_cmdline_mode(connector, xres,
+ yres, refresh,
+ interlaced);
+ if (mode)
+ break;
+ }
+
+ cursor += strlen(cursor) + 1;
+ }
+
+done:
+ free(conn_name);
+ free(str);
+ fclose(fp);
+ return mode;
+}
+
/**
* kmstest_get_connector_default_mode:
* @drm_fd: DRM fd
@@ -773,6 +898,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
* @mode: libdrm mode
*
* Retrieves the default mode for @connector and stores it in @mode.
+ * If video= is specified (optionally for this specific connector) on the
+ * kernel command line, then it is used as the default.
*
* Returns: true on success, false on failure
*/
@@ -780,6 +907,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
drmModeModeInfo *mode)
{
int i;
+ const drmModeModeInfo *cmdline_mode;
if (!connector->count_modes) {
igt_warn("no modes for connector %d\n",
@@ -787,6 +915,13 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
return false;
}
+ cmdline_mode = kmstest_get_cmdline_mode(drm_fd, connector);
+ if (cmdline_mode) {
+ *mode = *cmdline_mode;
+ igt_debug("Using cmdline mode\n");
+ return true;
+ }
+
for (i = 0; i < connector->count_modes; i++) {
if (i == 0 ||
connector->modes[i].type & DRM_MODE_TYPE_PREFERRED) {
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified
2017-04-25 16:45 ` [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified Brian Starkey
@ 2017-04-25 16:58 ` Ville Syrjälä
2017-04-25 17:06 ` Brian Starkey
0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2017-04-25 16:58 UTC (permalink / raw)
To: Brian Starkey; +Cc: intel-gfx, liviu.dudau
On Tue, Apr 25, 2017 at 05:45:13PM +0100, Brian Starkey wrote:
> If "video=" is specified on the kernel command-line, use it to override
> the default mode in kmstest_get_connector_default_mode.
>
> If a mode override was provided on the command-line, it was probably for
> good reason so we should honor it.
Isn't the kernel marking the cmdline mode as preferred already? And if
not, maybe it should.
>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
> lib/igt_kms.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 135 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 474aa005b9fa..97f80a46354d 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> igt_assert(ret != -1);
> }
>
> +/*
> + * Extract xres, yres, refresh and interlaced from a string of the form:
> + * <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
> + * xres and yres must be specified, refresh is optional and will be set to
> + * -1 if not present. interlaced defaults to false.
> + */
> +static int parse_cmdline_mode(char *video_str, unsigned int *xres,
> + unsigned int *yres,
> + int *refresh, bool *interlaced)
> +{
> + int match, len = strlen(video_str);
> + char *token = strtok(video_str, "@");
> +
> + if (!token)
> + return -1;
> +
> + *interlaced = false;
> + *refresh = -1;
> +
> + match = sscanf(token, "%dx%d", xres, yres);
> + if (match != 2)
> + return -1;
> +
> + if (strlen(token) < len - 1) {
> + token += strlen(token) + 1;
> +
> + match = sscanf(token, "%d", refresh);
> + if (match != 1 || (*refresh < 0))
> + return -1;
> +
> + if (strchr(token, 'i'))
> + *interlaced = true;
> + }
> +
> + return 0;
> +}
> +
> +static const drmModeModeInfo *
> +connector_match_cmdline_mode(const drmModeConnector *connector,
> + unsigned int xres, unsigned int yres, int refresh,
> + bool interlaced)
> +{
> + const drmModeModeInfo *mode = NULL;
> + int i;
> +
> + for (i = 0; i < connector->count_modes; i++) {
> + mode = &connector->modes[i];
> + if (mode->hdisplay == xres &&
> + mode->vdisplay == yres &&
> + (refresh < 0 || refresh == mode->vrefresh) &&
> + interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
> + return mode;
> + }
> +
> + return NULL;
> +}
> +
> +static const drmModeModeInfo *
> +kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
> +{
> + char c, *str = NULL, *cursor, *conn_name = NULL;
> + const drmModeModeInfo *mode = NULL;
> + unsigned int size = 0;
> + FILE *fp = NULL;
> +
> + fp = fopen("/proc/cmdline", "r");
> + if (!fp)
> + return NULL;
> +
> + /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
> + while (fread(&c, 1, 1, fp))
> + size++;
> + rewind(fp);
> +
> + str = calloc(1, size + 1);
> + if (!str)
> + goto done;
> +
> + if (fread(str, 1, size, fp) != size)
> + goto done;
> +
> + if (!asprintf(&conn_name, "%s-%d:",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id))
> + goto done;
> +
> + cursor = str;
> + while ((cursor = strstr(cursor, "video="))) {
> + unsigned int xres, yres;
> + bool interlaced;
> + int refresh;
> +
> + cursor += strlen("video=");
> + cursor = strtok(cursor, " \n");
> + if (!cursor)
> + break;
> +
> + /* Strip the name if it matches ours */
> + if (!strncmp(cursor, conn_name, strlen(conn_name)))
> + cursor += strlen(conn_name);
> +
> + /*
> + * Consider this "video=" specification only if it has no
> + * name. If the name matched, we would have already stripped it
> + * above
> + */
> + if (!strstr(cursor, ":") &&
> + !parse_cmdline_mode(cursor, &xres, &yres, &refresh, &interlaced)) {
> + mode = connector_match_cmdline_mode(connector, xres,
> + yres, refresh,
> + interlaced);
> + if (mode)
> + break;
> + }
> +
> + cursor += strlen(cursor) + 1;
> + }
> +
> +done:
> + free(conn_name);
> + free(str);
> + fclose(fp);
> + return mode;
> +}
> +
> /**
> * kmstest_get_connector_default_mode:
> * @drm_fd: DRM fd
> @@ -773,6 +898,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> * @mode: libdrm mode
> *
> * Retrieves the default mode for @connector and stores it in @mode.
> + * If video= is specified (optionally for this specific connector) on the
> + * kernel command line, then it is used as the default.
> *
> * Returns: true on success, false on failure
> */
> @@ -780,6 +907,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
> drmModeModeInfo *mode)
> {
> int i;
> + const drmModeModeInfo *cmdline_mode;
>
> if (!connector->count_modes) {
> igt_warn("no modes for connector %d\n",
> @@ -787,6 +915,13 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
> return false;
> }
>
> + cmdline_mode = kmstest_get_cmdline_mode(drm_fd, connector);
> + if (cmdline_mode) {
> + *mode = *cmdline_mode;
> + igt_debug("Using cmdline mode\n");
> + return true;
> + }
> +
> for (i = 0; i < connector->count_modes; i++) {
> if (i == 0 ||
> connector->modes[i].type & DRM_MODE_TYPE_PREFERRED) {
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified
2017-04-25 16:58 ` Ville Syrjälä
@ 2017-04-25 17:06 ` Brian Starkey
0 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 17:06 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, liviu.dudau
On Tue, Apr 25, 2017 at 07:58:18PM +0300, Ville Syrjälä wrote:
>On Tue, Apr 25, 2017 at 05:45:13PM +0100, Brian Starkey wrote:
>> If "video=" is specified on the kernel command-line, use it to override
>> the default mode in kmstest_get_connector_default_mode.
>>
>> If a mode override was provided on the command-line, it was probably for
>> good reason so we should honor it.
>
>Isn't the kernel marking the cmdline mode as preferred already? And if
>not, maybe it should.
>
It doesn't as far as I've seen. It uses the cmdline mode for setting
up the fbdev emulation but I don't think it marks the mode list (or we
aren't calling the helper that does it).
I'll have a look at what it would take to do that.
-Brian
>>
>> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>> ---
>> lib/igt_kms.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 135 insertions(+)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 474aa005b9fa..97f80a46354d 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -766,6 +766,131 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
>> igt_assert(ret != -1);
>> }
>>
>> +/*
>> + * Extract xres, yres, refresh and interlaced from a string of the form:
>> + * <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m][eDd]
>> + * xres and yres must be specified, refresh is optional and will be set to
>> + * -1 if not present. interlaced defaults to false.
>> + */
>> +static int parse_cmdline_mode(char *video_str, unsigned int *xres,
>> + unsigned int *yres,
>> + int *refresh, bool *interlaced)
>> +{
>> + int match, len = strlen(video_str);
>> + char *token = strtok(video_str, "@");
>> +
>> + if (!token)
>> + return -1;
>> +
>> + *interlaced = false;
>> + *refresh = -1;
>> +
>> + match = sscanf(token, "%dx%d", xres, yres);
>> + if (match != 2)
>> + return -1;
>> +
>> + if (strlen(token) < len - 1) {
>> + token += strlen(token) + 1;
>> +
>> + match = sscanf(token, "%d", refresh);
>> + if (match != 1 || (*refresh < 0))
>> + return -1;
>> +
>> + if (strchr(token, 'i'))
>> + *interlaced = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const drmModeModeInfo *
>> +connector_match_cmdline_mode(const drmModeConnector *connector,
>> + unsigned int xres, unsigned int yres, int refresh,
>> + bool interlaced)
>> +{
>> + const drmModeModeInfo *mode = NULL;
>> + int i;
>> +
>> + for (i = 0; i < connector->count_modes; i++) {
>> + mode = &connector->modes[i];
>> + if (mode->hdisplay == xres &&
>> + mode->vdisplay == yres &&
>> + (refresh < 0 || refresh == mode->vrefresh) &&
>> + interlaced == !!(mode->flags & DRM_MODE_FLAG_INTERLACE))
>> + return mode;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static const drmModeModeInfo *
>> +kmstest_get_cmdline_mode(int drm_fd, drmModeConnector *connector)
>> +{
>> + char c, *str = NULL, *cursor, *conn_name = NULL;
>> + const drmModeModeInfo *mode = NULL;
>> + unsigned int size = 0;
>> + FILE *fp = NULL;
>> +
>> + fp = fopen("/proc/cmdline", "r");
>> + if (!fp)
>> + return NULL;
>> +
>> + /* lseek/fseek+ftell/stat don't work on /proc/cmdline */
>> + while (fread(&c, 1, 1, fp))
>> + size++;
>> + rewind(fp);
>> +
>> + str = calloc(1, size + 1);
>> + if (!str)
>> + goto done;
>> +
>> + if (fread(str, 1, size, fp) != size)
>> + goto done;
>> +
>> + if (!asprintf(&conn_name, "%s-%d:",
>> + kmstest_connector_type_str(connector->connector_type),
>> + connector->connector_type_id))
>> + goto done;
>> +
>> + cursor = str;
>> + while ((cursor = strstr(cursor, "video="))) {
>> + unsigned int xres, yres;
>> + bool interlaced;
>> + int refresh;
>> +
>> + cursor += strlen("video=");
>> + cursor = strtok(cursor, " \n");
>> + if (!cursor)
>> + break;
>> +
>> + /* Strip the name if it matches ours */
>> + if (!strncmp(cursor, conn_name, strlen(conn_name)))
>> + cursor += strlen(conn_name);
>> +
>> + /*
>> + * Consider this "video=" specification only if it has no
>> + * name. If the name matched, we would have already stripped it
>> + * above
>> + */
>> + if (!strstr(cursor, ":") &&
>> + !parse_cmdline_mode(cursor, &xres, &yres, &refresh, &interlaced)) {
>> + mode = connector_match_cmdline_mode(connector, xres,
>> + yres, refresh,
>> + interlaced);
>> + if (mode)
>> + break;
>> + }
>> +
>> + cursor += strlen(cursor) + 1;
>> + }
>> +
>> +done:
>> + free(conn_name);
>> + free(str);
>> + fclose(fp);
>> + return mode;
>> +}
>> +
>> /**
>> * kmstest_get_connector_default_mode:
>> * @drm_fd: DRM fd
>> @@ -773,6 +898,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
>> * @mode: libdrm mode
>> *
>> * Retrieves the default mode for @connector and stores it in @mode.
>> + * If video= is specified (optionally for this specific connector) on the
>> + * kernel command line, then it is used as the default.
>> *
>> * Returns: true on success, false on failure
>> */
>> @@ -780,6 +907,7 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>> drmModeModeInfo *mode)
>> {
>> int i;
>> + const drmModeModeInfo *cmdline_mode;
>>
>> if (!connector->count_modes) {
>> igt_warn("no modes for connector %d\n",
>> @@ -787,6 +915,13 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>> return false;
>> }
>>
>> + cmdline_mode = kmstest_get_cmdline_mode(drm_fd, connector);
>> + if (cmdline_mode) {
>> + *mode = *cmdline_mode;
>> + igt_debug("Using cmdline mode\n");
>> + return true;
>> + }
>> +
>> for (i = 0; i < connector->count_modes; i++) {
>> if (i == 0 ||
>> connector->modes[i].type & DRM_MODE_TYPE_PREFERRED) {
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak
2017-04-25 16:45 ` [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak Brian Starkey
@ 2017-04-25 17:43 ` Gabriel Krisman Bertazi
2017-04-25 19:29 ` Brian Starkey
0 siblings, 1 reply; 12+ messages in thread
From: Gabriel Krisman Bertazi @ 2017-04-25 17:43 UTC (permalink / raw)
To: Brian Starkey; +Cc: intel-gfx, liviu.dudau
Brian Starkey <brian.starkey@arm.com> writes:
> igt_get_all_cairo_formats() allocates the format list on the heap, but
> returns it in a const pointer. Change this so that callers can free the
> array without a warning, and free the array in all callers.
>
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> ---
> lib/igt_fb.c | 4 +++-
> lib/igt_fb.h | 2 +-
> tests/kms_atomic.c | 3 ++-
> tests/kms_render.c | 4 +++-
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index d2b7e9e36606..b958c970973b 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1277,8 +1277,10 @@ const char *igt_format_str(uint32_t drm_format)
> *
> * This functions returns an array of all the drm fourcc codes supported by
> * cairo and this library.
> + *
> + * The array should be freed by the caller.
> */
> -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count)
> +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count)
> {
> static uint32_t *drm_formats;
> static int n_formats;
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index 4a680cefb16d..e124910367a3 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -154,7 +154,7 @@ int igt_cairo_printf_line(cairo_t *cr, enum igt_text_align align,
> uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth);
> uint32_t igt_drm_format_to_bpp(uint32_t drm_format);
> const char *igt_format_str(uint32_t drm_format);
> -void igt_get_all_cairo_formats(const uint32_t **formats, int *format_count);
> +void igt_get_all_cairo_formats(uint32_t **formats, int *format_count);
>
> #endif /* __IGT_FB_H__ */
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> index 6375fede7179..9c03f6e21ebb 100644
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -807,7 +807,7 @@ static void atomic_state_free(struct kms_atomic_state *state)
> static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane)
> {
> drmModePlanePtr plane_kms;
> - const uint32_t *igt_formats;
> + uint32_t *igt_formats;
> uint32_t ret = 0;
> int num_igt_formats;
> int i;
> @@ -827,6 +827,7 @@ static uint32_t plane_get_igt_format(struct kms_atomic_plane_state *plane)
> }
> }
>
> + free(igt_formats);
> drmModeFreePlane(plane_kms);
> return ret;
> }
> diff --git a/tests/kms_render.c b/tests/kms_render.c
> index fd33dfb7cafe..bc2ffc750c67 100644
> --- a/tests/kms_render.c
> +++ b/tests/kms_render.c
> @@ -176,7 +176,7 @@ static void test_connector(const char *test_name,
> struct kmstest_connector_config *cconf,
> enum test_flags flags)
> {
> - const uint32_t *formats;
> + uint32_t *formats;
> int format_count;
> int i;
>
> @@ -193,6 +193,8 @@ static void test_connector(const char *test_name,
> cconf, &cconf->connector->modes[0],
> formats[i], flags);
> }
> +
> + free(formats);
Hi,
Assuming I'm not missing anything, I think if you free formats here, the
static variable in igt_get_all_cairo_formats() will point to garbage and
further calls to that function will return uninitialized memory.
--
Gabriel Krisman Bertazi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak
2017-04-25 17:43 ` Gabriel Krisman Bertazi
@ 2017-04-25 19:29 ` Brian Starkey
0 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2017-04-25 19:29 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: intel-gfx, liviu.dudau
On Tue, Apr 25, 2017 at 02:43:13PM -0300, Gabriel Krisman Bertazi wrote:
>Brian Starkey <brian.starkey@arm.com> writes:
>
>> igt_get_all_cairo_formats() allocates the format list on the heap, but
>> returns it in a const pointer. Change this so that callers can free the
>> array without a warning, and free the array in all callers.
>>
>> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
>> ---
>
>Hi,
>
>Assuming I'm not missing anything, I think if you free formats here, the
>static variable in igt_get_all_cairo_formats() will point to garbage and
>further calls to that function will return uninitialized memory.
>
Hi!
Yes, sorry you're quite right. I'd managed to completely miss the fact
the pointer is static.
Please ignore this patch then,
Thanks,
Brian
>
>
>--
>Gabriel Krisman Bertazi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-04-25 19:29 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-25 16:45 [PATCH i-g-t 0/7] Misc fixes and cleanup Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t RESEND 1/7] lib/igt_kms: Fix erroneous assert Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 2/7] lib/igt_kms: Fix override_mode handling Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 3/7] lib: Stop igt_get_all_cairo_formats memory leak Brian Starkey
2017-04-25 17:43 ` Gabriel Krisman Bertazi
2017-04-25 19:29 ` Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 4/7] lib/igt_debugfs: Remove igt_debugfs_t Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 5/7] lib/igt_debugfs: Only use valid values in igt_crc_to_str() Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 6/7] igt: lib/igt_crc: Split out CRC functionality Brian Starkey
2017-04-25 16:45 ` [PATCH i-g-t 7/7] lib/igt_kms: Use kernel command line mode if specified Brian Starkey
2017-04-25 16:58 ` Ville Syrjälä
2017-04-25 17:06 ` Brian Starkey
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.