Enhance debug capability in kms_chamelium
Subsystems (2)
cover Enhance debug capability in kms_chamelium
The first patch in the series cleans up the missed requirement for VGA tests in CI due to lack of support in later hardware.
The second patch adds a new libssh interface to extract chameleond daemon logs from the Chamelion board. Over time, it might be interesting to add new remote logging capability into the Python daemon itself or to move this ssh based log capture into the existing xmlrpc interface. For the short term, I am hoping to extract some additional logs from CI with this patch.
Stuart Summers (2):
i915/kms_chamelium: Require gen < 9 for VGA testing i915/kms_chamelium: Pull chameleond logs for debugging
Dockerfile.debian | 1 + Dockerfile.fedora | 1 + lib/igt_chamelium.c | 150 +++++++++++++++++++++++++++++++++++++++++- lib/meson.build | 4 ++ meson.build | 4 +- tests/kms_chamelium.c | 2 + 6 files changed, 160 insertions(+), 2 deletions(-)
Signature · 6 lines
2.20.1 _______________________________________________ igt-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/igt-dev
PATCH 1/2 i915/kms_chamelium: Require gen < 9 for VGA testing
From: Stuart Summers <[email protected]>
VGA not supported on gen 9 and above. Add an explicit requirement to skip VGA tests to a Chamelium on later gens.
--- tests/kms_chamelium.c | 2 ++ 1 file changed, 2 insertions(+)
tests/kms_chamelium.c +2 -0
@@ -1015,6 +1015,8 @@ igt_main igt_subtest_group { igt_fixture { +int gen =intel_gen (intel_get_drm_devid (data.drm_fd)); +igt_require_f (gen <9 ,"VGA not supported\n" );require_connector_present ( &data,DRM_MODE_CONNECTOR_VGA ); }
Signature · 6 lines
2.20.1 _______________________________________________ igt-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/igt-dev
Re: [igt-dev] [PATCH i-g-t 1/2] i915/kms_chamelium: Require gen < 9 for VGA testing
Quoted 15 lines · Stuart Summers via igt-dev
From: Stuart Summers <[email protected]>
VGA not supported on gen 9 and above. Add an explicit requirement to skip VGA tests to a Chamelium on later gens.
Signed-off-by Stuart Summers <[email protected]>--- tests/kms_chamelium.c | 2 ++ 1 file changed, 2 insertions(+)tests/kms_chamelium.c +0
-0
tests/kms_chamelium.c igt_main · line 1021
Quoted 7 lines
tests/kms_chamelium.c +2
-0@@ -1015,6 +1015,8 @@ igt_main igt_subtest_group { igt_fixture { +int gen =intel_gen (intel_get_drm_devid (data.drm_fd)); +igt_require_f (gen <9 ,"VGA not supported\n" );require_connector_present ( &data,DRM_MODE_CONNECTOR_VGA );
The obvious first comment is that you're not necessarily running on an intel device. But let's put that aside, there's a more important question.
What problem does this solve? The line directly after checks for a VGA connector being present. VGA via DP not good enough? If there's problems with that, surely there are better ways to get around it than ignoring it.
Signature · 5 lines
Petri Latvala _______________________________________________ igt-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/igt-dev
Re: [igt-dev] [PATCH i-g-t 1/2] i915/kms_chamelium: Require gen < 9 for VGA testing
Quoted 18 lines · Petri Latvala
Quoted 16 lines · Stuart Summers via igt-dev
From: Stuart Summers <[email protected]>
VGA not supported on gen 9 and above. Add an explicit requirement to skip VGA tests to a Chamelium on later gens.
Signed-off-by Stuart Summers <[email protected]>--- tests/kms_chamelium.c | 2 ++ 1 file changed, 2 insertions(+)tests/kms_chamelium.c +0
-0@@ -1015,6 +1015,8 @@ igt_main
Quoted 12 lines
tests/kms_chamelium.c igt_main · line 1021
Quoted 7 lines
tests/kms_chamelium.c +2
-0igt_subtest_group { igt_fixture { +int gen =intel_gen (intel_get_drm_devid (data.drm_fd)); +igt_require_f (gen <9 ,"VGA not supported\n" );require_connector_present ( &data,DRM_MODE_CONNECTOR_VGA );The obvious first comment is that you're not necessarily running on an
intel device. But let's put that aside, there's a more important question.
Good point. From your below feedback, I'm thinking of just dropping this. If it seems interesting to keep this around (if I hear more feedback here), I'll clean this up.
Quoted 3 lines
What problem does this solve? The line directly after checks for a VGA
I personally found this useful to differenciate an invalid system configuration causing CI to complain (i.e. someone forgot to plug in the cable) vs an intentional limitation (i.e. the platform doesn't support this). I agree it isn't strictly necessary since the subsequent check will prevent any actual test content from being exercised.
Also, maybe a precident of checking all platform capabilities for each of these isn't really the right direction. I'll drop this. Thanks for the feedback!
Quoted 6 lines
connector being present. VGA via DP not good enough? If there's problems with that, surely there are better ways to get around it than
ignoring it.
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
PATCH 2/2 i915/kms_chamelium: Pull chameleond logs for debugging
In reply to Stuart Summers via igt-devIGT currently logs some state information when passing commands to the Chamelium. There is a daemon running on the Chamelium itself, chameleond, which logs some additional information locally. Add a new set of libssh calls to extract those logs to be used in CI debug.
--- Dockerfile.debian | 1 + Dockerfile.fedora | 1 + lib/igt_chamelium.c | 150 +++++++++++++++++++++++++++++++++++++++++++- lib/meson.build | 4 ++ meson.build | 4 +- 5 files changed, 158 insertions(+), 2 deletions(-)
Dockerfile.debian +1 -0
@@ -18,6 +18,7 @@ RUN apt-get install -y \ libudev-dev \ libgsl-dev \ libasound2-dev \ + libssh-dev \ libxmlrpc-core-c3-dev \ libjson-c-dev \ libcurl4-openssl-dev \
Dockerfile.fedora +1 -0
@@ -16,6 +16,7 @@ RUN dnf install -y gcc \ json-c-devel \ libdrm-devel \ libudev-devel \ + libssh-devel \ xmlrpc-c-devel \ elfutils-devel \ libunwind-devel \
lib/igt_chamelium.c +149 -1
207 lines not on this page · raw message
lib/meson.build +4 -0
@@ -97,6 +97,10 @@ if alsa.found() lib_sources += 'igt_alsa.c' endif +if libssh.found() + lib_deps += libssh +endif + if chamelium.found() lib_deps += chamelium lib_sources += 'igt_chamelium.c'
meson.build +3 -1
@@ -166,6 +166,8 @@ cairo = dependency('cairo', version : '>1.12.0', required : true) libudev = dependency('libudev', required : true) glib = dependency('glib-2.0', required : true) +libssh = dependency('libssh', required : true) + gsl = null_dep alsa = null_dep if _build_audio or _build_chamelium
@@ -205,7 +207,7 @@ endif chamelium = null_dep chameliuminfo = 'No' -if _build_chamelium and gsl.found() and xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found() +if _build_chamelium and gsl.found() and libssh.found() and xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found() chamelium = declare_dependency(dependencies : [ xmlrpc, xmlrpc_util, xmlrpc_client]) config.set('HAVE_CHAMELIUM', 1)
Signature · 6 lines
2.20.1 _______________________________________________ igt-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/igt-dev
Re: [igt-dev] [PATCH i-g-t 2/2] i915/kms_chamelium: Pull chameleond logs for debugging
Quoted 44 lines · Stuart Summers via igt-dev
IGT currently logs some state information when passing commands to the Chamelium. There is a daemon running on the Chamelium itself, chameleond, which logs some additional information locally. Add a new set of libssh calls to extract those logs to be used in CI debug.
Signed-off-by Stuart Summers <[email protected]>--- Dockerfile.debian | 1 + Dockerfile.fedora | 1 + lib/igt_chamelium.c | 150 +++++++++++++++++++++++++++++++++++++++++++- lib/meson.build | 4 ++ meson.build | 4 +- 5 files changed, 158 insertions(+), 2 deletions(-)Dockerfile.debian +1
-0@@ -18,6 +18,7 @@ RUN apt-get install -y \ libudev-dev \ libgsl-dev \ libasound2-dev \ + libssh-dev \ libxmlrpc-core-c3-dev \ libjson-c-dev \ libcurl4-openssl-dev \Dockerfile.fedora +1
-0@@ -16,6 +16,7 @@ RUN dnf install -y gcc \ json-c-devel \ libdrm-devel \ libudev-devel \ + libssh-devel \ xmlrpc-c-devel \ elfutils-devel \ libunwind-devel \lib/igt_chamelium.c +0
-0
lib/igt_chamelium.c line 33
Quoted 4 lines
lib/igt_chamelium.c +1
-0@@ -30,6 +30,7 @@#include <errno.h> #include <xmlrpc-c/base.h> #include <xmlrpc-c/client.h> +#include <libssh/libssh.h>
The logs looks super useful to have, indeed!
Pulling ssh library and adding all the complexity, especially when we already have another way of talking to chamelium, seems a little too much though.
Have you looked into how complex it would be to add a XMLRPC call to fetch them instead?
Quoted 20 lines
lib/meson.build +4
-0@@ -97,6 +97,10 @@ if alsa.found() lib_sources += 'igt_alsa.c' endif +if libssh.found() + lib_deps += libssh +endif + if chamelium.found() lib_deps += chamelium lib_sources += 'igt_chamelium.c'meson.build +0
-0
meson.build cairo = dependency('cairo', version : '>1.12.0', required : true) · line 169
Quoted 4 lines
meson.build +1
-0@@ -166,6 +166,8 @@ cairo = dependency('cairo', version : '>1.12.0', required : true) libudev = dependency('libudev', required : true) glib = dependency('glib-2.0', required : true) +libssh = dependency('libssh', required : true)
"required : true" makes this dependency mandatory for the whole project, making the build fail.
What you want to do here is to make it not-required. The check you have below, 'libssh.found()' will make sure that the dependency is there for just the Chamelium.
Signature · 6 lines
Cheers, Arek _______________________________________________ igt-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/igt-dev
Re: [igt-dev] [PATCH i-g-t 2/2] i915/kms_chamelium: Pull chameleond logs for debugging
Quoted 57 lines · Arkadiusz Hiler
Quoted 47 lines · Stuart Summers via igt-dev
IGT currently logs some state information when passing commands to the Chamelium. There is a daemon running on the Chamelium itself,
chameleond, which logs some additional information locally. Add a new set of libssh calls to extract those logs to be used in CI debug.Signed-off-by Stuart Summers <[email protected]>--- Dockerfile.debian | 1 + Dockerfile.fedora | 1 + lib/igt_chamelium.c | 150 +++++++++++++++++++++++++++++++++++++++++++- lib/meson.build | 4 ++ meson.build | 4 +- 5 files changed, 158 insertions(+), 2 deletions(-)Dockerfile.debian +0
-0chameli
Dockerfile.debian +1
-0@@ -18,6 +18,7 @@ RUN apt-get install -y \ libudev-dev \ libgsl-dev \ libasound2-dev \ + libssh-dev \ libxmlrpc-core-c3-dev \ libjson-c-dev \ libcurl4-openssl-dev \Dockerfile.fedora +1
-0@@ -16,6 +16,7 @@ RUN dnf install -y gcc \ json-c-devel \ libdrm-devel \ libudev-devel \ + libssh-devel \ xmlrpc-c-devel \ elfutils-devel \ libunwind-devel \lib/igt_chamelium.c +0
-0lib/igt_chamelium.c line 33
Quoted 4 lines
lib/igt_chamelium.c +1
-0@@ -30,6 +30,7 @@#include <errno.h> #include <xmlrpc-c/base.h> #include <xmlrpc-c/client.h> +#include <libssh/libssh.h> The logs looks super useful to have, indeed!dy have another way of talking to chameli
Quoted 7 lines
Pulling ssh library and adding all the complexity, especially when we already have another way of talking to chamelium, seems a little too much though.
Have you looked into how complex it would be to add a XMLRPC call to fetch them instead?
Yeah I thought about that...
Basically I'm thinking we have a few options:
1) Do nothing. Don't get the logs.
2) Implement C-based SSH. Obviously there is the code complexity.
3) Implement shell-based SSH. This seems contradictory to the rest of IGT, but would of course remove the code complexity. Assuming this was some sort of post script, it would also not give you quite the fine grained granular logging I have in this patch (i.e. only take the logs leading up to an assert).
4) Implement RPC based approach. I like this idea, since like you said we already have the interface. It means we're adding more code here (minimal) and in chameleond (also minimal). Which means the turn-around time is going to be greater.
5) Implement in both SSH here and in RPC in chameleond. This could be a two staged approach where we first push something like what I have, then either get rid of it once we get the RPC interface in chameleond. or support both options if that makes sense.
Thoughts?
Quoted 29 lines
Quoted 20 lines
lib/meson.build +4
-0@@ -97,6 +97,10 @@ if alsa.found() lib_sources += 'igt_alsa.c' endif +if libssh.found() + lib_deps += libssh +endif + if chamelium.found() lib_deps += chamelium lib_sources += 'igt_chamelium.c'meson.build +0
-0meson.build cairo = dependency('cairo', version : · line 170
Quoted 5 lines
meson.build +1
-0@@ -166,6 +166,8 @@ cairo = dependency('cairo', version : '>1.12.0', required : true) libudev = dependency('libudev', required : true) glib = dependency('glib-2.0', required : true) +libssh = dependency('libssh', required : true)"required : true" makes this dependency mandatory for the whole project, making the build fail.
Quoted 5 lines
What you want to do here is to make it not-required. The check you have
below, 'libssh.found()' will make sure that the dependency is there for
just the Chamelium.
Thanks for the tip! If we get the infrastructure to support libssh on the chamelium hosts, I'll post an update with this change.
Quoted 1 line
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev
Patchwork
CI[igt-dev] ✗ Fi.CI.BAT: failure for Enhance debug capability in kms_chamelium
In reply to Stuart Summers via igt-dev== Series Details ==
== Series Details ==
Series: Enhance debug capability in kms_chamelium
URL : https://patchwork.freedesktop.org/series/56699/
State : failure
== Summary ==
IGT patchset build failed on latest successful build 395eaffd7e1390c9d6043c2980dc14ce3e08b154 kms_content_protection: Macros replace the constants
Compiler for C supports argument -Wmissing-format-attribute: YES Compiler for C supports argument -Wmissing-noreturn: YES Compiler for C supports argument -Wmissing-prototypes: YES Compiler for C supports argument -Wnested-externs: YES Compiler for C supports argument -Wold-style-definition: YES Compiler for C supports argument -Wpointer-arith: YES Compiler for C supports argument -Wredundant-decls: YES Compiler for C supports argument -Wshadow: YES
Compiler for C supports argument -Wstrict-prototypes: YES Compiler for C supports argument -Wuninitialized: YES Compiler for C supports argument -Wunused: YES
Compiler for C supports argument -Wno-clobbered: YES Compiler for C supports argument -Wno-maybe-uninitialized: YES Compiler for C supports argument -Wno-missing-field-initializers: YES Compiler for C supports argument -Wno-pointer-arith: YES Compiler for C supports argument -Wno-sign-compare: YES Compiler for C supports argument -Wno-type-limits: YES Compiler for C supports argument -Wno-unused-parameter: YES Compiler for C supports argument -Wno-unused-result: YES Compiler for C supports argument -Werror=address: YES Compiler for C supports argument -Werror=array-bounds: YES Compiler for C supports argument -Werror=implicit: YES Compiler for C supports argument -Werror=init-self: YES Compiler for C supports argument -Werror=int-to-pointer-cast: YES Compiler for C supports argument -Werror=main: YES Compiler for C supports argument -Werror=missing-braces: YES Compiler for C supports argument -Werror=nonnull: YES Compiler for C supports argument -Werror=pointer-to-int-cast: YES Compiler for C supports argument -Werror=return-type: YES Compiler for C supports argument -Werror=sequence-point: YES Compiler for C supports argument -Werror=trigraphs: YES Compiler for C supports argument -Werror=write-strings: YES Found pkg-config: /usr/bin/pkg-config (0.29.1)
Native dependency libdrm found: YES 2.4.97
Native dependency libdrm_intel found: YES 2.4.97
Native dependency libdrm_nouveau found: YES 2.4.91
Native dependency libdrm_amdgpu found: YES 2.4.97
Native dependency pciaccess found: YES 0.14
Native dependency libkmod found: YES 24
Native dependency libprocps found: YES 3.3.12
Native dependency libunwind found: YES 1.21
Native dependency libdw found: YES 0.170
Native dependency pixman-1 found: YES 0.34.0
Native dependency cairo found: YES 1.15.10
Native dependency libudev found: YES 237
Native dependency glib-2.0 found: YES 2.56.3
meson.build:169:0: ERROR: Native dependency 'libssh' not found
A full log can be found at /home/cidrm/igt-gpu-tools/build/meson-logs/meson-log.txt
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev