* [PATCH igt 2/4] lib/igt_aux: add functions to manipulate i915.ko parameters
2015-06-04 14:31 [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Paulo Zanoni
@ 2015-06-04 14:31 ` Paulo Zanoni
2015-06-04 14:31 ` [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default Paulo Zanoni
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2015-06-04 14:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Some i915.ko features have very nice IGT tests, which are never
executed because the features are disabled by default. This leads to
unnoticed regressions both in the Kernel and in the IGT tests. We
have seen this multiple times, for example, on FBC and PSR.
We want to be able to run IGT and actually test these
disabled-by-default features in order to make sure we at least don't
break them even more. Sometimes they may be disabled for some specific
reason, and we don't want to increase the set of reasons without
noticing.
To help solving this problem, this commit adds some helper functions
that should make it easier to change certain i915.ko parameters and
then restore their original values at the end of the test. With this,
I'm hoping QA will be able to detect any regressions and automatically
bisect them - or, with PRTS, reject the patches before they are even
merged.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
lib/igt_aux.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
lib/igt_aux.h | 4 ++
2 files changed, 138 insertions(+)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index d5c70fa..5392e1a 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -735,3 +735,137 @@ void igt_unlock_mem(void)
free(locked_mem);
locked_mem = NULL;
}
+
+
+#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
+#define PARAM_NAME_MAX_SZ 32
+#define PARAM_VALUE_MAX_SZ 16
+#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
+
+struct module_param_data {
+ char name[PARAM_NAME_MAX_SZ];
+ char original_value[PARAM_VALUE_MAX_SZ];
+
+ struct module_param_data *next;
+};
+struct module_param_data *module_params = NULL;
+
+static void igt_module_param_exit_handler(int sig)
+{
+ const size_t dir_len = strlen(MODULE_PARAM_DIR);
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ struct module_param_data *data;
+ int fd;
+
+ /* We don't need to assert string sizes on this function since they were
+ * already checked before being stored on the lists. Besides,
+ * igt_assert() is not AS-Safe. */
+ strcpy(file_path, MODULE_PARAM_DIR);
+
+ for (data = module_params; data != NULL; data = data->next) {
+ strcpy(file_path + dir_len, data->name);
+
+ fd = open(file_path, O_RDWR);
+ if (fd >= 0) {
+ write(fd, data->original_value,
+ strlen(data->original_value));
+ close(fd);
+ }
+ }
+ /* free() is not AS-Safe, so we can't call it here. */
+}
+
+/**
+ * igt_save_module_param:
+ * @name: name of the i915.ko module parameter
+ * @file_path: full sysfs file path for the parameter
+ *
+ * Reads the current value of an i915.ko module parameter, saves it on an array,
+ * then installs an exit handler to restore it when the program exits.
+ *
+ * It is safe to call this function multiple times for the same parameter.
+ *
+ * Notice that this function is called by igt_set_module_param(), so that one -
+ * or one of its wrappers - is the only function the test programs need to call.
+ */
+static void igt_save_module_param(const char *name, const char *file_path)
+{
+ struct module_param_data *data;
+ size_t n;
+ int fd;
+
+ /* Check if this parameter is already saved. */
+ for (data = module_params; data != NULL; data = data->next)
+ if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
+ return;
+
+ if (!module_params)
+ igt_install_exit_handler(igt_module_param_exit_handler);
+
+ data = calloc(1, sizeof (*data));
+ igt_assert(data);
+
+ strncpy(data->name, name, PARAM_NAME_MAX_SZ);
+
+ fd = open(file_path, O_RDONLY);
+ igt_assert(fd >= 0);
+
+ n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
+ igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_assert(close(fd) == 0);
+
+ data->next = module_params;
+ module_params = data;
+}
+
+/**
+ * igt_set_module_param:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This function sets the desired value for the given i915.ko parameter. It also
+ * takes care of saving and restoring the values that were already set before
+ * the test was run by calling igt_save_module_param().
+ *
+ * Please consider using igt_set_module_param_int() for the integer and bool
+ * parameters.
+ */
+void igt_set_module_param(const char *name, const char *val)
+{
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ size_t len = strlen(val);
+ int fd;
+
+ igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
+ "Need to increase PARAM_NAME_MAX_SZ\n");
+ strcpy(file_path, MODULE_PARAM_DIR);
+ strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
+
+ igt_save_module_param(name, file_path);
+
+ fd = open(file_path, O_RDWR);
+ igt_assert(write(fd, val, len) == len);
+ igt_assert(close(fd) == 0);
+}
+
+/**
+ * igt_set_module_param_int:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This is a wrapper for igt_set_module_param() that takes an integer instead of
+ * a string. Please see igt_set_module_param().
+ */
+void igt_set_module_param_int(const char *name, int val)
+{
+ char str[PARAM_VALUE_MAX_SZ];
+ int n;
+
+ n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
+ igt_assert_f(n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_set_module_param(name, str);
+}
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index b2dc267..9ea50de 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -145,4 +145,8 @@ void igt_unlock_mem(void);
ret_; \
})
+
+void igt_set_module_param(const char *name, const char *val);
+void igt_set_module_param_int(const char *name, int val);
+
#endif /* IGT_AUX_H */
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
2015-06-04 14:31 [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Paulo Zanoni
2015-06-04 14:31 ` [PATCH igt 2/4] lib/igt_aux: add functions to manipulate i915.ko parameters Paulo Zanoni
@ 2015-06-04 14:31 ` Paulo Zanoni
2015-06-15 12:11 ` Daniel Vetter
2015-06-04 14:31 ` [PATCH igt 4/4] tests/kms_psr_sink_crc: test even if PSR " Paulo Zanoni
2015-06-15 12:11 ` [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Daniel Vetter
3 siblings, 1 reply; 9+ messages in thread
From: Paulo Zanoni @ 2015-06-04 14:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We may not be perfect, but if we don't even test, we will probably
only get worse over time.
The function called makes sure we restore whatever was the original
FBC parameter when we exit the test, so this should not affect the
other tests.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_fbc_crc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 37221ac..d964224 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -559,10 +559,10 @@ igt_main
igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
fclose(status);
buf[sizeof(buf) - 1] = '\0';
- igt_require_f(!strstr(buf, "unsupported on this chipset") &&
- !strstr(buf, "disabled per module param") &&
- !strstr(buf, "disabled per chip default"),
- "FBC not supported/enabled\n");
+ igt_require_f(!strstr(buf, "unsupported on this chipset"),
+ "FBC not supported\n");
+
+ igt_set_module_param_int("enable_fbc", 1);
data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
igt_assert(data.bufmgr);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
2015-06-04 14:31 ` [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default Paulo Zanoni
@ 2015-06-15 12:11 ` Daniel Vetter
2015-06-15 14:08 ` Paulo Zanoni
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2015-06-15 12:11 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> We may not be perfect, but if we don't even test, we will probably
> only get worse over time.
>
> The function called makes sure we restore whatever was the original
> FBC parameter when we exit the test, so this should not affect the
> other tests.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> tests/kms_fbc_crc.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 37221ac..d964224 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -559,10 +559,10 @@ igt_main
> igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
> fclose(status);
> buf[sizeof(buf) - 1] = '\0';
> - igt_require_f(!strstr(buf, "unsupported on this chipset") &&
> - !strstr(buf, "disabled per module param") &&
> - !strstr(buf, "disabled per chip default"),
> - "FBC not supported/enabled\n");
> + igt_require_f(!strstr(buf, "unsupported on this chipset"),
> + "FBC not supported\n");
> +
> + igt_set_module_param_int("enable_fbc", 1);
This is risky since on older chips fbc is known to pretty much insta-kill
the box. Imo we should have a gen6+ here at least (that's roughly the
point where the hw workarounds start to sound less scary).
Trying to enable this on older platforms just isn't worth it imo.
-Daniel
>
> data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> igt_assert(data.bufmgr);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
2015-06-15 12:11 ` Daniel Vetter
@ 2015-06-15 14:08 ` Paulo Zanoni
2015-06-15 15:07 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Paulo Zanoni @ 2015-06-15 14:08 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
2015-06-15 9:11 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
> On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We may not be perfect, but if we don't even test, we will probably
>> only get worse over time.
>>
>> The function called makes sure we restore whatever was the original
>> FBC parameter when we exit the test, so this should not affect the
>> other tests.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> tests/kms_fbc_crc.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
>> index 37221ac..d964224 100644
>> --- a/tests/kms_fbc_crc.c
>> +++ b/tests/kms_fbc_crc.c
>> @@ -559,10 +559,10 @@ igt_main
>> igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
>> fclose(status);
>> buf[sizeof(buf) - 1] = '\0';
>> - igt_require_f(!strstr(buf, "unsupported on this chipset") &&
>> - !strstr(buf, "disabled per module param") &&
>> - !strstr(buf, "disabled per chip default"),
>> - "FBC not supported/enabled\n");
>> + igt_require_f(!strstr(buf, "unsupported on this chipset"),
>> + "FBC not supported\n");
>> +
>> + igt_set_module_param_int("enable_fbc", 1);
>
> This is risky since on older chips fbc is known to pretty much insta-kill
> the box. Imo we should have a gen6+ here at least (that's roughly the
> point where the hw workarounds start to sound less scary).
Well, we can try and then keep an eye on bugzilla, watching for the
possible insta-kill bug reports, can't we? At least to me, this was
not known as instal-kill since it's not documented anywhere and we
don't have bug reports. Also, the move to the frontbuffer tracking
infrastructure could maybe have solved the insta-kill problem.
>
> Trying to enable this on older platforms just isn't worth it imo.
If we're not even going to try this, shouldn't we remove all the
Kernel code support for FBC on these platforms? This is something I
wanted to discuss at some point, but now you gave us a nice excuse :).
Maybe it's better to just not have the code at all vs have a code that
(maybe) instakills the machine and we're probably not going to fix
anytime soon. I don't know, just an idea.
Btw, those patches are already on IGT. Thomas seemed to be OK with
the ideas, and after one week on the mailing list I pushed them.
> -Daniel
>
>
>>
>> data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
>> igt_assert(data.bufmgr);
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default
2015-06-15 14:08 ` Paulo Zanoni
@ 2015-06-15 15:07 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-06-15 15:07 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Mon, Jun 15, 2015 at 11:08:27AM -0300, Paulo Zanoni wrote:
> 2015-06-15 9:11 GMT-03:00 Daniel Vetter <daniel@ffwll.ch>:
> > On Thu, Jun 04, 2015 at 11:31:05AM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> We may not be perfect, but if we don't even test, we will probably
> >> only get worse over time.
> >>
> >> The function called makes sure we restore whatever was the original
> >> FBC parameter when we exit the test, so this should not affect the
> >> other tests.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >> tests/kms_fbc_crc.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> >> index 37221ac..d964224 100644
> >> --- a/tests/kms_fbc_crc.c
> >> +++ b/tests/kms_fbc_crc.c
> >> @@ -559,10 +559,10 @@ igt_main
> >> igt_assert_lt(0, fread(buf, 1, sizeof(buf), status));
> >> fclose(status);
> >> buf[sizeof(buf) - 1] = '\0';
> >> - igt_require_f(!strstr(buf, "unsupported on this chipset") &&
> >> - !strstr(buf, "disabled per module param") &&
> >> - !strstr(buf, "disabled per chip default"),
> >> - "FBC not supported/enabled\n");
> >> + igt_require_f(!strstr(buf, "unsupported on this chipset"),
> >> + "FBC not supported\n");
> >> +
> >> + igt_set_module_param_int("enable_fbc", 1);
> >
> > This is risky since on older chips fbc is known to pretty much insta-kill
> > the box. Imo we should have a gen6+ here at least (that's roughly the
> > point where the hw workarounds start to sound less scary).
>
> Well, we can try and then keep an eye on bugzilla, watching for the
> possible insta-kill bug reports, can't we? At least to me, this was
> not known as instal-kill since it's not documented anywhere and we
> don't have bug reports. Also, the move to the frontbuffer tracking
> infrastructure could maybe have solved the insta-kill problem.
Afaik fbc kills the chip on gen2 reliable when enabling, and iirc the same
on gen3. Not sure about gen4, but on ilk the w/a list in bspec is scary
enough to make grown-ups scream. Art recommended to just not bother, since
some of the required w/a (which we don't implement) force the chip into an
ill-defined state to provoke some kind of soft-reset. It's really all
fairly horrible.
> > Trying to enable this on older platforms just isn't worth it imo.
>
> If we're not even going to try this, shouldn't we remove all the
> Kernel code support for FBC on these platforms? This is something I
> wanted to discuss at some point, but now you gave us a nice excuse :).
> Maybe it's better to just not have the code at all vs have a code that
> (maybe) instakills the machine and we're probably not going to fix
> anytime soon. I don't know, just an idea.
Yeah it might be time to nuke the old fbc code entirely.
> Btw, those patches are already on IGT. Thomas seemed to be OK with
> the ideas, and after one week on the mailing list I pushed them.
Yeah didn't notice that yet. I'll follow up with a quick patch just for
paranoia.
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH igt 4/4] tests/kms_psr_sink_crc: test even if PSR is disabled by default
2015-06-04 14:31 [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Paulo Zanoni
2015-06-04 14:31 ` [PATCH igt 2/4] lib/igt_aux: add functions to manipulate i915.ko parameters Paulo Zanoni
2015-06-04 14:31 ` [PATCH igt 3/4] tests/kms_fbc_crc: run even if FBC is disabled by default Paulo Zanoni
@ 2015-06-04 14:31 ` Paulo Zanoni
2015-06-04 16:01 ` Rodrigo Vivi
2015-06-15 12:11 ` [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Daniel Vetter
3 siblings, 1 reply; 9+ messages in thread
From: Paulo Zanoni @ 2015-06-04 14:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Use the igt_set_module_param_int() call to enable it, then restore the
previous value after we are done testing.
With this, we can change the psr_enabled() function to psr_possible():
the only requirement should be that we have a PSR capable sink. The
test should now be able to make "Source_OK" and "Enabled" become true
whenever it wants.
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_psr_sink_crc.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 099391b..1823a68 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -203,7 +203,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
gem_bo_busy(data->drm_fd, handle);
}
-static bool psr_enabled(data_t *data)
+static bool psr_possible(data_t *data)
{
FILE *file;
char buf[4096];
@@ -229,14 +229,6 @@ static bool psr_enabled(data_t *data)
igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
"Sink_Support: %s\n", buf);
- ret = fscanf(file, "Source_OK: %s\n", buf);
- igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
- "Source_OK: %s\n", buf);
-
- ret = fscanf(file, "Enabled: %s\n", buf);
- igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
- "Enabled: %s\n", buf);
-
fclose(file);
return true;
}
@@ -570,7 +562,9 @@ int main(int argc, char *argv[])
kmstest_set_vt_graphics_mode();
data.devid = intel_get_drm_devid(data.drm_fd);
- igt_skip_on(!psr_enabled(&data));
+ igt_set_module_param_int("enable_psr", 1);
+
+ igt_skip_on(!psr_possible(&data));
data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
igt_assert(data.bufmgr);
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH igt 4/4] tests/kms_psr_sink_crc: test even if PSR is disabled by default
2015-06-04 14:31 ` [PATCH igt 4/4] tests/kms_psr_sink_crc: test even if PSR " Paulo Zanoni
@ 2015-06-04 16:01 ` Rodrigo Vivi
0 siblings, 0 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2015-06-04 16:01 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Thu, Jun 4, 2015 at 7:31 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Use the igt_set_module_param_int() call to enable it, then restore the
> previous value after we are done testing.
>
> With this, we can change the psr_enabled() function to psr_possible():
> the only requirement should be that we have a PSR capable sink. The
> test should now be able to make "Source_OK" and "Enabled" become true
> whenever it wants.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> tests/kms_psr_sink_crc.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 099391b..1823a68 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -203,7 +203,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color)
> gem_bo_busy(data->drm_fd, handle);
> }
>
> -static bool psr_enabled(data_t *data)
> +static bool psr_possible(data_t *data)
> {
> FILE *file;
> char buf[4096];
> @@ -229,14 +229,6 @@ static bool psr_enabled(data_t *data)
> igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
> "Sink_Support: %s\n", buf);
>
> - ret = fscanf(file, "Source_OK: %s\n", buf);
> - igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
> - "Source_OK: %s\n", buf);
> -
> - ret = fscanf(file, "Enabled: %s\n", buf);
> - igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
> - "Enabled: %s\n", buf);
> -
> fclose(file);
> return true;
> }
> @@ -570,7 +562,9 @@ int main(int argc, char *argv[])
> kmstest_set_vt_graphics_mode();
> data.devid = intel_get_drm_devid(data.drm_fd);
>
> - igt_skip_on(!psr_enabled(&data));
> + igt_set_module_param_int("enable_psr", 1);
> +
> + igt_skip_on(!psr_possible(&data));
>
> data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> igt_assert(data.bufmgr);
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION
2015-06-04 14:31 [PATCH igt 1/4] tests/template: add IGT_TEST_DESCRIPTION Paulo Zanoni
` (2 preceding siblings ...)
2015-06-04 14:31 ` [PATCH igt 4/4] tests/kms_psr_sink_crc: test even if PSR " Paulo Zanoni
@ 2015-06-15 12:11 ` Daniel Vetter
3 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-06-15 12:11 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Thu, Jun 04, 2015 at 11:31:03AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> So people that write tests based on the template don't forget to use
> the macro.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
With the nit to not try to enable fbc on pre-gen6 address the series is
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> tests/template.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/template.c b/tests/template.c
> index 24fd850..4b5794b 100644
> --- a/tests/template.c
> +++ b/tests/template.c
> @@ -26,6 +26,8 @@
>
> #include "drmtest.h"
>
> +IGT_TEST_DESCRIPTION("Template test.");
> +
> /*
> * Note that test function (and code called by them) should generally not return
> * a variable indicating success/failure. Instead use the igt_require/igt_assert
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread