Re: [Piglit] [PATCH v4] glsl-4.50: add a test for helper invocations
On Thu, Nov 12, 2015 at 2:55 PM, Glenn Kennard wrote: > Tested on r600g implementation > > Signed-off-by: Glenn Kennard > --- I used this to test the i965 implementation as well. Please push it. ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/6] shader_runner: Don't call glDeleteProgram on an ARB program.
On Tuesday, November 24, 2015 02:20:10 PM Matt Turner wrote:
> On Wed, Nov 11, 2015 at 4:53 PM, Kenneth Graunke
> wrote:
> > On Tuesday, November 10, 2015 10:46:19 PM Matt Turner wrote:
> >> I don't feel good about this check, but it is done elsewhere in the same
> >> file ("prog == 0").
> >> ---
> >> tests/shaders/shader_runner.c | 8 ++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> >> index 4597b46..bb17381 100644
> >> --- a/tests/shaders/shader_runner.c
> >> +++ b/tests/shaders/shader_runner.c
> >> @@ -3081,8 +3081,12 @@ piglit_display(void)
> >> if (piglit_automatic) {
> >> free_subroutine_uniforms();
> >> /* Free our resources, useful for valgrinding. */
> >> - glDeleteProgram(prog);
> >> - glUseProgram(0);
> >> + if (prog != 0) {
> >> + glDeleteProgram(prog);
> >> + glUseProgram(0);
> >> + } else {
> >> + glDeleteProgramsARB(1, &prog);
> >> + }
> >> }
> >>
> >> return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> >>
> >
> > Wow. Nasty.
> >
> > Both link_and_use_shaders() [GLSL] and compile_and_bind_program() [ARB]
> > set prog to a non-zero value. So at first glance this seems utterly bunk.
> > However, compile_and_bind_program() creates a local "GLuint prog" variable
> > that shadows the global one, so it never actually sets this. Heh.
> >
> > I was about to say this was correct.
> >
> > However, I don't see anything actually initializing the global prog
> > variable to 0...so won't this be using an uninitialized value? (As
> > would the existing code you patterned this after?)
>
> Sorry for the late reply -- I apparently didn't read your reply
> carefully enough initially.
>
> Globals in C are automatically initialized to 0. Given that, I think
> this is okay.
Oh, right. I think it's correct then. Still nasty, but functional.
Reviewed-by: Kenneth Graunke
signature.asc
Description: This is a digitally signed message part.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH V3] arb_separate_shader_objects: test mixed explicit and non-explicit locations
From: Timothy Arceri This tests a bug in Mesa where explicit locations are not taken into account when assigning varying locations which results in two inputs/outputs being given the same location. Test results: Nvidia GeForce 840M - NVIDIA 352.41: pass i965 - Mesa 11.1-dev: fail V3: - use helper function to format and link shaders - add array and arrays of arrays subtests - allow test to be run concurrently V2: use pick_a_glsl_version() helper Cc: Gregory Hainaut --- tests/all.py | 2 + .../arb_separate_shader_objects/CMakeLists.gl.txt | 1 + .../mixed_explicit_and_non_explicit_locations.c| 343 + 3 files changed, 346 insertions(+) create mode 100644 tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_explicit_locations.c diff --git a/tests/all.py b/tests/all.py index 07e3599..7689796 100644 --- a/tests/all.py +++ b/tests/all.py @@ -2161,6 +2161,8 @@ with profile.group_manager( 'Rendezvous by name') g(['arb_separate_shader_object-rendezvous_by_name_interpolation'], 'Rendezvous by name with multiple interpolation qualifier') +g(['arb_separate_shader_object-mixed_explicit_and_non_explicit_locations'], + 'Mixed explicit and non-explicit locations') g(['arb_separate_shader_object-rendezvous_by_location', '-fbo'], 'Rendezvous by location', run_concurrent=False) g(['arb_separate_shader_object-rendezvous_by_location-5-stages'], diff --git a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt index b4fce73..8e4011f 100644 --- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt +++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt @@ -15,6 +15,7 @@ piglit_add_executable (arb_separate_shader_object-compat-builtins compat-builtin piglit_add_executable (arb_separate_shader_object-explicit_locations_and_transform_feedback explicit_locations_and_transform_feedback.c) piglit_add_executable (arb_separate_shader_object-GetProgramPipelineiv GetProgramPipelineiv.c) piglit_add_executable (arb_separate_shader_object-IsProgramPipeline IsProgramPipeline.c) +piglit_add_executable (arb_separate_shader_object-mixed_explicit_and_non_explicit_locations mixed_explicit_and_non_explicit_locations.c sso-common.c) piglit_add_executable (arb_separate_shader_object-ProgramUniform-coverage ProgramUniform-coverage.c) piglit_add_executable (arb_separate_shader_object-rendezvous_by_location rendezvous_by_location.c sso-common.c) piglit_add_executable (arb_separate_shader_object-rendezvous_by_location-3-stages rendezvous_by_location-3-stages.c) diff --git a/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_explicit_locations.c b/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_explicit_locations.c new file mode 100644 index 000..3bca2bc --- /dev/null +++ b/tests/spec/arb_separate_shader_objects/mixed_explicit_and_non_explicit_locations.c @@ -0,0 +1,343 @@ +/* + * Copyright © 2015 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. + */ + +/** + * This tests a bug in Mesa where explicit locations are not taken into + * account when assigning varying locations which results in two + * inputs/outputs being given the same location. + */ +#include "piglit-util-gl.h" +#include "sso-common.h" + +PIGLIT_GL_TEST_CONFIG_BEGIN + + config.supports_gl_compat_version = 10; + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE; + +PIGLIT_GL_TEST_CONFIG_END + +static GLuint pipeline = 0; +static GLuint pipeline_arrays = 0; +static GLuint pipeline_arrays_of_arrays = 0; + +static const char *vs_code_template = + "#version %d\n" + "#extension GL_ARB_separate_shader_objects: require\n" + "#extension GL_ARB_explicit_attrib_location: require\n" + "\n" + "layout(location = 0)
Re: [Piglit] [PATCH v4 0/3] Fix glslparsertest python layer for gles 3.1+
On Fri, Nov 20, 2015 at 05:37:10PM -0800, baker.dyla...@gmail.com wrote: > From: Dylan Baker > > Changes from previous version: > - rebase on master > - fix bug in v3 > - small cleanups > > Dylan Baker (3): > glsl_parser_test.py: Fix which versions are sent to > glslparsertest_gles2 > framework/test/glsl_parser_test.py: Handle gl versions correctly > framework/test/glsl_parser_test.py: allow forcing the desktop version > > framework/test/glsl_parser_test.py| 79 ++-- > framework/tests/glsl_parser_test_tests.py | 152 > ++ > 2 files changed, 186 insertions(+), 45 deletions(-) > > -- > 2.6.2 > bump signature.asc Description: PGP signature ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] Add dmesg option for reboot policy
On Tue, Nov 24, 2015 at 02:10:34PM +, Emil Velikov wrote: > Hi Yan, > > The plan of having such a module is pretty sound. > > That said I think that the actual policy/implementation could use some tweaks. > > On 24 November 2015 at 12:14, wrote: > > From: Yann Argotti > > Date: Tue, 24 Nov 2015 12:16:34 +0100 > > > > This adds a policy which advises when user should reboot system to avoid > > noisy test results due to system becoming unstable, for instance, and > > therefore continues testing successfully. To do this, a new Dmesg class is > > proposed which is not filtering dmesg and monitors whether or not one of > > the following event occurs: > > - gpu reset failed (not just gpu reset happened, that happens > > way too often and many tests even provoke hangs intentionally) - gpu > > crash, > > - Oops: - BUG - lockdep splat that causes the locking validator to get > > disabled If one of these issues happen, piglit test execution is stopped > > -terminating test thread pool- and exit with code 3 to inform that reboot > > is > > advised. Then test execution resume, after rebooting system or not, is done > > like usually with command line parameter "resume". > > > Shouldn't one check for the above issues and trigger only when GPU > reset was not successful ? > Otherwise the idea of robustness, webgl and friends go down the drain. > > After all I'd imagine that the kernel devs want to know when GPU reset > does not work properly, albeit in a perfect world usespace should not > be able to lockup/crash the GPU. > > From a quick look at the patterns used, it seems that we'll trigger on > any BUG/Oops, regardless of the source - gpu, wifi, fs, etc driver. > This is bound to cause a lot of false positives, esp if the human > being running these tests does not check through the actual messages. Should the patterns be configured using piglit.conf or similar? I'm thinking about working on a laptop with two graphics cards, and having piglit stop because the other card spewed into dmesg. > These are topics for discussion, rather than a request for changing > the current patch. > > Cheers, > Emil > ___ > Piglit mailing list > Piglit@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/piglit signature.asc Description: PGP signature ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] Add dmesg option for reboot policy
On Tue, Nov 24, 2015 at 04:14:23AM -0800, yann.argo...@linux.intel.com wrote:
> From: Yann Argotti
> Date: Tue, 24 Nov 2015 12:16:34 +0100
>
> This adds a policy which advises when user should reboot system to avoid
> noisy test results due to system becoming unstable, for instance, and
> therefore continues testing successfully. To do this, a new Dmesg class is
> proposed which is not filtering dmesg and monitors whether or not one of
> the following event occurs:
> - gpu reset failed (not just gpu reset happened, that happens
> way too often and many tests even provoke hangs intentionally) - gpu crash,
> - Oops: - BUG - lockdep splat that causes the locking validator to get
> disabled If one of these issues happen, piglit test execution is stopped
> -terminating test thread pool- and exit with code 3 to inform that reboot is
> advised. Then test execution resume, after rebooting system or not, is done
> like usually with command line parameter "resume".
>
> To call it, use command line parameter: --dmesg monitored
> (this is original patch for review and therefore didn't apply any proposed
> change yet)
>
> Signed-off-by: Yann Argotti
I don't like the aproach of passing the returncode through every level
of piglit at all. I'd much rather use an exception for this, either by
extending PiglitFatalError or by adding a new exception, and then
catching it either in the exceptions.handler decorator, or if we have to
in run/resume.
I've got a couple more comments inline.
Also, please figure out where the line-wrapping is coming from and
remove it (if you're pasting this into a window in your email client you
will get line-wrap which breaks git am and git apply). Using git
send-email is really the best way to do this.
[snip]
> +
> +def reboot_is_needed(self):
> +""" Simply return the reboot_needed variable state """
> +return self._reboot_needed
This is what a property is for:
@property
def reboot_needed(self):
return self._reboot_needed
Then you access the value as an attribute rather than as a function.
If self.reboot_needed:
print('reboot now!')
> +
> +def update_dmesg(self):
> +""" Call dmesg using subprocess.check_output
> +
> +Get the contents of dmesg, then calculate new messages, finally set
> +self.dmesg equal to the just read contents of dmesg.
> +
> +"""
> +dmesg =
> subprocess.check_output(self.DMESG_COMMAND).strip().splitlines()
> +
> +# Find all new entries, do this by slicing the list of dmesg to only
> +# returns elements after the last element stored. If there are not
> +# matches a value error is raised, that means all of dmesg is new
> +l = 0
> +for index, item in enumerate(reversed(dmesg)):
> +if item == self._last_message:
> +l = len(dmesg) - index # don't include the matched element
> +break
> +self._new_messages = dmesg[l:]
> +
> +if (not self._first_dmesg_process):
> +if (any("[drm] Failed to reset chip" in msg for msg in
> self._new_messages) or
> +any("[drm] GPU crash" in msg for msg in self._new_messages)
> or
> +any("Oops:" in msg for msg in self._new_messages) or
> +any("BUG:" in msg for msg in self._new_messages) or
> +any("turning off the locking correctness validator" in msg
> for msg in self._new_messages)):
> + self._reboot_needed = True
> +else:
> + self._first_dmesg_process = False
> +
> +# Attempt to store the last element of dmesg, unless there was no
> dmesg
> +self._last_message = dmesg[-1] if dmesg else None
> +
> +def update_result(self, result):
> +""" Takes a TestResult object and updates it with dmesg statuses
> +
> +There is no filtering/change of status in dmesg and return the
> +original result passed in.
> +
> +Arguments:
> +result -- A TestResult instance
> +
> +"""
> +
> +# Get a new snapshot of dmesg
> +self.update_dmesg()
> +
> +# if update_dmesg() found new entries replace the results of the test
> +# and subtests
> +if self._new_messages:
> +
> +if self.regex:
> +for line in self._new_messages:
> +if self.regex.search(line):
> +break
> +else:
> +return result
> +
> +# Add the dmesg values to the result
> +result.dmesg = "\n".join(self._new_messages)
> +
> +return result
> +
There is a ton of duplication between LinuxDmesg and
LinuxMonitoredDmesg. I haven't looked too closely, but it seems like
LinuxMonitoredDmesg could inherit from LinuxDmesg?
[snip]
Dylan
signature.asc
Description: PGP signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://li
Re: [Piglit] [PATCH 2/6] shader_runner: Don't call glDeleteProgram on an ARB program.
On Wed, Nov 11, 2015 at 4:53 PM, Kenneth Graunke wrote:
> On Tuesday, November 10, 2015 10:46:19 PM Matt Turner wrote:
>> I don't feel good about this check, but it is done elsewhere in the same
>> file ("prog == 0").
>> ---
>> tests/shaders/shader_runner.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index 4597b46..bb17381 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -3081,8 +3081,12 @@ piglit_display(void)
>> if (piglit_automatic) {
>> free_subroutine_uniforms();
>> /* Free our resources, useful for valgrinding. */
>> - glDeleteProgram(prog);
>> - glUseProgram(0);
>> + if (prog != 0) {
>> + glDeleteProgram(prog);
>> + glUseProgram(0);
>> + } else {
>> + glDeleteProgramsARB(1, &prog);
>> + }
>> }
>>
>> return pass ? PIGLIT_PASS : PIGLIT_FAIL;
>>
>
> Wow. Nasty.
>
> Both link_and_use_shaders() [GLSL] and compile_and_bind_program() [ARB]
> set prog to a non-zero value. So at first glance this seems utterly bunk.
> However, compile_and_bind_program() creates a local "GLuint prog" variable
> that shadows the global one, so it never actually sets this. Heh.
>
> I was about to say this was correct.
>
> However, I don't see anything actually initializing the global prog
> variable to 0...so won't this be using an uninitialized value? (As
> would the existing code you patterned this after?)
Sorry for the late reply -- I apparently didn't read your reply
carefully enough initially.
Globals in C are automatically initialized to 0. Given that, I think
this is okay.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH][RFC] Add dmesg option for reboot policy
On Tue, Nov 24, 2015 at 5:33 AM, Martin Peres wrote: > On 24/11/15 07:09, Dylan Baker wrote: >> >> On Mon, Nov 23, 2015 at 08:54:46PM -0500, Ilia Mirkin wrote: >>> >>> On Mon, Nov 23, 2015 at 8:41 PM, Dylan Baker >>> wrote: On Mon, Nov 23, 2015 at 08:24:22PM -0500, Ilia Mirkin wrote: > > On Mon, Nov 23, 2015 at 8:07 PM, Dylan Baker > wrote: >> >> On Mon, Nov 23, 2015 at 02:48:35PM +0200, Martin Peres wrote: >>> >>> On 02/11/15 16:57, yann.argo...@linux.intel.com wrote: This adds a policy which advises when user should reboot system to avoid noisy test results due to system becoming unstable, for instance, and therefore continues testing successfully. To do this, a new Dmesg class is proposed which is not filtering dmesg and monitors whether or not one of the following event occurs: - gpu reset failed (not just gpu reset happened, that happens way too often and many tests even provoke hangs intentionally) - gpu crash, - Oops: - BUG - lockdep splat that causes the locking validator to get disabled If one of these issues happen, piglit test execution is stopped -terminating test thread pool- and exit with code 3 to inform that reboot is advised. Then test execution resume, after rebooting system or not, is done like usually with command line parameter "resume". To call it, use command line parameter: --dmesg monitored >>> >>> Hello Yann, >>> >>> The rationale behind this patch is very sound and we need something >>> like >>> this. Here are however a list of nitpicks: >>> >>> - Please send patches with git send-email, otherwise, it makes it >>> impossible for us to comment inline which is the usual process for >>> patch >>> review. Please re-send :) >>> >>> - varaiable -> variable; double space after "when a reboot may be >>> required" >>> >>> - I am not a big fan of changing the semantic of arguments that >>> have been >>> there forever. Can you think of a case where the user would not want >>> the >>> test to abort if we reach a state where we cannot trust the result? I >>> am >>> including Dylan on this. Also, if we are to keep these modes, can we >>> rename >>> the "simple" mode to "warning" and "monitored" to "abort"? This would >>> make >>> more sense and clearly state the goal of the modes. >> >> Ilia, Daniel, Thomas, Glenn, I know that y'all use the dmesg support. >> What do you think? > > Can you provide a summary of what this patch does? It was submitted as > an attachment, so I can't (easily) look at it... either way, as long > as running with --dmesg doesn't break, I probably don't care. I use > --dmesg to know which tests cause the GPU to complain, which I then, > in turn, use to pick which tests to debug further. (And since I > normally run with -1 anyways, it's ~free to add...) > >-ilia I'm going to interpret that as "I'd be annoyed if piglit just stopped when there was dmesg chatter"? >>> >>> That would definitely be a deal breaker. My wifi goes in and out and >>> complains loudly about it in dmesg :) > > > That should not happen since it will wait for specific events in dmesg for > this (gpu reset failed, gpu crash, oops. bug and lockdep splat). > What does your wifi driver produce? Nothing horrible... just errors about... stuff. And obviously nouveau prints errors too, many of which are expected due to tests doing funny things (like reading out-of-bounds things). > >>> >>>-ilia >> >> I think that answers that question Martin. >> >> I would lean towards adding a "--dmesg-abort" (or whatever name) flag to >> be passed in addition to --dmesg when the user wants to stop on dmesg >> errors. > > > I would personally say that we should not call it dmesg at all and just use > whatever information we have to detect when we need to abort. So, something > like --abort-on-critical-error would make more sense and would not be > misleading the users into thinking it is solely using dmesg as an input. That makes a lot more sense. And if it did that automatically, I'd be fine with that... as long as it had good "critical error" detection. I can't imagine what that would look like. So until it's 100%, it should stay off by default. -ilia ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] Add dmesg option for reboot policy
Hi Yan, The plan of having such a module is pretty sound. That said I think that the actual policy/implementation could use some tweaks. On 24 November 2015 at 12:14, wrote: > From: Yann Argotti > Date: Tue, 24 Nov 2015 12:16:34 +0100 > > This adds a policy which advises when user should reboot system to avoid > noisy test results due to system becoming unstable, for instance, and > therefore continues testing successfully. To do this, a new Dmesg class is > proposed which is not filtering dmesg and monitors whether or not one of > the following event occurs: > - gpu reset failed (not just gpu reset happened, that happens > way too often and many tests even provoke hangs intentionally) - gpu crash, > - Oops: - BUG - lockdep splat that causes the locking validator to get > disabled If one of these issues happen, piglit test execution is stopped > -terminating test thread pool- and exit with code 3 to inform that reboot is > advised. Then test execution resume, after rebooting system or not, is done > like usually with command line parameter "resume". > Shouldn't one check for the above issues and trigger only when GPU reset was not successful ? Otherwise the idea of robustness, webgl and friends go down the drain. After all I'd imagine that the kernel devs want to know when GPU reset does not work properly, albeit in a perfect world usespace should not be able to lockup/crash the GPU. From a quick look at the patterns used, it seems that we'll trigger on any BUG/Oops, regardless of the source - gpu, wifi, fs, etc driver. This is bound to cause a lot of false positives, esp if the human being running these tests does not check through the actual messages. These are topics for discussion, rather than a request for changing the current patch. Cheers, Emil ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH v3] sso: test program pipeline with atomic counters
Test fails on current Mesa and NVIDIA binary driver version 355.11 and
passes on AMD binary driver version 15.201.1151.
v2: - add error checks
- modify to render points
- make it possible to use regular program (makes test pass on Mesa)
v3: - bump up glsl version to 1.50 and redeclare gl_Position in vs
(makes the vs compile and test pass on AMD)
Signed-off-by: Tapani Pälli
---
tests/all.py | 2 +
.../arb_separate_shader_objects/CMakeLists.gl.txt | 1 +
.../arb_separate_shader_objects/atomic-counter.c | 161 +
3 files changed, 164 insertions(+)
create mode 100644 tests/spec/arb_separate_shader_objects/atomic-counter.c
diff --git a/tests/all.py b/tests/all.py
index 07e3599..5b22fe1 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -2173,6 +2173,8 @@ with profile.group_manager(
'400 combinations by name', run_concurrent=False)
g(['arb_separate_shader_object-active-sampler-conflict'],
'active sampler conflict')
+g(['arb_separate_shader_object-atomic-counter'],
+ 'atomic counter')
# Group ARB_sampler_objects
with profile.group_manager(
diff --git a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
index 1dc5f14..ed9be84 100644
--- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
+++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
@@ -22,3 +22,4 @@ piglit_add_executable
(arb_separate_shader_object-rendezvous_by_name rendezvous_
piglit_add_executable
(arb_separate_shader_object-rendezvous_by_name_interpolation
rendezvous_by_name_interpolation.c sso-common.c)
piglit_add_executable
(arb_separate_shader_object-UseProgramStages-non-separable
UseProgramStages-non-separable.c)
piglit_add_executable (arb_separate_shader_object-ValidateProgramPipeline
ValidateProgramPipeline.c)
+piglit_add_executable (arb_separate_shader_object-atomic-counter
atomic-counter.c)
diff --git a/tests/spec/arb_separate_shader_objects/atomic-counter.c
b/tests/spec/arb_separate_shader_objects/atomic-counter.c
new file mode 100644
index 000..3d8357f
--- /dev/null
+++ b/tests/spec/arb_separate_shader_objects/atomic-counter.c
@@ -0,0 +1,161 @@
+/*
+ * Copyright © 2015 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.
+ */
+
+/**
+ * \file atomic-counter.c
+ *
+ * Test incrementing atomic counter in a separable program.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_core_version = 31;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+/* Use version 150 so that we don't need to redeclare gl_Position. */
+const char *vs_source =
+ "#version 150\n"
+ "in vec4 vertex;\n"
+ "out gl_PerVertex { vec4 gl_Position; }; \n"
+ "void main() {\n"
+ " gl_Position = vertex;\n"
+ "}\n";
+
+const char *fs_source =
+ "#version 150\n"
+ "#extension GL_ARB_shader_atomic_counters : enable\n"
+ "layout(binding = 0, offset = 0) uniform atomic_uint counter;\n"
+ "void main() {\n"
+ " atomicCounterIncrement(counter);\n"
+ "}\n";
+
+#define AMOUNT 100
+
+enum piglit_result
+piglit_display(void)
+{
+ return PIGLIT_FAIL;
+}
+
+void
+piglit_init(int argc, char **argv)
+{
+ GLuint buffer, vao, vbo;
+#ifdef REGULAR_PROGRAM
+ GLuint prog;
+#else
+ GLuint vs, fs, pipe;
+#endif
+ uint32_t counter = 0;
+ uint32_t *data;
+ bool pass = true;
+ unsigned i;
+
+ piglit_require_gl_version(31);
+ piglit_require_extension("GL_ARB_shader_atomic_counters");
+ piglit_require_extension("GL_ARB_separate_shader_objects");
+
+ glViewport(0, 0, piglit_width, piglit_height);
+
+#ifdef REGULAR_PROGRAM
+ /* Just a regular program. */
+ prog = piglit_build_simple_program(vs_source, fs_source);
+ glUseProgram(prog
[Piglit] [PATCH] Add dmesg option for reboot policy
From: Yann Argotti
Date: Tue, 24 Nov 2015 12:16:34 +0100
This adds a policy which advises when user should reboot system to avoid
noisy test results due to system becoming unstable, for instance, and
therefore continues testing successfully. To do this, a new Dmesg class is
proposed which is not filtering dmesg and monitors whether or not one of
the following event occurs:
- gpu reset failed (not just gpu reset happened, that happens
way too often and many tests even provoke hangs intentionally) - gpu crash,
- Oops: - BUG - lockdep splat that causes the locking validator to get
disabled If one of these issues happen, piglit test execution is stopped
-terminating test thread pool- and exit with code 3 to inform that reboot is
advised. Then test execution resume, after rebooting system or not, is done
like usually with command line parameter "resume".
To call it, use command line parameter: --dmesg monitored
(this is original patch for review and therefore didn't apply any proposed
change yet)
Signed-off-by: Yann Argotti
---
framework/dmesg.py| 118
+-
framework/exceptions.py | 6 ++-
framework/profile.py | 35 --
framework/programs/run.py | 45 ++
4 files changed, 185 insertions(+), 19 deletions(-)
diff --git a/framework/dmesg.py b/framework/dmesg.py
index daeeeae..66c95f5 100644
--- a/framework/dmesg.py
+++ b/framework/dmesg.py
@@ -46,6 +46,7 @@ import gzip
__all__ = [
'BaseDmesg',
'LinuxDmesg',
+'LinuxMonitoredDmesg',
'DummyDmesg',
'get_dmesg',
]
@@ -212,6 +213,116 @@ class LinuxDmesg(BaseDmesg):
# Attempt to store the last element of dmesg, unless there was no
dmesg
self._last_message = dmesg[-1] if dmesg else None
+class LinuxMonitoredDmesg(BaseDmesg):
+""" Read and monitor dmesg on posix systems
+
+This reads the dmesg ring buffer, stores the contents of the buffer, and
+then compares it whenever called, returning the difference between the
+calls. It requires timestamps to be enabled.
+
+"""
+# We are considering all levels of dmesg to avoid missing
+# any important ones and with an inaccurate reporting level
+DMESG_COMMAND = ['dmesg']
+# With _first_dmesg_process, we are not checking dmesg at test execution
+# starting time: user must know current machine/GPU state before
+# starting piglit: we can resume test execution based on last stopping
+# if we consider that our system is still in good shape (ie not blocking
+# run forever)
+_first_dmesg_process = True
+_reboot_needed = False
+
+def __init__(self):
+""" Create a dmesg instance """
+# _last_message store the contents of the last entry in dmesg the
last
+# time it was read. Because dmesg is a ringbuffer old entries can
fall
+# off the end if it becomes too full. To track new entries search
for
+# this entry and take any entries that were added after it. This
works
+# on Linux because each entry is guaranteed unique by timestamps.
+self._last_message = None
+super(LinuxMonitoredDmesg, self).__init__()
+self._reboot_needed = False
+
+if not self._last_message:
+# We need to ensure that there is something in dmesg to check
+# timestamps.
+warnings.warn("Cannot check dmesg for timestamp support. If
you "
+ "do not have timestamps enabled in your kernel
you "
+ "get incomplete dmesg captures", RuntimeWarning)
+elif not re.match(r'\[\s*\d+\.\d+\]', self._last_message):
+# Do an initial check to ensure that dmesg has timestamps, we
need
+# timestamps to work correctly. A proper linux dmesg timestamp
+# looks like this: [0.0]
+raise DmesgError(
+"Your kernel does not seem to support timestamps, which "
+"are required by the --dmesg option")
+
+def reboot_is_needed(self):
+""" Simply return the reboot_needed variable state """
+return self._reboot_needed
+
+def update_dmesg(self):
+""" Call dmesg using subprocess.check_output
+
+Get the contents of dmesg, then calculate new messages, finally set
+self.dmesg equal to the just read contents of dmesg.
+
+"""
+dmesg =
subprocess.check_output(self.DMESG_COMMAND).strip().splitlines()
+
+# Find all new entries, do this by slicing the list of dmesg to only
+# returns elements after the last element stored. If there are not
+# matches a value error is raised, that means all of dmesg is new
+l = 0
+for index, item in enumerate(reversed(dmesg)):
+if item == self._last_message:
+l = len(dmesg) - index # don't include the matched element
+break
+self._new_messages = dmesg[l:]
+
+
Re: [Piglit] [PATCH][RFC] Add dmesg option for reboot policy
On 24/11/15 07:09, Dylan Baker wrote: On Mon, Nov 23, 2015 at 08:54:46PM -0500, Ilia Mirkin wrote: On Mon, Nov 23, 2015 at 8:41 PM, Dylan Baker wrote: On Mon, Nov 23, 2015 at 08:24:22PM -0500, Ilia Mirkin wrote: On Mon, Nov 23, 2015 at 8:07 PM, Dylan Baker wrote: On Mon, Nov 23, 2015 at 02:48:35PM +0200, Martin Peres wrote: On 02/11/15 16:57, yann.argo...@linux.intel.com wrote: This adds a policy which advises when user should reboot system to avoid noisy test results due to system becoming unstable, for instance, and therefore continues testing successfully. To do this, a new Dmesg class is proposed which is not filtering dmesg and monitors whether or not one of the following event occurs: - gpu reset failed (not just gpu reset happened, that happens way too often and many tests even provoke hangs intentionally) - gpu crash, - Oops: - BUG - lockdep splat that causes the locking validator to get disabled If one of these issues happen, piglit test execution is stopped -terminating test thread pool- and exit with code 3 to inform that reboot is advised. Then test execution resume, after rebooting system or not, is done like usually with command line parameter "resume". To call it, use command line parameter: --dmesg monitored Hello Yann, The rationale behind this patch is very sound and we need something like this. Here are however a list of nitpicks: - Please send patches with git send-email, otherwise, it makes it impossible for us to comment inline which is the usual process for patch review. Please re-send :) - varaiable -> variable; double space after "when a reboot may be required" - I am not a big fan of changing the semantic of arguments that have been there forever. Can you think of a case where the user would not want the test to abort if we reach a state where we cannot trust the result? I am including Dylan on this. Also, if we are to keep these modes, can we rename the "simple" mode to "warning" and "monitored" to "abort"? This would make more sense and clearly state the goal of the modes. Ilia, Daniel, Thomas, Glenn, I know that y'all use the dmesg support. What do you think? Can you provide a summary of what this patch does? It was submitted as an attachment, so I can't (easily) look at it... either way, as long as running with --dmesg doesn't break, I probably don't care. I use --dmesg to know which tests cause the GPU to complain, which I then, in turn, use to pick which tests to debug further. (And since I normally run with -1 anyways, it's ~free to add...) -ilia I'm going to interpret that as "I'd be annoyed if piglit just stopped when there was dmesg chatter"? That would definitely be a deal breaker. My wifi goes in and out and complains loudly about it in dmesg :) That should not happen since it will wait for specific events in dmesg for this (gpu reset failed, gpu crash, oops. bug and lockdep splat). What does your wifi driver produce? -ilia I think that answers that question Martin. I would lean towards adding a "--dmesg-abort" (or whatever name) flag to be passed in addition to --dmesg when the user wants to stop on dmesg errors. I would personally say that we should not call it dmesg at all and just use whatever information we have to detect when we need to abort. So, something like --abort-on-critical-error would make more sense and would not be misleading the users into thinking it is solely using dmesg as an input. However, I'd really like to see the patch come in with git-send-email so I can have a proper look at it. Agreed. Martin ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH v2] sso: test program pipeline with atomic counters
Test fails on current Mesa and NVIDIA binary driver version 355.11,
don't have currently AMD machine to test with.
v2: - add error checks
- modify to render points
- make it possible to use regular program (makes test pass on Mesa)
Signed-off-by: Tapani Pälli
---
tests/all.py | 2 +
.../arb_separate_shader_objects/CMakeLists.gl.txt | 1 +
.../arb_separate_shader_objects/atomic-counter.c | 157 +
3 files changed, 160 insertions(+)
create mode 100644 tests/spec/arb_separate_shader_objects/atomic-counter.c
diff --git a/tests/all.py b/tests/all.py
index e80f3af..19fec49 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -2172,6 +2172,8 @@ with profile.group_manager(
'400 combinations by name', run_concurrent=False)
g(['arb_separate_shader_object-active-sampler-conflict'],
'active sampler conflict')
+g(['arb_separate_shader_object-atomic-counter'],
+ 'atomic counter')
# Group ARB_sampler_objects
with profile.group_manager(
diff --git a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
index 1dc5f14..ed9be84 100644
--- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
+++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
@@ -22,3 +22,4 @@ piglit_add_executable
(arb_separate_shader_object-rendezvous_by_name rendezvous_
piglit_add_executable
(arb_separate_shader_object-rendezvous_by_name_interpolation
rendezvous_by_name_interpolation.c sso-common.c)
piglit_add_executable
(arb_separate_shader_object-UseProgramStages-non-separable
UseProgramStages-non-separable.c)
piglit_add_executable (arb_separate_shader_object-ValidateProgramPipeline
ValidateProgramPipeline.c)
+piglit_add_executable (arb_separate_shader_object-atomic-counter
atomic-counter.c)
diff --git a/tests/spec/arb_separate_shader_objects/atomic-counter.c
b/tests/spec/arb_separate_shader_objects/atomic-counter.c
new file mode 100644
index 000..0b4ad92
--- /dev/null
+++ b/tests/spec/arb_separate_shader_objects/atomic-counter.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright © 2015 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.
+ */
+
+/**
+ * \file atomic-counter.c
+ *
+ * Test incrementing atomic counter in a separable program.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+ config.supports_gl_core_version = 31;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+const char *vs_source =
+ "#version 140\n"
+ "in vec4 vertex;\n"
+ "void main() {\n"
+ " gl_Position = vertex;\n"
+ "}\n";
+
+const char *fs_source =
+ "#version 140\n"
+ "#extension GL_ARB_shader_atomic_counters : enable\n"
+ "layout(binding = 0, offset = 0) uniform atomic_uint counter;\n"
+ "void main() {\n"
+ " atomicCounterIncrement(counter);\n"
+ "}\n";
+
+#define AMOUNT 100
+
+enum piglit_result
+piglit_display(void)
+{
+ return PIGLIT_FAIL;
+}
+
+void
+piglit_init(int argc, char **argv)
+{
+ GLuint buffer, vao, vbo;
+#ifdef REGULAR_PROGRAM
+ GLuint prog;
+#else
+ GLuint vs, fs, pipe;
+#endif
+ uint32_t counter = 0;
+ uint32_t *data;
+ bool pass = true;
+ unsigned i;
+
+ piglit_require_gl_version(31);
+ piglit_require_extension("GL_ARB_shader_atomic_counters");
+ piglit_require_extension("GL_ARB_separate_shader_objects");
+
+#ifdef REGULAR_PROGRAM
+ /* Just a regular program. */
+ prog = piglit_build_simple_program(vs_source, fs_source);
+ glUseProgram(prog);
+#else
+ /* Create program pipeline. */
+ glGenProgramPipelines(1, &pipe);
+ glBindProgramPipeline(pipe);
+
+ vs = glCreateShaderProgramv(GL_VERTEX_SHADER, 1, &vs_source);
+ pass = piglit_link_check_status(vs) && pass;
+
+ fs = glCreateShaderProgramv(GL_FRAGMENT_SH
[Piglit] [PATCH] shader_runner: fix GL_SHADER_STORAGE_BLOCK enum name
Signed-off-by: Samuel Iglesias Gonsálvez --- tests/shaders/shader_runner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c index 9308905..14bca06 100644 --- a/tests/shaders/shader_runner.c +++ b/tests/shaders/shader_runner.c @@ -2100,7 +2100,7 @@ active_program_interface(const char *line) ENUM_STRING(GL_PROGRAM_INPUT), ENUM_STRING(GL_PROGRAM_OUTPUT), ENUM_STRING(GL_BUFFER_VARIABLE), - ENUM_STRING(GL_SHADER_STORAGE_BUFFER), + ENUM_STRING(GL_SHADER_STORAGE_BLOCK), ENUM_STRING(GL_ATOMIC_COUNTER_BUFFER), ENUM_STRING(GL_VERTEX_SUBROUTINE), ENUM_STRING(GL_TESS_CONTROL_SUBROUTINE), -- 2.5.0 ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
