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

2021-03-25 Thread 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


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

2021-03-25 Thread 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 5/5] Get rid of GNU Autotools

2021-03-25 Thread 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 3/5] tests: Remove ddx_intel_after_fbdev

2021-03-25 Thread 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 2/5] tests: Build gem_concurrent_all with meson

2021-03-25 Thread 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


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

2019-04-10 Thread 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(&f, 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, &f);
> -
> - *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] [igt-dev] [PATCH i-g-t] lib: Rework __kms_addfb() function

2019-04-10 Thread 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] ✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev6)

2019-04-23 Thread 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: 
0

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

2019-02-18 Thread 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 8/8] kms_fence_pin_leak: Move beneath i915/

2019-02-18 Thread 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/

2019-02-18 Thread 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 i9

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

2019-02-28 Thread 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 v6 0/3] drm & vgaarb: handle vgacon removal in vgaarb.

2019-03-01 Thread 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

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

2019-09-12 Thread 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

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

2019-09-12 Thread 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

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

2019-06-06 Thread 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] [PATCH i-g-t 2/4] gitlab-ci: add libatomic to Fedora docker image

2019-06-06 Thread 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] gitlab-ci: add build for MIPS

2019-06-06 Thread 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] i915/gem_exec_balancer: Fix typo in memcpy

2019-06-07 Thread 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 v2] gitlab-ci: add build for MIPS

2019-06-14 Thread 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 5/5] tests/kms_selftest: Integrate kernel selftest test-drm_modeset

2019-06-24 Thread 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 v3 1/1] gitlab-ci: add build and tests for MIPS

2019-06-27 Thread 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] [igt-dev] [PATCH i-g-t] i915/gem_pread/pwrite: Rename "basic"

2019-06-27 Thread 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] gitlab-ci: add tests for MIPS

2019-06-27 Thread 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] [PATCH i-g-t 5/5] tests/kms_selftest: Integrate kernel selftest test-drm_modeset

2019-07-03 Thread 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&rev=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] ✗ Fi.CI.IGT: failure for series starting with [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites (rev2)

2019-10-14 Thread 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

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)

2019-10-15 Thread 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
> > +++-===---===
> 

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

2019-11-08 Thread 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

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

2019-11-11 Thread 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()

2019-11-19 Thread 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

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

2019-10-25 Thread 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] [PATCH] drm/i915/lmem: add the fake lmem region

2019-10-31 Thread 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

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

2019-11-06 Thread 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

2019-11-06 Thread 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

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

2019-11-07 Thread 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] ✗ Fi.CI.BUILD: warning for Enable second DBuf slice for ICL and TGL (rev13)

2020-01-15 Thread 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] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

2020-03-18 Thread 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] ✗ Fi.CI.DOCS: warning for Consider DBuf bandwidth when calculating CDCLK (rev2)

2020-03-18 Thread 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


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

2020-03-19 Thread 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] [PATCH] drm/i915/display: Increase the DDI idle timeout to 500us

2020-03-19 Thread 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


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

2019-08-22 Thread 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(&i915->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] [igt-dev] [PATCH i-g-t] i915/gem_mmap_gtt: Race mmap offset generation against closure

2019-08-27 Thread 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(&handles[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(&handles[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(&handles[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] drm/i915/mst: fix pipe and vblank enable

2020-02-10 Thread 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] [igt-dev] [RFC PATCH i-g-t v2 1/8] tests/core_hotunplug: Duplicate debug messages in dmesg

2020-07-08 Thread 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 i-g-t 1/2] test/kms_cursor_crc: release old pipe_crc before create a new one

2020-07-15 Thread 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] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank

2020-07-15 Thread 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 0/2] minor improvements to the kms_cursor_crc doc and some comments cleanup

2020-07-15 Thread 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] [CI] PR for TGL DMC v2.06

2020-03-02 Thread 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&since=2020-02-20&name=pull-request-new
> > 
> 
> Hi Petri
> 
> Still needed? According to 
> https://patchwork.freedesktop.org/patch/355624/?series=74048&rev=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] [QUERY] How many CI mails is too many?

2017-11-28 Thread 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


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

2017-11-29 Thread 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 
> 
> Acked-by: Daniel Vetter 
> 
> 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


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

2017-12-01 Thread 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 
Cc: Maarten Lankhorst 
Signed-off-by: Arkadiusz Hiler 
---
 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, &crc);
+   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(&mode_lowres, mode2);
 
-   display_commit_mode(&data->display, pipe_crc, pipe, flags, crc_lowres);
+   display_commit_mode(&data->display, pipe_crc, pipe, flags, &crc_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(&data->display, pipe_crc, pipe, flags, crc_hires2);
+   display_commit_mode(&data->display, pipe_crc, pipe, flags, &crc_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


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

2017-12-01 Thread 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 
Cc: Chris Wilson 
Signed-off-by: Arkadiusz Hiler 
---
 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


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

2017-12-04 Thread 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 v2] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

2017-12-12 Thread 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 
Cc: Maarten Lankhorst 
Signed-off-by: Arkadiusz Hiler 
---
 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, &crc);
+   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, &crcs);
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, &crcs[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(&mode_lowres, mode2);
 
-   display_commit_mode(&data->display, pipe_crc, pipe, flags, crc_lowres);
+   display_commit_mode(&data->display, pipe_crc, pipe, flags, &crc_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(&data->display, pipe_crc, pipe, flags, crc_hires2);
+   display_commit_mode(&data->display, pipe_crc, pipe, flags, &crc_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 v3 i-g-t] test/kms_plane_lowres: Fix display_commit_mode() so it returns the crc

2017-12-12 Thread Arkadiusz Hiler
On Tue, Dec 12, 2017 at 11:59:13AM +0100, Maarten Lankhorst wrote:
> From: 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
> v3: Don't do memory allocations, it's hard. (Maarten)
> 
> Cc: Mika Kahola 
> Cc: Maarten Lankhorst 
> Signed-off-by: Arkadiusz Hiler 
> Signed-off-by: Maarten Lankhorst 
> ---
> 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, &crc);
> + n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, &crcs);
>   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


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

2017-12-12 Thread 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 
> > 
> > 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 
> > Cc: Maarten Lankhorst 
> > Signed-off-by: Arkadiusz Hiler 
> > Signed-off-by: Maarten Lankhorst 
> > ---
> > 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, &crc);
> > +   n = igt_pipe_crc_get_crcs(pipe_crc, vblank_stop - vblank_start, &crcs);
> > 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 
___
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

2017-12-13 Thread 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 
> Signed-off-by: Lucas De Marchi 

---
% (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 
and merged, thanks!
___
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)

2017-12-13 Thread 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 4/4] run-tests.sh: Allow users to override IGT_TEST_ROOT

2017-12-13 Thread Arkadiusz Hiler
On Wed, Dec 13, 2017 at 02:58:16PM +0200, Petri Latvala wrote:
> Signed-off-by: Petri Latvala 
> 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] ✓ Fi.CI.IGT: success for igt/core_suspend: Exercise igt_system_suspend_autoresume() (rev2)

2017-12-15 Thread 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] intel_vbt_decode: Typo fixes

2017-12-18 Thread 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] [PATCH igt v2] igt/gem_linear_blits: Compute GTT size using 4G limit

2017-12-19 Thread 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 
> 
> 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 

- 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] ✗ Fi.CI.IGT: failure for igt/pm_rps: Always allocate spin[0]

2017-12-19 Thread 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(&lh.igt_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 2/2] igt/kms_frontbuffer_tracking: Access via GGTT is not guaranteed to be tracked

2017-12-19 Thread 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 
> 
> Ping?

Reviewed-by: Arkadiusz Hiler 

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] [PATCH igt] lib: Convert sw_sync to use sync_file uapi imported from the kernel

2017-12-27 Thread 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 
Reviewed-by: Arkadiusz Hiler 

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

2018-01-04 Thread 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 
Reviewed-by: Arkadiusz Hiler 

and pushed
___
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

2018-01-04 Thread 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 
Reviewed-by: Arkadiusz Hiler 

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] ✗ Fi.CI.BAT: failure for drm/i915: Call i915_perf_fini() on init_hw error unwind

2018-04-14 Thread 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] ✗ Fi.CI.BAT: failure for linux-next: build failure after merge of the drm-intel tree

2018-05-08 Thread 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


[Intel-gfx] [PATCH i-g-t 00/13] Fix IGTs for Android

2017-04-19 Thread Arkadiusz Hiler
IGTs are broken for Android since the introduction of dependency on procps. Over
time other incompatibilities built up.

I took the liberty to fix some of the issues, workaround couple of others and
blacklist heavily incompatible tools/tests.

It builds on (almost) vanilla AOSP now.

Github: <https://github.com/ivyl/igt/commits/android>
Howto:  <https://gist.github.com/ivyl/1e05af15ae37b575e03dc69e5e2488fc>

DEP1: <https://github.com/ivyl/libkmod-android>
DEP2: <https://github.com/android-ia/external_libpciaccess>

We should include a note on Android compatibility in the README and do
"continuous compilation" of the patches as they arrive on the mailing list,
otherwise this **will get broken again soon**.

This is long as it is, but not complete yet.

Here are some of more obvious TODOs:
 * introduce something like IGT_HAS_CAIRO define for convenience
 * revise igt_kms dependency on cairo and enable everything what is independent
 * revise kms tests and do the above
 * review all things that are disabled on Android and try to enable them
 * do something less ugly with config.h generation on Android

Cc: Antonio Argenziano 
Cc: Vinay Belgaumkar 
Cc: Chris Wilson 
Cc: Robert Foss 

Arkadiusz Hiler (13):
  tests/drm_import_export: Include {i915_,}drm.h properly
  Make conditions on HAVE_UDEV consistent
  lib/igt_aux: Include unistd.h for gettid() on Android
  lib/igt_aux: Make procps optional
  chamelium: Fix build issues on Android
  tools/Android.mk: Add guc_logger and l3_parity skip list
  tests/Android.mk: Add perf to skip list
  benchmarks/Android.mk: Add gem_latency to skip list
  Android.mk: Fix libkmod use
  Android.mk: Filter out *.h from src files
  Android.mk: Use drm stubs
  tools/Android.mk: Fix zlib inclusion
  tests/gem_exec_nop: Disable headless subtest on cairoless Android

 benchmarks/Android.mk  |  9 +++--
 configure.ac   |  6 +-
 demos/Android.mk   |  3 ++-
 lib/Android.mk |  8 +---
 lib/Makefile.am|  7 +++
 lib/Makefile.sources   |  7 ---
 lib/igt.h  |  2 ++
 lib/igt_aux.c  | 26 ++
 lib/igt_aux.h  |  5 +
 lib/igt_chamelium.h|  3 +++
 lib/igt_kmod.h |  4 
 lib/tests/Android.mk   |  4 ++--
 tests/Android.mk   |  9 +
 tests/Makefile.am  |  6 ++
 tests/Makefile.sources |  6 --
 tests/drm_import_export.c  |  4 ++--
 tests/gem_exec_nop.c   |  4 
 tests/testdisplay_hotplug.c|  2 +-
 tools/Android.mk   | 14 +-
 tools/intel_l3_parity.c|  2 +-
 tools/intel_l3_parity.h|  2 +-
 tools/intel_l3_udev_listener.c |  2 +-
 22 files changed, 94 insertions(+), 41 deletions(-)

-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 03/13] lib/igt_aux: Include unistd.h for gettid() on Android

2017-04-19 Thread Arkadiusz Hiler
We define gettid() using syscall() because glibc does not provide a
wrapper.

Android's bionic got the syscall covered though.

Signed-off-by: Arkadiusz Hiler 
---
 lib/igt_aux.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index e62858e..54b9716 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -43,7 +43,12 @@ extern int num_trash_bos;
 #define NSEC_PER_SEC (1000*USEC_PER_SEC)
 
 /* signal interrupt helpers */
+#ifdef ANDROID
+#include  /* on Android bionic has this implemented */
+#else
 #define gettid() syscall(__NR_gettid)
+#endif
+
 #define sigev_notify_thread_id _sigev_un._tid
 
 /* auxialiary igt helpers from igt_aux.c */
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 02/13] Make conditions on HAVE_UDEV consistent

2017-04-19 Thread Arkadiusz Hiler
We have a lot of `#ifdef HAVE_UDEV` and ` #if HAVE_UDEV` all over the
place, but ifdef and if have a slightly different semantics.

Let make it consistent by using #ifdefs only.

Signed-off-by: Arkadiusz Hiler 
---
 lib/igt_aux.c  | 2 +-
 tests/testdisplay_hotplug.c| 2 +-
 tools/intel_l3_parity.c| 2 +-
 tools/intel_l3_parity.h| 2 +-
 tools/intel_l3_udev_listener.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 1222806..2ec6b78 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -388,7 +388,7 @@ void igt_stop_shrink_helper(void)
igt_stop_helper(&shrink_helper);
 }
 
-#if HAVE_UDEV
+#ifdef HAVE_UDEV
 #include 
 
 static struct igt_helper_process hang_detector;
diff --git a/tests/testdisplay_hotplug.c b/tests/testdisplay_hotplug.c
index 3b900ca..cf15511 100644
--- a/tests/testdisplay_hotplug.c
+++ b/tests/testdisplay_hotplug.c
@@ -34,7 +34,7 @@
 #endif
 
 
-#if HAVE_UDEV
+#ifdef HAVE_UDEV
 #include 
 static struct udev_monitor *uevent_monitor;
 static struct udev *udev;
diff --git a/tools/intel_l3_parity.c b/tools/intel_l3_parity.c
index dce7f32..eb00c50 100644
--- a/tools/intel_l3_parity.c
+++ b/tools/intel_l3_parity.c
@@ -42,7 +42,7 @@
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
-#if HAVE_UDEV
+#ifdef HAVE_UDEV
 #include 
 #include 
 #endif
diff --git a/tools/intel_l3_parity.h b/tools/intel_l3_parity.h
index 65697c4..759c4f4 100644
--- a/tools/intel_l3_parity.h
+++ b/tools/intel_l3_parity.h
@@ -18,7 +18,7 @@ struct l3_location {
uint8_t subbank;
 };
 
-#if HAVE_UDEV
+#ifdef HAVE_UDEV
 int l3_uevent_setup(struct l3_parity *par);
 /* Listens (blocks) for an l3 parity event. Returns the location of the error. 
*/
 int l3_listen(struct l3_parity *par, bool daemon, struct l3_location *loc);
diff --git a/tools/intel_l3_udev_listener.c b/tools/intel_l3_udev_listener.c
index 0b94c1c..270bfff 100644
--- a/tools/intel_l3_udev_listener.c
+++ b/tools/intel_l3_udev_listener.c
@@ -25,7 +25,7 @@
 #include "config.h"
 #endif
 
-#if HAVE_UDEV
+#ifdef HAVE_UDEV
 #include 
 #ifndef _GNU_SOURCE
 #define _GNU_SOURCE
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 01/13] tests/drm_import_export: Include {i915_, }drm.h properly

2017-04-19 Thread Arkadiusz Hiler
Using `libdrm/` impairs compatibility with android build system and
pkg-config manages -I for us on regular distros.

Signed-off-by: Arkadiusz Hiler 
---
 tests/drm_import_export.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
index cfe5f6d..f1234bd 100644
--- a/tests/drm_import_export.c
+++ b/tests/drm_import_export.c
@@ -32,8 +32,8 @@
 #include 
 #include 
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 06/13] tools/Android.mk: Add guc_logger and l3_parity skip list

2017-04-19 Thread Arkadiusz Hiler
Those tools does not build on Android due to "Linux-only" dependencies.

Let's blacklist them for now.

Signed-off-by: Arkadiusz Hiler 
---
 tools/Android.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/Android.mk b/tools/Android.mk
index 6cdedeb..0602e8c 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -59,6 +59,8 @@ bin_PROGRAMS := $(tools_prog_lists)
 
 skip_tools_list := \
 intel_framebuffer_dump \
+intel_guc_logger \
+intel_l3_parity \
 intel_reg_dumper \
 intel_vga_read \
 intel_vga_write
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 10/13] Android.mk: Filter out *.h from src files

2017-04-19 Thread Arkadiusz Hiler
Newer Android's build system complains about unused files if we leave
those there.

Signed-off-by: Arkadiusz Hiler 
---
 lib/Android.mk   | 2 +-
 tools/Android.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Android.mk b/lib/Android.mk
index 0596e4a..003ade5 100644
--- a/lib/Android.mk
+++ b/lib/Android.mk
@@ -45,7 +45,7 @@ skip_lib_list := \
 -DANDROID_HAS_CAIRO=0
 endif
 
-LOCAL_SRC_FILES := $(filter-out $(skip_lib_list),$(lib_source_list))
+LOCAL_SRC_FILES := $(filter-out %.h $(skip_lib_list),$(lib_source_list))
 
 include $(BUILD_STATIC_LIBRARY)
 
diff --git a/tools/Android.mk b/tools/Android.mk
index a8e649b..0bad29c 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -12,7 +12,7 @@ define add_tool
 ifeq ($($(1)_SOURCES),)
 LOCAL_SRC_FILES := $1.c
 else
-LOCAL_SRC_FILES := $($(1)_SOURCES)
+LOCAL_SRC_FILES := $(filter-out %.h,$($(1)_SOURCES))
 endif
 
 LOCAL_CFLAGS += -DHAVE_TERMIOS_H
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 13/13] tests/gem_exec_nop: Disable headless subtest on cairoless Android

2017-04-19 Thread Arkadiusz Hiler
Currently whole igt_kms.c is disabled while compiling on Android without
cairo, so this tests does not compile.

There should be cleaner a way to disable only cairo dependant parts
which should allow us to enable at least some of the KMS tests, but
that's a bigger rework for another time.

Signed-off-by: Arkadiusz Hiler 
---
 lib/Android.mk   | 1 +
 tests/gem_exec_nop.c | 4 
 2 files changed, 5 insertions(+)

diff --git a/lib/Android.mk b/lib/Android.mk
index 31f88be..dc538b8 100644
--- a/lib/Android.mk
+++ b/lib/Android.mk
@@ -38,6 +38,7 @@ ifeq ("${ANDROID_HAS_CAIRO}", "1")
 LOCAL_C_INCLUDES += $(ANDROID_BUILD_TOP)/external/cairo-1.12.16/src
 LOCAL_CFLAGS += -DANDROID_HAS_CAIRO=1 -DIGT_DATADIR=\".\" 
-DIGT_SRCDIR=\".\"
 else
+
 skip_lib_list := \
 igt_kms.c \
 igt_kms.h \
diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 66c2fc1..967caef 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -138,6 +138,7 @@ stable_nop_on_ring(int fd, uint32_t handle, unsigned int 
engine,
return n;
 }
 
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
 #define assert_within_epsilon(x, ref, tolerance) \
 igt_assert_f((x) <= (1.0 + tolerance) * ref && \
  (x) >= (1.0 - tolerance) * ref, \
@@ -178,6 +179,7 @@ static void headless(int fd, uint32_t handle)
/* check that the two execution speeds are roughly the same */
assert_within_epsilon(n_headless, n_display, 0.1f);
 }
+#endif
 
 static bool ignore_engine(int fd, unsigned engine)
 {
@@ -561,8 +563,10 @@ igt_main
igt_subtest("context-sequential")
sequential(device, handle, FORKED | CONTEXT, 150);
 
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
igt_subtest("headless")
headless(device, handle);
+#endif
 
igt_fixture {
igt_stop_hang_detector();
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 11/13] Android.mk: Use drm stubs

2017-04-19 Thread Arkadiusz Hiler
Use drm stubs that sit under lib/stubs.

Also drop strange, nonexistent additions to LOCAL_C_INCLUDES.

Signed-off-by: Arkadiusz Hiler 
---
 benchmarks/Android.mk | 3 ++-
 demos/Android.mk  | 3 ++-
 lib/Android.mk| 4 ++--
 lib/tests/Android.mk  | 4 ++--
 tests/Android.mk  | 4 ++--
 tools/Android.mk  | 2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
index fa9eec6..a4f1858 100644
--- a/benchmarks/Android.mk
+++ b/benchmarks/Android.mk
@@ -10,7 +10,8 @@ define add_benchmark
 
 LOCAL_SRC_FILES := $1.c
 
-LOCAL_C_INCLUDES = ${IGT_LOCAL_C_INCLUDES}
+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
diff --git a/demos/Android.mk b/demos/Android.mk
index 90d8b37..1f50fdc 100644
--- a/demos/Android.mk
+++ b/demos/Android.mk
@@ -16,7 +16,8 @@ LOCAL_CFLAGS += -std=gnu99
 # Excessive complaining for established cases. Rely on the Linux version 
warnings.
 LOCAL_CFLAGS += -Wno-sign-compare
 
-LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
+LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib \
+   $(LOCAL_PATH)/../lib/stubs/drm/
 
 LOCAL_MODULE := intel_sprite_on
 
diff --git a/lib/Android.mk b/lib/Android.mk
index 003ade5..31f88be 100644
--- a/lib/Android.mk
+++ b/lib/Android.mk
@@ -17,8 +17,8 @@ LOCAL_GENERATED_SOURCES :=   \
$(IGT_LIB_PATH)/version.h  \
$(GPU_TOOLS_PATH)/config.h
 
-LOCAL_C_INCLUDES +=  \
-   $(LOCAL_PATH)/..
+LOCAL_C_INCLUDES += $(LOCAL_PATH)/.. \
+$(LOCAL_PATH)/stubs/drm/
 
 LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)
 
diff --git a/lib/tests/Android.mk b/lib/tests/Android.mk
index 026f17f..bbbd4d8 100644
--- a/lib/tests/Android.mk
+++ b/lib/tests/Android.mk
@@ -30,8 +30,8 @@ IGT_LOCAL_CFLAGS += -std=gnu99
 IGT_LOCAL_CFLAGS += -Wno-error=return-type
 
 # set local includes
-IGT_LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
-IGT_LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm
+IGT_LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib \
+   $(LOCAL_PATH)/../lib/stubs/drm/
 
 # set local libraries
 IGT_LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
diff --git a/tests/Android.mk b/tests/Android.mk
index b664dff..c6e966f 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -40,8 +40,8 @@ IGT_LOCAL_CFLAGS += -Wno-error=return-type
 IGT_LOCAL_CFLAGS += -Wno-sign-compare
 
 # set local includes
-IGT_LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
-IGT_LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm
+IGT_LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib \
+   $(LOCAL_PATH)/../lib/stubs/drm/
 
 # set local libraries
 IGT_LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
diff --git a/tools/Android.mk b/tools/Android.mk
index 0bad29c..5653572 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -30,7 +30,7 @@ define add_tool
 endif
 
 LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
-LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm
+LOCAL_C_INCLUDES += $(LOCAL_PATH)/../lib/stubs/drm/
 LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/zlib
 
 LOCAL_MODULE := $1_tool
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 12/13] tools/Android.mk: Fix zlib inclusion

2017-04-19 Thread Arkadiusz Hiler
Add dependency on libz instead of doing path magic.

Signed-off-by: Arkadiusz Hiler 
---
 tools/Android.mk | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/Android.mk b/tools/Android.mk
index 5653572..96075c9 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -29,9 +29,8 @@ define add_tool
 LOCAL_LDFLAGS += $($(1)_LDFLAGS)
 endif
 
-LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
-LOCAL_C_INCLUDES += $(LOCAL_PATH)/../lib/stubs/drm/
-LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/zlib
+LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib \
+   $(LOCAL_PATH)/../lib/stubs/drm/
 
 LOCAL_MODULE := $1_tool
 LOCAL_MODULE_TAGS := optional
@@ -41,7 +40,8 @@ define add_tool
 LOCAL_SHARED_LIBRARIES := libpciaccess  \
   libkmod   \
   libdrm\
-  libdrm_intel
+  libdrm_intel \
+  libz
 
 # Tools dir on host
 LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/$(LOCAL_TOOLS_DIR)
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 08/13] benchmarks/Android.mk: Add gem_latency to skip list

2017-04-19 Thread Arkadiusz Hiler
AOSP, as of this commit, does not include libdrm with fence defines.

Signed-off-by: Arkadiusz Hiler 
---
 benchmarks/Android.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
index c0fa09f..a790ec7 100644
--- a/benchmarks/Android.mk
+++ b/benchmarks/Android.mk
@@ -34,7 +34,9 @@ endef
 
 ##
 
-benchmark_list := $(benchmarks_prog_list)
+skip_benchmark_list = gem_latency
+
+benchmark_list := $(filter-out $(skip_benchmark_list),$(benchmarks_prog_list))
 
 ifeq ($(HAVE_LIBDRM_INTEL),true)
 benchmark_list += $(LIBDRM_INTEL_BENCHMARKS)
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 09/13] Android.mk: Fix libkmod use

2017-04-19 Thread Arkadiusz Hiler
On Android libkmod.h is nested under libkmod directory, so we should
include appropriately.

Also we need to link with it.

Signed-off-by: Arkadiusz Hiler 
---
 benchmarks/Android.mk | 2 ++
 lib/Android.mk| 1 +
 lib/igt_kmod.h| 4 
 tests/Android.mk  | 4 ++--
 tools/Android.mk  | 2 ++
 5 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk
index a790ec7..fa9eec6 100644
--- a/benchmarks/Android.mk
+++ b/benchmarks/Android.mk
@@ -18,6 +18,7 @@ define add_benchmark
 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
@@ -26,6 +27,7 @@ define add_benchmark
 LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
 
 LOCAL_SHARED_LIBRARIES := libpciaccess  \
+  libkmod   \
   libdrm\
   libdrm_intel
 
diff --git a/lib/Android.mk b/lib/Android.mk
index eb72f84..0596e4a 100644
--- a/lib/Android.mk
+++ b/lib/Android.mk
@@ -29,6 +29,7 @@ LOCAL_CFLAGS += -std=gnu99 -UNDEBUG
 LOCAL_MODULE:= libintel_gpu_tools
 
 LOCAL_SHARED_LIBRARIES := libpciaccess  \
+ libkmod   \
  libdrm\
  libdrm_intel
 
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index fd307a4..6a7584f 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -24,7 +24,11 @@
 #ifndef IGT_KMOD_H
 #define IGT_KMOD_H
 
+#ifdef ANDROID
+#include 
+#else
 #include 
+#endif
 
 #include "igt_aux.h"
 
diff --git a/tests/Android.mk b/tests/Android.mk
index c67ddbd..b664dff 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -19,7 +19,7 @@ define add_test
 
 LOCAL_MODULE_TAGS := optional
 # ask linker to define a specific symbol; we use this to identify IGT tests
-LOCAL_LDFLAGS := -Wl,--defsym=$2=0
+LOCAL_LDFLAGS := -Wl,--defsym=$2=0 -lkmod
 LOCAL_MODULE_PATH := $(TARGET_OUT_VENDOR)/intel/validation/core/igt
 
 include $(BUILD_EXECUTABLE)
@@ -45,7 +45,7 @@ IGT_LOCAL_C_INCLUDES += 
${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm
 
 # set local libraries
 IGT_LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
-IGT_LOCAL_SHARED_LIBRARIES := libpciaccess libdrm libdrm_intel
+IGT_LOCAL_SHARED_LIBRARIES := libpciaccess libkmod libdrm libdrm_intel
 
 # handle cairo requirements if it is enabled
 ifeq ("${ANDROID_HAS_CAIRO}", "1")
diff --git a/tools/Android.mk b/tools/Android.mk
index 0602e8c..a8e649b 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -23,6 +23,7 @@ define add_tool
 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
 ifeq ($($(1)_LDFLAGS),)
 else
 LOCAL_LDFLAGS += $($(1)_LDFLAGS)
@@ -38,6 +39,7 @@ define add_tool
 LOCAL_STATIC_LIBRARIES := libintel_gpu_tools
 
 LOCAL_SHARED_LIBRARIES := libpciaccess  \
+  libkmod   \
   libdrm\
   libdrm_intel
 
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 05/13] chamelium: Fix build issues on Android

2017-04-19 Thread Arkadiusz Hiler
Makefile.sources are included 1:1 in Android.mk files, and are not
parsed by automake. And yet those had some automake conditional logic.
Moving it to .am file is enough for now.

Also igt_chamelium.h included config.h without proper "HAVE_CONFIG_H"
guard, and the file itself was included unconditionally.

Signed-off-by: Arkadiusz Hiler 
---
 lib/Makefile.am| 7 +++
 lib/Makefile.sources   | 7 ---
 lib/igt.h  | 2 ++
 lib/igt_chamelium.h| 3 +++
 tests/Makefile.am  | 6 ++
 tests/Makefile.sources | 6 --
 6 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index c0ddf29..91e72c4 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -22,6 +22,13 @@ if !HAVE_LIBDRM_INTEL
 stubs/drm/intel_bufmgr.h
 endif
 
+if HAVE_CHAMELIUM
+lib_source_list += \
+   igt_chamelium.h \
+   igt_chamelium.c \
+   $(NULL)
+endif
+
 AM_CPPFLAGS = -I$(top_srcdir)
 AM_CFLAGS = \
$(CWARNFLAGS) \
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 6348487..53fdb54 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -85,13 +85,6 @@ lib_source_list =\
igt_kmod.h  \
$(NULL)
 
-if HAVE_CHAMELIUM
-lib_source_list += \
-   igt_chamelium.h \
-   igt_chamelium.c \
-   $(NULL)
-endif
-
 .PHONY: version.h.tmp
 
 # leaving a space here to work around automake's conditionals
diff --git a/lib/igt.h b/lib/igt.h
index a97923e..a069deb 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -38,7 +38,9 @@
 #include "igt_kms.h"
 #include "igt_pm.h"
 #include "igt_stats.h"
+#ifdef HAVE_CHAMELIUM
 #include "igt_chamelium.h"
+#endif
 #include "instdone.h"
 #include "intel_batchbuffer.h"
 #include "intel_chipset.h"
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index f421d83..15f6024 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -26,7 +26,10 @@
 #ifndef IGT_CHAMELIUM_H
 #define IGT_CHAMELIUM_H
 
+#ifdef HAVE_CONFIG_H
 #include "config.h"
+#endif
+
 #include "igt.h"
 #include 
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8930c24..f2358d5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -8,6 +8,12 @@ if HAVE_LIBDRM_VC4
 TESTS_progs_M += $(VC4_TESTS_M)
 endif
 
+if HAVE_CHAMELIUM
+TESTS_progs_M += \
+   chamelium \
+   $(NULL)
+endif
+
 if BUILD_TESTS
 test-list.txt: Makefile.sources
@echo TESTLIST > $@
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 7fa9b8f..3f10cd2 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -146,12 +146,6 @@ TESTS_progs_M = \
meta_test \
$(NULL)
 
-if HAVE_CHAMELIUM
-TESTS_progs_M += \
-   chamelium \
-   $(NULL)
-endif
-
 TESTS_progs_XM = \
 gem_concurrent_all \
 $(NULL)
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 04/13] lib/igt_aux: Make procps optional

2017-04-19 Thread Arkadiusz Hiler
Android does not have procps and it's not easy to compile it as a
dependency.

We can provide alternative, "naive" implementation that just shells out
to external commands (i.e. pkill and lsof) in case we do not have the
library.

Signed-off-by: Arkadiusz Hiler 
---
 configure.ac  |  6 +-
 lib/igt_aux.c | 24 +---
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 12b0ab0..a5a92d5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,7 +123,11 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 PKG_CHECK_MODULES(DRM, [libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
-PKG_CHECK_MODULES(PROCPS, [libprocps])
+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(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 2ec6b78..4efb45b 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -50,9 +50,7 @@
 #include 
 #include 
 #include 
-
-#include 
-
+#include 
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -71,6 +69,10 @@
 #include/* for dirname() */
 #endif
 
+#ifdef HAVE_PROCPS
+#include 
+#endif
+
 /**
  * SECTION:igt_aux
  * @short_description: Auxiliary libraries and support functions
@@ -1295,6 +1297,7 @@ void igt_set_module_param_int(const char *name, int val)
  */
 int igt_terminate_process(int sig, const char *comm)
 {
+#ifdef HAVE_PROCPS
PROCTAB *proc;
proc_t *proc_info;
int err = 0;
@@ -1316,6 +1319,12 @@ int igt_terminate_process(int sig, const char *comm)
 
closeproc(proc);
return err;
+#else
+#warning "No procps, using naive implementation of igt_terminate_process"
+   char pkill_cmd[NAME_MAX];
+   snprintf(pkill_cmd, sizeof(pkill_cmd), "pkill -x -%d %s", sig, comm);
+   return system(pkill_cmd);
+#endif
 }
 
 struct pinfo {
@@ -1324,6 +1333,7 @@ struct pinfo {
const char *fn;
 };
 
+#ifdef HAVE_PROCPS
 static void
 __igt_show_stat(struct pinfo *info)
 {
@@ -1499,6 +1509,7 @@ __igt_lsof(const char *dir)
 
closeproc(proc);
 }
+#endif
 
 /**
  * igt_lsof: Lists information about files opened by processes.
@@ -1510,6 +1521,7 @@ __igt_lsof(const char *dir)
 void
 igt_lsof(const char *dpath)
 {
+#ifdef HAVE_PROCPS
struct stat st;
size_t len = strlen(dpath);
char *sanitized;
@@ -1530,6 +1542,12 @@ igt_lsof(const char *dpath)
__igt_lsof(sanitized);
 
free(sanitized);
+#else
+#warning "No procps, using naive implementation of igt_lsof"
+   char lsof_cmd[NAME_MAX];
+   snprintf(lsof_cmd, sizeof(lsof_cmd), "lsof +d %s", dpath);
+   system(lsof_cmd);
+#endif
 }
 
 static struct igt_siglatency {
-- 
2.9.3

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


[Intel-gfx] [PATCH i-g-t 07/13] tests/Android.mk: Add perf to skip list

2017-04-19 Thread Arkadiusz Hiler
It does not build on Android with top libdrm.

Temporary hax.

Signed-off-by: Arkadiusz Hiler 
---
 tests/Android.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/Android.mk b/tests/Android.mk
index 3186a2a..c67ddbd 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -59,6 +59,7 @@ else
 pm_lpsp \
drm_read \
gem_exec_blt \
+   perf \
prime_mmap_kms
 
 # All kms tests depend on cairo
-- 
2.9.3

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


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

2017-04-19 Thread Arkadiusz Hiler
Android does not have procps and it's not easy to compile it as a
dependency.

We can provide alternative, "naive" implementation that just shells out
to external commands (i.e. pkill and lsof) in case we do not have the
library.

v2: do not ifdef insides of functions (J. Nikula)

Cc: Jani Nikula 
Signed-off-by: Arkadiusz Hiler 
---
 configure.ac  |  6 +-
 lib/igt_aux.c | 32 +---
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 12b0ab0..a5a92d5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -123,7 +123,11 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 PKG_CHECK_MODULES(DRM, [libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
-PKG_CHECK_MODULES(PROCPS, [libprocps])
+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(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 2ec6b78..3695fa5 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -50,9 +50,7 @@
 #include 
 #include 
 #include 
-
-#include 
-
+#include 
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -71,6 +69,10 @@
 #include/* for dirname() */
 #endif
 
+#ifdef HAVE_PROCPS
+#include 
+#endif
+
 /**
  * SECTION:igt_aux
  * @short_description: Auxiliary libraries and support functions
@@ -1281,6 +1283,7 @@ void igt_set_module_param_int(const char *name, int val)
igt_set_module_param(name, str);
 }
 
+#ifdef HAVE_PROCPS
 /**
  * igt_terminate_process:
  * @sig: Signal to send
@@ -1317,7 +1320,18 @@ int igt_terminate_process(int sig, const char *comm)
closeproc(proc);
return err;
 }
+#else /* HAVE_PROCPS */
+#warning "No procps, using naive implementatio 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 /* HAVE_PROCPS */
+
+#ifdef HAVE_PROCPS
 struct pinfo {
pid_t pid;
const char *comm;
@@ -1531,6 +1545,18 @@ igt_lsof(const char *dpath)
 
free(sanitized);
 }
+#else /* HAVE_PROCPS */
+#warning "No procps, using naive implementatio 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 /* HAVE_PROCPS */
 
 static struct igt_siglatency {
timer_t timer;
-- 
2.9.3

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


Re: [Intel-gfx] [PATCH i-g-t 03/13] lib/igt_aux: Include unistd.h for gettid() on Android

2017-04-19 Thread Arkadiusz Hiler
On Wed, Apr 19, 2017 at 03:22:19PM +0300, Jani Nikula wrote:
> On Wed, 19 Apr 2017, Arkadiusz Hiler  wrote:
> > We define gettid() using syscall() because glibc does not provide a
> > wrapper.
> >
> > Android's bionic got the syscall covered though.
> >
> > Signed-off-by: Arkadiusz Hiler 
> > ---
> >  lib/igt_aux.h | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index e62858e..54b9716 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -43,7 +43,12 @@ extern int num_trash_bos;
> >  #define NSEC_PER_SEC (1000*USEC_PER_SEC)
> >  
> >  /* signal interrupt helpers */
> > +#ifdef ANDROID
> 
> Seems like this should be something like HAVE_GETTID, defined by
> configure or by android makefiles?

Yeah, but that's not that easy (yet) and that's not the only area which
would use it - notice the thing with cairo from the cover letter.

config.h is generated in a ugly way for Android - lib/Android.mk does
that. Also if you ./configure it stops compiling for Android causing
confusing error.

Whole area could use some heavy reworking.

One approach would be to mimic what other projects do:

 * have config_android.h with set of sane #defines (as environment is
   more static and there is no ac/am)

 * don't define HAVE_CONFIG_H and just `-include config_android.h` on...
   Android

 * add gettid() discovery via ac for Linux (I don't think that any
   libc other than bionic wraps that call though)


But that would made a whole series.
I would like to go with #ifdef ANDROID for now.

-- 
Cheers,
Arek

> > +#include  /* on Android bionic has this implemented */
> > +#else
> >  #define gettid() syscall(__NR_gettid)
> > +#endif
> > +
> >  #define sigev_notify_thread_id _sigev_un._tid
> >  
> >  /* auxialiary igt helpers from igt_aux.c */
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t 08/13] benchmarks/Android.mk: Add gem_latency to skip list

2017-04-20 Thread Arkadiusz Hiler
On Wed, Apr 19, 2017 at 05:58:28PM +0100, Chris Wilson wrote:
> On Wed, Apr 19, 2017 at 01:01:50PM +0200, Arkadiusz Hiler wrote:
> > AOSP, as of this commit, does not include libdrm with fence defines.
> 
> Pushed local defines that should keep the benchmark happy.
> 
> Please do reset the configure libdrm requirements to what is available
> on min(Android, debian stable). They should be our compile targets.
> -Chris

Series rebased (sans this patch) cleanly on current master.

Everything compiles just fine :-)

Thanks!

-- 
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 03/13] lib/igt_aux: Include unistd.h for gettid() on Android

2017-04-20 Thread Arkadiusz Hiler
On Wed, Apr 19, 2017 at 05:23:46PM +0300, Jani Nikula wrote:
> On Wed, 19 Apr 2017, Arkadiusz Hiler  wrote:
> > On Wed, Apr 19, 2017 at 03:22:19PM +0300, Jani Nikula wrote:
> >> On Wed, 19 Apr 2017, Arkadiusz Hiler  wrote:
> >> > We define gettid() using syscall() because glibc does not provide a
> >> > wrapper.
> >> >
> >> > Android's bionic got the syscall covered though.
> >> >
> >> > Signed-off-by: Arkadiusz Hiler 
> >> > ---
> >> >  lib/igt_aux.h | 5 +
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> >> > index e62858e..54b9716 100644
> >> > --- a/lib/igt_aux.h
> >> > +++ b/lib/igt_aux.h
> >> > @@ -43,7 +43,12 @@ extern int num_trash_bos;
> >> >  #define NSEC_PER_SEC (1000*USEC_PER_SEC)
> >> >  
> >> >  /* signal interrupt helpers */
> >> > +#ifdef ANDROID
> >> 
> >> Seems like this should be something like HAVE_GETTID, defined by
> >> configure or by android makefiles?
> >
> > Yeah, but that's not that easy (yet) and that's not the only area which
> > would use it - notice the thing with cairo from the cover letter.
> >
> > config.h is generated in a ugly way for Android - lib/Android.mk does
> > that. Also if you ./configure it stops compiling for Android causing
> > confusing error.
> >
> > Whole area could use some heavy reworking.
> >
> > One approach would be to mimic what other projects do:
> >
> >  * have config_android.h with set of sane #defines (as environment is
> >more static and there is no ac/am)
> >
> >  * don't define HAVE_CONFIG_H and just `-include config_android.h` on...
> >Android
> >
> >  * add gettid() discovery via ac for Linux (I don't think that any
> >libc other than bionic wraps that call though)
> >
> >
> > But that would made a whole series.
> > I would like to go with #ifdef ANDROID for now.
> 
> Fair enough.
> 
> It's just that I cringe seeing #ifdef  instead of #ifdef
> , when  and  are not interchangeable or
> 1:1. For example, glibc might include gettid later.
> 
> BR,
> Jani.

I completetly get that, I had mixed feeling adding the ifdef.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Update MOCS settings for gen 9

2017-04-27 Thread Arkadiusz Hiler
On Wed, Apr 26, 2017 at 06:00:41PM +0300, David Weinehall wrote:
> Add a bunch of MOCS entries for gen 9 that were missing from intel_mocs.
> Some of these are used by media-sdk; if these entries are missing
> the default will instead be to do everything uncached.
> 
> This patch improves media-sdk performance with up to 60%
> with the (admittedly synthetic) benchmarks we use in our nightly
> testing, without regressing any other benchmarks.

Hey David,

I am testing some of the extended MOCS with Mesa and the differences I
see fit in the margins of statistical error.

Odd, I thought, so to make sure I haven't messed up anything in the
process of compiling, setting LD_LIBRARY_PATH and benchmarking I turned
everything to UNCACHED - and I saw severe performance drop.

So here is the question it induced:

Have you used the "closest neighbour" from entries available or did you
defaulted to the UNCACHED ones? That could be the culprit.

Note: I have tested MOCS for VB and Render Target only, and only in a
few synthetic cases - it will require much more fine-tuning and
benchmarking before any final conclusions.

-- 
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: Update MOCS settings for gen 9

2017-05-04 Thread Arkadiusz Hiler
On Thu, Apr 27, 2017 at 05:23:16PM +0100, Chris Wilson wrote:
> On Thu, Apr 27, 2017 at 06:30:42PM +0300, David Weinehall wrote:
> > On Thu, Apr 27, 2017 at 04:55:20PM +0200, Arkadiusz Hiler wrote:
> > > On Wed, Apr 26, 2017 at 06:00:41PM +0300, David Weinehall wrote:
> > > > Add a bunch of MOCS entries for gen 9 that were missing from intel_mocs.
> > > > Some of these are used by media-sdk; if these entries are missing
> > > > the default will instead be to do everything uncached.
> > > > 
> > > > This patch improves media-sdk performance with up to 60%
> > > > with the (admittedly synthetic) benchmarks we use in our nightly
> > > > testing, without regressing any other benchmarks.
> > > 
> > > Hey David,
> > > 
> > > I am testing some of the extended MOCS with Mesa and the differences I
> > > see fit in the margins of statistical error.
> > > 
> > > Odd, I thought, so to make sure I haven't messed up anything in the
> > > process of compiling, setting LD_LIBRARY_PATH and benchmarking I turned
> > > everything to UNCACHED - and I saw severe performance drop.
> > > 
> > > So here is the question it induced:
> > > 
> > > Have you used the "closest neighbour" from entries available or did you
> > > defaulted to the UNCACHED ones? That could be the culprit.
> > > 
> > > Note: I have tested MOCS for VB and Render Target only, and only in a
> > > few synthetic cases - it will require much more fine-tuning and
> > > benchmarking before any final conclusions.
> > 
> > As I mentioned in the commit message, the improvements only manifest
> > themselves for media-sdk workloads (and presumably other workloads
> > that uses the same hardware); if you see any performance regressions
> > with these additional entries I'd be interested to know.
> 
> But what is being counter suggested is that their is no reason for these
> mocs entries. If the sdk is just using mocs registers without first
> programming them outside of the kernel abi, then it will be hitting
> uncached memory - and then the only benefit is from simply enabling
> cached access. The kernel ABI is minimalist for a reason, and we want to
> know why we should be adding tables that we need to maintain forever
> (bonus points for making that a consistent interface for hardware for
> years to come).
> -Chris

Thanks for rephrasing - that's exactly what I am concerned with.

Did you just use the MediaSDK as it is - meaning that MOCS entries
beyond the set of the 3 we have defined had been naively utilized?

If that's the case it is probably the cause of the performance
difference - everything beyond "the 3" means UNCACHED.

Can you try changing MediaSDK to only use entries that are already in?
How the performance differs in that case?

-- 
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: Update MOCS settings for gen 9

2017-05-05 Thread Arkadiusz Hiler
On Thu, May 04, 2017 at 07:46:34PM -0700, Dmitry Rogozhkin wrote:
> 
> 
> On 5/4/2017 9:51 AM, Kenneth Graunke wrote:
> > MediaSDK is not a benchmark.  If I'm not mistaken, it's a userspace
> > driver produced by Intel engineers, one which Intel has the full
> > capability to change.  What you're saying is that Intel's MediaSDK
> > engineers are unwilling to change their software to provide better
> > performance for their Linux users.
> > 
> > That's pretty mental.
> You are mistaken. Media SDK is not a driver. It is a user space library
> which talks to the user space driver. And Media SDK does not set _any_
> caching policies you are discussing here. That's the driver who sets these
> policies. I don't want to go further here who supports this driver, Intel or
> not, but there are mediasdk engineers whom you blame to not willing to do
> something and who actually only indirectly are related to this topic.
> Please, if you mean driver, say a driver.

You are right. It is very unfortunate that those two were confused.

To further clarify, here's an excerpt from MediaSDK's README.md:

"Intel Media SDK depends on a special version of LibVA which comes with
Intel Media Server Studio installation and is not in upstream, so this
version is not compatible with the LibVA/driver available at 01org."


Nonetheless, the main question still stands:

How big is the performance gap when using appropriate, existing entries
entries vs the entries proposed here?


Also a couple more questions arise:

* What about versioning schema? We definitely need a consumer for that.

* The libva you use is a **closed** UMD. It this enough of a reason to
  do the change (sans the versioning)? It starts to get blurry here.

Don't get me wrong, I've seen the spec and believe that a lot of hard
work was put into determining the entries and the usage scenarios, and
that they can benefit us.

I even have some patches ready[0] and I would love to get them upstream,
but they have to be tested in reproducible manner, with clear
methodology. They need to be discussed and deemed useful by providing
real value. We cannot simply relay on opaque claims that they are good
for us.


To do the above I am fiddling with Mesa - if we will see performance
boost, then this would provide both a solid reason to add the entries
and a consumer for the versioning API.


As of proprietary libva/MediaSDK combo I see at least three options to
do "the 3" vs "extended mocs" testing:

1. change the libva you use to utilize only the basic 3 entries only and
   do the testing - if you can change the source

2. change kernel and fill in entries with copies of the 3 basic ones and
   do the testing - will work even without the source code of libva

3. port the MOCS changes to 01org's libva and either use this version
   from MediaSDK for testing or use some other benchmarks -  this could
   be the consumer we need

I hope that this will move us forward.

[0]: https://github.com/ivyl/linux/commits/mocs

-- 
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 05/13] chamelium: Fix build issues on Android

2017-05-05 Thread Arkadiusz Hiler
On Tue, May 02, 2017 at 12:53:16PM +0300, Petri Latvala wrote:
> On Wed, Apr 19, 2017 at 01:01:47PM +0200, Arkadiusz Hiler wrote:
> > Also igt_chamelium.h included config.h without proper "HAVE_CONFIG_H"
> > guard, and the file itself was included unconditionally.
> 
> I see unconditional config.h inclusion in several other places,
> is igt_chamelium.h the only file where it causes problems?

Yes that we the reason for this ifdef, but that might have been fixed in
other way by different patch as I can't reproduce the error anymore.

I'll drop that from this patch and later we can have one dedicated to
adding ifdefs everywhere.

The approach with config_android.h would need that, so that would be
good series to do it.

-- 
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 13/13] tests/gem_exec_nop: Disable headless subtest on cairoless Android

2017-05-05 Thread Arkadiusz Hiler
On Thu, May 04, 2017 at 11:00:33AM +0300, Petri Latvala wrote:
> On Wed, Apr 19, 2017 at 01:01:55PM +0200, Arkadiusz Hiler wrote:
> > Currently whole igt_kms.c is disabled while compiling on Android without
> > cairo, so this tests does not compile.
> > 
> > There should be cleaner a way to disable only cairo dependant parts
> > which should allow us to enable at least some of the KMS tests, but
> > that's a bigger rework for another time.
> > 
> > Signed-off-by: Arkadiusz Hiler 
> > ---
> >  lib/Android.mk   | 1 +
> >  tests/gem_exec_nop.c | 4 
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/lib/Android.mk b/lib/Android.mk
> > index 31f88be..dc538b8 100644
> > --- a/lib/Android.mk
> > +++ b/lib/Android.mk
> > @@ -38,6 +38,7 @@ ifeq ("${ANDROID_HAS_CAIRO}", "1")
> >  LOCAL_C_INCLUDES += $(ANDROID_BUILD_TOP)/external/cairo-1.12.16/src
> >  LOCAL_CFLAGS += -DANDROID_HAS_CAIRO=1 -DIGT_DATADIR=\".\" 
> > -DIGT_SRCDIR=\".\"
> >  else
> > +
> >  skip_lib_list := \
> >  igt_kms.c \
> >  igt_kms.h \
> > diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
> > index 66c2fc1..967caef 100644
> > --- a/tests/gem_exec_nop.c
> > +++ b/tests/gem_exec_nop.c
> > @@ -138,6 +138,7 @@ stable_nop_on_ring(int fd, uint32_t handle, unsigned 
> > int engine,
> > return n;
> >  }
> >  
> > +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
> 
> 
> Tautological check for ANDROID being defined. Is it too confusing to reduce 
> this to
> 
> #if !defined(ANDROID) || ANDROID_HAS_CAIRO

Indeed, thanks.

-- 
Cheers,
Arek

> >  #define assert_within_epsilon(x, ref, tolerance) \
> >  igt_assert_f((x) <= (1.0 + tolerance) * ref && \
> >   (x) >= (1.0 - tolerance) * ref, \
> > @@ -178,6 +179,7 @@ static void headless(int fd, uint32_t handle)
> > /* check that the two execution speeds are roughly the same */
> > assert_within_epsilon(n_headless, n_display, 0.1f);
> >  }
> > +#endif
> >  
> >  static bool ignore_engine(int fd, unsigned engine)
> >  {
> > @@ -561,8 +563,10 @@ igt_main
> > igt_subtest("context-sequential")
> > sequential(device, handle, FORKED | CONTEXT, 150);
> >  
> > +#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
> 
> 
> Likewise.
> 
> 
> 
> 
> --
> Petri Latvala
> 
> 
> 
> 
> 
> > igt_subtest("headless")
> > headless(device, handle);
> > +#endif
> >  
> > igt_fixture {
> > igt_stop_hang_detector();
> > -- 
> > 2.9.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] drm/i915: Update MOCS settings for gen 9

2017-05-05 Thread Arkadiusz Hiler
On Fri, May 05, 2017 at 12:49:49PM +0100, Chris Wilson wrote:
> On Fri, May 05, 2017 at 01:31:37PM +0200, Arkadiusz Hiler wrote:
> > On Thu, May 04, 2017 at 07:46:34PM -0700, Dmitry Rogozhkin wrote:
> > > 
> > > 
> > > On 5/4/2017 9:51 AM, Kenneth Graunke wrote:
> > > > MediaSDK is not a benchmark.  If I'm not mistaken, it's a userspace
> > > > driver produced by Intel engineers, one which Intel has the full
> > > > capability to change.  What you're saying is that Intel's MediaSDK
> > > > engineers are unwilling to change their software to provide better
> > > > performance for their Linux users.
> > > > 
> > > > That's pretty mental.
> > > You are mistaken. Media SDK is not a driver. It is a user space library
> > > which talks to the user space driver. And Media SDK does not set _any_
> > > caching policies you are discussing here. That's the driver who sets these
> > > policies. I don't want to go further here who supports this driver, Intel 
> > > or
> > > not, but there are mediasdk engineers whom you blame to not willing to do
> > > something and who actually only indirectly are related to this topic.
> > > Please, if you mean driver, say a driver.
> > 
> > You are right. It is very unfortunate that those two were confused.
> > 
> > To further clarify, here's an excerpt from MediaSDK's README.md:
> > 
> > "Intel Media SDK depends on a special version of LibVA which comes with
> > Intel Media Server Studio installation and is not in upstream, so this
> > version is not compatible with the LibVA/driver available at 01org."
> > 
> > 
> > Nonetheless, the main question still stands:
> > 
> > How big is the performance gap when using appropriate, existing entries
> > entries vs the entries proposed here?
> > 
> > 
> > Also a couple more questions arise:
> > 
> > * What about versioning schema? We definitely need a consumer for that.
> > 
> > * The libva you use is a **closed** UMD. It this enough of a reason to
> >   do the change (sans the versioning)? It starts to get blurry here.
> > 
> > Don't get me wrong, I've seen the spec and believe that a lot of hard
> > work was put into determining the entries and the usage scenarios, and
> > that they can benefit us.
> > 
> > I even have some patches ready[0] and I would love to get them upstream,
> > but they have to be tested in reproducible manner, with clear
> > methodology. They need to be discussed and deemed useful by providing
> > real value. We cannot simply relay on opaque claims that they are good
> > for us.
> > 
> > 
> > To do the above I am fiddling with Mesa - if we will see performance
> > boost, then this would provide both a solid reason to add the entries
> > and a consumer for the versioning API.
> > 
> > 
> > As of proprietary libva/MediaSDK combo I see at least three options to
> > do "the 3" vs "extended mocs" testing:
> > 
> > 1. change the libva you use to utilize only the basic 3 entries only and
> >do the testing - if you can change the source
> > 
> > 2. change kernel and fill in entries with copies of the 3 basic ones and
> >do the testing - will work even without the source code of libva
> > 
> > 3. port the MOCS changes to 01org's libva and either use this version
> >from MediaSDK for testing or use some other benchmarks -  this could
> >be the consumer we need
> > 
> > I hope that this will move us forward.
> 
> Michal (W or W) has some patches to allow contexts to set their own mocs,
> which is definitely my preferred solution.
> -Chris

Michal Winiarski (CCed).

IIRC the series was only for render - other rings does not have
context-stored MOCS.

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


[Intel-gfx] [PATCH] drm/i915/gen9: Reintroduce WaEnableYV12BugFixInHalfSliceChicken7

2017-05-12 Thread Arkadiusz Hiler
This basically reverts commit 465418c6064c
("drm/i915/gen9: Remove WaEnableYV12BugFixInHalfSliceChicken7")
with small addition - marking it as affecting KBL as well.

It was incorrectly considered fixed in production steppings.

References: HSD#2126385, HSD#2131381, HSDES#1504433555, BSID#0764
Cc: Mika Kuoppala 
Cc: Jeff McGee 
Signed-off-by: Arkadiusz Hiler 
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 483ed76..49c2315 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -851,8 +851,10 @@ static int gen9_init_workarounds(struct intel_engine_cs 
*engine)
 */
}
 
+   /* WaEnableYV12BugFixInHalfSliceChicken7:skl,bxt,kbl,glk */
/* WaEnableSamplerGPGPUPreemptionSupport:skl,bxt,kbl */
WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
+ GEN9_ENABLE_YV12_BUGFIX |
  GEN9_ENABLE_GPGPU_PREEMPTION);
 
/* Wa4x4STCOptimizationDisable:skl,bxt,kbl,glk */
-- 
2.9.3

___
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

2018-07-10 Thread 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 i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

2018-07-10 Thread 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 4/7] Fix truncate string in the snprintf

2018-07-10 Thread 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] [igt-dev] [PATCH i-g-t 2/7] Cast void * pointer used in arithmetic to uint32_t*

2018-07-10 Thread 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 5/7] Make string commands dynamic allocate

2018-07-11 Thread 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] [CI] drm/i915: Split GPU commands definitions into separate header

2018-03-15 Thread 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


  1   2   3   4   5   6   >