Re: [Intel-gfx] [PATCH v3 1/5] drm: Add config for detecting libdrm

2015-07-24 Thread Mike Frysinger
On 01 Jul 2015 14:52, Patrik Jakobsson wrote:
 Use pkg-config to try to find libdrm. If that fails use the standard
 include directory for kernel drm headers in /usr/include/drm.
 
 * configure.ac: Use pkg-config to find libdrm
 
 Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com
 ---
  configure.ac | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/configure.ac b/configure.ac
 index bb8bf46..aa63af7 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -844,6 +844,10 @@ fi
  AM_CONDITIONAL([USE_LIBUNWIND], [test x$use_libunwind = xyes])
  AC_MSG_RESULT([$use_libunwind])
  
 +PKG_CHECK_MODULES([libdrm], [libdrm],
 + [CPPFLAGS=$CPPFLAGS $libdrm_CFLAGS],
 + [CPPFLAGS=$CPPFLAGS -I/usr/include/drm])

yikes, no, this is a really really bad idea.  it should read:
PKG_CHECK_MODULES([LIBDRM], [libdrm],
[CPPFLAGS=$CPPFLAGS $LIBDRM_CFLAGS], [:])
-mike


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 1/5] drm: Add config for detecting libdrm

2015-07-24 Thread Mike Frysinger
On 23 Jul 2015 13:44, Dmitry V. Levin wrote:
 On Thu, Jul 23, 2015 at 05:48:21AM -0400, Mike Frysinger wrote:
  On 01 Jul 2015 14:52, Patrik Jakobsson wrote:
   Use pkg-config to try to find libdrm. If that fails use the standard
   include directory for kernel drm headers in /usr/include/drm.
   
   * configure.ac: Use pkg-config to find libdrm
   
   Signed-off-by: Patrik Jakobsson patrik.jakobs...@linux.intel.com
   ---
configure.ac | 4 
1 file changed, 4 insertions(+)
   
   diff --git a/configure.ac b/configure.ac
   index bb8bf46..aa63af7 100644
   --- a/configure.ac
   +++ b/configure.ac
   @@ -844,6 +844,10 @@ fi
AM_CONDITIONAL([USE_LIBUNWIND], [test x$use_libunwind = xyes])
AC_MSG_RESULT([$use_libunwind])

   +PKG_CHECK_MODULES([libdrm], [libdrm],
   + [CPPFLAGS=$CPPFLAGS $libdrm_CFLAGS],
   + [CPPFLAGS=$CPPFLAGS -I/usr/include/drm])
  
  yikes, no, this is a really really bad idea.  it should read:
  PKG_CHECK_MODULES([LIBDRM], [libdrm],
  [CPPFLAGS=$CPPFLAGS $LIBDRM_CFLAGS], [:])
 
 Why [:] ?  Wouldn't [] suffice?

probably ... force of habit after being bitten by m4 macros that did not expect
to expand empty code and thus lead to shell errors (include macros by autotools
projects).  i.e. if the m4 looked something like:
if ...check if pkg is available...; then
$3
else
$4
fi
then the generated configure script would have syntax errors.
-mike


signature.asc
Description: Digital signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Parsing LFP brightness control from VBT

2015-07-24 Thread Kannan, Vandana

Any inputs on this patch ?

- Vandana

On 7/6/2015 4:35 PM, Vandana Kannan wrote:

From: Deepak M m.dee...@intel.com

LFP brighness control from the VBT block 43 indicates which
controller is used for brightness.
LFP1 brightness control method:
Bit 7-4 = This field controller number of the brightnes controller.
0 = Controller 0
1 = Controller 1
2 = Controller 2
3 = Controller 3
Others = Reserved

Bits 3-0 are for Control pin
0 = PMIC pin is used for brightness control
1 = LPSS PWM is used for brightness control
2 = Display DDI is used for brightness control
3 = CABC method to control brightness
Others = Reserved

History:
This patch was submitted earlier including a check for control pin.
http://lists.freedesktop.org/archives/intel-gfx/2014-December/057530.html
Since it caused the issue, https://bugs.freedesktop.org/show_bug.cgi?id=87671,
it was reverted in
http://lists.freedesktop.org/archives/intel-gfx/2015-January/058110.html

The current patch reads controller and control pin from VBT (version = 191)

 From VBT version = 197, default value of control pin is set to DDI, so the
corresponding check during backlight setup will be made in a future patch

Signed-off-by: Deepak M m.dee...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
Cc: Jani Nikula jani.nik...@intel.com
---
  drivers/gpu/drm/i915/i915_drv.h   |  2 ++
  drivers/gpu/drm/i915/intel_bios.c |  9 +
  drivers/gpu/drm/i915/intel_bios.h | 11 +++
  3 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 950a981..a89e9a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1461,6 +1461,8 @@ struct intel_vbt_data {
bool present;
bool active_low_pwm;
u8 min_brightness;  /* min_brightness/255 of max */
+   u8 controller;  /* brightness controller number */
+   u8 control_pin; /* brightness control pin */
} backlight;

/* MIPI DSI */
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 2ff9eb0..32c1ef2 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -256,6 +256,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
  {
const struct bdb_lfp_backlight_data *backlight_data;
const struct bdb_lfp_backlight_data_entry *entry;
+   const struct bdb_lfp_backlight_control_data *bl_ctrl_data;

backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
if (!backlight_data)
@@ -268,6 +269,7 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
}

entry = backlight_data-data[panel_type];
+   bl_ctrl_data = backlight_data-blc_ctl[panel_type];

dev_priv-vbt.backlight.present = entry-type == BDB_BACKLIGHT_TYPE_PWM;
if (!dev_priv-vbt.backlight.present) {
@@ -279,12 +281,19 @@ parse_lfp_backlight(struct drm_i915_private *dev_priv,
dev_priv-vbt.backlight.pwm_freq_hz = entry-pwm_freq_hz;
dev_priv-vbt.backlight.active_low_pwm = entry-active_low_pwm;
dev_priv-vbt.backlight.min_brightness = entry-min_brightness;
+   dev_priv-vbt.backlight.controller = bl_ctrl_data-controller;
+   dev_priv-vbt.backlight.control_pin = bl_ctrl_data-pin;
+
DRM_DEBUG_KMS(VBT backlight PWM modulation frequency %u Hz, 
  active %s, min brightness %u, level %u\n,
  dev_priv-vbt.backlight.pwm_freq_hz,
  dev_priv-vbt.backlight.active_low_pwm ? low : high,
  dev_priv-vbt.backlight.min_brightness,
  backlight_data-level[panel_type]);
+
+   DRM_DEBUG_KMS(VBT BL controller %u, BL control pin %u\n,
+   dev_priv-vbt.backlight.controller,
+   dev_priv-vbt.backlight.control_pin);
  }

  /* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_bios.h 
b/drivers/gpu/drm/i915/intel_bios.h
index af0b476..e97c1c0 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -402,10 +402,21 @@ struct bdb_lfp_backlight_data_entry {
u8 obsolete3;
  } __packed;

+#define BLC_CONTROL_PIN_PMIC   0
+#define BLC_CONTROL_PIN_LPSS_PWM   1
+#define BLC_CONTROL_PIN_DDI2
+#define BLC_CONTROL_PIN_CABC   3
+
+struct bdb_lfp_backlight_control_data {
+   u8 controller:4;
+   u8 pin:4;
+} __packed;
+
  struct bdb_lfp_backlight_data {
u8 entry_size;
struct bdb_lfp_backlight_data_entry data[16];
u8 level[16];
+   struct bdb_lfp_backlight_control_data blc_ctl[16];
  } __packed;

  struct aimdb_header {


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


[Intel-gfx] [PATCH i-g-t] tests/drm_import_export: Add tests for prime/flink sharing races

2015-07-24 Thread Michał Winiarski
It is possible to race between unreference of the underlying BO and
importing it from prime_fd/name. Verify that the behaviour of libdrm
is consistent for prime/flink.

Signed-off-by: Michał Winiarski michal.winiar...@intel.com
---
 tests/drm_import_export.c | 103 ++
 1 file changed, 103 insertions(+)

diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
index 57b13dd..db11c18 100644
--- a/tests/drm_import_export.c
+++ b/tests/drm_import_export.c
@@ -131,6 +131,98 @@ static void * test_thread(void * par)
return NULL;
 }
 
+#define IMPORT_RACE_LOOPS 10
+
+struct import_race_thread_data {
+   int prime_fd;
+   uint32_t flink_name;
+   unsigned int stop;
+   pthread_mutex_t mutex;
+};
+
+static void *import_close_thread(void *data)
+{
+   struct import_race_thread_data *t = (struct import_race_thread_data 
*)data;
+   drm_intel_bo *bo;
+   pthread_mutex_lock(t-mutex);
+   while (!t-stop) {
+   pthread_mutex_unlock(t-mutex);
+   bo = NULL;
+   if (use_flink)
+   bo = drm_intel_bo_gem_create_from_name(bufmgr, 
buf-shared, t-flink_name);
+   else {
+   pthread_mutex_lock(t-mutex);
+   if (t-prime_fd != -1) {
+   bo = drm_intel_bo_gem_create_from_prime(bufmgr, 
t-prime_fd, 4096);
+   pthread_mutex_unlock(t-mutex);
+   }
+   else
+   /* We take the lock right after entering the 
loop */
+   continue;
+   }
+   if (bo == NULL) {
+   /*
+* If the bo is NULL it means that we've unreferenced 
in other
+* thread - therefore we should expect ENOENT
+*/
+   igt_assert_eq(errno, ENOENT);
+   continue;
+   }
+
+   drm_intel_bo_unreference(bo);
+
+   pthread_mutex_lock(t-mutex);
+   }
+   pthread_mutex_unlock(t-mutex);
+
+   return NULL;
+}
+
+static void test_import_close_race(void)
+{
+   pthread_t t;
+   unsigned int loops = IMPORT_RACE_LOOPS;
+   drm_intel_bo *bo;
+   struct import_race_thread_data t_data;
+
+   memset(t_data, 0, sizeof(t_data));
+   pthread_mutex_init(t_data.mutex, NULL);
+   t_data.prime_fd = -1;
+
+   igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 
0);
+
+   while (loops--) {
+   bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096);
+   igt_assert(bo != NULL);
+   /*
+* We setup the test in such way, that create_from_* can race 
between
+* unreference. If we're using prime, prime_fd is always a 
valid fd.
+*/
+   if (use_flink)
+   igt_assert_eq(drm_intel_bo_flink(bo, 
(t_data.flink_name)), 0);
+   else {
+   pthread_mutex_lock(t_data.mutex);
+   igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, 
(t_data.prime_fd)), 0);
+   igt_assert(t_data.prime_fd != -1);
+   pthread_mutex_unlock(t_data.mutex);
+   }
+
+   drm_intel_bo_unreference(bo);
+
+   pthread_mutex_lock(t_data.mutex);
+   close(t_data.prime_fd);
+   t_data.prime_fd = -1;
+   pthread_mutex_unlock(t_data.mutex);
+   }
+
+   pthread_mutex_lock(t_data.mutex);
+   t_data.stop = 1;
+   pthread_mutex_unlock(t_data.mutex);
+
+   pthread_join(t, NULL);
+   pthread_mutex_destroy(t_data.mutex);
+}
+
 pthread_t test_thread_id1;
 pthread_t test_thread_id2;
 pthread_t test_thread_id3;
@@ -153,6 +245,16 @@ igt_main {
drm_intel_bufmgr_gem_enable_reuse(bufmgr);
}
 
+   igt_subtest(import-close-race-flink) {
+   use_flink = true;
+   test_import_close_race();
+   }
+
+   igt_subtest(import-close-race-prime) {
+   use_flink = false;
+   test_import_close_race();
+   }
+
igt_subtest(flink) {
use_flink = true;
 
@@ -180,4 +282,5 @@ igt_main {
pthread_join(test_thread_id3, NULL);
pthread_join(test_thread_id4, NULL);
}
+
 }
-- 
2.4.3

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


[Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex

2015-07-24 Thread Michał Winiarski
From: Rafał Sapała rafal.a.sap...@intel.com

It is possible to hit a race condition in create_from_prime, when trying
to import a BO that's currently being freed. In case of prime sharing
we'll succesfully get a handle, but fail on get_tiling call, potentially
confusing the caller (and requiring different locking scheme than with
sharing using flink). Wrap fd_to_handle with struct_mutex to force
a more consistent behaviour between prime/flink, convert fprintf to DBG
when handling errors.

Testcase: igt/drm_import_export/import-close-race-prime
Signed-off-by: Rafał Sapała rafal.a.sap...@intel.com
Signed-off-by: Michał Winiarski michal.winiar...@intel.com
---
 intel/intel_bufmgr_gem.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index b1c3b3a..ed4ffd2 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -2728,14 +2728,19 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
struct drm_i915_gem_get_tiling get_tiling;
drmMMListHead *list;
 
+   pthread_mutex_lock(bufmgr_gem-lock);
ret = drmPrimeFDToHandle(bufmgr_gem-fd, prime_fd, handle);
+   if (ret) {
+   DBG(create_from_prime: failed to obtain handle from fd: %s\n, 
strerror(errno));
+   pthread_mutex_unlock(bufmgr_gem-lock);
+   return NULL;
+   }
 
/*
 * See if the kernel has already returned this buffer to us. Just as
 * for named buffers, we must not create two bo's pointing at the same
 * kernel object
 */
-   pthread_mutex_lock(bufmgr_gem-lock);
for (list = bufmgr_gem-named.next;
 list != bufmgr_gem-named;
 list = list-next) {
@@ -2747,12 +2752,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
}
}
 
-   if (ret) {
- fprintf(stderr,ret is %d %d\n, ret, errno);
- pthread_mutex_unlock(bufmgr_gem-lock);
-   return NULL;
-   }
-
bo_gem = calloc(1, sizeof(*bo_gem));
if (!bo_gem) {
pthread_mutex_unlock(bufmgr_gem-lock);
@@ -2793,6 +2792,7 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
   DRM_IOCTL_I915_GEM_GET_TILING,
   get_tiling);
if (ret != 0) {
+   DBG(create_from_prime: failed to get tiling: %s\n, 
strerror(errno));
drm_intel_gem_bo_unreference(bo_gem-bo);
return NULL;
}
-- 
2.4.3

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


Re: [Intel-gfx] [PATCH libdrm] intel: Serialize drmPrimeFDToHandle with struct_mutex

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 11:22:34AM +0200, Michał Winiarski wrote:
 From: Rafał Sapała rafal.a.sap...@intel.com
 
 It is possible to hit a race condition in create_from_prime, when trying
 to import a BO that's currently being freed. In case of prime sharing
 we'll succesfully get a handle, but fail on get_tiling call, potentially
 confusing the caller (and requiring different locking scheme than with
 sharing using flink). Wrap fd_to_handle with struct_mutex to force
 a more consistent behaviour between prime/flink, convert fprintf to DBG
 when handling errors.

The race is that the kernel returns us the same file-private handle as
the first thread, but that first thread is about to call gem_close
(thereby removing the handle from the file completely) and does so
between us acquiring the handle and taking the mutex. If we take
the mutex, then we acquire the refcnt on the bo prior to the first
thread completing its unref (and so preventing the early close). Or we
acquire the handle after the earlier close, in which case we are the new
owner.

Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFCv2 00/12] TDR/watchdog timeout support for gen8 (edit: fixed coverletter)

2015-07-24 Thread Tomas Elf

On 21/07/2015 15:51, Tomas Elf wrote:

This patch series introduces the following features:

* Feature 1: TDR (Timeout Detection and Recovery) for gen8 execlist mode.

TDR is an umbrella term for anything that goes into detecting and recovering 
from GPU hangs and is a term more widely used outside of the upstream driver.
This feature introduces an extensible framework that currently supports gen8 
but that can be easily extended to support gen7 as well (which is already the 
case in GMIN but unfortunately in a not quite upstreamable form). The code 
contained in this submission represents the essentials of what is currently in 
GMIN merged with what is currently in upstream (as of the time when this work 
commenced a few months back).

This feature adds a new hang recovery path alongside the legacy GPU reset path, 
which takes care of engine recovery only. Aside from adding support for 
per-engine recovery this feature also introduces rules for how to promote a 
potential per-engine reset to a legacy, full GPU reset.

The hang checker now integrates with the error handler in a slightly different 
way in that it allows hang recovery on multiple engines at the same time by 
passing an engine flag mask to the error handler where flags representing all 
of the hung engines are set. This allows us to schedule hang recovery once for 
all currently hung engines instead of one hang recovery per detected engine 
hang. Previously, when only full GPU reset was supported this was all the same 
since it wouldn't matter if one or four engines were hung at any given point 
since it would all amount to the same thing - the GPU getting reset. As it 
stands now the behaviour is different depending on which engine is hung since 
each engine is reset separately from all the other engines, therefore we have 
to think about this in terms of scheduling cost and recovery latency. (see open 
question below)

OPEN QUESTIONS:

1. Do we want to investigate the possibility of per-engine hang
detection? In the current upstream driver there is only one work queue
that handles the hang checker and everything from initial hang
detection to final hang recovery runs in this thread. This makes sense
if you're only supporting one form of hang recovery - using full GPU
reset and nothing tied to any particular engine. However, as part
of this patch series we're changing that by introducing per-engine
hang recovery. It could make sense to introduce multiple work
queues - one per engine - to run multiple hang checking threads in
parallel.

This would potentially allow savings in terms of recovery latency since
we don't have to scan all engines every time the hang checker is
scheduled and the error handler does not have to scan all engines every
time it is scheduled. Instead, we could implement one work queue per
engine that would invoke the hang checker that only checks _that_
particular engine and then the error handler is invoked for _that_
particular engine. If one engine has hung the latency for getting to
the hang recovery path for that particular engine would be (Time For
Hang Checking One Engine) + (Time For Error Handling One Engine) rather
than the time it takes to do hang checking for all engines + the time
it takes to do error handling for all engines that have been detected
as hung (which in the worst case would be all engines). There would
potentially be as many hang checker and error handling threads going on
concurrently as there are engines in the hardware but they would all be
running in parallel without any significant locking. The first time
where any thread needs exclusive access to the driver is at the point
of the actual hang recovery but the time it takes to get there would
theoretically be lower and the time it actually takes to do per-engine
hang recovery is quite a lot lower than the time it takes to actually
detect a hang reliably.

How much we would save by such a change still needs to be analysed and
compared against the current single-thread model but it makes sense
from a theoretical design point of view.

* Feature 2: Watchdog Timeout (a.k.a media engine reset) for gen8.

This feature allows userland applications to control whether or not individual 
batch buffers should have a first-level, fine-grained, hardware-based hang 
detection mechanism on top of the ordinary, software-based periodic hang 
checker that is already in the driver. The advantage over relying solely on the 
current software-based hang checker is that the watchdog timeout mechanism is 
about 1000x quicker and more precise. Since it's not a full driver-level hang 
detection mechanism but only targetting one individual batch buffer at a time 
it can afford to be that quick without 

[Intel-gfx] [PATCH i-g-t v2] tests/drm_import_export: Add tests for prime/flink sharing races

2015-07-24 Thread Michał Winiarski
It is possible to race between unreference of the underlying BO and
importing it from prime_fd/name. Verify that the behaviour of libdrm
is consistent for prime/flink.

v2: more comments in source file, dropped extra whitespace

Signed-off-by: Michał Winiarski michal.winiar...@intel.com
Cc: Thomas Wood thomas.w...@intel.com
---
 tests/drm_import_export.c | 112 ++
 1 file changed, 112 insertions(+)

diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
index 57b13dd..e24e0df 100644
--- a/tests/drm_import_export.c
+++ b/tests/drm_import_export.c
@@ -131,6 +131,108 @@ static void * test_thread(void * par)
return NULL;
 }
 
+#define IMPORT_RACE_LOOPS 10
+
+struct import_race_thread_data {
+   int prime_fd;
+   uint32_t flink_name;
+   unsigned int stop;
+   pthread_mutex_t mutex;
+};
+
+/*
+ * Attempt to import the bo. It is possible that GEM_CLOSE was already called
+ * in different thread and from i915 point of view the handle is no longer
+ * valid (thus create_from_prime/name should fail).
+ */
+static void *import_close_thread(void *data)
+{
+   struct import_race_thread_data *t = (struct import_race_thread_data 
*)data;
+   drm_intel_bo *bo;
+   pthread_mutex_lock(t-mutex);
+   while (!t-stop) {
+   pthread_mutex_unlock(t-mutex);
+   bo = NULL;
+   if (use_flink)
+   bo = drm_intel_bo_gem_create_from_name(bufmgr, 
buf-shared, t-flink_name);
+   else {
+   pthread_mutex_lock(t-mutex);
+   if (t-prime_fd != -1) {
+   bo = drm_intel_bo_gem_create_from_prime(bufmgr, 
t-prime_fd, 4096);
+   pthread_mutex_unlock(t-mutex);
+   }
+   else
+   /* We take the lock right after entering the 
loop */
+   continue;
+   }
+   if (bo == NULL) {
+   /*
+* If the bo is NULL it means that we've unreferenced 
in other
+* thread - therefore we should expect ENOENT
+*/
+   igt_assert_eq(errno, ENOENT);
+   continue;
+   }
+
+   drm_intel_bo_unreference(bo);
+
+   pthread_mutex_lock(t-mutex);
+   }
+   pthread_mutex_unlock(t-mutex);
+
+   return NULL;
+}
+
+/*
+ * It is possible to race between unreference of the underlying BO and 
importing
+ * it from prime_fd/name. Verify that the behaviour of libdrm is consistent for
+ * prime/flink.
+ */
+static void test_import_close_race(void)
+{
+   pthread_t t;
+   unsigned int loops = IMPORT_RACE_LOOPS;
+   drm_intel_bo *bo;
+   struct import_race_thread_data t_data;
+
+   memset(t_data, 0, sizeof(t_data));
+   pthread_mutex_init(t_data.mutex, NULL);
+   t_data.prime_fd = -1;
+
+   igt_assert_eq(pthread_create(t, NULL, import_close_thread , t_data), 
0);
+
+   while (loops--) {
+   bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096);
+   igt_assert(bo != NULL);
+   /*
+* We setup the test in such way, that create_from_* can race 
between
+* unreference. If we're using prime, prime_fd is always a 
valid fd.
+*/
+   if (use_flink)
+   igt_assert_eq(drm_intel_bo_flink(bo, 
(t_data.flink_name)), 0);
+   else {
+   pthread_mutex_lock(t_data.mutex);
+   igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, 
(t_data.prime_fd)), 0);
+   igt_assert(t_data.prime_fd != -1);
+   pthread_mutex_unlock(t_data.mutex);
+   }
+
+   drm_intel_bo_unreference(bo);
+
+   pthread_mutex_lock(t_data.mutex);
+   close(t_data.prime_fd);
+   t_data.prime_fd = -1;
+   pthread_mutex_unlock(t_data.mutex);
+   }
+
+   pthread_mutex_lock(t_data.mutex);
+   t_data.stop = 1;
+   pthread_mutex_unlock(t_data.mutex);
+
+   pthread_join(t, NULL);
+   pthread_mutex_destroy(t_data.mutex);
+}
+
 pthread_t test_thread_id1;
 pthread_t test_thread_id2;
 pthread_t test_thread_id3;
@@ -153,6 +255,16 @@ igt_main {
drm_intel_bufmgr_gem_enable_reuse(bufmgr);
}
 
+   igt_subtest(import-close-race-flink) {
+   use_flink = true;
+   test_import_close_race();
+   }
+
+   igt_subtest(import-close-race-prime) {
+   use_flink = false;
+   test_import_close_race();
+   }
+
igt_subtest(flink) {
use_flink = true;
 
-- 
2.4.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] [PATCH i-g-t] tools/Makefile.sources: fix igt_stats complie error on android

2015-07-24 Thread Morton, Derek J
Found a simpler way of doing this using 'LOCAL_MODULE'. Will prepare a 
replacement patch.

//Derek



-Original Message-
From: Morton, Derek J 
Sent: Thursday, July 23, 2015 1:59 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; Morton, Derek J
Subject: [PATCH i-g-t] tools/Makefile.sources: fix igt_stats complie error on 
android

There are two versions of igt_stats.c in tools and lib\tests which causes the 
build system to have two build modules with the same name.
This patch specifies a different target name for the tool so there is no clash.

Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
 tools/Makefile.sources | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/Makefile.sources b/tools/Makefile.sources index 
8ca9351..d19ef96 100644
--- a/tools/Makefile.sources
+++ b/tools/Makefile.sources
@@ -5,7 +5,7 @@ noinst_PROGRAMS = \
   $(NULL)
 
 bin_PROGRAMS =\
-  igt_stats   \
+  igt_statistics  \
   intel_audio_dump\
   intel_reg   \
   intel_backlight \
@@ -63,3 +63,9 @@ intel_l3_parity_SOURCES =\
   intel_l3_parity.h   \
   intel_l3_udev_listener.c
 
+# Android does not like building multiple targets with the same name # 
+tools/igt_stats.c clashes with lib/tests/igt_stats.c. Simplest fix is 
+to # specify a different target name here.
+igt_statistics_SOURCES =  \
+  igt_stats.c
+
--
1.9.1


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


Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android

2015-07-24 Thread Morton, Derek J


-Original Message-
From: Thomas Wood [mailto:thomas.w...@intel.com] 
Sent: Friday, July 24, 2015 4:33 PM
To: Morton, Derek J
Cc: Intel Graphics Development; Gore, Tim
Subject: Re: [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on 
android

On 24 July 2015 at 14:35, Derek Morton derek.j.mor...@intel.com wrote:
 There are two versions of gem_exec_nop.c in benchmarks and tests which 
 causes the build system to have two build modules with the same name.
 This patch renames benchmarks/gem_exec_nop.c to 
 benchmarks/gem_exec_nop_benchmark.c using the existing 
 gem_userptr_benchmark.c as a naming convention.

Would using LOCAL_MODULE_FILENAME help here, allowing an alternative output 
name for Android?
https://developer.android.com/ndk/guides/android_mk.html#mdv

Not really, but changing LOCAL_MODULE to be $1_benchmarks instead of $1 to 
'namespace' the directory does, and will prevent the same problem reoccuring 
when more files are added in the future.

I will prepare a new patch.




 v2: Also rename gem_mmap to gem_mmap_benchmark. Another file which 
 breaks android which was added after this patch was 1st submitted.

 Signed-off-by: Derek Morton derek.j.mor...@intel.com
 ---
  benchmarks/Makefile.sources |   9 +-
  benchmarks/gem_exec_nop.c   | 153 -
  benchmarks/gem_exec_nop_benchmark.c | 153 +
  benchmarks/gem_mmap.c   | 165 
 
  benchmarks/gem_mmap_benchmark.c | 165 
 
  5 files changed, 325 insertions(+), 320 deletions(-)  delete mode 
 100644 benchmarks/gem_exec_nop.c  create mode 100644 
 benchmarks/gem_exec_nop_benchmark.c
  delete mode 100644 benchmarks/gem_mmap.c  create mode 100644 
 benchmarks/gem_mmap_benchmark.c

 diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources 
 index 7ad95a5..35722b5 100644
 --- a/benchmarks/Makefile.sources
 +++ b/benchmarks/Makefile.sources
 @@ -1,11 +1,16 @@
 +# If you copy a test to benckmarks, rename it _benchmark # The 
 +andriod build will fail when trying to build multiple binaries with # 
 +the same name.
 +
  bin_PROGRAMS =  \
 intel_upload_blit_large \
 intel_upload_blit_large_gtt \
 intel_upload_blit_large_map \
 intel_upload_blit_small \
 -   gem_exec_nop\
 -   gem_mmap\
 +   gem_exec_nop_benchmark  \
 +   gem_mmap_benchmark  \
 gem_prw \
 gem_userptr_benchmark   \
 kms_vblank  \
 $(NULL)
 +
 diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c 
 deleted file mode 100644 index 2a3abd2..000
 --- a/benchmarks/gem_exec_nop.c
 +++ /dev/null
 @@ -1,153 +0,0 @@
 -/*
 - * Copyright © 2011 Intel Corporation
 - *
 - * Permission is hereby granted, free of charge, to any person 
 obtaining a
 - * copy of this software and associated documentation files (the 
 Software),
 - * to deal in the Software without restriction, including without 
 limitation
 - * the rights to use, copy, modify, merge, publish, distribute, 
 sublicense,
 - * and/or sell copies of the Software, and to permit persons to whom 
 the
 - * Software is furnished to do so, subject to the following conditions:
 - *
 - * The above copyright notice and this permission notice (including 
 the next
 - * paragraph) shall be included in all copies or substantial portions 
 of the
 - * Software.
 - *
 - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, 
 EXPRESS OR
 - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 MERCHANTABILITY,
 - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
 SHALL
 - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
 OR OTHER
 - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
 ARISING
 - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
 OTHER DEALINGS
 - * IN THE SOFTWARE.
 - *
 - * Authors:
 - *Chris Wilson ch...@chris-wilson.co.uk
 - *
 - */
 -
 -#include unistd.h
 -#include stdlib.h
 -#include stdint.h
 -#include stdio.h
 -#include string.h
 -#include fcntl.h
 -#include inttypes.h
 -#include errno.h
 -#include sys/stat.h
 -#include sys/ioctl.h
 -#include sys/time.h
 -#include time.h
 -
 -#include drm.h
 -#include ioctl_wrappers.h
 -#include drmtest.h
 -#include intel_io.h
 -#include igt_stats.h
 -
 -#define LOCAL_I915_EXEC_NO_RELOC (111) -#define 
 LOCAL_I915_EXEC_HANDLE_LUT (112)
 -
 -static uint64_t elapsed(const struct timespec *start,
 -   const struct timespec *end,
 -   int loop)
 -{
 -   return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec 
 - start-tv_nsec))/loop;
 -}
 -
 -static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 
 *execbuf) -{
 -   int 

Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android

2015-07-24 Thread Thomas Wood
On 24 July 2015 at 14:43, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Fri, Jul 24, 2015 at 02:35:29PM +0100, Derek Morton wrote:
 There are two versions of gem_exec_nop.c in benchmarks and tests
 which causes the build system to have two build modules with
 the same name.
 This patch renames benchmarks/gem_exec_nop.c to
 benchmarks/gem_exec_nop_benchmark.c using the existing
 gem_userptr_benchmark.c as a naming convention.

 Surely a simpler fix than breaking external tools would be to fix the
 Makefile?

If the binaries do have to be renamed, the .gitignore files will also
need updating.


 -Chris

 --
 Chris Wilson, Intel Open Source Technology Centre
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915: Remove bogus kerneldoc include directive

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 05:40:13PM +0200, Daniel Vetter wrote:
 Afaict intel_irq_fini never existed. No idea how that one came
 about.

It's notable by its absence. We should write it! There are a few steps
in intel_irq_init() that would be best undone intel_irq_fini(). The work
handlers could be dealt with by a comment that the core does a
flush_wq() - that at leasts says we have done due diligence and checked
that the work will be complete before module unload. But we should move
the pm_qos teardown here for instance.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t v2] tests/drm_import_export: Add tests for prime/flink sharing races

2015-07-24 Thread Thomas Wood
On 24 July 2015 at 15:43, Michał Winiarski michal.winiar...@intel.com wrote:
 It is possible to race between unreference of the underlying BO and
 importing it from prime_fd/name. Verify that the behaviour of libdrm
 is consistent for prime/flink.

 v2: more comments in source file, dropped extra whitespace

Thanks, patch pushed.


 Signed-off-by: Michał Winiarski michal.winiar...@intel.com
 Cc: Thomas Wood thomas.w...@intel.com
 ---
  tests/drm_import_export.c | 112 
 ++
  1 file changed, 112 insertions(+)

 diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
 index 57b13dd..e24e0df 100644
 --- a/tests/drm_import_export.c
 +++ b/tests/drm_import_export.c
 @@ -131,6 +131,108 @@ static void * test_thread(void * par)
 return NULL;
  }

 +#define IMPORT_RACE_LOOPS 10
 +
 +struct import_race_thread_data {
 +   int prime_fd;
 +   uint32_t flink_name;
 +   unsigned int stop;
 +   pthread_mutex_t mutex;
 +};
 +
 +/*
 + * Attempt to import the bo. It is possible that GEM_CLOSE was already called
 + * in different thread and from i915 point of view the handle is no longer
 + * valid (thus create_from_prime/name should fail).
 + */
 +static void *import_close_thread(void *data)
 +{
 +   struct import_race_thread_data *t = (struct import_race_thread_data 
 *)data;
 +   drm_intel_bo *bo;
 +   pthread_mutex_lock(t-mutex);
 +   while (!t-stop) {
 +   pthread_mutex_unlock(t-mutex);
 +   bo = NULL;
 +   if (use_flink)
 +   bo = drm_intel_bo_gem_create_from_name(bufmgr, 
 buf-shared, t-flink_name);
 +   else {
 +   pthread_mutex_lock(t-mutex);
 +   if (t-prime_fd != -1) {
 +   bo = 
 drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096);
 +   pthread_mutex_unlock(t-mutex);
 +   }
 +   else
 +   /* We take the lock right after entering the 
 loop */
 +   continue;
 +   }
 +   if (bo == NULL) {
 +   /*
 +* If the bo is NULL it means that we've unreferenced 
 in other
 +* thread - therefore we should expect ENOENT
 +*/
 +   igt_assert_eq(errno, ENOENT);
 +   continue;
 +   }
 +
 +   drm_intel_bo_unreference(bo);
 +
 +   pthread_mutex_lock(t-mutex);
 +   }
 +   pthread_mutex_unlock(t-mutex);
 +
 +   return NULL;
 +}
 +
 +/*
 + * It is possible to race between unreference of the underlying BO and 
 importing
 + * it from prime_fd/name. Verify that the behaviour of libdrm is consistent 
 for
 + * prime/flink.
 + */
 +static void test_import_close_race(void)
 +{
 +   pthread_t t;
 +   unsigned int loops = IMPORT_RACE_LOOPS;
 +   drm_intel_bo *bo;
 +   struct import_race_thread_data t_data;
 +
 +   memset(t_data, 0, sizeof(t_data));
 +   pthread_mutex_init(t_data.mutex, NULL);
 +   t_data.prime_fd = -1;
 +
 +   igt_assert_eq(pthread_create(t, NULL, import_close_thread , 
 t_data), 0);
 +
 +   while (loops--) {
 +   bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096);
 +   igt_assert(bo != NULL);
 +   /*
 +* We setup the test in such way, that create_from_* can race 
 between
 +* unreference. If we're using prime, prime_fd is always a 
 valid fd.
 +*/
 +   if (use_flink)
 +   igt_assert_eq(drm_intel_bo_flink(bo, 
 (t_data.flink_name)), 0);
 +   else {
 +   pthread_mutex_lock(t_data.mutex);
 +   igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, 
 (t_data.prime_fd)), 0);
 +   igt_assert(t_data.prime_fd != -1);
 +   pthread_mutex_unlock(t_data.mutex);
 +   }
 +
 +   drm_intel_bo_unreference(bo);
 +
 +   pthread_mutex_lock(t_data.mutex);
 +   close(t_data.prime_fd);
 +   t_data.prime_fd = -1;
 +   pthread_mutex_unlock(t_data.mutex);
 +   }
 +
 +   pthread_mutex_lock(t_data.mutex);
 +   t_data.stop = 1;
 +   pthread_mutex_unlock(t_data.mutex);
 +
 +   pthread_join(t, NULL);
 +   pthread_mutex_destroy(t_data.mutex);
 +}
 +
  pthread_t test_thread_id1;
  pthread_t test_thread_id2;
  pthread_t test_thread_id3;
 @@ -153,6 +255,16 @@ igt_main {
 drm_intel_bufmgr_gem_enable_reuse(bufmgr);
 }

 +   igt_subtest(import-close-race-flink) {
 +   use_flink = true;
 +   test_import_close_race();
 +   }
 +
 +   igt_subtest(import-close-race-prime) {
 +   use_flink = false;
 +   

Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android

2015-07-24 Thread Thomas Wood
On 24 July 2015 at 14:35, Derek Morton derek.j.mor...@intel.com wrote:
 There are two versions of gem_exec_nop.c in benchmarks and tests
 which causes the build system to have two build modules with
 the same name.
 This patch renames benchmarks/gem_exec_nop.c to
 benchmarks/gem_exec_nop_benchmark.c using the existing
 gem_userptr_benchmark.c as a naming convention.

Would using LOCAL_MODULE_FILENAME help here, allowing an alternative
output name for Android?
https://developer.android.com/ndk/guides/android_mk.html#mdv



 v2: Also rename gem_mmap to gem_mmap_benchmark. Another file
 which breaks android which was added after this patch was 1st
 submitted.

 Signed-off-by: Derek Morton derek.j.mor...@intel.com
 ---
  benchmarks/Makefile.sources |   9 +-
  benchmarks/gem_exec_nop.c   | 153 -
  benchmarks/gem_exec_nop_benchmark.c | 153 +
  benchmarks/gem_mmap.c   | 165 
 
  benchmarks/gem_mmap_benchmark.c | 165 
 
  5 files changed, 325 insertions(+), 320 deletions(-)
  delete mode 100644 benchmarks/gem_exec_nop.c
  create mode 100644 benchmarks/gem_exec_nop_benchmark.c
  delete mode 100644 benchmarks/gem_mmap.c
  create mode 100644 benchmarks/gem_mmap_benchmark.c

 diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
 index 7ad95a5..35722b5 100644
 --- a/benchmarks/Makefile.sources
 +++ b/benchmarks/Makefile.sources
 @@ -1,11 +1,16 @@
 +# If you copy a test to benckmarks, rename it _benchmark
 +# The andriod build will fail when trying to build multiple binaries with
 +# the same name.
 +
  bin_PROGRAMS =  \
 intel_upload_blit_large \
 intel_upload_blit_large_gtt \
 intel_upload_blit_large_map \
 intel_upload_blit_small \
 -   gem_exec_nop\
 -   gem_mmap\
 +   gem_exec_nop_benchmark  \
 +   gem_mmap_benchmark  \
 gem_prw \
 gem_userptr_benchmark   \
 kms_vblank  \
 $(NULL)
 +
 diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c
 deleted file mode 100644
 index 2a3abd2..000
 --- a/benchmarks/gem_exec_nop.c
 +++ /dev/null
 @@ -1,153 +0,0 @@
 -/*
 - * Copyright © 2011 Intel Corporation
 - *
 - * Permission is hereby granted, free of charge, to any person obtaining a
 - * copy of this software and associated documentation files (the Software),
 - * to deal in the Software without restriction, including without limitation
 - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 - * and/or sell copies of the Software, and to permit persons to whom the
 - * Software is furnished to do so, subject to the following conditions:
 - *
 - * The above copyright notice and this permission notice (including the next
 - * paragraph) shall be included in all copies or substantial portions of the
 - * Software.
 - *
 - * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
 DEALINGS
 - * IN THE SOFTWARE.
 - *
 - * Authors:
 - *Chris Wilson ch...@chris-wilson.co.uk
 - *
 - */
 -
 -#include unistd.h
 -#include stdlib.h
 -#include stdint.h
 -#include stdio.h
 -#include string.h
 -#include fcntl.h
 -#include inttypes.h
 -#include errno.h
 -#include sys/stat.h
 -#include sys/ioctl.h
 -#include sys/time.h
 -#include time.h
 -
 -#include drm.h
 -#include ioctl_wrappers.h
 -#include drmtest.h
 -#include intel_io.h
 -#include igt_stats.h
 -
 -#define LOCAL_I915_EXEC_NO_RELOC (111)
 -#define LOCAL_I915_EXEC_HANDLE_LUT (112)
 -
 -static uint64_t elapsed(const struct timespec *start,
 -   const struct timespec *end,
 -   int loop)
 -{
 -   return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec - 
 start-tv_nsec))/loop;
 -}
 -
 -static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf)
 -{
 -   int err = 0;
 -   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf))
 -   err = -errno;
 -   return err;
 -}
 -
 -static uint32_t batch(int fd)
 -{
 -   const uint32_t buf[] = {MI_BATCH_BUFFER_END};
 -   uint32_t handle = gem_create(fd, 4096);
 -   gem_write(fd, handle, 0, buf, sizeof(buf));
 -   return handle;
 -}
 -
 -static int loop(unsigned ring, int reps)
 -{
 -   struct drm_i915_gem_execbuffer2 execbuf;
 -   struct drm_i915_gem_exec_object2 gem_exec;
 -   int 

Re: [Intel-gfx] [PATCH 1/4] drm/i915: kerneldoc for fences

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 05:40:12PM +0200, Daniel Vetter wrote:
 v2: Clarify that this is about fence _registers_. Also clarify that
 the fence code revokes cpu ptes and not gtt ptes. Both suggested by
 Chris.
 
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Daniel Vetter daniel.vet...@intel.com

Quibble over 2, but 1,3,4 are
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/4] drm/i915: Remove bogus kerneldoc include directive

2015-07-24 Thread Daniel Vetter
Afaict intel_irq_fini never existed. No idea how that one came
about.

Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 Documentation/DocBook/drm.tmpl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index d7dc91b7e98c..e56bc0d01fdd 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3920,7 +3920,6 @@ int num_ioctls;/synopsis
 titleInterrupt Handling/title
 !Pdrivers/gpu/drm/i915/i915_irq.c interrupt handling
 !Fdrivers/gpu/drm/i915/i915_irq.c intel_irq_init intel_irq_init_hw 
intel_hpd_init
-!Fdrivers/gpu/drm/i915/i915_irq.c intel_irq_fini
 !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts
 !Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts
   /sect2
-- 
2.1.4

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


[Intel-gfx] [PATCH 4/4] drm/i915: kerneldoc for tiling IOCTL and swizzle functions

2015-07-24 Thread Daniel Vetter
Chris rightfully suggested that documenting fences without documenting
the BO tiling tracking doesn't make much sense, so fix that.

The important bit to stress here (since it lead to some confusion) is
the GEM doesn't really care about tiling. Except for a few select cases
where the kernel needs to manage something that userspace can't take
care of: Namely the limited number of fences and fixing up swizzling,
although we still fail at the later.

v2: Move the low-level tiling/swizzling functions and kerneldoc to
i915_gem_fence.c and leave only the userspace interface here.
Suggested by Chris.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 Documentation/DocBook/drm.tmpl | 16 ++-
 drivers/gpu/drm/i915/i915_gem_fence.c  | 28 ++--
 drivers/gpu/drm/i915/i915_gem_tiling.c | 84 +-
 3 files changed, 80 insertions(+), 48 deletions(-)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index e56bc0d01fdd..f1884038b90f 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4050,9 +4050,21 @@ int num_ioctls;/synopsis
 !Idrivers/gpu/drm/i915/i915_gem_gtt.c
   /sect2
   sect2
-titleGlobal GTT Fence Handling/title
-!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling
+titleGTT Fences and Swizzling/title
 !Idrivers/gpu/drm/i915/i915_gem_fence.c
+sect3
+  titleGlobal GTT Fence Handling/title
+!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling
+/sect3
+sect3
+  titleHardware Tiling and Swizzling Details/title
+!Pdrivers/gpu/drm/i915/i915_gem_fence.c tiling swizzling details
+/sect3
+  /sect2
+  sect2
+titleObject Tiling IOCTLs/title
+!Idrivers/gpu/drm/i915/i915_gem_tiling.c
+!Pdrivers/gpu/drm/i915/i915_gem_tiling.c buffer object tiling
   /sect2
   sect2
 titleBuffer Object Eviction/title
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c 
b/drivers/gpu/drm/i915/i915_gem_fence.c
index c643260a90c5..af1f8c461060 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -497,8 +497,7 @@ void i915_gem_restore_fences(struct drm_device *dev)
 }
 
 /**
- *
- * Support for managing tiling state of buffer objects.
+ * DOC: tiling swizzling details
  *
  * The idea behind tiling is to increase cache hit rates by rearranging
  * pixel data so that a group of pixel accesses are in the same cacheline.
@@ -546,6 +545,9 @@ void i915_gem_restore_fences(struct drm_device *dev)
  */
 
 /**
+ * i915_gem_detect_bit_6_swizzle - detect bit 6 swizzling pattern
+ * @dev: DRM device
+ *
  * Detects bit 6 swizzling of address lookup between IGD access and CPU
  * access through main memory.
  */
@@ -692,7 +694,7 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
dev_priv-mm.bit_6_swizzle_y = swizzle_y;
 }
 
-/**
+/*
  * Swap every 64 bytes of this page around, to account for it having a new
  * bit 17 of its physical address and therefore being interpreted differently
  * by the GPU.
@@ -715,6 +717,18 @@ i915_gem_swizzle_page(struct page *page)
kunmap(page);
 }
 
+/**
+ * i915_gem_object_do_bit_17_swizzle - fixup bit 17 swizzling
+ * @obj: i915 GEM buffer object
+ *
+ * This function fixes up the swizzling in case any page frame number for this
+ * object has changed in bit 17 since that state has been saved with
+ * i915_gem_object_save_bit_17_swizzle().
+ *
+ * This is called when pinning backing storage again, since the kernel is free
+ * to move unpinned backing storage around (either by directly moving pages or
+ * by swapping them out and back in again).
+ */
 void
 i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj)
 {
@@ -737,6 +751,14 @@ i915_gem_object_do_bit_17_swizzle(struct 
drm_i915_gem_object *obj)
}
 }
 
+/**
+ * i915_gem_object_save_bit_17_swizzle - save bit 17 swizzling
+ * @obj: i915 GEM buffer object
+ *
+ * This function saves the bit 17 of each page frame number so that swizzling
+ * can be fixed up later on with i915_gem_object_do_bit_17_swizzle(). This must
+ * be called before the backing storage can be unpinned.
+ */
 void
 i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c 
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index fa7a8d7a24e0..ac3eb566c9d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -31,53 +31,31 @@
 #include drm/i915_drm.h
 #include i915_drv.h
 
-/** @file i915_gem_tiling.c
- *
- * Support for managing tiling state of buffer objects.
- *
- * The idea behind tiling is to increase cache hit rates by rearranging
- * pixel data so that a group of pixel accesses are in the same cacheline.
- * Performance improvement from doing this on the back/depth buffer are on
- * the order of 30%.
- *
- * Intel architectures make this 

[Intel-gfx] [PATCH 3/4] drm/i915: Move low-level swizzling code to i915_gem_fence.c

2015-07-24 Thread Daniel Vetter
It fits more with the low-level fence code, and this move leaves only
the userspace tiling ioctl handling in i915_gem_tiling.c.

Suggested-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h|   8 +-
 drivers/gpu/drm/i915/i915_gem_fence.c  | 268 +
 drivers/gpu/drm/i915/i915_gem_tiling.c | 219 ---
 3 files changed, 272 insertions(+), 223 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4c34fb15d0df..9aebf92132a5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3058,6 +3058,10 @@ void i915_gem_object_unpin_fence(struct 
drm_i915_gem_object *obj);
 
 void i915_gem_restore_fences(struct drm_device *dev);
 
+void i915_gem_detect_bit_6_swizzle(struct drm_device *dev);
+void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj);
+void i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj);
+
 /* i915_gem_context.c */
 int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
@@ -3150,10 +3154,6 @@ static inline bool 
i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_objec
obj-tiling_mode != I915_TILING_NONE;
 }
 
-void i915_gem_detect_bit_6_swizzle(struct drm_device *dev);
-void i915_gem_object_do_bit_17_swizzle(struct drm_i915_gem_object *obj);
-void i915_gem_object_save_bit_17_swizzle(struct drm_i915_gem_object *obj);
-
 /* i915_gem_debug.c */
 #if WATCH_LISTS
 int i915_verify_lists(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c 
b/drivers/gpu/drm/i915/i915_gem_fence.c
index 0434c42d8c11..c643260a90c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -495,3 +495,271 @@ void i915_gem_restore_fences(struct drm_device *dev)
}
}
 }
+
+/**
+ *
+ * Support for managing tiling state of buffer objects.
+ *
+ * The idea behind tiling is to increase cache hit rates by rearranging
+ * pixel data so that a group of pixel accesses are in the same cacheline.
+ * Performance improvement from doing this on the back/depth buffer are on
+ * the order of 30%.
+ *
+ * Intel architectures make this somewhat more complicated, though, by
+ * adjustments made to addressing of data when the memory is in interleaved
+ * mode (matched pairs of DIMMS) to improve memory bandwidth.
+ * For interleaved memory, the CPU sends every sequential 64 bytes
+ * to an alternate memory channel so it can get the bandwidth from both.
+ *
+ * The GPU also rearranges its accesses for increased bandwidth to interleaved
+ * memory, and it matches what the CPU does for non-tiled.  However, when tiled
+ * it does it a little differently, since one walks addresses not just in the
+ * X direction but also Y.  So, along with alternating channels when bit
+ * 6 of the address flips, it also alternates when other bits flip --  Bits 9
+ * (every 512 bytes, an X tile scanline) and 10 (every two X tile scanlines)
+ * are common to both the 915 and 965-class hardware.
+ *
+ * The CPU also sometimes XORs in higher bits as well, to improve
+ * bandwidth doing strided access like we do so frequently in graphics.  This
+ * is called Channel XOR Randomization in the MCH documentation.  The result
+ * is that the CPU is XORing in either bit 11 or bit 17 to bit 6 of its address
+ * decode.
+ *
+ * All of this bit 6 XORing has an effect on our memory management,
+ * as we need to make sure that the 3d driver can correctly address object
+ * contents.
+ *
+ * If we don't have interleaved memory, all tiling is safe and no swizzling is
+ * required.
+ *
+ * When bit 17 is XORed in, we simply refuse to tile at all.  Bit
+ * 17 is not just a page offset, so as we page an objet out and back in,
+ * individual pages in it will have different bit 17 addresses, resulting in
+ * each 64 bytes being swapped with its neighbor!
+ *
+ * Otherwise, if interleaved, we have to tell the 3d driver what the address
+ * swizzling it needs to do is, since it's writing with the CPU to the pages
+ * (bit 6 and potentially bit 11 XORed in), and the GPU is reading from the
+ * pages (bit 6, 9, and 10 XORed in), resulting in a cumulative bit swizzling
+ * required by the CPU of XORing in bit 6, 9, 10, and potentially 11, in order
+ * to match what the GPU expects.
+ */
+
+/**
+ * Detects bit 6 swizzling of address lookup between IGD access and CPU
+ * access through main memory.
+ */
+void
+i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
+   uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
+
+   if (INTEL_INFO(dev)-gen = 8 || IS_VALLEYVIEW(dev)) {
+   /*
+* On BDW+, 

[Intel-gfx] [PATCH 1/4] drm/i915: kerneldoc for fences

2015-07-24 Thread Daniel Vetter
v2: Clarify that this is about fence _registers_. Also clarify that
the fence code revokes cpu ptes and not gtt ptes. Both suggested by
Chris.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 Documentation/DocBook/drm.tmpl|  5 +++
 drivers/gpu/drm/i915/i915_gem_fence.c | 75 +++
 2 files changed, 80 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 86680f074111..d7dc91b7e98c 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4051,6 +4051,11 @@ int num_ioctls;/synopsis
 !Idrivers/gpu/drm/i915/i915_gem_gtt.c
   /sect2
   sect2
+titleGlobal GTT Fence Handling/title
+!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence register handling
+!Idrivers/gpu/drm/i915/i915_gem_fence.c
+  /sect2
+  sect2
 titleBuffer Object Eviction/title
para
  This section documents the interface functions for evicting buffer
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c 
b/drivers/gpu/drm/i915/i915_gem_fence.c
index d3284ee04794..0434c42d8c11 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -25,6 +25,36 @@
 #include drm/i915_drm.h
 #include i915_drv.h
 
+/**
+ * DOC: fence register handling
+ *
+ * Important to avoid confusions: fences in the i915 driver are not execution
+ * fences used to track command completion but hardware detiler objects which
+ * wrap a given range of the global GTT. Each platform has only a fairly 
limited
+ * set of these objects.
+ *
+ * Fences are used to detile GTT memory mappings. They're also connected to the
+ * hardware frontbuffer render tracking and hence interract with frontbuffer
+ * conmpression. Furthermore on older platforms fences are required for tiled
+ * objects used by the display engine. They can also be used by the render
+ * engine - they're required for blitter commands and are optional for render
+ * commands. But on gen4+ both display (with the exception of fbc) and 
rendering
+ * have their own tiling state bits and don't need fences.
+ *
+ * Also note that fences only support X and Y tiling and hence can't be used 
for
+ * the fancier new tiling formats like W, Ys and Yf.
+ *
+ * Finally note that because fences are such a restricted resource they're
+ * dynamically associated with objects. Furthermore fence state is committed to
+ * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code 
must
+ * explictly call i915_gem_object_get_fence() to synchronize fencing status
+ * for cpu access. Also note that some code wants an unfenced view, for those
+ * cases the fence can be removed forcefully with i915_gem_object_put_fence().
+ *
+ * Internally these functions will synchronize with userspace access by 
removing
+ * CPU ptes into GTT mmaps (not the GTT ptes themselves) as needed.
+ */
+
 static void i965_write_fence_reg(struct drm_device *dev, int reg,
 struct drm_i915_gem_object *obj)
 {
@@ -247,6 +277,17 @@ i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
return 0;
 }
 
+/**
+ * i915_gem_object_put_fence - force-remove fence for an object
+ * @obj: object to map through a fence reg
+ *
+ * This function force-removes any fence from the given object, which is useful
+ * if the kernel wants to do untiled GTT access.
+ *
+ * Returns:
+ *
+ * 0 on success, negative error code on failure.
+ */
 int
 i915_gem_object_put_fence(struct drm_i915_gem_object *obj)
 {
@@ -322,6 +363,10 @@ deadlock:
  * and tiling format.
  *
  * For an untiled surface, this removes any existing fence.
+ *
+ * Returns:
+ *
+ * 0 on success, negative error code on failure.
  */
 int
 i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
@@ -374,6 +419,21 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
return 0;
 }
 
+/**
+ * i915_gem_object_pin_fence - pin fencing state
+ * @obj: object to pin fencing for
+ *
+ * This pins the fencing state (whether tiled or untiled) to make sure the
+ * object is ready to be used as a scanout target. Fencing status must be
+ * synchronize first by calling i915_gem_object_get_fence():
+ *
+ * The resulting fence pin reference must be released again with
+ * i915_gem_object_unpin_fence().
+ *
+ * Returns:
+ *
+ * True if the object has a fence, false otherwise.
+ */
 bool
 i915_gem_object_pin_fence(struct drm_i915_gem_object *obj)
 {
@@ -390,6 +450,14 @@ i915_gem_object_pin_fence(struct drm_i915_gem_object *obj)
return false;
 }
 
+/**
+ * i915_gem_object_unpin_fence - unpin fencing state
+ * @obj: object to unpin fencing for
+ *
+ * This releases the fence pin reference acquired through
+ * i915_gem_object_pin_fence. It will handle both objects with and without an
+ * attached fence correctly, callers do not need to distinguish this.
+ */
 void
 i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)

Re: [Intel-gfx] [PATCH 2/7] drm/i915: Try to stop sink crc calculation on error.

2015-07-24 Thread Rafael Antognolli
On Thu, Jul 23, 2015 at 04:35:45PM -0700, Rodrigo Vivi wrote:
 Right now if we face any kind of error sink crc calculation
 stays enabled.
 
 So, let's give a shot and try to stop it anyway if it got enabled.
 
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index f1b9f93..70a4a37 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -3994,7 +3994,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
 *crc)
  
   if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK_MISC, buf)  0) {
   ret = -EIO;
 - goto out;
 + goto stop;
   }
  
   test_crc_count = buf  DP_TEST_COUNT_MASK;
 @@ -4003,7 +4003,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
 *crc)
   if (drm_dp_dpcd_readb(intel_dp-aux,
 DP_TEST_SINK_MISC, buf)  0) {
   ret = -EIO;
 - goto out;
 + goto stop;
   }
   intel_wait_for_vblank(dev, intel_crtc-pipe);
   } while (--attempts  (buf  DP_TEST_COUNT_MASK) == test_crc_count);
 @@ -4011,14 +4011,15 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
 *crc)
   if (attempts == 0) {
   DRM_DEBUG_KMS(Panel is unable to calculate CRC after 6 
 vblanks\n);
   ret = -ETIMEDOUT;
 - goto out;
 + goto stop;
   }
  
   if (drm_dp_dpcd_read(intel_dp-aux, DP_TEST_CRC_R_CR, crc, 6)  0) {
   ret = -EIO;
 - goto out;
 + goto stop;
   }
  
 +stop:
   if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf)  0) {
   ret = -EIO;
   goto out;

Nice cleanup on exit.

Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com


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


Re: [Intel-gfx] [PATCH 3/7] drm/i915: Don't return error on sink crc stop.

2015-07-24 Thread Rafael Antognolli
On Thu, Jul 23, 2015 at 04:35:46PM -0700, Rodrigo Vivi wrote:
 If we got to the point where we are trying to stop sink CRC
 the main output of this function was already gotten properly,
 so don't return the error and let userspace use the crc data.
 
 Let's replace the errnos returns with some log messages.

So, up to this commit, there's no way to know that an error has ocurred
and the next CRC calculation can go wrong.

I know that in a follow up patch this is fixed by trying to stop the
calculation at the beginning too, but just pointing out that this one
specifically has this problem.

Not sure if this is a problem though, since the patches are submitted
together. If not, then

Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com


 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/intel_dp.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 70a4a37..44f8a32 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -4021,12 +4021,12 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 
 *crc)
  
  stop:
   if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf)  0) {
 - ret = -EIO;
 + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n);
   goto out;
   }
   if (drm_dp_dpcd_writeb(intel_dp-aux, DP_TEST_SINK,
  buf  ~DP_TEST_SINK_START)  0) {
 - ret = -EIO;
 + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n);
   goto out;
   }
  out:
 -- 
 2.1.0
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 3/7] drm/i915: Don't return error on sink crc stop.

2015-07-24 Thread Rodrigo Vivi
On Fri, Jul 24, 2015 at 11:25 AM Rafael Antognolli 
rafael.antogno...@intel.com wrote:

 On Thu, Jul 23, 2015 at 04:35:46PM -0700, Rodrigo Vivi wrote:
  If we got to the point where we are trying to stop sink CRC
  the main output of this function was already gotten properly,
  so don't return the error and let userspace use the crc data.
 
  Let's replace the errnos returns with some log messages.

 So, up to this commit, there's no way to know that an error has ocurred
 and the next CRC calculation can go wrong.

 I know that in a follow up patch this is fixed by trying to stop the
 calculation at the beginning too, but just pointing out that this one
 specifically has this problem.


Actually it isn't a problem if crc calculation continue enabled. Mainly
with the mask s/0x7/0xf fix.
But it is good to disable and force counter reset anyway. (the following
patch you mentioned)

This patch here is just to highlight that errors during read has more
priority than errors on stopping crc.

Another way would be to create a stop_ret and
if (!ret and stop_ret)
 ret = stop_ret;

But I decided this cleaned version would be better.



 Not sure if this is a problem though, since the patches are submitted
 together. If not, then

 Reviewed-by: Rafael Antognolli rafael.antogno...@intel.com


Thanks!




  Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
  ---
   drivers/gpu/drm/i915/intel_dp.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_dp.c
 b/drivers/gpu/drm/i915/intel_dp.c
  index 70a4a37..44f8a32 100644
  --- a/drivers/gpu/drm/i915/intel_dp.c
  +++ b/drivers/gpu/drm/i915/intel_dp.c
  @@ -4021,12 +4021,12 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp,
 u8 *crc)
 
   stop:
if (drm_dp_dpcd_readb(intel_dp-aux, DP_TEST_SINK, buf)  0) {
  - ret = -EIO;
  + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n);
goto out;
}
if (drm_dp_dpcd_writeb(intel_dp-aux, DP_TEST_SINK,
   buf  ~DP_TEST_SINK_START)  0) {
  - ret = -EIO;
  + DRM_DEBUG_KMS(Sink CRC couldn't be stopped properly\n);
goto out;
}
   out:
  --
  2.1.0
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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


Re: [Intel-gfx] [PATCH v3 0/4] i915 to call hda driver on HDMI plug/unplug

2015-07-24 Thread Takashi Iwai
On Thu, 23 Jul 2015 17:26:15 +0200,
David Henningsson wrote:
 
 Changes since v2 is that the patch set has diminished to not transfer
 connector/eld information, since it might be better that such information
 to be transferred by a separate call in the ordinary direction. That
 could be added in a later patch set.
 
 The audio_ptr is now a void* (instead of a hdac_bus*) so that the audio
 side can easily refactor the ptr to point to the codec instead of the bus
 if we would find that beneficial.

OK, that leaves some room if we want to change in future.

I still consider whether it's better to register from the codec driver
or from the bus driver.  The codec driver might be a bit cleaner, but
then it'd be a call like
snd_hda_i915_register_notifier()
instead of adding a callback in the codec ops.

What I see a bit ugly in the current patchset is the addition of
i915 specific callback to a generic hda codec ops.  If the codec
registers the callback dynamically like above, the pointer will be
stored in component directly, so we don't need it in codec ops.
(But we'd need to unregister at codec removal, too.)

Or, if we keep it as a codec ops callback, as already mentioned, it
might make more sense to define it as a generic notifier, i.e. drop
i915 prefix and pass some event type to identify the event.


thanks,

Takashi

 David Henningsson (4):
   drm/i915: Add audio hotplug info callback
   drm/i915: Call audio hotplug notify function
   ALSA: hda - Dispatch incoming HDMI hotplug i915 callback
   ALSA: hda - Wake the codec up on hotplug notify events
 
  drivers/gpu/drm/i915/i915_drv.h|1 +
  drivers/gpu/drm/i915/intel_audio.c |   23 ---
  include/drm/i915_component.h   |5 +
  include/sound/hdaudio.h|4 
  sound/hda/hdac_i915.c  |   23 +++
  sound/pci/hda/patch_hdmi.c |   13 -
  6 files changed, 65 insertions(+), 4 deletions(-)
 
 -- 
 1.7.9.5
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences

2015-07-24 Thread shuang . he
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@intel.com)
Task id: 6855
-Summary-
Platform  Delta  drm-intel-nightly  Series Applied
ILK  295/295  295/295
SNB  315/315  315/315
IVB  342/342  342/342
BYT -2  285/285  283/285
HSW  378/378  378/378
-Detailed-
Platform  Testdrm-intel-nightly  Series 
Applied
*BYT  igt@gem_partial_pwrite_pread@reads  PASS(1)  FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-display  PASS(1)  FAIL(1)
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/13 v4] drm/i915: Expose two LRC functions for GuC submission mode

2015-07-24 Thread O'Rourke, Tom
On Thu, Jul 09, 2015 at 07:29:07PM +0100, Dave Gordon wrote:
 GuC submission is basically execlist submission, but with the GuC
 handling the actual writes to the ELSP and the resulting context
 switch interrupts. So to prepare a context for submission via the
 GuC, we need some of the same functions used in execlist mode.
 This commit exposes two such functions, changing their names to
 better describe what they do (they're related to logical ring
 contexts rather than to execlists per se).
 
 v2:
 Replaces previous drm/i915: Move execlists defines from .c to .h
 
 v3:
 Incorporates a change to one of the functions exposed here that was
 previously part of an internal patch, but which was omitted from
 the version recently committed to drm-intel-nightly:
   7a01a0a drm/i915/lrc: Update PDPx registers with lri commands
 So we reinstate this change here.
 
 v4:
 Drop v3 change, update function parameters due to collision with
 8ee3615 drm/i915: Convert execlists_ctx_descriptor() for requests
 
 Issue: VIZ-4884
 Signed-off-by: Dave Gordon david.s.gor...@intel.com


Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 ---
  drivers/gpu/drm/i915/intel_lrc.c | 21 ++---
  drivers/gpu/drm/i915/intel_lrc.h |  3 +++
  2 files changed, 13 insertions(+), 11 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
 b/drivers/gpu/drm/i915/intel_lrc.c
 index d4f8b43..9e121d3 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.c
 +++ b/drivers/gpu/drm/i915/intel_lrc.c
 @@ -261,11 +261,11 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object 
 *ctx_obj)
   return lrca  12;
  }
  
 -static uint64_t execlists_ctx_descriptor(struct drm_i915_gem_request *rq)
 +uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
 +  struct intel_engine_cs *ring)
  {
 - struct intel_engine_cs *ring = rq-ring;
   struct drm_device *dev = ring-dev;
 - struct drm_i915_gem_object *ctx_obj = rq-ctx-engine[ring-id].state;
 + struct drm_i915_gem_object *ctx_obj = ctx-engine[ring-id].state;
   uint64_t desc;
   uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj);
  
 @@ -303,13 +303,13 @@ static void execlists_elsp_write(struct 
 drm_i915_gem_request *rq0,
   uint64_t desc[2];
  
   if (rq1) {
 - desc[1] = execlists_ctx_descriptor(rq1);
 + desc[1] = intel_lr_context_descriptor(rq1-ctx, rq1-ring);
   rq1-elsp_submitted++;
   } else {
   desc[1] = 0;
   }
  
 - desc[0] = execlists_ctx_descriptor(rq0);
 + desc[0] = intel_lr_context_descriptor(rq0-ctx, rq0-ring);
   rq0-elsp_submitted++;
  
   /* You must always write both descriptors in the order below. */
 @@ -328,7 +328,8 @@ static void execlists_elsp_write(struct 
 drm_i915_gem_request *rq0,
   spin_unlock(dev_priv-uncore.lock);
  }
  
 -static int execlists_update_context(struct drm_i915_gem_request *rq)
 +/* Update the ringbuffer pointer and tail offset in a saved context image */
 +void intel_lr_context_update(struct drm_i915_gem_request *rq)
  {
   struct intel_engine_cs *ring = rq-ring;
   struct i915_hw_ppgtt *ppgtt = rq-ctx-ppgtt;
 @@ -358,17 +359,15 @@ static int execlists_update_context(struct 
 drm_i915_gem_request *rq)
   }
  
   kunmap_atomic(reg_state);
 -
 - return 0;
  }
  
  static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
 struct drm_i915_gem_request *rq1)
  {
 - execlists_update_context(rq0);
 + intel_lr_context_update(rq0);
  
   if (rq1)
 - execlists_update_context(rq1);
 + intel_lr_context_update(rq1);
  
   execlists_elsp_write(rq0, rq1);
  }
 @@ -2051,7 +2050,7 @@ populate_lr_context(struct intel_context *ctx, struct 
 drm_i915_gem_object *ctx_o
   reg_state[CTX_RING_TAIL+1] = 0;
   reg_state[CTX_RING_BUFFER_START] = RING_START(ring-mmio_base);
   /* Ring buffer start address is not known until the buffer is pinned.
 -  * It is written to the context image in execlists_update_context()
 +  * It is written to the context image in intel_lr_context_update()
*/
   reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring-mmio_base);
   reg_state[CTX_RING_BUFFER_CONTROL+1] =
 diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
 b/drivers/gpu/drm/i915/intel_lrc.h
 index e0299fb..6ecc0b3 100644
 --- a/drivers/gpu/drm/i915/intel_lrc.h
 +++ b/drivers/gpu/drm/i915/intel_lrc.h
 @@ -73,6 +73,9 @@ int intel_lr_context_deferred_create(struct intel_context 
 *ctx,
  void intel_lr_context_unpin(struct drm_i915_gem_request *req);
  void intel_lr_context_reset(struct drm_device *dev,
   struct intel_context *ctx);
 +void intel_lr_context_update(struct drm_i915_gem_request *rq);
 +uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
 +  struct intel_engine_cs *ring);
  
  /* Execlists */
  int 

[Intel-gfx] [PATCH 1/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

2015-07-24 Thread Rodrigo Vivi
Since active function on VLV immediately activate PSR let's give more
time for idleness. Different from core platforms where we have idle_frames
count.

Also kms_psr_sink_crc now is automated and always get this:

[drm:intel_enable_pipe] enabling pipe A
[drm:intel_edp_backlight_on]
[drm:intel_panel_enable_backlight] pipe
[drm:intel_panel_enable_backlight] pipe A
[drm:intel_panel_actually_set_backlight] set backlight PWM = 7812

PSR gets enabled somewhere here after backlight.

[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0
[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp

PSR gets flushed around here by intel_atomic_commit

[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
[drm:intel_set_memory_cxsr] memory self-refresh is enabled
[drm:intel_connector_check_state] [CONNECTOR:39:eDP-1]
[drm:check_encoder_state] [ENCODER:30:DAC-30]
[drm:check_encoder_state] [ENCODER:31:TMDS-31]
[drm:check_encoder_state] [ENCODER:36:TMDS-36]
[drm:check_encoder_state] [ENCODER:38:TMDS-38]
[drm:check_crtc_state] [CRTC:21]
[drm:check_crtc_state] [CRTC:26]
[drm:intel_psr_activate [i915]] *ERROR* PSR Active
[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x
[drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun
[drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO
Underrun.

It is true that in a product we won't keep disabling and enabling planes so
frequently, but for safeness let's stay conservative.

It is also true that 500ms is an etternity. But PSR is anyway a power saving
feature for idle scenario. So if it is idle feature stays on and 500ms to get
it reanabled is not that insane.

v2: Rebase over intel_psr.c and fix typo.
v3: Revival: Manual tests indicated that this is needed. With a short delay
there is a huge risk of getting blank screens when planes are being enabled.
v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but
actually time for link training what we aren't doing, but with only 100 sec
in some cases kms_psr_sink_crc manual was showing blank screen,
so let's use this for now. Also changed comment by a FIXME.
v5: Rebase after a long time, remove FIXME and update comment above.

Reviewed-by: Durgadoss R durgados...@intel.com (v4)
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_psr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index acd8ec8..0f446b7 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
+   int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500);
 
mutex_lock(dev_priv-psr.lock);
if (!dev_priv-psr.enabled) {
@@ -733,7 +734,7 @@ void intel_psr_flush(struct drm_device *dev,
 
if (!dev_priv-psr.active  !dev_priv-psr.busy_frontbuffer_bits)
schedule_delayed_work(dev_priv-psr.work,
- msecs_to_jiffies(100));
+ msecs_to_jiffies(delay));
mutex_unlock(dev_priv-psr.lock);
 }
 
-- 
1.9.3

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


[Intel-gfx] [PATCH 2/2] drm/i915: Also call frontbuffer flip when disabling planes.

2015-07-24 Thread Rodrigo Vivi
We also need to call the frontbuffer flip to trigger proper
invalidations when disabling planes. Otherwise we will miss
screen updates when disabling sprites or cursor.

It was caught with kms_psr_sink_crc sprite_plane_onoff
and cursor_plane_onoff subtests.

This is probably a regression since I can also get this
with the manual test case, but with so many changes on atomic
modeset I couldn't track exactly when this was introduced.

Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index af0bcfe..bb124cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11716,7 +11716,7 @@ int intel_plane_atomic_calc_changes(struct 
drm_crtc_state *crtc_state,
intel_crtc-atomic.update_wm_pre = true;
}
 
-   if (visible)
+   if (visible || was_visible)
intel_crtc-atomic.fb_bits |=
to_intel_plane(plane)-frontbuffer_bit;
 
-- 
1.9.3

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


[Intel-gfx] [PATCH] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

2015-07-24 Thread Rodrigo Vivi
Since active function on VLV immediately activate PSR let's give more
time for idleness. Different from core platforms where we have idle_frames
count.

Also kms_psr_sink_crc now is automated and always get this:

[drm:intel_enable_pipe] enabling pipe A
[drm:intel_edp_backlight_on]
[drm:intel_panel_enable_backlight] pipe
[drm:intel_panel_enable_backlight] pipe A
[drm:intel_panel_actually_set_backlight] set backlight PWM = 7812

PSR gets enabled somewhere here after backlight.

[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x0
[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp

PSR gets flushed around here by intel_atomic_commit

[drm:vlv_pipe_set_fifo_size] Pipe A FIFO split 511 / 511 / 511
[drm:vlv_update_wm] Setting FIFO watermarks - A: plane=391, cursor=63, sp
[drm:intel_set_memory_cxsr] memory self-refresh is enabled
[drm:intel_connector_check_state] [CONNECTOR:39:eDP-1]
[drm:check_encoder_state] [ENCODER:30:DAC-30]
[drm:check_encoder_state] [ENCODER:31:TMDS-31]
[drm:check_encoder_state] [ENCODER:36:TMDS-36]
[drm:check_encoder_state] [ENCODER:38:TMDS-38]
[drm:check_crtc_state] [CRTC:21]
[drm:check_crtc_state] [CRTC:26]
[drm:intel_psr_activate [i915]] *ERROR* PSR Active
[drm:intel_get_hpd_pins] hotplug event received, stat 0x, dig 0x
[drm:intel_set_cpu_fifo_underrun_reporting [i915]] *ERROR* pipe A underrun
[drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO
Underrun.

It is true that in a product we won't keep disabling and enabling planes so
frequently, but for safeness let's stay conservative.

It is also true that 500ms is an etternity. But PSR is anyway a power saving
feature for idle scenario. So if it is idle feature stays on and 500ms to get
it reanabled is not that insane.

v2: Rebase over intel_psr.c and fix typo.
v3: Revival: Manual tests indicated that this is needed. With a short delay
there is a huge risk of getting blank screens when planes are being enabled.
v4: Revival 2 with reasonable delay. 1/2 sec instead of 5. VBT is 10 sec but
actually time for link training what we aren't doing, but with only 100 sec
in some cases kms_psr_sink_crc manual was showing blank screen,
so let's use this for now. Also changed comment by a FIXME.
v5: Rebase after a long time, remove FIXME and update comment above.
v6: msecs_to_jiffies is already on delay. remove duplication.

Reviewed-by: Durgadoss R durgados...@intel.com (v4)
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/intel_psr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index acd8ec8..bec13b8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -698,6 +698,7 @@ void intel_psr_flush(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
enum pipe pipe;
+   int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 500);
 
mutex_lock(dev_priv-psr.lock);
if (!dev_priv-psr.enabled) {
@@ -732,8 +733,7 @@ void intel_psr_flush(struct drm_device *dev,
}
 
if (!dev_priv-psr.active  !dev_priv-psr.busy_frontbuffer_bits)
-   schedule_delayed_work(dev_priv-psr.work,
- msecs_to_jiffies(100));
+   schedule_delayed_work(dev_priv-psr.work, delay);
mutex_unlock(dev_priv-psr.lock);
 }
 
-- 
1.9.3

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


Re: [Intel-gfx] [PATCH 08/13 v4] drm/i915: Enable GuC firmware log

2015-07-24 Thread O'Rourke, Tom
On Thu, Jul 09, 2015 at 07:29:09PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 Allocate a GEM object to hold GuC log data. A debugfs interface
 (i915_guc_log_dump) is provided to print out the log content.
 
 v2:
 Add struct members at point of use [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com

 ---
  drivers/gpu/drm/i915/i915_debugfs.c| 29 +++
  drivers/gpu/drm/i915/i915_guc_submission.c | 46 
 ++
  drivers/gpu/drm/i915/intel_guc.h   |  1 +
  3 files changed, 76 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 9ff5f17..13e37d1 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -2397,6 +2397,34 @@ static int i915_guc_load_status_info(struct seq_file 
 *m, void *data)
   return 0;
  }
  
 +static int i915_guc_log_dump(struct seq_file *m, void *data)
 +{
 + struct drm_info_node *node = m-private;
 + struct drm_device *dev = node-minor-dev;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + struct drm_i915_gem_object *log_obj = dev_priv-guc.log_obj;
 + u32 *log;
 + int i = 0, pg;
 +
 + if (!log_obj)
 + return 0;
 +
 + for (pg = 0; pg  log_obj-base.size / PAGE_SIZE; pg++) {
 + log = kmap_atomic(i915_gem_object_get_page(log_obj, pg));
 +
 + for (i = 0; i  PAGE_SIZE / sizeof(u32); i += 4)
 + seq_printf(m, 0x%08x 0x%08x 0x%08x 0x%08x\n,
 +*(log + i), *(log + i + 1),
 +*(log + i + 2), *(log + i + 3));
 +
 + kunmap_atomic(log);
 + }
 +
 + seq_putc(m, '\n');
 +
 + return 0;
 +}
 +
  static int i915_edp_psr_status(struct seq_file *m, void *data)
  {
   struct drm_info_node *node = m-private;
 @@ -5112,6 +5140,7 @@ static const struct drm_info_list i915_debugfs_list[] = 
 {
   {i915_gem_hws_vebox, i915_hws_info, 0, (void *)VECS},
   {i915_gem_batch_pool, i915_gem_batch_pool_info, 0},
   {i915_guc_load_status, i915_guc_load_status_info, 0},
 + {i915_guc_log_dump, i915_guc_log_dump, 0},
   {i915_frequency_info, i915_frequency_info, 0},
   {i915_hangcheck_info, i915_hangcheck_info, 0},
   {i915_drpc_info, i915_drpc_info, 0},
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index 70a0405..e9d46d6 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -75,6 +75,47 @@ static void gem_release_guc_obj(struct drm_i915_gem_object 
 *obj)
   drm_gem_object_unreference(obj-base);
  }
  
 +static void guc_create_log(struct intel_guc *guc)
 +{
 + struct drm_i915_private *dev_priv = guc_to_i915(guc);
 + struct drm_i915_gem_object *obj;
 + unsigned long offset;
 + uint32_t size, flags;
 +
 + if (i915.guc_log_level  GUC_LOG_VERBOSITY_MIN)
 + return;
 +
 + if (i915.guc_log_level  GUC_LOG_VERBOSITY_MAX)
 + i915.guc_log_level = GUC_LOG_VERBOSITY_MAX;
 +
 + /* The first page is to save log buffer state. Allocate one
 +  * extra page for others in case for overlap */
 + size = (1 + GUC_LOG_DPC_PAGES + 1 +
 + GUC_LOG_ISR_PAGES + 1 +
 + GUC_LOG_CRASH_PAGES + 1)  PAGE_SHIFT;
 +
 + obj = guc-log_obj;
 + if (!obj) {
 + obj = gem_allocate_guc_obj(dev_priv-dev, size);
 + if (!obj) {
 + /* logging will be off */
 + i915.guc_log_level = -1;
 + return;
 + }
 +
 + guc-log_obj = obj;
 + }
 +
 + /* each allocated unit is a page */
 + flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL |
 + (GUC_LOG_DPC_PAGES  GUC_LOG_DPC_SHIFT) |
 + (GUC_LOG_ISR_PAGES  GUC_LOG_ISR_SHIFT) |
 + (GUC_LOG_CRASH_PAGES  GUC_LOG_CRASH_SHIFT);
[TOR:] How does the log half full interrupt get handled?

 +
 + offset = i915_gem_obj_ggtt_offset(obj)  PAGE_SHIFT; /* in pages */
 + guc-log_flags = (offset  GUC_LOG_BUF_ADDR_SHIFT) | flags;
 +}
 +
  /*
   * Set up the memory resources to be shared with the GuC.  At this point,
   * we require just one object that can be mapped through the GGTT.
 @@ -99,6 +140,8 @@ int i915_guc_submission_init(struct drm_device *dev)
  
   ida_init(guc-ctx_ids);
  
 + guc_create_log(guc);
 +
   return 0;
  }
  
 @@ -107,6 +150,9 @@ void i915_guc_submission_fini(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_guc *guc = dev_priv-guc;
  
 + gem_release_guc_obj(dev_priv-guc.log_obj);
 + guc-log_obj = NULL;
 +
   if (guc-ctx_pool_obj)
   

Re: [Intel-gfx] [PATCH 07/13 v4] drm/i915: GuC submission setup, phase 1

2015-07-24 Thread O'Rourke, Tom
[TOR:] When I see phase 1 I also look for phase 2.  
A subject that better describes the change in this patch 
would help.

On Thu, Jul 09, 2015 at 07:29:08PM +0100, Dave Gordon wrote:
 From: Alex Dai yu@intel.com
 
 This adds the first of the data structures used to communicate with the
 GuC (the pool of guc_context structures).
 
 We create a GuC-specific wrapper round the GEM object allocator as all
 GEM objects shared with the GuC must be pinned into GGTT space at an
 address that is NOT in the range [0..WOPCM_SIZE), as that range of GGTT
 addresses is not accessible to the GuC (from the GuC's point of view,
 it's permanently reserved for other objects such as the BootROM  SRAM).
[TOR:] I would like a clarfication on the excluded range.
The excluded range should be 0 to size for guc within
WOPCM area and not 0 to size of WOPCM area.

 
 Later, we will need to allocate additional GuC-sharable objects for the
 submission client(s) and the GuC's debug log.
 
 v2:
 Remove redundant initialisation [Chris Wilson]
 Defer adding struct members until needed [Chris Wilson]
 Local functions should pass dev_priv rather than dev [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com
 ---
  drivers/gpu/drm/i915/Makefile  |   3 +-
  drivers/gpu/drm/i915/i915_guc_submission.c | 114 
 +
  drivers/gpu/drm/i915/intel_guc.h   |   7 ++
  drivers/gpu/drm/i915/intel_guc_loader.c|  19 +
  4 files changed, 142 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.c
 
 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
 index e604cfe..23f5612 100644
 --- a/drivers/gpu/drm/i915/Makefile
 +++ b/drivers/gpu/drm/i915/Makefile
 @@ -40,7 +40,8 @@ i915-y += i915_cmd_parser.o \
 intel_uncore.o
  
  # general-purpose microcontroller (GuC) support
 -i915-y += intel_guc_loader.o
 +i915-y += intel_guc_loader.o \
 +   i915_guc_submission.o
  
  # autogenerated null render state
  i915-y += intel_renderstate_gen6.o \
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 new file mode 100644
 index 000..70a0405
 --- /dev/null
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -0,0 +1,114 @@
 +/*
 + * Copyright © 2014 Intel Corporation
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice (including the next
 + * paragraph) shall be included in all copies or substantial portions of the
 + * Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
 DEALINGS
 + * IN THE SOFTWARE.
 + *
 + */
 +#include linux/firmware.h
 +#include linux/circ_buf.h
 +#include i915_drv.h
 +#include intel_guc.h
 +
 +/**
 + * gem_allocate_guc_obj() - Allocate gem object for GuC usage
 + * @dev: drm device
 + * @size:size of object
 + *
 + * This is a wrapper to create a gem obj. In order to use it inside GuC, the
 + * object needs to be pinned lifetime. Also we must pin it to gtt space other
 + * than [0, GUC_WOPCM_SIZE] because this range is reserved inside GuC.
 + *
 + * Return:   A drm_i915_gem_object if successful, otherwise NULL.
 + */
 +static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device 
 *dev,
 + u32 size)
 +{
 + struct drm_i915_gem_object *obj;
 +
 + obj = i915_gem_alloc_object(dev, size);
 + if (!obj)
 + return NULL;
 +
 + if (i915_gem_object_get_pages(obj)) {
 + drm_gem_object_unreference(obj-base);
 + return NULL;
 + }
 +
 + if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
 + PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) {
 + drm_gem_object_unreference(obj-base);
 + return NULL;
 + }
 +
 + return obj;
 +}
 +
 +/**
 + * gem_release_guc_obj() - Release gem object allocated for GuC usage
 + * @obj: gem obj to be released
 +  */
 +static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 +{
 +

Re: [Intel-gfx] [PATCH v1.1 1/5] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2.

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 04:26:27PM +0300, Ander Conselvan De Oliveira wrote:
 On Tue, 2015-07-21 at 16:09 +0200, Maarten Lankhorst wrote:
  -EDEADLK has special meaning in atomic, but get_fence may call
  i915_find_fence_reg which can return -EDEADLK.
  
  This has special meaning in the atomic world, so convert the error
  to -EBUSY for this case.
 
 Doesn't this change the behavior of intel_crtc_page_flip() slightly? It
 now would return -EBUSY to user space if it can't find a fence instead
 of -EDEADLK. Not sure if that is a problem though. I don't expect user
 space would actually check for -EDEADLK.

It's extreme, but we could fix the -EDEADLK by dropping the struct_mutex
and flushing the unpin workqueue (there is no other conceivable way of
triggering EDEADLK here other than uncompleted flips except for a kernel
bug ofc). I can confidently state that no one has or ever will see an
EDEADLK here - and even if it did userspace is best to treat it exactly
the same as any other error path, cancel the flip and try again later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/13 v4] drm/i915: Add GuC-related header files

2015-07-24 Thread O'Rourke, Tom
On Tue, Jul 21, 2015 at 08:38:35AM +0200, Daniel Vetter wrote:
 On Fri, Jul 17, 2015 at 05:38:05PM -0700, O'Rourke, Tom wrote:
  On Thu, Jul 09, 2015 at 07:29:04PM +0100, Dave Gordon wrote:
   intel_guc_fwif.h contains the subset of the GuC interface that we
   will need for submission of commands through the GuC. These MUST
   be kept in sync with the definitions used by the GuC firmware, and
   updates to this file will (or should) be autogenerated from the
   source files used to build the firmware. Editing this file is
   therefore not recommended.
   
   i915_guc_reg.h contains definitions of GuC-related hardware:
   registers, bitmasks, etc. These should match the BSpec.
   
   v2:
   Files renamed  resliced per review comments by Chris Wilson
   
   v4:
   Added DON'T-EDIT-ME warning [Tom O'Rourke]
   
   Issue: VIZ-4884
   Signed-off-by: Alex Dai yu@intel.com
   Signed-off-by: Dave Gordon david.s.gor...@intel.com
   ---
  Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 
 Merged up to this patch, thanks.
 -Daniel

[TOR:] Thank you. Please hold merging remaining patches in
this series until guc firmware v3 is available on 01.org.
Tom
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v2 00/25] replace ioremap_{cache|wt} with memremap

2015-07-24 Thread Dan Williams
Changes since v1 [1]:

1/ Drop the attempt at unifying ioremap() prototypes, just focus on
   converting ioremap_cache and ioremap_wt over to memremap (Christoph)

2/ Drop the unrelated cleanups to use %pa in __ioremap_caller (Thomas)

3/ Add support for memremap() attempts on System RAM to simply return
   the kernel virtual address for that range.  ARM depends on this
   functionality in ioremap_cache() and ACPI was open coding a similar
   solution. (Mark)

4/ Split the conversions of ioremap_{cache|wt} into separate patches per
   driver / arch.

5/ Fix bisection breakage and other reports from 0day-kbuild

---
While developing the pmem driver we noticed that the __iomem annotation
on the return value from ioremap_cache() was being mishandled by several
callers.  We also observed that all of the call sites expected to be
able to treat the return value from ioremap_cache() as normal
(non-__iomem) pointer to memory.

This patchset takes the opportunity to clean up the above confusion as
well as a few issues with the ioremap_{cache|wt} interface, including:

1/ Eliminating the possibility of function prototypes differing between
   architectures by defining a central memremap() prototype that takes
   flags to determine the mapping type.

2/ Returning NULL rather than falling back silently to a different
   mapping-type.  This allows drivers to be stricter about the
   mapping-type fallbacks that are permissible.

[1]: http://marc.info/?l=linux-arm-kernelm=143735199029255w=2

---

Dan Williams (22):
  mm: enhance region_is_ram() to distinguish 'unknown' vs 'mixed'
  arch, drivers: don't include asm/io.h directly, use linux/io.h instead
  cleanup IORESOURCE_CACHEABLE vs ioremap()
  intel_iommu: fix leaked ioremap mapping
  arch: introduce memremap()
  arm: switch from ioremap_cache to memremap
  x86: switch from ioremap_cache to memremap
  gma500: switch from acpi_os_ioremap to ioremap
  i915: switch from acpi_os_ioremap to ioremap
  acpi: switch from ioremap_cache to memremap
  toshiba laptop: replace ioremap_cache with ioremap
  memconsole: fix __iomem mishandling, switch to memremap
  visorbus: switch from ioremap_cache to memremap
  intel-iommu: switch from ioremap_cache to memremap
  libnvdimm, pmem: switch from ioremap_cache to memremap
  pxa2xx-flash: switch from ioremap_cache to memremap
  sfi: switch from ioremap_cache to memremap
  fbdev: switch from ioremap_wt to memremap
  pmem: switch from ioremap_wt to memremap
  arch: remove ioremap_cache, replace with arch_memremap
  arch: remove ioremap_wt, replace with arch_memremap
  pmem: convert to generic memremap

Toshi Kani (3):
  mm, x86: Fix warning in ioremap RAM check
  mm, x86: Remove region_is_ram() call from ioremap
  mm: Fix bugs in region_is_ram()


 arch/arc/include/asm/io.h  |1 
 arch/arm/Kconfig   |1 
 arch/arm/include/asm/io.h  |   13 +++-
 arch/arm/include/asm/xen/page.h|4 +
 arch/arm/mach-clps711x/board-cdb89712.c|2 -
 arch/arm/mach-shmobile/pm-rcar.c   |2 -
 arch/arm/mm/ioremap.c  |   12 +++-
 arch/arm/mm/nommu.c|   11 ++-
 arch/arm64/Kconfig |1 
 arch/arm64/include/asm/acpi.h  |   10 +--
 arch/arm64/include/asm/dmi.h   |8 +--
 arch/arm64/include/asm/io.h|8 ++-
 arch/arm64/kernel/efi.c|9 ++-
 arch/arm64/kernel/smp_spin_table.c |   19 +++---
 arch/arm64/mm/ioremap.c|   20 ++
 arch/avr32/include/asm/io.h|1 
 arch/frv/Kconfig   |1 
 arch/frv/include/asm/io.h  |   17 ++---
 arch/frv/mm/kmap.c |6 ++
 arch/ia64/Kconfig  |1 
 arch/ia64/include/asm/io.h |   11 +++
 arch/ia64/kernel/cyclone.c |2 -
 arch/m32r/include/asm/io.h |1 
 arch/m68k/Kconfig  |1 
 arch/m68k/include/asm/io_mm.h  |   14 +---
 arch/m68k/include/asm/io_no.h  |   12 ++--
 arch/m68k/include/asm/raw_io.h |4 +
 arch/m68k/mm/kmap.c|   17 +
 arch/m68k/mm/sun3kmap.c|6 ++
 arch/metag/include/asm/io.h|3 -
 arch/microblaze/include/asm/io.h   |1 
 arch/mn10300/include/asm/io.h  |1 
 arch/nios2/include/asm/io.h|1 
 arch/powerpc/kernel/pci_of_scan.c  |2 -
 arch/s390/include/asm/io.h |1 
 arch/sh/Kconfig|1 
 arch/sh/include/asm/io.h   |  

[Intel-gfx] [PATCH] benchmarks: Do not install to system-wide bin/

2015-07-24 Thread Chris Wilson
These benchmarks are first-and-foremost development tools, not aimed at
general users. As such they should not be installed into the system-wide
bin/ directory, but installing into libexec/ is a possibility when we
adpot a benchmarking system.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 benchmarks/Makefile.sources | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index 7ad95a5..90dae18 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -1,4 +1,4 @@
-bin_PROGRAMS =  \
+noinst_PROGRAMS =   \
intel_upload_blit_large \
intel_upload_blit_large_gtt \
intel_upload_blit_large_map \
-- 
2.1.4

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


Re: [Intel-gfx] [PATCH v1.1 1/5] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2.

2015-07-24 Thread Ander Conselvan De Oliveira
On Tue, 2015-07-21 at 16:09 +0200, Maarten Lankhorst wrote:
 -EDEADLK has special meaning in atomic, but get_fence may call
 i915_find_fence_reg which can return -EDEADLK.
 
 This has special meaning in the atomic world, so convert the error
 to -EBUSY for this case.

Doesn't this change the behavior of intel_crtc_page_flip() slightly? It
now would return -EBUSY to user space if it can't find a fence instead
of -EDEADLK. Not sure if that is a problem though. I don't expect user
space would actually check for -EDEADLK.

Ander

 Changes since v1:
 - Add comment in the code.
 
 Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
 ---
 Like this?
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index af0bcfee4771..11387f5ed681 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -2395,8 +2395,20 @@ intel_pin_and_fence_fb_obj(struct drm_plane 
 *plane,
* a fence as the cost is not that onerous.
*/
   ret = i915_gem_object_get_fence(obj);
 - if (ret)
 + if (ret) {
 + if (ret == -EDEADLK) {
 + /*
 +  * -EDEADLK means there are no free fences
 +  * and no pending flips.
 +  *
 +  * This is propagated to atomic, but it uses
 +  * -EDEADLK to force a locking recovery, so
 +  * change the returned error to -EBUSY.
 +  */
 + ret = -EBUSY;
 + }
   goto err_unpin;
 + }
  
   i915_gem_object_pin_fence(obj);
  
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android

2015-07-24 Thread Derek Morton
There are two versions of gem_exec_nop.c in benchmarks and tests
which causes the build system to have two build modules with
the same name.
This patch renames benchmarks/gem_exec_nop.c to
benchmarks/gem_exec_nop_benchmark.c using the existing
gem_userptr_benchmark.c as a naming convention.

v2: Also rename gem_mmap to gem_mmap_benchmark. Another file
which breaks android which was added after this patch was 1st
submitted.

Signed-off-by: Derek Morton derek.j.mor...@intel.com
---
 benchmarks/Makefile.sources |   9 +-
 benchmarks/gem_exec_nop.c   | 153 -
 benchmarks/gem_exec_nop_benchmark.c | 153 +
 benchmarks/gem_mmap.c   | 165 
 benchmarks/gem_mmap_benchmark.c | 165 
 5 files changed, 325 insertions(+), 320 deletions(-)
 delete mode 100644 benchmarks/gem_exec_nop.c
 create mode 100644 benchmarks/gem_exec_nop_benchmark.c
 delete mode 100644 benchmarks/gem_mmap.c
 create mode 100644 benchmarks/gem_mmap_benchmark.c

diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index 7ad95a5..35722b5 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -1,11 +1,16 @@
+# If you copy a test to benckmarks, rename it _benchmark
+# The andriod build will fail when trying to build multiple binaries with
+# the same name.
+
 bin_PROGRAMS =  \
intel_upload_blit_large \
intel_upload_blit_large_gtt \
intel_upload_blit_large_map \
intel_upload_blit_small \
-   gem_exec_nop\
-   gem_mmap\
+   gem_exec_nop_benchmark  \
+   gem_mmap_benchmark  \
gem_prw \
gem_userptr_benchmark   \
kms_vblank  \
$(NULL)
+
diff --git a/benchmarks/gem_exec_nop.c b/benchmarks/gem_exec_nop.c
deleted file mode 100644
index 2a3abd2..000
--- a/benchmarks/gem_exec_nop.c
+++ /dev/null
@@ -1,153 +0,0 @@
-/*
- * Copyright © 2011 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the Software),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- *
- * Authors:
- *Chris Wilson ch...@chris-wilson.co.uk
- *
- */
-
-#include unistd.h
-#include stdlib.h
-#include stdint.h
-#include stdio.h
-#include string.h
-#include fcntl.h
-#include inttypes.h
-#include errno.h
-#include sys/stat.h
-#include sys/ioctl.h
-#include sys/time.h
-#include time.h
-
-#include drm.h
-#include ioctl_wrappers.h
-#include drmtest.h
-#include intel_io.h
-#include igt_stats.h
-
-#define LOCAL_I915_EXEC_NO_RELOC (111)
-#define LOCAL_I915_EXEC_HANDLE_LUT (112)
-
-static uint64_t elapsed(const struct timespec *start,
-   const struct timespec *end,
-   int loop)
-{
-   return (10ULL*(end-tv_sec - start-tv_sec) + (end-tv_nsec - 
start-tv_nsec))/loop;
-}
-
-static int __gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf)
-{
-   int err = 0;
-   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf))
-   err = -errno;
-   return err;
-}
-
-static uint32_t batch(int fd)
-{
-   const uint32_t buf[] = {MI_BATCH_BUFFER_END};
-   uint32_t handle = gem_create(fd, 4096);
-   gem_write(fd, handle, 0, buf, sizeof(buf));
-   return handle;
-}
-
-static int loop(unsigned ring, int reps)
-{
-   struct drm_i915_gem_execbuffer2 execbuf;
-   struct drm_i915_gem_exec_object2 gem_exec;
-   int count, fd;
-
-   fd = drm_open_any();
-
-   memset(gem_exec, 0, sizeof(gem_exec));
-   gem_exec.handle = batch(fd);
-
-   memset(execbuf, 0, sizeof(execbuf));
-   execbuf.buffers_ptr = (uintptr_t)gem_exec;
-   execbuf.buffer_count = 1;
-   execbuf.flags = ring;
-   execbuf.flags |= LOCAL_I915_EXEC_HANDLE_LUT;
-   execbuf.flags |= 

Re: [Intel-gfx] [PATCH i-g-t] tests/drm_import_export: Add tests for prime/flink sharing races

2015-07-24 Thread Thomas Wood
On 24 July 2015 at 10:24, Michał Winiarski michal.winiar...@intel.com wrote:
 It is possible to race between unreference of the underlying BO and
 importing it from prime_fd/name. Verify that the behaviour of libdrm
 is consistent for prime/flink.

Could you add this description into the source file as a comment?
There also appears to be an extra white space change at the end of
your patch.


 Signed-off-by: Michał Winiarski michal.winiar...@intel.com
 ---
  tests/drm_import_export.c | 103 
 ++
  1 file changed, 103 insertions(+)

 diff --git a/tests/drm_import_export.c b/tests/drm_import_export.c
 index 57b13dd..db11c18 100644
 --- a/tests/drm_import_export.c
 +++ b/tests/drm_import_export.c
 @@ -131,6 +131,98 @@ static void * test_thread(void * par)
 return NULL;
  }

 +#define IMPORT_RACE_LOOPS 10
 +
 +struct import_race_thread_data {
 +   int prime_fd;
 +   uint32_t flink_name;
 +   unsigned int stop;
 +   pthread_mutex_t mutex;
 +};
 +
 +static void *import_close_thread(void *data)
 +{
 +   struct import_race_thread_data *t = (struct import_race_thread_data 
 *)data;
 +   drm_intel_bo *bo;
 +   pthread_mutex_lock(t-mutex);
 +   while (!t-stop) {
 +   pthread_mutex_unlock(t-mutex);
 +   bo = NULL;
 +   if (use_flink)
 +   bo = drm_intel_bo_gem_create_from_name(bufmgr, 
 buf-shared, t-flink_name);
 +   else {
 +   pthread_mutex_lock(t-mutex);
 +   if (t-prime_fd != -1) {
 +   bo = 
 drm_intel_bo_gem_create_from_prime(bufmgr, t-prime_fd, 4096);
 +   pthread_mutex_unlock(t-mutex);
 +   }
 +   else
 +   /* We take the lock right after entering the 
 loop */
 +   continue;
 +   }
 +   if (bo == NULL) {
 +   /*
 +* If the bo is NULL it means that we've unreferenced 
 in other
 +* thread - therefore we should expect ENOENT
 +*/
 +   igt_assert_eq(errno, ENOENT);
 +   continue;
 +   }
 +
 +   drm_intel_bo_unreference(bo);
 +
 +   pthread_mutex_lock(t-mutex);
 +   }
 +   pthread_mutex_unlock(t-mutex);
 +
 +   return NULL;
 +}
 +
 +static void test_import_close_race(void)
 +{
 +   pthread_t t;
 +   unsigned int loops = IMPORT_RACE_LOOPS;
 +   drm_intel_bo *bo;
 +   struct import_race_thread_data t_data;
 +
 +   memset(t_data, 0, sizeof(t_data));
 +   pthread_mutex_init(t_data.mutex, NULL);
 +   t_data.prime_fd = -1;
 +
 +   igt_assert_eq(pthread_create(t, NULL, import_close_thread , 
 t_data), 0);
 +
 +   while (loops--) {
 +   bo = drm_intel_bo_alloc(bufmgr, buf-shared, 4096, 4096);
 +   igt_assert(bo != NULL);
 +   /*
 +* We setup the test in such way, that create_from_* can race 
 between
 +* unreference. If we're using prime, prime_fd is always a 
 valid fd.
 +*/
 +   if (use_flink)
 +   igt_assert_eq(drm_intel_bo_flink(bo, 
 (t_data.flink_name)), 0);
 +   else {
 +   pthread_mutex_lock(t_data.mutex);
 +   igt_assert_eq(drm_intel_bo_gem_export_to_prime(bo, 
 (t_data.prime_fd)), 0);
 +   igt_assert(t_data.prime_fd != -1);
 +   pthread_mutex_unlock(t_data.mutex);
 +   }
 +
 +   drm_intel_bo_unreference(bo);
 +
 +   pthread_mutex_lock(t_data.mutex);
 +   close(t_data.prime_fd);
 +   t_data.prime_fd = -1;
 +   pthread_mutex_unlock(t_data.mutex);
 +   }
 +
 +   pthread_mutex_lock(t_data.mutex);
 +   t_data.stop = 1;
 +   pthread_mutex_unlock(t_data.mutex);
 +
 +   pthread_join(t, NULL);
 +   pthread_mutex_destroy(t_data.mutex);
 +}
 +
  pthread_t test_thread_id1;
  pthread_t test_thread_id2;
  pthread_t test_thread_id3;
 @@ -153,6 +245,16 @@ igt_main {
 drm_intel_bufmgr_gem_enable_reuse(bufmgr);
 }

 +   igt_subtest(import-close-race-flink) {
 +   use_flink = true;
 +   test_import_close_race();
 +   }
 +
 +   igt_subtest(import-close-race-prime) {
 +   use_flink = false;
 +   test_import_close_race();
 +   }
 +
 igt_subtest(flink) {
 use_flink = true;

 @@ -180,4 +282,5 @@ igt_main {
 pthread_join(test_thread_id3, NULL);
 pthread_join(test_thread_id4, NULL);
 }
 +
  }
 --
 2.4.3

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 

Re: [Intel-gfx] [PATCH i-g-t v2] benchmark/: fix gem_exec_nop complie error on android

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 02:35:29PM +0100, Derek Morton wrote:
 There are two versions of gem_exec_nop.c in benchmarks and tests
 which causes the build system to have two build modules with
 the same name.
 This patch renames benchmarks/gem_exec_nop.c to
 benchmarks/gem_exec_nop_benchmark.c using the existing
 gem_userptr_benchmark.c as a naming convention.

Surely a simpler fix than breaking external tools would be to fix the
Makefile?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 09/13 v4] drm/i915: Implementation of GuC client

2015-07-24 Thread O'Rourke, Tom
On Thu, Jul 09, 2015 at 07:29:10PM +0100, Dave Gordon wrote:
 A GuC client has its own doorbell and workqueue. It maintains the
 doorbell cache line, process description object and work queue item.
 
 A default guc_client is created for the i915 driver to use for
 normal-priority in-order submission.
 
 Note that the created client is not yet ready for use; doorbell
 allocation will fail as we haven't yet linked the GuC's context
 descriptor to the default contexts for each ring (see later patch).
 
 v2:
 Defer adding structure members until needed [Chris Wilson]
 Rationalise type declarations [Chris Wilson]
 
 v4:
 Rebased
 
 Issue: VIZ-4884
 Signed-off-by: Alex Dai yu@intel.com
 Signed-off-by: Dave Gordon david.s.gor...@intel.com

[TOR:] I had some non-critical questions below.

Reviewed-by: Tom O'Rourke Tom.O'rou...@intel.com
 ---
  drivers/gpu/drm/i915/i915_guc_submission.c | 649 
 +
  drivers/gpu/drm/i915/intel_guc.h   |  42 ++
  drivers/gpu/drm/i915/intel_guc_loader.c|  12 +
  3 files changed, 703 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
 b/drivers/gpu/drm/i915/i915_guc_submission.c
 index e9d46d6..25d8807 100644
 --- a/drivers/gpu/drm/i915/i915_guc_submission.c
 +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
 @@ -27,6 +27,512 @@
  #include intel_guc.h
  
  /**
 + * DOC: GuC Client
 + *
 + * i915_guc_client:
 + * We use the term client to avoid confusion with contexts. A 
 i915_guc_client is
 + * equivalent to GuC object guc_context_desc. This context descriptor is
 + * allocated from a pool of 1024 entries. Kernel driver will allocate 
 doorbell
 + * and workqueue for it. Also the process descriptor (guc_process_desc), 
 which
 + * is mapped to client space. So the client can write Work Item then ring the
 + * doorbell.
 + *
 + * To simplify the implementation, we allocate one gem object that contains 
 all
 + * pages for doorbell, process descriptor and workqueue.
 + *
 + * The Scratch registers:
 + * There are 16 MMIO-based registers start from 0xC180. The kernel driver 
 writes
 + * a value to the action register (SOFT_SCRATCH_0) along with any data. It 
 then
 + * triggers an interrupt on the GuC via another register write (0xC4C8).
 + * Firmware writes a success/fail code back to the action register after
 + * processes the request. The kernel driver polls waiting for this update and
 + * then proceeds.
 + * See host2guc_action()
 + *
 + * Doorbells:
 + * Doorbells are interrupts to uKernel. A doorbell is a single cache line 
 (QW)
 + * mapped into process space.
 + *
 + * Work Items:
 + * There are several types of work items that the host may place into a
 + * workqueue, each with its own requirements and limitations. Currently only
 + * WQ_TYPE_INORDER is needed to support legacy submission via GuC, which
 + * represents in-order queue. The kernel driver packs ring tail pointer and 
 an
 + * ELSP context descriptor dword into Work Item.
 + * See guc_add_workqueue_item()
 + *
 + */
 +
 +/*
 + * Read GuC command/status register (SOFT_SCRATCH_0)
 + * Return true if it contains a response rather than a command
 + */
 +static inline bool host2guc_action_response(struct drm_i915_private 
 *dev_priv,
 + u32 *status)
 +{
 + u32 val = I915_READ(SOFT_SCRATCH(0));
 + *status = val;
 + return GUC2HOST_IS_RESPONSE(val);
 +}
 +
 +static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len)
 +{
 + struct drm_i915_private *dev_priv = guc_to_i915(guc);
 + u32 status;
 + int i;
 + int ret;
 +
 + if (WARN_ON(len  1 || len  15))
 + return -EINVAL;
 +

[TOR:] Would it be good for host2guc_action to take a 
forcewake?  There are several writes and polling reads 
for completion.  Taking a forcewake could avoid surplus 
forcewakes for each register access.

 + spin_lock(dev_priv-guc.host2guc_lock);
 +
 + dev_priv-guc.action_count += 1;
 + dev_priv-guc.action_cmd = data[0];
 +
 + for (i = 0; i  len; i++)
 + I915_WRITE(SOFT_SCRATCH(i), data[i]);
 +
 + POSTING_READ(SOFT_SCRATCH(i - 1));
 +
 + I915_WRITE(HOST2GUC_INTERRUPT, HOST2GUC_TRIGGER);
 +
 + ret = wait_for_atomic(host2guc_action_response(dev_priv, status), 10);
[TOR:] Why 10?

 + if (status != GUC2HOST_STATUS_SUCCESS) {
 + /* either GuC doesn't respond, which is a TIMEOUT,
 +  * or a failure code is returned. */
 + if (ret != -ETIMEDOUT)
 + ret = -EIO;
 +
 + DRM_ERROR(GUC: host2guc action 0x%X failed. ret=%d 
 + status=0x%08X response=0x%08X\n,
 + data[0], ret, status,
 + I915_READ(SOFT_SCRATCH(15)));
 +
 + dev_priv-guc.action_fail += 1;
 + dev_priv-guc.action_err = ret;
 + }
 + dev_priv-guc.action_status = status;
 +
 + 

[Intel-gfx] [PATCH v2 12/25] i915: switch from acpi_os_ioremap to ioremap

2015-07-24 Thread Dan Williams
acpi_os_ioremap uses cached mappings, however it appears that i915
wants to read dynamic platform state.  Switch to ioremap() to prevent it
reading stale state from cache.

Cc: Daniel Vetter daniel.vet...@intel.com
Cc: Jani Nikula jani.nik...@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Cc: David Airlie airl...@linux.ie
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Dan Williams dan.j.willi...@intel.com
---
 drivers/gpu/drm/i915/intel_opregion.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index 481337436f72..16ba7c67410d 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -863,7 +863,7 @@ int intel_opregion_setup(struct drm_device *dev)
INIT_WORK(opregion-asle_work, asle_work);
 #endif
 
-   base = acpi_os_ioremap(asls, OPREGION_SIZE);
+   base = ioremap(asls, OPREGION_SIZE);
if (!base)
return -ENOMEM;
 

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


Re: [Intel-gfx] [PATCH 2/3] drm/i915: Extract i915_gem_fence.c

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 01:55:11PM +0200, Daniel Vetter wrote:
 No code changes, just moving all the fence related code into a
 separate file (and avoiding a bunch of forward declarations while at
 it).

As well as i915_gem_tiling? i915_gem_fence is a better name, so can we
move the tiling ioctl to i915_gem_fence.c as well?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences

2015-07-24 Thread Chris Wilson
On Fri, Jul 24, 2015 at 01:55:12PM +0200, Daniel Vetter wrote:
 Signed-off-by: Daniel Vetter daniel.vet...@intel.com
 ---
  Documentation/DocBook/drm.tmpl|  5 +++
  drivers/gpu/drm/i915/i915_gem_fence.c | 75 
 +++
  2 files changed, 80 insertions(+)
 
 diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
 index 458772e6ce08..86c9c453b218 100644
 --- a/Documentation/DocBook/drm.tmpl
 +++ b/Documentation/DocBook/drm.tmpl
 @@ -4137,6 +4137,11 @@ int num_ioctls;/synopsis
  !Idrivers/gpu/drm/i915/i915_gem_gtt.c
/sect2
sect2
 +titleGlobal GTT Fence Handling/title
 +!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence handling
 +!Idrivers/gpu/drm/i915/i915_gem_fence.c
 +  /sect2
 +  sect2
  titleBuffer Object Eviction/title
   para
 This section documents the interface functions for evicting buffer
 diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c 
 b/drivers/gpu/drm/i915/i915_gem_fence.c
 index d3284ee04794..63d8d0d8a9cb 100644
 --- a/drivers/gpu/drm/i915/i915_gem_fence.c
 +++ b/drivers/gpu/drm/i915/i915_gem_fence.c
 @@ -25,6 +25,36 @@
  #include drm/i915_drm.h
  #include i915_drv.h
  
 +/**
 + * DOC: fence handling

DOC: fence register handling

Might as well aim not to be ambiguous when you start out by pointing out
the confusion between the GTT detiling registers and dma-fence.

 + *
 + * Important to avoid confusions: fences in the i915 driver are not 
 execution
 + * fences used to track command completion but hardware detiler objects which
 + * wrap a given range of the global GTT. Each platform has only a fairly 
 limited
 + * set of these objects.

Hmm, good point. Maybe i915_gem_tiling was the better name after all!
Otoh, i915_gem_fence is better as it prevents us from wondering whether
this is suitable for Yf etc.

We really should emphasize that again in the set-tiling documentation -
that is not about declaring the tiling for the buffer per-se, but about
fence allocation.

 + *
 + * Fences are used to detile GTT memory mappings. They're also connected to 
 the
 + * hardware frontbuffer render tracking and hence interract with frontbuffer
 + * conmpression. Furthermore on older platforms fences are required for tiled
 + * objects used by the display engine. They can also be used by the render
 + * engine - they're required for blitter commands and are optional for render
 + * commands. But on gen4+ both display (with the exception of fbc) and 
 rendering
 + * have their own tiling state bits and don't need fences.
 + *
 + * Also note that fences only support X and Y tiling and hence can't be used 
 for
 + * the fancier new tiling formats like W, Ys and Yf.
 + *
 + * Finally note that because fences are such a restricted resource they're
 + * dynamically associated with objects. Furthermore fence state is committed 
 to
 + * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code 
 must
 + * explictly call i915_gem_object_get_fence() to synchronize fencing status
 + * for cpu access. Also note that some code wants an unfenced view, for those
 + * cases the fence can be removed forcefully with 
 i915_gem_object_put_fence().
 + *
 + * Internally these functions will synchronize with userspace access by 
 removing
 + * ptes as needed.

Revoking CPU ptes into GTT mmaps.

Important when discussing PTEs to be clear when we don't mean the GTT
PTEs (which I think is the default for the driver).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC CABC PATCH v2 3/3] drm/i915: CABC support for backlight control

2015-07-24 Thread Deepak M
In CABC (Content Adaptive Brightness Control) content grey level
scale can be increased while simultaneously decreasing
brightness of the backlight to achieve same perceived brightness.

The CABC is not standardized and panel vendors are free to follow
their implementation. The CABC implementaion here assumes that the
panels use standard SW register for control.

In this design there will be no PWM signal from the SoC and DCS
commands are sent to enable and control the backlight brightness.

v2:
- Created a new backlight driver for cabc, which will be registered
  only when it cabc is supported by panel.
  (Addressed comment from Daniel Vetter)

Signed-off-by: Deepak M m.dee...@intel.com
---
 drivers/gpu/drm/i915/Makefile |1 +
 drivers/gpu/drm/i915/intel_drv.h  |3 +-
 drivers/gpu/drm/i915/intel_dsi.c  |   13 ++
 drivers/gpu/drm/i915/intel_dsi_cabc.c |  349 +
 drivers/gpu/drm/i915/intel_dsi_cabc.h |   45 +
 drivers/gpu/drm/i915/intel_panel.c|   22 ++-
 include/video/mipi_display.h  |8 +
 7 files changed, 436 insertions(+), 5 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c
 create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index de21965..a73953c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -75,6 +75,7 @@ i915-y += dvo_ch7017.o \
  intel_dp_mst.o \
  intel_dsi.o \
  intel_dsi_pll.o \
+ intel_dsi_cabc.o \
  intel_dsi_panel_vbt.o \
  intel_dvo.o \
  intel_hdmi.o \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3f0a890..9f806dd5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1319,7 +1319,8 @@ extern struct drm_display_mode 
*intel_find_panel_downclock(
struct drm_connector *connector);
 void intel_backlight_register(struct drm_device *dev);
 void intel_backlight_unregister(struct drm_device *dev);
-
+extern int cabc_backlight_device_register(struct intel_connector *connector);
+extern void cabc_backlight_device_unregister(struct intel_connector 
*connector);
 
 /* intel_psr.c */
 void intel_psr_enable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 98998e9..8f006f2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -34,6 +34,7 @@
 #include i915_drv.h
 #include intel_drv.h
 #include intel_dsi.h
+#include intel_dsi_cabc.h
 
 static const struct {
u16 panel_id;
@@ -376,6 +377,7 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder-base);
+   struct intel_connector *connector = intel_dsi-attached_connector;
enum port port;
 
DRM_DEBUG_KMS(\n);
@@ -396,6 +398,9 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 
intel_dsi_port_enable(encoder);
}
+
+   if (dev_priv-vbt.dsi.config-cabc_supported)
+   cabc_enable_backlight(connector);
 }
 
 static void intel_dsi_pre_enable(struct intel_encoder *encoder)
@@ -469,11 +474,15 @@ static void intel_dsi_disable(struct intel_encoder 
*encoder)
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder-base);
+   struct intel_connector *connector = intel_dsi-attached_connector;
enum port port;
u32 temp;
 
DRM_DEBUG_KMS(\n);
 
+   if (dev_priv-vbt.dsi.config-cabc_supported)
+   cabc_disable_backlight(connector);
+
if (is_vid_mode(intel_dsi)) {
for_each_dsi_port(port, intel_dsi-ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -1099,6 +1108,10 @@ void intel_dsi_init(struct drm_device *dev)
 
intel_panel_init(intel_connector-panel, fixed_mode, NULL);
 
+   if (dev_priv-vbt.dsi.config-cabc_supported)
+   cabc_setup_backlight(connector,
+   intel_encoder-crtc_mask ==
+   (1  PIPE_A) ? PIPE_A : PIPE_B);
return;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c 
b/drivers/gpu/drm/i915/intel_dsi_cabc.c
new file mode 100644
index 000..2a78326
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright © 2006-2010 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, 

[Intel-gfx] [PATCH 1/3] drm/i915: Clean up Makefile

2015-07-24 Thread Daniel Vetter
Sorting became confused and a few new files ended up in strange
places. Also move i915_irq.c to core since with the recent-ish
extraction of i915_gpu_error.c and intel_hotplug.c it's more and more
really just basic irq handling code.

When adding new files please don't put them somewhere randomly.

Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 drivers/gpu/drm/i915/Makefile | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e52e01251644..bf91482e14a3 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -6,12 +6,13 @@
 
 # core driver code
 i915-y := i915_drv.o \
+ i915_irq.o \
  i915_params.o \
   i915_suspend.o \
  i915_sysfs.o \
+ intel_csr.o \
  intel_pm.o \
- intel_runtime_pm.o \
- intel_csr.o
+ intel_runtime_pm.o
 
 i915-$(CONFIG_COMPAT)   += i915_ioc32.o
 i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
@@ -20,21 +21,19 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
 i915-y += i915_cmd_parser.o \
  i915_gem_batch_pool.o \
  i915_gem_context.o \
- i915_gem_render_state.o \
  i915_gem_debug.o \
  i915_gem_dmabuf.o \
  i915_gem_evict.o \
  i915_gem_execbuffer.o \
  i915_gem_gtt.o \
  i915_gem.o \
+ i915_gem_render_state.o \
  i915_gem_shrinker.o \
  i915_gem_stolen.o \
  i915_gem_tiling.o \
  i915_gem_userptr.o \
  i915_gpu_error.o \
- i915_irq.o \
  i915_trace_points.o \
- intel_hotplug.o \
  intel_lrc.o \
  intel_mocs.o \
  intel_ringbuffer.o \
@@ -48,11 +47,14 @@ i915-y += intel_renderstate_gen6.o \
 
 # modesetting core code
 i915-y += intel_audio.o \
+ intel_atomic.o \
+ intel_atomic_plane.o \
  intel_bios.o \
  intel_display.o \
  intel_fbc.o \
  intel_fifo_underrun.o \
  intel_frontbuffer.o \
+ intel_hotplug.o \
  intel_modes.o \
  intel_overlay.o \
  intel_psr.o \
@@ -68,15 +70,13 @@ i915-y += dvo_ch7017.o \
  dvo_ns2501.o \
  dvo_sil164.o \
  dvo_tfp410.o \
- intel_atomic.o \
- intel_atomic_plane.o \
  intel_crt.o \
  intel_ddi.o \
- intel_dp.o \
  intel_dp_mst.o \
+ intel_dp.o \
  intel_dsi.o \
- intel_dsi_pll.o \
  intel_dsi_panel_vbt.o \
+ intel_dsi_pll.o \
  intel_dvo.o \
  intel_hdmi.o \
  intel_i2c.o \
-- 
2.1.4

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


[Intel-gfx] [PATCH 2/3] drm/i915: Extract i915_gem_fence.c

2015-07-24 Thread Daniel Vetter
No code changes, just moving all the fence related code into a
separate file (and avoiding a bunch of forward declarations while at
it).

Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 drivers/gpu/drm/i915/Makefile |   1 +
 drivers/gpu/drm/i915/i915_drv.h   |  16 +-
 drivers/gpu/drm/i915/i915_gem.c   | 401 
 drivers/gpu/drm/i915/i915_gem_fence.c | 422 ++
 4 files changed, 432 insertions(+), 408 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_fence.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index bf91482e14a3..41fb8a9c5bef 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \
  i915_gem_dmabuf.o \
  i915_gem_evict.o \
  i915_gem_execbuffer.o \
+ i915_gem_fence.o \
  i915_gem_gtt.o \
  i915_gem.o \
  i915_gem_render_state.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 773883d54b28..4c34fb15d0df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2858,11 +2858,6 @@ static inline bool i915_gem_request_completed(struct 
drm_i915_gem_request *req,
 
 int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
 int __must_check i915_gem_set_seqno(struct drm_device *dev, u32 seqno);
-int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
-int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
-
-bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj);
-void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj);
 
 struct drm_i915_gem_request *
 i915_gem_find_active_request(struct intel_engine_cs *ring);
@@ -2960,8 +2955,6 @@ struct drm_gem_object *i915_gem_prime_import(struct 
drm_device *dev,
 struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gem_obj, int flags);
 
-void i915_gem_restore_fences(struct drm_device *dev);
-
 unsigned long
 i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o,
  const struct i915_ggtt_view *view);
@@ -3056,6 +3049,15 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object 
*obj)
i915_gem_object_ggtt_unpin_view(obj, i915_ggtt_view_normal);
 }
 
+/* i915_gem_fence.c */
+int __must_check i915_gem_object_get_fence(struct drm_i915_gem_object *obj);
+int __must_check i915_gem_object_put_fence(struct drm_i915_gem_object *obj);
+
+bool i915_gem_object_pin_fence(struct drm_i915_gem_object *obj);
+void i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj);
+
+void i915_gem_restore_fences(struct drm_device *dev);
+
 /* i915_gem_context.c */
 int __must_check i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 322bbefafbc3..5d685789b1f9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -46,11 +46,6 @@ static void
 i915_gem_object_retire__write(struct drm_i915_gem_object *obj);
 static void
 i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring);
-static void i915_gem_write_fence(struct drm_device *dev, int reg,
-struct drm_i915_gem_object *obj);
-static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
-struct drm_i915_fence_reg *fence,
-bool enable);
 
 static bool cpu_cache_is_coherent(struct drm_device *dev,
  enum i915_cache_level level)
@@ -66,18 +61,6 @@ static bool cpu_write_needs_clflush(struct 
drm_i915_gem_object *obj)
return obj-pin_display;
 }
 
-static inline void i915_gem_object_fence_lost(struct drm_i915_gem_object *obj)
-{
-   if (obj-tiling_mode)
-   i915_gem_release_mmap(obj);
-
-   /* As we do not have an associated fence register, we will force
-* a tiling change if we ever need to acquire one.
-*/
-   obj-fence_dirty = false;
-   obj-fence_reg = I915_FENCE_REG_NONE;
-}
-
 /* some bookkeeping */
 static void i915_gem_info_add_obj(struct drm_i915_private *dev_priv,
  size_t size)
@@ -2793,27 +2776,6 @@ static void i915_gem_reset_ring_cleanup(struct 
drm_i915_private *dev_priv,
}
 }
 
-void i915_gem_restore_fences(struct drm_device *dev)
-{
-   struct drm_i915_private *dev_priv = dev-dev_private;
-   int i;
-
-   for (i = 0; i  dev_priv-num_fence_regs; i++) {
-   struct drm_i915_fence_reg *reg = dev_priv-fence_regs[i];
-
-   /*
-* Commit delayed tiling changes if we have an object still
-* attached to the fence, otherwise just clear the fence.
- 

[Intel-gfx] [PATCH 3/3] drm/i915: kerneldoc for fences

2015-07-24 Thread Daniel Vetter
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
 Documentation/DocBook/drm.tmpl|  5 +++
 drivers/gpu/drm/i915/i915_gem_fence.c | 75 +++
 2 files changed, 80 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 458772e6ce08..86c9c453b218 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -4137,6 +4137,11 @@ int num_ioctls;/synopsis
 !Idrivers/gpu/drm/i915/i915_gem_gtt.c
   /sect2
   sect2
+titleGlobal GTT Fence Handling/title
+!Pdrivers/gpu/drm/i915/i915_gem_fence.c fence handling
+!Idrivers/gpu/drm/i915/i915_gem_fence.c
+  /sect2
+  sect2
 titleBuffer Object Eviction/title
para
  This section documents the interface functions for evicting buffer
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c 
b/drivers/gpu/drm/i915/i915_gem_fence.c
index d3284ee04794..63d8d0d8a9cb 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -25,6 +25,36 @@
 #include drm/i915_drm.h
 #include i915_drv.h
 
+/**
+ * DOC: fence handling
+ *
+ * Important to avoid confusions: fences in the i915 driver are not execution
+ * fences used to track command completion but hardware detiler objects which
+ * wrap a given range of the global GTT. Each platform has only a fairly 
limited
+ * set of these objects.
+ *
+ * Fences are used to detile GTT memory mappings. They're also connected to the
+ * hardware frontbuffer render tracking and hence interract with frontbuffer
+ * conmpression. Furthermore on older platforms fences are required for tiled
+ * objects used by the display engine. They can also be used by the render
+ * engine - they're required for blitter commands and are optional for render
+ * commands. But on gen4+ both display (with the exception of fbc) and 
rendering
+ * have their own tiling state bits and don't need fences.
+ *
+ * Also note that fences only support X and Y tiling and hence can't be used 
for
+ * the fancier new tiling formats like W, Ys and Yf.
+ *
+ * Finally note that because fences are such a restricted resource they're
+ * dynamically associated with objects. Furthermore fence state is committed to
+ * the hardware lazily to avoid unecessary stalls on gen2/3. Therefore code 
must
+ * explictly call i915_gem_object_get_fence() to synchronize fencing status
+ * for cpu access. Also note that some code wants an unfenced view, for those
+ * cases the fence can be removed forcefully with i915_gem_object_put_fence().
+ *
+ * Internally these functions will synchronize with userspace access by 
removing
+ * ptes as needed.
+ */
+
 static void i965_write_fence_reg(struct drm_device *dev, int reg,
 struct drm_i915_gem_object *obj)
 {
@@ -247,6 +277,17 @@ i915_gem_object_wait_fence(struct drm_i915_gem_object *obj)
return 0;
 }
 
+/**
+ * i915_gem_object_put_fence - force-remove fence for an object
+ * @obj: object to map through a fence reg
+ *
+ * This function force-removes any fence from the given object, which is useful
+ * if the kernel wants to do untiled GTT access.
+ *
+ * Returns:
+ *
+ * 0 on success, negative error code on failure.
+ */
 int
 i915_gem_object_put_fence(struct drm_i915_gem_object *obj)
 {
@@ -322,6 +363,10 @@ deadlock:
  * and tiling format.
  *
  * For an untiled surface, this removes any existing fence.
+ *
+ * Returns:
+ *
+ * 0 on success, negative error code on failure.
  */
 int
 i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
@@ -374,6 +419,21 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj)
return 0;
 }
 
+/**
+ * i915_gem_object_pin_fence - pin fencing state
+ * @obj: object to pin fencing for
+ *
+ * This pins the fencing state (whether tiled or untiled) to make sure the
+ * object is ready to be used as a scanout target. Fencing status must be
+ * synchronize first by calling i915_gem_object_get_fence():
+ *
+ * The resulting fence pin reference must be released again with
+ * i915_gem_object_unpin_fence().
+ *
+ * Returns:
+ *
+ * True if the object has a fence, false otherwise.
+ */
 bool
 i915_gem_object_pin_fence(struct drm_i915_gem_object *obj)
 {
@@ -390,6 +450,14 @@ i915_gem_object_pin_fence(struct drm_i915_gem_object *obj)
return false;
 }
 
+/**
+ * i915_gem_object_unpin_fence - unpin fencing state
+ * @obj: object to unpin fencing for
+ *
+ * This releases the fence pin reference acquired through
+ * i915_gem_object_pin_fence. It will handle both objects with and without an
+ * attached fence correctly, callers do not need to distinguish this.
+ */
 void
 i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)
 {
@@ -400,6 +468,13 @@ i915_gem_object_unpin_fence(struct drm_i915_gem_object 
*obj)
}
 }
 
+/**
+ * i915_gem_restore_fences - restore fence state
+ * @dev: DRM device
+ *
+ * Restore the hw fence state to match the software tracking again, to