[Intel-gfx] [PATCH i-g-t 2/5] tests: Build gem_concurrent_all with meson

[Arkadiusz Hiler]

...and add it to test-list-full.txt just like we do when building with
autotools.

Signed-off-by: Arkadiusz Hiler 
---
 tests/meson.build | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/meson.build b/tests/meson.build
index 54a1a3c7..8e3cd390 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -402,6 +402,19 @@ test_list_target = custom_target('testlist',
  install : true,
  install_dir : libexecdir)
 
+test_executables += executable('gem_concurrent_all', 
'i915/gem_concurrent_all.c',
+  dependencies : test_deps + [ libatomic ],
+  install_dir : libexecdir,
+  install_rpath : libexecdir_rpathdir,
+  install : true)
+test_list += 'gem_concurrent_all'
+
+test_list_full_target = custom_target('testlist-full',
+ output : 'test-list-full.txt',
+ command : [ gen_testlist, '@OUTPUT@', test_list ],
+ install : true,
+ install_dir : libexecdir)
+
 test_script = find_program('igt_command_line.sh')
 foreach prog : test_list
test('testcase check ' + prog, test_script, args : prog)
-- 
2.31.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 3/5] tests: Remove ddx_intel_after_fbdev

[Arkadiusz Hiler]

It's not a even a proper test.

Suggested-by: Petri Latvala 
Signed-off-by: Arkadiusz Hiler 
---
 tests/Makefile.sources  |  4 --
 tests/ddx_intel_after_fbdev | 73 -
 2 files changed, 77 deletions(-)
 delete mode 100755 tests/ddx_intel_after_fbdev

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 4f24fb3a..ffc7878a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -550,10 +550,6 @@ kernel_tests_full = \
$(extra_kernel_tests) \
$(NULL)
 
-scripts = \
-   ddx_intel_after_fbdev \
-   $(NULL)
-
 IMAGES = pass.png 1080p-left.png 1080p-right.png
 
 testdisplay_SOURCES = \
diff --git a/tests/ddx_intel_after_fbdev b/tests/ddx_intel_after_fbdev
deleted file mode 100755
index f068209d..
--- a/tests/ddx_intel_after_fbdev
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/bin/bash
-#
-# Testcase: Load Intel DDX after fbdev was loaded
-#
-
-whoami | grep -q root || {
-   echo "ERROR: not running as root"
-   exit 1
-}
-
-# no other X session should be running
-find /tmp/ -name .X*lock 2>/dev/null | grep -q X && {
-   echo "ERROR: X session already running"
-   exit 1
-}
-
-TMPDIR=$(mktemp -d /tmp/igt.) || {
-   echo "ERROR: Failed to create temp dir"
-   exit 1
-}
-
-cat > $TMPDIR/xorg.conf.fbdev << EOF
-Section "Device"
-   Driver  "fbdev"
-   Identifier  "Device[fbdev]"
-EndSection
-EOF
-
-cat > $TMPDIR/xorg.conf.intel << EOF
-Section "Device"
-   Driver  "intel"
-   Identifier  "Device[intel]"
-EndSection
-EOF
-
-# log before fbdev
-dmesg -c > $TMPDIR/dmesg.1.before.fbdev
-cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.1.before.fbdev
-
-# run fbdev
-xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.fbdev & 
-sleep 5
-if [ -f `which intel_reg` ]; then
-`which intel_reg` dump > $TMPDIR/intel_reg_dump.1.fbdev
-fi
-killall X
-
-# log after fbdev & before intel
-dmesg -c > $TMPDIR/dmesg.2.after.fbdev.before.intel
-cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.2.after.fbdev.before.intel
-
-sleep 5
-
-# run intel
-xinit -- /usr/bin/X -config $TMPDIR/xorg.conf.intel & 
-sleep 5 
-if [ -f `which intel_reg` ]; then
-`which intel_reg` dump > $TMPDIR/intel_reg_dump.2.intel
-fi
-killall X
-
-# log after intel
-dmesg -c > $TMPDIR/dmesg.3.after.intel
-cp /var/log/Xorg.0.log $TMPDIR/Xorg.0.log.3.after.intel
-
-cp $0 $TMPDIR/
-
-tar czf $TMPDIR.tar.gz $TMPDIR/*
-if [ -f $TMPDIR.tar.gz ]; then
-   echo $TMPDIR.tar.gz contains this script, all configs and logs 
generated on this tests
-fi
-
-exit 0
-- 
2.31.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 5/5] Get rid of GNU Autotools

[Arkadiusz Hiler]

Autotools have been deprecated in favor of Meson since early 2019.

Cc: Daniel Vetter 
Cc: Petri Latvala 
Cc: Tomi Sarvela 
Signed-off-by: Arkadiusz Hiler 
---
 Makefile.am  |  44 ---
 autogen.sh   |  17 -
 benchmarks/Makefile.am   |  28 --
 benchmarks/Makefile.sources  |  28 --
 configure.ac | 343 --
 lib/Makefile.am  | 226 
 lib/Makefile.sources | 195 ---
 m4/as-compiler-flag.m4   |  62 
 m4/ax_gcc_func_attribute.m4  | 226 
 overlay/Makefile.am  |  70 
 scripts/Makefile.am  |   2 -
 tests/Makefile.am| 153 
 tests/Makefile.sources   | 581 ---
 tests/intel-ci/Makefile.am   |   5 -
 tools/Makefile.am|  31 --
 tools/Makefile.sources   |  80 -
 tools/i915-perf/Makefile.am  |  29 --
 tools/meson.build|   5 +-
 tools/null_state_gen/Makefile.am |  31 --
 tools/registers/Makefile.am  |   2 -
 20 files changed, 1 insertion(+), 2157 deletions(-)
 delete mode 100644 Makefile.am
 delete mode 100755 autogen.sh
 delete mode 100644 benchmarks/Makefile.am
 delete mode 100644 benchmarks/Makefile.sources
 delete mode 100644 configure.ac
 delete mode 100644 lib/Makefile.am
 delete mode 100644 lib/Makefile.sources
 delete mode 100644 m4/as-compiler-flag.m4
 delete mode 100644 m4/ax_gcc_func_attribute.m4
 delete mode 100644 overlay/Makefile.am
 delete mode 100644 scripts/Makefile.am
 delete mode 100644 tests/Makefile.am
 delete mode 100644 tests/Makefile.sources
 delete mode 100644 tests/intel-ci/Makefile.am
 delete mode 100644 tools/Makefile.am
 delete mode 100644 tools/Makefile.sources
 delete mode 100644 tools/i915-perf/Makefile.am
 delete mode 100644 tools/null_state_gen/Makefile.am
 delete mode 100644 tools/registers/Makefile.am

diff --git a/Makefile.am b/Makefile.am
deleted file mode 100644
index 94250964..
--- a/Makefile.am
+++ /dev/null
@@ -1,44 +0,0 @@
-# Copyright © 2005 Adam Jackson.
-# Copyright © 2009,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
-#  on the rights to use, copy, modify, merge, publish, distribute, sub
-#  license, 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 NON-INFRINGEMENT.  IN NO EVENT SHALL
-#  ADAM JACKSON 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.
-
-ACLOCAL_AMFLAGS = ${ACLOCAL_FLAGS} -I m4
-
-SUBDIRS = lib tools scripts benchmarks
-
-if BUILD_TESTS
-SUBDIRS += tests
-endif
-
-if BUILD_X86
-SUBDIRS += overlay benchmarks
-endif
-
-MAINTAINERCLEANFILES = ChangeLog INSTALL
-
-.PHONY: ChangeLog INSTALL
-
-INSTALL:
-   $(INSTALL_CMD)
-
-ChangeLog:
-   $(CHANGELOG_CMD)
-
-dist-hook: ChangeLog INSTALL
diff --git a/autogen.sh b/autogen.sh
deleted file mode 100755
index 65fcab2f..
--- a/autogen.sh
+++ /dev/null
@@ -1,17 +0,0 @@
-#! /bin/sh
-
-srcdir=`dirname $0`
-test -z "$srcdir" && srcdir=.
-
-ORIGDIR=`pwd`
-cd $srcdir
-
-autoreconf -v --install || exit 1
-cd $ORIGDIR || exit $?
-
-git config --local --get format.subjectPrefix >/dev/null 2>&1 ||
-git config --local format.subjectPrefix "PATCH i-g-t"
-
-if test -z "$NOCONFIGURE"; then
-$srcdir/configure "$@"
-fi
diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am
deleted file mode 100644
index 45b923eb..
--- a/benchmarks/Makefile.am
+++ /dev/null
@@ -1,28 +0,0 @@
-include Makefile.sources
-
-benchmarks_PROGRAMS = $(benchmarks_prog_list)
-
-if HAVE_LIBDRM_INTEL
-benchmarks_PROGRAMS += $(LIBDRM_INTEL_BENCHMARKS)
-endif
-
-AM_CPPFLAGS = \
-   -I$(top_srcdir) \
-   -I$(top_srcdir)/include/drm-uapi \
-   -I$(top_srcdir)/lib \
-   -I$(top_srcdir)/lib/stubs/syscalls
-
-AM_CFLAGS = -I$(top_srcdir)/include/drm-uapi \
-   $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) \
-   $(WERROR_CFLAGS) -D_GNU_SOURCE
-LDADD = $(top_builddir)/lib/libintel_tools.la
-
-benchmarks_LTLIBRARIES = gem_exec_tracer.la
-gem_exec_tracer_la_LD

[Intel-gfx] [PATCH i-g-t 4/5] .gitlab-ci: Don't test Autotools

[Arkadiusz Hiler]

Signed-off-by: Arkadiusz Hiler 
---
 .gitlab-ci.yml  | 18 --
 Dockerfile.build-debian |  8 
 2 files changed, 26 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index e226d9d7..2b03fc98 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -154,17 +154,6 @@ build:tests-debian-meson-mips:
 paths:
   - build
 
-build:tests-debian-autotools:
-  image: $CI_REGISTRY/$CI_PROJECT_PATH/build-debian:commit-$CI_COMMIT_SHA
-  stage: build
-  script:
-- ./autogen.sh --enable-{chamelium,audio,intel,amdgpu,nouveau,tests,runner}
-- make -j $(nproc) || make -j 1
-- cp tests/test-list.txt autotools-test-list.txt
-  artifacts:
-paths:
-  - autotools-test-list.txt
-
  TEST ##
 
 test:ninja-test:
@@ -236,13 +225,6 @@ test:ninja-test-mips:
   - build
 when: on_failure
 
-test:test-list-diff:
-  dependencies:
-- build:tests-debian-autotools
-- build:tests-debian-meson
-  stage: test
-  script: diff <(sed "s/ /\n/g" meson-test-list.txt| grep -v 
'vc4\|v3d\|panfrost\|nouveau' | sort) <(sed "s/ /\n/g" autotools-test-list.txt 
| sort)
-
 test:list-undocumented-tests:
   dependencies:
 - build:tests-fedora
diff --git a/Dockerfile.build-debian b/Dockerfile.build-debian
index b143a532..454f4bce 100644
--- a/Dockerfile.build-debian
+++ b/Dockerfile.build-debian
@@ -20,12 +20,4 @@ RUN apt-get install -y \
peg \
libdrm-intel1
 
-# autotools build deps
-RUN apt-get install -y \
-   autoconf \
-   automake \
-   xutils-dev \
-   libtool \
-   make
-
 RUN apt-get clean
-- 
2.31.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 1/5] benchmarks: Build gem_exec_tracer with meson

[Arkadiusz Hiler]

Seems it has been overlooked during mesonification.

It's a shared module that's meant to be LD_PRELOAD-ed to intercept
EXECBUFFER2 calls for the purpose of replaying them later.

Signed-off-by: Arkadiusz Hiler 
---
 benchmarks/meson.build | 8 
 1 file changed, 8 insertions(+)

diff --git a/benchmarks/meson.build b/benchmarks/meson.build
index c70e1aac..bede51dc 100644
--- a/benchmarks/meson.build
+++ b/benchmarks/meson.build
@@ -35,3 +35,11 @@ foreach prog : benchmark_progs
   install_dir : benchmarksdir,
   dependencies : igt_deps)
 endforeach
+
+lib_gem_exec_tracer = shared_module(
+  'gem_exec_tracer',
+  'gem_exec_tracer.c',
+  dependencies : dlsym,
+  include_directories : inc,
+  install_dir : benchmarksdir,
+  install: true)
-- 
2.31.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 0/2] minor improvements to the kms_cursor_crc doc and some comments cleanup

[Arkadiusz Hiler]

On Wed, Jun 24, 2020 at 06:54:00AM -0300, Melissa Wen wrote:
> Hi,
> 
> I was studying the code of kms_cursor_crc test, and I just adjusted some 
> comments
> and added descriptions for subtests.
> 
> Melissa Wen (2):
>   lib/igt_fb: change comments with fd description
>   test/kms_cursor_crc: update subtests descriptions and some comments

Seems like there's a conflict caused by your patch removing unused
parameters from igt_put_cairo_ctx().

Can you an send updated version and CC me on it?

In case of false positives please comment on the CI results with a short
explanation and CC Lakshmi 

Thanks for the cleanup!

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank

[Arkadiusz Hiler]

On Mon, Jun 22, 2020 at 01:38:26PM -0300, Melissa Wen wrote:
> When running subtests in sequence using vkms, the beginning of CRC capture
> process does not match the simulated vblank timing. This mismatch leads to
> an endless busy wait and, consequently, timeout failures for the remaining
> subtests in the test sequence. This patch sets the pace by waiting for
> vblank before starting the CRC capture.
> 
> Signed-off-by: Melissa Wen 

This one is quite interetesing. The test seems to be working just fine
on the real hardware and causes the endless busy wait on VKMS only...

DRM is bad at describing usage sequences and what's defined and what's
undefined when it comes to behavior. We just try not to break any of the
existing users. On top of that CRC capture is a testing/debug feature
that doesn't have have to be stable - it's not really obvious what's the
correct course of action here.

The vblank wait won't harm anyone, especially in the context presented
above. You have to keep in mind that other implementations of CRC
caputring doesn't have that requirement and you will likely find more
similar instances of this usage pattern. There may be even more of them
introduced over time - there's no CI on VKMS (fingers crossed that this
is going to change).

Have you thought about what's easier here - making the current code work
on the VKMS side or fixing the test? I would like to know your thoughts
on this.

-- 
Cheers,
Arek




> ---
>  tests/kms_cursor_crc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 5976df5f..755c34ed 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -474,6 +474,7 @@ static void prepare_crtc(data_t *data, igt_output_t 
> *output,
>   igt_assert(data->batch);
>   }
>  
> + igt_wait_for_vblank(data->drm_fd, data->pipe);
>   igt_pipe_crc_start(data->pipe_crc);
>  }
>  
> -- 
> 2.27.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/2] test/kms_cursor_crc: release old pipe_crc before create a new one

[Arkadiusz Hiler]

On Mon, Jun 22, 2020 at 01:37:55PM -0300, Melissa Wen wrote:
> When a subtest fails, it skips the cleanup, and its pipe_crc remains 
> allocated.
> As a consequence, the following subtest also fails (timeout) when trying to
> create a new one. This patch releases any remaining pipe_crc to enable the
> creation of a new one for the next subtest.
> 
> Signed-off-by: Melissa Wen 
> ---
>  tests/kms_cursor_crc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index f105e295..5976df5f 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -423,6 +423,8 @@ static void prepare_crtc(data_t *data, igt_output_t 
> *output,
>   igt_display_commit(display);
>  
>   /* create the pipe_crc object for this pipe */
> + if (data->pipe_crc)
> + igt_pipe_crc_free(data->pipe_crc);

That's a welcome improvement, but you may want to also look at
06333955bf3d ("tests/kms_cursor_crc: start crc only once per test")
for some extra inspiration for future work on this.

It should be possible to initiate pipe crc to be initalized only once
per each tested pipe in run_tests_on_pipe() - igt_pipe_crc_new() can be
costly on some real panels.

Anyway,
Reviewed-by: Arkadiusz Hiler 


>   data->pipe_crc = igt_pipe_crc_new(data->drm_fd, data->pipe,
> INTEL_PIPE_CRC_SOURCE_AUTO);
>  
> -- 
> 2.27.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

[Arkadiusz Hiler]

On Fri, Jun 26, 2020 at 12:18:00PM +0200, Janusz Krzysztofik wrote:
> Hi Michał,
> 
> Thanks for review.
> 
> On Thu, 2020-06-25 at 17:27 +0200, Michał Winiarski wrote:
> > Quoting Janusz Krzysztofik (2020-06-22 18:44:08)
> > > The purpose of debug messages displayed by the test is to make
> > > identification of a subtest phase that fails more easy.  Since issues
> > > exhibited by the test are mostly reported to dmesg, print those debug
> > > messages to /dev/kmsg as well.
> > 
> > I'm not a fan of spamming dmesg from IGT and I'd prefer if you add this 
> > logging
> > to the kernel, 
> The idea was to simply log IGT actions, not to log kernel reactions on
> them which already happens.  Doing that from the kernel would probably
> require modifications to PCI sysfs handling or to sysfs in general.
> 
> If you see no benefits from that, let's drop this patch.

We (me & Petri) were thinking about doing something similar, but only
for the places where kernel logs are hard to correlate with the test
actions, to have those "synchronization points" between logs for key
operations.

The main reason was Chamelium - external board that simulates display
and can cause hotplug events to happen. Logging Chamelium operations in
dmesg would make any kind of bug assessment or troubleshooting much
easier - was that a phantom hotplug? or something that was triggered by
us? Have we started doing anything else before the link has settled?
What happened during DP FSM handling?

This is a very special case as we deal with an external board that
drives our HW through wires and layers of firmware and for sure there be
dragons.


Anyway, I would like to see us having a way of logging those "sync
points" into kmesg in igt_core, e.g. by creating _kmesg suffixed version
of log functions.

But I also see Michał's point - too frivolous logging just creates
noise, and we shouldn't double log - if something is already easy to
find in the kernel logs and the correlating test action to logs is not
too hard why should we add more noise?

As of the examples in this thread - I am not very familiar with the area
and I leave it up to you two to decide what would be helpful, what would
be unnecessary repetition and what would be better off logged in the
kernel.

TL;DR: Yes for logging things into kmesg, but we should be careful about
   what we log to not make the situation any worse.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/display: Increase the DDI idle timeout to 500us

[Arkadiusz Hiler]

On Thu, Mar 19, 2020 at 11:20:34AM +0200, Arkadiusz Hiler wrote:
> Bspec says that we should timeout after 500us. Let's match this in the
> code. It may help with few of the timeouts we see here and there.

Plese disregard. it's 500us when waiting on non-idle and only 8 (16
for BXT) for back to idle.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/display: Increase the DDI idle timeout to 500us

[Arkadiusz Hiler]

Bspec says that we should timeout after 500us. Let's match this in the
code. It may help with few of the timeouts we see here and there.

Bspec: 22243, 49190
Issue: https://gitlab.freedesktop.org/drm/intel/issues/1069
Suggested-by: Uma Shankar 
Cc: Imre Deak 
Signed-off-by: Arkadiusz Hiler 
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 73d0f4648c06..28650797fc2f 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1097,7 +1097,7 @@ static void intel_wait_ddi_buf_idle(struct 
drm_i915_private *dev_priv,
i915_reg_t reg = DDI_BUF_CTL(port);
int i;
 
-   for (i = 0; i < 16; i++) {
+   for (i = 0; i < 500; i++) {
udelay(1);
if (intel_de_read(dev_priv, reg) & DDI_BUF_IS_IDLE)
return;
-- 
2.24.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

[Arkadiusz Hiler]

On Wed, Mar 18, 2020 at 10:43:29AM +0200, Lisovskiy, Stanislav wrote:
> Wonder, how we end up merging _stuff_ with failed IGT and loads of warnings..
> 
> https://patchwork.freedesktop.org/series/74738/
> 
> ... while I get beaten for each and every single warning in my patches 

True. This shouldn't get merged like this. Ask the authors and the
commiters why this got in without anyone looking at the docs issue and
without any explanation of the failures in Fi.CI.IGT.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

[Arkadiusz Hiler]

On Tue, Mar 17, 2020 at 12:43:40PM +0200, Lisovskiy, Stanislav wrote:
> What is this weird DOC warning about? "Error: Cannot open file ./drivers/gpu/
> drm/i915/i915_gem_fence_reg.c"
> 
> - wondering, how is that related to this patch.

Looks like you were unlucky and your series got tested with this merged:
https://patchwork.freedesktop.org/series/74738/

but before the fix has landed:
https://patchwork.freedesktop.org/series/74778/

It's all clean now.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [CI] PR for TGL DMC v2.06

[Arkadiusz Hiler]

On Fri, Feb 28, 2020 at 06:52:01PM +, Souza, Jose wrote:
> On Fri, 2020-02-28 at 12:21 +0200, Petri Latvala wrote:
> > On Thu, Feb 27, 2020 at 11:42:03PM +, Souza, Jose wrote:
> > > The following changes since commit
> > > efcfa03ae6100dfe523ebf612e03c3a90fc4c794:
> > > 
> > >   linux-firmware: Update firmware file for Intel Bluetooth AX201
> > > (2020-
> > > 02-24 07:43:42 -0500)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://anongit.freedesktop.org/drm/drm-firmware tgl_dmc_2.06
> > > 
> > > for you to fetch changes up to
> > > e2396319167724e9ffddc377f300469923fccdcb:
> > > 
> > >   i915: Add DMC firmware v2.06 for TGL (2020-02-27 15:24:56 2020
> > > -0800)
> > > 
> > > 
> > > José Roberto de Souza (1):
> > >   i915: Add DMC firmware v2.06 for TGL
> > > 
> > >  WHENCE  |   3 +++
> > >  i915/tgl_dmc_ver2_06.bin | Bin 0 -> 18660 bytes
> > >  2 files changed, 3 insertions(+)
> > >  create mode 100644 i915/tgl_dmc_ver2_06.bin
> > 
> > Patchwork didn't pick up this PR, I suspect the extra newlines to be
> > the issue. Can you try resending this without the automatic newlines
> > before the commit shas?
> > 
> > If patchwork recognizes it as a pull request, it will appear in here:
> > https://patchwork.freedesktop.org/api/1.0/projects/intel-gfx/events/?page=1=2020-02-20=pull-request-new
> > 
> 
> Hi Petri
> 
> Still needed? According to 
> https://patchwork.freedesktop.org/patch/355624/?series=74048=2 it
> was manually picked.

Hey,

Patchwork got defeated by the carriage return this time:

00d0: 7369 746f 7279 2061 743a 0d0a 0d0a 2020  sitory at:

Which doesn't match this regexp:

  git_re = re.compile(r'^The following changes since commit.*' +
r'^are available in the Git repository at:\n'
r'^\s*([\S]+://[^\n]+)$',
re.DOTALL | re.MULTILINE | re.IGNORECASE)

I'll turn that \n into \s+ or $ but you may also want to double check
you mailing setup. Parsing mails is notoriously hard and error prone -
those carriage returns may break something elsewhere for you.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/mst: fix pipe and vblank enable

[Arkadiusz Hiler]

On Mon, Feb 10, 2020 at 01:43:47PM +0200, Lisovskiy, Stanislav wrote:
> On Mon, 2020-02-10 at 13:32 +0200, Jani Nikula wrote:
> > On Wed, 05 Feb 2020, Jani Nikula  wrote:
> > > Commit 21fd23ac222f ("drm/i915: move pipe, pch and vblank enable to
> > > encoders on DDI platforms") pushed pipe and vblank enable to
> > > encoders on
> > > DDI platforms, however it missed the DP MST encoder. Fix it.
> > > 
> > > Fixes: 21fd23ac222f ("drm/i915: move pipe, pch and vblank enable to
> > > encoders on DDI platforms")
> > > Cc: Vandita Kulkarni 
> > > Cc: Ville Syrjala 
> > > Reported-by: Stanislav Lisovskiy 
> > > Signed-off-by: Jani Nikula 
> > 
> > Thanks for the reviews and testing, pushed to dinq.
> > 
> > I don't usually cut corners, but I've made an exception and pushed
> > this without full IGT results.
> > 
> > It's been 5 days since the patch was posted, the sharded run has
> > fallen between the cracks, and the queue is currently about three
> > days. IMHO it's intolerable for any patch, but especially so for a
> > regression fix that was posted within hours of the bug report.
> 
> Absolutely agree, since we already had a regression, it's pointless
> now to wait longer with such a trivial fix. We are anyway in a bad
> situation now, checking also some other MST issues and having to apply
> this patch manually first in order to get at least this issue ruled
> out.
> 
> Stan

As of why it was silently dropped:

We poke patchwork to check whether there is a newer version of a given
series. If there is we won't waste time on running the older one through
shards.

This bit looks more or less like this:

  RES="$(curl -q 
https://patchwork.freedesktop.org/api/1.0/series/$SER/revisions/$(( $REV + 1 
))/ 2>/dev/null)"
  [[ "$RES" = "" || "$RES" = *"ot found"* ]] || exit 1

If there is a network issue and curl exits with non-zero exit status
this aborts the shards because of `set -e`, which is what has happened:

  +++ curl -q 
https://patchwork.freedesktop.org/api/1.0/series/73006/revisions/2/
  ++ RES=
  Build step 'Execute shell' marked build as failure

So a network issue + not robust enough bash script is the cause.

I have fixed the logic there to account for this and in case of network
errors we just go ahead with testing. Thanks for rising this up.


As of the 3 days worth of queued shards:

I agree that this is unacceptable, but we can do only so much from the
CI/infra side. The time has been creeping up steadily over the last year
or so and the machines are not getting any faster.

We are currently sitting on ~58min for a run and Tomi has already done a
lot of in terms of optimization. The overhead is as minimal as it can be
and there is some logic tracking the test execution times and doing
random but balanced test distribution.

We are also considering introducing hard limits on subtest execution
times and hunting down the tests that are exceeding this.

On IGT side there was a recent introduction of dynamic subtest which
should help with time wasted on some of the skips, and I am working on a
more reliable skips for multiple mode testing (currently one subtest =
one execv) but without optimizing the test cases we won't shave off much
time.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BUILD: warning for Enable second DBuf slice for ICL and TGL (rev13)

[Arkadiusz Hiler]

On Wed, Jan 15, 2020 at 05:49:42PM +0200, Lisovskiy, Stanislav wrote:
> There is some kind of build failure happening with all recent series:
> 
> 
> CALLscripts/checksyscalls.sh
>   CALLscripts/atomic/check-atomics.sh
>   CHK include/generated/compile.h
> Kernel: arch/x86/boot/bzImage is ready  (#1)
>   Building modules, stage 2.
>   MODPOST 122 modules
> ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> scripts/Makefile.modpost:93: recipe for target '__modpost' failed
> make[1]: *** [__modpost] Error 1
> Makefile:1282: recipe for target 'modules' failed
> make: *** [modules] Error 2
> 

this is just the 32bit build failing, fix already on the mailing list
and even probably in:

https://patchwork.freedesktop.org/series/71961/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [ANNOUNCEMENT] Documenting tests with igt_describe()

[Arkadiusz Hiler]

On Tue, Nov 19, 2019 at 02:37:17PM +0200, Lionel Landwerlin wrote:
> On 08/11/2019 11:04, Arkadiusz Hiler wrote:
> > On Thu, Nov 07, 2019 at 09:09:34PM +, Chris Wilson wrote:
> > > Quoting Arkadiusz Hiler (2019-11-07 17:38:20)
> > > > We don't want you to translate C into English, we want you to provide a 
> > > > bit of
> > > > that extra information that you would have put in the comments anyway.
> > > The comments should exist and are _inline_ with the code.
> > And I encourage doing so. Detailed comments what this particular
> > non-obvious chunk of code is doing are always welcome.
> > 
> > > In all the examples of igt_describe() I have seen, it is nowhere near
> > > the code so is useless; the information conveyed does not assist
> > > anyone in diagnosing or debugging the problem, so I yet to understand
> > > how it helps.
> > They are supposed to document not the implementation but what is the
> > grand idea behind a given subtest, so they are on a subtest level (or a
> > group of subtests), which is our basic testing unit - this is what fails
> > in CI, this is what you execute locally to reproduce the issue.
> > 
> > Can you truly debug an issue or understand what the failure means
> > without knowing what the test is supposed to prove?
> > 
> > A lot of people here have struggled with this and often it takes a lot
> > of time and requires advanced archeology with `git blame` hoping that
> > there is at least one detailed enough commit message in there.
> > 
> > If not then talking to people and relying on our tribal knowledge is
> > required.
> > 
> > As I have mentioned - getting the bigger picture from implementation
> > details is hard. I get that you are not affected by this, but please do
> > not deny the others.
> 
> 
> I kind of agree with Chris that I don't find that additional macro useful
> from the point of view of reading a particular test.
> 
> A comment above the test function seems more appropriate, at least you don't
> have to look at 2 different places to find out about a particular test.

There is no easy way of automated enforcing such comments unless we want
to fork a tool like doxygen or write and maintain something on our own.

You don't have to go to two places, you can organize the comments however
you like, see what kms_chamelium is doing:

https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/tests/kms_chamelium.c#L456

You can even use a format string and igt_describe_f() in a loop
or just slap it over whole igt_subtest_group().

Once again - this is the only way we have found to make this enforcible,
and give us enough of flexibility to shuffle the text around.

> Unless there is some untold goal here, like producing some kind of report in
> an automated way.
> 
> 
> -Lionel

The goal is to have those descriptions in the first place and make them
more accessible to people. You have to keep in mind that we have
decently sized organization, people are coming and going. Not everyone
becomes a seasoned kernel developer day one and not everyone looking at
the tests and their results is a developer.

There are  some extra benefit that I see that is related to "automated
report" you have mentioned but it was not the main reason: you can put
the description of the tests with bugs filed with the CI.

There is probably more ways in which we could expose that data.

-- 
Cheers,
Arek

> > > What is more useful, a link to the kernel coverage of the test and
> > > link to the test source code, or waffle?
> > > -Chris
> > Those things are not exclusive. Coverage is extremely useful metric,
> > source code is where the action happens but some higher-level
> > explanations and waffles can coexist peacefully and make many lives much
> > more pleasant.
> > 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_blits: Use common igt_fls()

[Arkadiusz Hiler]

On Sat, Nov 09, 2019 at 03:10:02PM +, Chris Wilson wrote:
> igt_aux.h already provides the optimal igt_fls(), so use that in
> preference to open coding the brute force version.
> 
> Reported-by: Stuart Summers 
> Signed-off-by: Chris Wilson 
> Cc: Stuart Summers 
Reviewed-by: Arkadiusz Hiler 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [ANNOUNCEMENT] Documenting tests with igt_describe()

[Arkadiusz Hiler]

On Thu, Nov 07, 2019 at 09:09:34PM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-11-07 17:38:20)
> > We don't want you to translate C into English, we want you to provide a bit 
> > of
> > that extra information that you would have put in the comments anyway.
> 
> The comments should exist and are _inline_ with the code.

And I encourage doing so. Detailed comments what this particular
non-obvious chunk of code is doing are always welcome.

> In all the examples of igt_describe() I have seen, it is nowhere near
> the code so is useless; the information conveyed does not assist
> anyone in diagnosing or debugging the problem, so I yet to understand
> how it helps.

They are supposed to document not the implementation but what is the
grand idea behind a given subtest, so they are on a subtest level (or a
group of subtests), which is our basic testing unit - this is what fails
in CI, this is what you execute locally to reproduce the issue.

Can you truly debug an issue or understand what the failure means
without knowing what the test is supposed to prove?

A lot of people here have struggled with this and often it takes a lot
of time and requires advanced archeology with `git blame` hoping that
there is at least one detailed enough commit message in there.

If not then talking to people and relying on our tribal knowledge is
required.

As I have mentioned - getting the bigger picture from implementation
details is hard. I get that you are not affected by this, but please do
not deny the others.

> What is more useful, a link to the kernel coverage of the test and
> link to the test source code, or waffle?
> -Chris

Those things are not exclusive. Coverage is extremely useful metric,
source code is where the action happens but some higher-level
explanations and waffles can coexist peacefully and make many lives much
more pleasant.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [ANNOUNCEMENT] Documenting tests with igt_describe()

[Arkadiusz Hiler]

Hey all,

IGT tests are largely undocumented and a lot of them are quite enigmatic if you
haven't internalized the whole framework and are not familiar with naming
conventions that some people use.

To tackle this we require[0] documenting new tests with igt_describe()[1].

The idea is to provide a short description (one or two sentences) on
what the test is supposed to verify to give the reader enough of context
so they easily can tell what scenario the test is exercising.

This also makes reading the tests so much easier - sometimes it is
really hard and takes a long time to understand the main thought behind
a test just from the implementation details.

We don't want you to translate C into English, we want you to provide a bit of
that extra information that you would have put in the comments anyway.

See igt_describe() documentation[1] for guidelines on writing good descriptions.

[0]: 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/blob/master/CONTRIBUTING.md#L19
[1]: 
https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe

Signed-off-by: Arkadiusz Hiler 
Signed-off-by: Petri Latvala 


## FAQ

Q: How is this being enforced?
A: If your patch introduces undocumented tests you will get an email with
   instruction how to proceed. This is considered as failing the CI checks.
   e.g.: 
https://lists.freedesktop.org/archives/igt-dev/2019-November/017266.html

Q: I am not sure my descriptions are good...
A: That what the review is for. Feel free to poke me or Petri on IRC in case you
   want some help with writing them.

Q: Why are you using igt_describe() and not doxygen/sphinx/gtk-doc/etc.
   comments?
A: We are documenting tests, not C functions. Those tools do not
   understand comments over magic control blocks used by the test
   harness to define tests/subtests. Additional benefit is that the
   documentation is available not only in the source code and the
   generated documentation but also on the command line by using
   --describe switch.

-- 
Cheers,
Arek

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/lmem: add the fake lmem region

[Arkadiusz Hiler]

On Wed, Nov 06, 2019 at 11:17:29AM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-11-06 08:30:37)
> > On Tue, Nov 05, 2019 at 01:05:20PM +, Matthew Auld wrote:
> > > On Thu, 31 Oct 2019 at 20:40, Chris Wilson  
> > > wrote:
> > > >
> > > > Quoting Arkadiusz Hiler (2019-10-31 12:40:35)
> > > > > On Wed, Oct 30, 2019 at 10:22:37PM +, Matthew Auld wrote:
> > > > > > On Tue, 29 Oct 2019 at 16:51, Matthew Auld  
> > > > > > wrote:
> > > > > > >
> > > > > > > Intended for upstream testing so that we can still exercise the 
> > > > > > > LMEM
> > > > > > > plumbing and !i915_ggtt_has_aperture paths. Smoke tested on Skull 
> > > > > > > Canyon
> > > > > > > device. This works by allocating an intel_memory_region for a 
> > > > > > > reserved
> > > > > > > portion of system memory, which we treat like LMEM. For the 
> > > > > > > LMEMBAR we
> > > > > > > steal the aperture and 1:1 it map to the stolen region.
> > > > > > >
> > > > > > > To enable simply set the i915 modparam fake_lmem_start= on the 
> > > > > > > kernel
> > > > > > > cmdline with the start of reserved region(see memmap=). The size 
> > > > > > > of the
> > > > > > > region we can use is determined by the size of the mappable 
> > > > > > > aperture, so
> > > > > > > the size of reserved region should be >= mappable_end. For now we 
> > > > > > > only
> > > > > > > enable for the selftests. Depends on CONFIG_DRM_I915_UNSTABLE 
> > > > > > > being
> > > > > > > enabled.
> > > > > > >
> > > > > > > eg. memmap=2G$16G i915.fake_lmem_start=0x4
> > > > > >
> > > > > > Hi Arek,
> > > > > >
> > > > > > Would you be able to update the fi-skl-lmem kernel cmd line with
> > > > > > s/i915_fake_lmem_start/i915.fake_lmem_start?
> > > > >
> > > > > done
> > > >
> > > > <6>[  497.897456] [drm] Intel graphics fake LMEM: [mem 
> > > > 0x1-0x13fff]
> > > > <6>[  497.897459] [drm] Intel graphics fake LMEM IO start: 4000
> > > > <6>[  497.897461] [drm] Intel graphics fake LMEM size: 4000
> > > >
> > > > All present.
> > > 
> > > Arek,
> > > 
> > > Could we enable DRM_I915_UNSTABLE_FAKE_LMEM in CI? That should give us
> > > some coverage of the fake local-memory setup on fi-skl-lmem.
> > 
> > Hey,
> > 
> >   config DRM_I915_UNSTABLE
> >   bool "Enable unstable API for early prototype development"
> >   depends on EXPERT
> >   depends on STAGING
> >   depends on BROKEN # should never be enabled by distros!
> > 
> >   config DRM_I915_UNSTABLE_FAKE_LMEM
> >   bool "Enable the experimental fake lmem"
> >   depends on DRM_I915_UNSTABLE
> > 
> > AFAIU because of that dependency on CONFIG_BROKEN we cannot just enable
> > it through .config - we have to edit the value in init/Kconfig[0].
> 
> Before the revert last night, CONFIG_BROKEN was already enabled in
> CI. It's now enabled again. The above output was from CI setting up lmem
> without extra hackery.
> -Chris

CI_DRM_7269 should have the DRM_I915_UNSTABLE_FAKE_LMEM enabled.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/lmem: add the fake lmem region

[Arkadiusz Hiler]

On Tue, Nov 05, 2019 at 01:05:20PM +, Matthew Auld wrote:
> On Thu, 31 Oct 2019 at 20:40, Chris Wilson  wrote:
> >
> > Quoting Arkadiusz Hiler (2019-10-31 12:40:35)
> > > On Wed, Oct 30, 2019 at 10:22:37PM +, Matthew Auld wrote:
> > > > On Tue, 29 Oct 2019 at 16:51, Matthew Auld  
> > > > wrote:
> > > > >
> > > > > Intended for upstream testing so that we can still exercise the LMEM
> > > > > plumbing and !i915_ggtt_has_aperture paths. Smoke tested on Skull 
> > > > > Canyon
> > > > > device. This works by allocating an intel_memory_region for a reserved
> > > > > portion of system memory, which we treat like LMEM. For the LMEMBAR we
> > > > > steal the aperture and 1:1 it map to the stolen region.
> > > > >
> > > > > To enable simply set the i915 modparam fake_lmem_start= on the kernel
> > > > > cmdline with the start of reserved region(see memmap=). The size of 
> > > > > the
> > > > > region we can use is determined by the size of the mappable aperture, 
> > > > > so
> > > > > the size of reserved region should be >= mappable_end. For now we only
> > > > > enable for the selftests. Depends on CONFIG_DRM_I915_UNSTABLE being
> > > > > enabled.
> > > > >
> > > > > eg. memmap=2G$16G i915.fake_lmem_start=0x4
> > > >
> > > > Hi Arek,
> > > >
> > > > Would you be able to update the fi-skl-lmem kernel cmd line with
> > > > s/i915_fake_lmem_start/i915.fake_lmem_start?
> > >
> > > done
> >
> > <6>[  497.897456] [drm] Intel graphics fake LMEM: [mem 
> > 0x1-0x13fff]
> > <6>[  497.897459] [drm] Intel graphics fake LMEM IO start: 4000
> > <6>[  497.897461] [drm] Intel graphics fake LMEM size: 4000
> >
> > All present.
> 
> Arek,
> 
> Could we enable DRM_I915_UNSTABLE_FAKE_LMEM in CI? That should give us
> some coverage of the fake local-memory setup on fi-skl-lmem.

Hey,

  config DRM_I915_UNSTABLE
  bool "Enable unstable API for early prototype development"
  depends on EXPERT
  depends on STAGING
  depends on BROKEN # should never be enabled by distros!

  config DRM_I915_UNSTABLE_FAKE_LMEM
  bool "Enable the experimental fake lmem"
  depends on DRM_I915_UNSTABLE

AFAIU because of that dependency on CONFIG_BROKEN we cannot just enable
it through .config - we have to edit the value in init/Kconfig[0].

Please push that change to core-for-CI (or other branch that gets
integrated into drm-tip) and then just send a merge request to the
kernel configs the CI is using[1].

[0]: https://lkml.org/lkml/2006/1/6/248
[1]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/tree/master/kconfig

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/lmem: add the fake lmem region

[Arkadiusz Hiler]

On Wed, Oct 30, 2019 at 10:22:37PM +, Matthew Auld wrote:
> On Tue, 29 Oct 2019 at 16:51, Matthew Auld  wrote:
> >
> > Intended for upstream testing so that we can still exercise the LMEM
> > plumbing and !i915_ggtt_has_aperture paths. Smoke tested on Skull Canyon
> > device. This works by allocating an intel_memory_region for a reserved
> > portion of system memory, which we treat like LMEM. For the LMEMBAR we
> > steal the aperture and 1:1 it map to the stolen region.
> >
> > To enable simply set the i915 modparam fake_lmem_start= on the kernel
> > cmdline with the start of reserved region(see memmap=). The size of the
> > region we can use is determined by the size of the mappable aperture, so
> > the size of reserved region should be >= mappable_end. For now we only
> > enable for the selftests. Depends on CONFIG_DRM_I915_UNSTABLE being
> > enabled.
> >
> > eg. memmap=2G$16G i915.fake_lmem_start=0x4
> 
> Hi Arek,
> 
> Would you be able to update the fi-skl-lmem kernel cmd line with
> s/i915_fake_lmem_start/i915.fake_lmem_start?

done
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] CI: Test revert some of the documentation fixes

[Arkadiusz Hiler]

This reverts commit 900554dc6bfc996ad07b9e187bbfd3864cd5bed0 to make
sure that Fi.CI.DOCS complains :-)
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h 
b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index 2a104c64291d..104cf6d42333 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -337,11 +337,6 @@ struct intel_shared_dpll {
 * @info: platform specific info
 */
const struct dpll_info *info;
-
-   /**
-* @wakeref: In some platforms a device-level runtime pm reference may
-* need to be grabbed to disable DC states while this DPLL is enabled
-*/
intel_wakeref_t wakeref;
 };
 
-- 
2.21.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)

[Arkadiusz Hiler]

On Tue, Oct 15, 2019 at 12:25:59PM +0300, Petri Latvala wrote:
> On Tue, Oct 15, 2019 at 09:41:20AM +0300, Arkadiusz Hiler wrote:
> > On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 09, 2019 at 09:12:23PM -, Patchwork wrote:
> > > > == Series Details ==
> > > > 
> > > > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB 
> > > > formats on SNB-BDW sprites (rev2)
> > > > URL   : https://patchwork.freedesktop.org/series/67741/
> > > > State : failure
> > > > 
> > > > == Summary ==
> > > > 
> > > > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > > > 
> > > > 
> > > > Summary
> > > > ---
> > > > 
> > > >   **FAILURE**
> > > > 
> > > >   Serious unknown changes coming with Patchwork_14725_full absolutely 
> > > > need to be
> > > >   verified manually.
> > > >   
> > > >   If you think the reported changes have nothing to do with the changes
> > > >   introduced in Patchwork_14725_full, please notify your bug team to 
> > > > allow them
> > > >   to document this new failure mode, which will reduce false positives 
> > > > in CI.
> > > > 
> > > >   
> > > > 
> > > > Possible new issues
> > > > ---
> > > > 
> > > >   Here are the unknown changes that may have been introduced in 
> > > > Patchwork_14725_full:
> > > > 
> > > > ### IGT changes ###
> > > > 
> > > >  Possible regressions 
> > > > 
> > > >   * igt@gem_eio@in-flight-1us:
> > > > - shard-snb:  [PASS][1] -> [FAIL][2]
> > > >[1]: 
> > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_...@in-flight-1us.html
> > > >[2]: 
> > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_...@in-flight-1us.html
> > > > 
> > > >   * igt@kms_plane@pixel-format-pipe-a-planes:
> > > > - shard-iclb: [PASS][3] -> [FAIL][4] +13 similar issues
> > > >[3]: 
> > > > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_pl...@pixel-format-pipe-a-planes.html
> > > >[4]: 
> > > > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_pl...@pixel-format-pipe-a-planes.html
> > > 
> > > IGT-Version: 1.24-ge501741f
> > > ...
> > > Testing format AR30(0x30335241) / modifier 0x103 on A.0
> > > (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 
> > > 0x30335241 to 0x78464749)
> > > 
> > > DRM_FORMAT_ARGB2101010 =  0x30335241
> > > IGT_FORMAT_FLOAT = 0x78464749
> > > 
> > > { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
> > >   .pixman_id = PIXMAN_a2r10g10b10,
> > > 
> > > { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
> > >   .pixman_id = PIXMAN_rgba_float,
> > > 
> > > if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> > > (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
> > >   cnvert_pixman(cvt);
> > >   return;
> > > ...
> > > igt_assert_f(false, "Conversion not implemented ...);
> > > 
> > > So wtf?
> > > 
> > > Are we somehow compiling igt with an old pixman causing
> > >  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
> > >  #define PIXMAN_rgba_float PIXMAN_invalid
> > >  #endif
> > > to happen?
> > 
> > oof, seems like the building machine got downgraded somehow
> > 
> > ci-worker1:~$ dpkg -l '*pixman*'
> > Desired=Unknown/Install/Remove/Purge/Hold
> > | 
> > Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
> > |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
> > ||/ NameVersion 
> >  Architecture Description
> > +++-===---===
> > ii  l

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)

[Arkadiusz Hiler]

On Mon, Oct 14, 2019 at 10:23:42PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 09, 2019 at 09:12:23PM -, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats 
> > on SNB-BDW sprites (rev2)
> > URL   : https://patchwork.freedesktop.org/series/67741/
> > State : failure
> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_7042_full -> Patchwork_14725_full
> > 
> > 
> > Summary
> > ---
> > 
> >   **FAILURE**
> > 
> >   Serious unknown changes coming with Patchwork_14725_full absolutely need 
> > to be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_14725_full, please notify your bug team to allow 
> > them
> >   to document this new failure mode, which will reduce false positives in 
> > CI.
> > 
> >   
> > 
> > Possible new issues
> > ---
> > 
> >   Here are the unknown changes that may have been introduced in 
> > Patchwork_14725_full:
> > 
> > ### IGT changes ###
> > 
> >  Possible regressions 
> > 
> >   * igt@gem_eio@in-flight-1us:
> > - shard-snb:  [PASS][1] -> [FAIL][2]
> >[1]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-snb7/igt@gem_...@in-flight-1us.html
> >[2]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-snb7/igt@gem_...@in-flight-1us.html
> > 
> >   * igt@kms_plane@pixel-format-pipe-a-planes:
> > - shard-iclb: [PASS][3] -> [FAIL][4] +13 similar issues
> >[3]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7042/shard-iclb7/igt@kms_pl...@pixel-format-pipe-a-planes.html
> >[4]: 
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14725/shard-iclb8/igt@kms_pl...@pixel-format-pipe-a-planes.html
> 
> IGT-Version: 1.24-ge501741f
> ...
> Testing format AR30(0x30335241) / modifier 0x103 on A.0
> (kms_plane:1411) igt_fb-CRITICAL: Conversion not implemented (from format 
> 0x30335241 to 0x78464749)
> 
> DRM_FORMAT_ARGB2101010 =  0x30335241
> IGT_FORMAT_FLOAT = 0x78464749
> 
> { .name = "ARGB2101010", .depth = 30, .drm_id = DRM_FORMAT_ARGB2101010,
>   .pixman_id = PIXMAN_a2r10g10b10,
> 
> { .name = "IGT-FLOAT", .depth = -1, .drm_id = IGT_FORMAT_FLOAT,
>   .pixman_id = PIXMAN_rgba_float,
> 
> if ((drm_format_to_pixman(cvt->src.fb->drm_format) != PIXMAN_invalid) &&
> (drm_format_to_pixman(cvt->dst.fb->drm_format) != PIXMAN_invalid)) {
>   cnvert_pixman(cvt);
>   return;
> ...
> igt_assert_f(false, "Conversion not implemented ...);
> 
> So wtf?
> 
> Are we somehow compiling igt with an old pixman causing
>  #if PIXMAN_VERSION < PIXMAN_VERSION_ENCODE(0, 36, 0)
>  #define PIXMAN_rgba_float PIXMAN_invalid
>  #endif
> to happen?

oof, seems like the building machine got downgraded somehow

ci-worker1:~$ dpkg -l '*pixman*'
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ NameVersion 
 Architecture Description
+++-===---===
ii  libpixman-1-0:amd64 0.34.0-2
 amd64pixel-manipulation library for X and cairo
ii  libpixman-1-dev:amd64   0.34.0-2
 amd64pixel-manipulation library for X and cairo 
(development files)

that's bad...

> But the reference run shows it testing all the fancy YUV formats so
> I don't think that can be the case.

That's the weird bit...

Anyway the building machine needs updating and apt-mark hold.
This can cause fallout and we need to file bugs to limit the noise.

There is quite some queue right now, but hopefully by tomorrow it will
be drained. I'll do the necessary updates and force IGT run to see what
is going to happen in the morning. Then I'll rerun this series.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/1] drm/i915: Get the correct wakeref for reading HOTPLUG_EN et al.

[Arkadiusz Hiler]

Without it we get:
 Unclaimed read from register 0x1e1110
 WARNING: CPU: 2 PID: 1029 at drivers/gpu/drm/i915/intel_uncore.c:1101 
__unclaimed_reg_debug+0x40/0x50 [i915]
 Call Trace:
  fwtable_read32+0x233/0x300 [i915]
  i915_interrupt_info+0xa73/0xd60 [i915]
  seq_read+0xdb/0x3c0
  full_proxy_read+0x51/0x80
  vfs_read+0x9e/0x160
  ksys_read+0x8f/0xe0
  do_syscall_64+0x55/0x1c0
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Cc: Chris Wilson 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109824
Signed-off-by: Arkadiusz Hiler 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e5835337f022..29f3436167a2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -534,6 +534,7 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
 
gen8_display_interrupt_info(m);
} else if (IS_VALLEYVIEW(dev_priv)) {
+   intel_wakeref_t pref;
seq_printf(m, "Display IER:\t%08x\n",
   I915_READ(VLV_IER));
seq_printf(m, "Display IIR:\t%08x\n",
@@ -544,7 +545,6 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
   I915_READ(VLV_IMR));
for_each_pipe(dev_priv, pipe) {
enum intel_display_power_domain power_domain;
-   intel_wakeref_t pref;
 
power_domain = POWER_DOMAIN_PIPE(pipe);
pref = intel_display_power_get_if_enabled(dev_priv,
@@ -578,12 +578,14 @@ static int i915_interrupt_info(struct seq_file *m, void 
*data)
seq_printf(m, "PM IMR:\t\t%08x\n",
   I915_READ(GEN6_PMIMR));
 
+   pref = intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
seq_printf(m, "Port hotplug:\t%08x\n",
   I915_READ(PORT_HOTPLUG_EN));
seq_printf(m, "DPFLIPSTAT:\t%08x\n",
   I915_READ(VLV_DPFLIPSTAT));
seq_printf(m, "DPINVGTT:\t%08x\n",
   I915_READ(DPINVGTT));
+   intel_display_power_put(dev_priv, POWER_DOMAIN_INIT, pref);
 
} else if (!HAS_PCH_SPLIT(dev_priv)) {
seq_printf(m, "Interrupt enable:%08x\n",
-- 
2.21.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 0/1] Fix i915_interrupt_info debugfs with display off on VLV

[Arkadiusz Hiler]

Cover letter to use https://intel-gfx-ci.01.org/test-with.html

https://patchwork.freedesktop.org/patch/330337/?series=66602

Test-with: 20190912123320.13131-1-arkadiusz.hi...@intel.com

Arkadiusz Hiler (1):
  drm/i915: Get the correct wakeref for reading HOTPLUG_EN et al.

 drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.21.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_mmap_gtt: Race mmap offset generation against closure

[Arkadiusz Hiler]

On Mon, Aug 26, 2019 at 04:20:00PM +0100, Chris Wilson wrote:
> Signed-off-by: Chris Wilson 
> Cc: Abdiel Janulgue 
> ---
>  tests/i915/gem_mmap_gtt.c | 98 +++
>  1 file changed, 98 insertions(+)
> 
> diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
> index 8eff91850..81068f7d1 100644
> --- a/tests/i915/gem_mmap_gtt.c
> +++ b/tests/i915/gem_mmap_gtt.c
> @@ -26,6 +26,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -360,6 +361,99 @@ test_isolation(int i915)
>   igt_assert(ptr == MAP_FAILED);
>  }
>  
> +static void
> +test_close_race(int i915)
> +{
> + const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> + uint32_t *handles;
> +
> + handles = mmap64(0, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
> + igt_assert(handles != MAP_FAILED);
> +
> + igt_fork(child, ncpus) {
> + do {
> + struct drm_i915_gem_mmap_gtt mmap_arg = {};
> + int i = 1 + random() % ncpus;
> + uint32_t old;
> +
> + mmap_arg.handle = gem_create(i915, 4096);
> + old = atomic_exchange([i], mmap_arg.handle);

../tests/i915/gem_mmap_gtt.c:380:10: error: address argument to atomic 
operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int 
*') invalid)
old = atomic_exchange([i], mmap_arg.handle);
  ^   ~~~
/usr/lib64/clang/8.0.0/include/stdatomic.h:137:42: note: expanded from macro 
'atomic_exchange'
#define atomic_exchange(object, desired) __c11_atomic_exchange(object, desired, 
__ATOMIC_SEQ_CST)
 ^ ~~
../tests/i915/gem_mmap_gtt.c:423:10: error: address argument to atomic 
operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int 
*') invalid)
old = atomic_exchange([i],
  ^   ~~~
/usr/lib64/clang/8.0.0/include/stdatomic.h:137:42: note: expanded from macro 
'atomic_exchange'
#define atomic_exchange(object, desired) __c11_atomic_exchange(object, desired, 
__ATOMIC_SEQ_CST)
 ^ ~~
2 errors generated.

https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/535592

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/1] drm/i915/uc: Turn on GuC/HuC auto mode again

[Arkadiusz Hiler]

On Thu, Aug 22, 2019 at 09:31:07AM +0200, Michal Wajdeczko wrote:
> On Thu, 22 Aug 2019 08:40:33 +0200, Arkadiusz Hiler
>  wrote:
> 
> > On Mon, Aug 19, 2019 at 11:09:15AM +0300, Martin Peres wrote:
> > > On 18/08/2019 18:51, Michal Wajdeczko wrote:
> > > > We hope that now all issues related to missing uC firmwares
> > > > are fixed and that driver can continue to load without GuC
> > > > or HuC firmware available and running:
> > > >
> > > >  - missing or corrupted HuC firmware has no impact to driver
> > > >load (clients should continue to use HUC_STATUS to check
> > > >if HuC firmware was loaded and authenticated)
> > > >
> > > >  - missing or corrupted GuC firmware has no impact to driver
> > > >load (unless GuC firmware blob was overridden by the user
> > > >or GuC submission was requested or GuC was previously
> > > >enabled on this system without reboot - then we will mark
> > > >GPU as wedged and continue with KMS only)
> > > 
> > > Please hold merging this patch, as many more items need to be crossed
> > > off before such a patch can land.
> > > 
> > > Such items include:
> > > 
> > >  - Assess all the existing GUC-related bugs, and prove they won't
> > > suddenly get seen by more users
> > >  - add fault injection to the FW loading path
> > >  - add IGT tests to make sure driver behaves well on different FW
> > > loading errors
> > 
> > If this is going to get enabled by default we should add some tests to
> > verify that HuC is indeed loaded and operational. Otherwise this may
> > degrade without anyone noticing.
> 
> we were discussing such test some time ago [1], but we couldn't get
> into final agreement.
> 
> [1] https://patchwork.freedesktop.org/series/60800/

Oh, that's a good start. It would be good to land this along the default
enabling of HuC and have the agreement on the "best error codes" by then.

> > Something along those lines:
> > int huc_status = getparam(I915_PARAM_HUC_STATUS);
> > 
> > assert(MI_PREDICATE(HUC_STATUS) == !!huc_status);
> > skip_on_f(huc_status == 0, "HuC disabled\n");
> > 
> > assert_f(huc_status == 1, "HuC status is not enabled: %d\n",
> > huc_status);
> > assert(submit_commands_to_check_that_huc_is_operational());
> > 
> > 
> > 
> > The issue is that the PARAM_HUC_STATUS won't even work right now because:
> > 
> > case I915_PARAM_HUC_STATUS:
> > value = intel_huc_check_status(>gt.uc.huc);
> > if (value < 0)
> > return value;
> > break;
> > /* ... */
> > return 0;
> 
> Please note that this return if for ioctl() call status
> 
> > 
> > 
> > /**
> >  * intel_huc_check_status() - check HuC status
> >  * @huc: intel_huc structure
> >  *
> >  * This function reads status register to verify if HuC
> >  * firmware was successfully loaded.
> >  *
> >  * Returns: 1 if HuC firmware is loaded and verified,
> >  * 0 if HuC firmware is not loaded and -ENODEV if HuC
> >  * is not present on this platform.
> >  */
> > 
> > This is broken - we will get 0 whether it's enabled or disabled.
> 
> I don't think so. Negative values returned by this function are simply
> used as ioctl() errors, while 0/1 values are returned as 'value' field
> that holds reply with actual HuC status:
> 
>   if (put_user(value, param->value))
>   return -EFAULT;
> 
> More coffee?

My bad. coffee++ would have helped
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 5/5] tests/kms_selftest: Integrate kernel selftest test-drm_modeset

[Arkadiusz Hiler]

On Wed, Jul 03, 2019 at 12:37:46PM +0200, Daniel Vetter wrote:
> On Tue, Jun 25, 2019 at 05:37:01PM +0200, Daniel Vetter wrote:
> > On Tue, Jun 25, 2019 at 09:01:32AM +0300, Arkadiusz Hiler wrote:
> > > On Thu, Jun 20, 2019 at 03:20:02PM +0200, Daniel Vetter wrote:
> > > > On Tue, Oct 16, 2018 at 03:23:41PM -0700, Deepak Rawat wrote:
> > > > > Call kernel selftest module test-drm_modeset for testing KMS.
> > > > >
> > > > > v2:
> > > > > - Add test alphabetically.
> > > > > - Add test to meson build.
> > > > >
> > > > > v3: Rename to kms_selftest.
> > > > >
> > > > > Signed-off-by: Deepak Rawat 
> > > > 
> > > > Just realized that this never landed ... any reasons? Would be nice to 
> > > > get
> > > > this handled.
> > > > 
> > > > Petri/Arek?
> > > > 
> > > > Cheers, Daniel
> > > 
> > > What do you mean by "this never landed"?
> > > 
> > > https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/7766b1e2348b
> > > 
> > > commit 7766b1e2348b32cc8ed58a972c6fd53b20279549
> > > Author: Deepak Rawat 
> > > AuthorDate: Tue Oct 16 15:23:41 2018 -0700
> > > Commit: Daniel Vetter 
> > > CommitDate: Wed Oct 17 09:41:07 2018 +0200
> > > 
> > > tests/kms_selftest: Integrate kernel selftest test-drm_modeset
> > 
> > Hm not sure what I looked at, but it wasnt there. Sorry for the noise.
> 
> Ok this one here landed, but the other 4 didn't ... Can we unblock them
> somehow? Or any hold-up? Would be nice to have igts for at least the more
> recently added uapi, especially when the tests exist ...
> -Daniel

I guess you are talking about https://patchwork.freedesktop.org/series/51087/

Seems like patches 1, 2, 3 and 5 are merged (see below).

The only patch that was not merged is "Some simple test cases to use
FB_DAMAGE_CLIPS plane property.":
https://patchwork.freedesktop.org/patch/257081/?series=51087=1

Sorry but I jut feel lost here. Can you provide more context?
So what exactely haven't landed? What is still missing?


Looking through the archives it's impossible to tell what exactely
happened there. I have no idea why the 4th patch was left out and why
the other patches lack rb or ack and just have a second s-o-b.


commit 7766b1e2348b32cc8ed58a972c6fd53b20279549
Author: Deepak Rawat 
AuthorDate: Tue Oct 16 15:23:41 2018 -0700
Commit: Daniel Vetter 
CommitDate: Wed Oct 17 09:41:07 2018 +0200

tests/kms_selftest: Integrate kernel selftest test-drm_modeset

Call kernel selftest module test-drm_modeset for testing KMS.

v2:
- Add test alphabetically.
- Add test to meson build.

v3: Rename to kms_selftest.

Signed-off-by: Deepak Rawat 
Signed-off-by: Daniel Vetter 

commit 759af708db65d8f9fc2218e3445cfb903c8be72a
Author: Deepak Rawat 
AuthorDate: Tue Oct 16 15:23:39 2018 -0700
Commit: Daniel Vetter 
CommitDate: Wed Oct 17 09:41:00 2018 +0200

lib: Don't call igt_require_fb_modifiers() when no modifier

vmwgfx and amdgpu doesn't support fb modifiers, yet kms_addfb() requires
modifier support. Since many tests don't need this to run, the
requirement can be made less broad.

Therefore, tighten the requirement to cases where modifiers are actually
needed.

v2:
* In create_fb() calls to kms_addfb(), remove the modifier flag iff the
  driver doesn't support modifiers and the modifer is 0
* Don't modify the flag in kms_addfb().

Signed-off-by: Deepak Rawat 
Signed-off-by: Leo Li 
Signed-off-by: Daniel Vetter 

commit c7034c780629b6f678dfe5021c38bc820a34e19d
Author: Deepak Rawat 
AuthorDate: Tue Oct 16 15:23:38 2018 -0700
Commit: Daniel Vetter 
CommitDate: Wed Oct 17 09:40:55 2018 +0200

lib/igt_fb: Check for cairo surface success

For vmwgfx cairo surface creation fails due to stride mismatch, add a
igt_require_f() for surface.

v2: Check for surface creation failure.

Signed-off-by: Deepak Rawat 
Signed-off-by: Daniel Vetter 

commit 4ca3d1de874bd269b37055f1a4cc6de04ea2d050
Author: Deepak Rawat 
AuthorDate: Tue Oct 16 15:23:37 2018 -0700
Commit: Daniel Vetter 
CommitDate: Wed Oct 17 09:40:51 2018 +0200

lib/igt_fb: Call dumb_destroy ioctl in case of dumb buffers

vmwgfx does not support GEM interface so calling gem_close on vmwgfx
results in error.

v2: Use drmIoctl with error when ioctl() failed.

v3: Seperate ioctl wrapper.

Signed-off-by: Deepak Rawat 
Signed-off-by: Daniel Vetter 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] gitlab-ci: add tests for MIPS

[Arkadiusz Hiler]

On Thu, Jun 27, 2019 at 05:51:32PM +0300, Ser, Simon wrote:
> On Thu, 2019-06-27 at 14:30 +0100, Guillaume Tucker wrote:
> > Use the libatomic1:mips package only in the Debian Stretch Docker
> > image for MIPS and add Gitlab CI step to run tests on MIPS.
> > 
> > Signed-off-by: Guillaume Tucker 
> 
> With this tag added:
> 
> Fixes: 439a9f5d615f ("gitlab-ci: add build for MIPS")
> 
> This patch is:
> 
> Reviewed-by: Simon Ser 

Hey,

https://patchwork.freedesktop.org/series/62859/ and check GitLab.Pipeline

We will be running gitlab pipeline for everything pre-merge and if it
fails we will send out a email (currently we are testing this and no
emails are sent).

You can check the pieline status from patchwork at all times, even for
sucessful ones :-)

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/gem_pread/pwrite: Rename "basic"

[Arkadiusz Hiler]

On Thu, Jun 27, 2019 at 08:18:36AM +, Ser, Simon wrote:
> On Thu, 2019-06-27 at 08:36 +0100, Chris Wilson wrote:
> > The "basic" subtests perform no verification that the read/write work,
> > only function as mere API exercisers and loose benchmarks. Rename them
> > to reflect that they are poor benchmarks instead.
> > 
> > Signed-off-by: Chris Wilson 
> 
> Reviewed-by: Simon Ser 

you forgot to add r-b while pushing

Tests are now renamed in the cibuglog, so all the existing filters (1)
will apply. It's nice to CC someone handling cibuglog when renaming,
otherwise we will end up with more noise and spend time on creating and
deduplicating bugs later on.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v3 1/1] gitlab-ci: add build and tests for MIPS

[Arkadiusz Hiler]

On Thu, Jun 27, 2019 at 04:14:53PM +0300, Ser, Simon wrote:
> On Thu, 2019-06-27 at 11:02 +0100, Guillaume Tucker wrote:
> > On 27/06/2019 08:02, Ser, Simon wrote:
> > > On Tue, 2019-06-25 at 14:08 +0100, Guillaume Tucker wrote:
> > > > On 18/06/2019 13:42, Guillaume Tucker wrote:
> > > > > Add Docker image and Gitlab CI steps to run builds and tests for
> > > > > the
> > > > > MIPS architecture using Debian Stretch with backports.
> > > > > 
> > > > > Signed-off-by: Guillaume Tucker 
> > > > > ---
> > > > > 
> > > > > Notes:
> > > > > v2: use stretch-backports and require libatomic1
> > > > > v3: add mips ci tests and require Debian libatomic1 for mips
> > > > 
> > > > The series to use portable atomics functions was merged today, so
> > > > I think this one should now be good to go as well.  It applies
> > > > cleanly on top of the current master branch and the Gitlab CI
> > > > pipeline passed:
> > > > 
> > > >   
> > > > https://gitlab.freedesktop.org/gtucker/igt-gpu-tools/pipelines/44704
> > > > 
> > > > Please let me know if you want me to resubmit it to get another
> > > > Patchwork CI run or if anything else needs to be done.
> > > 
> > > LGTM!
> > > 
> > > Reviewed-by: Simon Ser 
> > > 
> > > And pushed:
> > > 
> > > To gitlab.freedesktop.org:drm/igt-gpu-tools.git
> > >15ad66453441..439a9f5d615f  master -> master
> > 
> > Thanks!
> > 
> > Err, however it looks like you pushed the v2 which had only
> > builds rather than this v3 which does builds and tests:
> > 
> >   439a9f5d615f gitlab-ci: add build for MIPS
> > 
> > I've made another patch with the difference between v2 and v3 and
> > pushed it to my branch:
> > 
> >   
> > https://gitlab.freedesktop.org/gtucker/igt-gpu-tools/commit/9693e28871f27efb7340ad29d54de4be7b5461a9
> > 
> > I'll wait for the Gitlab CI pipeline to complete and then I guess
> > I should send that to the mailing list.
> 
> Bleh, I'm sorry about this! It seems like patchwork got confused.
> 
> I'll gladly review and merge a fix, feel free to Cc me :)

The title of the first patch has changed, so patchwork treats it as a
separate series instead of a revision to existing one.

It's safer to take the patchwork links (both to series and the mbox)
from the CI results instead of trying to browse for them yourself.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 5/5] tests/kms_selftest: Integrate kernel selftest test-drm_modeset

[Arkadiusz Hiler]

On Thu, Jun 20, 2019 at 03:20:02PM +0200, Daniel Vetter wrote:
> On Tue, Oct 16, 2018 at 03:23:41PM -0700, Deepak Rawat wrote:
> > Call kernel selftest module test-drm_modeset for testing KMS.
> >
> > v2:
> > - Add test alphabetically.
> > - Add test to meson build.
> >
> > v3: Rename to kms_selftest.
> >
> > Signed-off-by: Deepak Rawat 
> 
> Just realized that this never landed ... any reasons? Would be nice to get
> this handled.
> 
> Petri/Arek?
> 
> Cheers, Daniel

What do you mean by "this never landed"?

https://gitlab.freedesktop.org/drm/igt-gpu-tools/commit/7766b1e2348b

commit 7766b1e2348b32cc8ed58a972c6fd53b20279549
Author: Deepak Rawat 
AuthorDate: Tue Oct 16 15:23:41 2018 -0700
Commit: Daniel Vetter 
CommitDate: Wed Oct 17 09:41:07 2018 +0200

tests/kms_selftest: Integrate kernel selftest test-drm_modeset

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t v2] gitlab-ci: add build for MIPS

[Arkadiusz Hiler]

On Thu, Jun 13, 2019 at 03:01:06PM +0100, Guillaume Tucker wrote:
> Add Docker image and Gitlab CI steps to run builds for the MIPS
> architecture using Debian Stretch with backports.
> 
> Signed-off-by: Guillaume Tucker 
> ---
>  .gitlab-ci.yml | 28 
>  Dockerfile.debian-mips | 39 +++
>  meson-cross-mips.txt   | 12 
>  3 files changed, 79 insertions(+)
>  create mode 100644 Dockerfile.debian-mips
>  create mode 100644 meson-cross-mips.txt
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 771143a9ea95..e390f8f472d5 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -90,6 +90,17 @@ build:tests-debian-meson-arm64:
>  paths:
>- build
>  
> +build:tests-debian-meson-mips:
> +  image: $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-mips:latest
> +  stage: build
> +  script:
> +- export PKG_CONFIG_PATH=/usr/lib/mips-linux-gnu/pkgconfig/
> +- meson --cross-file meson-cross-mips.txt build
> +- ninja -C build
> +  artifacts:
> +paths:
> +  - build
> +
>  build:tests-debian-autotools:
>image: $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian:latest
>stage: build
> @@ -221,6 +232,23 @@ containers:igt-debian-arm64:
>  - docker build -t $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-arm64 -f 
> Dockerfile.debian-arm64 .
>  - docker push $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-arm64

Any particular reason for not having ninja-test step for MIPS?

Other than that (and Petri's concern, since I don't speak Debian),
looks good.

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] i915/gem_exec_balancer: Fix typo in memcpy

[Arkadiusz Hiler]

On Thu, Jun 06, 2019 at 06:22:49PM +0100, Chris Wilson wrote:
> Fixes: c26e76418f49 ("tests/gem_exec_balancer: Manually calculate VLA struct 
> sizes")
> Signed-off-by: Chris Wilson 
> Cc: Arkadiusz Hiler 
Reviewed-by: Arkadiusz Hiler 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] gitlab-ci: add build for MIPS

[Arkadiusz Hiler]

On Wed, Jun 05, 2019 at 09:18:09PM +0100, Guillaume Tucker wrote:
> Add Docker image and Gitlab CI steps to run builds for the MIPS
> architecture using Debian Buster.
> 
> Signed-off-by: Guillaume Tucker 
> ---
>  .gitlab-ci.yml | 28 
>  Dockerfile.debian-mips | 38 ++
>  meson-cross-mips.txt   | 12 
>  3 files changed, 78 insertions(+)
>  create mode 100644 Dockerfile.debian-mips
>  create mode 100644 meson-cross-mips.txt
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 771143a9ea95..e390f8f472d5 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -90,6 +90,17 @@ build:tests-debian-meson-arm64:
>  paths:
>- build
>  
> +build:tests-debian-meson-mips:
> +  image: $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-mips:latest
> +  stage: build
> +  script:
> +- export PKG_CONFIG_PATH=/usr/lib/mips-linux-gnu/pkgconfig/
> +- meson --cross-file meson-cross-mips.txt build
> +- ninja -C build
> +  artifacts:
> +paths:
> +  - build
> +
>  build:tests-debian-autotools:
>image: $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian:latest
>stage: build
> @@ -221,6 +232,23 @@ containers:igt-debian-arm64:
>  - docker build -t $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-arm64 -f 
> Dockerfile.debian-arm64 .
>  - docker push $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-arm64
>  
> +containers:igt-debian-mips:
> +  stage: containers
> +  image: docker:stable
> +  only:
> +changes:
> +  - Dockerfile.debian-mips
> +  - .gitlab-ci.yml
> +  services:
> +- docker:dind
> +  variables:
> +DOCKER_HOST: tcp://docker:2375
> +DOCKER_DRIVER: overlay2
> +  script:
> +- docker login -u gitlab-ci-token -p $CI_JOB_TOKEN $CI_REGISTRY
> +- docker build -t $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-mips -f 
> Dockerfile.debian-mips .
> +- docker push $CI_REGISTRY/$CI_PROJECT_PATH/igt-debian-mips
> +
>  containers:igt-fedora:
>stage: containers
>image: docker:stable
> diff --git a/Dockerfile.debian-mips b/Dockerfile.debian-mips
> new file mode 100644
> index ..2612b7b148e3
> --- /dev/null
> +++ b/Dockerfile.debian-mips
> @@ -0,0 +1,38 @@
> +FROM debian:buster

Any particular reason you went here for buster instead of
stretch-backports like with other images? I am not very fluent in
Debian.

Other than that looks good to land after the atomic compatibility fixes.

> +
> +RUN apt-get update
> +RUN apt-get install -y \
> + flex \
> + bison \
> + pkg-config \
> + x11proto-dri2-dev \
> + python-docutils \
> + valgrind \
> + peg
> +
> +RUN dpkg --add-architecture mips
> +RUN apt-get update
> +RUN apt-get install -y \
> + gcc-mips-linux-gnu \
> + libpciaccess-dev:mips \
> + libkmod-dev:mips \
> + libprocps-dev:mips \
> + libunwind-dev:mips \
> + libdw-dev:mips \
> + zlib1g-dev:mips \
> + liblzma-dev:mips \
> + libcairo-dev:mips \
> + libpixman-1-dev:mips \
> + libudev-dev:mips \
> + libgsl-dev:mips \
> + libasound2-dev:mips \
> + libjson-c-dev:mips \
> + libcurl4-openssl-dev:mips \
> + libxrandr-dev:mips \
> + libxv-dev:mips
> +
> +RUN apt-get install -y \
> + meson \
> + libdrm-dev:mips \
> + qemu-user \
> + qemu-user-static
> diff --git a/meson-cross-mips.txt b/meson-cross-mips.txt
> new file mode 100644
> index ..6350d677e0bc
> --- /dev/null
> +++ b/meson-cross-mips.txt
> @@ -0,0 +1,12 @@
> +[binaries]
> +c = '/usr/bin/mips-linux-gnu-gcc'
> +ar = '/usr/bin/mips-linux-gnu-gcc-ar'
> +strip = '/usr/bin/mips-linux-gnu-strip'
> +pkgconfig = 'pkg-config'
> +exe_wrapper = 'qemu-mips'
> +
> +[host_machine]
> +system = 'linux'
> +cpu_family = 'mips'
> +cpu = 'mips'
> +endian = 'big'
> -- 
> 2.20.1
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/4] gitlab-ci: add libatomic to Fedora docker image

[Arkadiusz Hiler]

On Mon, Jun 03, 2019 at 12:54:48PM +0100, Guillaume Tucker wrote:
> Add libatomic to the Fedora docker image so it can link binaries that
> use __atomic_* functions.
> 
> Signed-off-by: Guillaume Tucker 
> ---
>  Dockerfile.fedora | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Dockerfile.fedora b/Dockerfile.fedora
> index 6686e587613d..c84b412b0723 100644
> --- a/Dockerfile.fedora
> +++ b/Dockerfile.fedora
> @@ -1,7 +1,7 @@
>  FROM fedora:30
>  
>  RUN dnf install -y \
> - gcc flex bison meson ninja-build xdotool \
> + gcc flex bison libatomic meson ninja-build xdotool \
>   'pkgconfig(libdrm)' \
>   'pkgconfig(pciaccess)' \
>   'pkgconfig(libkmod)' \

Reviewed-by: Arkadiusz Hiler 

I wonder how does the libatomic gets installed implicitly in Debian.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/4] tests: add libatomic dependency

[Arkadiusz Hiler]

On Mon, Jun 03, 2019 at 12:54:47PM +0100, Guillaume Tucker wrote:
> Add dependency to libatomic in order to be able to use the __atomic_*
> functions instead of the older __sync_* ones.  This is to enable
> atomic operations on a wider number of architectures including MIPS.
> 
> Signed-off-by: Guillaume Tucker 

Reviewed-by: Arkadiusz Hiler 

This points out that we need some cleanups in tests/meson.build, as it's
getting a bit messy with some of the test using igt_desp and others
test_deps, but that's out of the sope of this series.

Seems like for autotools we already have it:

% ag -G Makefile atomic
tests/Makefile.am
93:gem_create_LDADD = $(LDADD) -lpthread -latomic
125:sw_sync_LDADD = $(LDADD) -latomic
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev6)

[Arkadiusz Hiler]

On Thu, Apr 18, 2019 at 10:31:32AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: adding state checker for gamma lut values (rev6)
> URL   : https://patchwork.freedesktop.org/series/58039/
> State : failure


Hey,

This series is a rerun, which means that someone went to patchwork web
interface and clicked on "test this series again". This should be used
only when you thing there is something seriously wrong with CI results.

If you have something that looks like a false positive (e.g. a failure
in tests that your change should not affect see below).

> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5952 -> Patchwork_12827
> 
> 
> Summary
> ---
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12827 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12827, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: 
> https://patchwork.freedesktop.org/api/1.0/series/58039/revisions/6/mbox/

This is the relevant part about false positives. Just provide some
explanation why you think this is a false positive and CC:
Martin  and/or
Lakshmi 

> Possible new issues
> ---
> 
>   Here are the unknown changes that may have been introduced in 
> Patchwork_12827:
> 
> ### IGT changes ###
> 
>  Possible regressions 
> 
>   * igt@runner@aborted:
> - fi-ilk-650: NOTRUN -> FAIL
> - fi-pnv-d510:NOTRUN -> FAIL
> - fi-bdw-gvtdvm:  NOTRUN -> FAIL
> - fi-hsw-peppy:   NOTRUN -> FAIL
> - fi-gdg-551: NOTRUN -> FAIL
> - fi-snb-2520m:   NOTRUN -> FAIL
> - fi-hsw-4770:NOTRUN -> FAIL
> - fi-bxt-j4205:   NOTRUN -> FAIL
> - fi-whl-u:   NOTRUN -> FAIL
> - fi-icl-u3:  NOTRUN -> FAIL
> - fi-ivb-3770:NOTRUN -> FAIL
> - fi-bxt-dsi: NOTRUN -> FAIL
> - fi-byt-j1900:   NOTRUN -> FAIL
> - fi-icl-y:   NOTRUN -> FAIL
> - fi-blb-e6850:   NOTRUN -> FAIL
> - fi-bsw-kefka:   NOTRUN -> FAIL
> - fi-hsw-4770r:   NOTRUN -> FAIL
> - fi-byt-clapper: NOTRUN -> FAIL
> - fi-bdw-5557u:   NOTRUN -> FAIL
> - fi-bwr-2160:NOTRUN -> FAIL
> - fi-elk-e7500:   NOTRUN -> FAIL

This looks like a real issue though. Seems like igt_runner has aborted
execution on almost all the machines due to hitting a WARN.

Let's see the logs:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12822/fi-cfl-8700k/igt@run...@aborted.html
--
Aborting.
Previous test: nothing
Next test: core_auth (basic-auth)

Kernel badly tainted (0x200) (check dmesg for details):
(0x200) TAINT_WARN: WARN_ON has happened.
--

(You can get them through patchwork -> see full logs -> red cell in the
igt@runner@aborted row, on a machcine that has not aborted prior to that.)

This tells us that the kernel was tainted in a way that may cause
unexpeted behavior if testing would coninued (TAINT_WARN), so we abort.

Here it happned before we even run a single tests ("Previous test:
nothing"), so we have to look for the result in the boot dmesg. Link to
boot log is on top of the page with the result (boot0).

https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12822/fi-cfl-8700k/boot0.log
--
<4>[3.842910] [ cut here ]
<4>[3.842911] pipe state doesn't match!
<4>[3.842952] WARNING: CPU: 3 PID: 224 at 
drivers/gpu/drm/i915/intel_display.c:12700 
intel_atomic_commit_tail+0x127d/0x12e0 [i915]
<4>[3.842953] Modules linked in: i915 x86_pkg_temp_thermal mei_hdcp 
coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec snd_hwdep snd_hda_core 
crc32_pclmul e1000e snd_pcm mei_me ghash_clmulni_intel mei ptp pps_core 
prime_numbers
<4>[3.842961] CPU: 3 PID: 224 Comm: kworker/u24:7 Not tainted 
5.1.0-rc5-CI-Patchwork_12822+ #1
<4>[3.842962] Hardware name: Micro-Star International Co., Ltd. 
MS-7B54/Z370M MORTAR (MS-7B54), BIOS 1.00 10/31/2017
<4>[3.842964] Workqueue: events_unbound async_run_entry_fn
<4>[3.842993] RIP: 0010:intel_atomic_commit_tail+0x127d/0x12e0 [i915]
<4>[3.842995] Code: 45 0f b6 84 24 d2 07 00 00 e8 3f 36 3d e1 5e 5f e9 80 
fa ff ff e8 d3 8d e2 e0 0f 0b 0f b6 0c 24 e9 42 fc ff ff e8 c3 8d e2 e0 <0f> 0b 
e9 d2 f3 ff ff e8 b7 8d e2 e0 0f 0b 49 8b 44 24 50 e9 ae fc
<4>[3.842996] RSP: 0018:c90001aa7998 EFLAGS: 00010282
<4>[3.842997] RAX:  RBX: 888253eb92a8 RCX: 

<4>[3.842998] RDX: 0007 RSI: 88826274d9f8 RDI: 

<4>[3.842998] RBP: 888255fe9158 R08: 38571494 R09: 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] lib: Rework __kms_addfb() function

[Arkadiusz Hiler]

On Wed, Apr 10, 2019 at 12:34:07PM +0100, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-04-10 12:28:08)
> > On Wed, Apr 03, 2019 at 07:24:39PM -0300, Rodrigo Siqueira wrote:
> > > The function __kms_addfb() and drmModeAddFB2WithModifiers() have a
> > > similar code. Due to this similarity, this commit replace part of the
> > > code inside __kms_addfb() by using drmModeAddFB2WithModifiers().
> > 
> > igt master % grep '^libdrm_version' meson.build
> > libdrm_version = '>=2.4.82'
> 
> Why introduce a dependency? The primary purpose of igt is breaking
> ioctls, and that typically requires avoiding any protective libraries.
> In particular, we want to be very, very clear about what igt is doing
> (in terms of kernel api/abi) and not obfuscate.
> -Chris

We depend on one additional call from a library that we already use
widely across the whole codebase and it's straight up a simple wrapper.
Which seems to be a pattern - we us those simple wrapper where
available, and write our own if there is nothing suitable.

It's ok to have some tests that poke the IOCTL more directly, like we
kms_addfb_basic for ADDFB2 (which still uses drmIoctl, so where do we
draw a line?).

If any of thoe wrappers goes awry we can just drop-in replace it with
the original, simple version anyway.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] lib: Rework __kms_addfb() function

[Arkadiusz Hiler]

On Wed, Apr 03, 2019 at 07:24:39PM -0300, Rodrigo Siqueira wrote:
> The function __kms_addfb() and drmModeAddFB2WithModifiers() have a
> similar code. Due to this similarity, this commit replace part of the
> code inside __kms_addfb() by using drmModeAddFB2WithModifiers().

igt master % grep '^libdrm_version' meson.build
libdrm_version = '>=2.4.82'

libdrm master % git log -S drmModeAddFB2WithModifiers
commit abfa680dbdfa4600105d904f4903c047d453cdb5
Author: Kristian H. Kristensen 
Date:   Thu Sep 8 13:08:59 2016 -0700

Add drmModeAddFB2WithModifiers() which takes format modifiers

The only other user of this feature open codes the ioctl. Let's add an
entry point for this to libdrm.

Signed-off-by: Kristian H. Kristensen 
Reviewed-by: Rob Clark 

libdrm master % git describe abfa680
libdrm-2.4.70-15-gabfa680d

We are good on this front.

> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  lib/ioctl_wrappers.c | 27 ++-
>  1 file changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 39920f87..4240d138 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -46,6 +46,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "drmtest.h"
>  #include "i915_drm.h"
> @@ -1479,29 +1480,13 @@ int __kms_addfb(int fd, uint32_t handle,
>   uint32_t strides[4], uint32_t offsets[4],
>   int num_planes, uint32_t flags, uint32_t *buf_id)
>  {
> - struct drm_mode_fb_cmd2 f;
> - int ret, i;
> + uint32_t handles[4] = {handle};
> + uint64_t modifiers[4] = {modifier};

This notation will initialize first element to handle and zero out the
remining ones.

It is not equivalent to what is done below, where handle and modifier
are commpied to each of the num_planes first elements of the arrays.

>   if (flags & DRM_MODE_FB_MODIFIERS)
>   igt_require_fb_modifiers(fd);
>  
> - memset(, 0, sizeof(f));
> -
> - f.width  = width;
> - f.height = height;
> - f.pixel_format = pixel_format;
> - f.flags = flags;
> -
> - for (i = 0; i < num_planes; i++) {
> - f.handles[i] = handle;
> - f.modifier[i] = modifier;

here ^^^

Which may break testing if we ever call it with (num_planes > 1).

> - f.pitches[i] = strides[i];
> - f.offsets[i] = offsets[i];
> - }
> -
> - ret = igt_ioctl(fd, DRM_IOCTL_MODE_ADDFB2, );
> -
> - *buf_id = f.fb_id;
> -
> - return ret < 0 ? -errno : ret;

This also changes behavior, as we log return value of __kms_addfb in few
places, so we lose the information from errno and we would get -1 in
case of any failue now.

> + return drmModeAddFB2WithModifiers(fd, width, height, pixel_format,
> +   handles, strides, offsets, modifiers,
> +   buf_id, flags);
>  }

igt master % ff __kms_addfb
tests/kms_draw_crc.c:166:9: ret =  __kms_addfb(drm_fd, gem_handle, 
64, 64,
tests/kms_available_modes_crc.c:228:8:  ret = __kms_addfb(data->gfx_fd, 
data->gem_handle, w, h,
tests/prime_vgem.c:765:15:  igt_require(__kms_addfb(i915, handle[i],
lib/igt_fb.c:1243:12:   do_or_die(__kms_addfb(fb->fd, 
fb->gem_handle,
lib/ioctl_wrappers.h:217:5: int __kms_addfb(int fd, uint32_t handle,
lib/ioctl_wrappers.c:1457:5:int __kms_addfb(int fd, uint32_t handle,

We call __kms_addfb in 4 places only. It may be worth dropping this
ioctl wrapper introduced in 2015 (so prior to drmModeAddFB2WithModifiers
addition) altogether.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v6 0/3] drm & vgaarb: handle vgacon removal in vgaarb.

[Arkadiusz Hiler]

On Fri, Mar 01, 2019 at 07:12:04AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > Hmm, I see the test results in patchwork, but I can't remember having 
> > > seen a mail.
> > > So the next question: where the results are sent to?
> > From page above sent by Arek:
> > " Since we accept patches through mailing lists, this is where you can
> > find the results - they are sent out as a replies to the original
> > mail. Here are the mailing lists we currently support:"
> 
> Hmm, I'm not subscribed to intel-gfx, so that explains why I havn't
> seen the result mails.  Any chance to sent the results also to the
> patch submitter?

Yes, this is the case now. I re-enabled sending the result emails to the
authors just yesterday.

We had it like that since forever, but recently there was an change to
fdo mailing lists which was overwritting "From" headers to workaround
some delivery issues, so this feature had to be temporaily disabled.

Sorry for the incovenience.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v6 0/3] drm & vgaarb: handle vgacon removal in vgaarb.

[Arkadiusz Hiler]

On Thu, Feb 28, 2019 at 11:52:39AM +0100, Daniel Vetter wrote:
> Adding intel-gfx and CI folks.
> 
> On Thu, Feb 28, 2019 at 9:22 AM Gerd Hoffmann  wrote:
> > Quick question on the Intel CI:  How does it work?  I've seen it sending
> > mails.  Does it report failures only, i.e. no news is good news?  How
> > long does it usually take to run the tests?  Is waiting a day enough?
> > Is there a website where I can see the test results?
> 
> It always reports, and result links also get dumped onto patchwork (in
> the series view). There's 2 main runs: BAT and IGT (first with more
> machines, 2nd with more tests). BAT should be there within a few hours
> at most, IGT sometimes takes 1 day when there's a big backlog.
> 
> -Daniel

Hey,

We try to explain our CI here: https://intel-gfx-ci.01.org/

By the questions you have asked I already see that we should add
information on the results being sent out for successes too and give
some time estimations.

If you see anything elase missing and/or have suggestions on how to
improve what is already there it would be greatly appreciated :-)

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 02:08:12PM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> > On Mon, Feb 18, 2019 at 01:43:50PM +, Chris Wilson wrote:
> > > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > > On Sun, Feb 17, 2019 at 02:35:56PM +, Chris Wilson wrote:
> > > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific 
> > > > > > (and
> > > > > > requires using GEM to do so). It doesn't belong in the general 
> > > > > > paddock
> > > > > > of all driver tests, so move it into the i915/ stable.
> > > > > > 
> > > > > > Signed-off-by: Chris Wilson 
> > > > > > Cc: Arkadiusz Hiler 
> > > > > > Cc: Petri Latvala 
> > > > > > Acked-by: Petri Latvala 
> > > > > > ---
> > > > > >  tests/Makefile.sources| 5 -
> > > > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > > >  tests/meson.build | 2 +-
> > > > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > > 
> > > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > > --- a/tests/Makefile.sources
> > > > > > +++ b/tests/Makefile.sources
> > > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > > >   kms_dp_dsc \
> > > > > >   kms_draw_crc \
> > > > > >   kms_fbcon_fbt \
> > > > > > - kms_fence_pin_leak \
> > > > > >   kms_flip \
> > > > > >   kms_flip_event_leak \
> > > > > >   kms_flip_tiling \
> > > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > > >   vgem_slow \
> > > > > >   $(NULL)
> > > > > >  
> > > > > > +TESTS_progs += \
> > > > > > + i915/kms_fence_pin_leak \
> > > > > > + $(NULL)
> > > > > 
> > > > > This just moves it around, so we will end up having binary named
> > > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > > 
> > > > > That still will install as
> > > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > > 
> > > > > If you want to prefix it:
> > > > >  TESTS_progs += i915_kms_fence_pin_leak
> > > > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > Oterwise:
> > > > >  TESTS_progs += kms_fence_pin_leak
> > > > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > > diff --git a/tests/kms_fence_pin_leak.c 
> > > > > > b/tests/i915/kms_fence_pin_leak.c
> > > > > > similarity index 100%
> > > > > > rename from tests/kms_fence_pin_leak.c
> > > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index ec980651a..08e55b9c0 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > > >   'kms_dp_dsc',
> > > > > >   'kms_draw_crc',
> > > > > >   'kms_fbcon_fbt',
> > > > > > - 'kms_fence_pin_leak',
> > > > > >   'kms_flip',
> > > > > >   'kms_flip_event_leak',
> > > > > >   'kms_flip_tiling',
> > > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > > >   'fb_tiling',
> > > > > >   'getparams_basic',
> > > > > >   'hangman',
> > > > > > + 'kms_fence_pin_leak',
> > > > > >   'missed_irq',
> > > > > >   'module_load',
> > > > > >   'query',
> > > > > 
> > > > > Here, with meson, it will get prefixed with i915_. I'll add a comment 
> > > > > on
> > > > > top of i915_progs just to be more explicit.
> > > > > 
>

Re: [Intel-gfx] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 01:43:50PM +, Chris Wilson wrote:
> Quoting Chris Wilson (2019-02-18 13:42:48)
> > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > On Sun, Feb 17, 2019 at 02:35:56PM +, Chris Wilson wrote:
> > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > of all driver tests, so move it into the i915/ stable.
> > > > 
> > > > Signed-off-by: Chris Wilson 
> > > > Cc: Arkadiusz Hiler 
> > > > Cc: Petri Latvala 
> > > > Acked-by: Petri Latvala 
> > > > ---
> > > >  tests/Makefile.sources| 5 -
> > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > >  tests/meson.build | 2 +-
> > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > 
> > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > index d2c4f9fe9..9972b2dd1 100644
> > > > --- a/tests/Makefile.sources
> > > > +++ b/tests/Makefile.sources
> > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > >   kms_dp_dsc \
> > > >   kms_draw_crc \
> > > >   kms_fbcon_fbt \
> > > > - kms_fence_pin_leak \
> > > >   kms_flip \
> > > >   kms_flip_event_leak \
> > > >   kms_flip_tiling \
> > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > >   vgem_slow \
> > > >   $(NULL)
> > > >  
> > > > +TESTS_progs += \
> > > > + i915/kms_fence_pin_leak \
> > > > + $(NULL)
> > > 
> > > This just moves it around, so we will end up having binary named
> > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > 
> > > That still will install as
> > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > 
> > > If you want to prefix it:
> > >  TESTS_progs += i915_kms_fence_pin_leak
> > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > Oterwise:
> > >  TESTS_progs += kms_fence_pin_leak
> > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > > diff --git a/tests/kms_fence_pin_leak.c 
> > > > b/tests/i915/kms_fence_pin_leak.c
> > > > similarity index 100%
> > > > rename from tests/kms_fence_pin_leak.c
> > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > index ec980651a..08e55b9c0 100644
> > > > --- a/tests/meson.build
> > > > +++ b/tests/meson.build
> > > > @@ -27,7 +27,6 @@ test_progs = [
> > > >   'kms_dp_dsc',
> > > >   'kms_draw_crc',
> > > >   'kms_fbcon_fbt',
> > > > - 'kms_fence_pin_leak',
> > > >   'kms_flip',
> > > >   'kms_flip_event_leak',
> > > >   'kms_flip_tiling',
> > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > >   'fb_tiling',
> > > >   'getparams_basic',
> > > >   'hangman',
> > > > + 'kms_fence_pin_leak',
> > > >   'missed_irq',
> > > >   'module_load',
> > > >   'query',
> > > 
> > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > top of i915_progs just to be more explicit.
> > > 
> > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > yet?
> > 
> > I'd rather not have tests renamed. That's my personal preference. Either
> > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > or it should be installed under tests/i915/.

You mean you want the binaries to be a straight s/\.c//?

I personally like the current way mostly because we avoid 'i915/i915_'
redundancy and the resulting binaries dir is flat, which makes them
PATH-friendly.

The way we handle gem_ adds a little inconsistency, but i915_gem_ would
be too mouthful.

But if what you are proposing is really essential we can stir the pot
some more and i915_ all the .c(s).

> As a case in point, the renaming of benchmarks by meson breaks
> scripts. Please do not do that.
> -Chris

I lack the context here. Do we have any inconsistencies in how autotools
and meson generate binaries? Which scripts are getting broken?

I can look into that and add more GitLab CI/CD consistency checks.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Sun, Feb 17, 2019 at 02:35:56PM +, Chris Wilson wrote:
> kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> requires using GEM to do so). It doesn't belong in the general paddock
> of all driver tests, so move it into the i915/ stable.
> 
> Signed-off-by: Chris Wilson 
> Cc: Arkadiusz Hiler 
> Cc: Petri Latvala 
> Acked-by: Petri Latvala 
> ---
>  tests/Makefile.sources| 5 -
>  tests/{ => i915}/kms_fence_pin_leak.c | 0
>  tests/meson.build | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index d2c4f9fe9..9972b2dd1 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -40,7 +40,6 @@ TESTS_progs = \
>   kms_dp_dsc \
>   kms_draw_crc \
>   kms_fbcon_fbt \
> - kms_fence_pin_leak \
>   kms_flip \
>   kms_flip_event_leak \
>   kms_flip_tiling \
> @@ -99,6 +98,10 @@ TESTS_progs = \
>   vgem_slow \
>   $(NULL)
>  
> +TESTS_progs += \
> + i915/kms_fence_pin_leak \
> + $(NULL)

This just moves it around, so we will end up having binary named
'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.

That still will install as
$PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak

If you want to prefix it:
 TESTS_progs += i915_kms_fence_pin_leak
 i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

Oterwise:
 TESTS_progs += kms_fence_pin_leak
 kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

> diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> similarity index 100%
> rename from tests/kms_fence_pin_leak.c
> rename to tests/i915/kms_fence_pin_leak.c
> diff --git a/tests/meson.build b/tests/meson.build
> index ec980651a..08e55b9c0 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -27,7 +27,6 @@ test_progs = [
>   'kms_dp_dsc',
>   'kms_draw_crc',
>   'kms_fbcon_fbt',
> - 'kms_fence_pin_leak',
>   'kms_flip',
>   'kms_flip_event_leak',
>   'kms_flip_tiling',
> @@ -100,6 +99,7 @@ i915_progs = [
>   'fb_tiling',
>   'getparams_basic',
>   'hangman',
> + 'kms_fence_pin_leak',
>   'missed_irq',
>   'module_load',
>   'query',

Here, with meson, it will get prefixed with i915_. I'll add a comment on
top of i915_progs just to be more explicit.

Do we have any conclusion on prefixing Intel-specific kms tests
yet?

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 5/7] Make string commands dynamic allocate

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:23:30PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following GCC warning:
> 
> intel_gvtg_test.c: In function ‘create_guest’:
> intel_gvtg_test.c:127:50: warning: ‘%s’ directive writing up to 4095
> bytes into a region of size 4077 [-Wformat-overflow=]
> [..]
> intel_gvtg_test.c:127:5: note: ‘sprintf’ output between 36 and 8226
> bytes into a destination of size 4096
> [..]
> 
> This patch changes the approach for allocating memory to handle QEMU
> commands by dynamically allocate space to save the whole command.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  tools/intel_gvtg_test.c | 27 ++-
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/intel_gvtg_test.c b/tools/intel_gvtg_test.c
> index 659b7956..93c05e37 100644
> --- a/tools/intel_gvtg_test.c
> +++ b/tools/intel_gvtg_test.c
> @@ -120,16 +120,25 @@ static int check_tools(void)
>  
>  static void create_guest(void)
>  {
> -char create_qcow_cmd[PATH_MAX] = {0};
> -char create_vgpu_cmd[PATH_MAX] = {0};
> -char create_instance_cmd[PATH_MAX] = {0};
> +unsigned int max_size_cmd = sysconf(_SC_ARG_MAX);

That's 2097152 on my system. Quite an overkill. I know that we have lazy
page table assigement, but memseting the whole 2GB defeats it.

The qemu invocation looks like  the longest one, and we have 3
components that may take up to PATH_MAX each and some extra stuff, so
going with 4*PATH_MAX should be enough.

BTW, why do you memset it to 0 before each sprintf?

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Arkadiusz Hiler]

On Tue, Jul 10, 2018 at 03:09:25PM +0100, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2018-07-10 14:58:49)
> > On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> > > This commit fixes the GCC warning:
> > > 
> > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > >  memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> > > ^
> > > warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
> > >  memset(ptr + offsets[1], 0x80,
> > > 
> > > This commit cast the ptr pointer, which is a void *, to uint32_t * in
> > > the pointer arithmetic operation.
> > 
> > This will change semantics, as according to GNU C standard[1], void
> > pointers have a size of 1 for all arithmetical purposes.
> > 
> > So you should be using uint8_t (or char) pointer instead.
> 
> Please just fix the compiler flags, we want close compatibility with the
> kernel coding standards which explicitly allow void arithmetic for the
> simplicity it lends to writing code.
> -Chris

Fair point.

We don't rise the error with meson, so it is not a change in the gcc
defaults. Somehow autotools manage to end up adding -Wpointer-arith to
BASE_CFLAGS.

I don't think we should invest time into making autotools behave, since
it's going to be dropped completely. Hopefully this will happen sooner
than later.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 4/7] Fix truncate string in the snprintf

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:23:17PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following GCC warning:
> 
> intel_reg.c: In function ‘dump_decode’:
> warning: ‘snprintf’ output may be truncated before the last format character 
> [-Wformat-truncation=]
>snprintf(decode, sizeof(decode), "\n%s", bin);
> intel_reg.c:203:3: note: ‘snprintf’ output between 2 and 1025 bytes into a 
> destination of size 1024
> [..]

That's an odd error.

Line 203:
snprintf(decode, sizeof(decode), "\n%s", bin);

man snprintf states:
The functions snprintf() and vsnprintf() write at most size
bytes (including the terminating null byte ('\0')) to str.

So this should be truncated correctly. Am I missing something?

I'll set up gcc 8.1 tomorrow and poke around.

> Signed-off-by: Rodrigo Siqueira 
> ---
>  tools/intel_reg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index ddff2794..15ebb86a 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -180,7 +180,7 @@ static void to_binary(char *buf, size_t buflen, uint32_t 
> val)
>  
>  static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>  {
> - char decode[1024];
> + char decode[2060];
>   char tmp[1024];
>   char bin[1024];
>  
> -- 
> 2.18.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:22:38PM -0300, Rodrigo Siqueira wrote:
> This commit fixes the GCC warning:
> 
> warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
>  memset(ptr + offsets[0], full_range ? 0x00 : 0x10,
> ^
> warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
>  memset(ptr + offsets[1], 0x80,
> 
> This commit cast the ptr pointer, which is a void *, to uint32_t * in
> the pointer arithmetic operation.

This will change semantics, as according to GNU C standard[1], void
pointers have a size of 1 for all arithmetical purposes.

So you should be using uint8_t (or char) pointer instead.

[1]: http://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html

> Signed-off-by: Rodrigo Siqueira 
> ---
>  lib/igt_fb.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index ae71d967..ca905038 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -410,9 +410,11 @@ static int create_bo_for_fb(int fd, int width, int 
> height,
>  
>   switch (format->drm_id) {
>   case DRM_FORMAT_NV12:
> - memset(ptr + offsets[0], full_range ? 0x00 : 
> 0x10,
> + memset(((uint32_t *)ptr) + offsets[0],
> +full_range ? 0x00 : 0x10,
>  calculated_stride * height);
> - memset(ptr + offsets[1], 0x80,
> + memset(((uint32_t *)ptr) + offsets[1],
> +0x80,
>  calculated_stride * height/2);
>   break;
>   case DRM_FORMAT_YUYV:
> -- 
> 2.18.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/7] Remove parameter aliases with another argument

[Arkadiusz Hiler]

On Sat, Jul 07, 2018 at 08:22:25PM -0300, Rodrigo Siqueira wrote:
> This commit fixes the following GCC warning:
> 
> warning: passing argument 2 to restrict-qualified parameter aliases with
> argument 1 [-Wrestrict]
>   return (readlink (buf, buf, sizeof (buf)) != -1 &&
> 
> This commit fixes the GCC warning by creating a second buffer only to
> keep the path.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  lib/igt_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 3313050c..fa22f12d 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1169,10 +1169,10 @@ bool igt_can_fail(void)
>  
>  static bool run_under_gdb(void)
>  {
> - char buf[1024];
> + char pathname[1024], buf[1024];

1024 for pathname is quite an overshoot. We stash at most
"/proc/%d/exec" where %d should be at most 21 or so.

> - sprintf(buf, "/proc/%d/exe", getppid());
> - return (readlink (buf, buf, sizeof (buf)) != -1 &&
> + sprintf(pathname, "/proc/%d/exe", getppid());
> + return (readlink (pathname, buf, sizeof (buf)) != -1 &&

^ while we are here we can get rid of that space

>   strncmp(basename(buf), "gdb", 3) == 0);
>  }
>  
> -- 
> 2.18.0
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix guest virtual PCH detection on non-PCH systems

[Arkadiusz Hiler]

On Wed, Jun 13, 2018 at 10:16:07AM -0700, Lucas De Marchi wrote:
> On Wed, Jun 13, 2018 at 10:09 AM Lucas De Marchi
>  wrote:
> >
> > On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
> >  wrote:
> > >
> > > On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > > > On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > > > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  
> > > > > wrote:
> > > > >>
> > > > >> On Thu, 31 May 2018, Lucas De Marchi  
> > > > >> wrote:
> > > > >> > On Thu, May 31, 2018 at 02:56:21PM +0300, Jani Nikula wrote:
> > > > >> >> Virtualized non-PCH systems such as Broxton or Geminilake should 
> > > > >> >> use
> > > > >> >> PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a
> > > > >> >> specific case to indicate a PCH system without south display.
> > > > >> >
> > > > >> > Then let's go ahead and document it?
> > > > >>
> > > > >> Please avoid sending suggestion patches in-reply-to existing
> > > > >> series. This confused patchwork and screwed up CI for the series, 
> > > > >> which
> > > > >> was already a resend just to get CI. :(
> > > > >
> > > > > ugh, sorry.  Sometimes just adding a oneline diff is much better than
> > > > > a hundred words explaining :( ...
> > > >
> > > > I feel you, a patch is more precise.
> > > >
> > > > > IMO pw is trying to be smarter than it should here or not being smart
> > > > > enough. Nonetheless I won't do that anymore.
> > > >
> > > > I think there were earlier complaints about what it did recognize and
> > > > what it didn't. I'd be open to only accepting new versions of patches
> > > > from whoever sent the original patch. Or requiring patch subjects don't
> > > > start with "Re:". Or both.
> > >
> > > No matter what you do here it will misbehave in some scenarios and
> > > break someone's workflow :<
> > >
> > > Originally we required the patches to have X-Mailer set to
> > > git-send-email, which seems reasonable, but that annoyed people because
> > > their servers were stripping out those headers.
> > >
> > > Other people send out the patches by feeding them to the drafts folder
> > > through IMAP and then sending them out. This, depending on the
> > > provider's configuration, also gobbles up a thing or two.
> > >
> > > Because of the above I am not sure about trusting "Re:" and matching
> > > "From:" headers as good enough indicators either.
> > >
> > > It just adds more opaque "smartness". I already can foresee questions
> > > asking "why my v2 was not picked up?" and someone would have to debug it
> > > down the line.
> > >
> > > Was the address different (+XYZ before @)? Has that someone used
> > > --subject-prefix=re:? Is it an actual bug? Etc.
> > >
> > >
> > > > I was annoyed, but I'm perhaps more annoyed that you can't do this
> > > > without confusing patchwork. In the end, I wouldn't want to hamper
> > > > review by blocking something that comes naturally to people.
> > > >
> > > > Arek?
> > >
> > > Just add the following header:
> > > "X-Patchwork-Hint: comment"
> > > to emails that you type out manually.
> > >
> > > For mutt it's as easy as adding:
> > > "my_hdr X-Patchwork-Hint: comment"
> > > to your .muttrc
> >
> > This may not work for the same reasons you pointed out that wouldn't
> > work if people are sending patches.  Is there a format I can use that
> > will not trigger patchwork from parsing a _reply_? Does removing the
> > "" help? Under the hood does it use git am --scissors or
> > similar?

Yeah, it's far for perfection and needs additional effort to set the
header up. For me it works, but I always send patches via git-send-email
and always send replies via mutt - I am the simple case.

> Humn... it has its own parser. So if I read
> https://github.com/dlespiau/patchwork/blob/master/patchwork/parser.py#L36
> correctly, it should be just a matter of omitting the "diff | ---"
> lines (or prepending with a "#").
> 
> It does make things more difficult if the other person would use "git
> am --scissors" though.
> 
> Lucas De Marchi

Yes, that is my understanding as well - if you would ommit the "diff
header" it should not recognize your mail as a patch. But that's yet
another behavior you have to know upfront.

It's really hard to strike a balance here.

One idea is to optimize for the default case (i.e. behavior of
git-send-email), sans the known quirks we have seen so far,
and write this down.

Then, if some patches are getting ignored, this would make a handy
checklist that can be use for troubleshooting and we can also link to
it, kindly asking to adhere to a more standard way of sending patches.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm/i915: fix guest virtual PCH detection on non-PCH systems

[Arkadiusz Hiler]

On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  wrote:
> >>
> >> On Thu, 31 May 2018, Lucas De Marchi  wrote:
> >> > On Thu, May 31, 2018 at 02:56:21PM +0300, Jani Nikula wrote:
> >> >> Virtualized non-PCH systems such as Broxton or Geminilake should use
> >> >> PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a
> >> >> specific case to indicate a PCH system without south display.
> >> >
> >> > Then let's go ahead and document it?
> >>
> >> Please avoid sending suggestion patches in-reply-to existing
> >> series. This confused patchwork and screwed up CI for the series, which
> >> was already a resend just to get CI. :(
> >
> > ugh, sorry.  Sometimes just adding a oneline diff is much better than
> > a hundred words explaining :( ...
> 
> I feel you, a patch is more precise.
> 
> > IMO pw is trying to be smarter than it should here or not being smart
> > enough. Nonetheless I won't do that anymore.
> 
> I think there were earlier complaints about what it did recognize and
> what it didn't. I'd be open to only accepting new versions of patches
> from whoever sent the original patch. Or requiring patch subjects don't
> start with "Re:". Or both.

No matter what you do here it will misbehave in some scenarios and
break someone's workflow :<

Originally we required the patches to have X-Mailer set to
git-send-email, which seems reasonable, but that annoyed people because
their servers were stripping out those headers.

Other people send out the patches by feeding them to the drafts folder
through IMAP and then sending them out. This, depending on the
provider's configuration, also gobbles up a thing or two.

Because of the above I am not sure about trusting "Re:" and matching
"From:" headers as good enough indicators either.

It just adds more opaque "smartness". I already can foresee questions
asking "why my v2 was not picked up?" and someone would have to debug it
down the line.

Was the address different (+XYZ before @)? Has that someone used
--subject-prefix=re:? Is it an actual bug? Etc.


> I was annoyed, but I'm perhaps more annoyed that you can't do this
> without confusing patchwork. In the end, I wouldn't want to hamper
> review by blocking something that comes naturally to people.
> 
> Arek?

Just add the following header:
"X-Patchwork-Hint: comment"
to emails that you type out manually.

For mutt it's as easy as adding:
"my_hdr X-Patchwork-Hint: comment"
to your .muttrc

https://github.com/dlespiau/patchwork/commit/148f10115525

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/3] Remove extra '\0' in strncpy in igt_save_module_param

[Arkadiusz Hiler]

Title is slightly missleading. We don't remove extra NULL characer from
anywhere. We just account for it by subtracting 1 from the destination
buffer size.

So maybe "Account for NULL character when using strncpy"?

On Tue, May 29, 2018 at 09:46:55PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following gcc warning:
> 
> warning: ‘strncpy’ specified bound 32 equals destination size
> [-Wstringop-truncation]
>   strncpy(data->name, name, PARAM_NAME_MAX_SZ);
> 
> This error happens due to the '\0' character appended by strncpy. Notice
> that reduces by one in the total of bytes to be copied, in this case, is
> harmless because the strings received in the parameter already have
> '\0'.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  lib/igt_aux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 5dd2761e..06f586d6 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -1240,7 +1240,7 @@ static void igt_save_module_param(const char *name, 
> const char *file_path)
>   data = calloc(1, sizeof (*data));
>   igt_assert(data);
>  
> - strncpy(data->name, name, PARAM_NAME_MAX_SZ);
> + strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1); // -1 because of '\0'

I am not necessary sure that the comment is needed, as "buflen - 1" is
quite typical.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 3/3] Move declaration to the top of the code

[Arkadiusz Hiler]

On Tue, May 29, 2018 at 09:47:13PM -0300, Rodrigo Siqueira wrote:
> This patch fix the following gcc warnings:
> 
> warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement] [..]
> igt_color_encoding.c:45:2: warning: ISO C90 forbids mixed declarations
> and code [-Wdeclaration-after-statement] [..]
> igt_color_encoding.c: In function ‘ycbcr_to_rgb_matrix’:
> igt_color_encoding.c:72:2: warning: ISO C90 forbids mixed declarations
> and code [-Wdeclaration-after-statement] [..]
> 
> Signed-off-by: Rodrigo Siqueira 
Reviewed-by: Arkadiusz Hiler 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] Avoid truncate string in __igt_lsof_fds

[Arkadiusz Hiler]

On Tue, May 29, 2018 at 09:46:38PM -0300, Rodrigo Siqueira wrote:
> Note that 'proc_path' parameter in __igt_lsof_fds receives a string
> which was initialized with the size of PATH_MAX and the local variable
> 'path' has the same size, but it also have to append: '/', '\0', and the
> directory name. This situation caused the warning described below.
> 
> warning: ‘%s’ directive output may be truncated writing up to 255 bytes
> into a region of size between 0 and 4095 [-Wformat-truncation=]
> snprintf(path, sizeof(path), "%s/%s", proc_path, d->d_name);
> note: ‘snprintf’ output between 2 and 4352 bytes into a destination of
> size 4096 [..]
> 
> This patch fix the above problem.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  lib/igt_aux.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index acafb713..5dd2761e 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -1425,7 +1425,7 @@ __igt_lsof_fds(proc_t *proc_info, int *state, char 
> *proc_path, const char *dir)
>  {
>   struct dirent *d;
>   struct stat st;
> - char path[PATH_MAX];
> + char path[PATH_MAX + NAME_MAX + 2]; // 1 byte for '/' and another for 
> '\0'

Hey,

First of, thanks for looking at the new warnings. We definitely have to
fix those :-)

A couple of lines down we have lstat(path, ), which will return -1
with errno == ENAMETOOLONG if we go over PATH_MAX, so this does not feel
right, especially that we are not checking for that.

Digging deeper, __igt_lsof_fds() is used only inside __igt_lsof(), which
is the reason for such a high estimate.

It uses char path[PATH_MAX], which is the main contributing component,
but it contains at most strlen("/proc/%d/cwd")+1 where "%d" is
CEILING(LOG_10(INT_MAX)).

Limiting size of path there to sensible upper estimation should make us
fit in PATH_MAX in __igt_lsof_fds (or even less).

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for linux-next: build failure after merge of the drm-intel tree

[Arkadiusz Hiler]

On Tue, May 08, 2018 at 11:40:52AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> On Tue, 08 May 2018 01:36:25 - Patchwork 
>  wrote:
> >
> > == Series Details ==
> > 
> > Series: linux-next: build failure after merge of the drm-intel tree
> > URL   : https://patchwork.freedesktop.org/series/42839/
> > State : failure
> 
> Blah, blah :-)
> 
> Can someone please arrange for my linux-next merge fix patches to not
> be fed into your patchwork/ci as they will never work out side
> linux-next ...

Hey,

Sorry for clotting your inbox with those mails but I have to ask how
annoying is getting those results?

If it's bearable I would prefer to not introduce special cases basing on
contents of the mbox and/or its author. As of our CI we are not really
that much concerned with wasting time on something that wasn't meant to
be tested - such patches are extremely rare.


If this is necessary - do you have any suggestion how it should be done?
Filtering by "linux-next" in the Subject? By your name? Email address?

Is it the case that linux-next can appear only in untestable patches?
Is silently ignoring such a patch by the CI really okay?

Maybe we should introduce [NOCI] subject tag?
Would you remember to include it?


As of instant solution - you can include "X-Patchwork-Hint: comment" in
the headers, so patchwork won't recognize your email as a patch.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Call i915_perf_fini() on init_hw error unwind

[Arkadiusz Hiler]

On Sat, Apr 14, 2018 at 09:18:52AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Call i915_perf_fini() on init_hw error unwind
> URL   : https://patchwork.freedesktop.org/series/41711/
> State : failure
> 
> == Summary ==
> 
> CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALLscripts/checksyscalls.sh
>   DESCEND  objtool
>   CHK scripts/mod/devicetable-offsets.h
>   CHK include/generated/compile.h
>   CHK kernel/config_data.h
>   CHK include/generated/uapi/linux/version.h
>   DATAREL arch/x86/boot/compressed/vmlinux
>   LD  arch/x86/boot/setup.elf
>   OBJCOPY arch/x86/boot/setup.bin
>   OBJCOPY arch/x86/boot/vmlinux.bin
> objcopy:arch/x86/boot/vmlinux.bin[.rodata..compressed]: No space left on 
> device
> arch/x86/boot/Makefile:86: recipe for target 'arch/x86/boot/vmlinux.bin' 
> failed
> make[1]: *** [arch/x86/boot/vmlinux.bin] Error 1
> arch/x86/Makefile:307: recipe for target 'bzImage' failed
> make: *** [bzImage] Error 2

Whoops. Reclaimed some space, rerun queued.
-Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [CI] drm/i915: Split GPU commands definitions into separate header

[Arkadiusz Hiler]

On Wed, Mar 14, 2018 at 10:19:21AM +, Chris Wilson wrote:
> Quoting Chris Wilson (2018-03-13 23:19:20)
> > From: Michal Wajdeczko 
> > 
> > We should not mix MMIO with MI_INSTR definitions.
> > 
> > v2: sanitize comment, change include order (Chris)
> > 
> > Suggested-by: Chris Wilson 
> > Signed-off-by: Michal Wajdeczko 
> > Cc: Chris Wilson 
> > Reviewed-by: Chris Wilson 
> > Signed-off-by: Chris Wilson 
> > Link: 
> > https://patchwork.freedesktop.org/patch/msgid/20180313124109.39216-1-michal.wajdec...@intel.com
> 
> Doesn't like it when I send it either? Very odd.

Indeed. Same patch, different sender and it goes missing again.

There is no parse error and when the patchwork is fed with the patch
manually it goes through just fine.

There's no sign of the patch ever reaching the server. Looks like it
gets discarded somewhere on the way.

Ccing Daniel, maybe he will be able to help with root causing.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 0/4] drm/i915: misc fixes in headers (RESEND)

[Arkadiusz Hiler]

On Thu, Mar 08, 2018 at 04:22:55PM +0100, Michal Wajdeczko wrote:
> On Thu, 08 Mar 2018 13:58:48 +0100, Petri Latvala 
> wrote:
> 
> > On Thu, Mar 08, 2018 at 02:20:41PM +0200, Jani Nikula wrote:
> > > On Thu, 08 Mar 2018, Chris Wilson  wrote:
> > > > Quoting Michal Wajdeczko (2018-03-08 09:50:33)
> > > >> This is a resend, to make patchwork happy.
> > > >> All patches already been reviewed.
> > > >
> > > > It's still being ignored. :(
> > > 
> > > At least patchwork detected this as a series [1], often that's the
> > > problem.
> > 
> > 
> > Patchwork is not seeing patch 4/4, and due to its weird logic, it's
> > still waiting for it instead of considering the series "incomplete".
> > 
> > Looking at the series page, it doesn't even have a revision number.
> > 
> > This causes the pw REST api to show the series as one with 0 patches
> > when listing series, there's no series-new-revision event (CI is
> > watching that), and it also causes the below:
> > 
> > > I tried to start testing manually from patchwork, it gives me "failed!".
> > 
> > 
> > Arek checked the logs and patch 4/4 hasn't been received by pw at
> > all. Maybe it's still on the way, who knows.
> > 
> 
> I'm not sure if this is related, but list archive decided to assign
> patch 1/4 [1] to old series [2] instead new one [3]
> 
> [1] https://lists.freedesktop.org/archives/intel-gfx/2018-March/158080.html
> [2] 
> https://lists.freedesktop.org/archives/intel-gfx/2018-March/thread.html#157974
> [3] 
> https://lists.freedesktop.org/archives/intel-gfx/2018-March/thread.html#158081
> 
> ps. patch 4/4 is already in archive [4]
> 
> [4] https://lists.freedesktop.org/archives/intel-gfx/2018-March/158082.html

The patch got lost in time and space and has never reached patchwork's
mail server.

I feed the patchwork with a copy of the mail manually. Expect results soon.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 vs checkpatch

[Arkadiusz Hiler]

On Thu, Mar 01, 2018 at 11:17:50PM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2018-03-01 09:47:06)
> > Hey all,
> > 
> > Since not so long ago our CI is running and reporting sparse and
> > checkpatch. Sparse is doing just fine but I had to disable checkpatch
> > for the time being - too much "false" positives causing people to
> > complain. It's simply confusing to see one thing in the code, and
> > fitting your change in only to get a report that it's wrong.
> 
> Another aspect is that we use the kernel coding style for igt as well.
> checkpatch.pl should be able to pick up style issues on igt patches.
> -Chris

I was thinking the same. It should be doable with couple of ignores here
and there (e.g. complaining about new files and MAINTAINERS file).

I will get to it once we will have figured out how to checkpatch i915
properly.

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 vs checkpatch

[Arkadiusz Hiler]

On Mon, Mar 05, 2018 at 01:10:21PM +0200, Jani Nikula wrote:
> On Mon, 05 Mar 2018, Daniel Vetter  wrote:
> > I'd recommend not making checkpatch ever fail CI, but at most warning.
> 
> Agreed. But we want the automated warnings on the list, neutrally from a
> bot instead of a developer spending time nitpicking this stuff. And the
> committers should pay attention before pushing.

We are never failing CI because of it. We are sending it simply as a
warning (if there's anything to report).

> Really, everyone should be running checkpatch themselves locally before
> sending patches, ignoring the irrelevant warnings with good taste...
> 
> > Plus silence the ones we obviously think are silly (or currently
> > inconsistent in our code).
> >
> > I think the ingore list is probably best kept within maintainer-tools
> > itself, that way we at least have visibility into it from committers.
> 
> Agreed, but as I wrote in [1] we need to add checkpatch profiles or
> config or something, because I want *all* the warnings when I run it
> locally. And if we decide to, say, enforce kernel types in i915 but
> drm-misc decides otherwise, that's also another config.
> 
> BR,
> Jani.
> 
> 
> [1] 87zi3qtq9f.fsf@intel.com">http://mid.mail-archive.com/87zi3qtq9f.fsf@intel.com

Good. CI is using dim and I want too keep it that way. I prefer a cmd
line switch over .dimrc. Keeping track of an additional file for the
builder would be an annoyance.

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] i915 vs checkpatch

[Arkadiusz Hiler]

Hey all,

Since not so long ago our CI is running and reporting sparse and
checkpatch. Sparse is doing just fine but I had to disable checkpatch
for the time being - too much "false" positives causing people to
complain. It's simply confusing to see one thing in the code, and
fitting your change in only to get a report that it's wrong.

We are explicitly going against couple of the recommendations it tries
to enforce, e.g. not using BIT macro, splitting quoted strings:
https://lists.freedesktop.org/archives/intel-gfx/2018-February/156613.html

IMHO we should make a couple of decisions here:
 1. Do we really want to use the checkpatch / have CI reports?
 2. Which of the checkpatch checks we want to disabled for i915?
 3. How strongly do we want to enforce the rest?
 4. Do we want to change what's already in the tree, for compliance?

Recent-ish drm-tip, vanilla checkpatch on i915 reports:
total: 399 errors, 3573 warnings, 209374 lines checked

-- 
Cheers,
Arek


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt v2] tests: Add a random load generator

[Arkadiusz Hiler]

On Mon, Jan 22, 2018 at 11:14:01AM +0200, Chris Wilson wrote:
> Apply a random load to one or all engines in order to apply stress to
> RPS as it tries to constantly adjust the GPU frequency to meet the
> changing workload.

Doing some IGT archeology here.

Seems like both 'all' and 'pulse' subtests cause nasty incompletes.
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_809/shards-all.html#igt@gem_exec_load@all

What are the plans for the gem_exec_load nowadays?

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 1/2] tests/testdisplay: Explicitly use GLIB_CFLAGS

[Arkadiusz Hiler]

On Tue, Feb 27, 2018 at 03:38:58PM +0200, Petri Latvala wrote:
> On Mon, Feb 05, 2018 at 12:56:10PM +0100, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > testdisplay.h includes the glib.h header file but the Makefile does not
> > explicitly pass a -I option with the path containing that header, hence
> > causing the build to fail. Note that this doesn't seem to happen with a
> > recent enough version of cairo, which implicitly provides the correct
> > -I option.
> 
> 
> Hmm. We only have GLIB_CFLAGS when HAVE_GLIB, but testdisplay is
> always built. Same for intel_dp_compliance.
> 
> Should we make glib mandatory as well? Arek, thoughts?

Yep, making sounds good. It's extremely common and we use it also for
the igtrc handling.

This allows us to remove quite a few more ifdefs and will make couple of
pending reworks easier :-)

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt 1/2] lib: Export kmsg()

[Arkadiusz Hiler]

On Wed, Feb 21, 2018 at 11:28:45AM +, Chris Wilson wrote:
> Export the kmsg() function for use by tests to write into the kernel
> message log, useful for tests to inline their progress with kernel error
> messages.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>

Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

both patches and pushed.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for tests/kms_rotation_crc: Remove hardcoding of platforms in igt_require()

[Arkadiusz Hiler]

On Tue, Feb 13, 2018 at 01:38:21AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: tests/kms_rotation_crc: Remove hardcoding of platforms in 
> igt_require()
> URL   : https://patchwork.freedesktop.org/series/38108/
> State : failure
> 
> == Summary ==
> 
> Test kms_rotation_crc:
> Subgroup primary-rotation-180:
> pass   -> SKIP   (shard-snb)
> pass   -> SKIP   (shard-hsw)
> pass   -> SKIP   (shard-apl) fdo#103925 +1
> Subgroup exhaust-fences:
> skip   -> PASS   (shard-apl)
> Subgroup sprite-rotation-270:
> pass   -> SKIP   (shard-apl) fdo#103356 +1
> Subgroup primary-rotation-270:
> pass   -> SKIP   (shard-apl)
> Subgroup cursor-rotation-180:
> pass   -> SKIP   (shard-snb)
> pass   -> SKIP   (shard-hsw) fdo#102614
> pass   -> SKIP   (shard-apl)
> Subgroup sprite-rotation-180:
> pass   -> SKIP   (shard-snb)
> pass   -> SKIP   (shard-hsw)
> pass   -> SKIP   (shard-apl)

This is strange. Those tests were previously passing on apl, hsw and snb
and now there are skipping? Is the kernel misadvertising properties for
those GENs?

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✓ Fi.CI.IGT: success for igt/gem_ctx_isolation: Check isolation of registers between contexts (rev11)

[Arkadiusz Hiler]

On Wed, Feb 21, 2018 at 11:19:07AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: igt/gem_ctx_isolation: Check isolation of registers between contexts 
> (rev11)
> URL   : https://patchwork.freedesktop.org/series/32531/
> State : success

https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_967/shards-all.html#igt@gem_ctx_isolation@bcs0-clean
Seems like just the s3 and s4 are expectedly troubled.

Do you have a reviewer candidate for this patch?

Also can you please CC igt-dev? It makes things easier to track.

- Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Adding NV12 support (rev10)

[Arkadiusz Hiler]

On Wed, Feb 14, 2018 at 02:07:49PM +0100, Maarten Lankhorst wrote:
> Op 14-02-18 om 13:37 schreef Patchwork:
> > == Series Details ==
> >
> > Series: Adding NV12 support (rev10)
> > URL   : https://patchwork.freedesktop.org/series/28103/
> > State : warning
> >
> > == Summary ==
> >
> > $ dim sparse origin/drm-tip
> > Commit: drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values
> > Okay!
> >
> > Commit: drm/i915/skl+: refactor WM calculation for NV12
> > -drivers/gpu/drm/i915/gvt/mmio.c:256:23: warning: memcpy with byte count of 
> > 279040
> > -drivers/gpu/drm/i915/gvt/mmio.c:257:23: warning: memcpy with byte count of 
> > 279040
> > + ^~
> > +^~
> > -drivers/gpu/drm/i915/i915_perf.c:1366:15: warning: memset with byte count 
> > of 16777216
> > -drivers/gpu/drm/i915/i915_perf.c:1424:15: warning: memset with byte count 
> > of 16777216
> > +drivers/gpu/drm/i915/intel_pm.c:4656:19: error: no member 'is_nv12' in 
> > struct skl_plane_wm
> > +drivers/gpu/drm/i915/intel_pm.c:4656:19: warning: generating address of 
> > non-lvalue (8)
> > +drivers/gpu/drm/i915/intel_pm.c:4656:5: error: ‘struct skl_plane_wm’ has 
> > no member named ‘is_nv12’
> > +drivers/gpu/drm/i915/intel_pm.c:4834:15: error: no member 'is_nv12' in 
> > struct skl_plane_wm
> > +drivers/gpu/drm/i915/intel_pm.c:4834:15: warning: unknown expression (8 46)
> > +drivers/gpu/drm/i915/intel_pm.c:4834:8: error: ‘const struct skl_plane_wm’ 
> > has no member named ‘is_nv12’
> > +drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_compute_wm_levels’:
> > +drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_write_plane_wm’:
> > +  if (wm->is_nv12) {
> > +make[1]: *** [drivers/gpu/drm/i915] Error 2
> > +make[2]: *** [drivers/gpu/drm/i915/intel_pm.o] Error 1
> > +make[2]: *** Waiting for unfinished jobs
> > +make[2]: *** wait: No child processes.  Stop.
> > +make: *** [drivers/gpu/drm/] Error 2
> > +   wm->is_nv12 = true;
> >
> > Commit: drm/i915/skl+: add NV12 in skl_format_to_fourcc
> > -drivers/gpu/drm/i915/i915_perf.c:1366:15: warning: memset with byte count 
> > of 16777216
> > -drivers/gpu/drm/i915/i915_perf.c:1424:15: warning: memset with byte count 
> > of 16777216
> > +drivers/gpu/drm/i915/gvt/mmio.c:256:23: warning: memcpy with byte count of 
> > 279040
> > +drivers/gpu/drm/i915/gvt/mmio.c:257:23: warning: memcpy with byte count of 
> > 279040
> >
> > Commit: drm/i915/skl+: support verification of DDB HW state for NV12
> > -make[2]: *** wait: No child processes.  Stop.
> >
> > Commit: drm/i915/skl+: NV12 related changes for WM
> > - ^~
> > -^~
> > +drivers/gpu/drm/i915/gvt/mmio.c:256:23: warning: memcpy with byte count of 
> > 279040
> > +drivers/gpu/drm/i915/gvt/mmio.c:257:23: warning: memcpy with byte count of 
> > 279040
> > +drivers/gpu/drm/i915/i915_perf.c:1366:15: warning: memset with byte count 
> > of 16777216
> > +drivers/gpu/drm/i915/i915_perf.c:1424:15: warning: memset with byte count 
> > of 16777216
> > -O:drivers/gpu/drm/i915/intel_pm.c:4687:19: error: no member 'is_nv12' in 
> > struct skl_plane_wm
> > -O:drivers/gpu/drm/i915/intel_pm.c:4687:19: warning: generating address of 
> > non-lvalue (8)
> > -O:drivers/gpu/drm/i915/intel_pm.c:4687:5: error: ‘struct skl_plane_wm’ has 
> > no member named ‘is_nv12’
> > -O:drivers/gpu/drm/i915/intel_pm.c:4865:15: error: no member 'is_nv12' in 
> > struct skl_plane_wm
> > -O:drivers/gpu/drm/i915/intel_pm.c:4865:15: warning: unknown expression (8 
> > 46)
> > -O:drivers/gpu/drm/i915/intel_pm.c:4865:8: error: ‘const struct 
> > skl_plane_wm’ has no member named ‘is_nv12’
> > -drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_compute_wm_levels’:
> > -drivers/gpu/drm/i915/intel_pm.c: In function ‘skl_write_plane_wm’:
> > -  if (wm->is_nv12) {
> > -make[1]: *** [drivers/gpu/drm/i915] Error 2
> > -make[2]: *** [drivers/gpu/drm/i915/intel_pm.o] Error 1
> > -make[2]: *** Waiting for unfinished jobs
> > -make[2]: *** wait: No child processes.  Stop.
> > -make: *** [drivers/gpu/drm/] Error 2
> > -   wm->is_nv12 = true;
> Looks like not all commits compile on their own?
> ~Maarten

Yep. That is exactly the case. We run sparse build incrementally, i.e.
applying one patch after another, building and looking for any
difference in what is reported.

Patch
> Commit: drm/i915/skl+: rename skl_wm_values struct to skl_ddb_values
breaks compilation.

Please fix that. You can check your series locally with something like:
git rebase HEAD~12 --exec make

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/4] meson: Build cnl_compute_wrpll

[Arkadiusz Hiler]

On Tue, Jan 16, 2018 at 01:13:03PM +0200, Petri Latvala wrote:
> Signed-off-by: Petri Latvala <petri.latv...@intel.com>

Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

one the whole series

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/27] drm/i915/icl: Set graphics mode register for gen11

[Arkadiusz Hiler]

On Tue, Jan 09, 2018 at 09:28:22PM -0200, Paulo Zanoni wrote:
> From: kgardine 

Please fix this to use Kelvin's full name when pushing.
Both here and in the s-o-b line.

It may trigger this rule causing a rejection somewhere up the merge
chain:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n460

> This patch clears a single bit. The bit is 0 by default but expected not to be
> set. Explicitly clearing the bit in this patch is intended to indicate some
> thinking has occurred, and that we want this bit cleared and we are not just
> excepting the default value.
> 
> v2 (from Paulo): fix indentation.
> v3 (from Paulo): rebase.
> 
> Signed-off-by: kgardine 
> Signed-off-by: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  2 ++
>  drivers/gpu/drm/i915/intel_lrc.c | 10 --
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f383ee5cc592..a16a8a2b17b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2597,6 +2597,8 @@ enum i915_power_well_id {
>  #define   GFX_FORWARD_VBLANK_ALWAYS  (1<<5)
>  #define   GFX_FORWARD_VBLANK_COND(2<<5)
>  
> +#define   GEN11_GFX_DISABLE_LEGACY_MODE  (1<<3)
> +
>  #define VLV_DISPLAY_BASE 0x18
>  #define VLV_MIPI_BASE VLV_DISPLAY_BASE
>  #define BXT_MIPI_BASE 0x6
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index dab988f20833..d435a9982d0b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1500,8 +1500,14 @@ static void enable_execlists(struct intel_engine_cs 
> *engine)
>   struct drm_i915_private *dev_priv = engine->i915;
>  
>   I915_WRITE(RING_HWSTAM(engine->mmio_base), 0x);
> - I915_WRITE(RING_MODE_GEN7(engine),
> -_MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
> +
> + if (IS_GEN11(dev_priv))
> + I915_WRITE(RING_MODE_GEN7(engine),
> +_MASKED_BIT_DISABLE(GEN11_GFX_DISABLE_LEGACY_MODE));
> + else
> + I915_WRITE(RING_MODE_GEN7(engine),
> +_MASKED_BIT_ENABLE(GFX_RUN_LIST_ENABLE));
> +
>   I915_WRITE(RING_HWS_PGA(engine->mmio_base),
>  engine->status_page.ggtt_offset);
>   POSTING_READ(RING_HWS_PGA(engine->mmio_base));
> -- 
> 2.14.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt 1/4] igt/gem_busy: Remove repeated use of igt_spin_batch_new

[Arkadiusz Hiler]

On Wed, Jan 03, 2018 at 06:09:06PM +, Chris Wilson wrote:
> igt_spin_batch_new() includes a throttling check that GEM works, which
> breaks trying to create multiple spin batches, use
> __igt_spin_batch_new() instead, after verifying GEM works.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

on the whole series and pushed
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt] igt/kms_flip: Do igt_require_gem() just once

[Arkadiusz Hiler]

On Wed, Jan 03, 2018 at 05:57:28PM +, Chris Wilson wrote:
> Since igt_spin_batch_new() will do a stalling GEM check, it is not
> advisable to use it within loops. Perform the igt_require_gem() upfront
> and then use __igt_spin_batch_new() inside the test loop.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

and pushed
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt] lib: Convert sw_sync to use sync_file uapi imported from the kernel

[Arkadiusz Hiler]

On Tue, Dec 19, 2017 at 03:00:03PM +, Chris Wilson wrote:
> Similar to how we are now importing the drm uapi directly into igt, we
> also would like to have a copy of auxiliary uAPI such as sync_file.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

and pushed
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt 2/2] igt/kms_frontbuffer_tracking: Access via GGTT is not guaranteed to be tracked

[Arkadiusz Hiler]

On Thu, Dec 14, 2017 at 08:09:21PM +, Chris Wilson wrote:
> Quoting Chris Wilson (2017-12-07 09:41:26)
> > As the system may use a partial vma for a GGTT mmap, access via the GGTT
> > mmap is not guaranteed to be tracked by FBC's fence. The rule expressed has
> > been that any access to the frontbuffer should be followed by a fb-dirty
> > ioctl, so always apply and expect the driver to ellide no-ops.
> > 
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> 
> Ping?

Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

on both. Pushed.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for igt/pm_rps: Always allocate spin[0]

[Arkadiusz Hiler]

On Mon, Dec 11, 2017 at 04:18:40PM +, Patchwork wrote:
> == Series Details ==
> 
> Series: igt/pm_rps: Always allocate spin[0]
> URL   : https://patchwork.freedesktop.org/series/35176/
> State : failure
> 
> == Summary ==
> 
> Test gem_tiled_swapping:
> Subgroup non-threaded:
> incomplete -> PASS   (shard-hsw) fdo#104009
> Test pm_rps:
> Subgroup min-max-config-loaded:
> pass   -> FAIL   (shard-snb)
> pass   -> FAIL   (shard-hsw)

Just for the sanity, so it does not look like it's lingering here
unattended.

Seems to break it for all platforms:
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_642/shard-snb4/igt@pm_...@min-max-config-loaded.html
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_642/shard-hsw1/igt@pm_...@min-max-config-loaded.html
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_642/shard-apl1/igt@pm_...@min-max-config-loaded.html
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_642/shard-kbl7/igt@pm_...@min-max-config-loaded.html

Causing the same fail:

Increase max above RP0 (invalid)...
(pm_rps:2211) DEBUG: gt freq (MHz):  cur=1350  min=850  max=1350  RP0=1350  
RP1=850  RPn=850  boost=1350
(pm_rps:2211) DEBUG: Required 0 msec to reach cur=max
(pm_rps:2211) CRITICAL: Test assertion failure function load_helper_stop, file 
pm_rps.c:277:
(pm_rps:2211) CRITICAL: Failed assertion: igt_wait_helper(_proc) == 0
(pm_rps:2211) CRITICAL: Last errno: 22, Invalid argument
(pm_rps:2211) igt-core-INFO: Stack trace:
(pm_rps:2211) igt-core-INFO:   #0 [__igt_fail_assert+0x101]
(pm_rps:2211) igt-core-INFO:   #1 [load_helper_stop+0x4f]
(pm_rps:2211) igt-core-INFO:   #2 [__real_main564+0x1a8]
(pm_rps:2211) igt-core-INFO:   #3 [main+0x27]
(pm_rps:2211) igt-core-INFO:   #4 [__libc_start_main+0xf1]
(pm_rps:2211) igt-core-INFO:   #5 [_start+0x2a]
(pm_rps:2211) igt-core-INFO:   #6 [+0x2a]


-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt v2] igt/gem_linear_blits: Compute GTT size using 4G limit

[Arkadiusz Hiler]

On Fri, Dec 15, 2017 at 12:22:01PM +, Chris Wilson wrote:
> Quoting Chris Wilson (2017-09-13 11:39:14)
> > Both gem_linear_blits and gem_tiled_blit do not request the full 48b
> > GTT layout for their objects, restricting themselves to 4G. The
> > underlying test that they trigger eviction is unaffected by this
> > restriction, so we can simply reduce their memory requirements to fill
> > the low 4G GTT space and so allow them to run on 48b machines.
> > 
> > v2: gem_tiled_fenced_blits as well
> > 
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> 
> Ping ?

Pong.

Requeued this for testing, as the old results seem to be lost in time
and space.

> > ---
> >  tests/gem_linear_blits.c  | 18 ++
> >  tests/gem_tiled_blits.c   | 16 
> >  tests/gem_tiled_fence_blits.c | 11 +--
> >  3 files changed, 35 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tests/gem_linear_blits.c b/tests/gem_linear_blits.c
> > index eccfbcdd..8297416c 100644
> > --- a/tests/gem_linear_blits.c
> > +++ b/tests/gem_linear_blits.c
> > @@ -215,6 +215,8 @@ static void run_test(int fd, int count)
> > free(handle);
> >  }
> >  
> > +#define MAX_32b ((1ull << 32) - 4096)
> > +
> >  int main(int argc, char **argv)
> >  {
> > int fd = 0;
> > @@ -230,20 +232,28 @@ int main(int argc, char **argv)
> > run_test(fd, 2);
> >  
> > igt_subtest("normal") {
> > -   int count;
> > +   uint64_t count;
> >  
> > -   count = 3 * gem_aperture_size(fd) / (1024*1024) / 2;
> > +   count = gem_aperture_size(fd);
> > +   if (count >> 32)
> > +   count = MAX_32b;
> > +   count = 3 * count / (1024*1024) / 2;

Having that in 4 places may justify an introduction of a helper.

Nonetheless,
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

- Arek

> > igt_require(count > 1);
> > intel_require_memory(count, sizeof(linear), CHECK_RAM);
> > +
> > run_test(fd, count);
> > }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] intel_vbt_decode: Typo fixes

[Arkadiusz Hiler]

On Fri, Dec 15, 2017 at 02:59:36PM -0500, Adam Jackson wrote:
> Signed-off-by: Adam Jackson 
> ---
>  tools/intel_vbt_decode.c | 4 ++--
>  tools/intel_vbt_defs.h   | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)

This mail was rejected by Patchwork because of "X-Mailer" header,
indicating that it was sent with git send-email, is missing.

I forced it through manually, so we will have CI feedback soon
(so very necessary for this complex patch :-P).

May I ask why is the header missing from your patch?
I wonder whether should we relax patchwork's checks.

-- 
Cheers,
Arek

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✓ Fi.CI.IGT: success for igt/core_suspend: Exercise igt_system_suspend_autoresume() (rev2)

[Arkadiusz Hiler]

On Fri, Dec 08, 2017 at 11:30:07PM +, Patchwork wrote:
> == Series Details ==
> 
> Series: igt/core_suspend: Exercise igt_system_suspend_autoresume() (rev2)
> URL   : https://patchwork.freedesktop.org/series/31986/
> State : success
> 
> == Summary ==

https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_634/shards-all.html#igt@core_suspend@suspend-disk-core

igt@core_suspend@suspend-disk-full
fails on all platforms

igt@core_suspend@suspend-freeze-core
igt@core_suspend@suspend-freeze-processors
give us a system hand
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 4/4] run-tests.sh: Allow users to override IGT_TEST_ROOT

[Arkadiusz Hiler]

On Wed, Dec 13, 2017 at 02:58:16PM +0200, Petri Latvala wrote:
> Signed-off-by: Petri Latvala <petri.latv...@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.IGT: warning for test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc (rev4)

[Arkadiusz Hiler]

On Wed, Dec 13, 2017 at 11:08:39AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: test/kms_plane_lowres: Fix display_commit_mode() so it returns the 
> crc (rev4)
> URL   : https://patchwork.freedesktop.org/series/34749/
> State : warning
> 
> == Summary ==
> 
> Test pm_rc6_residency:
> Subgroup rc6-accuracy:
> pass   -> SKIP   (shard-snb)
> Test gem_tiled_swapping:
> Subgroup non-threaded:
> incomplete -> PASS   (shard-snb) fdo#104009
> Test gem_pwrite:
> Subgroup huge-gtt-backwards:
> incomplete -> PASS   (shard-hsw) fdo#104218
> Test gem_softpin:
> Subgroup noreloc-s4:
> fail   -> SKIP   (shard-snb) fdo#103375
> Test kms_frontbuffer_tracking:
> Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
> pass   -> FAIL   (shard-snb) fdo#101623 +1

Okay, finally it's all green on kms_plane_lowres and the tests does what
it was intended to do.

Thanks for improving on the original patch, the feedback and the reviews.

This is now merged with the typos corrected.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] lib/i915_pciids.h: synchronize with kernel header

[Arkadiusz Hiler]

On Fri, Dec 08, 2017 at 02:06:46PM -0800, Lucas De Marchi wrote:
> This copies include/drm/i915_pciids.h from kernel as of drm-tip:
> drm-tip: 2017y-12m-08d-21h-06m-35s UTC + patch adding INTEL_CFL_IDS that
> was missing there[1]. The goal is to keep track of the PCI IDs in a
> single place (kernel).
> 
> Right now a simple copy is done to catch up with latest changes there,
> although in future it could be more sofisticated pointing the build
> system to the external header.
> 
> [1] https://patchwork.freedesktop.org/patch/192410/
> 
> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demar...@intel.com>

---
% (cd igt ; git pw apply 35121)
% diff -u linux/include/drm/i915_pciids.h igt/lib/i915_pciids.h
--- linux/include/drm/i915_pciids.h 2017-11-21 13:24:48.921774670 +0200
+++ igt/lib/i915_pciids.h   2017-12-12 15:33:02.915711190 +0200
@@ -392,6 +392,12 @@
INTEL_VGA_DEVICE(0x3EA8, info), /* ULT GT3 */ \
INTEL_VGA_DEVICE(0x3EA5, info)  /* ULT GT3 */

+#define INTEL_CFL_IDS(info) \
+   INTEL_CFL_S_GT1_IDS(info), \
+   INTEL_CFL_S_GT2_IDS(info), \
+   INTEL_CFL_H_GT2_IDS(info), \
+   INTEL_CFL_U_GT3_IDS(info)
+
 /* CNL U 2+2 */
 #define INTEL_CNL_U_GT2_IDS(info) \
INTEL_VGA_DEVICE(0x5A52, info), \
-------

looks good

Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
and merged, thanks!
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

[Arkadiusz Hiler]

On Tue, Dec 12, 2017 at 02:10:56PM +0200, Arkadiusz Hiler wrote:
> On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote:
> > From: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> > 
> > Compiler complained on crc_lowres and crc_hires2 being uninitialized
> > and indeed, display_commit_mode() seems to have intention of retruning
> > the value through the parameter that is only a single pointer.
> > 
> > This couses only the local copy of the pointer, the one inside
> > display_commit_mode(), to be overwritten.
> > 
> > Let's fix that!
> > 
> > Also add missing hires crc comparison (M. Kahola).
> > 
> > v2: make display_commit_mode return just the last CRC
> > v3: Don't do memory allocations, it's hard. (Maarten)
> > 
> > Cc: Mika Kahola <mika.kah...@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> > Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> > ---
> > So sorry, didn't like the memory allocations, hope this works.. else I'll 
> > commit v2..
> > 
> > tests/kms_plane_lowres.c | 25 -
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> > index 85d3145de4b6..f643bb4b0e8f 100644
> > --- a/tests/kms_plane_lowres.c
> > +++ b/tests/kms_plane_lowres.c
> > @@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum 
> > pipe pipe)
> > data->fb = NULL;
> >  }
> >  
> > -static int
> > +static void
> >  display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc,
> > -   enum pipe pipe, int flags, igt_crc_t *crc)
> > +   enum pipe pipe, int flags, igt_crc_t *out_crc)
> >  {
> > char buf[256];
> > +   igt_crc_t *crcs;
> > struct drm_event *e = (void *)buf;
> > unsigned int vblank_start, vblank_stop;
> > int n, ret;
> > @@ -148,10 +149,12 @@ display_commit_mode(igt_display_t *display, 
> > igt_pipe_crc_t *pipe_crc,
> > igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
> > igt_reset_timeout();
> >  
> > -   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
> > +   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
> > igt_assert_eq(n, vblank_stop - vblank_start);
> >  
> > -   return n;
> > +   /* there is no need to return all the intermediary CRCs */
> > +   /* so let's return just the last one */
> > +   *out_crc = crcs[n-1];
> 
> free(crcs); ?

With that fixed
Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

[Arkadiusz Hiler]

On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote:
> From: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> 
> Compiler complained on crc_lowres and crc_hires2 being uninitialized
> and indeed, display_commit_mode() seems to have intention of retruning
> the value through the parameter that is only a single pointer.
> 
> This couses only the local copy of the pointer, the one inside
> display_commit_mode(), to be overwritten.
> 
> Let's fix that!
> 
> Also add missing hires crc comparison (M. Kahola).
> 
> v2: make display_commit_mode return just the last CRC
> v3: Don't do memory allocations, it's hard. (Maarten)
> 
> Cc: Mika Kahola <mika.kah...@intel.com>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> ---
> So sorry, didn't like the memory allocations, hope this works.. else I'll 
> commit v2..
> 
> tests/kms_plane_lowres.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
> index 85d3145de4b6..f643bb4b0e8f 100644
> --- a/tests/kms_plane_lowres.c
> +++ b/tests/kms_plane_lowres.c
> @@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum pipe 
> pipe)
>   data->fb = NULL;
>  }
>  
> -static int
> +static void
>  display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc,
> - enum pipe pipe, int flags, igt_crc_t *crc)
> + enum pipe pipe, int flags, igt_crc_t *out_crc)
>  {
>   char buf[256];
> + igt_crc_t *crcs;
>   struct drm_event *e = (void *)buf;
>   unsigned int vblank_start, vblank_stop;
>   int n, ret;
> @@ -148,10 +149,12 @@ display_commit_mode(igt_display_t *display, 
> igt_pipe_crc_t *pipe_crc,
>   igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
>   igt_reset_timeout();
>  
> - n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
> + n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
>   igt_assert_eq(n, vblank_stop - vblank_start);
>  
> - return n;
> + /* there is no need to return all the intermediary CRCs */
> + /* so let's return just the last one */
> + *out_crc = crcs[n-1];

free(crcs); ?

>  }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v2] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

[Arkadiusz Hiler]

Compiler complained on crc_lowres and crc_hires2 being uninitialized
and indeed, display_commit_mode() seems to have intention of retruning
the value through the parameter that is only a single pointer.

This couses only the local copy of the pointer, the one inside
display_commit_mode(), to be overwritten.

Let's fix that!

Also add missing hires crc comparison (M. Kahola).

v2: make display_commit_mode return just the last CRC

Cc: Mika Kahola <mika.kah...@intel.com>
Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 tests/kms_plane_lowres.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 85d3145d..a5518f17 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -125,11 +125,12 @@ test_fini(data_t *data, igt_output_t *output, enum pipe 
pipe)
data->fb = NULL;
 }
 
-static int
+static void
 display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc,
-   enum pipe pipe, int flags, igt_crc_t *crc)
+   enum pipe pipe, int flags, igt_crc_t **out_crc)
 {
char buf[256];
+   igt_crc_t *crcs;
struct drm_event *e = (void *)buf;
unsigned int vblank_start, vblank_stop;
int n, ret;
@@ -148,10 +149,14 @@ display_commit_mode(igt_display_t *display, 
igt_pipe_crc_t *pipe_crc,
igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
igt_reset_timeout();
 
-   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
+   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
igt_assert_eq(n, vblank_stop - vblank_start);
 
-   return n;
+   /* there is no need to return all the intermediary CRCs */
+   /* so let's return just the last one */
+   *out_crc = malloc(sizeof(igt_crc_t));
+   memcpy(*out_crc, [n-1], sizeof(igt_crc_t));
+   free(crcs);
 }
 
 static void
@@ -252,7 +257,8 @@ test_plane_position_with_output(data_t *data, enum pipe 
pipe,
 
check_mode(_lowres, mode2);
 
-   display_commit_mode(>display, pipe_crc, pipe, flags, crc_lowres);
+   display_commit_mode(>display, pipe_crc, pipe, flags, _lowres);
+   free(crc_lowres);
 
igt_assert_plane_visible(data->drm_fd, pipe, false);
 
@@ -264,10 +270,15 @@ test_plane_position_with_output(data_t *data, enum pipe 
pipe,
 
check_mode(mode1, mode3);
 
-   display_commit_mode(>display, pipe_crc, pipe, flags, crc_hires2);
+   display_commit_mode(>display, pipe_crc, pipe, flags, _hires2);
 
igt_assert_plane_visible(data->drm_fd, pipe, true);
 
+   igt_assert_crc_equal(crc_hires1, crc_hires2);
+
+   free(crc_hires1);
+   free(crc_hires2);
+
igt_pipe_crc_stop(pipe_crc);
igt_pipe_crc_free(pipe_crc);
 
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] igt: Make dependency on libunwind mandatory

[Arkadiusz Hiler]

On Fri, Dec 01, 2017 at 01:46:39PM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2017-12-01 13:19:54)
> > With Android support gone there is not much reason for keeping libunwind
> > dependency optional. This also deals (cheaply!) with ifdefs covering
> > huge portions of code, removing a placement minefield.
> 
> Could also force building with frame-pointers? I'm wonder if that would
> help with the less-than-useful stacktraces I get...
> -Chris

Give it a shot. Spin a series forcing FPs through the CI.

Thanks for the review, merged with the unnecessary newline removed.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] igt: Make dependency on libunwind mandatory

[Arkadiusz Hiler]

With Android support gone there is not much reason for keeping libunwind
dependency optional. This also deals (cheaply!) with ifdefs covering
huge portions of code, removing a placement minefield.

Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 configure.ac   | 12 ++--
 lib/igt_core.c | 13 -
 meson.build|  2 +-
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/configure.ac b/configure.ac
index 84c6e646..6908f742 100644
--- a/configure.ac
+++ b/configure.ac
@@ -124,8 +124,10 @@ PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.82])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
 PKG_CHECK_MODULES(PROCPS, [libprocps])
+PKG_CHECK_MODULES(LIBUNWIND, [libunwind])
 PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], 
[have_valgrind=no])
 
+
 if test x$have_valgrind = xyes; then
AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
 fi
@@ -330,15 +332,6 @@ AM_CONDITIONAL(BUILD_SHADER_DEBUGGER, [test 
"x$BUILD_SHADER_DEBUGGER" != xno])
 AS_IF([test "x$BUILD_SHADER_DEBUGGER" != xno],
   [enable_debugger=yes], [enable_debugger=no])
 
-AC_ARG_WITH(libunwind,
-   AS_HELP_STRING([--without-libunwind],
-  [Build tests without libunwind support]),
-   [], [with_libunwind=yes])
-if test "x$with_libunwind" = xyes; then
-   PKG_CHECK_MODULES(LIBUNWIND, libunwind, AC_DEFINE(HAVE_LIBUNWIND, 1, 
[libunwind support]),
- AC_MSG_ERROR([libunwind not found. Use 
--without-libunwind to disable libunwind support.]))
-fi
-
 # enable debug symbols
 AC_ARG_ENABLE(debug,
  AS_HELP_STRING([--disable-debug],
@@ -434,7 +427,6 @@ echo "   Build tests: ${BUILD_TESTS}"
 echo "   Chamelium tests: ${enable_chamelium}"
 echo "   Audio tests: ${enable_audio}"
 echo "   Compile prime tests: ${NOUVEAU}"
-echo "   Print stack traces : ${with_libunwind}"
 echo "   Debug flags: ${DEBUG_CFLAGS}"
 echo ""
 echo " • Tools:"
diff --git a/lib/igt_core.c b/lib/igt_core.c
index de9269b0..03fa6e4e 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -71,6 +71,9 @@
 #include "igt_sysfs.h"
 #include "igt_rc.h"
 
+#define UNW_LOCAL_ONLY
+#include 
+
 #ifdef HAVE_LIBGEN_H
 #include/* for basename() on Solaris */
 #endif
@@ -1173,10 +1176,6 @@ static void write_stderr(const char *str)
__write_stderr(str, strlen(str));
 }
 
-#ifdef HAVE_LIBUNWIND
-#define UNW_LOCAL_ONLY
-#include 
-
 static void print_backtrace(void)
 {
unw_cursor_t cursor;
@@ -1371,7 +1370,6 @@ static void print_backtrace_sig_safe(void)
 
}
 }
-#endif
 
 void __igt_fail_assert(const char *domain, const char *file, const int line,
   const char *func, const char *assertion,
@@ -1394,9 +1392,7 @@ void __igt_fail_assert(const char *domain, const char 
*file, const int line,
va_end(args);
}
 
-#ifdef HAVE_LIBUNWIND
print_backtrace();
-#endif
 
if (run_under_gdb())
abort();
@@ -1876,9 +1872,8 @@ static void fatal_sig_handler(int sig)
igt_exitcode = 128 + sig;
 
failed_one = true;
-#ifdef HAVE_LIBUNWIND
print_backtrace_sig_safe();
-#endif
+
if (in_subtest)
exit_subtest("CRASH");
}
diff --git a/meson.build b/meson.build
index 8e01b05d..a564893d 100644
--- a/meson.build
+++ b/meson.build
@@ -38,6 +38,7 @@ libdrm_amdgpu = dependency('libdrm_amdgpu', required : false)
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
 libprocps = dependency('libprocps', required : true)
+libunwind = dependency('libunwind', required : true)
 
 valgrind = dependency('valgrind', required : false)
 if valgrind.found()
@@ -56,7 +57,6 @@ if glib.found()
config.set('HAVE_GLIB', 1)
 endif
 
-libunwind = dependency('libunwind')
 gsl = dependency('gsl', required : false)
 alsa = dependency('alsa', required : false)
 
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

[Arkadiusz Hiler]

Compiler complained on crc_lowres and crc_hires2 being uninitialized
and indeed, display_commit_mode() seems to have intention of retruning
the value through the parameter that is only a single pointer.

This couses only the local copy of the pointer, the one inside
display_commit_mode(), to be overwritten.

Let's fix that!

Also add missing hires crc comparison (M. Kahola).

Cc: Mika Kahola <mika.kah...@intel.com>
Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 tests/kms_plane_lowres.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c
index 85d3145d..9cc9724c 100644
--- a/tests/kms_plane_lowres.c
+++ b/tests/kms_plane_lowres.c
@@ -127,7 +127,7 @@ test_fini(data_t *data, igt_output_t *output, enum pipe 
pipe)
 
 static int
 display_commit_mode(igt_display_t *display, igt_pipe_crc_t *pipe_crc,
-   enum pipe pipe, int flags, igt_crc_t *crc)
+   enum pipe pipe, int flags, igt_crc_t **crc)
 {
char buf[256];
struct drm_event *e = (void *)buf;
@@ -148,7 +148,7 @@ display_commit_mode(igt_display_t *display, igt_pipe_crc_t 
*pipe_crc,
igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
igt_reset_timeout();
 
-   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, );
+   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, crc);
igt_assert_eq(n, vblank_stop - vblank_start);
 
return n;
@@ -252,7 +252,8 @@ test_plane_position_with_output(data_t *data, enum pipe 
pipe,
 
check_mode(_lowres, mode2);
 
-   display_commit_mode(>display, pipe_crc, pipe, flags, crc_lowres);
+   display_commit_mode(>display, pipe_crc, pipe, flags, _lowres);
+   free(crc_lowres);
 
igt_assert_plane_visible(data->drm_fd, pipe, false);
 
@@ -264,10 +265,15 @@ test_plane_position_with_output(data_t *data, enum pipe 
pipe,
 
check_mode(mode1, mode3);
 
-   display_commit_mode(>display, pipe_crc, pipe, flags, crc_hires2);
+   display_commit_mode(>display, pipe_crc, pipe, flags, _hires2);
 
igt_assert_plane_visible(data->drm_fd, pipe, true);
 
+   igt_assert_crc_equal(crc_hires1, crc_hires2);
+
+   free(crc_hires1);
+   free(crc_hires2);
+
igt_pipe_crc_stop(pipe_crc);
igt_pipe_crc_free(pipe_crc);
 
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

[Arkadiusz Hiler]

On Wed, Nov 29, 2017 at 12:07:06PM +0100, Daniel Vetter wrote:
> On Fri, Nov 24, 2017 at 05:17:48PM +0200, Arkadiusz Hiler wrote:
> > This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
> > and gets rid of the config option from the meson.build.
> > 
> > It was needed only for the Android support.
> > 
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
> 
> Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> 
> on both patches.
> 
> I think there's a bunch more things that are only optional because of
> Android. Stuff like udev and glib iirc. But maybe we want to keep those,
> to avoid to much pain for the next time around someone wants to implement
> Android support natively.
> -Daniel

Pushed, thanks for Acks and R-bs!

As of further cleanups, there is one really impending on us
- the libunwind one.

We have huge sections of the code wrapped in ifdefs which bit us more
than once - it's easy to misplace code in there, code that should always
be compiled.

This needs a rework, initial ideas is to put all the unwind related mess
into separate file and compile the whole thing conditionally - for
clearer separation.

We would also need "fallback" noop implementation of some of those
functions.

Or we may ask ourself, with Android support gone, is this really worth
it and shouldn't we make libunwind non-optional and just get rid of the
preprocessor macors? :-)

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [QUERY] How many CI mails is too many?

[Arkadiusz Hiler]

On Mon, Nov 27, 2017 at 09:10:37PM +0530, Sagar Arun Kamble wrote:
> I feel we generally tend to ignore the results mails for series that
> we are not actively involved on (although we might be interested in
> series itself). Also if number of revisions some series can undergo is
> high, this tendency can grow.

It is true that I don't care that much about results tied to series I am
not interested in, but I don't find this too distracting. They are
nested nicely in the thread and are also very easy to distinguish
visually.

> How about the option of sending the results mails to only author and
> all committers. Ideally, author should include at lease one committer
> and in that case we can possibly avoid mail to all committers.
> 
> Another option would be no results mails at all and enforce authors to
> ensure all green at
> https://patchwork.freedesktop.org/project/intel-gfx/series/?ordering=-last_updated
> 
> Thanks
> Sagar

In my mind the CI system should complement mailing list, not change it
or require unnecessary, external interactions. By sending the result to
intel-gfx we get the gist of the results here, with the direct links to
the patchwork and intel-gfx-ci grids provided (so no need to hunt for
those).

The results also stores nicely in the online mailing list archives if we
send it to the intel-gfx@fdo.

Searchability and easy categorization is an added bonus if you subscribe
to dozen or so mailing lists.

Cheers,
Arek

> On 11/27/2017 8:24 PM, Arkadiusz Hiler wrote:
> > Hey all,
> > 
> > For some time already CI sends out 1-2 mails per series per (re)run, i.e. 
> > BAT
> > results and "full IGT" results (if BAT has not failed).
> > 
> > Recently we have added 32bit build check, and if that fails it sends out
> > additional mail In-Reply-To the series.
> > 
> > I am working on adding some static checks to the CI (spare and checkpatch 
> > at the
> > moment, more may come in the future), which may generate even more 
> > commotion on
> > the mailing list.
> > 
> > How much of CI noise is too much and how you would like to have the results
> > grouped?
> > 
> > Couple of options to start the discussion:
> > 
> >   1. Group all static checks (and the 32bit build?) into one mail:
> >  - just one additional mail,
> >  - may be hard to read in case of catastrophic failure,
> >  - we can send it only when something actually fails.
> > 
> >   2. Send out the results as a part of BAT results:
> >  - even less noise than (1),
> >  - BAT results already feel cluttered, this may decrease readability.
> > 
> >   3. Have each check as a separate mail, but send it only if the check 
> > fails:
> >  - noisy: may result in many mails, depending how many checks fail,
> >  - easier to read and easier to follow on patchwork.
> > 
> > Any opinions? Any other ideas?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [QUERY] How many CI mails is too many?

[Arkadiusz Hiler]

Hey all,

For some time already CI sends out 1-2 mails per series per (re)run, i.e. BAT
results and "full IGT" results (if BAT has not failed).

Recently we have added 32bit build check, and if that fails it sends out
additional mail In-Reply-To the series.

I am working on adding some static checks to the CI (spare and checkpatch at the
moment, more may come in the future), which may generate even more commotion on
the mailing list.

How much of CI noise is too much and how you would like to have the results
grouped?

Couple of options to start the discussion:

 1. Group all static checks (and the 32bit build?) into one mail:
- just one additional mail,
- may be hard to read in case of catastrophic failure,
- we can send it only when something actually fails.

 2. Send out the results as a part of BAT results:
- even less noise than (1),
- BAT results already feel cluttered, this may decrease readability.

 3. Have each check as a separate mail, but send it only if the check fails:
- noisy: may result in many mails, depending how many checks fail,
- easier to read and easier to follow on patchwork.

Any opinions? Any other ideas?

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/2] Revert "lib/igt_aux: Make procps optional"

[Arkadiusz Hiler]

This reverts commit d7d3f4e87b827152f00bdf89a67871736672b492
and gets rid of the config option from the meson.build.

It was needed only for the Android support.

Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 configure.ac  |  6 +-
 lib/igt_aux.c | 35 +++
 meson.build   |  5 +
 3 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1ac2e8e8..84c6e646 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,11 +123,7 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 PKG_CHECK_MODULES(DRM, [libdrm >= 2.4.82])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
-PKG_CHECK_MODULES(PROCPS, [libprocps], [procps=yes], [procps=no])
-AM_CONDITIONAL(HAVE_PROCPS, [test "x$procps" = xyes])
-if test x"$procps" = xyes; then
-   AC_DEFINE(HAVE_PROCPS,1,[Enable process managment without shelling out])
-fi
+PKG_CHECK_MODULES(PROCPS, [libprocps])
 PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], 
[have_valgrind=no])
 
 if test x$have_valgrind = xyes; then
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index a41ae2f1..e2424109 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -48,7 +48,9 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
+
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -68,10 +70,6 @@
 #include/* for dirname() */
 #endif
 
-#ifdef HAVE_PROCPS
-#include 
-#endif
-
 /**
  * SECTION:igt_aux
  * @short_description: Auxiliary libraries and support functions
@@ -1296,7 +1294,6 @@ void igt_set_module_param_int(const char *name, int val)
  * This function sends the signal @sig for a process found in process table
  * with name @comm.
  */
-#ifdef HAVE_PROCPS
 int igt_terminate_process(int sig, const char *comm)
 {
PROCTAB *proc;
@@ -1321,19 +1318,7 @@ int igt_terminate_process(int sig, const char *comm)
closeproc(proc);
return err;
 }
-#else
-#warning "No procps, using naive implementation of igt_terminate_process"
 
-int igt_terminate_process(int sig, const char *comm)
-{
-   char pkill_cmd[NAME_MAX];
-
-   snprintf(pkill_cmd, sizeof(pkill_cmd), "pkill -x -%d %s", sig, comm);
-   return system(pkill_cmd);
-}
-#endif
-
-#ifdef HAVE_PROCPS
 struct pinfo {
pid_t pid;
const char *comm;
@@ -1515,7 +1500,6 @@ __igt_lsof(const char *dir)
 
closeproc(proc);
 }
-#endif
 
 /**
  * igt_lsof: Lists information about files opened by processes.
@@ -1524,7 +1508,6 @@ __igt_lsof(const char *dir)
  * This function mimics (a restrictive form of) lsof(8), but also shows
  * information about opened fds.
  */
-#ifdef HAVE_PROCPS
 void
 igt_lsof(const char *dpath)
 {
@@ -1549,18 +1532,6 @@ igt_lsof(const char *dpath)
 
free(sanitized);
 }
-#else
-#warning "No procps, using naive implementation of igt_lsof"
-
-void
-igt_lsof(const char *dpath)
-{
-   char lsof_cmd[NAME_MAX];
-
-   snprintf(lsof_cmd, sizeof(lsof_cmd), "lsof +d %s", dpath);
-   system(lsof_cmd);
-}
-#endif
 
 static struct igt_siglatency {
timer_t timer;
diff --git a/meson.build b/meson.build
index 2361866b..8e01b05d 100644
--- a/meson.build
+++ b/meson.build
@@ -37,10 +37,7 @@ libdrm_amdgpu = dependency('libdrm_amdgpu', required : false)
 
 pciaccess = dependency('pciaccess', version : '>=0.10')
 libkmod = dependency('libkmod')
-libprocps = dependency('libprocps', required : false)
-if libprocps.found()
-   config.set('HAVE_PROCPS', 1)
-endif
+libprocps = dependency('libprocps', required : true)
 
 valgrind = dependency('valgrind', required : false)
 if valgrind.found()
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 1/2] igt: Remove Android support

[Arkadiusz Hiler]

This patch gets rid of the Android support, deleting all the hacks and
moving code around to the places it belongs.

Android build is not really maintained properly and rots rather fast.
With recent push for Meson here and Android going for Soong it will only
accelerate.

It's a good time to drop the illusion of providing any support.

Cc: Daniel Vetter <daniel.vet...@intel.com>
Cc: Kalyan Kondapally <kalyan.kondapa...@intel.com>
Cc: Petri Latvala <petri.latv...@intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenb...@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 Android.mk   |  4 --
 assembler/ralloc.c   |  5 ---
 benchmarks/Android.mk| 46 -
 benchmarks/gem_syslatency.c  |  2 -
 lib/Android.mk   | 53 
 lib/drmtest.h| 16 
 lib/igt_aux.c| 95 
 lib/igt_aux.h|  5 ---
 lib/igt_core.c   | 68 ---
 lib/igt_debugfs.c| 25 +---
 lib/igt_fb.h |  7 
 lib/igt_kmod.h   |  4 --
 lib/igt_kms.c| 89 +
 lib/tests/Android.mk | 41 ---
 tests/Android.mk | 83 --
 tests/core_get_client_auth.c |  4 +-
 tests/gem_exec_nop.c |  4 --
 tools/Android.mk | 82 --
 18 files changed, 91 insertions(+), 542 deletions(-)
 delete mode 100644 Android.mk
 delete mode 100644 benchmarks/Android.mk
 delete mode 100644 lib/Android.mk
 delete mode 100644 lib/tests/Android.mk
 delete mode 100644 tests/Android.mk
 delete mode 100644 tools/Android.mk

diff --git a/Android.mk b/Android.mk
deleted file mode 100644
index 3690fc5a..
--- a/Android.mk
+++ /dev/null
@@ -1,4 +0,0 @@
-HAVE_LIBDRM_INTEL := true
-
-include $(call all-named-subdir-makefiles, lib tests tools benchmarks)
-
diff --git a/assembler/ralloc.c b/assembler/ralloc.c
index 59e71c48..69c1da4d 100644
--- a/assembler/ralloc.c
+++ b/assembler/ralloc.c
@@ -28,11 +28,6 @@
 #include 
 #include 
 
-/* Android defines SIZE_MAX in limits.h, instead of the standard stdint.h */
-#ifdef ANDROID
-#include 
-#endif
-
 /* Some versions of MinGW are missing _vscprintf's declaration, although they
  * still provide the symbol in the import library. */
 #ifdef __MINGW32__
diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
deleted file mode 100644
index 4ea275c4..
--- a/benchmarks/Android.mk
+++ /dev/null
@@ -1,46 +0,0 @@
-LOCAL_PATH := $(call my-dir)
-
-include $(LOCAL_PATH)/Makefile.sources
-IGT_LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
-
-##
-
-define add_benchmark
-include $(CLEAR_VARS)
-
-LOCAL_SRC_FILES := $1.c
-
-LOCAL_C_INCLUDES = ${IGT_LOCAL_C_INCLUDES} \
-   $(LOCAL_PATH)/../lib/stubs/drm/
-LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM
-LOCAL_CFLAGS += -DANDROID -UNDEBUG -include "check-ndebug.h"
-LOCAL_CFLAGS += -std=gnu99
-# FIXME: drop once Bionic correctly annotates "noreturn" on pthread_exit
-LOCAL_CFLAGS += -Wno-error=return-type
-# Excessive complaining for established cases. Rely on the Linux version 
warnings.
-LOCAL_CFLAGS += -Wno-sign-compare
-LOCAL_LDFLAGS += -lkmod
-
-LOCAL_MODULE := $1_benchmark
-LOCAL_MODULE_TAGS := optional
-LOCAL_MODULE_PATH := 
$(TARGET_OUT_VENDOR)/intel/validation/core/igt/benchmarks
-
-LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
-
-LOCAL_SHARED_LIBRARIES := libpciaccess  \
-  libkmod   \
-  libdrm\
-  libdrm_intel
-
-include $(BUILD_EXECUTABLE)
-endef
-
-##
-
-benchmark_list := $(benchmarks_prog_list)
-
-ifeq ($(HAVE_LIBDRM_INTEL),true)
-benchmark_list += $(LIBDRM_INTEL_BENCHMARKS)
-endif
-
-$(foreach item,$(benchmark_list),$(eval $(call add_benchmark,$(item
diff --git a/benchmarks/gem_syslatency.c b/benchmarks/gem_syslatency.c
index 580edc5f..de59eaf8 100644
--- a/benchmarks/gem_syslatency.c
+++ b/benchmarks/gem_syslatency.c
@@ -219,7 +219,6 @@ static void *sys_thp_alloc(void *arg)
 static void bind_cpu(pthread_attr_t *attr, int cpu)
 {
 #ifdef __USE_GNU
-#ifndef ANDROID
cpu_set_t mask;
 
if (cpu == -1)
@@ -230,7 +229,6 @@ static void bind_cpu(pthread_attr_t *attr, int cpu)
 
pthread_attr_setaffinity_np(attr, sizeof(mask), );
 #endif
-#endif
 }
 
 static void rtprio(pthread_attr_t *attr, int prio)
diff --git a/lib/Android.mk b/lib/Android.mk
deleted file mode 100644
index 31f88be7..
--- a/lib/Android.mk
+++ /dev/null
@@ -1,53 +0,0 @@
-LOCAL_PATH := $(call my-dir)
-
-GPU_TOOLS_PATH := $(LOCAL_PATH)/..
-IGT_LIB_PATH := $(LOCAL_PATH)
-
-# FIXME: au

Re: [Intel-gfx] [PATCH i-g-t] lib/igt_core: Move write_stderr out of LIBUNWIND ifdef

[Arkadiusz Hiler]

On Fri, Nov 24, 2017 at 10:51:38AM +, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2017-11-24 10:32:07)
> > write_stderr() and __write_stderr() are defined behind ifdef on
> > HAVE_LIBUNWIND, but do no depend on the lib in any way.
> 
> Hmm, we keep getting caught out by large ifdefs. Although I guess
> conditionally enabled code itself is less-than-welcome, we should
> probably aim to make whole C files be conditionally compiled, exporting
> no-op interfaces when not enabled. Just a thought.
> -Chris

Sounds about right, and... unwinding the libunwind mess would be a good
way to start.

Any volunteers? I may be able to give it a shot next week...

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] lib/igt_core: Move write_stderr out of LIBUNWIND ifdef

[Arkadiusz Hiler]

write_stderr() and __write_stderr() are defined behind ifdef on
HAVE_LIBUNWIND, but do no depend on the lib in any way.

fatal_sig_handler() uses those helpers unconditionally.

This patch just moves the code couple of lines up, so the helpers are
always available and do not break build on systems without libunwind.

Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
---
 lib/igt_core.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 4fe48c97..6ce83bec 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1231,6 +1231,16 @@ static bool run_under_gdb(void)
strncmp(basename(buf), "gdb", 3) == 0);
 }
 
+static void __write_stderr(const char *str, size_t len)
+{
+   igt_ignore_warn(write(STDERR_FILENO, str, len));
+}
+
+static void write_stderr(const char *str)
+{
+   __write_stderr(str, strlen(str));
+}
+
 #ifdef HAVE_LIBUNWIND
 #define UNW_LOCAL_ONLY
 #include 
@@ -1407,16 +1417,6 @@ xprintf(const char *fmt, ...)
va_end(ap);
 }
 
-static void __write_stderr(const char *str, size_t len)
-{
-   igt_ignore_warn(write(STDERR_FILENO, str, len));
-}
-
-static void write_stderr(const char *str)
-{
-   __write_stderr(str, strlen(str));
-}
-
 static void print_backtrace_sig_safe(void)
 {
unw_cursor_t cursor;
-- 
2.13.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Avoid enum conversion warning

[Arkadiusz Hiler]

On Thu, Nov 23, 2017 at 08:58:47AM +, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Avoid enum conversion warning
> URL   : https://patchwork.freedesktop.org/series/34269/
> State : failure
> 
> == Summary ==

This change does not apply cleanly on top of the commit below:

> commit ffa17d69cf53bd5af4d7e41900c3ba40f6656e31
> Author: Chris Wilson 
> Date:   Thu Nov 23 07:47:39 2017 +
> 
> drm-tip: 2017y-11m-23d-07h-46m-50s UTC integration manifest

Sorry for confusion.

Typical PEBKAC on my side where I used ">" instead of ">>" which
overwrote the message, instead of appending to it.

Should be fixed from now on.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt] lib: Dump /sys/kernel/debug/suspend_stats after suspend failure

[Arkadiusz Hiler]

On Tue, Nov 14, 2017 at 01:19:01PM +, Chris Wilson wrote:
> I noticed that dpm was storing some information about which phase of
> suspend failed inside suspend_stats. That will be useful to help debug
> such failures, so automatically dump it after suspend fails.
> 
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
>  lib/igt_aux.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index ee53559c..0bcf792c 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -803,10 +803,24 @@ static void suspend_via_rtcwake(enum igt_suspend_state 
> state)
>   snprintf(cmd, sizeof(cmd), "rtcwake -s %d -m %s ",
>delay, suspend_state_name[state]);
>   ret = igt_system(cmd);
> - igt_assert_f(ret == 0,
> -  "rtcwake failed with %i\n"
> -  "Check dmesg for further details.\n",
> -  ret);
> + if (ret) {
> + const char *path = "suspend_stats";
> + char *info;
> + int dir;
> +
> + igt_warn("rtcwake failed with %i\n"
> +  "Check dmesg for further details.\n",
> +  ret);
> +
> + dir = open(igt_debugfs_mount(), O_RDONLY);
> + info = igt_sysfs_get(dir, path);
> + close(dir);
> + if (info) {
> + igt_debug("%s:\n%s\n", path, info);
> + free(info);
> + }
> + }
> + igt_assert_eq(ret, 0);
>  }
>  
>  static void suspend_via_sysfs(int power_dir, enum igt_suspend_state state)

We can see it in action here:
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_496/shard-apl3/igt@gem_...@in-flight-suspend.html

(gem_eio:7686) igt-aux-DEBUG: suspend_stats:
success: 10
fail: 1
failed_freeze: 0
failed_prepare: 0
failed_suspend: 1
failed_suspend_late: 0
failed_suspend_noirq: 0
failed_resume: 0
failed_resume_early: 0
failed_resume_noirq: 0
failures:
  last_failed_dev:  

  last_failed_errno:-16
0
  last_failed_step: suspend


Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
and pushed

-- 
Cheers,
Arek

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/debugfs_test: Pretty print subdirectories

[Arkadiusz Hiler]

On Thu, Nov 02, 2017 at 01:20:26PM +0100, Maarten Lankhorst wrote:
> Instead of:
> 
> (debugfs_test:1499) DEBUG: Reading file "data"
> (debugfs_test:1499) DEBUG: Could not open file "data" with error: 
> Input/output error
> 
> Print:
> 
> (debugfs_test:1360) DEBUG: Entering subdir crtc-2
> (debugfs_test:1360) DEBUG:  Entering subdir crc
> (debugfs_test:1360) DEBUG:  Reading file "data"
> (debugfs_test:1360) DEBUG:  Could not open file "data" with 
> error: Input/output error
> (debugfs_test:1360) DEBUG:  Reading file "control"
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>

Reviewed-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>

and pushed, since the test results are clean on this binary.

Thanks!

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx