[Intel-gfx] [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache for Gen9+

2016-01-15 Thread Artur Harasimiuk
Starting from Gen9 we can use IA-Coherent caches. Generally, coherency
can be programmed using RENDER_SURFACE_STATE or BTI 255, depending if
surface state model or stateless model is used. It is important to control
whether IA or GPU cache coherency should be used, especially for non-LLC
devices. However this control is complicated when stateless memory access
model is in action. It would require dedicated ISA code depending on
coherency requirement.

By setting HDC_FORCE_NON_COHERENT we *Force* data port to ignore these
attributes and all caches are GPU-Coherent. This register is part of HW
context, however it is private and cannot be programmed from
non-privileged batch buffer.

Default operation mode is as programmed by workaround. When
WaForceEnableNonCoherent is in place caches are GPU-Coherent and we
should not change it back to IA-Coherent because this can lead to GPU
hangs (as workaround description says).

A new device parameter is to inform user space about kernel capability.
It tells if can request to disable IA-Coherency.

Exec flag is to allow UMD to decide whether IA-Coherency is not needed
for submitted batch buffer. Exec flag behavior:
1. flag is not set - use system default
2. flag is set but WaForceEnableNonCoherent is
   a) not programmed - *Force* GPU-Coherent cache by setting
  HDC_FORCE_NON_COHERENT prior to bb_start and clearing after
   b) programmed - do nothing, GPU-Coherent is already in place

v2: Ringbufer handling fixes (Chris)
Moved workarounds to common place (Chris)
Removed flag cleanup (Dave)
Updated commit message to reflect comments (Chris,Dave)

Signed-off-by: Artur Harasimiuk 
---
 drivers/gpu/drm/i915/i915_dma.c|  4 
 drivers/gpu/drm/i915/i915_drv.h|  4 
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 
 drivers/gpu/drm/i915/intel_lrc.c   | 38 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c|  2 ++
 include/uapi/drm/i915_drm.h|  8 ++-
 6 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44a896c..f735e56 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -172,6 +172,10 @@ static int i915_getparam(struct drm_device *dev, void 
*data,
case I915_PARAM_HAS_EXEC_SOFTPIN:
value = 1;
break;
+   case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
+   value = !dev_priv->workarounds.WaForceEnableNonCoherent &&
+   INTEL_INFO(dev)->gen >= 9;
+   break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104bd18..793be854 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1658,6 +1658,10 @@ struct i915_wa_reg {
 struct i915_workarounds {
struct i915_wa_reg reg[I915_MAX_WA_REGS];
u32 count;
+
+   struct {
+   unsigned int WaForceEnableNonCoherent:1;
+   };
 };
 
 struct i915_virtual_gpu {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d469c47..5db3806 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
*data,
if (!i915_gem_check_execbuffer(args))
return -EINVAL;
 
+   if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
+   INTEL_INFO(dev)->gen < 9)
+   return -EINVAL;
+
ret = validate_exec_list(dev, exec, args->buffer_count);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ab344e0..f37d12f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -879,6 +879,36 @@ int intel_logical_ring_reserve_space(struct 
drm_i915_gem_request *request)
return intel_logical_ring_begin(request, 0);
 }
 
+static inline int
+intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
+   struct drm_i915_gem_execbuffer2 *args, bool force)
+{
+   struct drm_device   *dev = params->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   int ret;
+
+   if (dev_priv->workarounds.WaForceEnableNonCoherent)
+   return 0;
+
+   if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
+   struct intel_ringbuffer *ringbuf = params->request->ringbuf;
+
+   ret = intel_logical_ring_begin(params->request, 4);
+   if (ret)
+   return ret;
+
+   intel_logical_ring_emit(ringbuf, MI_NOOP);
+   intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
+   intel_l

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2)

2016-01-15 Thread Patchwork
== Summary ==

Built on 4efb7ea717f8478860c4cb829fc0956f195c0b50 drm-intel-nightly: 
2016y-01m-15d-02h-35m-27s UTC integration manifest

Test gem_ctx_basic:
pass   -> FAIL   (bdw-ultra)
Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (skl-i5k-2) UNSTABLE
pass   -> DMESG-WARN (bdw-nuci7)
dmesg-warn -> PASS   (skl-i7k-2) UNSTABLE
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b-frame-sequence:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:0   dfail:0   fail:1   skip:6  
byt-nuc  total:141  pass:121  dwarn:5   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1193/

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


[Intel-gfx] [PATCH i-g-t] tools/intel_bios_reader: Fix typo on dump info

2016-01-15 Thread Mika Kahola
Fix typo on intel_bios_reader.c

Signed-off-by: Mika Kahola 
---
 tools/intel_bios_reader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c
index b31f648..5ca50ff 100644
--- a/tools/intel_bios_reader.c
+++ b/tools/intel_bios_reader.c
@@ -843,7 +843,7 @@ static void dump_mipi_config(const struct bdb_block *block)
 
printf("\tMIPI PPS\n");
printf("\t\tPanel power ON delay: %d\n", pps->panel_on_delay);
-   printf("\t\tPanel power on to Baklight enable delay: %d\n", 
pps->bl_enable_delay);
+   printf("\t\tPanel power on to Backlight enable delay: %d\n", 
pps->bl_enable_delay);
printf("\t\tBacklight disable to Panel power OFF delay: %d\n", 
pps->bl_disable_delay);
printf("\t\tPanel power OFF delay: %d\n", pps->panel_off_delay);
printf("\t\tPanel power cycle delay: %d\n", 
pps->panel_power_cycle_delay);
-- 
1.9.1

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


[Intel-gfx] [PATCH i-g-t] lib/igt_kms: Add COMIT_ATOMIC to igt_display_commit2()

2016-01-15 Thread Marius Vlad
So far, we had only COMMIT_UNIVERSAL and COMMIT_LEGACY, using
drmModeSetPlane()/drmSetCrtc(). This patch adds COMMIT_ATOMIC
to igt_display_commit2() that makes use of drmModeAtomicCommit().

Signed-off-by: Marius Vlad 
---
 lib/igt_kms.c | 190 +-
 lib/igt_kms.h |  33 +-
 2 files changed, 221 insertions(+), 2 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 497118a..61f7a39 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1306,6 +1306,191 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t 
*plane)
igt_assert(r == 0); \
 }
 
+static const char *igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
+   "SRC_X",
+   "SRC_Y",
+   "SRC_W",
+   "SRC_H",
+   "CRTC_X",
+   "CRTC_Y",
+   "CRTC_W",
+   "CRTC_H",
+   "FB_ID",
+   "CRTC_ID",
+   "type"
+};
+
+/*
+ * Retrieve all the properies specified in props_name and store them into
+ * plane->atomic_props_plane.
+ */
+static void
+igt_atomic_fill_plane_props(igt_display_t *display, igt_plane_t *plane,
+   int type, int num_props, const char **prop_names)
+{
+   drmModeObjectPropertiesPtr props;
+   int i, j, fd;
+
+   fd = display->drm_fd;
+
+   props = drmModeObjectGetProperties(fd, plane->drm_plane->plane_id, 
type);
+   igt_assert(props);
+
+   for (i = 0; i < props->count_props; i++) {
+   drmModePropertyPtr prop =
+   drmModeGetProperty(fd, props->props[i]);
+
+   for (j = 0; j < num_props; j++) {
+   if (strcmp(prop->name, prop_names[j]) != 0)
+   continue;
+   plane->atomic_props_plane[j] = props->props[i];
+   break;
+   }
+
+   drmModeFreeProperty(prop);
+   }
+
+   drmModeFreeObjectProperties(props);
+}
+
+/*
+ * Commit position and fb changes to a DRM plane via the AtomicCommit()
+ * ioctl; if the DRM call to program the plane fails, we'll either fail
+ * immediately (for tests that expect the commit to succeed) or return the
+ * failure code (for tests that expect a specific error code).
+ */
+static int
+igt_atomic_plane_commit(igt_plane_t *plane, igt_output_t *output,
+   bool fail_on_error)
+{
+   igt_display_t *display = output->display;
+
+   uint32_t fb_id, crtc_id;
+   int ret;
+   uint32_t src_x;
+   uint32_t src_y;
+   uint32_t src_w;
+   uint32_t src_h;
+   int32_t crtc_x;
+   int32_t crtc_y;
+   uint32_t crtc_w;
+   uint32_t crtc_h;
+   drmModeAtomicReq *req;
+
+   igt_assert(plane->drm_plane);
+
+   do_or_die(drmSetClientCap(display->drm_fd, DRM_CLIENT_CAP_ATOMIC, 1));
+
+   /* it's an error to try an unsupported feature */
+   igt_assert(igt_plane_supports_rotation(plane) ||
+   !plane->rotation_changed);
+
+   fb_id = igt_plane_get_fb_id(plane);
+   crtc_id = output->config.crtc->crtc_id;
+
+   if ((plane->fb_changed || plane->size_changed) && fb_id == 0) {
+
+   LOG(display,
+   "%s: drmModeAtomicCommit pipe %s, plane %d, disabling\n",
+igt_output_name(output),
+kmstest_pipe_name(output->config.pipe),
+plane->index);
+
+   req = drmModeAtomicAlloc();
+   igt_atomic_fill_plane_props(display, plane,
+   DRM_MODE_OBJECT_PLANE,
+   IGT_NUM_PLANE_PROPS,
+   igt_plane_prop_names);
+
+   drmModeAtomicSetCursor(req, 0);
+
+   /* populate plane req */
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_ID, 
crtc_id);
+
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_X, 
IGT_FIXED(0, 0));
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_Y, 
IGT_FIXED(0, 0));
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_W, 
IGT_FIXED(0, 0));
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_SRC_H, 
IGT_FIXED(0, 0));
+
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_X, 0);
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_Y, 0);
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_W, 0);
+   igt_atomic_populate_plane_req(req, plane, IGT_PLANE_CRTC_H, 0);
+
+   ret = drmModeAtomicCommit(display->drm_fd, req, 0, NULL);
+   drmModeAtomicFree(req);
+
+   CHECK_RETURN(ret, fail_on_error);
+
+   } else if (plane->fb_changed || plane->position_changed ||
+   plane->size_changed) {
+
+   src_x = IGT_FIXED(plane->fb->src_x,0); /* src_x */
+   src_y = IGT_FIXED(plane->fb->src_y,0); /* src_y */
+

[Intel-gfx] [PATCH i-g-t 1/2 v2] lib/igt_kms: Various fixes and add remaining documentation

2016-01-15 Thread Marius Vlad
lib/igt_kms: Fix minor spelling/gtk-doc links. Style changes -- trimmed several
calls to asprintf(). Placed SECTION header at the beginning of the file.  Add
documentation for remaining functions and macros.

v2: Initially, I wanted to aggregate the documentation in header file(s).
This version keeps the initial format of the documentation.

Signed-off-by: Marius Vlad 
---
 lib/igt_kms.c | 160 +++---
 lib/igt_kms.h | 123 ++--
 2 files changed, 236 insertions(+), 47 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5d5a95c..c7a0b77 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -49,6 +49,30 @@
 #include "intel_chipset.h"
 #include "igt_debugfs.h"
 
+/**
+ * SECTION:igt_kms
+ * @short_description: Kernel modesetting support library
+ * @title: KMS
+ * @include: igt.h
+ *
+ * This library provides support to enumerate and set modeset configurations.
+ *
+ * There are two parts in this library: First the low level helper function
+ * which directly build on top of raw ioctls or the interfaces provided by
+ * libdrm. Those functions all have a kmstest_ prefix.
+ *
+ * The second part is a high-level library to manage modeset configurations
+ * which abstracts away some of the low-level details like the difference
+ * between legacy and universal plane support for setting cursors or in the
+ * future the difference between legacy and atomic commit. These high-level
+ * functions have all igt_ prefixes. This part is still very much work in
+ * progress and so also lacks a bit documentation for the individual functions.
+ *
+ * Note that this library's header pulls in the [i-g-t
+ * framebuffer](intel-gpu-tools-Framebuffer.html) library as a
+ * dependency.
+ */
+
 /* list of connectors that need resetting on exit */
 #define MAX_CONNECTORS 32
 static char *forced_connectors[MAX_CONNECTORS + 1];
@@ -104,7 +128,7 @@ static void update_edid_csum(unsigned char *edid)
  *
  * Returns: a basic edid block
  */
-const unsigned char* igt_kms_get_base_edid(void)
+const unsigned char *igt_kms_get_base_edid(void)
 {
update_edid_csum(base_edid);
 
@@ -145,7 +169,7 @@ const unsigned char* igt_kms_get_base_edid(void)
  *
  * Returns: an alternate edid block
  */
-const unsigned char* igt_kms_get_alt_edid(void)
+const unsigned char *igt_kms_get_alt_edid(void)
 {
update_edid_csum(alt_edid);
 
@@ -153,33 +177,10 @@ const unsigned char* igt_kms_get_alt_edid(void)
 }
 
 /**
- * SECTION:igt_kms
- * @short_description: Kernel modesetting support library
- * @title: KMS
- * @include: igt.h
- *
- * This library provides support to enumerate and set modeset configurations.
- *
- * There are two parts in this library: First the low level helper function
- * which directly build on top of raw ioctls or the interfaces provided by
- * libdrm. Those functions all have a kmstest_ prefix.
- *
- * The second part is a high-level library to manage modeset configurations
- * which abstracts away some of the low-level details like the difference
- * between legacy and universal plane support for setting cursors or in the
- * future the difference between legacy and atomic commit. These high-level
- * functions have all igt_ prefixes. This part is still very much work in
- * progress and so also lacks a bit documentation for the individual functions.
- *
- * Note that this library's header pulls in the [i-g-t 
framebuffer](intel-gpu-tools-i-g-t-framebuffer.html)
- * library as a dependency.
- */
-
-/**
  * kmstest_pipe_name:
  * @pipe: display pipe
  *
- * Returns: String represnting @pipe, e.g. "A".
+ * Returns: String representing @pipe, e.g. "A".
  */
 const char *kmstest_pipe_name(enum pipe pipe)
 {
@@ -195,7 +196,7 @@ const char *kmstest_pipe_name(enum pipe pipe)
  * kmstest_plane_name:
  * @plane: display plane
  *
- * Returns: String represnting @pipe, e.g. "plane1".
+ * Returns: String representing @pipe, e.g. "plane1".
  */
 const char *kmstest_plane_name(enum igt_plane plane)
 {
@@ -239,7 +240,7 @@ static const char *mode_stereo_name(const drmModeModeInfo 
*mode)
  * kmstest_dump_mode:
  * @mode: libdrm mode structure
  *
- * Prints @mode to stdout in a huma-readable form.
+ * Prints @mode to stdout in a human-readable form.
  */
 void kmstest_dump_mode(drmModeModeInfo *mode)
 {
@@ -413,8 +414,9 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector 
*connector,
break;
}
 
-   igt_assert_neq(asprintf(&path, "%s-%d/force", 
kmstest_connector_type_str(connector->connector_type), 
connector->connector_type_id),
-  -1);
+   igt_assert_neq(asprintf(&path, "%s-%d/force",
+  kmstest_connector_type_str(connector->connector_type),
+  connector->connector_type_id), -1);
debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
 
if (debugfs_fd == -1) {
@@ -478,8 +480,9 @@ void kmstest_force_edid(int drm_fd, drmModeCo

[Intel-gfx] [PATCH i-g-t 2/2] lib/igt_kms: Short description between Intel/DRM terminology.

2016-01-15 Thread Marius Vlad
lib/igt_kms: Briefly describe Intel-to-DRM mapping between pipes, encoders and
connectors.

Signed-off-by: Marius Vlad 
---
 lib/igt_kms.c | 82 +++
 1 file changed, 82 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c7a0b77..caa8837 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -68,6 +68,88 @@
  * functions have all igt_ prefixes. This part is still very much work in
  * progress and so also lacks a bit documentation for the individual functions.
  *
+ * Intel/DRM terminology and display connections:
+ *
+ * Intel documentation describes the road from memory to an output connector as
+ * follows:
+ *
+ * |[!<-- language="C" -->
+ * .. .---. .-. .-.
+ * | Memory |>| Pipes |>| Transcoders |>| DDI |
+ * '' '---' '-' '-'
+ * ]|
+ *
+ * Pipes represent the front-end of the display contain the planes, blending,
+ * and color correction, while the transcoders contain timing generators,
+ * encoders, A/V mixers and PSR (Panel-Self Refresh) controllers. Finally the
+ * DDI represent the connectors attached to the display.
+ *
+ *
+ *  In DRM we have the following:
+ *
+ * |[!<-- language="C" -->
+ * .---. .---. .---. ..
+ * | Framebuff |>| Pipes |>| Encoders  |>| Connectors |
+ * '---' '---' '---' ''
+ * ]|
+ *
+ *
+ * The frame buffer ties a reference to a memory object and provides a pointer
+ * to the actual data.
+ *
+ * The pipe is used to set the display mode, timings and gamma tables. On some
+ * hardware models this is tied with the transcoder.  In DRM-parlance this is
+ * referred as a CRTC.
+ *
+ * Each pipe has multiple planes. On older hardware these planes where known as
+ * primary plane, overlay/sprite plane, and cursor plane. From GEN9 (SKL/BXT)
+ * each pipe has three planes and a cursor plane. Each plane can be used as a
+ * primary, as a sprite or as an overlay plane. The planes are the ones that
+ * retrieve the pixels from memory and pushes them to the encoder.
+ *
+ * A pipe prior to GEN9:
+ *
+ * |[!<-- language="C" -->
+ * ..
+ * | Memory | ..
+ * ||>| Cursor |-->...
+ * || ''
+ * ||
+ * || ..
+ * ||>| Sprite |-->...
+ * || ''
+ * ||
+ * ||  .-.
+ * ||->| Primary |>...
+ * ||  '-'
+ * ''
+ * ]|
+ *
+ * A pipe with universal planes:
+ *
+ * |[!<-- language="C" -->
+ * ..
+ * | Memory | ..
+ * ||>| Cursor |-->...
+ * || ''
+ * ||
+ * || .---.
+ * ||>| Plane |-->...
+ * || '---'
+ * ||
+ * ||  .---.
+ * ||->| Plane |>...
+ * ||  '---'
+ * ''
+ * ]|
+ *
+ * The encoder is used to convert, depending on the output, pixels from pipes
+ * to signals understood by the connector.
+ *
+ * The connector will connect to the output display. This contains information
+ * about the attached display such as EDID, DPMS and information about modes
+ * supported by the display.
+ *
  * Note that this library's header pulls in the [i-g-t
  * framebuffer](intel-gpu-tools-Framebuffer.html) library as a
  * dependency.
-- 
2.6.2

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


Re: [Intel-gfx] [PATCH igt] igt/gem_softpin: Remove false dependencies on esoteric features

2016-01-15 Thread Tvrtko Ursulin


Hi,

On 14/01/16 11:02, Chris Wilson wrote:

For softpinning, we do not require either userptr or extended ppgtt, so
remove those requirements and make the tests work universally. (Certain
ABI tests require large GTT, or per-process GTT.)

In the process, make the tests more extensive - validate overlapping
handling more careful, explicitly test no-relocation support, validate
more ABI handling. And for fun, cause a kernel GPF.

Signed-off-by: Chris Wilson 
---
  tests/gem_softpin.c | 1313 +--
  1 file changed, 324 insertions(+), 989 deletions(-)


Adding some people to Cc who could be potential reviewers. We were 
tracking blanket improvements agreed during code review of the initial 
version in VIZ-6951. I've put a reference to this patch in there.


Regards,

Tvrtko


diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c
index 1cbde4e..f188559 100644
--- a/tests/gem_softpin.c
+++ b/tests/gem_softpin.c
@@ -26,80 +26,10 @@
   *
   */

-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include "drm.h"
-#include "ioctl_wrappers.h"
-#include "drmtest.h"
-#include "intel_chipset.h"
-#include "intel_io.h"
-#include "i915_drm.h"
-#include 
-#include 
-#include 
-#include 
-#include "igt_kms.h"
-#include 
-#include 
-#include 
-
-#define BO_SIZE 4096
-#define MULTIPAGE_BO_SIZE 2 * BO_SIZE
-#define STORE_BATCH_BUFFER_SIZE 4
+#include "igt.h"
+
  #define EXEC_OBJECT_PINNED(1<<4)
  #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
-#define SHARED_BUFFER_SIZE 4096
-
-typedef struct drm_i915_gem_userptr i915_gem_userptr;
-
-static uint32_t init_userptr(int fd, i915_gem_userptr *, void *ptr, uint64_t 
size);
-static void *create_mem_buffer(uint64_t size);
-static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr);
-static void gem_pin_userptr_test(void);
-static void gem_pin_bo_test(void);
-static void gem_pin_invalid_vma_test(bool test_decouple_flags, bool 
test_canonical_offset);
-static void gem_pin_overlap_test(void);
-static void gem_pin_high_address_test(void);
-
-#define NO_PPGTT 0
-#define ALIASING_PPGTT 1
-#define FULL_32_BIT_PPGTT 2
-#define FULL_48_BIT_PPGTT 3
-/* uses_full_ppgtt
- * Finds supported PPGTT details.
- * @fd DRM fd
- * @min can be
- * 0 - No PPGTT
- * 1 - Aliasing PPGTT
- * 2 - Full PPGTT (32b)
- * 3 - Full PPGTT (48b)
- * RETURNS true/false if min support is present
-*/
-static bool uses_full_ppgtt(int fd, int min)
-{
-   struct drm_i915_getparam gp;
-   int val = 0;
-
-   memset(&gp, 0, sizeof(gp));
-   gp.param = 18; /* HAS_ALIASING_PPGTT */
-   gp.value = &val;
-
-   if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
-   return 0;
-
-   errno = 0;
-   return val >= min;
-}

  /* has_softpin_support
   * Finds if softpin feature is supported
@@ -121,83 +51,6 @@ static bool has_softpin_support(int fd)
return (val == 1);
  }

-/* gem_call_userptr_ioctl
- * Helper to call ioctl - TODO: move to lib
- * @fd - drm fd
- * @userptr - pointer to initialised userptr
- * RETURNS status of ioctl call
-*/
-static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr)
-{
-   int ret;
-
-   ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_USERPTR, userptr);
-
-   if (ret)
-   ret = errno;
-
-   return ret;
-}
-
-/* init_userptr
- * Helper that inits userptr an returns handle
- * @fd - drm fd
- * @userptr - pointer to empty userptr
- * @ptr - buffer to be shared
- * @size - size of buffer
- * @ro - read only flag
- * RETURNS handle to shared buffer
-*/
-static uint32_t init_userptr(int fd, i915_gem_userptr *userptr, void *ptr,
-uint64_t size)
-{
-   int ret;
-
-   memset((void*)userptr, 0, sizeof(i915_gem_userptr));
-
-   userptr->user_ptr = (unsigned long)ptr; /* Need the cast to overcome 
compiler warning */
-   userptr->user_size = size;
-   userptr->flags = 0; /* use synchronized operation */
-
-   ret = gem_call_userptr_ioctl(fd, userptr);
-   igt_assert_eq(ret, 0);
-
-   return userptr->handle;
-}
-
-/* create_mem_buffer
- * Creates a 4K aligned CPU buffer
- * @size - size of buffer
- * RETURNS pointer to buffer of @size
-*/
-static void *create_mem_buffer(uint64_t size)
-{
-   void *addr;
-   int ret;
-
-   ret = posix_memalign(&addr, 4096, size);
-   igt_assert(ret == 0);
-
-   return addr;
-}
-
-/* setup_exec_obj
- * populate exec object
- * @exec - exec object
- * @handle - handle to gem buffer
- * @flags - any flags
- * @offset - requested VMA
-*/
-static void setup_exec_obj(struct drm_i915_gem_exec_object2 *exec,
-  uint32_t handle, uint32_t flags,
-  uint64_t offset)
-{
-   memset(exec, 0, sizeof(struct drm_i915_gem_exec_object2));
-   exec->handle = handle;
-   exec->flags = flags;
-   exec->offset = offset;
-}
-
  /* gen8_canonical_ad

Re: [Intel-gfx] [PATCH] drm/i915/dp: fall back to 18 bpp when sink capability is unknown

2016-01-15 Thread Jani Nikula
On Wed, 13 Jan 2016, Ville Syrjälä  wrote:
> On Wed, Jan 13, 2016 at 04:35:20PM +0200, Jani Nikula wrote:
>> Per DP spec, the source device should fall back to 18 bpp, VESA range
>> RGB when the sink capability is unknown. Fix the color depth
>> clamping. 18 bpp color depth should ensure full color range in automatic
>> mode.
>> 
>> The clamping has been HDMI specific since its introduction in
>> 
>> commit 996a2239f93b03c5972923f04b097f65565c5bed
>> Author: Daniel Vetter 
>> Date:   Fri Apr 19 11:24:34 2013 +0200
>> 
>> drm/i915: Disable high-bpc on pre-1.4 EDID screens
>> 
>> Cc: sta...@vger.kernel.org
>> Reported-by: Dihan Wickremasuriya 
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331
>> Signed-off-by: Jani Nikula 
>
> Makes sense to me as far as the spec is concerned.
> Reviewed-by: Ville Syrjälä 

Also got Tested-by in the bug. Pushed to drm-intel-next-queued, thanks
for the review.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 20 +++-
>>  1 file changed, 15 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 07ca19b0ec17..6eaecd9385ab 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12171,11 +12171,21 @@ connected_sink_compute_bpp(struct intel_connector 
>> *connector,
>>  pipe_config->pipe_bpp = connector->base.display_info.bpc*3;
>>  }
>>  
>> -/* Clamp bpp to 8 on screens without EDID 1.4 */
>> -if (connector->base.display_info.bpc == 0 && bpp > 24) {
>> -DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit 
>> of 24\n",
>> -  bpp);
>> -pipe_config->pipe_bpp = 24;
>> +/* Clamp bpp to default limit on screens without EDID 1.4 */
>> +if (connector->base.display_info.bpc == 0) {
>> +int type = connector->base.connector_type;
>> +int clamp_bpp = 24;
>> +
>> +/* Fall back to 18 bpp when DP sink capability is unknown. */
>> +if (type == DRM_MODE_CONNECTOR_DisplayPort ||
>> +type == DRM_MODE_CONNECTOR_eDP)
>> +clamp_bpp = 18;
>> +
>> +if (bpp > clamp_bpp) {
>> +DRM_DEBUG_KMS("clamping display bpp (was %d) to default 
>> limit of %d\n",
>> +  bpp, clamp_bpp);
>> +pipe_config->pipe_bpp = clamp_bpp;
>> +}
>>  }
>>  }
>>  
>> -- 
>> 2.1.4

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bios: Fix the sequence size calculations for MIPI seq v3

2016-01-15 Thread Jani Nikula
On Thu, 14 Jan 2016, Ville Syrjälä  wrote:
> On Thu, Jan 14, 2016 at 05:12:07PM +0200, Jani Nikula wrote:
>> Two errors in a single line. The size was read from the wrong offset,
>> and the end index didn't take the five bytes for sequence byte and size
>> of sequence into account. Fix it all, and break up the calculations a
>> bit to make it clearer.
>> 
>> Cc: Ville Syrjälä 
>> Reported-by: Mika Kahola 
>> Fixes: 2a33d93486f2 ("drm/i915/bios: add support for MIPI sequence block v3")
>> Signed-off-by: Jani Nikula 
>> ---
>>  drivers/gpu/drm/i915/intel_bios.c | 17 ++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
>> b/drivers/gpu/drm/i915/intel_bios.c
>> index 12e2f8b8bf9c..bf62a19c8f69 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -842,6 +842,7 @@ static int goto_next_sequence_v3(const u8 *data, int 
>> index, int total)
>>  {
>>  int seq_end;
>>  u16 len;
>> +u32 size_of_sequence;
>>  
>>  /*
>>   * Could skip sequence based on Size of Sequence alone, but also do some
>> @@ -852,14 +853,24 @@ static int goto_next_sequence_v3(const u8 *data, int 
>> index, int total)
>>  return 0;
>>  }
>>  
>> -seq_end = index + *((const u32 *)(data + 1));
>> +/* Skip Sequence Byte. */
>> +index++;
>> +
>> +/*
>> + * Size of Sequence. Excludes the Sequence Byte and the size itself,
>> + * includes MIPI_SEQ_ELEM_END byte, excludes the final MIPI_SEQ_END
>> + * byte.
>> + */
>> +size_of_sequence = *((const uint32_t *)(data + index));
>
> Hmm. So it was reading from 'data+1' and now it's basically 'data+index+1'.
> So it was correct for the first sequence, and busted for later ones I
> suppose.
>
>> +index += 4;
>> +
>> +seq_end = index + size_of_sequence;
>
> And now we count the size of the sequence starting from the operation
> byte, before we counted it from the sequence byte. "Fortunately" the spec
> doesn't even tell us which is correct. If it works, it works.
>
> Reviewed-by: Ville Syrjälä 

Pushed to drm-intel-next-queued, thanks for the review and testing.

BR,
Jani.

>
> BTW I was thinking that we could maybe add some kind of
> "read the thing at index, and and increment the index past it" helpers.
>
> Eg.
> int get_u8(const void *data, int index, int size, u8 *ret);
> int get_u32(const void *data, int index, int size, u32 *ret);
>
> they could also do the index vs. size check and just return an error if
> we try to go too far.
>
>>  if (seq_end > total) {
>>  DRM_ERROR("Invalid sequence size\n");
>>  return 0;
>>  }
>>  
>> -/* Skip Sequence Byte and Size of Sequence. */
>> -for (index = index + 5; index < total; index += len) {
>> +for (; index < total; index += len) {
>>  u8 operation_byte = *(data + index);
>>  index++;
>>  
>> -- 
>> 2.1.4

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Make sure DC writes are coherent on flush.

2016-01-15 Thread Jani Nikula
On Thu, 14 Jan 2016, Francisco Jerez  wrote:
> Jani Nikula  writes:
>
>> On Thu, 14 Jan 2016, Francisco Jerez  wrote:
>>> We need to set the DC FLUSH PIPE_CONTROL bit on Gen7+ to guarantee
>>> that writes performed via the HDC are visible in memory.  Fixes an
>>> intermittent failure in a Piglit test that writes to a BO from a
>>> shader using GL atomic counters (implemented as HDC untyped atomics)
>>> and then expects the memory to read back the same value after mapping
>>> it on the CPU.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91298
>>> Tested-by: Mark Janes 
>>> Cc: Rodrigo Vivi 
>>
>> Francisco, this is missing your Signed-off-by i.e. developer certificate
>> of origin http://developercertificate.org/ - can't push without. Please
>> reply with that.
>>
> Oops, sorry for that -- And yeah it shouldn't hurt to CC stable too.
>
> Signed-off-by: Francisco Jerez 

Pushed to drm-intel-next-queued, thanks for the patch and review.

BR,
Jani.



>
>> BR,
>> Jani.
>>
>>
>>> ---
>>>  drivers/gpu/drm/i915/intel_lrc.c| 1 +
>>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index ab344e0..02213c6 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1735,6 +1735,7 @@ static int gen8_emit_flush_render(struct 
>>> drm_i915_gem_request *request,
>>> if (flush_domains) {
>>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
>>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
>>> +   flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
>>> flags |= PIPE_CONTROL_FLUSH_ENABLE;
>>> }
>>>  
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
>>> b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index 4060acf..8cd8aab 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -331,6 +331,7 @@ gen7_render_ring_flush(struct drm_i915_gem_request *req,
>>> if (flush_domains) {
>>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
>>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
>>> +   flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
>>> flags |= PIPE_CONTROL_FLUSH_ENABLE;
>>> }
>>> if (invalidate_domains) {
>>> @@ -403,6 +404,7 @@ gen8_render_ring_flush(struct drm_i915_gem_request *req,
>>> if (flush_domains) {
>>> flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
>>> flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
>>> +   flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
>>> flags |= PIPE_CONTROL_FLUSH_ENABLE;
>>> }
>>> if (invalidate_domains) {
>>
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add support for mapping an object page by page

2016-01-15 Thread Tvrtko Ursulin


On 14/01/16 13:34, Chris Wilson wrote:

On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote:

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad8..5f86596 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -317,6 +317,11 @@ struct i915_address_space {
uint64_t start,
uint64_t length,
bool use_scratch);
+   void (*insert_page)(struct i915_address_space *vm,
+   dma_addr_t addr,
+   uint64_t offset,
+   enum i915_cache_level cache_level,
+   u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
   struct sg_table *st,
   uint64_t start,


Why not extend the current API to support start page offset and
number of pages? Could default to full object like today if zero.
Eg:

  void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
+   unsigned page_offset,
+   unsigned num_pages,


Ouch. That would be quite slow for the insert_page() use case of
page-by-page looping.


It could use the same last page caching trick so why would be be so slow?


uint64_t start,
enum i915_cache_level cache_level,
u32 flags);

That way we would not have two functions for effectively the same
thing operating on different type of input parameters.

If extending insert_entries is not preferable, then still we could
add another compatible one, like insert_entries_range or something,
and then both could share the same underlying implementation for
less code.

Like this, this patch already does not match current codebase - see
assert_rpm_atomic_begin in insert_entries.

Also if API between the two was compatible there would be no need
for i915_gem_object_get_dma_address() and i915_gem_object_get_page()
could be used instead.


The point was to write a lowlevel analog to provide a complementary API
to insert_entries that could be used for everywhere the we wanted to peek
through the GTT without even touching an object, i.e. for cases where we
might allocate a scratch page and temporarily put it into the GTT.


Well maybe it is not that bad on the API front, but code duplication 
would still not be my first choice, as demonstrated by the divergence 
which already happened. Need to think about it some more.


Regards,

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


Re: [Intel-gfx] [PATCH 7/8] drm/i915: Make display gtt offsets u32

2016-01-15 Thread Ville Syrjälä
On Thu, Jan 14, 2016 at 10:36:53PM +, Chris Wilson wrote:
> On Thu, Jan 14, 2016 at 03:22:15PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Using 'unsigned long' for ggtt offsets doesn't make much sense. Use
> > 'u32' instead since we've not yet seen a >4GiB ggtt.
> 
> We should do:
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 2e460b369e82..2ce16acceb75 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev)
> if (ret)
> return ret;
>  
> +   if (gtt->base.total >> 32) {
> +   DRM_ERROR("The code does not expect a Global GTT with more 
> that 32bits of address space! Found %lldM!\n",
> + gtt->base.total >> 20);
> +   gtt->base.total = 1ull << 32;
> +   gtt->mappable_end = min(gtt->mappable_end, gtt->base.total);
> +   }
> +
> /* GMADR is the PCI mmio aperture into the global GTT. */
> DRM_INFO("Memory usable by graphics device = %lluM\n",
>  gtt->base.total >> 20);
> 
> To get that early warning in.

Seems reasonable. Feel free to slap on
Reviewed-by: Ville Syrjälä 

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/6] drm/i915: Splitting intel_dp_detect

2016-01-15 Thread Ander Conselvan De Oliveira
On Thu, 2016-01-14 at 19:20 +0530, Shubhangi Shrivastava wrote:
> 
> On Wednesday 13 January 2016 07:03 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2016-01-13 at 13:20 +0200, Ander Conselvan De Oliveira wrote:
> > > On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote:
> > > > intel_dp_detect() is called for not just detection but
> > > > during modes enumeration as well. Repeating the whole
> > > > sequence during each of these calls is wasteful and
> > > > time consuming.
> > > > This patch moves probing for panel, DPCD read etc done in
> > > > intel_dp_detect() to a new function intel_dp_long_pulse().
> > > > Note that the behavior of intel_dp_detect() is changed to
> > > > report connected or disconnected depending on whether the
> > > > EDID is available or not.
> > > > This change will be required by further patches in the series
> > > > to avoid performing duplicated DPCD operations on hotplug.
> > > > 
> > > > v2: Moved a hunk to next patch of the series.
> > > >  Moved intel_dp_unset_edid to out. (Ander)
> > > > v3: Rephrased commit message and intel_dp_unset_dp() is called
> > > >  within intel_dp_set_dp() to free the previous EDID. (Ander)
> > > > 
> > > > Tested-by: Nathan D Ciobanu 
> > > > Signed-off-by: Sivakumar Thulasimani 
> > > > Signed-off-by: Shubhangi Shrivastava 
> > > > ---
> > > >   drivers/gpu/drm/i915/intel_dp.c | 56 +
> > > > -
> > > > --
> > > > -
> > > >   1 file changed, 35 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 796e3d3..e3b4208 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -129,6 +129,7 @@ static void edp_panel_vdd_off(struct intel_dp
> > > > *intel_dp,
> > > > bool sync);
> > > >   static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
> > > >   static void vlv_steal_power_sequencer(struct drm_device *dev,
> > > >   enum pipe pipe);
> > > > +static void intel_dp_unset_edid(struct intel_dp *intel_dp);
> > > >   
> > > >   static unsigned int intel_dp_unused_lane_mask(int lane_count)
> > > >   {
> > > > @@ -4587,6 +4588,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> > > > struct intel_connector *intel_connector = intel_dp
> > > > ->attached_connector;
> > > > struct edid *edid;
> > > >   
> > > > +   intel_dp_unset_edid(intel_dp);
> > > > edid = intel_dp_get_edid(intel_dp);
> > > > intel_connector->detect_edid = edid;
> > > >   
> > > > @@ -4607,9 +4609,10 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > > > intel_dp->has_audio = false;
> > > >   }
> > > >   
> > > > -static enum drm_connector_status
> > > > -intel_dp_detect(struct drm_connector *connector, bool force)
> > > > +static void
> > > > +intel_dp_long_pulse(struct intel_connector *intel_connector)
> > > >   {
> > > > +   struct drm_connector *connector = &intel_connector->base;
> > > > struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > > struct intel_digital_port *intel_dig_port =
> > > > dp_to_dig_port(intel_dp);
> > > > struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > > > @@ -4619,17 +4622,6 @@ intel_dp_detect(struct drm_connector *connector,
> > > > bool
> > > > force)
> > > > bool ret;
> > > > u8 sink_irq_vector;
> > > >   
> > > > -   DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > > > - connector->base.id, connector->name);
> > > > -   intel_dp_unset_edid(intel_dp);
> > > > -
> > > > -   if (intel_dp->is_mst) {
> > > > -   /* MST devices are disconnected from a monitor POV */
> > > > -   if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > > > -   intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > > > -   return connector_status_disconnected;
> > > > -   }
> > > > -
> > > > power_domain =
> > > > intel_display_port_aux_power_domain(intel_encoder);
> > > > intel_display_power_get(to_i915(dev), power_domain);
> > > >   
> > > > @@ -4653,14 +4645,8 @@ intel_dp_detect(struct drm_connector *connector,
> > > > bool
> > > > force)
> > > > intel_dp_probe_oui(intel_dp);
> > > >   
> > > > ret = intel_dp_probe_mst(intel_dp);
> > > > -   if (ret) {
> > > > -   /* if we are in MST mode then this connector
> > > > -  won't appear connected or have anything with EDID on
> > > > it
> > > > */
> > > > -   if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > > > -   intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> > > This deletion is new in this version of the patch. I think we still need
> > > the
> > > hunk above, otherwise we might not properly update the encoder type when
> > > we
> > > switch from an HDMI sink connected through a level shifte

[Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Chris Wilson
Throughout the code base, we use u32 for offsets into the global GTT. If
we ever see any hardware with a larger GGTT, then we run the real risk
of silent corruption. So test for our assumption up front so that we
have a nice reminder should the time come when it fails.

Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2e460b369e82..0d910638972c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev)
if (ret)
return ret;
 
+   if ((gtt->base.total - 1) >> 32) {
+   DRM_ERROR("We never expected a Global GTT with more than 32bits 
of address space! Found %lldM!\n",
+ gtt->base.total >> 20);
+   gtt->base.total = 1ull << 32;
+   gtt->mappable_end = min(gtt->mappable_end, gtt->base.total);
+   }
+
/* GMADR is the PCI mmio aperture into the global GTT. */
DRM_INFO("Memory usable by graphics device = %lluM\n",
 gtt->base.total >> 20);
-- 
2.7.0.rc3

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


Re: [Intel-gfx] [PATCH] drm/i915: Decouple execbuf uAPI from internal implementation

2016-01-15 Thread Tvrtko Ursulin


On 14/01/16 22:09, Chris Wilson wrote:

On Thu, Jan 14, 2016 at 03:02:59PM +, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

At the moment execbuf ring selection is fully coupled to
internal ring ids which is not a good thing on its own.

This dependency is also spread between two source files and
not spelled out at either side which makes it hidden and
fragile.

This patch decouples this dependency by introducing an explicit
translation table of execbuf uAPI to ring id close to the only
call site (i915_gem_do_execbuffer).

This way we are free to change driver internal implementation
details without breaking userspace. All state relating to the
uAPI is now contained in, or next to, i915_gem_do_execbuffer.


I was hesistant at first, since "why?" isn't fully developed, but the
code won me over.


+#define I915_USER_RINGS (4)
+
+static const enum intel_ring_id user_ring_map[I915_USER_RINGS + 1] = {
+   [I915_EXEC_DEFAULT] = RCS,
+   [I915_EXEC_RENDER]  = RCS,
+   [I915_EXEC_BLT] = BCS,
+   [I915_EXEC_BSD] = VCS,
+   [I915_EXEC_VEBOX]   = VECS
+};


I was wondering whether packing here mattered at all, but we're under a
cacheline so highly unlikely.


  static int
  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
   struct drm_file *file,
@@ -1393,6 +1393,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct i915_execbuffer_params params_master; /* XXX: will be removed 
later */
struct i915_execbuffer_params *params = ¶ms_master;
const u32 ctx_id = i915_execbuffer2_get_context_id(*args);
+   unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK;
u32 dispatch_flags;
int ret;
bool need_relocs;
@@ -1414,49 +1415,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
*data,
if (args->flags & I915_EXEC_IS_PINNED)
dispatch_flags |= I915_DISPATCH_PINNED;

-   if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
-   DRM_DEBUG("execbuf with unknown ring: %d\n",
- (int)(args->flags & I915_EXEC_RING_MASK));
+   if (user_ring_id > I915_USER_RINGS) {
+   DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id);
return -EINVAL;
}

-   if (((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_BSD) &&
+   if ((user_ring_id != I915_EXEC_BSD) &&
((args->flags & I915_EXEC_BSD_MASK) != 0)) {
DRM_DEBUG("execbuf with non bsd ring but with invalid "
"bsd dispatch flags: %d\n", (int)(args->flags));
return -EINVAL;
-   }
-
-   if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
-   ring = &dev_priv->ring[RCS];
-   else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
-   if (HAS_BSD2(dev)) {
-   int ring_id;
-
-   switch (args->flags & I915_EXEC_BSD_MASK) {
-   case I915_EXEC_BSD_DEFAULT:
-   ring_id = gen8_dispatch_bsd_ring(dev, file);
-   ring = &dev_priv->ring[ring_id];
-   break;
-   case I915_EXEC_BSD_RING1:
-   ring = &dev_priv->ring[VCS];
-   break;
-   case I915_EXEC_BSD_RING2:
-   ring = &dev_priv->ring[VCS2];
-   break;
-   default:
-   DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
- (int)(args->flags & 
I915_EXEC_BSD_MASK));
-   return -EINVAL;
-   }
-   } else
-   ring = &dev_priv->ring[VCS];
-   } else
-   ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1];
+   }
+
+   if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev)) {


HAS_BSD2(dev_priv)


+   unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
+
+   if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
+   bsd_idx = gen8_dispatch_bsd_ring(dev_priv, file);
+   } else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
+  bsd_idx <= I915_EXEC_BSD_RING2) {
+   bsd_idx--;
+   } else {
+   DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
+ bsd_idx);
+   return -EINVAL;
+   }
+
+   ring = &dev_priv->ring[_VCS(bsd_idx)];
+   } else {
+   ring = &dev_priv->ring[user_ring_map[user_ring_id]];
+   }

if (!intel_ring_initialized(ring)) {
-   DRM_DEBUG("execbuf with invalid ring: %d\n",
- (int)(args->flags & I915_EXEC_RING_MASK));
+   

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (bdw-nuci7)
pass   -> DMESG-WARN (bdw-ultra)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)

bdw-nuci7total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1194/

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


Re: [Intel-gfx] [PATCH v10] drm/i915: Extend LRC pinning to cover GPU context writeback

2016-01-15 Thread Nick Hoath

On 14/01/2016 12:37, Nick Hoath wrote:

On 14/01/2016 12:31, Chris Wilson wrote:

On Thu, Jan 14, 2016 at 11:56:07AM +, Nick Hoath wrote:

On 14/01/2016 11:36, Chris Wilson wrote:

On Wed, Jan 13, 2016 at 04:19:45PM +, Nick Hoath wrote:

+   if (ctx->engine[ring->id].dirty) {
+   struct drm_i915_gem_request *req = NULL;
+
+   /**
+* If there is already a request pending on
+* this ring, wait for that to complete,
+* otherwise create a switch to idle request
+*/
+   if (list_empty(&ring->request_list)) {
+   int ret;
+
+   ret = i915_gem_request_alloc(
+   ring,
+   ring->default_context,
+   &req);
+   if (!ret)
+   i915_add_request(req);
+   else
+   DRM_DEBUG("Failed to ensure context saved");
+   } else {
+   req = list_first_entry(
+   &ring->request_list,
+   typeof(*req), list);
+   }
+   if (req) {
+   ret = i915_wait_request(req);
+   if (ret != 0) {
+   /**
+* If we get here, there's probably been a ring
+* reset, so we just clean up the dirty flag.&
+* pin count.
+*/
+   ctx->engine[ring->id].dirty = false;
+   __intel_lr_context_unpin(
+   ring,
+   ctx);
+   }
+   }


If you were to take a lr_context_pin on the last_context, and only
release that pin when you change to a new context, you do not need to


That what this patch does.


introduce a blocking context-close, nor do you need to introduce the
usage of default_context.


The use of default_context here is to stop a context hanging around
after it is no longer needed.


By blocking, which is not acceptable. Also we can eliminate the
default_context and so pinning that opposed to the last_context serves
no purpose other than by chance having a more preferrable position when
it comes to defragmentation. But you don't enable that anyway and we


Enabling the shrinker on execlists is something I'm working on which is
predicated on this patch. Also why is blocking on closing a context not
acceptable?



As a clarification: Without rewriting the execlist code to not submit or 
cleanup from an interrupt handler, we can't use refcounting to allow non 
blocking closing.



have alternative strategies now that avoid the issue with fragmentation
of the mappable aperture.


(lr_context_pin should take a reference on the ctx to prevent early
freeeing ofc).


You can't clear the reference on the ctx in an interrupt context.


The execlists submission should moved out of the interrupt context, for
the very simple reason that it is causing machine panics. userspace
submits a workload, machine lockups


Create a jira, and I'm sure we'll look at making that change.


-Chris



___
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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
Tvrtko was looking through the execbuffer-ioctl and noticed that the
uABI was tightly coupled to our internal engine identifiers. Close
inspection also revealed that we leak those internal engine identifiers
through the busy-ioctl, and those internal identifiers already do not
match the user identifiers. Fortuitiously, there is only one user of the
set of busy rings from the busy-ioctl, and they only wish to choose
between the RENDER and the BLT engines.

Let's fix the userspace ABI while we still can.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem.c | 18 ++
 drivers/gpu/drm/i915/intel_lrc.c|  5 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb44bad15403..85797813a3de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret)
goto unref;
 
-   BUILD_BUG_ON(I915_NUM_RINGS > 16);
-   args->busy = obj->active << 16;
-   if (obj->last_write_req)
-   args->busy |= obj->last_write_req->ring->id;
+   args->busy = 0;
+   if (obj->active) {
+   int i;
+
+   for (i = 0; i < I915_NUM_RINGS; i++) {
+   struct drm_i915_gem_request *req;
+
+   req = obj->last_read_req[i];
+   if (req)
+   args->busy |= 1 << (16 + req->ring->exec_id);
+   }
+   if (obj->last_write_req)
+   args->busy |= obj->last_write_req->ring->exec_id;
+   }
 
 unref:
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f5d89c845ede..4aa209483237 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device 
*dev)
 
ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
@@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)
 
ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN6_BSD_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
@@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)
 
ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN8_BSD2_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
@@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev)
 
ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;
ring->mmio_base = BLT_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
@@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)
 
ring->name = "video enhancement ring";
ring->id = VECS;
+   ring->exec_id = I915_EXEC_VEBOX;
ring->mmio_base = VEBOX_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8cd8aabcc3ff..310d151c0db2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;
 
if (INTEL_INFO(dev)->gen >= 8) {
@@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 
ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;
 
ring->write_tail = ring_write_tail;
if (INTEL_INFO(dev)->gen >= 6) {
@@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 
ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;
 
ring->write_tail = ring_write_tail;
ring->mmio_base = GEN8_BSD2_RING_BASE;
@@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 
ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;
 
ring->mmio_base = BLT_RING_BASE;
ring->write_tail = ring_write_tail;
@@ -2991,6 +2995,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 
ring->name = "video enhancement ring";
r

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (bdw-nuci7)
pass   -> DMESG-WARN (skl-i7k-2) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:122  dwarn:4   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1195/

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


Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Dave Gordon

On 15/01/16 10:20, Chris Wilson wrote:

Throughout the code base, we use u32 for offsets into the global GTT. If
we ever see any hardware with a larger GGTT, then we run the real risk
of silent corruption. So test for our assumption up front so that we
have a nice reminder should the time come when it fails.

Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2e460b369e82..0d910638972c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev)
if (ret)
return ret;

+   if ((gtt->base.total - 1) >> 32) {
+   DRM_ERROR("We never expected a Global GTT with more than 32bits of 
address space! Found %lldM!\n",
+ gtt->base.total >> 20);
+   gtt->base.total = 1ull << 32;
+   gtt->mappable_end = min(gtt->mappable_end, gtt->base.total);


Assuming Mika's comment on 'struct i915_address_space' is correct:
...
u64 start;  /* Start offset always 0 for dri2 */
...
otherwise this calculation would need to be adjusted.


+
/* GMADR is the PCI mmio aperture into the global GTT. */
DRM_INFO("Memory usable by graphics device = %lluM\n",
 gtt->base.total >> 20);


LGTM.

Reviewed-by: Dave Gordon 

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


Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 10:20:11AM +, Chris Wilson wrote:
> Throughout the code base, we use u32 for offsets into the global GTT. If
> we ever see any hardware with a larger GGTT, then we run the real risk
> of silent corruption. So test for our assumption up front so that we
> have a nice reminder should the time come when it fails.
> 
> Signed-off-by: Chris Wilson 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 

From the earlier cut-n-paste,

Reviewed-by: Ville Syrjälä 

(so I hope it still holds with the minor correction applied :)
-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] drm/i915: Decouple execbuf uAPI from internal implementation

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 10:29:05AM +, Tvrtko Ursulin wrote:
> >One question, how does this look if we move this section to its own
> >function:
> >
> >ring = eb_select_ring(dev_priv, file, args);
> >if (IS_ERR(ring))
> > return PTR_ERR(ring);
> >
> >Do you keep your code size reduction?
> 
> No, perhaps because of all the ERR_PTR business. But decoupling the
> ring and return value keeps it:
> 
> static int
> eb_select_ring(struct drm_i915_private *dev_priv,
>  struct drm_file *file,
>  struct drm_i915_gem_execbuffer2 *args,
>  struct intel_engine_cs **ring)

Hmm, I wonder why. ERR_PTR(ret) should be a no-op, so the only thing
that I would have thought changed would be the test afterwards. I guess
a foray into the assembly would be a good learning experience for me then!

> Saves ~100 bytes out of ~4600 on my build.
> 
> >>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
> >>b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>index 7349d9258191..fdc220fc2b18 100644
> >>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>@@ -148,14 +148,14 @@ struct  i915_ctx_workarounds {
> >>  struct  intel_engine_cs {
> >>const char  *name;
> >>enum intel_ring_id {
> >>-   RCS = 0x0,
> >>-   VCS,
> >>+   RCS = 0,
> >>BCS,
> >>-   VECS,
> >>-   VCS2
> >>+   VCS,
> >>+   VCS2,   /* Keep instances of the same type engine together. */
> >>+   VECS
> >
> >Technically, this breaks userspace ABI elsewhere. :| Though the only
> >user of the id mask is only looking for RCS==0 vs the reset.
> >
> >So I think we would cope, but to be extra safe we could just avoid
> >reshuffling the ids. Let me sleep on the implications. We may say just
> >break that ABI whilst we still can and do a reverse-map to EXEC bit.
> >Hmm.
> 
> What are the other places it breaks the ABI? I'd like to understand it.

The busy-ioctl is strong abi, i915_trace.h has a few weak abi uses of
ring->id, and the use in debugfs is no abi barrier at all.

ring->id also crops up in the guc_context_desc, which is intriguing.
-Cris

-- 
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 01/11] drm/i915: Add support for mapping an object page by page

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 09:53:46AM +, Tvrtko Ursulin wrote:
> 
> On 14/01/16 13:34, Chris Wilson wrote:
> >On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote:
> >>>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
> >>>b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>>index b448ad8..5f86596 100644
> >>>--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>>@@ -317,6 +317,11 @@ struct i915_address_space {
> >>>   uint64_t start,
> >>>   uint64_t length,
> >>>   bool use_scratch);
> >>>+  void (*insert_page)(struct i915_address_space *vm,
> >>>+  dma_addr_t addr,
> >>>+  uint64_t offset,
> >>>+  enum i915_cache_level cache_level,
> >>>+  u32 flags);
> >>>   void (*insert_entries)(struct i915_address_space *vm,
> >>>  struct sg_table *st,
> >>>  uint64_t start,
> >>
> >>Why not extend the current API to support start page offset and
> >>number of pages? Could default to full object like today if zero.
> >>Eg:
> >>
> >>  void (*insert_entries)(struct i915_address_space *vm,
> >>struct sg_table *st,
> >>+   unsigned page_offset,
> >>+   unsigned num_pages,
> >
> >Ouch. That would be quite slow for the insert_page() use case of
> >page-by-page looping.
> 
> It could use the same last page caching trick so why would be be so slow?

We don't have the convenient struct? I presumed you were thinking of
passing the offset through to sg_page_iter

for_each_sg_page(st->sgl, &sg_iter, st->nents, page_offset) {
if (--num_pages)
break;

/* original pt insertion code */
}

Is it worth expanding struct sg_table for this shortcut?

There are some games where we spend more time in sg_page_iter_next() than
anything else in the kernel. The games have more textures than GTT
space, they aren't playable right now nor will they ever likely to be. :(
Improving these loops would definitely be a boon nevertheless.
-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] drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 11:25:38AM +, Dave Gordon wrote:
> On 15/01/16 10:20, Chris Wilson wrote:
> >Throughout the code base, we use u32 for offsets into the global GTT. If
> >we ever see any hardware with a larger GGTT, then we run the real risk
> >of silent corruption. So test for our assumption up front so that we
> >have a nice reminder should the time come when it fails.
> >
> >Signed-off-by: Chris Wilson 
> >Cc: Ville Syrjälä 
> >Cc: Daniel Vetter 
> >---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> >b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >index 2e460b369e82..0d910638972c 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >@@ -3143,6 +3143,13 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > if (ret)
> > return ret;
> >
> >+if ((gtt->base.total - 1) >> 32) {
> >+DRM_ERROR("We never expected a Global GTT with more than 32bits 
> >of address space! Found %lldM!\n",
> >+  gtt->base.total >> 20);
> >+gtt->base.total = 1ull << 32;
> >+gtt->mappable_end = min(gtt->mappable_end, gtt->base.total);
> 
> Assuming Mika's comment on 'struct i915_address_space' is correct:
> ...
>   u64 start;  /* Start offset always 0 for dri2 */
> ...
> otherwise this calculation would need to be adjusted.

The address_space start was for the obsolete UMS call where the xserver
would tell the kernel the range of the (then only global) GTT to use for
itself.

The vgpu plugin comes later? Though if I remember correctly, it reserves
ranges of the GGTT for itself rather than alter the drm_mm.

So I don't think it has been revitalised since. Removing the start value
from the struct i915_address_space confirms it is obsolete.
-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] drm/i915: Remove obsolete starting offset from struct i915_address_space

2016-01-15 Thread Chris Wilson
Since the removal of UMS, we have always had sole ownership of the GTTs.
We no longer have to track the subsection we are allowed to use (denoted
by start + total). vGPU does restrict the range of the global GTT we can
use (as it has to share it amongst all the clients on the host), but
that is achieved by ballooning reserved node within the whole (so that
it could adjust available space dynamically). As such
i915_address_space.start is always 0 and we can remove having to worry
about such complications.

Signed-off-by: Chris Wilson 
Cc: Dave Gordon 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_debugfs.c |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 24 
 drivers/gpu/drm/i915/i915_gem_gtt.h |  1 -
 drivers/gpu/drm/i915/i915_vgpu.c| 17 +++--
 4 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e3377abc0d4d..b1bd710f645a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -495,7 +495,7 @@ static int i915_gem_object_info(struct seq_file *m, void* 
data)
 
seq_printf(m, "%llu [%llu] gtt total\n",
   dev_priv->gtt.base.total,
-  (u64)dev_priv->gtt.mappable_end - dev_priv->gtt.base.start);
+  dev_priv->gtt.mappable_end);
 
seq_putc(m, '\n');
print_batch_pool_stats(m, dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 56f4f2e58d53..11080f00bda1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1443,7 +1443,7 @@ static void gen8_dump_pdp(struct 
i915_page_directory_pointer *pdp,
 static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 {
struct i915_address_space *vm = &ppgtt->base;
-   uint64_t start = ppgtt->base.start;
+   uint64_t start = 0;
uint64_t length = ppgtt->base.total;
gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
 I915_CACHE_LLC, true);
@@ -1507,7 +1507,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
if (ret)
return ret;
 
-   ppgtt->base.start = 0;
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.allocate_va_range = gen8_alloc_va_range;
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
@@ -1560,7 +1559,7 @@ static void gen6_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, 
struct seq_file *m)
gen6_pte_t scratch_pte;
uint32_t pd_entry;
uint32_t  pte, pde, temp;
-   uint32_t start = ppgtt->base.start, length = ppgtt->base.total;
+   uint32_t start = 0, length = ppgtt->base.total;
 
scratch_pte = vm->pte_encode(px_dma(vm->scratch_page),
 I915_CACHE_LLC, true, 0);
@@ -2086,7 +2085,6 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
ppgtt->base.unbind_vma = ppgtt_unbind_vma;
ppgtt->base.bind_vma = ppgtt_bind_vma;
ppgtt->base.cleanup = gen6_ppgtt_cleanup;
-   ppgtt->base.start = 0;
ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
ppgtt->debug_dump = gen6_dump_ppgtt;
 
@@ -2123,7 +2121,7 @@ static int __hw_ppgtt_init(struct drm_device *dev, struct 
i915_hw_ppgtt *ppgtt)
 static void i915_address_space_init(struct i915_address_space *vm,
struct drm_i915_private *dev_priv)
 {
-   drm_mm_init(&vm->mm, vm->start, vm->total);
+   drm_mm_init(&vm->mm, 0, vm->total);
vm->dev = dev_priv->dev;
INIT_LIST_HEAD(&vm->active_list);
INIT_LIST_HEAD(&vm->inactive_list);
@@ -2312,8 +2310,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_device *dev)
i915_check_and_clear_faults(dev);
 
dev_priv->gtt.base.clear_range(&dev_priv->gtt.base,
-  dev_priv->gtt.base.start,
-  dev_priv->gtt.base.total,
+  0, dev_priv->gtt.base.total,
   true);
 
i915_ggtt_flush(dev_priv);
@@ -2681,7 +2678,6 @@ static void i915_gtt_color_adjust(struct drm_mm_node 
*node,
 }
 
 static int i915_gem_setup_global_gtt(struct drm_device *dev,
-u64 start,
 u64 mappable_end,
 u64 end)
 {
@@ -2703,11 +2699,9 @@ static int i915_gem_setup_global_gtt(struct drm_device 
*dev,
 
BUG_ON(mappable_end > end);
 
-   ggtt_vm->start = start;
-
/* Subtract the guard page before address space initialization to
 * shrink the range used by drm_mm */
-   ggtt_vm->total = end - start - PAGE_SIZE;
+   ggtt_vm->total = end - PAGE_SIZE;
i915_address_space_init(ggtt_vm, dev_priv);
ggtt_vm->total += PAGE_SIZE;
 
@@ -2773,8 +2767,

Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 11:06, Chris Wilson wrote:

Tvrtko was looking through the execbuffer-ioctl and noticed that the
uABI was tightly coupled to our internal engine identifiers. Close
inspection also revealed that we leak those internal engine identifiers
through the busy-ioctl, and those internal identifiers already do not
match the user identifiers. Fortuitiously, there is only one user of the
set of busy rings from the busy-ioctl, and they only wish to choose
between the RENDER and the BLT engines.

Let's fix the userspace ABI while we still can.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/i915/i915_gem.c | 18 ++
  drivers/gpu/drm/i915/intel_lrc.c|  5 +
  drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +
  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
  4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb44bad15403..85797813a3de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret)
goto unref;

-   BUILD_BUG_ON(I915_NUM_RINGS > 16);
-   args->busy = obj->active << 16;
-   if (obj->last_write_req)
-   args->busy |= obj->last_write_req->ring->id;
+   args->busy = 0;
+   if (obj->active) {
+   int i;
+
+   for (i = 0; i < I915_NUM_RINGS; i++) {
+   struct drm_i915_gem_request *req;
+
+   req = obj->last_read_req[i];
+   if (req)
+   args->busy |= 1 << (16 + req->ring->exec_id);


If I got it right bit 16 was RCS, now will always be clear. And blitter 
was bit 17 and now is 19.


Regards,

Tvrtko


+   }
+   if (obj->last_write_req)
+   args->busy |= obj->last_write_req->ring->exec_id;
+   }

  unref:
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f5d89c845ede..4aa209483237 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device 
*dev)

ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;

logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
@@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)

ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN6_BSD_RING_BASE;

logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
@@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)

ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN8_BSD2_RING_BASE;

logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
@@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev)

ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;
ring->mmio_base = BLT_RING_BASE;

logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
@@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)

ring->name = "video enhancement ring";
ring->id = VECS;
+   ring->exec_id = I915_EXEC_VEBOX;
ring->mmio_base = VEBOX_RING_BASE;

logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8cd8aabcc3ff..310d151c0db2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)

ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;

if (INTEL_INFO(dev)->gen >= 8) {
@@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)

ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;

ring->write_tail = ring_write_tail;
if (INTEL_INFO(dev)->gen >= 6) {
@@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)

ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;

ring->write_tail = ring_write_tail;
ring->mmio_base = GEN8_BSD2_RING_BASE;
@@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)

ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;

ring->mmio_base = BLT_RING_BASE;
ring->write_tail = ring

Re: [Intel-gfx] ✗ failure: Fi.CI.BAT

2016-01-15 Thread Chris Wilson
On Thu, Jan 14, 2016 at 05:00:02PM +0200, Ville Syrjälä wrote:
> On Thu, Jan 14, 2016 at 02:49:45PM -, Patchwork wrote:
> > == Summary ==
> > 
> > Built on 8fb2feecca499d11e104264071ac55e273e23af5 drm-intel-nightly: 
> > 2016y-01m-14d-13h-06m-44s UTC integration manifest
> > 
> > Test gem_ctx_basic:
> > pass   -> FAIL   (bdw-ultra)
> 
> "Returncode -15" and nothing more. Weird.

Strikes me as a bug in the testrunner. That's not one of ours.
-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 079/190] drm/i915: Reduce the pointer dance of i915_is_ggtt()

2016-01-15 Thread Dave Gordon

On 11/01/16 09:17, Chris Wilson wrote:

The multiple levels of indirect do nothing but hinder the compiler and
the pointer chasing turns to be quite painful but painless to fix.

Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/i915_debugfs.c| 13 ++---
  drivers/gpu/drm/i915/i915_drv.h|  7 ---
  drivers/gpu/drm/i915/i915_gem.c| 18 +++---
  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 ++---
  drivers/gpu/drm/i915/i915_gem_gtt.c| 12 +---
  drivers/gpu/drm/i915/i915_gem_gtt.h|  5 +
  drivers/gpu/drm/i915/i915_trace.h  | 27 ---
  7 files changed, 33 insertions(+), 54 deletions(-)


[snip]


diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9c1a5cdc1e5..f840cc55f1ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2905,18 +2905,11 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object 
*obj);
  /* Some GGTT VM helpers */
  #define i915_obj_to_ggtt(obj) \
(&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base)
-static inline bool i915_is_ggtt(struct i915_address_space *vm)
-{
-   struct i915_address_space *ggtt =
-   &((struct drm_i915_private *)(vm)->dev->dev_private)->gtt.base;
-   return vm == ggtt;
-}



diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b5c3bbe6dc2a..06117bd0fc00 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3150,6 +3150,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
}

gtt->base.dev = dev;
+   gtt->base.is_ggtt = true;


So, it looks like the plan here is that when we need to determine 
whether something is the special distinguished instance of a type, then 
instead of comparing its address against the global pointer to the 
distinguished instance, we'll just look for a flag /inside/ the object 
itself, which is set /only/ on the distinguished instance.


Now why didn't I think of that? That looks like such a good idea, we 
should apply it in other CONTEXTs!


Reviewed-by: Dave Gordon 

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


[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Remove obsolete starting offset from struct i915_address_space

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i5k-2) UNSTABLE
dmesg-warn -> PASS   (bdw-nuci7)
pass   -> DMESG-WARN (skl-i7k-2) UNSTABLE
pass   -> DMESG-WARN (bdw-ultra)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)

bdw-nuci7total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1196/

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


Re: [Intel-gfx] [PATCH 079/190] drm/i915: Reduce the pointer dance of i915_is_ggtt()

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 12:12:15PM +, Dave Gordon wrote:
> On 11/01/16 09:17, Chris Wilson wrote:
> >The multiple levels of indirect do nothing but hinder the compiler and
> >the pointer chasing turns to be quite painful but painless to fix.
> >
> >Signed-off-by: Chris Wilson 
> >---
> >  drivers/gpu/drm/i915/i915_debugfs.c| 13 ++---
> >  drivers/gpu/drm/i915/i915_drv.h|  7 ---
> >  drivers/gpu/drm/i915/i915_gem.c| 18 +++---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  5 ++---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c| 12 +---
> >  drivers/gpu/drm/i915/i915_gem_gtt.h|  5 +
> >  drivers/gpu/drm/i915/i915_trace.h  | 27 ---
> >  7 files changed, 33 insertions(+), 54 deletions(-)
> 
> [snip]
> 
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> >b/drivers/gpu/drm/i915/i915_drv.h
> >index c9c1a5cdc1e5..f840cc55f1ab 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2905,18 +2905,11 @@ bool i915_gem_obj_is_pinned(struct 
> >drm_i915_gem_object *obj);
> >  /* Some GGTT VM helpers */
> >  #define i915_obj_to_ggtt(obj) \
> > (&((struct drm_i915_private *)(obj)->base.dev->dev_private)->gtt.base)
> >-static inline bool i915_is_ggtt(struct i915_address_space *vm)
> >-{
> >-struct i915_address_space *ggtt =
> >-&((struct drm_i915_private *)(vm)->dev->dev_private)->gtt.base;
> >-return vm == ggtt;
> >-}
> 
> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> >b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >index b5c3bbe6dc2a..06117bd0fc00 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >@@ -3150,6 +3150,7 @@ int i915_gem_gtt_init(struct drm_device *dev)
> > }
> >
> > gtt->base.dev = dev;
> >+gtt->base.is_ggtt = true;
> 
> So, it looks like the plan here is that when we need to determine
> whether something is the special distinguished instance of a type,
> then instead of comparing its address against the global pointer to
> the distinguished instance, we'll just look for a flag /inside/ the
> object itself, which is set /only/ on the distinguished instance.
> 
> Now why didn't I think of that? That looks like such a good idea, we
> should apply it in other CONTEXTs!

But we already have that flag in contexts! It also happens to be useful
for other tracking as well. And we demonstrated that we didn't even need
the checks for the kernel context anyway.

You will also note this is a small stepping patch after which we
transition away from i915_address_space.is_ggtt to using the owner.
-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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 11:58:32AM +, Tvrtko Ursulin wrote:
> 
> On 15/01/16 11:06, Chris Wilson wrote:
> >Tvrtko was looking through the execbuffer-ioctl and noticed that the
> >uABI was tightly coupled to our internal engine identifiers. Close
> >inspection also revealed that we leak those internal engine identifiers
> >through the busy-ioctl, and those internal identifiers already do not
> >match the user identifiers. Fortuitiously, there is only one user of the
> >set of busy rings from the busy-ioctl, and they only wish to choose
> >between the RENDER and the BLT engines.
> >
> >Let's fix the userspace ABI while we still can.
> >
> >Signed-off-by: Chris Wilson 
> >Cc: Tvrtko Ursulin 
> >Cc: Daniel Vetter 
> >---
> >  drivers/gpu/drm/i915/i915_gem.c | 18 ++
> >  drivers/gpu/drm/i915/intel_lrc.c|  5 +
> >  drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
> >  4 files changed, 25 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> >b/drivers/gpu/drm/i915/i915_gem.c
> >index bb44bad15403..85797813a3de 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void 
> >*data,
> > if (ret)
> > goto unref;
> >
> >-BUILD_BUG_ON(I915_NUM_RINGS > 16);
> >-args->busy = obj->active << 16;
> >-if (obj->last_write_req)
> >-args->busy |= obj->last_write_req->ring->id;
> >+args->busy = 0;
> >+if (obj->active) {
> >+int i;
> >+
> >+for (i = 0; i < I915_NUM_RINGS; i++) {
> >+struct drm_i915_gem_request *req;
> >+
> >+req = obj->last_read_req[i];
> >+if (req)
> >+args->busy |= 1 << (16 + req->ring->exec_id);
> 
> If I got it right bit 16 was RCS, now will always be clear. And
> blitter was bit 17 and now is 19.

Sssh. You weren't meant to point that out.

I am willing to take the hit in order to decouple the uABI from
internals.

I am also willing to codify this busy-ioctl ABI into igt!
-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 2/2] lib/igt_kms: Short description between Intel/DRM terminology.

2016-01-15 Thread Ville Syrjälä
On Fri, Jan 15, 2016 at 11:06:42AM +0200, Marius Vlad wrote:
> lib/igt_kms: Briefly describe Intel-to-DRM mapping between pipes, encoders and
> connectors.
> 
> Signed-off-by: Marius Vlad 
> ---
>  lib/igt_kms.c | 82 
> +++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index c7a0b77..caa8837 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -68,6 +68,88 @@
>   * functions have all igt_ prefixes. This part is still very much work in
>   * progress and so also lacks a bit documentation for the individual 
> functions.
>   *
> + * Intel/DRM terminology and display connections:
> + *
> + * Intel documentation describes the road from memory to an output connector 
> as
> + * follows:
> + *
> + * |[!<-- language="C" -->
> + * .. .---. .-. .-.
> + * | Memory |>| Pipes |>| Transcoders |>| DDI |
> + * '' '---' '-' '-'
> + * ]|
> + *
> + * Pipes represent the front-end of the display contain the planes, blending,
> + * and color correction, while the transcoders contain timing generators,
> + * encoders, A/V mixers and PSR (Panel-Self Refresh) controllers. Finally the
> + * DDI represent the connectors attached to the display.
> + *
> + *
> + *  In DRM we have the following:
> + *
> + * |[!<-- language="C" -->
> + * .---. .---. .---. ..
> + * | Framebuff |>| Pipes |>| Encoders  |>| Connectors |
> + * '---' '---' '---' ''

"Framebuff"?

Also should this say crtc instead of pipe?

> + * ]|
> + *
> + *
> + * The frame buffer ties a reference to a memory object and provides a 
> pointer
> + * to the actual data.
> + *
> + * The pipe is used to set the display mode, timings and gamma tables. On 
> some
> + * hardware models this is tied with the transcoder.  In DRM-parlance this is
> + * referred as a CRTC.
> + *
> + * Each pipe has multiple planes. On older hardware these planes where known 
> as
> + * primary plane, overlay/sprite plane, and cursor plane. From GEN9 (SKL/BXT)
> + * each pipe has three planes and a cursor plane.

Not quite true.

> Each plane can be used as a
> + * primary, as a sprite or as an overlay plane. The planes are the ones that
> + * retrieve the pixels from memory and pushes them to the encoder.
> + *
> + * A pipe prior to GEN9:

Really more like g4x-bdw. Before g4x it was totally different, and gen4 was
sort of mix of both the old and the new. And vlv/chv have two sprites on
each pipe.

So given all the variations in the hardware, and the fact that it keeps
changing all the time, I'm not convinced there's any point in
documenting this in igt. It'll get stale real quick, and there are
efforts to decouple igt from i915 as much as possible, so next thing you
know someone else will want to docuemnt their favorite hardware here as
well.

So if we do want to document this stuff, then I think it should be
somewhere in the kernel modeset code (around crtc/plane init I suppose).

> + *
> + * |[!<-- language="C" -->
> + * ..
> + * | Memory | ..
> + * ||>| Cursor |-->...
> + * || ''
> + * ||
> + * || ..
> + * ||>| Sprite |-->...
> + * || ''
> + * ||
> + * ||  .-.
> + * ||->| Primary |>...
> + * ||  '-'
> + * ''
> + * ]|
> + *
> + * A pipe with universal planes:
> + *
> + * |[!<-- language="C" -->
> + * ..
> + * | Memory | ..
> + * ||>| Cursor |-->...
> + * || ''
> + * ||
> + * || .---.
> + * ||>| Plane |-->...
> + * || '---'
> + * ||
> + * ||  .---.
> + * ||->| Plane |>...
> + * ||  '---'
> + * ''
> + * ]|
> + *
> + * The encoder is used to convert, depending on the output, pixels from pipes
> + * to signals understood by the connector.
> + *
> + * The connector will connect to the output display. This contains 
> information
> + * about the attached display such as EDID, DPMS and information about modes
> + * supported by the display.
> + *
>   * Note that this library's header pulls in the [i-g-t
>   * framebuffer](intel-gpu-tools-Framebuffer.html) library as a
>   * dependency.
> -- 
> 2.6.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Codify our assumption that the Global GTT is <= 4GiB

2016-01-15 Thread Ville Syrjälä
On Fri, Jan 15, 2016 at 11:28:16AM +, Chris Wilson wrote:
> On Fri, Jan 15, 2016 at 10:20:11AM +, Chris Wilson wrote:
> > Throughout the code base, we use u32 for offsets into the global GTT. If
> > we ever see any hardware with a larger GGTT, then we run the real risk
> > of silent corruption. So test for our assumption up front so that we
> > have a nice reminder should the time come when it fails.
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Ville Syrjälä 
> > Cc: Daniel Vetter 
> 
> >From the earlier cut-n-paste,
> 
> Reviewed-by: Ville Syrjälä 
> 
> (so I hope it still holds with the minor correction applied :)

Yes. Please excuse the sucky review I gave to the original.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 12:27, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 11:58:32AM +, Tvrtko Ursulin wrote:


On 15/01/16 11:06, Chris Wilson wrote:

Tvrtko was looking through the execbuffer-ioctl and noticed that the
uABI was tightly coupled to our internal engine identifiers. Close
inspection also revealed that we leak those internal engine identifiers
through the busy-ioctl, and those internal identifiers already do not
match the user identifiers. Fortuitiously, there is only one user of the
set of busy rings from the busy-ioctl, and they only wish to choose
between the RENDER and the BLT engines.

Let's fix the userspace ABI while we still can.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Daniel Vetter 
---
  drivers/gpu/drm/i915/i915_gem.c | 18 ++
  drivers/gpu/drm/i915/intel_lrc.c|  5 +
  drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +
  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
  4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb44bad15403..85797813a3de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret)
goto unref;

-   BUILD_BUG_ON(I915_NUM_RINGS > 16);
-   args->busy = obj->active << 16;
-   if (obj->last_write_req)
-   args->busy |= obj->last_write_req->ring->id;
+   args->busy = 0;
+   if (obj->active) {
+   int i;
+
+   for (i = 0; i < I915_NUM_RINGS; i++) {
+   struct drm_i915_gem_request *req;
+
+   req = obj->last_read_req[i];
+   if (req)
+   args->busy |= 1 << (16 + req->ring->exec_id);


If I got it right bit 16 was RCS, now will always be clear. And
blitter was bit 17 and now is 19.


Sssh. You weren't meant to point that out.

I am willing to take the hit in order to decouple the uABI from
internals.

I am also willing to codify this busy-ioctl ABI into igt!


Looks like your DDX is the only user not using it in the boolean mode?

And libdrm is a bit confused in its return statements:

ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
if (ret == 0) {
bo_gem->idle = !busy.busy;
return busy.busy;
} else {
return false;
}
return (ret == 0 && busy.busy);

Looks like it was a boolean as well until commit 
02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident 
started exposing the bits.


Regards,

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


Re: [Intel-gfx] [PATCH i-g-t] tools/intel_bios_reader: Fix typo on dump info

2016-01-15 Thread Jani Nikula
On Fri, 15 Jan 2016, Mika Kahola  wrote:
> Fix typo on intel_bios_reader.c
>
> Signed-off-by: Mika Kahola 

Pushed, thanks for the patch.

BR,
Jani.

> ---
>  tools/intel_bios_reader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/intel_bios_reader.c b/tools/intel_bios_reader.c
> index b31f648..5ca50ff 100644
> --- a/tools/intel_bios_reader.c
> +++ b/tools/intel_bios_reader.c
> @@ -843,7 +843,7 @@ static void dump_mipi_config(const struct bdb_block 
> *block)
>  
>   printf("\tMIPI PPS\n");
>   printf("\t\tPanel power ON delay: %d\n", pps->panel_on_delay);
> - printf("\t\tPanel power on to Baklight enable delay: %d\n", 
> pps->bl_enable_delay);
> + printf("\t\tPanel power on to Backlight enable delay: %d\n", 
> pps->bl_enable_delay);
>   printf("\t\tBacklight disable to Panel power OFF delay: %d\n", 
> pps->bl_disable_delay);
>   printf("\t\tPanel power OFF delay: %d\n", pps->panel_off_delay);
>   printf("\t\tPanel power cycle delay: %d\n", 
> pps->panel_power_cycle_delay);

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [i-g-t PATCH 0/6] intel_bios_reader: support MIPI sequence block v3

2016-01-15 Thread Jani Nikula
On Thu, 14 Jan 2016, Jani Nikula  wrote:
> Support MIPI sequence block v3 in the intel_bios_reader tool. This is
> mostly copied from the kernel. It makes some of the parts a bit
> artifical for an userspace tool, but hey, this pattern has been followed
> all around in IGT and it makes debugging kernel issues much easier that
> the code is similar.

I don't really expect anyone else to care, so I pushed the patches. Any
objections will be considered volunteering for bios/opregion tools
work. ;)

BR,
Jani.




>
> BR,
> Jani.
>
>
> Jani Nikula (6):
>   intel_bios_reader: pass bdb pointer around instead of having as global
>   intel_bios_reader: fix size handling for 32-bit block size
>   intel_bios_reader: make the VBT pointers more const
>   intel_bios_reader: port find_panel_sequence_block from kernel
>   intel_bios_reader: port the sequence block parsing from kernel
>   intel_bios_reader: dump MIPI sequence block v3
>
>  tools/intel_bios_reader.c | 412 
> +-
>  1 file changed, 296 insertions(+), 116 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] PROBLEM: displayport MST external monitors don't return from sleep mode

2016-01-15 Thread Derek Yerger


Kind Regards,
Derek Yerger
Computer Engineering
10171358 / djy24
[1.] displayport MST external monitors don't return from sleep mode

[2.] When displays go to sleep, upon return from sleep the two monitors 
attached to a belkin mini displayport to 2x HDMI, do not return from 
sleep. This results in part of the desktop being inaccessible, until the 
external displays are unplugged, and then plugged back in.


Sometimes, Xorg will also back down to one monitor. The data 
accompanying this report is for an instance where the power save mode 
caused Xorg to drop to the primary monitor only.


This is on a Dell XPS 13 (9333) using intel i915. There are two external 
monitors connected by HDMI to a DP-MST adapter which connects to the 
single mini-displayport. A single-HDMI-to-mini-displayport adapter works 
as expected (coming out of power save).


The bug can be reproduced by either letting the screen power save 
timeout elapse, or by running "xset dpms force off"


[3.]
[4.] Linux version 4.4.0-999-generic (kernel@tangerine) (gcc version 
5.2.1 20151010 (Ubuntu 5.2.1-22ubuntu2) ) #201601122100 SMP Wed Jan 13 
02:02:49 UTC 2016

[5.]
[6.]
[7.] Description:Ubuntu 15.10
Release:15.10

[7.1.] Linux exa 4.4.0-999-generic #201601122100 SMP Wed Jan 13 02:02:49 
UTC 2016 x86_64 x86_64 x86_64 GNU/Linux


GNU C5.2.1
GNU Make4.0
Binutils2.25.1
Util-linux2.26.2
Mount2.26.2
Module-init-tools21
E2fsprogs1.42.12
Pcmciautils018
PPP2.4.6
Linux C Library2.21
Dynamic linker (ldd)2.21
Linux C++ Library6.0.21
Procps3.3.9
Net-tools1.60
Kbd1.15.5
Console-tools1.15.5
Sh-utils8.23
Udev225
Wireless-tools30
Modules Loaded8250_dw ablk_helper ac97_bus acpi_als aesni_intel 
aes_x86_64 ahci ansi_cprng arc4 autofs4 bluetooth bnep btbcm btintel 
btrtl btusb ccm cfg80211 coretemp crc32_pclmul crct10dif_pclmul cryptd 
ctr dcdbas dell_laptop dell_rbtn dell_smm_hwmon dell_wmi drbg drm 
drm_kms_helper dw_dmac dw_dmac_core elan_i2c fb_sys_fops fjes gf128mul 
glue_helper hid hid_generic hid_multitouch hid_rmi i2c_algo_bit 
i2c_designware_core i2c_designware_platform i2c_hid i915 industrialio 
input_leds intel_powerclamp intel_rapl intel_smartconnect 
ip6table_filter ip6_tables ip6t_REJECT ip6t_rt iptable_filter ip_tables 
ipt_REJECT irqbypass iwlmvm iwlwifi joydev kfifo_buf kvm kvm_intel 
libahci lp lpc_ich lrw mac80211 mac_hid media mei mei_me nf_conntrack 
nf_conntrack_broadcast nf_conntrack_ftp nf_conntrack_ipv4 
nf_conntrack_ipv6 nf_conntrack_netbios_ns nf_defrag_ipv4 nf_defrag_ipv6 
nf_log_common nf_log_ipv4 nf_log_ipv6 nf_nat nf_nat_ftp nf_reject_ipv4 
nf_reject_ipv6 parport parport_pc ppdev psmouse rfcomm sdhci sdhci_acpi 
serio_raw shpchp snd snd_compress snd_hda_codec snd_hda_codec_generic 
snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_core snd_hda_intel 
snd_hwdep snd_pcm snd_pcm_dmaengine snd_rawmidi snd_seq snd_seq_device 
snd_seq_midi snd_seq_midi_event snd_soc_core snd_soc_rl6231 
snd_soc_rt5640 snd_soc_sst_acpi snd_timer soundcore sparse_keymap 
spi_pxa2xx_platform syscopyarea sysfillrect sysimgblt uas usbhid 
usb_storage uvcvideo video videobuf2_core videobuf2_memops 
videobuf2_v4l2 videobuf2_vmalloc videodev vmw_vmci 
vmw_vsock_vmci_transport vsock wmi x86_pkg_temp_thermal x_tables 
xt_addrtype xt_conntrack xt_hl xt_limit xt_LOG xt_tcpudp


[7.2.] processor: 0
vendor_id: GenuineIntel
cpu family: 6
model: 69
model name: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz
stepping: 1
microcode: 0x1c
cpu MHz: 1800.000
cache size: 4096 KB
physical id: 0
siblings: 4
core id: 0
cpu cores: 2
apicid: 0
initial apicid: 0
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall 
nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl 
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor 
ds_cpl vmx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 
epb tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 avx2 
smep bmi2 erms invpcid xsaveopt dtherm ida arat pln pts

bugs:
bogomips: 4788.86
clflush size: 64
cache_alignment: 64
address sizes: 39 bits physical, 48 bits virtual
power management:

processor: 1
vendor_id: GenuineIntel
cpu family: 6
model: 69
model name: Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz
stepping: 1
microcode: 0x1c
cpu MHz: 1905.843
cache size: 4096 KB
physical id: 0
siblings: 4
core id: 0
cpu cores: 2
apicid: 1
initial apicid: 1
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes

[Intel-gfx] [PATCH igt] tests: Add gem_busy

2016-01-15 Thread Chris Wilson
Exercise the busy-ioctl and verify it reports the right active engines
using the execbuffer notation.

Signed-off-by: Chris Wilson 
---
 tests/Makefile.sources |   1 +
 tests/gem_busy.c   | 233 +
 2 files changed, 234 insertions(+)
 create mode 100644 tests/gem_busy.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d2c7d6f..545aca0 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -11,6 +11,7 @@ TESTS_progs_M = \
drv_hangman \
gem_bad_reloc \
gem_basic \
+   gem_busy \
gem_caching \
gem_close_race \
gem_concurrent_blit \
diff --git a/tests/gem_busy.c b/tests/gem_busy.c
new file mode 100644
index 000..c6b8a8b
--- /dev/null
+++ b/tests/gem_busy.c
@@ -0,0 +1,233 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+
+/* Exercise the busy-ioctl, ensuring the ABI is never broken */
+IGT_TEST_DESCRIPTION("Basic check of busy-ioctl ABI.");
+
+enum { TEST = 0, BUSY, BATCH };
+
+static void __gem_busy(int fd,
+  uint32_t handle,
+  uint32_t *read,
+  uint32_t *write)
+{
+   struct drm_i915_gem_busy busy;
+
+   memset(&busy, 0, sizeof(busy));
+   busy.handle = handle;
+
+   do_ioctl(fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
+
+   *write = busy.busy & 0x;
+   *read = busy.busy >> 16;
+}
+
+static uint32_t busy_blt(int fd)
+{
+   const int gen = intel_gen(intel_get_drm_devid(fd));
+   const int has_64bit_reloc = gen >= 8;
+   struct drm_i915_gem_execbuffer2 execbuf;
+   struct drm_i915_gem_exec_object2 object[2];
+   struct drm_i915_gem_relocation_entry reloc[20], *r;
+   uint32_t read, write;
+   uint32_t *map;
+   int factor = 10;
+   int i = 0;
+
+   memset(object, 0, sizeof(object));
+   object[0].handle = gem_create(fd, 1024*1024);
+   object[1].handle = gem_create(fd, 4096);
+
+   r = memset(reloc, 0, sizeof(reloc));
+   map = gem_mmap__cpu(fd, object[1].handle, 0, 4096, PROT_WRITE);
+   gem_set_domain(fd, object[1].handle,
+  I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+#define COPY_BLT_CMD   (2<<29|0x53<<22|0x6)
+#define BLT_WRITE_ALPHA(1<<21)
+#define BLT_WRITE_RGB  (1<<20)
+   while (factor--) {
+   /* XY_SRC_COPY */
+   map[i++] = COPY_BLT_CMD | BLT_WRITE_ALPHA | BLT_WRITE_RGB;
+   if (has_64bit_reloc)
+   map[i-1] += 2;
+   map[i++] = 0xcc << 16 | 1 << 25 | 1 << 24 | (4*1024);
+   map[i++] = 0;
+   map[i++] = 256 << 16 | 1024;
+
+   r->offset = i * sizeof(uint32_t);
+   r->target_handle = object[0].handle;
+   r->read_domains = I915_GEM_DOMAIN_RENDER;
+   r->write_domain = I915_GEM_DOMAIN_RENDER;
+   r++;
+   map[i++] = 0;
+   if (has_64bit_reloc)
+   map[i++] = 0;
+
+   map[i++] = 0;
+   map[i++] = 4096;
+
+   r->offset = i * sizeof(uint32_t);
+   r->target_handle = object[0].handle;
+   r->read_domains = I915_GEM_DOMAIN_RENDER;
+   r->write_domain = 0;
+   r++;
+   map[i++] = 0;
+   if (has_64bit_reloc)
+   map[i++] = 0;
+   }
+   map[i++] = MI_BATCH_BUFFER_END;
+   munmap(map, 4096);
+
+   object[1].relocs_ptr = (uintptr_t)reloc;
+   object[1].relocation_count = r - reloc;
+
+   memset(&execbuf, 0, sizeof(execbuf));
+   execbuf.buffers_ptr = (unsigned long)object;
+   execbuf.buffer_count = 2;
+   if (gen >= 6)
+   execbuf.flags = I915_EXEC_BLT;
+   gem_execbuf(fd, &exec

Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote:
> Looks like your DDX is the only user not using it in the boolean mode?

As far as I am aware, that is the only user that worries about which
engine the object is currently active on.
 
> And libdrm is a bit confused in its return statements:
> 
> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> if (ret == 0) {
> bo_gem->idle = !busy.busy;
> return busy.busy;
> } else {
> return false;
> }
> return (ret == 0 && busy.busy);
> 
> Looks like it was a boolean as well until commit
> 02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident
> started exposing the bits.

Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch
postdates when we started storing read/write bits in the return value.
So definitely an unintentional leakage.
-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] "missed-interrupt" syndrome on Broadwell+

2016-01-15 Thread Chris Wilson
Mika wanted to bump these patches as they should help reduce both real
and false declarations of missed interrupts plaguing Broadwell/Skylake
CI.
-Chris

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


[Intel-gfx] [PATCH 1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+

2016-01-15 Thread Chris Wilson
In order to ensure seqno/irq coherency, we current read a ring register.
We are not sure quite how it works, only that is does. Experiments show
that e.g. doing a clflush(seqno) instead is not sufficient, but we can
remove the forcewake dance from the mmio access.

v2: Baytrail wants a clflush too.

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8cd8aabcc3ff..935add1422ae 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1492,10 +1492,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, bool 
lazy_coherency)
 {
/* Workaround to force correct ordering between irq and seqno writes on
 * ivb (and maybe also on snb) by reading from a CS register (like
-* ACTHD) before reading the status page. */
+* ACTHD) before reading the status page.
+*
+* Note that this effectively effectively stalls the read by the time
+* it takes to do a memory transaction, which more or less ensures
+* that the write from the GPU has sufficient time to invalidate
+* the CPU cacheline. Alternatively we could delay the interrupt from
+* the CS ring to give the write time to land, but that would incur
+* a delay after every batch i.e. much more frequent than a delay
+* when waiting for the interrupt (with the same net latency).
+*/
if (!lazy_coherency) {
struct drm_i915_private *dev_priv = ring->dev->dev_private;
-   POSTING_READ(RING_ACTHD(ring->mmio_base));
+   POSTING_READ_FW(RING_ACTHD(ring->mmio_base));
+
+   intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
}
 
return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
-- 
2.7.0.rc3

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


[Intel-gfx] [PATCH 2/6] drm/i915: Separate out the seqno-barrier from engine->get_seqno

2016-01-15 Thread Chris Wilson
In order to simplify future patches, extract the
lazy_coherency optimisation our of the engine->get_seqno() vfunc into
its own callback.

v2: Rename the barrier to engine->irq_seqno_barrier to try and better
reflect that the barrier is only required after the user interrupt before
reading the seqno (to ensure that the seqno update lands in time as we
do not have strict seqno-irq ordering on all platforms).

Reviewed-by: Dave Gordon  [#v2]

v3: Comments for hangcheck paranoia. Mika wanted to keep the extra
barrier inside the hangcheck, just in case. I can argue that it doesn't
provide a barrier against anything, but the side-effects of applying the
barrier may prevent a false declaration of a hung GPU.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_debugfs.c |  6 +++---
 drivers/gpu/drm/i915/i915_drv.h | 12 +++
 drivers/gpu/drm/i915/i915_gpu_error.c   |  2 +-
 drivers/gpu/drm/i915/i915_irq.c | 14 +++--
 drivers/gpu/drm/i915/i915_trace.h   |  2 +-
 drivers/gpu/drm/i915/intel_lrc.c| 18 ++---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 36 +
 drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++--
 8 files changed, 52 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e3377abc0d4d..b421b53ca128 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -600,7 +600,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void 
*data)
   ring->name,
   
i915_gem_request_get_seqno(work->flip_queued_req),
   dev_priv->next_seqno,
-  ring->get_seqno(ring, true),
+  ring->get_seqno(ring),
   
i915_gem_request_completed(work->flip_queued_req, true));
} else
seq_printf(m, "Flip not associated with any 
ring\n");
@@ -732,7 +732,7 @@ static void i915_ring_seqno_info(struct seq_file *m,
 {
if (ring->get_seqno) {
seq_printf(m, "Current sequence (%s): %x\n",
-  ring->name, ring->get_seqno(ring, false));
+  ring->name, ring->get_seqno(ring));
}
 }
 
@@ -1342,8 +1342,8 @@ static int i915_hangcheck_info(struct seq_file *m, void 
*unused)
intel_runtime_pm_get(dev_priv);
 
for_each_ring(ring, dev_priv, i) {
-   seqno[i] = ring->get_seqno(ring, false);
acthd[i] = intel_ring_get_active_head(ring);
+   seqno[i] = ring->get_seqno(ring);
}
 
i915_get_extra_instdone(dev, instdone);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb7bb97f7316..0cdaed66b786 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2961,15 +2961,19 @@ i915_seqno_passed(uint32_t seq1, uint32_t seq2)
 static inline bool i915_gem_request_started(struct drm_i915_gem_request *req,
   bool lazy_coherency)
 {
-   u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency);
-   return i915_seqno_passed(seqno, req->previous_seqno);
+   if (!lazy_coherency && req->ring->irq_seqno_barrier)
+   req->ring->irq_seqno_barrier(req->ring);
+   return i915_seqno_passed(req->ring->get_seqno(req->ring),
+req->previous_seqno);
 }
 
 static inline bool i915_gem_request_completed(struct drm_i915_gem_request *req,
  bool lazy_coherency)
 {
-   u32 seqno = req->ring->get_seqno(req->ring, lazy_coherency);
-   return i915_seqno_passed(seqno, req->seqno);
+   if (!lazy_coherency && req->ring->irq_seqno_barrier)
+   req->ring->irq_seqno_barrier(req->ring);
+   return i915_seqno_passed(req->ring->get_seqno(req->ring),
+req->seqno);
 }
 
 int __must_check i915_gem_get_seqno(struct drm_device *dev, u32 *seqno);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 06ca4082735b..b978febf67a0 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -902,8 +902,8 @@ static void i915_record_ring_state(struct drm_device *dev,
 
ering->waiting = waitqueue_active(&ring->irq_queue);
ering->instpm = I915_READ(RING_INSTPM(ring->mmio_base));
-   ering->seqno = ring->get_seqno(ring, false);
ering->acthd = intel_ring_get_active_head(ring);
+   ering->seqno = ring->get_seqno(ring);
ering->start = I915_READ_START(ring);
ering->head = I915_READ_HEAD(ring);
ering->tail = I915_READ_TAIL(ring);
diff --git a/drivers/gpu/

[Intel-gfx] [PATCH 5/6] drm/i915: Use simplest form for flushing the single cacheline in the HWS

2016-01-15 Thread Chris Wilson
Rather than call a function to compute the matching cachelines and
clflush them, just call the clflush *instruction* directly. We also know
that we can use the unpatched plain clflush rather than the clflushopt
alternative.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Imre Deak 
---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b22573561669..1603cb1af12f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -389,8 +389,9 @@ intel_ring_sync_index(struct intel_engine_cs *ring,
 static inline void
 intel_flush_status_page(struct intel_engine_cs *ring, int reg)
 {
-   drm_clflush_virt_range(&ring->status_page.page_addr[reg],
-  sizeof(uint32_t));
+   mb();
+   clflush(&ring->status_page.page_addr[reg]);
+   mb();
 }
 
 static inline u32
-- 
2.7.0.rc3

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


[Intel-gfx] [PATCH 6/6] drm/i915: Replace manual barrier() with READ_ONCE() in HWS accessor

2016-01-15 Thread Chris Wilson
When reading from the HWS page, we use barrier() to prevent the compiler
optimising away the read from the volatile (may be updated by the GPU)
memory address. This is more suited to READ_ONCE(); make it so.

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: Mika Kuoppala 
Reviewed-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1603cb1af12f..78d5192ecf9c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -399,8 +399,7 @@ intel_read_status_page(struct intel_engine_cs *ring,
   int reg)
 {
/* Ensure that the compiler doesn't optimize away the load. */
-   barrier();
-   return ring->status_page.page_addr[reg];
+   return READ_ONCE(ring->status_page.page_addr[reg]);
 }
 
 static inline void
-- 
2.7.0.rc3

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


[Intel-gfx] [PATCH 4/6] drm/i915: Harden detection of missed interrupts

2016-01-15 Thread Chris Wilson
Only declare a missed interrupt if we find that the GPU is idle with
waiters and a hangcheck interval has passed in which no new user
interrupts have been raised.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 11 +++
 drivers/gpu/drm/i915/i915_irq.c |  7 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index b421b53ca128..966fc022418c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -730,10 +730,10 @@ static int i915_gem_request_info(struct seq_file *m, void 
*data)
 static void i915_ring_seqno_info(struct seq_file *m,
 struct intel_engine_cs *ring)
 {
-   if (ring->get_seqno) {
-   seq_printf(m, "Current sequence (%s): %x\n",
-  ring->name, ring->get_seqno(ring));
-   }
+   seq_printf(m, "Current sequence (%s): %x\n",
+  ring->name, ring->get_seqno(ring));
+   seq_printf(m, "Current user interrupts (%s): %x\n",
+  ring->name, READ_ONCE(ring->user_interrupts));
 }
 
 static int i915_gem_seqno_info(struct seq_file *m, void *data)
@@ -1361,6 +1361,9 @@ static int i915_hangcheck_info(struct seq_file *m, void 
*unused)
seq_printf(m, "%s:\n", ring->name);
seq_printf(m, "\tseqno = %x [current %x]\n",
   ring->hangcheck.seqno, seqno[i]);
+   seq_printf(m, "\tuser interrupts = %x [current %x]\n",
+  ring->hangcheck.user_interrupts,
+  ring->user_interrupts);
seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
   (long long)ring->hangcheck.acthd,
   (long long)acthd[i]);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 07bc2cdd6252..978eebcf4594 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1000,6 +1000,7 @@ static void notify_ring(struct intel_engine_cs *ring)
return;
 
trace_i915_gem_request_notify(ring);
+   ring->user_interrupts++;
 
wake_up_all(&ring->irq_queue);
 }
@@ -3097,6 +3098,7 @@ static void i915_hangcheck_elapsed(struct work_struct 
*work)
for_each_ring(ring, dev_priv, i) {
u64 acthd;
u32 seqno;
+   unsigned user_interrupts;
bool busy = true;
 
semaphore_clear_deadlocks(dev_priv);
@@ -3113,6 +3115,7 @@ static void i915_hangcheck_elapsed(struct work_struct 
*work)
 
acthd = intel_ring_get_active_head(ring);
seqno = ring->get_seqno(ring);
+   user_interrupts = READ_ONCE(ring->user_interrupts);
 
if (ring->hangcheck.seqno == seqno) {
if (ring_idle(ring, seqno)) {
@@ -3120,7 +3123,8 @@ static void i915_hangcheck_elapsed(struct work_struct 
*work)
 
if (waitqueue_active(&ring->irq_queue)) {
/* Issue a wake-up to catch stuck h/w. 
*/
-   if (!test_and_set_bit(ring->id, 
&dev_priv->gpu_error.missed_irq_rings)) {
+   if (ring->hangcheck.user_interrupts == 
user_interrupts &&
+   !test_and_set_bit(ring->id, 
&dev_priv->gpu_error.missed_irq_rings)) {
if 
(!(dev_priv->gpu_error.test_irq_rings & intel_ring_flag(ring)))
DRM_ERROR("Hangcheck 
timer elapsed... %s idle\n",
  ring->name);
@@ -3187,6 +3191,7 @@ static void i915_hangcheck_elapsed(struct work_struct 
*work)
 
ring->hangcheck.seqno = seqno;
ring->hangcheck.acthd = acthd;
+   ring->hangcheck.user_interrupts = user_interrupts;
busy_count += busy;
}
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 8fb02b21e75d..b22573561669 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -90,6 +90,7 @@ struct intel_ring_hangcheck {
u64 acthd;
u64 max_acthd;
u32 seqno;
+   unsigned user_interrupts;
int score;
enum intel_ring_hangcheck_action action;
int deadlock;
@@ -301,6 +302,7 @@ struct  intel_engine_cs {
 * inspecting request list.
 */
u32 last_submitted_seqno;
+   unsigned user_interrupts;
 
bool gpu_caches_dirty;
 
-- 
2.7.0.rc3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lis

[Intel-gfx] [PATCH 3/6] drm/i915: Broadwell execlists needs exactly the same seqno w/a as legacy

2016-01-15 Thread Chris Wilson
In legacy mode, we use the gen6 seqno barrier to insert a delay after
the interrupt before reading the seqno (as the seqno write is not
flushed before the interrupt is sent, the interrupt arrives before the
seqno is visible). Execlists ignored the evidence of igt.

Note that is harder, but not impossible, to reproduce the missed
interrupt syndrome with execlists. This is primarily because execlists
itself being interrupt driven helps mask the issue.

v2: Rebase and unsquash!

I kept it as gen6_seqno_barrier() for I have a long term plan to merge
the two implementations.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala dev);
+   POSTING_READ_FW(RING_ACTHD(ring->mmio_base));
+
intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
 }
 
@@ -1949,12 +1959,11 @@ logical_ring_default_vfuncs(struct drm_device *dev,
ring->irq_get = gen8_logical_ring_get_irq;
ring->irq_put = gen8_logical_ring_put_irq;
ring->emit_bb_start = gen8_emit_bb_start;
+   ring->irq_seqno_barrier = gen6_seqno_barrier;
ring->get_seqno = gen8_get_seqno;
ring->set_seqno = gen8_set_seqno;
-   if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
-   ring->irq_seqno_barrier = bxt_seqno_barrier;
+   if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
ring->set_seqno = bxt_a_set_seqno;
-   }
 }
 
 static inline void
-- 
2.7.0.rc3

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


Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

2016-01-15 Thread Imre Deak
Adding Chris. We would need stolen_base and size early, could you check
if moving i915_gem_init_stolen() earlier based on the diff at the end
is ok?

On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> setup registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> RC6 related instability can be avoided by disabling via BIOS settings
> till driver fixes it.
> 
> v2: Had placed logic in gen8 function by mistake. Fixed it.
> Ensuring RPM is not enabled in case BIOS disabled RC6.
> 
> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
> Runtime PM enabling happens before gen9_enable_rc6.
> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
> 
> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. 
> (Imre)
> 
> v5: Caching reserved stolen base and size in the driver private data.
> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to 
> intel_uncore_sanitize. (Imre)
> 
> Cc: Imre Deak 
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble 
> ---
>  drivers/gpu/drm/i915/i915_drv.h|  1 +
>  drivers/gpu/drm/i915/i915_gem_gtt.h|  2 ++
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++
>  drivers/gpu/drm/i915/i915_reg.h| 11 +++
>  drivers/gpu/drm/i915/intel_drv.h   |  1 +
>  drivers/gpu/drm/i915/intel_pm.c| 59 
> --
>  drivers/gpu/drm/i915/intel_uncore.c|  4 +++
>  7 files changed, 109 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cf7e0fc..0c8e61c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3223,6 +3223,7 @@ int i915_gem_stolen_insert_node_in_range(struct 
> drm_i915_private *dev_priv,
>    u64 end);
>  void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
>    struct drm_mm_node *node);
> +int i915_gem_init_stolen_reserved(struct drm_device *dev);
>  int i915_gem_init_stolen(struct drm_device *dev);
>  void i915_gem_cleanup_stolen(struct drm_device *dev);
>  struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index b448ad8..20ff6ca 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -343,6 +343,8 @@ struct i915_gtt {
>  
>   size_t stolen_size; /* Total size of stolen memory */
>   size_t stolen_usable_size;  /* Total size minus BIOS reserved */
> + size_t stolen_reserved_base;
> + size_t stolen_reserved_size;
>   u64 mappable_end;   /* End offset that we can CPU map */
>   struct io_mapping *mappable;/* Mapping to our CPU mappable region */
>   phys_addr_t mappable_base;  /* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c 
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 3476877..d5a65d9 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct 
> drm_i915_private *dev_priv,
>   *size = stolen_top - *base;
>  }
>  
> -int i915_gem_init_stolen(struct drm_device *dev)
> +int i915_gem_init_stolen_reserved(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> - unsigned long reserved_total, reserved_base = 0, reserved_size;
> + unsigned long reserved_base = 0, reserved_size;
>   unsigned long stolen_top;
>  
> - mutex_init(&dev_priv->mm.stolen_lock);
> -
>  #ifdef CONFIG_INTEL_IOMMU
>   if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
>   DRM_INFO("DMAR active, disabling use of stolen memory\n");
> @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device *dev)
>   return 0;
>   }
>  
> + dev_priv->gtt.stolen_reserved_base = reserved_base;
> + dev_priv->gtt.stolen_reserved_size = reserved_size;
> +
> + return 0;
> +}
> +
> +int i915_gem_init_stolen(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + unsigned long reserved_total;
> + unsigned long stolen_top;
> +
> + mutex_init(&dev_priv->mm.stolen_lock);
> +
> +#ifdef CONFIG_INTEL_IOMMU
> + if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> + DRM_INFO("DMAR active, disabling use of stolen memory\n");
> + return 0;
> + }
> +#endif
> +
> + if (dev_priv->gtt.stolen_size == 0)
> + return 0;
> +
> + if (dev_priv->mm.stolen_base == 0)
> + return 0;

[Intel-gfx] [PATCH] MAINTAINERS: Add "B:" preferred bug reporting method

2016-01-15 Thread Jani Nikula
Different subsystems and drivers have different preferred ways of
receiving bug reports; mailing list or bugzillas at various
locations. Add "B:" entry for specifying the preference to guide bug
reporters at the right location.

While at it, document the freedesktop.org bugzilla as the preferred
location for drm/i915 bug reports. We have more reassignment of bugs
between the kernel and userspace components than with other kernel
components, and we've thus consolidated our bug tracking there.

Cc: Daniel Vetter 
Cc: Joe Perches 
Cc: Andrew Morton 
Signed-off-by: Jani Nikula 
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 32eda9d0be0c..a803066a9422 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -75,6 +75,8 @@ Descriptions of section entries:
L: Mailing list that is relevant to this area
W: Web-page with status/info
Q: Patchwork web based patch tracking system site
+   B: Preferred method for reporting bugs; bug tracking system site
+  or "List" for mailing list.
T: SCM tree type and location.
   Type is one of: git, hg, quilt, stgit, topgit
S: Status, one of the following:
@@ -3655,6 +3657,7 @@ L:intel-gfx@lists.freedesktop.org
 L: dri-de...@lists.freedesktop.org
 W: https://01.org/linuxgraphics/
 Q: http://patchwork.freedesktop.org/project/intel-gfx/
+B: 
https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel
 T: git git://anongit.freedesktop.org/drm-intel
 S: Supported
 F: drivers/gpu/drm/i915/
-- 
2.1.4

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


Re: [Intel-gfx] [PATCH igt] tests: Add gem_busy

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 13:53, Chris Wilson wrote:

Exercise the busy-ioctl and verify it reports the right active engines
using the execbuffer notation.

Signed-off-by: Chris Wilson 
---
  tests/Makefile.sources |   1 +
  tests/gem_busy.c   | 233 +
  2 files changed, 234 insertions(+)
  create mode 100644 tests/gem_busy.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d2c7d6f..545aca0 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -11,6 +11,7 @@ TESTS_progs_M = \
drv_hangman \
gem_bad_reloc \
gem_basic \
+   gem_busy \
gem_caching \
gem_close_race \
gem_concurrent_blit \
diff --git a/tests/gem_busy.c b/tests/gem_busy.c
new file mode 100644
index 000..c6b8a8b
--- /dev/null
+++ b/tests/gem_busy.c
@@ -0,0 +1,233 @@
+/*
+ * Copyright © 2015 Intel Corporation


2016


+ *
+ * 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 "igt.h"
+
+/* Exercise the busy-ioctl, ensuring the ABI is never broken */
+IGT_TEST_DESCRIPTION("Basic check of busy-ioctl ABI.");
+
+enum { TEST = 0, BUSY, BATCH };
+
+static void __gem_busy(int fd,
+  uint32_t handle,
+  uint32_t *read,
+  uint32_t *write)
+{
+   struct drm_i915_gem_busy busy;
+
+   memset(&busy, 0, sizeof(busy));
+   busy.handle = handle;
+
+   do_ioctl(fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
+
+   *write = busy.busy & 0x;
+   *read = busy.busy >> 16;
+}
+
+static uint32_t busy_blt(int fd)
+{
+   const int gen = intel_gen(intel_get_drm_devid(fd));
+   const int has_64bit_reloc = gen >= 8;
+   struct drm_i915_gem_execbuffer2 execbuf;
+   struct drm_i915_gem_exec_object2 object[2];
+   struct drm_i915_gem_relocation_entry reloc[20], *r;
+   uint32_t read, write;
+   uint32_t *map;
+   int factor = 10;
+   int i = 0;
+
+   memset(object, 0, sizeof(object));
+   object[0].handle = gem_create(fd, 1024*1024);
+   object[1].handle = gem_create(fd, 4096);
+
+   r = memset(reloc, 0, sizeof(reloc));
+   map = gem_mmap__cpu(fd, object[1].handle, 0, 4096, PROT_WRITE);
+   gem_set_domain(fd, object[1].handle,
+  I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+#define COPY_BLT_CMD   (2<<29|0x53<<22|0x6)
+#define BLT_WRITE_ALPHA(1<<21)
+#define BLT_WRITE_RGB  (1<<20)
+   while (factor--) {
+   /* XY_SRC_COPY */
+   map[i++] = COPY_BLT_CMD | BLT_WRITE_ALPHA | BLT_WRITE_RGB;
+   if (has_64bit_reloc)
+   map[i-1] += 2;
+   map[i++] = 0xcc << 16 | 1 << 25 | 1 << 24 | (4*1024);
+   map[i++] = 0;
+   map[i++] = 256 << 16 | 1024;
+
+   r->offset = i * sizeof(uint32_t);
+   r->target_handle = object[0].handle;
+   r->read_domains = I915_GEM_DOMAIN_RENDER;
+   r->write_domain = I915_GEM_DOMAIN_RENDER;
+   r++;
+   map[i++] = 0;
+   if (has_64bit_reloc)
+   map[i++] = 0;
+
+   map[i++] = 0;
+   map[i++] = 4096;
+
+   r->offset = i * sizeof(uint32_t);
+   r->target_handle = object[0].handle;
+   r->read_domains = I915_GEM_DOMAIN_RENDER;
+   r->write_domain = 0;
+   r++;
+   map[i++] = 0;
+   if (has_64bit_reloc)
+   map[i++] = 0;
+   }
+   map[i++] = MI_BATCH_BUFFER_END;
+   munmap(map, 4096);
+
+   object[1].relocs_ptr = (uintptr_t)reloc;
+   object[1].relocation_count = r - reloc;
+
+   memset(&execbuf, 0, sizeof(execbuf));
+   execbuf.buffers_ptr = (unsigned long)object;
+   execbuf.buffer_count = 2;
+   if (gen >= 6)
+   execb

Re: [Intel-gfx] [PATCH v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

2016-01-15 Thread Imre Deak
On pe, 2016-01-15 at 16:44 +0200, Imre Deak wrote:
> Adding Chris. We would need stolen_base and size early, could you
> check
> if moving i915_gem_init_stolen() earlier based on the diff at the end
> is ok?

Oops, I meant stolen reserved_base and reserved_size.

> On to, 2016-01-14 at 21:26 +0530, Sagar Arun Kamble wrote:
> > RC6 setup is shared between BIOS and Driver. BIOS sets up subset of
> > RC6
> > setup registers. If those are not setup Driver should not enable
> > RC6.
> > For implementing this, driver can check RC_CTRL0 and RC_CTRL1
> > values
> > to know if BIOS has enabled HW/SW RC6.
> > This will also enable user to control RC6 using BIOS settings
> > alone.
> > RC6 related instability can be avoided by disabling via BIOS
> > settings
> > till driver fixes it.
> > 
> > v2: Had placed logic in gen8 function by mistake. Fixed it.
> > Ensuring RPM is not enabled in case BIOS disabled RC6.
> > 
> > v3: Need to disable RPM if RC6 is disabled due to BIOS settings.
> > (Daniel)
> > Runtime PM enabling happens before gen9_enable_rc6.
> > Moved the updation of enable_rc6 parameter in
> > intel_uncore_sanitize.
> > 
> > v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx
> > for bxt. (Imre)
> > 
> > v5: Caching reserved stolen base and size in the driver private
> > data.
> > Reorganized RC6 setup check. Moved from gen9_enable_rc6 to
> > intel_uncore_sanitize. (Imre)
> > 
> > Cc: Imre Deak 
> > Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> > Signed-off-by: Sagar Arun Kamble 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h|  1 +
> >  drivers/gpu/drm/i915/i915_gem_gtt.h|  2 ++
> >  drivers/gpu/drm/i915/i915_gem_stolen.c | 39 ++
> >  drivers/gpu/drm/i915/i915_reg.h| 11 +++
> >  drivers/gpu/drm/i915/intel_drv.h   |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c| 59
> > --
> >  drivers/gpu/drm/i915/intel_uncore.c|  4 +++
> >  7 files changed, 109 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index cf7e0fc..0c8e61c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3223,6 +3223,7 @@ int
> > i915_gem_stolen_insert_node_in_range(struct drm_i915_private
> > *dev_priv,
> >      u64 end);
> >  void i915_gem_stolen_remove_node(struct drm_i915_private
> > *dev_priv,
> >      struct drm_mm_node *node);
> > +int i915_gem_init_stolen_reserved(struct drm_device *dev);
> >  int i915_gem_init_stolen(struct drm_device *dev);
> >  void i915_gem_cleanup_stolen(struct drm_device *dev);
> >  struct drm_i915_gem_object *
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > index b448ad8..20ff6ca 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> > @@ -343,6 +343,8 @@ struct i915_gtt {
> >  
> >     size_t stolen_size; /* Total size of stolen
> > memory */
> >     size_t stolen_usable_size;  /* Total size minus BIOS
> > reserved */
> > +   size_t stolen_reserved_base;
> > +   size_t stolen_reserved_size;
> >     u64 mappable_end;   /* End offset that we can
> > CPU map */
> >     struct io_mapping *mappable;/* Mapping to our CPU
> > mappable region */
> >     phys_addr_t mappable_base;  /* PA of our GMADR */
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 3476877..d5a65d9 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -386,14 +386,12 @@ static void bdw_get_stolen_reserved(struct
> > drm_i915_private *dev_priv,
> >     *size = stolen_top - *base;
> >  }
> >  
> > -int i915_gem_init_stolen(struct drm_device *dev)
> > +int i915_gem_init_stolen_reserved(struct drm_device *dev)
> >  {
> >     struct drm_i915_private *dev_priv = dev->dev_private;
> > -   unsigned long reserved_total, reserved_base = 0,
> > reserved_size;
> > +   unsigned long reserved_base = 0, reserved_size;
> >     unsigned long stolen_top;
> >  
> > -   mutex_init(&dev_priv->mm.stolen_lock);
> > -
> >  #ifdef CONFIG_INTEL_IOMMU
> >     if (intel_iommu_gfx_mapped && INTEL_INFO(dev)->gen < 8) {
> >     DRM_INFO("DMAR active, disabling use of stolen
> > memory\n");
> > @@ -458,9 +456,38 @@ int i915_gem_init_stolen(struct drm_device
> > *dev)
> >     return 0;
> >     }
> >  
> > +   dev_priv->gtt.stolen_reserved_base = reserved_base;
> > +   dev_priv->gtt.stolen_reserved_size = reserved_size;
> > +
> > +   return 0;
> > +}
> > +
> > +int i915_gem_init_stolen(struct drm_device *dev)
> > +{
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   unsigned long reserved_total;
> > +   unsigned long stolen_top;
> > +
> > +   mutex_init(&dev_priv->mm.stolen_lock);
> > +
> > +#ifdef CONFIG_INTEL_I

Re: [Intel-gfx] [PATCH 10/11] acpi: Export acpi_bus_type

2016-01-15 Thread Rafael J. Wysocki
On Thursday, January 14, 2016 11:46:46 AM ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma 
> 
> Some modules, like i915.ko, needs to detect when certain ACPI features
> are active inorder to prevent corruption on contended resources.
> In particular, use of BIOS RapidStart Technology may corrupt the contents
> of the reserved graphics memory, due to unalarmed hibernation. In which
> case i915.ko cannot assume that it (reserved gfx memory) remains
> unmodified and must recreate teh contents and importantly not use it to
> store unrecoverable user data.
> 
> Signed-off-by: Ankitprasad Sharma 
> Cc: "Rafael J. Wysocki" 
> Cc: Len Brown 
> Cc: linux-a...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/acpi/bus.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cef..69509c7 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -814,6 +814,7 @@ struct bus_type acpi_bus_type = {
>   .remove = acpi_device_remove,
>   .uevent = acpi_device_uevent,
>  };
> +EXPORT_SYMBOL_GPL(acpi_bus_type);
>  
>  /* --
>   Initialization/Cleanup
> 

No.

I see no reason whatsoever for doing this.

Thanks,
Rafael

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


[Intel-gfx] [PATCH 3/3] drm/i915: Cache LRC state page in the context

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

LRC lifetime is well defined so we can cache the page pointing
to the object backing store in the context in order to avoid
walking over the object SG page list from the interrupt context
without the big lock held.

v2: Also cache the mapping. (Chris Wilson)
v3: Unmap on the error path.
v4: No need to cache the page. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_lrc.c | 53 ++--
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index acff98b9c148..af301482e6f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -890,6 +890,7 @@ struct intel_context {
int pin_count;
struct i915_vma *lrc_vma;
u64 lrc_desc;
+   uint32_t *lrc_reg_state;
} engine[I915_NUM_RINGS];
 
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 545173c1b5c5..70bd28cc8887 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -390,14 +390,7 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
 {
struct intel_engine_cs *ring = rq->ring;
struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
-   struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-   struct page *page;
-   uint32_t *reg_state;
-
-   BUG_ON(!ctx_obj);
-
-   page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
-   reg_state = kmap_atomic(page);
+   uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state;
 
reg_state[CTX_RING_TAIL+1] = rq->tail;
reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start;
@@ -414,8 +407,6 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
}
 
-   kunmap_atomic(reg_state);
-
return 0;
 }
 
@@ -1067,6 +1058,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs 
*ring,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+   struct page *lrc_state_page;
+   uint32_t *lrc_reg_state;
int ret;
 
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
@@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct 
intel_engine_cs *ring,
if (ret)
return ret;
 
+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
+   if (WARN_ON(!lrc_state_page)) {
+   ret = -ENODEV;
+   goto unpin_ctx_obj;
+   }
+
+   lrc_reg_state = kmap(lrc_state_page);
+   if (!lrc_reg_state) {
+   ret = -ENOMEM;
+   goto unpin_ctx_obj;
+   }
+
ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
if (ret)
-   goto unpin_ctx_obj;
+   goto unmap_state_page;
 
ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, ring);
+   ctx->engine[ring->id].lrc_reg_state = lrc_reg_state;
ctx_obj->dirty = true;
 
/* Invalidate GuC TLB. */
@@ -1090,6 +1096,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs 
*ring,
 
return ret;
 
+unmap_state_page:
+   kunmap(lrc_state_page);
 unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
 
@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request 
*rq)
struct intel_engine_cs *ring = rq->ring;
struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = rq->ringbuf;
+   struct page *lrc_state_page;
 
-   if (ctx_obj) {
-   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-   if (--rq->ctx->engine[ring->id].pin_count == 0) {
-   intel_unpin_ringbuffer_obj(ringbuf);
-   i915_gem_object_ggtt_unpin(ctx_obj);
-   rq->ctx->engine[ring->id].lrc_vma = NULL;
-   rq->ctx->engine[ring->id].lrc_desc = 0;
-   }
+   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+
+   if (!ctx_obj)
+   return;
+
+   if (--rq->ctx->engine[ring->id].pin_count == 0) {
+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj,
+   LRC_STATE_PN);
+   kunmap(lrc_state_page);
+   intel_unpin_ringbuffer_obj(ringbuf);
+   i915_gem_object_ggtt_unpin(ctx_obj);
+   rq->ctx->engine[ring->id].lrc_vma = NULL;
+   rq->ctx->

[Intel-gfx] [PATCH] drm/i915: Only grab timestamps when needed

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

No need to call ktime_get_raw_ns twice per unlimited wait and can
also elimate a local variable.

v2: Added comment about silencing the compiler warning. (Daniel Vetter)

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Dave Gordon 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ddc21d4b388d..6b0102da859c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1251,7 +1251,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
DEFINE_WAIT(wait);
unsigned long timeout_expire;
-   s64 before, now;
+   s64 before = 0; /* Only to silence a compiler warning. */
int ret;
 
WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
@@ -1271,14 +1271,17 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
return -ETIME;
 
timeout_expire = jiffies + nsecs_to_jiffies_timeout(*timeout);
+
+   /*
+* Record current time in case interrupted by signal, or wedged.
+*/
+   before = ktime_get_raw_ns();
}
 
if (INTEL_INFO(dev_priv)->gen >= 6)
gen6_rps_boost(dev_priv, rps, req->emitted_jiffies);
 
-   /* Record current time in case interrupted by signal, or wedged */
trace_i915_gem_request_wait_begin(req);
-   before = ktime_get_raw_ns();
 
/* Optimistic spin for the next jiffie before touching IRQs */
ret = __i915_spin_request(req, state);
@@ -1343,11 +1346,10 @@ int __i915_wait_request(struct drm_i915_gem_request 
*req,
finish_wait(&ring->irq_queue, &wait);
 
 out:
-   now = ktime_get_raw_ns();
trace_i915_gem_request_wait_end(req);
 
if (timeout) {
-   s64 tres = *timeout - (now - before);
+   s64 tres = *timeout - (ktime_get_raw_ns() - before);
 
*timeout = tres < 0 ? 0 : tres;
 
-- 
1.9.1

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


[Intel-gfx] [PATCH 2/3] drm/i915: Cache ringbuffer GTT VMA

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

Purpose is to avoid calling i915_gem_obj_ggtt_offset from the
interrupt context without the big lock held.

v2: Renamed gtt_start to gtt_offset. (Daniel Vetter)
v3: Cache the VMA instead of address. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/intel_lrc.c| 3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1a0d85325072..545173c1b5c5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -391,7 +391,6 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
struct intel_engine_cs *ring = rq->ring;
struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-   struct drm_i915_gem_object *rb_obj = rq->ringbuf->obj;
struct page *page;
uint32_t *reg_state;
 
@@ -401,7 +400,7 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
reg_state = kmap_atomic(page);
 
reg_state[CTX_RING_TAIL+1] = rq->tail;
-   reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(rb_obj);
+   reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start;
 
if (ppgtt && !USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
/* True 32b PPGTT with dynamic page allocation: update PDP
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4060acf0601a..4150a240d47d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1997,6 +1997,7 @@ void intel_unpin_ringbuffer_obj(struct intel_ringbuffer 
*ringbuf)
else
iounmap(ringbuf->virtual_start);
ringbuf->virtual_start = NULL;
+   ringbuf->vma = NULL;
i915_gem_object_ggtt_unpin(ringbuf->obj);
 }
 
@@ -2063,6 +2064,8 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device 
*dev,
}
}
 
+   ringbuf->vma = i915_gem_obj_to_ggtt(obj);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 85ce2272f92c..ede57954080e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -99,6 +99,7 @@ struct intel_ring_hangcheck {
 struct intel_ringbuffer {
struct drm_i915_gem_object *obj;
void __iomem *virtual_start;
+   struct i915_vma *vma;
 
struct intel_engine_cs *ring;
struct list_head link;
-- 
1.9.1

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


[Intel-gfx] [PATCH 1/3] drm/i915: Do not call API requiring struct_mutex where it is not available

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

LRC code was calling GEM API like i915_gem_obj_ggtt_offset from
places where the struct_mutex cannot be grabbed (irq handlers).

To avoid that this patch caches some interesting bits and values
in the engine and context structures.

Some usages are also removed where they are not needed like a
few asserts which are either impossible or have been checked
already during engine initialization.

Side benefit is also that interrupt handlers and command
submission stop evaluating invariant conditionals, like what
Gen we are running on, on every interrupt and every command
submitted.

This patch deals with logical ring context id and descriptors
while subsequent patches will deal with the remaining issues.

v2:
 * Cache the VMA instead of the address. (Chris Wilson)
 * Incorporate Dave Gordon's good comments and function name.

v3:
 * Extract ctx descriptor template to a function and group
   functions dealing with ctx descriptor & co together near
   top of the file. (Dave Gordon)

Signed-off-by: Tvrtko Ursulin 
Reviewed-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_debugfs.c |  15 ++--
 drivers/gpu/drm/i915/i915_drv.h |   2 +
 drivers/gpu/drm/i915/i915_gem_gtt.h |   1 -
 drivers/gpu/drm/i915/intel_lrc.c| 151 +++-
 drivers/gpu/drm/i915/intel_lrc.h|   4 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h |   2 +
 6 files changed, 103 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e3377abc0d4d..0b3550f05026 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1994,12 +1994,13 @@ static int i915_context_status(struct seq_file *m, void 
*unused)
 }
 
 static void i915_dump_lrc_obj(struct seq_file *m,
- struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj)
+ struct intel_context *ctx,
+ struct intel_engine_cs *ring)
 {
struct page *page;
uint32_t *reg_state;
int j;
+   struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
unsigned long ggtt_offset = 0;
 
if (ctx_obj == NULL) {
@@ -2009,7 +2010,7 @@ static void i915_dump_lrc_obj(struct seq_file *m,
}
 
seq_printf(m, "CONTEXT: %s %u\n", ring->name,
-  intel_execlists_ctx_id(ctx_obj));
+  intel_execlists_ctx_id(ctx, ring));
 
if (!i915_gem_obj_ggtt_bound(ctx_obj))
seq_puts(m, "\tNot bound in GGTT\n");
@@ -2058,8 +2059,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
list_for_each_entry(ctx, &dev_priv->context_list, link) {
for_each_ring(ring, dev_priv, i) {
if (ring->default_context != ctx)
-   i915_dump_lrc_obj(m, ring,
- ctx->engine[i].state);
+   i915_dump_lrc_obj(m, ctx, ring);
}
}
 
@@ -2133,11 +2133,8 @@ static int i915_execlists(struct seq_file *m, void *data)
 
seq_printf(m, "\t%d requests in queue\n", count);
if (head_req) {
-   struct drm_i915_gem_object *ctx_obj;
-
-   ctx_obj = head_req->ctx->engine[ring_id].state;
seq_printf(m, "\tHead request id: %u\n",
-  intel_execlists_ctx_id(ctx_obj));
+  intel_execlists_ctx_id(head_req->ctx, ring));
seq_printf(m, "\tHead request tail: %u\n",
   head_req->tail);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb7bb97f7316..acff98b9c148 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -888,6 +888,8 @@ struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
int pin_count;
+   struct i915_vma *lrc_vma;
+   u64 lrc_desc;
} engine[I915_NUM_RINGS];
 
struct list_head link;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad832dcf..e5737963ab79 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -44,7 +44,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;
 
 #define gtt_total_entries(gtt) ((gtt).base.total >> PAGE_SHIFT)
 
-
 /* gen6-hsw has bit 11-4 for physical addr bit 39-32 */
 #define GEN6_GTT_ADDR_ENCODE(addr) ((addr) | (((addr) >> 28) & 0xff0))
 #define GEN6_PTE_ADDR_ENCODE(addr) GEN6_GTT_ADDR_ENCODE(addr)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5027699c5291..1a0d85325072 100644
--- a/drivers

[Intel-gfx] [PATCH v2] drm/i915: Decouple execbuf uAPI from internal implementation

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

At the moment execbuf ring selection is fully coupled to
internal ring ids which is not a good thing on its own.

This dependency is also spread between two source files and
not spelled out at either side which makes it hidden and
fragile.

This patch decouples this dependency by introducing an explicit
translation table of execbuf uAPI to ring id close to the only
call site (i915_gem_do_execbuffer).

This way we are free to change driver internal implementation
details without breaking userspace. All state relating to the
uAPI is now contained in, or next to, i915_gem_do_execbuffer.

As a side benefit, this patch decreases the compiled size
of i915_gem_do_execbuffer.

v2: Extract ring selection into eb_select_ring. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin 
Cc: Daniel Vetter 
Cc: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.h|   4 +-
 drivers/gpu/drm/i915/i915_gem.c|   2 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 139 +++--
 drivers/gpu/drm/i915/intel_ringbuffer.h|  10 +--
 4 files changed, 81 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb7bb97f7316..35d5d6099a44 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -334,7 +334,7 @@ struct drm_i915_file_private {
unsigned boosts;
} rps;
 
-   struct intel_engine_cs *bsd_ring;
+   unsigned int bsd_ring;
 };
 
 enum intel_dpll_id {
@@ -1300,7 +1300,7 @@ struct i915_gem_mm {
bool busy;
 
/* the indicator for dispatch video commands on two BSD rings */
-   int bsd_ring_dispatch_index;
+   unsigned int bsd_ring_dispatch_index;
 
/** Bit 6 swizzling required for X tiling */
uint32_t bit_6_swizzle_x;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ddc21d4b388d..26e6842f2df3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5112,6 +5112,8 @@ int i915_gem_open(struct drm_device *dev, struct drm_file 
*file)
spin_lock_init(&file_priv->mm.lock);
INIT_LIST_HEAD(&file_priv->mm.request_list);
 
+   file_priv->bsd_ring = -1;
+
ret = i915_gem_context_open(dev, file);
if (ret)
kfree(file_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d469c4779ff5..497a2f87836c 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1328,33 +1328,23 @@ i915_gem_ringbuffer_submission(struct 
i915_execbuffer_params *params,
 
 /**
  * Find one BSD ring to dispatch the corresponding BSD command.
- * The Ring ID is returned.
+ * The ring index is returned.
  */
-static int gen8_dispatch_bsd_ring(struct drm_device *dev,
- struct drm_file *file)
+static unsigned int
+gen8_dispatch_bsd_ring(struct drm_i915_private *dev_priv, struct drm_file 
*file)
 {
-   struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_file_private *file_priv = file->driver_priv;
 
-   /* Check whether the file_priv is using one ring */
-   if (file_priv->bsd_ring)
-   return file_priv->bsd_ring->id;
-   else {
-   /* If no, use the ping-pong mechanism to select one ring */
-   int ring_id;
-
-   mutex_lock(&dev->struct_mutex);
-   if (dev_priv->mm.bsd_ring_dispatch_index == 0) {
-   ring_id = VCS;
-   dev_priv->mm.bsd_ring_dispatch_index = 1;
-   } else {
-   ring_id = VCS2;
-   dev_priv->mm.bsd_ring_dispatch_index = 0;
-   }
-   file_priv->bsd_ring = &dev_priv->ring[ring_id];
-   mutex_unlock(&dev->struct_mutex);
-   return ring_id;
+   /* Check whether the file_priv has already selected one ring. */
+   if ((int)file_priv->bsd_ring < 0) {
+   /* If not, use the ping-pong mechanism to select one. */
+   mutex_lock(&dev_priv->dev->struct_mutex);
+   file_priv->bsd_ring = dev_priv->mm.bsd_ring_dispatch_index;
+   dev_priv->mm.bsd_ring_dispatch_index ^= 1;
+   mutex_unlock(&dev_priv->dev->struct_mutex);
}
+
+   return file_priv->bsd_ring;
 }
 
 static struct drm_i915_gem_object *
@@ -1377,6 +1367,63 @@ eb_get_batch(struct eb_vmas *eb)
return vma->obj;
 }
 
+#define I915_USER_RINGS (4)
+
+static const enum intel_ring_id user_ring_map[I915_USER_RINGS + 1] = {
+   [I915_EXEC_DEFAULT] = RCS,
+   [I915_EXEC_RENDER]  = RCS,
+   [I915_EXEC_BLT] = BCS,
+   [I915_EXEC_BSD] = VCS,
+   [I915_EXEC_VEBOX]   = VECS
+};
+
+static int
+eb_select_ring(struct drm_i915_private *dev_priv,
+  struct drm_file *file,
+  st

Re: [Intel-gfx] [PATCH 01/11] drm/i915: Add support for mapping an object page by page

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 11:44, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 09:53:46AM +, Tvrtko Ursulin wrote:


On 14/01/16 13:34, Chris Wilson wrote:

On Thu, Jan 14, 2016 at 10:32:11AM +, Tvrtko Ursulin wrote:

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b448ad8..5f86596 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -317,6 +317,11 @@ struct i915_address_space {
uint64_t start,
uint64_t length,
bool use_scratch);
+   void (*insert_page)(struct i915_address_space *vm,
+   dma_addr_t addr,
+   uint64_t offset,
+   enum i915_cache_level cache_level,
+   u32 flags);
void (*insert_entries)(struct i915_address_space *vm,
   struct sg_table *st,
   uint64_t start,


Why not extend the current API to support start page offset and
number of pages? Could default to full object like today if zero.
Eg:

  void (*insert_entries)(struct i915_address_space *vm,
struct sg_table *st,
+   unsigned page_offset,
+   unsigned num_pages,


Ouch. That would be quite slow for the insert_page() use case of
page-by-page looping.


It could use the same last page caching trick so why would be be so slow?


We don't have the convenient struct? I presumed you were thinking of
passing the offset through to sg_page_iter

for_each_sg_page(st->sgl, &sg_iter, st->nents, page_offset) {
if (--num_pages)
break;

/* original pt insertion code */
}

Is it worth expanding struct sg_table for this shortcut?

There are some games where we spend more time in sg_page_iter_next() than
anything else in the kernel. The games have more textures than GTT
space, they aren't playable right now nor will they ever likely to be. :(
Improving these loops would definitely be a boon nevertheless.


Okay, dead end, mission abort. :)

Ankit please just sync the patch in next respin with the RPM changes 
which happened in the meantime.


Regards,

Tvrtko


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


[Intel-gfx] ✗ Fi.CI.BAT: warning for series starting with [1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (bdw-nuci7) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass   -> DMESG-WARN (bdw-ultra)
Subgroup suspend-read-crc-pipe-a:
pass   -> SKIP   (hsw-gt2)

bdw-nuci7total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:136  dwarn:0   dfail:0   fail:0   skip:5  
hsw-xps12total:138  pass:133  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1197/

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


Re: [Intel-gfx] [PATCH igt] tests: Add gem_busy

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 02:45:49PM +, Tvrtko Ursulin wrote:
> 
> On 15/01/16 13:53, Chris Wilson wrote:
> >Exercise the busy-ioctl and verify it reports the right active engines
> >using the execbuffer notation.
> >
> >Signed-off-by: Chris Wilson 
> >---
> >  tests/Makefile.sources |   1 +
> >  tests/gem_busy.c   | 233 
> > +
> >  2 files changed, 234 insertions(+)
> >  create mode 100644 tests/gem_busy.c
> >
> >diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >index d2c7d6f..545aca0 100644
> >--- a/tests/Makefile.sources
> >+++ b/tests/Makefile.sources
> >@@ -11,6 +11,7 @@ TESTS_progs_M = \
> > drv_hangman \
> > gem_bad_reloc \
> > gem_basic \
> >+gem_busy \
> > gem_caching \
> > gem_close_race \
> > gem_concurrent_blit \
> >diff --git a/tests/gem_busy.c b/tests/gem_busy.c
> >new file mode 100644
> >index 000..c6b8a8b
> >--- /dev/null
> >+++ b/tests/gem_busy.c
> >@@ -0,0 +1,233 @@
> >+/*
> >+ * Copyright © 2015 Intel Corporation
> 
> 2016

Already?

> >+__gem_busy(fd, object[0].handle, &read, &write);
> >+igt_assert_eq(read, 1 << write);
> >+igt_assert_eq(write, gen >= 6 ? I915_EXEC_BLT : I915_EXEC_RENDER);
> 
> Will the blits be long enough for this to be stable?

I can bump it by a factor of 5 easily enough, that should put it in the
range of a few milliseconds even on a fast system. If we get rescheduled,
we might miss the window for the test.

I did think of asserting that the blit was still busy eash time.

> >+/* Queue a batch after the busy, it should block and remain "busy" */
> >+igt_require(exec_noop(fd, handle, ring | flags, false));
> 
> Too bad we don't get ENODEV but a boring old EINVAL on !HAS_BSD2 -
> this was this can be too skip happy if something else gets broken.
> :(

True, but it would be better to have a test that checked which rings the
kernel said were on the hardware and that execbuffer supported them.

> >+igt_main
> >+{
> >+int fd = -1;
> >+
> >+igt_skip_on_simulation();
> >+
> >+igt_fixture {
> >+fd = drm_open_driver(DRIVER_INTEL);
> >+igt_require(has_semaphores(fd));
> 
> Copy&paste, or?

A requirement. At least on the current infrastructure. It uses the sync
with the write on the BLT ring to block the GPU on the other rings, so
that the nops don't complete too early. We would need a busy workload on
each engine - code that we don't have yet.

Without semaphores, and without deferred scheduling, we current wait for
the busy workload to complete before scheduling on the next...

If I lie about the busywork being a write target, we will get parallel
execution rather than blocking and so not be able to set the right
flags. I couldn't see a way around semaphores without gen specific new
code.

> Maybe mark as basic since it is ABI?

Can't until the kernel abides by the abi we just concocted :) I
definitely don't trust my judgement as to what is basic. Surviving weeks
of stress testing is a basic requirement for us :)
-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 v5 1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 04:44:46PM +0200, Imre Deak wrote:
> Adding Chris. We would need stolen_base and size early, could you check
> if moving i915_gem_init_stolen() earlier based on the diff at the end
> is ok?

Had a quick discussion with Imre on irc, and yes this is fine. Though we
could move gem_init_stolen even earlier, into i915_gem_gtt_init() or
just after. I favour gtt_init since the stolen arena is closely coupled
with the gtt probing in i915_gem_gtt_init().

Also intel_setup_mchbar() is semantically related to our
dev_priv->regs = pci_iomap(), and I would suggest a new
intel_setup_mmio() to do both, nice and early.
-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 i-g-t] tests/kms_fbc_crc.c: No longer dependant on Cairo A setup function that used to use Cairo to draw 2 rectangles covering the whole screen has been changed to use igt_draw

2016-01-15 Thread devon . davies
From: Devon Davies 

tests/kms_frontbuffer_tracking.c: Now builds with DRM_PRIMARY_DISABLE
Each call to the function drmModeSetPlane now has an addtional NULL in the
arguments if DRM_PRIMARY_DISABLE is set.

tests/Android.mk: Allow the above tests to be built without Cairo
Simply removed them from the tests the be skipped.

libs/igt_kms.c: Now builds with DRM_PRIMARY_DISABLE
I had to define ffs as __builtin_fss due to compiler issues.
Each call to the function drmModeSetPlane now has an addtional NULL in the
arguments if DRM_PRIMARY_DISABLE is set.

libs/igt_fb.c: Will now build some functions without Cairo
Functions which aren't used by the framebuffer compression tests are
now behind an #if (!defined(ANDROID)) || (defined(ANDROID) &&
ANDROID_HAS_CAIRO

libs/Android.mk
igt_fb and igt_kms are no longer ignored if we don't have Cairo.

The tests kms_fbc_crc and kms_frontbuffer_tracking had an unnecessary
dependance on the Cairo graphics engine.
Also, drmModeSetPlane may have an additional argument if DRM_PRIMARY_DISABLE
is set (as it was for me), I have fixed that issue.

Signed-off-by: Devon Davies 
---
 lib/Android.mk   |  4 --
 lib/igt_fb.c | 26 -
 lib/igt_kms.c| 15 ++--
 tests/Android.mk |  5 +++
 tests/kms_fbc_crc.c  | 20 ++
 tests/kms_frontbuffer_tracking.c | 79 +---
 6 files changed, 119 insertions(+), 30 deletions(-)

diff --git a/lib/Android.mk b/lib/Android.mk
index badec1e..bbdb051 100644
--- a/lib/Android.mk
+++ b/lib/Android.mk
@@ -37,10 +37,6 @@ ifeq ("${ANDROID_HAS_CAIRO}", "1")
 LOCAL_C_INCLUDES += $(ANDROID_BUILD_TOP)/external/cairo-1.12.16/src
 LOCAL_CFLAGS += -DANDROID_HAS_CAIRO=1 -DIGT_DATADIR=\".\" 
-DIGT_SRCDIR=\".\"
 else
-skip_lib_list := \
-igt_kms.c \
-igt_kms.h \
-igt_fb.c
 -DANDROID_HAS_CAIRO=0
 endif
 
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index c985824..5acdaa7 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -33,6 +33,7 @@
 #include "igt_fb.h"
 #include "ioctl_wrappers.h"
 
+
 /**
  * SECTION:igt_fb
  * @short_description: Framebuffer handling and drawing library
@@ -52,11 +53,23 @@
  */
 
 /* drm fourcc/cairo format maps */
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
+
 #define DF(did, cid, _bpp, _depth) \
{ DRM_FORMAT_##did, CAIRO_FORMAT_##cid, # did, _bpp, _depth }
+
+#else
+
+#define DF(did, cid, _bpp, _depth) \
+   { DRM_FORMAT_##did, # did, _bpp, _depth }
+
+#endif
+
 static struct format_desc_struct {
uint32_t drm_id;
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
cairo_format_t cairo_id;
+#endif
const char *name;
int bpp;
int depth;
@@ -72,7 +85,6 @@ static struct format_desc_struct {
 #define for_each_format(f) \
for (f = format_desc; f - format_desc < ARRAY_SIZE(format_desc); f++)
 
-
 /* helpers to create nice-looking framebuffers */
 static int create_bo_for_fb(int fd, int width, int height, int bpp,
uint64_t tiling, unsigned bo_size,
@@ -125,6 +137,8 @@ static int create_bo_for_fb(int fd, int width, int height, 
int bpp,
return ret;
 }
 
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
+
 /**
  * igt_paint_color:
  * @cr: cairo drawing context
@@ -394,6 +408,7 @@ void igt_paint_image(cairo_t *cr, const char *filename,
 
fclose(f);
 }
+#endif
 
 /**
  * igt_create_fb_with_bo_size:
@@ -494,6 +509,7 @@ unsigned int igt_create_fb(int fd, int width, int height, 
uint32_t format,
return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb,
  0, 0);
 }
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
 
 /**
  * igt_create_color_fb:
@@ -985,6 +1001,7 @@ void igt_write_fb_to_png(int fd, struct igt_fb *fb, const 
char *filename)
 
igt_assert(status == CAIRO_STATUS_SUCCESS);
 }
+#endif
 
 /**
  * igt_remove_fb:
@@ -997,10 +1014,13 @@ void igt_write_fb_to_png(int fd, struct igt_fb *fb, 
const char *filename)
  */
 void igt_remove_fb(int fd, struct igt_fb *fb)
 {
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
cairo_surface_destroy(fb->cairo_surface);
+#endif
do_or_die(drmModeRmFB(fd, fb->fb_id));
gem_close(fd, fb->gem_handle);
 }
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
 
 /**
  * igt_bpp_depth_to_drm_format:
@@ -1024,6 +1044,8 @@ uint32_t igt_bpp_depth_to_drm_format(int bpp, int depth)
 depth);
 }
 
+#endif
+
 /**
  * igt_drm_format_to_bpp:
  * @drm_format: drm fourcc pixel format code
@@ -1062,6 +1084,7 @@ const char *igt_format_str(uint32_t drm_format)
 
return "invalid";
 }
+#if (!defined(ANDROID)) || (defined(ANDROID) && ANDROID_HAS_CAIRO)
 
 /**
  * igt_get_all_formats:
@@ -1089,3 +1112,4 @@ void igt_get_

[Intel-gfx] ✗ Fi.CI.BAT: failure for MAINTAINERS: Add "B:" preferred bug reporting method

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_ctx_basic:
pass   -> FAIL   (bdw-ultra)
Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i7k-2) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
pass   -> SKIP   (hsw-gt2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-c:
pass   -> DMESG-WARN (bdw-ultra)

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:130  dwarn:1   dfail:0   fail:1   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:136  dwarn:0   dfail:0   fail:0   skip:5  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1198/

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


Re: [Intel-gfx] [PATCH i-g-t] tools/Android.mk: Add zlib support

2016-01-15 Thread Morton, Derek J
Can this be merged so IGT on Android builds? No one has raise any objections 
since Monday about this patch.

//Derek

-Original Message-
From: Morton, Derek J 
Sent: Monday, January 11, 2016 1:32 PM
To: intel-gfx@lists.freedesktop.org
Cc: Wood, Thomas; Gore, Tim; ch...@chris-wilson.co.uk; Morton, Derek J
Subject: [PATCH] tools/Android.mk: Add zlib support

IGT does not build for Android due to a zlib dependency being added to 
intel_error_decode.c in a recent patch.
This patch fixes the error by updating the Android makefile to add the path to 
the zlib library and using any LDFLAGS specified in Makefile.sources.

Signed-off-by: Derek Morton 
---
 tools/Android.mk | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/Android.mk b/tools/Android.mk index 934d3a1..da4f3c0 100644
--- a/tools/Android.mk
+++ b/tools/Android.mk
@@ -23,9 +23,14 @@ define add_tool
 LOCAL_CFLAGS += -Wno-error=return-type
 # Excessive complaining for established cases. Rely on the Linux version 
warnings.
 LOCAL_CFLAGS += -Wno-sign-compare
+ifeq ($($(1)_LDFLAGS),)
+else
+LOCAL_LDFLAGS += $($(1)_LDFLAGS)
+endif
 
 LOCAL_C_INCLUDES = $(LOCAL_PATH)/../lib
 LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/PRIVATE/drm/include/drm
+LOCAL_C_INCLUDES += ${ANDROID_BUILD_TOP}/external/zlib
 
 LOCAL_MODULE := $1_tool
 LOCAL_MODULE_TAGS := optional
--
1.9.1

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


Re: [Intel-gfx] [PATCH v2] drm/i915: Decouple execbuf uAPI from internal implementation

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 03:12:50PM +, Tvrtko Ursulin wrote:
> +static int
> +eb_select_ring(struct drm_i915_private *dev_priv,
> +struct drm_file *file,
> +struct drm_i915_gem_execbuffer2 *args,
> +struct intel_engine_cs **ring)
> +{
> + unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK;
> +
> + if (user_ring_id > I915_USER_RINGS) {
> + DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id);
> + return -EINVAL;
> + }
> +
> + if ((user_ring_id != I915_EXEC_BSD) &&
> + ((args->flags & I915_EXEC_BSD_MASK) != 0)) {
> + DRM_DEBUG("execbuf with non bsd ring but with invalid "
> +   "bsd dispatch flags: %d\n", (int)(args->flags));

Not your bug, but we should limit the flags reported here to
(int)(args->flags & I915_EXEC_BSD_MASK).

Though actually just nuke the test. At the moment, we complain for !BSD,
then allow them even if we don't have BSD2 (and ignore the setting).
A little inconsistent.

If we just document that these flags only provide extra selection
criteria for the I915_EXEC_BSD ring, we would be done. I'll pretend that
it is adequately documented...

Looks good and we completed our review of ABI impact for reordering
the ring ids, so

Reviewed-by: Chris Wilson 
-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] tools/Android.mk: Add zlib support

2016-01-15 Thread Damien Lespiau
On Fri, Jan 15, 2016 at 03:49:00PM +, Morton, Derek J wrote:
> Can this be merged so IGT on Android builds? No one has raise any
> objections since Monday about this patch.

Merged, thanks for the patch.

-- 
Damien
___
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: Cache LRC state page in the context

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote:
> @@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct 
> drm_i915_gem_request *rq)
> + if (--rq->ctx->engine[ring->id].pin_count == 0) {
> + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj,
> + LRC_STATE_PN);

Interesting choice. We called set_page_dirty() when we took the mapping.
Should that page flag be preserved whilst we hold the kmap - I think so,
i.e. the mm cannot flush the page whilst it has an elevated mapcount. So
calling set_page_dirty() again is redundant, right?
-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] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 13:57, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote:

Looks like your DDX is the only user not using it in the boolean mode?


As far as I am aware, that is the only user that worries about which
engine the object is currently active on.


And libdrm is a bit confused in its return statements:

 ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
 if (ret == 0) {
 bo_gem->idle = !busy.busy;
 return busy.busy;
 } else {
 return false;
 }
 return (ret == 0 && busy.busy);

Looks like it was a boolean as well until commit
02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident
started exposing the bits.


Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch
postdates when we started storing read/write bits in the return value.
So definitely an unintentional leakage.


In that case I think just respin with comment corrections in uapi header 
for drm_i915_gem_busy?


Regards,

Tvrtko
___
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: Cache LRC state page in the context

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 16:01, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote:

@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request 
*rq)
+   if (--rq->ctx->engine[ring->id].pin_count == 0) {
+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj,
+   LRC_STATE_PN);


Interesting choice. We called set_page_dirty() when we took the mapping.
Should that page flag be preserved whilst we hold the kmap - I think so,
i.e. the mm cannot flush the page whilst it has an elevated mapcount. So
calling set_page_dirty() again is redundant, right?


If you call mindless copy & paste interesting. :D

Any other concerns or I can respin with that only?

Regards,

Tvrtko
___
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: Cache LRC state page in the context

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 04:05:24PM +, Tvrtko Ursulin wrote:
> 
> On 15/01/16 16:01, Chris Wilson wrote:
> >On Fri, Jan 15, 2016 at 03:10:29PM +, Tvrtko Ursulin wrote:
> >>@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct 
> >>drm_i915_gem_request *rq)
> >>+   if (--rq->ctx->engine[ring->id].pin_count == 0) {
> >>+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj,
> >>+   LRC_STATE_PN);
> >
> >Interesting choice. We called set_page_dirty() when we took the mapping.
> >Should that page flag be preserved whilst we hold the kmap - I think so,
> >i.e. the mm cannot flush the page whilst it has an elevated mapcount. So
> >calling set_page_dirty() again is redundant, right?
> 
> If you call mindless copy & paste interesting. :D
> 
> Any other concerns or I can respin with that only?

No. I was quibbling over the excess clearing of state on unpinning :)

Pity we have to respin even for innoculous changes just to get a CI
tick.
-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] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Do not call API requiring struct_mutex where it is not available

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i7k-2) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b:
pass   -> DMESG-WARN (ilk-hp8440p)

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
ilk-hp8440p  total:141  pass:100  dwarn:4   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a

Results at /archive/results/CI_IGT_test/Patchwork_1199/

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


Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Tvrtko Ursulin


On 15/01/16 16:19, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 04:02:52PM +, Tvrtko Ursulin wrote:


On 15/01/16 13:57, Chris Wilson wrote:

On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote:

Looks like your DDX is the only user not using it in the boolean mode?


As far as I am aware, that is the only user that worries about which
engine the object is currently active on.


And libdrm is a bit confused in its return statements:

 ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
 if (ret == 0) {
 bo_gem->idle = !busy.busy;
 return busy.busy;
 } else {
 return false;
 }
 return (ret == 0 && busy.busy);

Looks like it was a boolean as well until commit
02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident
started exposing the bits.


Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch
postdates when we started storing read/write bits in the return value.
So definitely an unintentional leakage.


In that case I think just respin with comment corrections in uapi
header for drm_i915_gem_busy?


/** Return busy status
  *
  * A return of 0 implies that the object is idle (after
  * having flushed any pending activity), and a non-zero return that
  * the object is still in-flight on the GPU. (The GPU has not yet
  * signaled completion for all pending requests that reference the
  * object.)
  *
  * The returned dword is split into two fields to indicate both
  * the engines on which the object is being read, and the
  * engine on which is currently being writtern to (if any).
  *
  * The low word (bits 0:15) indicate if the object is being written
  * to by any engine (there can only be one, as the GEM implicit
  * synchronisation rules force writes to be serialised.) Only the
  * engine for last write is reported.
  *
  * The high word (bits 16:31) are a bitmask of which engines are
  * currently reading from the object.
  *
  * The value of each engine is the same as specified in the
  * EXECBUFFER2 ioctl, i.e. I915_EXEC_RENDER, I915_EXEC_BSD etc.
  * (Note I915_EXEC_DEFAULT is a symbolic value and is mapped to
  * the I915_EXEC_RENDER engine for execution, and so never reported
  * as active itself.)
  */



Very good, r-b on the resulting patch.

Regards,

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


Re: [Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 04:02:52PM +, Tvrtko Ursulin wrote:
> 
> On 15/01/16 13:57, Chris Wilson wrote:
> >On Fri, Jan 15, 2016 at 01:22:39PM +, Tvrtko Ursulin wrote:
> >>Looks like your DDX is the only user not using it in the boolean mode?
> >
> >As far as I am aware, that is the only user that worries about which
> >engine the object is currently active on.
> >
> >>And libdrm is a bit confused in its return statements:
> >>
> >> ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
> >> if (ret == 0) {
> >> bo_gem->idle = !busy.busy;
> >> return busy.busy;
> >> } else {
> >> return false;
> >> }
> >> return (ret == 0 && busy.busy);
> >>
> >>Looks like it was a boolean as well until commit
> >>02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident
> >>started exposing the bits.
> >
> >Hmm, libdrm bo_is_busy() was always meant to be boolean and that patch
> >postdates when we started storing read/write bits in the return value.
> >So definitely an unintentional leakage.
> 
> In that case I think just respin with comment corrections in uapi
> header for drm_i915_gem_busy?

/** Return busy status
 *
 * A return of 0 implies that the object is idle (after
 * having flushed any pending activity), and a non-zero return that
 * the object is still in-flight on the GPU. (The GPU has not yet
 * signaled completion for all pending requests that reference the
 * object.)
 *
 * The returned dword is split into two fields to indicate both
 * the engines on which the object is being read, and the
 * engine on which is currently being writtern to (if any).
 *
 * The low word (bits 0:15) indicate if the object is being written
 * to by any engine (there can only be one, as the GEM implicit
 * synchronisation rules force writes to be serialised.) Only the
 * engine for last write is reported.
 *
 * The high word (bits 16:31) are a bitmask of which engines are
 * currently reading from the object.
 *
 * The value of each engine is the same as specified in the
 * EXECBUFFER2 ioctl, i.e. I915_EXEC_RENDER, I915_EXEC_BSD etc.
 * (Note I915_EXEC_DEFAULT is a symbolic value and is mapped to
 * the I915_EXEC_RENDER engine for execution, and so never reported
 * as active itself.)
 */

-- 
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 v5] drm/i915: Cache LRC state page in the context

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

LRC lifetime is well defined so we can cache the page pointing
to the object backing store in the context in order to avoid
walking over the object SG page list from the interrupt context
without the big lock held.

v2: Also cache the mapping. (Chris Wilson)
v3: Unmap on the error path.
v4: No need to cache the page. (Chris Wilson)
v5: No need to dirty the page on unpin. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_lrc.c | 53 ++--
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index acff98b9c148..af301482e6f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -890,6 +890,7 @@ struct intel_context {
int pin_count;
struct i915_vma *lrc_vma;
u64 lrc_desc;
+   uint32_t *lrc_reg_state;
} engine[I915_NUM_RINGS];
 
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 545173c1b5c5..dc3ea03a887d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -390,14 +390,7 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
 {
struct intel_engine_cs *ring = rq->ring;
struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
-   struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-   struct page *page;
-   uint32_t *reg_state;
-
-   BUG_ON(!ctx_obj);
-
-   page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
-   reg_state = kmap_atomic(page);
+   uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state;
 
reg_state[CTX_RING_TAIL+1] = rq->tail;
reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start;
@@ -414,8 +407,6 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
}
 
-   kunmap_atomic(reg_state);
-
return 0;
 }
 
@@ -1067,6 +1058,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs 
*ring,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+   struct page *lrc_state_page;
+   uint32_t *lrc_reg_state;
int ret;
 
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
@@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct 
intel_engine_cs *ring,
if (ret)
return ret;
 
+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
+   if (WARN_ON(!lrc_state_page)) {
+   ret = -ENODEV;
+   goto unpin_ctx_obj;
+   }
+
+   lrc_reg_state = kmap(lrc_state_page);
+   if (!lrc_reg_state) {
+   ret = -ENOMEM;
+   goto unpin_ctx_obj;
+   }
+
ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
if (ret)
-   goto unpin_ctx_obj;
+   goto unmap_state_page;
 
ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, ring);
+   ctx->engine[ring->id].lrc_reg_state = lrc_reg_state;
ctx_obj->dirty = true;
 
/* Invalidate GuC TLB. */
@@ -1090,6 +1096,8 @@ static int intel_lr_context_do_pin(struct intel_engine_cs 
*ring,
 
return ret;
 
+unmap_state_page:
+   kunmap(lrc_state_page);
 unpin_ctx_obj:
i915_gem_object_ggtt_unpin(ctx_obj);
 
@@ -1118,15 +1126,22 @@ void intel_lr_context_unpin(struct drm_i915_gem_request 
*rq)
struct intel_engine_cs *ring = rq->ring;
struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = rq->ringbuf;
+   struct page *lrc_state_page;
 
-   if (ctx_obj) {
-   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-   if (--rq->ctx->engine[ring->id].pin_count == 0) {
-   intel_unpin_ringbuffer_obj(ringbuf);
-   i915_gem_object_ggtt_unpin(ctx_obj);
-   rq->ctx->engine[ring->id].lrc_vma = NULL;
-   rq->ctx->engine[ring->id].lrc_desc = 0;
-   }
+   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+
+   if (!ctx_obj)
+   return;
+
+   if (--rq->ctx->engine[ring->id].pin_count == 0) {
+   lrc_state_page = i915_gem_object_get_page(ctx_obj,
+ LRC_STATE_PN);
+   kunmap(lrc_state_page);
+   intel_unpin_ringbuffer_obj(ringbuf);
+   i915_gem_object_ggtt_unpin(ctx_obj);
+   rq->ctx->engine[ring->id

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Only grab timestamps when needed

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (bdw-ultra) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:1   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:122  dwarn:4   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1200/

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


[Intel-gfx] [PATCH v2] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
Tvrtko was looking through the execbuffer-ioctl and noticed that the
uABI was tightly coupled to our internal engine identifiers. Close
inspection also revealed that we leak those internal engine identifiers
through the busy-ioctl, and those internal identifiers already do not
match the user identifiers. Fortuitiously, there is only one user of the
set of busy rings from the busy-ioctl, and they only wish to choose
between the RENDER and the BLT engines.

Let's fix the userspace ABI while we still can.

v2: Update the uAPI documentation to explain the identifiers.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Daniel Vetter 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem.c | 18 ++
 drivers/gpu/drm/i915/intel_lrc.c|  5 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
 include/uapi/drm/i915_drm.h | 33 +
 5 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb44bad15403..85797813a3de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret)
goto unref;
 
-   BUILD_BUG_ON(I915_NUM_RINGS > 16);
-   args->busy = obj->active << 16;
-   if (obj->last_write_req)
-   args->busy |= obj->last_write_req->ring->id;
+   args->busy = 0;
+   if (obj->active) {
+   int i;
+
+   for (i = 0; i < I915_NUM_RINGS; i++) {
+   struct drm_i915_gem_request *req;
+
+   req = obj->last_read_req[i];
+   if (req)
+   args->busy |= 1 << (16 + req->ring->exec_id);
+   }
+   if (obj->last_write_req)
+   args->busy |= obj->last_write_req->ring->exec_id;
+   }
 
 unref:
drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f5d89c845ede..4aa209483237 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device 
*dev)
 
ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
@@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)
 
ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN6_BSD_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
@@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)
 
ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN8_BSD2_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
@@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev)
 
ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;
ring->mmio_base = BLT_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
@@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)
 
ring->name = "video enhancement ring";
ring->id = VECS;
+   ring->exec_id = I915_EXEC_VEBOX;
ring->mmio_base = VEBOX_RING_BASE;
 
logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8cd8aabcc3ff..310d151c0db2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 
ring->name = "render ring";
ring->id = RCS;
+   ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE;
 
if (INTEL_INFO(dev)->gen >= 8) {
@@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 
ring->name = "bsd ring";
ring->id = VCS;
+   ring->exec_id = I915_EXEC_BSD;
 
ring->write_tail = ring_write_tail;
if (INTEL_INFO(dev)->gen >= 6) {
@@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
 
ring->name = "bsd2 ring";
ring->id = VCS2;
+   ring->exec_id = I915_EXEC_BSD;
 
ring->write_tail = ring_write_tail;
ring->mmio_base = GEN8_BSD2_RING_BASE;
@@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 
ring->name = "blitter ring";
ring->id = BCS;
+   ring->exec_id = I915_EXEC_BLT;
 
ring->mmio_base = BLT_RING_BASE;
r

Re: [Intel-gfx] [PATCH v5] drm/i915: Cache LRC state page in the context

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 04:39:29PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> LRC lifetime is well defined so we can cache the page pointing
> to the object backing store in the context in order to avoid
> walking over the object SG page list from the interrupt context
> without the big lock held.
> 
> v2: Also cache the mapping. (Chris Wilson)
> v3: Unmap on the error path.
> v4: No need to cache the page. (Chris Wilson)
> v5: No need to dirty the page on unpin. (Chris Wilson)

Sorry.

> @@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct 
> intel_engine_cs *ring,
>   if (ret)
>   return ret;
>  
> + lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
> + if (WARN_ON(!lrc_state_page)) {
> + ret = -ENODEV;
> + goto unpin_ctx_obj;
> + }
> +
> + lrc_reg_state = kmap(lrc_state_page);

kmap() cannot fail.

> + if (!lrc_reg_state) {
> + ret = -ENOMEM;
> + goto unpin_ctx_obj;
> + }
> + if (--rq->ctx->engine[ring->id].pin_count == 0) {

I was wondering if we should just use

kunmap(kmap_to_page(ce->lrc_reg_state));

Except until we get struct intel_context_engine that line will exceed
80cols!
-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 v2] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 04:51:46PM +, Chris Wilson wrote:
> Tvrtko was looking through the execbuffer-ioctl and noticed that the
> uABI was tightly coupled to our internal engine identifiers. Close
> inspection also revealed that we leak those internal engine identifiers
> through the busy-ioctl, and those internal identifiers already do not
> match the user identifiers. Fortuitiously, there is only one user of the
> set of busy rings from the busy-ioctl, and they only wish to choose
> between the RENDER and the BLT engines.
> 
> Let's fix the userspace ABI while we still can.
> 
> v2: Update the uAPI documentation to explain the identifiers.

Testcase: igt/gem_busy
> Signed-off-by: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Cc: Daniel Vetter 
> Reviewed-by: Tvrtko Ursulin 
-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 v6] drm/i915: Cache LRC state page in the context

2016-01-15 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

LRC lifetime is well defined so we can cache the page pointing
to the object backing store in the context in order to avoid
walking over the object SG page list from the interrupt context
without the big lock held.

v2: Also cache the mapping. (Chris Wilson)
v3: Unmap on the error path.
v4: No need to cache the page. (Chris Wilson)
v5: No need to dirty the page on unpin. (Chris Wilson)
v6: kmap() cannot fail and use kmap_to_page to simplify unpin.
(Chris Wilson)

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: Dave Gordon 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_lrc.c | 39 +--
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index acff98b9c148..af301482e6f8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -890,6 +890,7 @@ struct intel_context {
int pin_count;
struct i915_vma *lrc_vma;
u64 lrc_desc;
+   uint32_t *lrc_reg_state;
} engine[I915_NUM_RINGS];
 
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 545173c1b5c5..285e7f26760c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -390,14 +390,7 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
 {
struct intel_engine_cs *ring = rq->ring;
struct i915_hw_ppgtt *ppgtt = rq->ctx->ppgtt;
-   struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
-   struct page *page;
-   uint32_t *reg_state;
-
-   BUG_ON(!ctx_obj);
-
-   page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
-   reg_state = kmap_atomic(page);
+   uint32_t *reg_state = rq->ctx->engine[ring->id].lrc_reg_state;
 
reg_state[CTX_RING_TAIL+1] = rq->tail;
reg_state[CTX_RING_BUFFER_START+1] = rq->ringbuf->vma->node.start;
@@ -414,8 +407,6 @@ static int execlists_update_context(struct 
drm_i915_gem_request *rq)
ASSIGN_CTX_PDP(ppgtt, reg_state, 0);
}
 
-   kunmap_atomic(reg_state);
-
return 0;
 }
 
@@ -1067,6 +1058,7 @@ static int intel_lr_context_do_pin(struct intel_engine_cs 
*ring,
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+   struct page *lrc_state_page;
int ret;
 
WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
@@ -1076,12 +1068,19 @@ static int intel_lr_context_do_pin(struct 
intel_engine_cs *ring,
if (ret)
return ret;
 
+   lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
+   if (WARN_ON(!lrc_state_page)) {
+   ret = -ENODEV;
+   goto unpin_ctx_obj;
+   }
+
ret = intel_pin_and_map_ringbuffer_obj(ring->dev, ringbuf);
if (ret)
goto unpin_ctx_obj;
 
ctx->engine[ring->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj);
intel_lr_context_descriptor_update(ctx, ring);
+   ctx->engine[ring->id].lrc_reg_state = kmap(lrc_state_page);
ctx_obj->dirty = true;
 
/* Invalidate GuC TLB. */
@@ -1119,14 +1118,18 @@ void intel_lr_context_unpin(struct drm_i915_gem_request 
*rq)
struct drm_i915_gem_object *ctx_obj = rq->ctx->engine[ring->id].state;
struct intel_ringbuffer *ringbuf = rq->ringbuf;
 
-   if (ctx_obj) {
-   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
-   if (--rq->ctx->engine[ring->id].pin_count == 0) {
-   intel_unpin_ringbuffer_obj(ringbuf);
-   i915_gem_object_ggtt_unpin(ctx_obj);
-   rq->ctx->engine[ring->id].lrc_vma = NULL;
-   rq->ctx->engine[ring->id].lrc_desc = 0;
-   }
+   WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+
+   if (!ctx_obj)
+   return;
+
+   if (--rq->ctx->engine[ring->id].pin_count == 0) {
+   kunmap(kmap_to_page(rq->ctx->engine[ring->id].lrc_reg_state));
+   intel_unpin_ringbuffer_obj(ringbuf);
+   i915_gem_object_ggtt_unpin(ctx_obj);
+   rq->ctx->engine[ring->id].lrc_vma = NULL;
+   rq->ctx->engine[ring->id].lrc_desc = 0;
+   rq->ctx->engine[ring->id].lrc_reg_state = NULL;
}
 }
 
-- 
1.9.1

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


[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Decouple execbuf uAPI from internal implementation (rev2)

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i5k-2) UNSTABLE
pass   -> DMESG-WARN (skl-i7k-2) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-b-frame-sequence:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:122  dwarn:4   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a

Results at /archive/results/CI_IGT_test/Patchwork_1201/

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


[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids (rev2)

2016-01-15 Thread Patchwork
== Summary ==

Built on 615fbad7f4cea73b1a8eccdcc942c8ca1a708dab drm-intel-nightly: 
2016y-01m-15d-09h-46m-32s UTC integration manifest

Test gem_storedw_loop:
Subgroup basic-render:
pass   -> DMESG-WARN (skl-i5k-2) UNSTABLE
dmesg-warn -> PASS   (bdw-nuci7) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-modeset:
dmesg-warn -> PASS   (skl-i5k-2)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
pass   -> FAIL   (snb-x220t)
Subgroup read-crc-pipe-b-frame-sequence:
pass   -> DMESG-WARN (byt-nuc)

bdw-nuci7total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc  total:141  pass:122  dwarn:4   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p  total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:121  dwarn:5   dfail:0   fail:2   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1202/

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


Re: [Intel-gfx] [PATCH 1/6] drm/i915: Remove forcewake dance from seqno/irq barrier on legacy gen6+

2016-01-15 Thread Mika Kuoppala
Chris Wilson  writes:

> In order to ensure seqno/irq coherency, we current read a ring register.
> We are not sure quite how it works, only that is does. Experiments show
> that e.g. doing a clflush(seqno) instead is not sufficient, but we can
> remove the forcewake dance from the mmio access.
>
> v2: Baytrail wants a clflush too.
>
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 8cd8aabcc3ff..935add1422ae 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1492,10 +1492,21 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, 
> bool lazy_coherency)
>  {
>   /* Workaround to force correct ordering between irq and seqno writes on
>* ivb (and maybe also on snb) by reading from a CS register (like
> -  * ACTHD) before reading the status page. */
> +  * ACTHD) before reading the status page.
> +  *
> +  * Note that this effectively effectively stalls the read by the
> time

effectively twice there.

Reviewed-by: Mika Kuoppala 

> +  * it takes to do a memory transaction, which more or less ensures
> +  * that the write from the GPU has sufficient time to invalidate
> +  * the CPU cacheline. Alternatively we could delay the interrupt from
> +  * the CS ring to give the write time to land, but that would incur
> +  * a delay after every batch i.e. much more frequent than a delay
> +  * when waiting for the interrupt (with the same net latency).
> +  */
>   if (!lazy_coherency) {
>   struct drm_i915_private *dev_priv = ring->dev->dev_private;
> - POSTING_READ(RING_ACTHD(ring->mmio_base));
> + POSTING_READ_FW(RING_ACTHD(ring->mmio_base));
> +
> + intel_flush_status_page(ring, I915_GEM_HWS_INDEX);
>   }
>  
>   return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
> -- 
> 2.7.0.rc3
>
> ___
> 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] drm/i915: Don't reject primayr plane windowing with color keying enabled on SKL+

2016-01-15 Thread ville . syrjala
From: Ville Syrjälä 

On SKL+ plane scaling is mutually exclusive with color keying. The code
check for this, but during some refactoring the code got changed to
also reject primary plane windowing when color keying is used. There is
no such restriction in the hardware, so restore the original logic.

Cc: Maarten Lankhorst 
Fixes: 061e4b8d650a ("drm/i915: clean up atomic plane check functions, v2.")
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a726c5e91220..7bc56e217ecc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14019,11 +14019,12 @@ intel_check_primary_plane(struct drm_plane *plane,
int max_scale = DRM_PLANE_HELPER_NO_SCALING;
bool can_position = false;
 
-   /* use scaler when colorkey is not required */
-   if (INTEL_INFO(plane->dev)->gen >= 9 &&
-   state->ckey.flags == I915_SET_COLORKEY_NONE) {
-   min_scale = 1;
-   max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
+   if (INTEL_INFO(plane->dev)->gen >= 9) {
+   /* use scaler when colorkey is not required */
+   if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
+   min_scale = 1;
+   max_scale = skl_max_scale(to_intel_crtc(crtc), 
crtc_state);
+   }
can_position = true;
}
 
-- 
2.4.10

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


Re: [Intel-gfx] [PATCH] drm/i915: skl_update_scaler() wants a rotation bitmask instead of bit number

2016-01-15 Thread Ville Syrjälä
On Thu, Oct 15, 2015 at 05:01:58PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Pass BIT(DRM_ROTATE_0) instead of DRM_ROTATE_0 to skl_update_scaler().
> The former is a mask, the latter just the bit number.
> 
> Fortunately the only thing skl_update_scaler() does with the rotation
> is check if it's 90/270 degrees or not, and so in this case it would
> still do the right thing.
> 
> Cc: Chandra Konduru 
> Signed-off-by: Ville Syrjälä 

Ping, anyone care to r-b this one?

> ---
>  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 7498c9d..02316d0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4673,7 +4673,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state 
> *state)
> intel_crtc->base.base.id, intel_crtc->pipe, 
> SKL_CRTC_INDEX);
>  
>   return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
> - &state->scaler_state.scaler_id, DRM_ROTATE_0,
> + &state->scaler_state.scaler_id, BIT(DRM_ROTATE_0),
>   state->pipe_src_w, state->pipe_src_h,
>   adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
>  }
> -- 
> 2.4.9

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use the active wm config for merging on ILK-BDW

2016-01-15 Thread Ville Syrjälä
On Thu, Jan 14, 2016 at 06:37:32PM -0800, Matt Roper wrote:
> On Thu, Jan 14, 2016 at 02:53:35PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > ilk_program_watermarks() is supposed to merge the active watermarks from
> > all pipes. Thus we need to use the active config too instead of some
> > precomputed stuff.
> 
> So to clarify, the bug you're fixing here would be if we have racing
> commits that operate on disjoint sets of CRTC's; in that case the second
> one that actually gets into wm_mutex will fail to see the config changes
> made by the first commit, right?

That, or I suppose cases where the intermediate and optimal watermarks
have a different idea about the sprite enabled/scaled flags. I assume
what it was doing is picking those flags always based on the optimal wms
even when programming the intermedidate wms.

> 
> Seems like we could go ahead and remove dev_priv->wm.config (and
> calc_watermark_data() that builds it) since it's not actually doing us
> any good.  Although it's probably fine to hold that off to a subsequent
> patch.

IIRC I was a reference to that stuff in the SKL code too, so I didn't bother
trying to kill it without knowing what it was doing. And I was too lazy to
take a deeper look at the code right now.

> 
> Both of your patches are
> 
> Reviewed-by: Matt Roper 
> 
> CI results do report SKL failures, but those are clearly bogus since SKL
> doesn't even run any of the functions that you're changing in these two
> patches; as you noted, it looks more like the machine had some kind of
> bizarre hardware failure that was unrelated to the patchset.  The other
> results look clean.  Given that, I've gone ahead and pushed your patches
> to dinq.  Thanks!

Cheers.

> 
> 
> Matt
> 
> 
> > 
> > Fixes: aa363136866c ("drm/i915: Calculate watermark configuration during 
> > atomic check (v2)")
> > Cc: Matt Roper 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 32 ++--
> >  1 file changed, 26 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index e9f4e6e7b736..f44a961183d7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3680,23 +3680,43 @@ static void skl_update_wm(struct drm_crtc *crtc)
> > dev_priv->wm.skl_hw = *results;
> >  }
> >  
> > +static void ilk_compute_wm_config(struct drm_device *dev,
> > + struct intel_wm_config *config)
> > +{
> > +   struct intel_crtc *crtc;
> > +
> > +   /* Compute the currently _active_ config */
> > +   for_each_intel_crtc(dev, crtc) {
> > +   const struct intel_pipe_wm *wm = &crtc->wm.active.ilk;
> > +
> > +   if (!wm->pipe_enabled)
> > +   continue;
> > +
> > +   config->sprites_enabled |= wm->sprites_enabled;
> > +   config->sprites_scaled |= wm->sprites_scaled;
> > +   config->num_pipes_active++;
> > +   }
> > +}
> > +
> >  static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
> >  {
> > struct drm_device *dev = dev_priv->dev;
> > struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
> > struct ilk_wm_maximums max;
> > -   struct intel_wm_config *config = &dev_priv->wm.config;
> > +   struct intel_wm_config config = {};
> > struct ilk_wm_values results = {};
> > enum intel_ddb_partitioning partitioning;
> >  
> > -   ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max);
> > -   ilk_wm_merge(dev, config, &max, &lp_wm_1_2);
> > +   ilk_compute_wm_config(dev, &config);
> > +
> > +   ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max);
> > +   ilk_wm_merge(dev, &config, &max, &lp_wm_1_2);
> >  
> > /* 5/6 split only in single pipe config on IVB+ */
> > if (INTEL_INFO(dev)->gen >= 7 &&
> > -   config->num_pipes_active == 1 && config->sprites_enabled) {
> > -   ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, 
> > &max);
> > -   ilk_wm_merge(dev, config, &max, &lp_wm_5_6);
> > +   config.num_pipes_active == 1 && config.sprites_enabled) {
> > +   ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, 
> > &max);
> > +   ilk_wm_merge(dev, &config, &max, &lp_wm_5_6);
> >  
> > best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6);
> > } else {
> > -- 
> > 2.4.10
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/8] drm/i915: Some more fb offsets[] prep stuff

2016-01-15 Thread Ville Syrjälä
On Thu, Jan 14, 2016 at 03:22:08PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Yanked a few more prep patches from my earlier fb offsets[] work [1]. First
> few have r-bs, the rest don't.
> 
> The last two patches are new. Things just tickled my OCD a bit too much so
> had to deal with them.
> 
> [1] http://lists.freedesktop.org/archives/intel-gfx/2015-October/078050.html
> 
> Ville Syrjälä (8):
>   drm/i915: Don't leak framebuffer_references if drm_framebuffer_init()
> fails
>   drm/i915: Set i915_ggtt_view_normal type explicitly
>   drm/i915: Pass the dma_addr_t array as const to rotate_pages()

These first three had r-bs already, so I went and pushed them to dinq.
Thanks for the reviews.

>   drm/i915: Rename the rotated gtt view member to 'rotated'
>   drm/i915: Pass stride to rotate_pages()
>   drm/i915: Pass rotation_info to intel_rotate_fb_obj_pages()
>   drm/i915: Make display gtt offsets u32
>   drm/i915: Standardize on 'cpp' for bytes per pixel
> 
>  drivers/gpu/drm/i915/i915_gem_gtt.c  |  22 +++---
>  drivers/gpu/drm/i915/i915_gem_gtt.h  |   2 +-
>  drivers/gpu/drm/i915/intel_display.c |  49 +++---
>  drivers/gpu/drm/i915/intel_drv.h |  12 ++--
>  drivers/gpu/drm/i915/intel_pm.c  | 128 
> +--
>  drivers/gpu/drm/i915/intel_sprite.c  |  40 +--
>  6 files changed, 123 insertions(+), 130 deletions(-)
> 
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6] drm/i915: Cache LRC state page in the context

2016-01-15 Thread Chris Wilson
On Fri, Jan 15, 2016 at 05:12:45PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> LRC lifetime is well defined so we can cache the page pointing
> to the object backing store in the context in order to avoid
> walking over the object SG page list from the interrupt context
> without the big lock held.
> 
> v2: Also cache the mapping. (Chris Wilson)
> v3: Unmap on the error path.
> v4: No need to cache the page. (Chris Wilson)
> v5: No need to dirty the page on unpin. (Chris Wilson)
> v6: kmap() cannot fail and use kmap_to_page to simplify unpin.
> (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Chris Wilson 
> Cc: Dave Gordon 
Reviewed-by: Chris Wilson 
-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] drm/i915: Don't do pre plane update on disabled crtcs

2016-01-15 Thread Matt Roper
On Thu, Jan 14, 2016 at 06:32:10PM +0200, Mika Kuoppala wrote:
> CI/Bat got following (shortened) trace on byt and also
> on bsw:
> 
> [ cut here ]---
> Unclaimed register detected before reading register 0x186500
> Call Trace:
>  __unclaimed_reg_debug+0x68/0x80 [i915]
> vlv_read32+0x2de/0x370 [i915]
> intel_set_memory_cxsr+0x87/0x1a0 [i915]
> intel_pre_plane_update+0xb3/0xf0 [i915]
> intel_atomic_commit+0x3b5/0x17c0 [i915]
> ...
> ---[ end trace 6387a0ad001bb39f ]---
> 
> Fix this by limiting pre plane update only to active crtcs.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=93698
> Cc: Ville Syrjälä 
> Cc: Maarten Lankhorst 
> Signed-off-by: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index aa24f79d85bf..a134a698d97d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13580,9 +13580,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>   if (!needs_modeset(crtc->state))
>   continue;
>  
> - intel_pre_plane_update(intel_crtc);
> -
>   if (crtc_state->active) {
> + intel_pre_plane_update(intel_crtc);

Won't this change prevent us from setting up watermarks before turning
on the CRTC in a disabled->enabled transition?


Matt

>   intel_crtc_disable_planes(crtc, crtc_state->plane_mask);
>   dev_priv->display.crtc_disable(crtc);
>   intel_crtc->active = false;
> -- 
> 2.5.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: skl_update_scaler() wants a rotation bitmask instead of bit number

2016-01-15 Thread Matt Roper
On Fri, Jan 15, 2016 at 08:48:26PM +0200, Ville Syrjälä wrote:
> On Thu, Oct 15, 2015 at 05:01:58PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > Pass BIT(DRM_ROTATE_0) instead of DRM_ROTATE_0 to skl_update_scaler().
> > The former is a mask, the latter just the bit number.
> > 
> > Fortunately the only thing skl_update_scaler() does with the rotation
> > is check if it's 90/270 degrees or not, and so in this case it would
> > still do the right thing.
> > 
> > Cc: Chandra Konduru 
> > Signed-off-by: Ville Syrjälä 
> 
> Ping, anyone care to r-b this one?

Reviewed-by: Matt Roper 

Looks like this bug has been present since scalers were first added in
6156a45602f9 ("drm/i915: skylake primary plane scaling using shared scalers")


Matt

> 
> > ---
> >  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 7498c9d..02316d0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4673,7 +4673,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state 
> > *state)
> >   intel_crtc->base.base.id, intel_crtc->pipe, 
> > SKL_CRTC_INDEX);
> >  
> > return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
> > -   &state->scaler_state.scaler_id, DRM_ROTATE_0,
> > +   &state->scaler_state.scaler_id, BIT(DRM_ROTATE_0),
> > state->pipe_src_w, state->pipe_src_h,
> > adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
> >  }
> > -- 
> > 2.4.9
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Don't reject primayr plane windowing with color keying enabled on SKL+

2016-01-15 Thread Matt Roper
On Fri, Jan 15, 2016 at 08:46:53PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> On SKL+ plane scaling is mutually exclusive with color keying. The code
> check for this, but during some refactoring the code got changed to
> also reject primary plane windowing when color keying is used. There is
> no such restriction in the hardware, so restore the original logic.
> 
> Cc: Maarten Lankhorst 
> Fixes: 061e4b8d650a ("drm/i915: clean up atomic plane check functions, v2.")
> Signed-off-by: Ville Syrjälä 

s/primayr/primary in your headline, but otherwise

Reviewed-by: Matt Roper 

Still waiting for CI to run the BAT on this...


Matt

> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index a726c5e91220..7bc56e217ecc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14019,11 +14019,12 @@ intel_check_primary_plane(struct drm_plane *plane,
>   int max_scale = DRM_PLANE_HELPER_NO_SCALING;
>   bool can_position = false;
>  
> - /* use scaler when colorkey is not required */
> - if (INTEL_INFO(plane->dev)->gen >= 9 &&
> - state->ckey.flags == I915_SET_COLORKEY_NONE) {
> - min_scale = 1;
> - max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
> + if (INTEL_INFO(plane->dev)->gen >= 9) {
> + /* use scaler when colorkey is not required */
> + if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
> + min_scale = 1;
> + max_scale = skl_max_scale(to_intel_crtc(crtc), 
> crtc_state);
> + }
>   can_position = true;
>   }
>  
> -- 
> 2.4.10
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] MAINTAINERS: Add "B:" preferred bug reporting method

2016-01-15 Thread Joe Perches
On Fri, 2016-01-15 at 16:45 +0200, Jani Nikula wrote:
> Different subsystems and drivers have different preferred ways of
> receiving bug reports; mailing list or bugzillas at various
> locations. Add "B:" entry for specifying the preference to guide bug
> reporters at the right location.
[]
> diff --git a/MAINTAINERS b/MAINTAINERS
[]
> @@ -75,6 +75,8 @@ Descriptions of section entries:
>   L: Mailing list that is relevant to this area
>   W: Web-page with status/info
>   Q: Patchwork web based patch tracking system site
> + B: Preferred method for reporting bugs; bug tracking system site
> +    or "List" for mailing list.
>   T: SCM tree type and location.
>      Type is one of: git, hg, quilt, stgit, topgit
>   S: Status, one of the following:
> @@ -3655,6 +3657,7 @@ L:  intel-gfx@lists.freedesktop.org
>  L:   dri-de...@lists.freedesktop.org
>  W:   https://01.org/linuxgraphics/
>  Q:   http://patchwork.freedesktop.org/project/intel-gfx/
> +B:   
> https://bugs.freedesktop.org/enter_bug.cgi?product=DRI&component=DRM/Intel

This requires a LoginID & password

Maybe useful to show the open bugs too:
https://bugs.freedesktop.org/buglist.cgi?component=DRM%2FIntel&product=DRI&resolution=---

Maybe the get_maintainer script should be updated.
Something like: (untested)
---
 scripts/get_maintainer.pl | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 1873421..bbe5337 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -47,6 +47,7 @@ my $output_rolestats = 1;
 my $output_section_maxlen = 50;
 my $scm = 0;
 my $web = 0;
+my $bug = 0;
 my $subsystem = 0;
 my $status = 0;
 my $keywords = 1;
@@ -239,6 +240,7 @@ if (!GetOptions(
    'status!' => \$status,
    'scm!' => \$scm,
    'web!' => \$web,
+   'bug!' => \$bug,
    'pattern-depth=i' => \$pattern_depth,
    'k|keywords!' => \$keywords,
    'sections!' => \$sections,
@@ -276,12 +278,13 @@ if ($sections) {
 $status = 0;
 $subsystem = 0;
 $web = 0;
+$bug = 0;
 $keywords = 0;
 $interactive = 0;
 } else {
-my $selections = $email + $scm + $status + $subsystem + $web;
+my $selections = $email + $scm + $status + $subsystem + $web + $bug;
 if ($selections == 0) {
-   die "$P:  Missing required option: email, scm, status, subsystem or 
web\n";
+   die "$P:  Missing required option: email, scm, status, subsystem, web 
or bug\n";
 }
 }
 
@@ -505,6 +508,7 @@ my %hash_list_to;
 my @list_to = ();
 my @scm = ();
 my @web = ();
+my @bug = ();
 my @subsystem = ();
 my @status = ();
 my %deduplicate_name_hash = ();
@@ -537,6 +541,11 @@ if ($web) {
 output(@web);
 }
 
+if ($bug) {
+@bug = uniq(@bug);
+output(@bug);
+}
+
 exit($exit);
 
 sub ignore_email_address {
@@ -593,6 +602,7 @@ sub get_maintainers {
 @list_to = ();
 @scm = ();
 @web = ();
+@bug = ();
 @subsystem = ();
 @status = ();
 %deduplicate_name_hash = ();
@@ -802,6 +812,7 @@ MAINTAINER field selection options:
   --status => print status if any
   --subsystem => print subsystem name if any
   --web => print website(s) if any
+  --bug => print bug reporting mechanism(s) if any
 
 Output type options:
   --separator [, ] => separator for multiple entries on 1 line
@@ -1129,6 +1140,8 @@ sub add_categories {
    push(@scm, $pvalue);
    } elsif ($ptype eq "W") {
    push(@web, $pvalue);
+   } elsif ($ptype eq "B") {
+   push(@bug, $pvalue);
    } elsif ($ptype eq "S") {
    push(@status, $pvalue);
    }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Do not call API requiring struct_mutex where it is not available (rev3)

2016-01-15 Thread Patchwork
== Summary ==

Built on 0ea9000bcb6f394edde5111494a92b0607214cfa drm-intel-nightly: 
2016y-01m-15d-19h-09m-45s UTC integration manifest

Test gem_ctx_basic:
pass   -> FAIL   (bdw-ultra)
Test gem_storedw_loop:
Subgroup basic-render:
dmesg-warn -> PASS   (bdw-ultra) UNSTABLE
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
dmesg-warn -> PASS   (snb-dellxps)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (snb-dellxps)
Test pm_rpm:
Subgroup basic-rte:
dmesg-warn -> PASS   (byt-nuc) UNSTABLE

bdw-nuci7total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultratotal:138  pass:131  dwarn:0   dfail:0   fail:1   skip:6  
byt-nuc  total:141  pass:124  dwarn:2   dfail:0   fail:0   skip:15 
hsw-brixbox  total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2  total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ivb-t430stotal:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps  total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220ttotal:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1203/

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