Re: [Intel-gfx] [PATCH v2 08/16] drm/i915/guc: Update CT message header definition

[Daniele Ceraolo Spurio]

On 07/08/17 09:14, Michal Wajdeczko wrote:

Flags bits are different in G2H message.

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Cc: Kelvin Gardiner 


Matches GuC defines, so:

Reviewed-by: Daniele Ceraolo Spurio 

-Daniele


---
  drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 367aa65..89781d3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
   *
   * bit[4..0]  message len (in dwords)
   * bit[7..5]  reserved
+ * bit[10..8]  flags
+ * bit[15..11] reserved
+ * bit[31..16] action code
+ *
+ * H2G flags:
   * bit[8] write fence to desc
   * bit[9] write status to H2G buff
   * bit[10]send status (via G2H)
- * bit[15..11] reserved
- * bit[31..16] action code
+ *
+ * G2H flags:
+ * bit[8]  is response
   */
  #define GUC_CT_MSG_LEN_SHIFT  0
  #define GUC_CT_MSG_LEN_MASK   0x1F
  #define GUC_CT_MSG_WRITE_FENCE_TO_DESC(1 << 8)
  #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF   (1 << 9)
  #define GUC_CT_MSG_SEND_STATUS(1 << 10)
+#define GUC_CT_MSG_IS_RESPONSE (1 << 8)
  #define GUC_CT_MSG_ACTION_SHIFT   16
  #define GUC_CT_MSG_ACTION_MASK0x
  


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

Re: [Intel-gfx] [PATCH 01/16] drm/i915: Keep a small stash of preallocated WC pages

[Matthew Auld]

On 26 July 2017 at 14:25, Chris Wilson  wrote:
> We use WC pages for coherent writes into the ppGTT on !llc
> architectuures. However, to create a WC page requires a stop_machine(),
architectures

> i.e. is very slow. To compensate we currently keep a per-vm cache of
> recently freed pages, but we still see the slow startup of new contexts.
> We can amoritize that cost slightly by allocating WC pages in small
> batches (PAGEVEC_SIZE == 14) and since creating a WC page implies a
> stop_machine() there is no penalty for keeping that stash global.
>
> Signed-off-by: Chris Wilson 

fwiw,
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 15/16] drm/i915/guc: Trace messages from CT while in debug

[Daniele Ceraolo Spurio]

On 07/08/17 09:14, Michal Wajdeczko wrote:

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)

Signed-off-by: Michal Wajdeczko 
---
  drivers/gpu/drm/i915/intel_guc_ct.c | 41 +++--
  1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index e6912be..568fbc0 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
  #include "i915_drv.h"
  #include "intel_guc_ct.h"
  
+#ifdef CONFIG_DRM_I915_DEBUG


Personally I'd prefer to have a separate kconfig for this (e.g. 
CONFIG_DRM_I915_DEBUG_GUC) as this can be quite verbose in some 
GuC-centric scenarios.


-Daniele


+#define CT_DEBUG_DRIVER(...)   DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
  struct ct_request {
struct list_head link;
u32 fence;
@@ -69,8 +75,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
  static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
u32 cmds_addr, u32 size, u32 owner)
  {
-   DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
-desc, cmds_addr, size, owner);
+   CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
+   desc, cmds_addr, size, owner);
memset(desc, 0, sizeof(*desc));
desc->addr = cmds_addr;
desc->size = size;
@@ -79,8 +85,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc 
*desc,
  
  static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)

  {
-   DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
-desc, desc->head, desc->tail);
+   CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
+   desc, desc->head, desc->tail);
desc->head = 0;
desc->tail = 0;
desc->is_in_error = 0;
@@ -176,7 +182,7 @@ static int ctch_init(struct intel_guc *guc,
err = PTR_ERR(blob);
goto err_vma;
}
-   DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+   CT_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
  
  	/* store pointers to desc and cmds */

for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -190,8 +196,8 @@ static int ctch_init(struct intel_guc *guc,
  err_vma:
i915_vma_unpin_and_release(>vma);
  err_out:
-   DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
-ctch->owner, err);
+   CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
+   ctch->owner, err);
return err;
  }
  
@@ -211,8 +217,8 @@ static int ctch_open(struct intel_guc *guc,

int err;
int i;
  
-	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",

-ctch->owner, yesno(ctch_is_open(ctch)));
+   CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
+   ctch->owner, yesno(ctch_is_open(ctch)));
  
  	if (!ctch->vma) {

err = ctch_init(guc, ctch);
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
  
+	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",

+   4, , 4, ,
+   4*(len - 1), [1]);
+
cmds[tail] = header;
tail = (tail + 1) % size;
  
@@ -500,6 +510,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,

if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
  action[0], ret, status);
+   } else if (unlikely(ret)) {
+   CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+   action[0], ret, ret);
}
  
  	mutex_unlock(>send_mutex);

@@ -546,10 +559,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
/* beware of buffer wrap case */
if (unlikely(available < 0))
available += size;
+   CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
GEM_BUG_ON(available < 0);
  
  	data[0] = cmds[head];

head = (head + 1) % size;
+   CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
  
  	/* message len with header */

len = ct_header_get_len(data[0]) + 1;
@@ -567,6 +582,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
data[i] = cmds[head];
head = (head + 1) % size;
}
+   CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
  
  	desc->head = head * 4;

return 0;

Re: [Intel-gfx] [PATCH i-g-t] tests/perf: fix userspace configs issues on HSW

[Matthew Auld]

On 7 August 2017 at 18:20, Lionel Landwerlin
 wrote:
> We don't have flex eu counters on HSW, so don't try to program for
> thoses.
>
> Reported-by: CI \o/
> Fixes: 609cb5e30b4 ("tests/perf: add tests to verify create/destroy userspace 
> configs")
> Signed-off-by: Lionel Landwerlin 

Crumbs...
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v13 5/7] vfio: ABI for mdev display dma-buf operation

[Alex Williamson]

On Mon, 7 Aug 2017 08:11:43 +
"Zhang, Tina"  wrote:

> After going through the previous discussions, here are some summaries may be 
> related to the current discussion:
> 1. How does user mode figure the device capabilities between region and 
> dma-buf?
> VFIO_DEVICE_GET_REGION_INFO could tell if the mdev supports region case. 
> Otherwise, the mdev supports dma-buf.

Why do we need to make this assumption?  What happens when dma-buf is
superseded?  What happens if a device supports both dma-buf and
regions?  We have a flags field in vfio_device_gfx_plane_info, doesn't
it make sense to use it to identify which field, between region_index
and fd, is valid?  We could even put region_index and fd into a union
with the flag bits indicating how to interpret the union, but I'm not
sure everyone was onboard with this idea.  Seems like a waste of 4
bytes not to do that though.

Thinking further, is the user ever in a situation where they query the
graphics plane info and can handle either a dma-buf or a region?  It
seems more likely that the user needs to know early on which is
supported and would then require that they continue to see compatible
plane information...  Should the user then be able to specify whether
they want a dma-buf or a region?  Perhaps these flag bits are actually
input and the return should be -errno if the driver cannot produce
something compatible.

Maybe we'd therefore define 3 flag bits: PROBE, DMABUF, REGION.  In
this initial implementation, DMABUF or REGION would always be set by
the user to request that type of interface.  Additionally, the QUERY
bit could be set to probe compatibility, thus if PROBE and REGION are
set, the vendor driver would return success only if it supports the
region based interface.  If PROBE and DMABUF are set, the vendor driver
returns success only if the dma-buf based interface is supported.  The
value of the remainder of the structure is undefined for PROBE.
Additionally setting both DMABUF and REGION is invalid.  Undefined
flags bits must be validated as zero by the drivers for future use
(thus if we later define DMABUFv2, an older driver should
automatically return -errno when probed or requested).

It seems like this handles all the cases, the user can ask what's
supported and specifies the interface they want on every call.  The
user therefore can also choose between region_index and fd and we can
make that a union.

> 2. For dma-buf, how to differentiate unsupported vs not initialized?
> For dma-buf, when the mdev doesn't support some arguments, -EINVAL will be 
> returned. And -errno will return when meeting other failures, like -ENOMEM.
> If the mdev is not initialized, there won't be any returned err. Just zero 
> all the fields in structure vfio_device_gfx_plane_info.

So we're relying on special values again :-\  For which fields is zero
not a valid value?  I prefer the probe interface above unless there are
better ideas.
 
> 3. The id field in structure vfio_device_gfx_plane_info
> So far we haven't figured out the usage of this field for dma-buf usage. So, 
> this field is changed to "region_index" and only used for region usage.
> In previous discussions, we thought this "id" field might be used for both 
> dma-buf and region usages.
> So, we proposed some ways, like adding flags field to the structure. Another 
> way to do it was to add this:
> enum vfio_device_gfx_type {
>  VFIO_DEVICE_GFX_NONE,
>  VFIO_DEVICE_GFX_DMABUF,
>  VFIO_DEVICE_GFX_REGION,
>  };
>  
>  struct vfio_device_gfx_query_caps {
>  __u32 argsz;
>  __u32 flags;
>  enum vfio_device_gfx_type;
>  };
> Obviously, we don't need to consider this again, unless we find the 
> "region_index" means something for dma-buf usage.

The usefulness of this ioctl seems really limited and once again we get
into a situation where having two ioctls leaves doubt whether this is
describing the current plane state.  Thanks,

Alex

> > > > > > > >  include/uapi/linux/vfio.h | 28 
> > > > > > > >  1 file changed, 28 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/include/uapi/linux/vfio.h
> > > > > > > > b/include/uapi/linux/vfio.h index ae46105..827a230 100644
> > > > > > > > --- a/include/uapi/linux/vfio.h
> > > > > > > > +++ b/include/uapi/linux/vfio.h
> > > > > > > > @@ -502,6 +502,34 @@ struct vfio_pci_hot_reset {
> > > > > > > >
> > > > > > > >  #define VFIO_DEVICE_PCI_HOT_RESET  _IO(VFIO_TYPE,  
> > VFIO_BASE +  
> > > > > > > 13)  
> > > > > > > >
> > > > > > > > +/**
> > > > > > > > + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE,  
> > VFIO_BASE  
> > > +  
> > > > > 14,  
> > > > > > > > +struct vfio_device_query_gfx_plane)
> > > > > > > > + *
> > > > > > > > + * Set the drm_plane_type and retrieve information about
> > > > > > > > +the gfx  
> > > plane.  
> > > > > > > > + *
> > > > > > > > + * Return: 0 on success, -errno on failure.
> > > > > > > > + */
> > > > > > > > 

Re: [Intel-gfx] [PATCH] drm/i915: Clear lost context-switch interrupts across reset

[Michel Thierry]

On 8/7/2017 5:19 AM, Chris Wilson wrote:

During a global reset, we disable the irq. As we disable the irq, the
hardware may be raising a GT interrupt that we then ignore, leaving it
pending in the GTIIR. After the reset, we then re-enable the irq,
triggering the pending interrupt. However, that interrupt was for the
stale state from before the reset, and the contents of the CSB buffer
are now invalid.

Reported-by: "Dong, Chuanxiao" 
Signed-off-by: Chris Wilson 
Cc: "Dong, Chuanxiao" 
Cc: Tvrtko Ursulin 
Cc: Michal Winiarski 
Cc: Michel Thierry 
---
  drivers/gpu/drm/i915/intel_lrc.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b0738d2b2a7f..bc61948e2601 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1221,6 +1221,14 @@ static int intel_init_workaround_bb(struct 
intel_engine_cs *engine)
return ret;
  }
  
+static u8 gtiir[] = {

+   [RCS] = 0,
+   [BCS] = 0,
+   [VCS] = 1,
+   [VCS2] = 1,
+   [VECS] = 3,
+};
+
  static int gen8_init_common_ring(struct intel_engine_cs *engine)
  {
struct drm_i915_private *dev_priv = engine->i915;
@@ -1245,9 +1253,16 @@ static int gen8_init_common_ring(struct intel_engine_cs 
*engine)
  
  	DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
  
-	/* After a GPU reset, we may have requests to replay */

+   GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
+
+   /* Clear any pending interrupt state */
+   I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
+   I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);


Clear twice? Or the second was supposed to be user_interrupt?


clear_bit(ENGINE_IRQ_EXECLIST, >irq_posted);
  
+	/* After a GPU reset, we may have requests to replay */

submit = false;
for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) {
if (!port_isset([n]))


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

[Intel-gfx] ✓ Fi.CI.BAT: success for tests/perf: fix userspace configs issues on HSW

[Patchwork]

== Series Details ==

Series: tests/perf: fix userspace configs issues on HSW
URL   : https://patchwork.freedesktop.org/series/28463/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
79d6f77fa1ff33f198d954a3c7f1028322fcce52 tests/perf: follow up build fix

with latest DRM-Tip kernel build CI_DRM_2929
96c5eac5f202 drm-tip: 2017y-08m-07d-10h-55m-52s UTC integration manifest

Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:435s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:421s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:364s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:504s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:494s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:528s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:513s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:584s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:431s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:414s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:419s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:501s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:477s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:459s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:573s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:584s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:575s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:450s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:644s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:469s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:426s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:483s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:552s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_28/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4] i915: Add support for drm syncobjs

[Jason Ekstrand]

On Thu, Aug 3, 2017 at 11:58 AM, Chris Wilson 
wrote:

> From: Jason Ekstrand 
>
> This commit adds support for waiting on or signaling DRM syncobjs as
> part of execbuf.  It does so by hijacking the currently unused cliprects
> pointer to instead point to an array of i915_gem_exec_fence structs
> which containe a DRM syncobj and a flags parameter which specifies
> whether to wait on it or to signal it.  This implementation
> theoretically allows for both flags to be set in which case it waits on
> the dma_fence that was in the syncobj and then immediately replaces it
> with the dma_fence from the current execbuf.
>
> v2:
>  - Rebase on new syncobj API
> v3:
>  - Pull everything out into helpers
>  - Do all allocation in gem_execbuffer2
>  - Pack the flags in the bottom 2 bits of the drm_syncobj*
> v4:
>  - Prevent a potential race on syncobj->fence
>
> Testcase: igt/gem_exec_fence/syncobj*
> Signed-off-by: Jason Ekstrand 
> Link: https://patchwork.freedesktop.org/patch/msgid/1499289202-
> 25441-1-git-send-email-jason.ekstr...@intel.com
> Reviewed-by: Chris Wilson 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_drv.c|   3 +-
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 146
> -
>  include/uapi/drm/i915_drm.h|  31 +-
>  3 files changed, 172 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_
> drv.c
> index cc25115c2db7..e55d1224a74f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -388,6 +388,7 @@ static int i915_getparam(struct drm_device *dev, void
> *data,
> case I915_PARAM_HAS_EXEC_FENCE:
> case I915_PARAM_HAS_EXEC_CAPTURE:
> case I915_PARAM_HAS_EXEC_BATCH_FIRST:
> +   case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
> /* For the time being all of these are always true;
>  * if some supported hardware does not have one of these
>  * features this value needs to be provided from
> @@ -2739,7 +2740,7 @@ static struct drm_driver driver = {
>  */
> .driver_features =
> DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> DRIVER_PRIME |
> -   DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC,
> +   DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC |
> DRIVER_SYNCOBJ,
> .release = i915_driver_release,
> .open = i915_driver_open,
> .lastclose = i915_driver_lastclose,
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 5fa44767c29e..c4db64cfcdae 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -32,6 +32,7 @@
>  #include 
>
>  #include 
> +#include 
>  #include 
>
>  #include "i915_drv.h"
> @@ -1884,8 +1885,10 @@ static bool i915_gem_check_execbuffer(struct
> drm_i915_gem_execbuffer2 *exec)
> return false;
>
> /* Kernel clipping was a DRI1 misfeature */
> -   if (exec->num_cliprects || exec->cliprects_ptr)
> -   return false;
> +   if (!(exec->flags & I915_EXEC_FENCE_ARRAY)) {
> +   if (exec->num_cliprects || exec->cliprects_ptr)
> +   return false;
> +   }
>
> if (exec->DR4 == 0x) {
> DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
> @@ -2116,11 +2119,125 @@ eb_select_engine(struct drm_i915_private
> *dev_priv,
> return engine;
>  }
>
> +static void __free_fence_array(struct drm_syncobj **fences, unsigned int
> n)
> +{
> +   while (n--)
> +   drm_syncobj_put(ptr_mask_bits(fences[n], 2));
> +   kvfree(fences);
> +}
> +
> +static struct drm_syncobj **get_fence_array(struct
> drm_i915_gem_execbuffer2 *args,
> +   struct drm_file *file)
> +{
> +   const unsigned int nfences = args->num_cliprects;
> +   struct drm_i915_gem_exec_fence __user *user;
> +   struct drm_syncobj **fences;
> +   unsigned int n;
> +   int err;
> +
> +   if (!(args->flags & I915_EXEC_FENCE_ARRAY))
> +   return NULL;
> +
> +   if (nfences > SIZE_MAX / sizeof(*fences))
> +   return ERR_PTR(-EINVAL);
> +
> +   user = u64_to_user_ptr(args->cliprects_ptr);
> +   if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32)))
> +   return ERR_PTR(-EFAULT);
> +
> +   fences = kvmalloc_array(args->num_cliprects, sizeof(*fences),
> +   __GFP_NOWARN | GFP_TEMPORARY);
> +   if (!fences)
> +   return ERR_PTR(-ENOMEM);
> +
> +   for (n = 0; n < nfences; n++) {
> +   struct drm_i915_gem_exec_fence fence;
> +   struct drm_syncobj *syncobj;
> +
> +   if (__copy_from_user(, 

[Intel-gfx] [PATCH i-g-t] tests/perf: fix userspace configs issues on HSW

[Lionel Landwerlin]

We don't have flex eu counters on HSW, so don't try to program for
thoses.

Reported-by: CI \o/
Fixes: 609cb5e30b4 ("tests/perf: add tests to verify create/destroy userspace 
configs")
Signed-off-by: Lionel Landwerlin 
---
 tests/perf.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/perf.c b/tests/perf.c
index 95d898ee..89f69b4b 100644
--- a/tests/perf.c
+++ b/tests/perf.c
@@ -4197,12 +4197,17 @@ test_create_destroy_userspace_config(void)
 
config.n_mux_regs = 1;
config.mux_regs_ptr = to_user_pointer(mux_regs);
-   for (i = 0; i < ARRAY_SIZE(flex_regs) / 2; i++) {
-   flex_regs[i * 2] = 0xe458; /* EU_PERF_CNTL0 */
-   flex_regs[i * 2 + 1] = 0x0;
+
+   /* Flex EU counters are only available on gen8+ */
+   if (intel_gen(devid) >= 8) {
+   for (i = 0; i < ARRAY_SIZE(flex_regs) / 2; i++) {
+   flex_regs[i * 2] = 0xe458; /* EU_PERF_CNTL0 */
+   flex_regs[i * 2 + 1] = 0x0;
+   }
+   config.flex_regs_ptr = to_user_pointer(flex_regs);
+   config.n_flex_regs = ARRAY_SIZE(flex_regs) / 2;
}
-   config.flex_regs_ptr = to_user_pointer(flex_regs);
-   config.n_flex_regs = ARRAY_SIZE(flex_regs) / 2;
+
config.n_boolean_regs = 0;
 
/* Creating configs without permissions shouldn't work. */
@@ -4283,13 +4288,15 @@ test_whitelisted_registers_userspace_config(void)
}
config.boolean_regs_ptr = (uintptr_t) b_counters_regs;
 
-   /* Flex EU registers */
-   for (i = 0; i < ARRAY_SIZE(flex); i++) {
-   flex_regs[config.n_flex_regs * 2] = flex[i];
-   flex_regs[config.n_flex_regs * 2 + 1] = 0;
-   config.n_flex_regs++;
+   if (intel_gen(devid) >= 8) {
+   /* Flex EU registers, only from Gen8+. */
+   for (i = 0; i < ARRAY_SIZE(flex); i++) {
+   flex_regs[config.n_flex_regs * 2] = flex[i];
+   flex_regs[config.n_flex_regs * 2 + 1] = 0;
+   config.n_flex_regs++;
+   }
+   config.flex_regs_ptr = (uintptr_t) flex_regs;
}
-   config.flex_regs_ptr = (uintptr_t) flex_regs;
 
/* Mux registers (too many of them, just checking bounds) */
i = 0;
-- 
2.13.3

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

Re: [Intel-gfx] [PATCH v3 1/4] drm/i915/psr: Clean-up intel_enable_source_psr1()

[Jim Bride]

On Mon, Aug 07, 2017 at 08:55:00AM -0700, Jim Bride wrote:
> On Fri, Aug 04, 2017 at 10:29:33AM +0300, Jani Nikula wrote:
> > On Thu, 03 Aug 2017, Jim Bride  wrote:
> > > On Fri, Jul 14, 2017 at 12:34:28PM +0300, Jani Nikula wrote:
> > >> On Wed, 12 Jul 2017, Chris Wilson  wrote:
> > >> > Quoting Dhinakaran Pandiyan (2017-07-12 09:47:25)
> > >> >> On Tuesday, July 11, 2017 3:19:53 PM PDT Jim Bride wrote:
> > >> >> > On SKL+ there is a bit in SRD_CTL that software is not supposed to
> > >> >> > modify, but we currently clobber that bit when we enable PSR.  In
> > >> >> > order to preserve the value of that bit, go ahead and read SRD_CTL 
> > >> >> 
> > >> >> And which bit is that?
> > >
> > > Bit 29 (Context restore to PSR Active) in SRD_CTL.  I'll add it to the
> > > commit message.  It's worth noting that the bit is not technically
> > > reserved, but rather that SW is not allowed to change it.
> > >
> > >> >
> > >> > I think we would all be happier with keeping the explicit construction
> > >> > (and a smaller patch) if we used
> > >> >
> > >> >val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_CTL_RSVD_MASK;
> > >> 
> > >> Agreed. Avoid read-modify-write as much as possible.
> > >
> > > I can do this if everyone thinks it's the thing to do, but it
> > > does open us up to a similar class of bug (B-Spec restricting mods
> > > to a bit / bit range after initial support for a platform was added)
> > > in the future.  IMHO the code as written is safer.
> > 
> > Chris' suggestion preserves the restricted bits that must remain the
> > same, while initializing everything else. Instead of only changing the
> > bits we must change, only preserve the bits we must not change. Sorry if
> > I wasn't clear with the "as much as possible" part there.
> 
> I think I followed you.  What I was trying to highlight is that the
> patch as written doesn't touch anything other than what we explicitly
> need to initialize.  While Chris' suggestion is much more terse, it
> leaves us open to another bit being flagged out as 'software
> shouldn't change' and we'd have a similar bug again.  The patch as
> written doesn't expose us to that situation.  I'm happy to go with
> Chris' suggestion if everyone still thinks it's the right thing, but
> I wanted to highlight that it's not entirely equivalent to what was
> in the original patch and in my opinion it's less safe than the
> original patch.


Ok, folks think brevity wins out here, so I'm going to go ahead and
spin a different, stand-alone  patch following Chris' suggestion.
Please disregard this one.

Jim


> > Preserving the restricted bits is a functional change, and the subject
> > of this patch does not reflect that. When I look at the logs, I pretty
> > much expect clean up commits to be non-functional. There are some areas
> > where I'd look the other way, but PSR is something where we must
> > carefully split up the patches and write the commit messages diligently,
> > because I know we will be spending time debugging this code and reading
> > the logs.
> 
> I will remove the word 'clean-up' and reword the subject, independent
> of what we decide relative to the two approaches described above.
> The body of the commit message (IMHO) does a good job (and I'll add
> the specific bit in SRD_CTL to the body also) of describing the
> functional changes that the patch makes.
> 
> Jim
> 
> > BR,
> > Jani.
> > 
> > 
> > 
> > >
> > > Jim
> > >
> > >
> > >> 
> > >> BR,
> > >> Jani.
> > >> 
> > >> 
> > >> -- 
> > >> Jani Nikula, Intel Open Source Technology Center
> > >> ___
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 01/16] drm/i915/guc: Add support for data reporting in GuC responses

[Michel Thierry]

On 8/7/2017 9:14 AM, Michal Wajdeczko wrote:

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.
Currently used actions don't return data that way yet.

v2: fix prohibited space after '~' (Michel)
 update commit message (Daniele)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
  drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
  drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
  drivers/gpu/drm/i915/intel_uc.c   |  5 -
  3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
err = wait_for_response(desc, fence, status);
if (unlikely(err))
return err;
-   if (*status != INTEL_GUC_STATUS_SUCCESS)
+   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
return -EIO;
-   return 0;
+   return INTEL_GUC_RECV_TO_DATA(*status);
  }
  
  /*

@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len)
  {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
-   int err;
+   int ret;
  
  	mutex_lock(>send_mutex);
  
-	err = ctch_send(guc, ctch, action, len, );

-   if (unlikely(err)) {
+   ret = ctch_send(guc, ctch, action, len, );
+   if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
- action[0], err, status);
+ action[0], ret, status);
}
  
  	mutex_unlock(>send_mutex);

-   return err;
+   return ret;
  }
  
  /**

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..367aa65 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
   * command in SS0. The response is distinguishable from a command
   * by the fact that all the MASK bits are set. The remaining bits
   * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
   */
  #define   INTEL_GUC_RECV_MASK ((u32)0xF000)
  #define   INTEL_GUC_RECV_IS_RESPONSE(x)   ((u32)(x) >= 
INTEL_GUC_RECV_MASK)
  #define   INTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT  16
+#define INTEL_GUC_RECV_DATA_MASK   (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~INTEL_GUC_RECV_DATA_MASK)
+#define INTEL_GUC_RECV_TO_DATA(x)  (((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+INTEL_GUC_RECV_DATA_SHIFT)
  
  /* GUC will return status back to SOFT_SCRATCH_O_REG */

  enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..3aa0f44 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
   INTEL_GUC_RECV_MASK,
   INTEL_GUC_RECV_MASK,
   10, 10, );
-   if (status != INTEL_GUC_STATUS_SUCCESS) {
+   if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
/*
 * Either the GuC explicitly returned an error (which
 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+   } else {
+   /* Use data encoded by Guc in status dword as return value */
+   ret = INTEL_GUC_RECV_TO_DATA(status);
}
  
  	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);




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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/renderstate: Sandybridge golden renderstate is b0rked

[Patchwork]

== Series Details ==

Series: drm/i915/renderstate: Sandybridge golden renderstate is b0rked
URL   : https://patchwork.freedesktop.org/series/28461/
State : success

== Summary ==

Series 28461v1 drm/i915/renderstate: Sandybridge golden renderstate is b0rked
https://patchwork.freedesktop.org/api/1.0/series/28461/revisions/1/mbox/

Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:435s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:418s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:359s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:487s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:483s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:522s
fi-byt-n2820 total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:505s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:588s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:435s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:407s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:420s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:516s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:478s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:462s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:568s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:582s
fi-pnv-d510  total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  
time:574s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:448s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:643s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:424s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:551s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC 
integration manifest
cbc4a66dfe91 drm/i915/renderstate: Sandybridge golden renderstate is b0rked

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5336/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/7] drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit

[Michel Thierry]

On 8/7/2017 8:33 AM, Daniel Vetter wrote:

On Thu, Aug 03, 2017 at 12:44:40PM -0700, Michel Thierry wrote:

On 7/20/2017 10:57 AM, Daniel Vetter wrote:

Blocking in a worker is ok, that's what the unbound_wq is for. And it
unifies the paths between the blocking and nonblocking commit, giving
me just one path where I have to implement the deadlock avoidance
trickery in the next patch.

I first tried to implement the following patch without this rework, but
force-completing i915_sw_fence creates some serious challenges around
properly cleaning things up. So wasn't a feasible short-term approach.
Another approach would be to simple keep track of all pending atomic
commit work items and manually queue them from the reset code. With the
caveat that double-queue in case we race with the i915_sw_fence must be
avoided. Given all that, taking the cost of a double schedule in atomic
for the short-term fix is the best approach, but can be changed in the
future of course.

v2: Amend commit message (Chris).

Reviewed-by: Maarten Lankhorst 
Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: Joonas Lahtinen 
Signed-off-by: Daniel Vetter 
---
   drivers/gpu/drm/i915/intel_display.c | 15 +++
   1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 995522e40ec1..f6bd6282d7f7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12394,6 +12394,8 @@ static void intel_atomic_commit_tail(struct 
drm_atomic_state *state)
  unsigned crtc_vblank_mask = 0;
  int i;

+   i915_sw_fence_wait(_state->commit_ready);
+
  drm_atomic_helper_wait_for_dependencies(state);

  if (intel_state->modeset)
@@ -12561,10 +12563,7 @@ intel_atomic_commit_ready(struct i915_sw_fence *fence,

  switch (notify) {
  case FENCE_COMPLETE:
-   if (state->base.commit_work.func)
-   queue_work(system_unbound_wq, >base.commit_work);


I would add a small comment here, because later-on if someone has doubts
(and use git-blame), it won't be visible that something changed (the case
and break were added by the same commit).


Hm, not sure what comment I should put here? Suggestions? Only thing I
could come up with was

/* we do blocking waits in the worker, nothing to do here */

But not sure that adds the information you're looking for.


That sounds good to me, or maybe
"any blocking waits already handled in the worker"

But I think both are ok.

-Michel






  break;
-
  case FENCE_FREE:
  {
  struct intel_atomic_helper *helper =
@@ -12668,14 +12667,14 @@ static int intel_atomic_commit(struct drm_device *dev,
  }

  drm_atomic_state_get(state);
-   INIT_WORK(>commit_work,
- nonblock ? intel_atomic_commit_work : NULL);
+   INIT_WORK(>commit_work, intel_atomic_commit_work);

  i915_sw_fence_commit(_state->commit_ready);
-   if (!nonblock) {
-   i915_sw_fence_wait(_state->commit_ready);
+   if (nonblock)
+   queue_work(system_unbound_wq, >commit_work);
+   else
  intel_atomic_commit_tail(state);
-   }
+

  return 0;
   }


Reviewed-by: Michel Thierry 



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

Re: [Intel-gfx] [PATCH] drm/i915: add register macro definition style guide

[Rodrigo Vivi]

good idea.


On Mon, Aug 7, 2017 at 9:10 AM, Daniel Vetter  wrote:
> On Fri, Aug 04, 2017 at 01:38:36PM +0300, Jani Nikula wrote:
>> This is not to try to force a new style; this is my interpretation of
>> what the most common existing style is.
>>
>> With hopes I don't need to answer so many questions about style going
>> forward.
>>
>> Cc: Daniel Vetter 
>> Signed-off-by: Jani Nikula 
>>
>> ---
>>
>> N.b. only the *interpretation* of the existing style is up for debate
>> here. Proposals to *change* the style going forward can be done in other
>> patches changing this description. However, I doubt the usefulness of
>> such changes, with the possible exception of promoting the use of BIT().
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h | 77 
>> +
>>  1 file changed, 77 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index b2546ade2c45..324cf04d6bfe 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -25,6 +25,83 @@
>>  #ifndef _I915_REG_H_
>>  #define _I915_REG_H_
>>
>> +/*
>
> DOC: section, plus pull it into our kerneldoc?
>
>> + * The i915 register macro definition style guide.
>> + *
>> + * Follow the style described here for new macros, and while changing 
>> existing
>> + * macros. Do not mass change existing definitions just to update the style.
>> + *
>> + * LAYOUT
>> + *
>> + * Keep helper macros near the top. For example, _PIPE() and friends.
>> + *
>> + * Prefix macros that generally should not be used outside of this file with
>> + * underscore '_'. For example, _PIPE() and friends, single instances of
>> + * registers that are defined solely for the use by function-like macros.
>> + *
>> + * Avoid using the underscore prefixed macros outside of this file. There 
>> are
>> + * exceptions, but keep them to a minimum.
>> + *
>> + * There are two basic types of register definitions: Single registers and
>> + * register groups. Register groups are registers which have two or more
>> + * instances, for example one per pipe, port, transcoder, etc. Register 
>> groups
>> + * should be defined using function-like macros.
>> + *
>> + * For single registers, define the register offset first, followed by 
>> register
>> + * contents.
>> + *
>> + * For register groups, define the register instance offsets first, prefixed
>> + * with underscore, followed by a function-like macro choosing the right
>> + * instance based on the parameter, followed by register contents.
>> + *
>> + * Define the register contents from most significant to least significant
>> + * bit. Indent the bit and bit field macros using two extra spaces between
>> + * #define and the macro name.
>
> Maybe note that since hw engineers love to use bit 31 for enabling a block
> this gives some natural ordering.
>
>> + *
>> + * For bit fields, define a _MASK and a _SHIFT macro. Define bit field 
>> contents
>> + * so that they are already shifted in place, and can be directly OR'd. For
>> + * convenience, function-like macros may be used to define bit fields, but 
>> do
>> + * note that the macros may be needed to read as well as write the register
>> + * contents.

I'd mention define as needed if needed...
to avoid people adding many definition for shifts and masks that will
never get used...

Also I prefer the function-like macros to set the bits if we only need
to set but never read back...

>> + *
>> + * Define bits using (1 << N) instead of BIT(N). We may change this in the
>> + * future, but this is the prevailing style.
>> + *
>> + * Group the register and its contents together without blank lines, 
>> separate
>> + * from other registers and their contents with one blank line.
>> + *
>> + * Indent macro values from macro names using TABs. Use braces in macro 
>> values
>> + * as needed to avoid unintended precedence after macro substitution. Use 
>> spaces
>> + * in macro values according to kernel coding style. Use lower case in
>> + * hexadecimal values.
>
> I think we should add:
>
> "Indent register contents macros by an additional space, to set them off
> from the register they are for."

agree.

also maybe to add that between name and content the real tabs are required
as much tab as needed to make sure that all bits defines on that bit
is vertically aligned.

but with or without any change:

Reviewed-by: Rodrigo Vivi 

>
> Feel free to reword/place more suitably.
>
>> + *
>> + * NAMING
>> + *
>> + * Try to name registers according to the specs. If the register name 
>> changes in
>> + * the specs from platform to another, stick to the original name.
>> + *
>> + * Try to re-use existing register macro definitions. Only add new macros 
>> for
>> + * new register offsets, or when the register contents have changed enough 
>> to
>> + * warrant a full redefinition.
>> + *
>> + * When a register or a bit (field) 

[Intel-gfx] [PATCH] drm/i915/renderstate: Sandybridge golden renderstate is b0rked

[Chris Wilson]

In the null state batch, we try to load the CC_STATE_POINTERS, but pass
in a pointer to invalid state for the color-calc and depth-stencil
state. In the rendercopy batch this was imported from, the 1024 offset
used is known to be 64bytes of zeroes. Tweaking the
gen6_null_state_batch to point those two offset at known zeroes however
was not sufficient.

The effect of loading depth-stencil and color-calc causes the golden
renderstate batch to take 10s (yes, ten seconds) on my snb gt1 (i5-2500).

It would be nice if we could drop this incomplete "golden render state"
entirely.

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

diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.c 
b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
index 11c8e7b3dd7c..46cfc405ca30 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen6.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
@@ -107,8 +107,8 @@ static const u32 gen6_null_state_batch[] = {
0x,
0x780e0002,
0x0441,
-   0x0401,
-   0x0401,
+   0x0400,
+   0x0400,
0x78021002,
0x,
0x,
-- 
2.13.3

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc: Support for Guc responses and requests

[Patchwork]

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests
URL   : https://patchwork.freedesktop.org/series/28393/
State : failure

== Summary ==

Series 28393v1 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/1/mbox/

Test drv_hangman:
Subgroup error-state-basic:
pass   -> INCOMPLETE (fi-skl-6260u)
pass   -> INCOMPLETE (fi-skl-6700k)
pass   -> INCOMPLETE (fi-skl-6770hq)
pass   -> INCOMPLETE (fi-skl-gvtdvm)
pass   -> INCOMPLETE (fi-skl-x1585l)
pass   -> INCOMPLETE (fi-bxt-j4205)
pass   -> INCOMPLETE (fi-kbl-7500u)
pass   -> INCOMPLETE (fi-kbl-7560u)
pass   -> INCOMPLETE (fi-kbl-r)
Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705
Test drv_module_reload:
Subgroup basic-reload:
pass   -> DMESG-WARN (fi-glk-2a)
Subgroup basic-no-display:
pass   -> DMESG-WARN (fi-glk-2a)
Subgroup basic-reload-inject:
pass   -> DMESG-WARN (fi-glk-2a)

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:433s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:416s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:363s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:498s
fi-bxt-j4205 total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:526s
fi-byt-n2820 total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:514s
fi-glk-2atotal:279  pass:257  dwarn:3   dfail:0   fail:0   skip:19  
time:588s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:437s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:407s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:426s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:511s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:473s
fi-kbl-7500u total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-7560u total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-kbl-r total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-pnv-d510  total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  
time:579s
fi-skl-6260u total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6700k total:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-6770hqtotal:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-gvtdvmtotal:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-skl-x1585ltotal:6pass:5dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:552s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:410s

96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC 
integration manifest
19a746ca7b28 HAX Enable GuC loading & submission
4d5d579265bd drm/i915/guc: Trace messages from CT while in debug
abccbeafa1ea drm/i915/guc: Enable GuC interrupts when using CT
aff2520a47de drm/i915/guc: Handle default action received over CT
fe52f747c1d8 drm/i915/guc: Prepare to process incoming requests from CT
afafb7904900 drm/i915/guc: Implement response handling in send_ct()
244c0164ba7a drm/i915/guc: Use better name for helper wait function
da67c8b8a7e0 drm/i915/guc: Prepare to handle messages from CT RECV buffer
35b9fa7838f8 drm/i915/guc: Update CT message header definition
96158b0c4fca drm/i915/guc: Create a GuC receive function
e3e4f8f1464c drm/i915/guc: Move flushing the GuC logs outside notification 
handler
f9b68cdc9346 drm/i915/guc: Move Guc notification handling to separate function
4a294751a3dd drm/i915/guc: Implement response handling in send_mmio()
d76483eeffac drm/i915/guc: Add send_and_receive() helper function
f7fe86291307 drm/i915/guc: Prepare send() function to accept bigger response

Re: [Intel-gfx] [PATCH i-g-t] lib: don't hang on blt on snb

[Chris Wilson]

Quoting Daniel Vetter (2017-08-07 17:26:56)
> On Fri, Aug 04, 2017 at 06:05:10PM +0100, Chris Wilson wrote:
> > Quoting Daniel Vetter (2017-08-04 17:07:22)
> > > We now have full (or a lot at least) igt running in beta CI, and snb
> > > blt hangs are really unhappy:
> > > 
> > > - drv_hangman@error-state-capture-blt and gem_exec_capture@capture-blt
> > >   reliably result in insta-machine death when we try to reset the gpu,
> > >   both on the CI snb and the one I have here.
> > > 
> > > - Other testcases also randomly (and sometimes rather rarely) die on
> > >   snb.
> > > 
> > > We can't use the endless batch because that results in a reset failure
> > > and wedged gpu, so also not really better.
> > 
> > It shouldn't be the recursion, but the invalid instruction we use to try
> > and trigger the hang quicker (otherwise hangcheck may see the advancing
> > ACTHD and give us longer to escape the loop).
> > 
> > In gem_exec_capture we shouldn't even need that invalid instruction, we
> > just need the busy batch as we pull the trigger ourselves, and if that
> > fails to reset a simple recursive batch we have some issues to resolve.
> 
> Endless loop for haning results in a reset failure on blt as described in
> the commit message. We end up with a permanent and unrecoverable -EIO,
> which is as deadly to CI as outright killing the machine.

No, it doesn't. snb-gt1 exhibiting the machine death on invalid blt
instruction as reported, after fixes:

Subtest error-state-basic: SUCCESS (0.001s)
Subtest error-state-capture-render: SUCCESS (7.740s)
Subtest error-state-capture-bsd: SUCCESS (6.024s)
Test requirement not met in function test_error_state_capture, file 
drv_hangman.c:187:
Test requirement: gem_has_ring(device, ring_id)
Subtest error-state-capture-bsd1: SKIP (0.000s)
Test requirement not met in function test_error_state_capture, file 
drv_hangman.c:187:
Test requirement: gem_has_ring(device, ring_id)
Subtest error-state-capture-bsd2: SKIP (0.000s)
Subtest error-state-capture-blt: SUCCESS (13.965s)
Test requirement not met in function test_error_state_capture, file 
drv_hangman.c:187:
Test requirement: gem_has_ring(device, ring_id)
Subtest error-state-capture-vebox: SKIP (0.000s)

Subtest capture-render: SUCCESS (0.003s)
Test requirement not met in function __real_main175, file 
gem_exec_capture.c:202:
Test requirement: gem_can_store_dword(fd, e->exec_id | e->flags)
Subtest capture-bsd: SKIP (0.000s)
Test requirement not met in function gem_require_ring, file 
ioctl_wrappers.c:1642:
Test requirement: gem_has_ring(fd, ring)
Subtest capture-bsd1: SKIP (0.000s)
Test requirement not met in function gem_require_ring, file 
ioctl_wrappers.c:1642:
Test requirement: gem_has_ring(fd, ring)
Subtest capture-bsd2: SKIP (0.000s)
Subtest capture-blt: SUCCESS (0.002s)
Test requirement not met in function gem_require_ring, file 
ioctl_wrappers.c:1642:
Test requirement: gem_has_ring(fd, ring)
Subtest capture-vebox: SKIP (0.000s)

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

Re: [Intel-gfx] [PATCH i-g-t] lib: don't hang on blt on snb

[Daniel Vetter]

On Fri, Aug 04, 2017 at 06:05:10PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-08-04 17:07:22)
> > We now have full (or a lot at least) igt running in beta CI, and snb
> > blt hangs are really unhappy:
> > 
> > - drv_hangman@error-state-capture-blt and gem_exec_capture@capture-blt
> >   reliably result in insta-machine death when we try to reset the gpu,
> >   both on the CI snb and the one I have here.
> > 
> > - Other testcases also randomly (and sometimes rather rarely) die on
> >   snb.
> > 
> > We can't use the endless batch because that results in a reset failure
> > and wedged gpu, so also not really better.
> 
> It shouldn't be the recursion, but the invalid instruction we use to try
> and trigger the hang quicker (otherwise hangcheck may see the advancing
> ACTHD and give us longer to escape the loop).
> 
> In gem_exec_capture we shouldn't even need that invalid instruction, we
> just need the busy batch as we pull the trigger ourselves, and if that
> fails to reset a simple recursive batch we have some issues to resolve.

Endless loop for haning results in a reset failure on blt as described in
the commit message. We end up with a permanent and unrecoverable -EIO,
which is as deadly to CI as outright killing the machine.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix I915_EXEC_RING_MASK

[Daniel Vetter]

On Mon, Aug 07, 2017 at 12:43:58PM +0100, Chris Wilson wrote:
> This was supposed to be a mask of all known rings, but it is being used
> by execbuffer to filter out invalid rings, and so is instead mapping high
> unused values onto valid rings. Instead of a mask of all known rings,
> we need it to be the mask of all possible rings.
> 
> Fixes: 549f7365820a ("drm/i915: Enable SandyBridge blitter ring")
> Fixes: de1add360522 ("drm/i915: Decouple execbuf uAPI from internal 
> implementation")
> Signed-off-by: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Cc:  # v4.6+

Do we have an evil gem_exec_params subtest that blows up here? Since it
aliases as-is, should be easy to write one ...
-Daniel
> ---
>  include/uapi/drm/i915_drm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index e6b8ae712ae3..b20e41fd2062 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -907,7 +907,7 @@ struct drm_i915_gem_execbuffer2 {
>   * struct drm_i915_gem_exec_fence *fences.
>   */
>   __u64 cliprects_ptr;
> -#define I915_EXEC_RING_MASK  (7<<0)
> +#define I915_EXEC_RING_MASK  (0x3f)
>  #define I915_EXEC_DEFAULT(0<<0)
>  #define I915_EXEC_RENDER (1<<0)
>  #define I915_EXEC_BSD(2<<0)
> -- 
> 2.13.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms: increase max threshold time for edid read

[Daniel Vetter]

On Fri, Aug 04, 2017 at 11:23:18AM -0700, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor 
> 
> Current 50ms max threshold timing for an EDID read is very close to the
> actual time for a 2 block HDMI EDID read of 48ms. Any delay like a clock
> stretch by the EDID eeprom will cause this test to fail. A 4 block HDMI
> EDID read takes approximately 88ms under nominal conditions, making the max
> threshold 95ms will allow this test to pass regardless of monitor attached.
> 
> Signed-off-by: Clint Taylor 

Per internal mail, this needs to be runtime adjusted to fit the EDID we're
reading. Maybe 30ms per edid block.

Thanks, Daniel

> ---
>  tests/kms_sysfs_edid_timing.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> index 1201388..b45e080 100644
> --- a/tests/kms_sysfs_edid_timing.c
> +++ b/tests/kms_sysfs_edid_timing.c
> @@ -27,14 +27,14 @@
>  #include 
>  
>  #define THRESHOLD_PER_CONNECTOR  10
> -#define THRESHOLD_TOTAL  50
> +#define THRESHOLD_TOTAL  95
>  #define CHECK_TIMES  15
>  
>  IGT_TEST_DESCRIPTION("This check the time we take to read the content of all 
> "
>"the possible connectors. Without the edid -ENXIO patch "
>
> "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> -  "we sometimes take a *really* long time. "
> -  "So let's just check for some reasonable timing here");
> +  "we sometimes take a *really* long time. So let's just "
> +  "check an approximate HDMI 4 block edid read timing here");
>  
>  
>  igt_simple_main
> -- 
> 1.9.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: convert macros to functions

[Daniel Vetter]

On Fri, Aug 04, 2017 at 03:30:30PM -0300, Paulo Zanoni wrote:
> Em Sex, 2017-08-04 às 18:21 +0200, Daniel Vetter escreveu:
> > I guess this was done to have a better indication of which testcase
> > and function failed, but igt nowadays dumps an entire stacktrace.
> 
> But we may have multiple do_assertions() calls in a single function.
> 
> >  And
> > macros of this magnitude mean the line number is entirely
> > meaningless,
> > since it doesn't point at a specific check.
> 
> False. It always points to a do_assertions() call, which is what
> matters.
> 
> > 
> > Reason I've started to looking into this is that in our full igt CI
> > runs we have a serious problem with the fbc testcases randomly
> > failing with
> > 
> > (kms_frontbuffer_tracking:1609) CRITICAL: Test assertion failure
> > function enable_prim_screen_and_wait, file
> > kms_frontbuffer_tracking.c:1771:
> 
> https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tests/kms_fr
> ontbuffer_tracking.c#n1771
> 
> See?

Yeah, but why did it fall over? There's an "Assertion failed: false" and
everything else in your test is non-standard by usual igt test patterns.

You know how to look at this, I had absolutely no idea at all what's going
on here.

> > (kms_frontbuffer_tracking:1609) CRITICAL: Failed assertion: false
> > (kms_frontbuffer_tracking:1609) CRITICAL: FBC disabled
> > 
> > And that's not entirely helpful. Also, macros of this magnitude are
> > just horrible to read.
> 
> NAK. Being a macro instead of a function is extremely helpful and the
> line number always points me to the correct do_assertions() call, at
> least when I run this locally.
> 
> If the line number in the CI system doesn't match what you see in your
> file, then it's a problem either on your side or on the CI side. But I
> don't think that's your problem. I think your problem is that we print
> two different line numbers (1609 and 1771), and you're looking at the
> wrong one. I would totally ACK a patch removing the 1609 one... But I
> don't think that would require patching kms_frontbuffuer_tracking.c.
> 
> If you really really want to change this to a function, can't you try
> to find a way to pass a __LINE__ argument that corresponds to the exact
> line of the do_assertions() call and print it somewhere? Maybe another
> wrapper macro could auto-include __LINE__? But seriously, patch IGT to
> not print those bogus numbers, so you won't be confused next time.

Imo other people than you need to be able to understand tests. This much
macro abuse really doesn't help. I agree that I've been thrown off by
what's really going on here, but the underlying problem stands.

If you're typing macros instead of C functions and the macro very much
looks like a C function, something is wrong and needs to be fixed.

Note that you can sprinkle igt_debug log calls into your test, which would
a) help even more understanding it.
b) also make it clear which assert blew up, because we dump the entire
debug log upon failure.

E.g. you could dump __LINE__ in there too if you want that.

As is this is a mess no one but you understands, and some way we need to
improve this.

Please reconsider your nack.

Thanks, Daniel

> 
> > 
> > Cc: Paulo Zanoni 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  tests/kms_frontbuffer_tracking.c | 166 -
> > --
> >  1 file changed, 84 insertions(+), 82 deletions(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index e03524f1c45b..8d11dc065623 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -1677,88 +1677,90 @@ static int adjust_assertion_flags(const
> > struct test_mode *t, int flags)
> >     return flags;
> >  }
> >  
> > -#define do_crc_assertions(flags, mandatory_sink_crc) do {  
> > \
> > -   int flags__ = (flags);  
> > \
> > -   struct both_crcs crc_;  
> > \
> > -   
> > \
> > -   if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))  
> > \
> > -   break;  
> > \
> > -   
> > \
> > -   collect_crcs(_, mandatory_sink_crc);
> > \
> > -   print_crc("Calculated CRC:", _);
> > \
> > -   
> > \
> > -   igt_assert(wanted_crc); 
> > \
> > -   igt_assert_crc_equal(_.pipe, _crc->pipe);
> > \
> > -   if (mandatory_sink_crc) 
> > \
> > -   assert_sink_crc_equal(_.sink, _crc-
> > >sink); \
> > -   else if (sink_crc.reliable &&   
> > \
> > -    !sink_crc_equal(_.sink, 

[Intel-gfx] [PATCH v2 16/16] HAX Enable GuC loading & submission

[Michal Wajdeczko]

This is just for CI testing, *** DO NOT MERGE ***

Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/i915_params.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index 14e2c2e..4a4c378 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
.verbose_state_checks = 1,
.nuclear_pageflip = 0,
.edp_vswing = 0,
-   .enable_guc_loading = 0,
-   .enable_guc_submission = 0,
+   .enable_guc_loading = 1,
+   .enable_guc_submission = 1,
.guc_log_level = -1,
.guc_firmware_path = NULL,
.huc_firmware_path = NULL,
@@ -221,12 +221,12 @@ MODULE_PARM_DESC(edp_vswing,
 module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 
0400);
 MODULE_PARM_DESC(enable_guc_loading,
"Enable GuC firmware loading "
-   "(-1=auto, 0=never [default], 1=if available, 2=required)");
+   "(-1=auto, 0=never, 1=if available [default], 2=required)");
 
 module_param_named_unsafe(enable_guc_submission, i915.enable_guc_submission, 
int, 0400);
 MODULE_PARM_DESC(enable_guc_submission,
"Enable GuC submission "
-   "(-1=auto, 0=never [default], 1=if available, 2=required)");
+   "(-1=auto, 0=never, 1=if available [default], 2=required)");
 
 module_param_named(guc_log_level, i915.guc_log_level, int, 0400);
 MODULE_PARM_DESC(guc_log_level,
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 11/16] drm/i915/guc: Implement response handling in send_ct()

[Michal Wajdeczko]

GuC may return not only small data encoded in the status dword,
but can also append additional data into the response message.
We will copy this extra data into provided buffer, and use
number of received data dwords as new success return value.

v2: fix timeout and checkpatch warnings (Michal)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 121 
 drivers/gpu/drm/i915/intel_guc_ct.h |   5 ++
 2 files changed, 115 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 235446e..3187bf2 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+   struct list_head link;
+   u32 fence;
+   u32 status;
+   u32 response_len;
+   u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -32,6 +40,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
/* we're using static channel owners */
ct->host_channel.owner = CTB_OWNER_HOST;
+
+   spin_lock_init(>lock);
+   INIT_LIST_HEAD(>pending_requests);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -265,7 +276,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel 
*ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 const u32 *action,
 u32 len /* in dwords */,
-u32 fence)
+u32 fence,
+bool send_response)
 {
struct guc_ct_buffer_desc *desc = ctb->desc;
u32 head = desc->head / 4;  /* in dwords */
@@ -301,6 +313,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 */
header = (len << GUC_CT_MSG_LEN_SHIFT) |
 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+(send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
cmds[tail] = header;
@@ -367,14 +380,49 @@ static int wait_for_desc_update(struct guc_ct_buffer_desc 
*desc,
return err;
 }
 
+/**
+ * Wait for the Guc response.
+ * @req:   pointer to pending request
+ * @status:placeholder for status
+ *
+ * We will update request status from the response message handler.
+ * Returns:
+ * 0 response received (status is valid)
+ * -ETIMEDOUT no response within hardcoded timeout
+ */
+static int wait_for_response_msg(struct ct_request *req, u32 *status)
+{
+   int err;
+
+   /*
+* Fast commands should complete in less than 10us, so sample quickly
+* up to that length of time, then switch to a slower sleep-wait loop.
+* No GuC command should ever take longer than 10ms.
+*/
+#define done INTEL_GUC_RECV_IS_RESPONSE(READ_ONCE(req->status))
+   err = wait_for_us(done, 10);
+   if (err)
+   err = wait_for(done, 10);
+#undef done
+
+   if (unlikely(err))
+   DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+   *status = req->status;
+   return err;
+}
+
 static int ctch_send(struct intel_guc *guc,
 struct intel_guc_ct_channel *ctch,
 const u32 *action,
 u32 len,
-u32 *status)
+u32 *status,
+u32 *response)
 {
struct intel_guc_ct_buffer *ctb = >ctbs[CTB_SEND];
struct guc_ct_buffer_desc *desc = ctb->desc;
+   struct ct_request request;
+   unsigned long flags;
u32 fence;
int err;
 
@@ -383,18 +431,48 @@ static int ctch_send(struct intel_guc *guc,
GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
 
fence = ctch_get_next_fence(ctch);
-   err = ctb_write(ctb, action, len, fence);
+   request.fence = fence;
+   request.status = 0;
+   request.response_len = 0;
+   request.response_buf = response;
+
+   spin_lock_irqsave(>ct.lock, flags);
+   list_add_tail(, >ct.pending_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   err = ctb_write(ctb, action, len, fence, !!response);
if (unlikely(err))
-   return err;
+   goto unlink;
 
intel_guc_notify(guc);
 
-   err = wait_for_desc_update(desc, fence, status);
+   if (response)
+   err = wait_for_response_msg(, status);
+   else
+   err = wait_for_desc_update(desc, fence, status);
if (unlikely(err))
-   return err;
-   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-   return -EIO;
-   return INTEL_GUC_RECV_TO_DATA(*status);
+   goto unlink;
+
+   if 

[Intel-gfx] [PATCH v2 15/16] drm/i915/guc: Trace messages from CT while in debug

[Michal Wajdeczko]

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)

Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 41 +++--
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index e6912be..568fbc0 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG
+#define CT_DEBUG_DRIVER(...)   DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
struct list_head link;
u32 fence;
@@ -69,8 +75,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
 static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
u32 cmds_addr, u32 size, u32 owner)
 {
-   DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
-desc, cmds_addr, size, owner);
+   CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
+   desc, cmds_addr, size, owner);
memset(desc, 0, sizeof(*desc));
desc->addr = cmds_addr;
desc->size = size;
@@ -79,8 +85,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc 
*desc,
 
 static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
 {
-   DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
-desc, desc->head, desc->tail);
+   CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
+   desc, desc->head, desc->tail);
desc->head = 0;
desc->tail = 0;
desc->is_in_error = 0;
@@ -176,7 +182,7 @@ static int ctch_init(struct intel_guc *guc,
err = PTR_ERR(blob);
goto err_vma;
}
-   DRM_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
+   CT_DEBUG_DRIVER("CT: vma base=%#x\n", guc_ggtt_offset(ctch->vma));
 
/* store pointers to desc and cmds */
for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -190,8 +196,8 @@ static int ctch_init(struct intel_guc *guc,
 err_vma:
i915_vma_unpin_and_release(>vma);
 err_out:
-   DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
-ctch->owner, err);
+   CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
+   ctch->owner, err);
return err;
 }
 
@@ -211,8 +217,8 @@ static int ctch_open(struct intel_guc *guc,
int err;
int i;
 
-   DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
-ctch->owner, yesno(ctch_is_open(ctch)));
+   CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
+   ctch->owner, yesno(ctch_is_open(ctch)));
 
if (!ctch->vma) {
err = ctch_init(guc, ctch);
@@ -325,6 +331,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 (send_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+   CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+   4, , 4, ,
+   4*(len - 1), [1]);
+
cmds[tail] = header;
tail = (tail + 1) % size;
 
@@ -500,6 +510,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len,
if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
  action[0], ret, status);
+   } else if (unlikely(ret)) {
+   CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+   action[0], ret, ret);
}
 
mutex_unlock(>send_mutex);
@@ -546,10 +559,12 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
/* beware of buffer wrap case */
if (unlikely(available < 0))
available += size;
+   CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
GEM_BUG_ON(available < 0);
 
data[0] = cmds[head];
head = (head + 1) % size;
+   CT_DEBUG_DRIVER("CT: header %#x\n", data[0]);
 
/* message len with header */
len = ct_header_get_len(data[0]) + 1;
@@ -567,6 +582,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 
*data)
data[i] = cmds[head];
head = (head + 1) % size;
}
+   CT_DEBUG_DRIVER("CT: received %*phn\n", 4*len, data);
 
desc->head = head * 4;
return 0;
@@ -592,12 +608,13 @@ static int guc_handle_response(struct intel_guc *guc, 
const u32 *msg)
DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
status = 

[Intel-gfx] [PATCH v2 13/16] drm/i915/guc: Handle default action received over CT

[Michal Wajdeczko]

With enabled CT, instead of programming SCRATCH 15 register with the
Guc to host message, Guc will send us unsolicited CT message.
Content of the first payload dword of this request has the same
format as data expected in the above scratch register.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 3 +++
 drivers/gpu/drm/i915/intel_uc.c | 7 +++
 drivers/gpu/drm/i915/intel_uc.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index abf2bd8..e6912be 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -664,6 +664,9 @@ static bool guc_process_incoming_requests(struct intel_guc 
*guc)
len = ct_header_get_len(header) + 1; /* also count header dw */
 
switch (action) {
+   case INTEL_GUC_ACTION_DEFAULT:
+   intel_guc_process_default_action(guc, request->data[1]);
+   break;
default:
DRM_ERROR("CT: unexpected request %*phn\n",
  4*len, request->data);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2590769..774d740 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -597,3 +597,10 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg)
+{
+   if (msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER))
+   intel_guc_log_flush(guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 445bca3..478df86 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -230,6 +230,7 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 void intel_guc_notification_handler(struct intel_guc *guc);
+void intel_guc_process_default_action(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 14/16] drm/i915/guc: Enable GuC interrupts when using CT

[Michal Wajdeczko]

We will need them in G2H communication to properly handle
responses and requests from the Guc.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Daniele Ceraolo Spurio 
Cc: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 drivers/gpu/drm/i915/intel_uc.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..509497e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1328,7 +1328,7 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
return 0;
 
-   if (i915.guc_log_level >= 0)
+   if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
 
ctx = dev_priv->kernel_context;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 774d740..0209ad0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -395,7 +395,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
intel_guc_auth_huc(dev_priv);
if (i915.enable_guc_submission) {
-   if (i915.guc_log_level >= 0)
+   if (HAS_GUC_CT(dev_priv) || i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
 
ret = i915_guc_submission_enable(dev_priv);
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 12/16] drm/i915/guc: Prepare to process incoming requests from CT

[Michal Wajdeczko]

Requests are read from CT in the irq handler, but actual processing
will be done in the work thread. Processing of specific actions will
be added in the upcoming patches.

v2: don't use GEM_BUG_ON (Chris)
don't kmalloc too large buffer (Michal)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 71 -
 drivers/gpu/drm/i915/intel_guc_ct.h |  4 +++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 3187bf2..abf2bd8 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -32,10 +32,17 @@ struct ct_request {
u32 *response_buf;
 };
 
+struct ct_incoming_request {
+   struct list_head link;
+   u32 data[];
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
 
+static void ct_worker_func(struct work_struct *w);
+
 void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
/* we're using static channel owners */
@@ -43,6 +50,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 
spin_lock_init(>lock);
INIT_LIST_HEAD(>pending_requests);
+   INIT_LIST_HEAD(>incoming_requests);
+   INIT_WORK(>worker, ct_worker_func);
 }
 
 static inline const char *guc_ct_buffer_type_to_str(u32 type)
@@ -608,14 +617,74 @@ static int guc_handle_response(struct intel_guc *guc, 
const u32 *msg)
 static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
 {
u32 header = msg[0];
+   u32 len = ct_header_get_len(header) + 1; /* total len with header */
+   struct ct_incoming_request *request;
+   unsigned long flags;
 
GEM_BUG_ON(ct_header_is_response(header));
/* Request message layout beyond header is request specific */
 
-   /* XXX */
+   request = kmalloc(sizeof(*request) + 4*len, GFP_ATOMIC);
+   if (unlikely(!request)) {
+   DRM_ERROR("CT: dropping request %*phn\n", 4*len, msg);
+   return 0; /* XXX: -ENOMEM ? */
+   }
+   memcpy(request->data, msg, 4*len);
+
+   spin_lock_irqsave(>ct.lock, flags);
+   list_add_tail(>link, >ct.incoming_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   queue_work(system_unbound_wq, >ct.worker);
return 0;
 }
 
+static bool guc_process_incoming_requests(struct intel_guc *guc)
+{
+   unsigned long flags;
+   struct ct_incoming_request *request;
+   bool done;
+   u32 header;
+   u32 action;
+   u32 len;
+
+   spin_lock_irqsave(>ct.lock, flags);
+   request = list_first_entry_or_null(>ct.incoming_requests,
+  struct ct_incoming_request, link);
+   if (request)
+   list_del(>link);
+   done = !!list_empty(>ct.incoming_requests);
+   spin_unlock_irqrestore(>ct.lock, flags);
+
+   if (!request)
+   return true;
+
+   header = request->data[0];
+   action = ct_header_get_action(header);
+   len = ct_header_get_len(header) + 1; /* also count header dw */
+
+   switch (action) {
+   default:
+   DRM_ERROR("CT: unexpected request %*phn\n",
+ 4*len, request->data);
+   break;
+   }
+
+   kfree(request);
+   return done;
+}
+
+static void ct_worker_func(struct work_struct *w)
+{
+   struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+   struct intel_guc *guc = container_of(ct, struct intel_guc, ct);
+   bool done;
+
+   done = guc_process_incoming_requests(guc);
+   if (!done)
+   queue_work(system_unbound_wq, >worker);
+}
+
 static void intel_guc_receive_ct(struct intel_guc *guc)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h 
b/drivers/gpu/drm/i915/intel_guc_ct.h
index 557c1e8..125e004 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -73,6 +73,8 @@ struct intel_guc_ct_channel {
  * @host_channel: main channel used by the host
  * @lock: spin lock for pending requests list
  * @pending_requests: list of pending requests
+ * @incoming_requests: list of incoming requests
+ * @tasklet: tasklet for handling incoming requests
  */
 struct intel_guc_ct {
struct intel_guc_ct_channel host_channel;
@@ -80,6 +82,8 @@ struct intel_guc_ct {
 
spinlock_t lock;
struct list_head pending_requests;
+   struct list_head incoming_requests;
+   struct work_struct worker;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 07/16] drm/i915/guc: Create a GuC receive function

[Michal Wajdeczko]

From: Oscar Mateo 

This function, symmetrical to the send(), will handle Guc2Host message
interrupts (which at the moment still only covers requests to flush
the GuC logs).

Cc: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_uc.c | 18 +-
 drivers/gpu/drm/i915/intel_uc.h |  5 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f1a6af7..2590769 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -109,6 +109,7 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 
mutex_init(>send_mutex);
guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
guc->notify = guc_write_irq_trigger;
 }
 
@@ -315,6 +316,7 @@ static int guc_enable_communication(struct intel_guc *guc)
return intel_guc_enable_ct(guc);
 
guc->send = intel_guc_send_mmio;
+   guc->recv = intel_guc_receive_mmio;
return 0;
 }
 
@@ -326,6 +328,7 @@ static void guc_disable_communication(struct intel_guc *guc)
intel_guc_disable_ct(guc);
 
guc->send = intel_guc_send_nop;
+   guc->recv = intel_guc_receive_nop;
 }
 
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
@@ -466,6 +469,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len,
return -ENODEV;
 }
 
+void intel_guc_receive_nop(struct intel_guc *guc)
+{
+   WARN(1, "Unexpected receive\n");
+}
+
 /*
  * This function implements the MMIO based host to GuC interface.
  */
@@ -533,7 +541,10 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
return ret;
 }
 
-void intel_guc_notification_handler(struct intel_guc *guc)
+/*
+ * This function implements the MMIO based GuC to host interface.
+ */
+void intel_guc_receive_mmio(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
u32 msg, flush;
@@ -566,6 +577,11 @@ void intel_guc_notification_handler(struct intel_guc *guc)
}
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+   guc->recv(guc);
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 9979a22..445bca3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,9 @@ struct intel_guc {
/* GuC's FW specific send function */
int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
+   /* GuC's FW specific receive function */
+   void (*recv)(struct intel_guc *guc);
+
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
 };
@@ -230,6 +233,8 @@ void intel_guc_notification_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
+void intel_guc_receive_nop(struct intel_guc *guc);
+void intel_guc_receive_mmio(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 
len)
 {
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 09/16] drm/i915/guc: Prepare to handle messages from CT RECV buffer

[Michal Wajdeczko]

GuC can respond to our commands not only by updating SEND buffer
descriptor, but can send us message over RECV buffer. Additionally
Guc can also send us unsolicited requests over RECV buffer.
Lets start reading those messages and make placeholders for actual
response/request handlers.

v2: misc improvements (Michal)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 127 
 1 file changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c17cb42..b25f478 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -414,6 +414,131 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len,
return ret;
 }
 
+static inline unsigned int ct_header_get_len(u32 header)
+{
+   return (header >> GUC_CT_MSG_LEN_SHIFT) & GUC_CT_MSG_LEN_MASK;
+}
+
+static inline unsigned int ct_header_get_action(u32 header)
+{
+   return (header >> GUC_CT_MSG_ACTION_SHIFT) & GUC_CT_MSG_ACTION_MASK;
+}
+
+static inline bool ct_header_is_response(u32 header)
+{
+   return !!(header & GUC_CT_MSG_IS_RESPONSE);
+}
+
+static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
+{
+   struct guc_ct_buffer_desc *desc = ctb->desc;
+   u32 head = desc->head / 4;  /* in dwords */
+   u32 tail = desc->tail / 4;  /* in dwords */
+   u32 size = desc->size / 4;  /* in dwords */
+   u32 *cmds = ctb->cmds;
+   s32 available;  /* in dwords */
+   unsigned int len;
+   unsigned int i;
+
+   GEM_BUG_ON(desc->size % 4);
+   GEM_BUG_ON(desc->head % 4);
+   GEM_BUG_ON(desc->tail % 4);
+   GEM_BUG_ON(tail >= size);
+   GEM_BUG_ON(head >= size);
+
+   /* tail == head condition indicates empty */
+   available = tail - head;
+   if (unlikely(available == 0))
+   return -ENODATA;
+
+   /* beware of buffer wrap case */
+   if (unlikely(available < 0))
+   available += size;
+   GEM_BUG_ON(available < 0);
+
+   data[0] = cmds[head];
+   head = (head + 1) % size;
+
+   /* message len with header */
+   len = ct_header_get_len(data[0]) + 1;
+   if (unlikely(len > (u32)available)) {
+   DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n",
+ 4, data,
+ 4 * (head + available - 1 > size ?
+  size - head : available - 1), [head],
+ 4 * (head + available - 1 > size ?
+  available - 1 - size + head : 0), [0]);
+   return -EPROTO;
+   }
+
+   for (i = 1; i < len; i++) {
+   data[i] = cmds[head];
+   head = (head + 1) % size;
+   }
+
+   desc->head = head * 4;
+   return 0;
+}
+
+static int guc_handle_response(struct intel_guc *guc, const u32 *msg)
+{
+   u32 header = msg[0];
+   u32 status = msg[2];
+   u32 len = ct_header_get_len(header) + 1; /* total len with header */
+
+   GEM_BUG_ON(!ct_header_is_response(header));
+   /* Response message shall at least include header, fence and status */
+   if (unlikely(len < 3)) {
+   DRM_ERROR("CT: corrupted response %*phn\n", 4*len, msg);
+   return -EPROTO;
+   }
+   if (unlikely(!INTEL_GUC_RECV_IS_RESPONSE(status))) {
+   DRM_ERROR("CT: corrupted status %*phn\n", 4*len, msg);
+   status = INTEL_GUC_STATUS_GENERIC_FAIL;
+   }
+
+   /* XXX */
+   return 0;
+}
+
+static int guc_handle_request(struct intel_guc *guc, const u32 *msg)
+{
+   u32 header = msg[0];
+
+   GEM_BUG_ON(ct_header_is_response(header));
+   /* Request message layout beyond header is request specific */
+
+   /* XXX */
+   return 0;
+}
+
+static void intel_guc_receive_ct(struct intel_guc *guc)
+{
+   struct intel_guc_ct_channel *ctch = >ct.host_channel;
+   struct intel_guc_ct_buffer *ctb = >ctbs[CTB_RECV];
+   u32 msg[GUC_CT_MSG_LEN_MASK+1]; /* one extra dw for the header */
+   int err = 0;
+
+   if (!ctch_is_open(ctch))
+   return;
+
+   do {
+   err = ctb_read(ctb, msg);
+   if (err)
+   break;
+
+   if (ct_header_is_response(msg[0]))
+   err = guc_handle_response(guc, msg);
+   else
+   err = guc_handle_request(guc, msg);
+   } while (!err);
+
+   if (GEM_WARN_ON(err == -EPROTO)) {
+   DRM_ERROR("CT: corrupted message detected!\n");
+   ctb->desc->is_in_error = 1;
+   }
+}
+
 /**
  * Enable buffer based command transport
  * Shall only be called for 

[Intel-gfx] [PATCH v2 10/16] drm/i915/guc: Use better name for helper wait function

[Michal Wajdeczko]

In next patch we will introduce another way of waiting for the response
that will use RECV buffer. To avoid misleading names, rename old wait
function to reflect the fact that it is based on descriptor update.

v2: fix comment style (Michal)

Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index b25f478..235446e 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -321,16 +321,21 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
return 0;
 }
 
-/* Wait for the response from the GuC.
+/**
+ * Wait for the descriptor update.
+ * @desc:  buffer descriptor
  * @fence: response fence
  * @status:placeholder for status
- * return: 0 response received (status is valid)
- * -ETIMEDOUT no response within hardcoded timeout
- * -EPROTO no response, ct buffer was in error
+ *
+ * Guc will update this descriptor with new fence and status.
+ * Returns:
+ * 0 response received (status is valid)
+ * -ETIMEDOUT no response within hardcoded timeout
+ * -EPROTO no response, ct buffer is in error
  */
-static int wait_for_response(struct guc_ct_buffer_desc *desc,
-u32 fence,
-u32 *status)
+static int wait_for_desc_update(struct guc_ct_buffer_desc *desc,
+   u32 fence,
+   u32 *status)
 {
int err;
 
@@ -384,7 +389,7 @@ static int ctch_send(struct intel_guc *guc,
 
intel_guc_notify(guc);
 
-   err = wait_for_response(desc, fence, status);
+   err = wait_for_desc_update(desc, fence, status);
if (unlikely(err))
return err;
if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 05/16] drm/i915/guc: Move Guc notification handling to separate function

[Michal Wajdeczko]

To allow future code reuse. While here, fix comment style.

Suggested-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Sagar Arun Kamble 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/i915_irq.c | 33 ++---
 drivers/gpu/drm/i915/intel_uc.c | 37 +
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 196caa4..ac69534 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1675,37 +1675,8 @@ static void gen6_rps_irq_handler(struct drm_i915_private 
*dev_priv, u32 pm_iir)
 
 static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 gt_iir)
 {
-   if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT) {
-   /* Sample the log buffer flush related bits & clear them out now
-* itself from the message identity register to minimize the
-* probability of losing a flush interrupt, when there are back
-* to back flush interrupts.
-* There can be a new flush interrupt, for different log buffer
-* type (like for ISR), whilst Host is handling one (for DPC).
-* Since same bit is used in message register for ISR & DPC, it
-* could happen that GuC sets the bit for 2nd interrupt but Host
-* clears out the bit on handling the 1st interrupt.
-*/
-   u32 msg, flush;
-
-   msg = I915_READ(SOFT_SCRATCH(15));
-   flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
-  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
-   if (flush) {
-   /* Clear the message bits that are handled */
-   I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
-
-   /* Handle flush interrupt in bottom half */
-   queue_work(dev_priv->guc.log.runtime.flush_wq,
-  _priv->guc.log.runtime.flush_work);
-
-   dev_priv->guc.log.flush_interrupt_count++;
-   } else {
-   /* Not clearing of unhandled event bits won't result in
-* re-triggering of the interrupt.
-*/
-   }
-   }
+   if (gt_iir & GEN9_GUC_TO_HOST_INT_EVENT)
+   intel_guc_notification_handler(_priv->guc);
 }
 
 static void valleyview_pipestat_irq_ack(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 433a6a3..3d997a3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -533,6 +533,43 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
return ret;
 }
 
+void intel_guc_notification_handler(struct intel_guc *guc)
+{
+   struct drm_i915_private *dev_priv = guc_to_i915(guc);
+   u32 msg, flush;
+
+   /*
+* Sample the log buffer flush related bits & clear them out now
+* itself from the message identity register to minimize the
+* probability of losing a flush interrupt, when there are back
+* to back flush interrupts.
+* There can be a new flush interrupt, for different log buffer
+* type (like for ISR), whilst Host is handling one (for DPC).
+* Since same bit is used in message register for ISR & DPC, it
+* could happen that GuC sets the bit for 2nd interrupt but Host
+* clears out the bit on handling the 1st interrupt.
+*/
+
+   msg = I915_READ(SOFT_SCRATCH(15));
+   flush = msg & (INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED |
+  INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER);
+   if (flush) {
+   /* Clear the message bits that are handled */
+   I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
+
+   /* Handle flush interrupt in bottom half */
+   queue_work(dev_priv->guc.log.runtime.flush_wq,
+   _priv->guc.log.runtime.flush_work);
+
+   dev_priv->guc.log.flush_interrupt_count++;
+   } else {
+   /*
+* Not clearing of unhandled event bits won't result in
+* re-triggering of the interrupt.
+*/
+   }
+}
+
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 482dfa5..14dbe9f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -226,6 +226,7 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void 

[Intel-gfx] [PATCH v2 08/16] drm/i915/guc: Update CT message header definition

[Michal Wajdeczko]

Flags bits are different in G2H message.

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Cc: Kelvin Gardiner 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 367aa65..89781d3 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -358,17 +358,24 @@ struct guc_ct_buffer_desc {
  *
  * bit[4..0]   message len (in dwords)
  * bit[7..5]   reserved
+ * bit[10..8]  flags
+ * bit[15..11] reserved
+ * bit[31..16] action code
+ *
+ * H2G flags:
  * bit[8]  write fence to desc
  * bit[9]  write status to H2G buff
  * bit[10] send status (via G2H)
- * bit[15..11] reserved
- * bit[31..16] action code
+ *
+ * G2H flags:
+ * bit[8]  is response
  */
 #define GUC_CT_MSG_LEN_SHIFT   0
 #define GUC_CT_MSG_LEN_MASK0x1F
 #define GUC_CT_MSG_WRITE_FENCE_TO_DESC (1 << 8)
 #define GUC_CT_MSG_WRITE_STATUS_TO_BUFF(1 << 9)
 #define GUC_CT_MSG_SEND_STATUS (1 << 10)
+#define GUC_CT_MSG_IS_RESPONSE (1 << 8)
 #define GUC_CT_MSG_ACTION_SHIFT16
 #define GUC_CT_MSG_ACTION_MASK 0x
 
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 06/16] drm/i915/guc: Move flushing the GuC logs outside notification handler

[Michal Wajdeczko]

From: Oscar Mateo 

To allow future code reuse.

Cc: Sagar Arun Kamble 
Cc: Daniele Ceraolo Spurio 
Signed-off-by: Oscar Mateo 
Signed-off-by: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/intel_guc_log.c | 8 
 drivers/gpu/drm/i915/intel_uc.c  | 6 +-
 drivers/gpu/drm/i915/intel_uc.h  | 1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..acd9a3f 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -520,6 +520,14 @@ static void guc_flush_logs(struct intel_guc *guc)
guc_log_capture_logs(guc);
 }
 
+void intel_guc_log_flush(struct intel_guc *guc)
+{
+   /* Handle flush interrupt in bottom half */
+   queue_work(guc->log.runtime.flush_wq, >log.runtime.flush_work);
+
+   guc->log.flush_interrupt_count++;
+}
+
 int intel_guc_log_create(struct intel_guc *guc)
 {
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3d997a3..f1a6af7 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -557,11 +557,7 @@ void intel_guc_notification_handler(struct intel_guc *guc)
/* Clear the message bits that are handled */
I915_WRITE(SOFT_SCRATCH(15), msg & ~flush);
 
-   /* Handle flush interrupt in bottom half */
-   queue_work(dev_priv->guc.log.runtime.flush_wq,
-   _priv->guc.log.runtime.flush_work);
-
-   dev_priv->guc.log.flush_interrupt_count++;
+   intel_guc_log_flush(_priv->guc);
} else {
/*
 * Not clearing of unhandled event bits won't result in
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 14dbe9f..9979a22 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -270,6 +270,7 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc 
*guc, u32 size);
 /* intel_guc_log.c */
 int intel_guc_log_create(struct intel_guc *guc);
 void intel_guc_log_destroy(struct intel_guc *guc);
+void intel_guc_log_flush(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 04/16] drm/i915/guc: Implement response handling in send_mmio()

[Michal Wajdeczko]

In addition to already returned small data encoded in the status MMIO,
GuC may write more additional data in remaining MMIO regs. Lets copy
all that regs into optionally provided response buffer.

v2: new line (Michel)

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Oscar Mateo 
Reviewed-by: Michel Thierry 
---
 drivers/gpu/drm/i915/intel_uc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1bb3bc0..433a6a3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -517,6 +517,12 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len,
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
} else {
+   if (response) {
+   /* Skip reg[0] with the status/response mask */
+   for (i = 1; i < guc->send_regs.count; i++)
+   response[i] = I915_READ(guc_send_reg(guc, i));
+   }
+
/* Use data encoded by Guc in status dword as return value */
ret = INTEL_GUC_RECV_TO_DATA(status);
}
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 03/16] drm/i915/guc: Add send_and_receive() helper function

[Michal Wajdeczko]

In the previous patch we have changed signature of the send function
pointer but we didn't modify signature of the corresponding helper
function to minimize number of required changes. Let's add separate
helper to expose new functionality but still hide underlying details.

v2: enforce response buffer size check (Michal)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_uc.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 53ea5f1..482dfa5 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -235,6 +235,16 @@ static inline int intel_guc_send(struct intel_guc *guc, 
const u32 *action, u32 l
return guc->send(guc, action, len, NULL);
 }
 
+static inline int intel_guc_send_and_receive(struct intel_guc *guc,
+const u32 *action, u32 len,
+u32 *response_buf, u32 size)
+{
+   BUILD_BUG_ON(!__builtin_constant_p(size));
+   BUILD_BUG_ON(size < GUC_CT_MSG_LEN_MASK);
+
+   return guc->send(guc, action, len, response_buf);
+}
+
 static inline void intel_guc_notify(struct intel_guc *guc)
 {
guc->notify(guc);
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 00/16] drm/i915/guc: Support for Guc responses and requests

[Michal Wajdeczko]

With this series we will be able to receive more data from the Guc.
New Guc firmware will be required to actually use that feature.

v2: misc improvements after review + HAX

Michal Wajdeczko (14):
  drm/i915/guc: Add support for data reporting in GuC responses
  drm/i915/guc: Prepare send() function to accept bigger response
  drm/i915/guc: Add send_and_receive() helper function
  drm/i915/guc: Implement response handling in send_mmio()
  drm/i915/guc: Move Guc notification handling to separate function
  drm/i915/guc: Update CT message header definition
  drm/i915/guc: Prepare to handle messages from CT RECV buffer
  drm/i915/guc: Use better name for helper wait function
  drm/i915/guc: Implement response handling in send_ct()
  drm/i915/guc: Prepare to process incoming requests from CT
  drm/i915/guc: Handle default action received over CT
  drm/i915/guc: Enable GuC interrupts when using CT
  drm/i915/guc: Trace messages from CT while in debug
  HAX Enable GuC loading & submission

Oscar Mateo (2):
  drm/i915/guc: Move flushing the GuC logs outside notification handler
  drm/i915/guc: Create a GuC receive function

 drivers/gpu/drm/i915/i915_guc_submission.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c|  33 +--
 drivers/gpu/drm/i915/i915_params.c |   8 +-
 drivers/gpu/drm/i915/intel_guc_ct.c| 389 ++---
 drivers/gpu/drm/i915/intel_guc_ct.h|   9 +
 drivers/gpu/drm/i915/intel_guc_fwif.h  |  17 +-
 drivers/gpu/drm/i915/intel_guc_log.c   |   8 +
 drivers/gpu/drm/i915/intel_uc.c|  75 +-
 drivers/gpu/drm/i915/intel_uc.h|  26 +-
 9 files changed, 488 insertions(+), 79 deletions(-)

-- 
2.7.4

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

[Intel-gfx] [PATCH v2 02/16] drm/i915/guc: Prepare send() function to accept bigger response

[Michal Wajdeczko]

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
Reviewed-by: Michel Thierry 
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 7 ---
 drivers/gpu/drm/i915/intel_uc.c | 6 --
 drivers/gpu/drm/i915/intel_uc.h | 8 
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1249868..c17cb42 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -79,7 +79,7 @@ static int guc_action_register_ct_buffer(struct intel_guc 
*guc,
int err;
 
/* Can't use generic send(), CT registration must go over MMIO */
-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)
DRM_ERROR("CT: register %s buffer failed; err=%d\n",
  guc_ct_buffer_type_to_str(type), err);
@@ -98,7 +98,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc 
*guc,
int err;
 
/* Can't use generic send(), CT deregistration must go over MMIO */
-   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+   err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL);
if (err)
DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
  guc_ct_buffer_type_to_str(type), owner, err);
@@ -395,7 +395,8 @@ static int ctch_send(struct intel_guc *guc,
 /*
  * Command Transport (CT) buffer based GuC send function.
  */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+u32 *response)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3aa0f44..1bb3bc0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
i915_ggtt_disable_guc(dev_priv);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+  u32 *response)
 {
WARN(1, "Unexpected send: action=%#x\n", *action);
return -ENODEV;
@@ -468,7 +469,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 
*action, u32 len)
 /*
  * This function implements the MMIO based host to GuC interface.
  */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+   u32 *response)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
u32 status;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b..53ea5f1 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -206,7 +206,7 @@ struct intel_guc {
struct mutex send_mutex;
 
/* GuC's FW specific send function */
-   int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+   int (*send)(struct intel_guc *guc, const u32 *data, u32 len, u32 *resp);
 
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
@@ -227,12 +227,12 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len, u32 
*response);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 
len)
 {
-   return guc->send(guc, action, len);
+   return guc->send(guc, action, len, NULL);
 }
 
 static inline void intel_guc_notify(struct intel_guc *guc)
-- 
2.7.4

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

[Intel-gfx] [PATCH v2 01/16] drm/i915/guc: Add support for data reporting in GuC responses

[Michal Wajdeczko]

GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.
Currently used actions don't return data that way yet.

v2: fix prohibited space after '~' (Michel)
update commit message (Daniele)

Signed-off-by: Michal Wajdeczko 
Cc: Oscar Mateo 
Cc: Michel Thierry 
Cc: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 14 +++---
 drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++
 drivers/gpu/drm/i915/intel_uc.c   |  5 -
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
err = wait_for_response(desc, fence, status);
if (unlikely(err))
return err;
-   if (*status != INTEL_GUC_STATUS_SUCCESS)
+   if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
return -EIO;
-   return 0;
+   return INTEL_GUC_RECV_TO_DATA(*status);
 }
 
 /*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const 
u32 *action, u32 len)
 {
struct intel_guc_ct_channel *ctch = >ct.host_channel;
u32 status = ~0; /* undefined */
-   int err;
+   int ret;
 
mutex_lock(>send_mutex);
 
-   err = ctch_send(guc, ctch, action, len, );
-   if (unlikely(err)) {
+   ret = ctch_send(guc, ctch, action, len, );
+   if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
- action[0], err, status);
+ action[0], ret, status);
}
 
mutex_unlock(>send_mutex);
-   return err;
+   return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..367aa65 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
  * command in SS0. The response is distinguishable from a command
  * by the fact that all the MASK bits are set. The remaining bits
  * give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
  */
 #defineINTEL_GUC_RECV_MASK ((u32)0xF000)
 #defineINTEL_GUC_RECV_IS_RESPONSE(x)   ((u32)(x) >= 
INTEL_GUC_RECV_MASK)
 #defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT  16
+#define INTEL_GUC_RECV_DATA_MASK   (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x)((x) & ~INTEL_GUC_RECV_DATA_MASK)
+#define INTEL_GUC_RECV_TO_DATA(x)  (((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+INTEL_GUC_RECV_DATA_SHIFT)
 
 /* GUC will return status back to SOFT_SCRATCH_O_REG */
 enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..3aa0f44 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
   INTEL_GUC_RECV_MASK,
   INTEL_GUC_RECV_MASK,
   10, 10, );
-   if (status != INTEL_GUC_STATUS_SUCCESS) {
+   if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
/*
 * Either the GuC explicitly returned an error (which
 * we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 
*action, u32 len)
DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
 " ret=%d status=0x%08X response=0x%08X\n",
 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+   } else {
+   /* Use data encoded by Guc in status dword as return value */
+   ret = INTEL_GUC_RECV_TO_DATA(status);
}
 
intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
-- 
2.7.4

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

Re: [Intel-gfx] [PATCH] drm/i915: remove unused function declaration

[Daniel Vetter]

On Fri, Aug 04, 2017 at 03:03:48PM +0100, Lionel Landwerlin wrote:
> This function is not part of the driver anymore.
> 
> Signed-off-by: Lionel Landwerlin 
Fixes: 90f4fcd56bda ("drm/i915: Remove forced stop ring on suspend/unload")
Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/i915/intel_lrc.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h 
> b/drivers/gpu/drm/i915/intel_lrc.h
> index d02a96a011cf..4ef6a6143f5d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -63,7 +63,6 @@ enum {
>  };
>  
>  /* Logical Rings */
> -void intel_logical_ring_stop(struct intel_engine_cs *engine);
>  void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
>  int logical_render_ring_init(struct intel_engine_cs *engine);
>  int logical_xcs_ring_init(struct intel_engine_cs *engine);
> -- 
> 2.13.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: add register macro definition style guide

[Daniel Vetter]

On Fri, Aug 04, 2017 at 01:38:36PM +0300, Jani Nikula wrote:
> This is not to try to force a new style; this is my interpretation of
> what the most common existing style is.
> 
> With hopes I don't need to answer so many questions about style going
> forward.
> 
> Cc: Daniel Vetter 
> Signed-off-by: Jani Nikula 
> 
> ---
> 
> N.b. only the *interpretation* of the existing style is up for debate
> here. Proposals to *change* the style going forward can be done in other
> patches changing this description. However, I doubt the usefulness of
> such changes, with the possible exception of promoting the use of BIT().
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 77 
> +
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b2546ade2c45..324cf04d6bfe 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -25,6 +25,83 @@
>  #ifndef _I915_REG_H_
>  #define _I915_REG_H_
>  
> +/*

DOC: section, plus pull it into our kerneldoc?

> + * The i915 register macro definition style guide.
> + *
> + * Follow the style described here for new macros, and while changing 
> existing
> + * macros. Do not mass change existing definitions just to update the style.
> + *
> + * LAYOUT
> + *
> + * Keep helper macros near the top. For example, _PIPE() and friends.
> + *
> + * Prefix macros that generally should not be used outside of this file with
> + * underscore '_'. For example, _PIPE() and friends, single instances of
> + * registers that are defined solely for the use by function-like macros.
> + *
> + * Avoid using the underscore prefixed macros outside of this file. There are
> + * exceptions, but keep them to a minimum.
> + *
> + * There are two basic types of register definitions: Single registers and
> + * register groups. Register groups are registers which have two or more
> + * instances, for example one per pipe, port, transcoder, etc. Register 
> groups
> + * should be defined using function-like macros.
> + *
> + * For single registers, define the register offset first, followed by 
> register
> + * contents.
> + *
> + * For register groups, define the register instance offsets first, prefixed
> + * with underscore, followed by a function-like macro choosing the right
> + * instance based on the parameter, followed by register contents.
> + *
> + * Define the register contents from most significant to least significant
> + * bit. Indent the bit and bit field macros using two extra spaces between
> + * #define and the macro name.

Maybe note that since hw engineers love to use bit 31 for enabling a block
this gives some natural ordering.

> + *
> + * For bit fields, define a _MASK and a _SHIFT macro. Define bit field 
> contents
> + * so that they are already shifted in place, and can be directly OR'd. For
> + * convenience, function-like macros may be used to define bit fields, but do
> + * note that the macros may be needed to read as well as write the register
> + * contents.
> + *
> + * Define bits using (1 << N) instead of BIT(N). We may change this in the
> + * future, but this is the prevailing style.
> + *
> + * Group the register and its contents together without blank lines, separate
> + * from other registers and their contents with one blank line.
> + *
> + * Indent macro values from macro names using TABs. Use braces in macro 
> values
> + * as needed to avoid unintended precedence after macro substitution. Use 
> spaces
> + * in macro values according to kernel coding style. Use lower case in
> + * hexadecimal values.

I think we should add:

"Indent register contents macros by an additional space, to set them off
from the register they are for."

Feel free to reword/place more suitably.

> + *
> + * NAMING
> + *
> + * Try to name registers according to the specs. If the register name 
> changes in
> + * the specs from platform to another, stick to the original name.
> + *
> + * Try to re-use existing register macro definitions. Only add new macros for
> + * new register offsets, or when the register contents have changed enough to
> + * warrant a full redefinition.
> + *
> + * When a register or a bit (field) changes for a new platform, prefix the 
> new
> + * macro using the platform acronym or generation. For example, SKL_ or
> + * GEN8_. The prefix signifies the start platform/generation of the register.

s/of/using/

Note that we also have piles of register definitions using platform
postfix. That tends to be used when we have an extension of an existing
register (i.e. for new bit values), instead of a completely new register
set.

Since you want to just describe the current style I think this should be
added.

I'll leave the nits to your judgement, but imo the kerneldoc DOC: section
should be done. With that:

Reviewed-by: Daniel Vetter 

> + *
> + * EXAMPLE
> + *
> + * #define 

Re: [Intel-gfx] [PATCH v4 2/4] drm/i915/psr: Account for sink CRC raciness on some panels

[Jim Bride]

On Fri, Aug 04, 2017 at 06:38:02PM +, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Thu, 2017-08-03 at 11:07 -0700, Rodrigo Vivi wrote:
> > On Tue, Jul 18, 2017 at 2:34 PM, Jim Bride  
> > wrote:
> > > According to the eDP spec, when the count field in TEST_SINK_MISC
> > > increments then the six bytes of sink CRC information in the DPCD
> > > should be valid.  Unfortunately, this doesn't seem to be the case
> > > on some panels, and as a result we get some incorrect and inconsistent
> > > values from the sink CRC DPCD locations at times.  This problem exhibits
> > > itself more on faster processors (relative failure rates HSW < SKL < KBL.)
> > > In order to try and account for this, we try a lot harder to read the sink
> > > CRC until we get consistent values twice in a row before returning what we
> > > read and delay for a time before trying to read.  We still see some
> > > occasional failures, but reading the sink CRC is much more reliable,
> > > particularly on SKL and KBL, with these changes than without.
> 
> I'm curious if we get the correct crc if we waited a full second.

On SKL, times less than a second work fine generally.  On KBL, the
sink CRC is *way* less reliable, and I've seen runs where I set the
retry counts ridiculously high (> 30) and still not received valid
values.


Jim


> > 
> > Is DK now ok with this description?
> > I believe he requested more info here.
> > 
> > >
> > > v2: * Reduce number of retries when reading the sink CRC (Jani)
> > > * Refactor to minimize changes to the code (Jani)
> > > * Rebase
> > > v3: * Rebase
> > > v4: * Switch from do-while to for loop when reading CRC values (Jani)
> > > * Rebase
> > > Cc: Rodrigo Vivi 
> > > Cc: Paulo Zanoni 
> > > Cc: Jani Nikula 
> > > Signed-off-by: Jim Bride 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 33 ++---
> > >  1 file changed, 30 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 2d42d09..c90ca1c 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3906,6 +3906,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, 
> > > u8 *crc)
> > > u8 buf;
> > > int count, ret;
> > > int attempts = 6;
> > > +   u8 old_crc[6];
> > > +
> > > +   if (crc == NULL) {
> > > +   return -ENOMEM;
> > > +   }
> > 
> > wouldn't we drop this check per DK and Jani request?
> > I believe we don't need it, but even if there are cases that we need
> > we could remove the braces..
> > 
> 
> Yeah, crc is allocated on the stack. If that is null, we'll have bigger
> problems to deal with. And I think it's reasonable to assume the caller
> is sending a valid array to fill data.
> 
> > >
> > > ret = intel_dp_sink_crc_start(intel_dp);
> > > if (ret)
> > > @@ -3929,11 +3934,33 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, 
> > > u8 *crc)
> > > goto stop;
> > > }
> > >
> > > -   if (drm_dp_dpcd_read(_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 
> > > 0) {
> > > -   ret = -EIO;
> > > -   goto stop;
> > > +   /*
> > > +* Sometimes it takes a while for the "real" CRC values to land in
> > > +* the DPCD, so try several times until we get two reads in a row
> > > +* that are the same.  If we're an eDP panel, delay between reads
> > > +* for a while since the values take a bit longer to propagate.
> > > +*/
> > > +   for (attempts = 0; attempts < 6; attempts++) {
> > > +   intel_wait_for_vblank(dev_priv, intel_crtc->pipe);
> > 
> > DK, we need vblank wait because the crc calculation also may take one 
> > vblank.
> > usually 2 actually... one to make sure you have the full screen
> > updated and one for the calculation.
> > In the past when we didn't used the count we were waiting 2 vblanks...
> > 
> 
> 
> Thanks for the clarification. My reasoning was, after the first two
> vblank_waits for the sink to calculate crc, the ones in the retry path
> were unnecessary. We just need some delay before reading the dpcd again
> without having to enable vblank interrupts. Anyway, the number of
> retries is low enough that it shouldn't matter.
> 
> On the other hand, since the only consumers of dp sink crc are tests,
> why can't the kernel just dump what it reads to debugfs and let the test
> deal with erroneous results?
> 
> > > +
> > > +   if (drm_dp_dpcd_read(_dp->aux, DP_TEST_CRC_R_CR,
> > > +crc, 6) < 0) {
> > > +   ret = -EIO;
> > > +   break;
> > > +   }
> > > +
> > > +   if (attempts && memcmp(old_crc, crc, 6) == 0)
> > > +   break;
> > > + 

Re: [Intel-gfx] [PATCH v3 1/4] drm/i915/psr: Clean-up intel_enable_source_psr1()

[Jim Bride]

On Fri, Aug 04, 2017 at 10:29:33AM +0300, Jani Nikula wrote:
> On Thu, 03 Aug 2017, Jim Bride  wrote:
> > On Fri, Jul 14, 2017 at 12:34:28PM +0300, Jani Nikula wrote:
> >> On Wed, 12 Jul 2017, Chris Wilson  wrote:
> >> > Quoting Dhinakaran Pandiyan (2017-07-12 09:47:25)
> >> >> On Tuesday, July 11, 2017 3:19:53 PM PDT Jim Bride wrote:
> >> >> > On SKL+ there is a bit in SRD_CTL that software is not supposed to
> >> >> > modify, but we currently clobber that bit when we enable PSR.  In
> >> >> > order to preserve the value of that bit, go ahead and read SRD_CTL 
> >> >> 
> >> >> And which bit is that?
> >
> > Bit 29 (Context restore to PSR Active) in SRD_CTL.  I'll add it to the
> > commit message.  It's worth noting that the bit is not technically
> > reserved, but rather that SW is not allowed to change it.
> >
> >> >
> >> > I think we would all be happier with keeping the explicit construction
> >> > (and a smaller patch) if we used
> >> >
> >> >  val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_CTL_RSVD_MASK;
> >> 
> >> Agreed. Avoid read-modify-write as much as possible.
> >
> > I can do this if everyone thinks it's the thing to do, but it
> > does open us up to a similar class of bug (B-Spec restricting mods
> > to a bit / bit range after initial support for a platform was added)
> > in the future.  IMHO the code as written is safer.
> 
> Chris' suggestion preserves the restricted bits that must remain the
> same, while initializing everything else. Instead of only changing the
> bits we must change, only preserve the bits we must not change. Sorry if
> I wasn't clear with the "as much as possible" part there.

I think I followed you.  What I was trying to highlight is that the
patch as written doesn't touch anything other than what we explicitly
need to initialize.  While Chris' suggestion is much more terse, it
leaves us open to another bit being flagged out as 'software
shouldn't change' and we'd have a similar bug again.  The patch as
written doesn't expose us to that situation.  I'm happy to go with
Chris' suggestion if everyone still thinks it's the right thing, but
I wanted to highlight that it's not entirely equivalent to what was
in the original patch and in my opinion it's less safe than the
original patch.

> Preserving the restricted bits is a functional change, and the subject
> of this patch does not reflect that. When I look at the logs, I pretty
> much expect clean up commits to be non-functional. There are some areas
> where I'd look the other way, but PSR is something where we must
> carefully split up the patches and write the commit messages diligently,
> because I know we will be spending time debugging this code and reading
> the logs.

I will remove the word 'clean-up' and reword the subject, independent
of what we decide relative to the two approaches described above.
The body of the commit message (IMHO) does a good job (and I'll add
the specific bit in SRD_CTL to the body also) of describing the
functional changes that the patch makes.

Jim

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

Re: [Intel-gfx] [PATCH] tests/kms_ccs: Fix the color/ccs surface generation

[Daniel Vetter]

On Fri, Aug 04, 2017 at 07:56:11AM -0700, Jason Ekstrand wrote:
> On August 4, 2017 2:59:56 AM Daniel Stone  wrote:
> 
> > Hi Jason,
> > 
> > On 4 August 2017 at 01:52, Jason Ekstrand  wrote:
> > > Previously, the test used the old 64x64 convention that Ville introduced
> > > for CCS tiles and not the current 128x32 Y-tile convention.  Also, the
> > > original scheme for generating the CCS data was over-complicated and
> > > didn't work correctly because it assumed you could cut the main surface
> > > at an arbitrary Y coordinate.  While you clearly *can* do this (the
> > > hardware does), it's not a good idea for a generator in a test.  The new
> > > scheme, introduced here, is entirely based on the relationship between
> > > cache-lines in the main surface and the CCS that's documented in the
> > > PRM.  By keeping everything CCS cache-line aligned, our chances of
> > > generating correct data for an arbitrary-size surface are much higher.
> > 
> > Thanks for fixing this!
> > 
> > > +* We need to cut the surface on a CCS cache-line 
> > > boundary,
> > > +* otherwise, we're going to be in trouble when we try to
> > > +* generate CCS data for the surface.  A cache line in the
> > > +* CCS is 16x16 cache-line-pairs in the main surface.  16
> > > +* cache lines is 64 rows high.
> > > +*/
> > > +   half_height = ALIGN(height, 128) / 2;
> > > +   half_size = half_height * stride;
> > > +   for (i = 0; i < fb->size / 4; i++) {
> > > +   if (i < half_size / 4)
> > > +   ptr[i] = RED;
> > > +   else
> > > +   ptr[i] = COMPRESSED_RED;
> > 
> > I toyed with:
> > else if (!(i & 0xe))
> > ptr[i] = COMPRESSED_RED;
> > else
> > ptr[i] = BLACK;
> > 
> > ... to make the failure mode even more obvious. But that still writes
> > in (far) more compressed-red pixels than we strictly need for CCS,
> > right? Anyway, follow-up patch.
> 
> Yeah, we can probably do quite a bit of patterning so long as we keep the
> compressed/uncompressed split simple and make sure our pattern works in
> whole cache lines.
> 
> > > +static unsigned int
> > > +y_tile_y_pos(unsigned int offset, unsigned int stride)
> > >  {
> > > -   return ptr +
> > > -   ((y & ~0x3f) * stride) +
> > > -   ((x & ~0x7) * 64) +
> > > -   ((y & 0x3f) * 8) +
> > > -   (x & 7);
> > > +   unsigned int y_tiles, y;
> > > +   y_tiles = (offset / 4096) / (stride / 128);
> > > +   y = y_tiles * 32 + ((offset & 0x1f0) >> 4);
> > > +   return y;
> > >  }
> > > 
> > > @@ -143,12 +168,26 @@ static void display_fb(data_t *data, int compressed)
> > > size[0] = f.pitches[0] * ALIGN(height, 32);
> > > 
> > > if (compressed) {
> > > -   width = ALIGN(f.width, 16) / 16;
> > > -   height = ALIGN(f.height, 8) / 8;
> > > -   f.pitches[1] = ALIGN(width * 1, 64);
> > > +   /* From the Sky Lake PRM, Vol 12, "Color Control Surface":
> > > +*
> > > +*"The compression state of the cache-line pair is
> > > +*specified by 2 bits in the CCS.  Each CCS cache-line
> > > +*represents an area on the main surface of 16x16 sets
> > > +*of 128 byte Y-tiled cache-line-pairs. CCS is always 
> > > Y
> > > +*tiled."
> > > +*
> > > +* A "cache-line-pair" for a Y-tiled surface is two
> > > +* horizontally adjacent cache lines.  When operating in
> > > +* bytes and rows, this gives us a scale-down factor of
> > > +* 32x16.  Since the main surface has a 32-bit format, we
> > > +* need to multiply width by 4 to get bytes.
> > > +*/
> > 
> > Yeah, this code and comment match better (& helped clarify) my
> > understanding of how it works. But given my understanding was mostly
> > gleaned from other, borderline incomprehensible, comments, as well as
> > manually poking bits and observing the result, maybe that's not a
> > ringing endorsement.
> > 
> > > +   width = ALIGN(f.width * 4, 32) / 32;
> > > +   height = ALIGN(f.height, 16) / 16;
> > > +   f.pitches[1] = ALIGN(width * 1, 128);
> > > f.modifier[1] = modifier;
> > > f.offsets[1] = size[0];
> > > -   size[1] = f.pitches[1] * ALIGN(height, 64);
> > > +   size[1] = f.pitches[1] * ALIGN(height, 32);
> > 
> > I changed this to f.height rather than height, because otherwise the
> > kernel was rejecting the aux buffer for being too small.
> 
> Congratulations, you found a bug in the kernel branch you're running.  The
> downsized height 

Re: [Intel-gfx] [PATCH i-g-t v2] kms_rotation_crc: 90 degree flip test is not a stress test

[Daniel Vetter]

On Fri, Aug 04, 2017 at 09:43:41AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> To the best of my recollection the page flipping test was added
> simply to start exercising page flips with 90/270 rotation.
> 
> There is no need to do 60 flips which can take quite some time,
> because we do 60 flips against each pipe and each fb geometry.
> 
> Also, calling this a stress test is also not matching the
> original idea of the test.
> 
> Several changes:
> 
> 1. Remove the stress from the name and reduce the number of
> flips to one only.
> 
> 2. Move the page flip before CRC collection for a more useful
> test.
> 
> 3. Add more flipping tests, for different rotation and sprite
> planes.

I assume you didn't make the test overall slower with this?

> 4. Convert to table driven subtest generation.
> 
> Signed-off-by: Tvrtko Ursulin 
> Cc: Daniel Vetter 
> Cc: Chris Wilson 

Didn't do a full review, but sounds all reasonable. And I assume you've
tested this at least locally (the igt patchwork CI instance doesn't do
full runs ... yet).

Acked-by: Daniel Vetter 
> ---
>  tests/intel-ci/extended.testlist |   2 +-
>  tests/kms_rotation_crc.c | 137 
> ---
>  2 files changed, 85 insertions(+), 54 deletions(-)
> 
> diff --git a/tests/intel-ci/extended.testlist 
> b/tests/intel-ci/extended.testlist
> index 17eed013f810..fb71091758c6 100644
> --- a/tests/intel-ci/extended.testlist
> +++ b/tests/intel-ci/extended.testlist
> @@ -82,7 +82,7 @@ igt@gem_ringfill@vebox-bomb
>  igt@gem_userptr_blits@stress-mm
>  igt@gem_userptr_blits@stress-mm-invalidate-close
>  igt@gem_userptr_blits@stress-mm-invalidate-close-overlap
> -igt@kms_rotation_crc@primary-rotation-90-flip-stress
> +igt@kms_rotation_crc@primary-rotation-90-flip
>  igt@pm_rpm@gem-execbuf-stress
>  igt@pm_rpm@gem-execbuf-stress-extra-wait
>  igt@pm_rpm@gem-execbuf-stress-pc8
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 83e37f126f40..20f1adb67769 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -41,7 +41,7 @@ typedef struct {
>   int pos_y;
>   uint32_t override_fmt;
>   uint64_t override_tiling;
> - unsigned int flip_stress;
> + unsigned int flips;
>  } data_t;
>  
>  static void
> @@ -219,7 +219,7 @@ static void prepare_fbs(data_t *data, igt_output_t 
> *output,
>  
>   igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, >fb);
>  
> - if (data->flip_stress) {
> + if (data->flips) {
>   igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling, 
> >fb_flip);
>   paint_squares(data, IGT_ROTATION_0, >fb_flip, 0.92);
>   }
> @@ -351,15 +351,17 @@ static void test_plane_rotation(data_t *data, int 
> plane_type)
>   ret = igt_display_try_commit2(display, commit);
>   if (data->override_fmt || data->override_tiling) {
>   igt_assert_eq(ret, -EINVAL);
> - } else {
> - igt_assert_eq(ret, 0);
> - igt_pipe_crc_collect_crc(data->pipe_crc,
> -   _output);
> - igt_assert_crc_equal(>ref_crc,
> -   _output);
> + continue;
>   }
>  
> - flip_count = data->flip_stress;
> + /* Verify commit was ok. */
> + igt_assert_eq(ret, 0);
> +
> + /*
> +  * If flips are requested flip away and back before
> +  * checking CRC.
> +  */
> + flip_count = data->flips;
>   while (flip_count--) {
>   ret = drmModePageFlip(data->gfx_fd,
> 
> output->config.crtc->crtc_id,
> @@ -376,6 +378,9 @@ static void test_plane_rotation(data_t *data, int 
> plane_type)
>   igt_assert_eq(ret, 0);
>   wait_for_pageflip(data->gfx_fd);
>   }
> +
> + igt_pipe_crc_collect_crc(data->pipe_crc, _output);
> + igt_assert_crc_equal(>ref_crc, _output);
>   }
>  
>   valid_tests++;
> @@ -569,8 +574,66 @@ err_commit:
>   igt_assert_eq(ret, 0);
>  }
>  
> +static const char *plane_test_str(unsigned plane)
> +{
> + switch (plane) {
> + case DRM_PLANE_TYPE_PRIMARY:
> + return "primary";
> + case DRM_PLANE_TYPE_OVERLAY:
> + return "sprite";
> + case DRM_PLANE_TYPE_CURSOR:
> + return "cursor";
> + default:
> + igt_assert(0);
> + }
> +}
> +
> +static const char 

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Daniel Vetter]

On Mon, Aug 07, 2017 at 10:59:55AM +0100, Chris Wilson wrote:
> Quoting Maarten Lankhorst (2017-08-07 10:45:38)
> > Op 04-08-17 om 09:50 schreef Chris Wilson:
> > > Quoting Maarten Lankhorst (2017-08-02 11:29:17)
> > >> Export 2 functions, igt_signal_helper_get_num and
> > >> igt_signal_helper_get_hz.
> > >>
> > >> This will allow tests to measure how much time in a test was spent
> > >> in a uninterruptible state, which is useful when testing whether
> > >> certain ioctl's can be interrupted or not.
> > > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> > > was interrupted. Refine it to suit your purposes, it is the surgical
> > > scalpel compared to the shotgun of signal_helper.
> > > -Chris
> > 
> > I've been using the igt display helpers now so I don't have to worry about 
> > the ioctl's. Using sig_ioctl would mean having to perform the ioctl's 
> > myself, and that makes the code a lot more verbose..
> 
> You know that we igt uses igt_ioctl so that we can switch the ioctl
> wrapper. What you have here is sig_ioctl so rather than painting the
> shotgun signal helper a different colour, use the interface that
> actually supports what you have in mind.

Might be good to spicy the docs up a bit to make this clearer, maybe even
with an example and some links between the different bits? Mika/Maarten?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/7] drm/i915: More surgically unbreak the modeset vs reset deadlock

[Daniel Vetter]

On Thu, Aug 03, 2017 at 12:35:48PM -0700, Michel Thierry wrote:
> Hi,
> 
> First sorry about the delay...
> 
> On 7/20/2017 10:57 AM, Daniel Vetter wrote:
> > There's no reason to entirely wedge the gpu, for the minimal deadlock
> > bugfix we only need to unbreak/decouple the atomic commit from the gpu
> > reset. The simplest way to fix that is by replacing the
> > unconditional fence wait a the top of commit_tail by a wait which
> > completes either when the fences are done (normal case, or when a
> > reset doesn't need to touch the display state). Or when the gpu reset
> > needs to force-unblock all pending modeset states.
> > 
> > The lesser source of deadlocks is when we try to pin a new framebuffer
> > and run into a stall. There's a bunch of places this can happen, like
> > eviction, changing the caching mode, acquiring a fence on older
> > platforms. And we can't just break the depency loop and keep going,
> > the only way would be to break out and restart. But the problem with
> > that approach is that we must stall for the reset to complete before
> > we grab any locks, and with the atomic infrastructure that's a bit
> > tricky. The only place is the ioctl code, and we don't want to insert
> > code into e.g. the BUSY ioctl. Hence for that problem just create a
> > critical section, and if any code is in there, wedge the GPU. For the
> > steady-state this should never be a problem.
> > 
> > Note that in both cases TDR itself keeps working, so from a userspace
> > pov this trickery isn't observable. Users themselvs might spot a short
> > glitch while the rendering is catching up again, but that's still
> > better than pre-TDR where we've thrown away all the rendering,
> > including innocent batches. Also, this fixes the regression TDR
> > introduced of making gpu resets deadlock-prone when we do need to
> > touch the display.
> > 
> > One thing I noticed is that gpu_error.flags seems to use both our own
> > wait-queue in gpu_error.wait_queue, and the generic wait_on_bit
> > facilities. Not entirely sure why this inconsistency exists, I just
> > picked one style.
> > 
> > A possible future avenue could be to insert the gpu reset in-between
> > ongoing modeset changes, which would avoid the momentary glitch. But
> > that's a lot more work to implement in the atomic commit machinery,
> > and given that we only need this for pre-g4x hw, of questionable
> > utility just for the sake of polishing gpu reset even more on those
> > old boxes. It might be useful for other features though.
> > 
> > v2: Rebase onto 4.13 with a s/wait_queue_t/struct wait_queue_entry/.
> > 
> > v3: Really emabarrassing fixup, I checked the wrong bit and broke the
> > unbreak/wakeup logic.
> > 
> > v4: Also handle deadlocks in pin_to_display.
> > 
> > Cc: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Cc: Joonas Lahtinen 
> > Signed-off-by: Daniel Vetter 
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h  |  3 +++
> >   drivers/gpu/drm/i915/intel_display.c | 45 
> > +++-
> >   2 files changed, 42 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 07e98b07c5bc..9b055deca36d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1505,6 +1505,8 @@ struct i915_gpu_error {
> >  /* Protected by the above dev->gpu_error.lock. */
> >  struct i915_gpu_state *first_error;
> > 
> > +   atomic_t pending_fb_pin;
> > +
> >  unsigned long missed_irq_rings;
> > 
> >  /**
> > @@ -1564,6 +1566,7 @@ struct i915_gpu_error {
> >  unsigned long flags;
> >   #define I915_RESET_BACKOFF 0
> >   #define I915_RESET_HANDOFF 1
> > +#define I915_RESET_MODESET 2
> >   #define I915_WEDGED(BITS_PER_LONG - 1)
> >   #define I915_RESET_ENGINE  (I915_WEDGED - I915_NUM_ENGINES)
> > 
> 
> Since you still need to resend this, could you also update the
> I915_RESET_ENGINE overflow check in i915_handle_error? i.e.:
> 
> ---BUILD_BUG_ON(I915_RESET_HANDOFF >= I915_RESET_ENGINE);
> +++BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
> 
> Not that we would have this problem anytime soon...

Yeah missed that, will fix.

> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index f6bd6282d7f7..63ea8d6b2ebd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2162,6 +2162,8 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer 
> > *fb, unsigned int rotation)
> >   */
> >  intel_runtime_pm_get(dev_priv);
> > 
> > +   atomic_inc(_priv->gpu_error.pending_fb_pin);
> > +
> 
> Do we also need this in intel_overlay_do_put_image? It is also being called
> when the struct_mutex is held.

Indeed I missed that, will fix too and 

Re: [Intel-gfx] [PATCH 2/7] drm/i915: Push i915_sw_fence_wait into the nonblocking atomic commit

[Daniel Vetter]

On Thu, Aug 03, 2017 at 12:44:40PM -0700, Michel Thierry wrote:
> On 7/20/2017 10:57 AM, Daniel Vetter wrote:
> > Blocking in a worker is ok, that's what the unbound_wq is for. And it
> > unifies the paths between the blocking and nonblocking commit, giving
> > me just one path where I have to implement the deadlock avoidance
> > trickery in the next patch.
> > 
> > I first tried to implement the following patch without this rework, but
> > force-completing i915_sw_fence creates some serious challenges around
> > properly cleaning things up. So wasn't a feasible short-term approach.
> > Another approach would be to simple keep track of all pending atomic
> > commit work items and manually queue them from the reset code. With the
> > caveat that double-queue in case we race with the i915_sw_fence must be
> > avoided. Given all that, taking the cost of a double schedule in atomic
> > for the short-term fix is the best approach, but can be changed in the
> > future of course.
> > 
> > v2: Amend commit message (Chris).
> > 
> > Reviewed-by: Maarten Lankhorst 
> > Cc: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Cc: Joonas Lahtinen 
> > Signed-off-by: Daniel Vetter 
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 15 +++
> >   1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 995522e40ec1..f6bd6282d7f7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12394,6 +12394,8 @@ static void intel_atomic_commit_tail(struct 
> > drm_atomic_state *state)
> >  unsigned crtc_vblank_mask = 0;
> >  int i;
> > 
> > +   i915_sw_fence_wait(_state->commit_ready);
> > +
> >  drm_atomic_helper_wait_for_dependencies(state);
> > 
> >  if (intel_state->modeset)
> > @@ -12561,10 +12563,7 @@ intel_atomic_commit_ready(struct i915_sw_fence 
> > *fence,
> > 
> >  switch (notify) {
> >  case FENCE_COMPLETE:
> > -   if (state->base.commit_work.func)
> > -   queue_work(system_unbound_wq, 
> > >base.commit_work);
> 
> I would add a small comment here, because later-on if someone has doubts
> (and use git-blame), it won't be visible that something changed (the case
> and break were added by the same commit).

Hm, not sure what comment I should put here? Suggestions? Only thing I
could come up with was

/* we do blocking waits in the worker, nothing to do here */

But not sure that adds the information you're looking for.
-Daniel

> 
> >  break;
> > -
> >  case FENCE_FREE:
> >  {
> >  struct intel_atomic_helper *helper =
> > @@ -12668,14 +12667,14 @@ static int intel_atomic_commit(struct drm_device 
> > *dev,
> >  }
> > 
> >  drm_atomic_state_get(state);
> > -   INIT_WORK(>commit_work,
> > - nonblock ? intel_atomic_commit_work : NULL);
> > +   INIT_WORK(>commit_work, intel_atomic_commit_work);
> > 
> >  i915_sw_fence_commit(_state->commit_ready);
> > -   if (!nonblock) {
> > -   i915_sw_fence_wait(_state->commit_ready);
> > +   if (nonblock)
> > +   queue_work(system_unbound_wq, >commit_work);
> > +   else
> >  intel_atomic_commit_tail(state);
> > -   }
> > +
> > 
> >  return 0;
> >   }
> 
> Reviewed-by: Michel Thierry 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait timeout to 5s

[Paulo Zanoni]

Em Seg, 2017-08-07 às 06:51 +, Lofstedt, Marta escreveu:
> > -Original Message-
> > From: Zanoni, Paulo R
> > Sent: Friday, August 4, 2017 9:56 PM
> > To: Lofstedt, Marta ; intel-
> > g...@lists.freedesktop.org
> > Cc: Latvala, Petri 
> > Subject: Re: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase
> > FBC wait
> > timeout to 5s
> > 
> > Em Sex, 2017-08-04 às 09:47 +, Lofstedt, Marta escreveu:
> > > +Paolo
> > > 
> > > > -Original Message-
> > > > From: Lofstedt, Marta
> > > > Sent: Wednesday, June 28, 2017 2:17 PM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Latvala, Petri ; Lofstedt, Marta
> > > > 
> > > > Subject: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase
> > > > FBC
> > > > wait timeout to 5s
> > > > 
> > > > The subtests: igt@kms_frontbuffer_tracking@fbc-*draw*
> > > > has non-consistent results, pending between fail and pass.
> > > > The fails are always due to "FBC disabled".
> > > > With this increase in timeout the flip-flop behavior is no
> > > > longer
> > > > reproducible.
> > 
> > This is a partial revert of:
> > 
> > 64590c7b768dc8d8dd962f812d5ff5a39e7e8b54
> > kms_frontbuffer_tracking: reduce the FBC wait timeout to 2s
> > 
> > (but there's no need to make it a full revert if you don't need)
> > 
> > It would be nice to investigate why we're needing 5 seconds instead
> > of
> > 2 now, the document it in the commit message. Also document that
> > this is a
> > partial revert.
> 
> Paulo, do you have data backing up that 2 seconds was ever OK, I fail
> ~1/10 on various fbc subtests. 

All the data I have is the commit message of 64590c7b and the testing I
did. I would imagine something changed in the upstream tree since then,
causing this to need a longer timeout, that's why I suggested
investigating.

> 
> /Marta
> > 
> > Acked-by: Paulo Zanoni 
> > 
> 
> Thanks,
> 
> > > > 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101623
> > > > Signed-off-by: Marta Lofstedt 
> > > > ---
> > > >  tests/kms_frontbuffer_tracking.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > > b/tests/kms_frontbuffer_tracking.c
> > > > index c24e4a81..8bec5d5a 100644
> > > > --- a/tests/kms_frontbuffer_tracking.c
> > > > +++ b/tests/kms_frontbuffer_tracking.c
> > > > @@ -923,7 +923,7 @@ static bool fbc_stride_not_supported(void)
> > > > 
> > > >  static bool fbc_wait_until_enabled(void)  {
> > > > -   return igt_wait(fbc_is_enabled(), 2000, 1);
> > > > +   return igt_wait(fbc_is_enabled(), 5000, 1);
> > > >  }
> > > > 
> > > >  static bool psr_wait_until_enabled(void)
> > > > --
> > > > 2.11.0
> > > 
> > > 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt 2/2] lib: Remove illegal instructions from hang injection

[Mika Kuoppala]

Chris Wilson  writes:

> The idea behind using an illegal instruction was to hang the GPU must
> faster than simply using the recursive batch. However, we stopped doing
> so on gen8+ as the CS parser was much laxer and allowed the illegal
> command through but still interpreted the packet length (jumping over
> the recursive batch buffer start that followed). Sandybridge doesn't
> just hang the GPU when it encounters an illegal command on the BLT
> engine, it hangs the machine. That goes above and beyond testing our
> hangcheck + reset, so remove the deadly instructions.
>
> Signed-off-by: Chris Wilson 
> ---
>  lib/igt_gt.c | 25 +
>  1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 6f7daa5e..d5e8b557 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -270,30 +270,15 @@ igt_hang_t igt_hang_ctx(int fd,
>  
>   memset(b, 0xc5, sizeof(b));
>  
> - /*
> -  * We emit invalid command to provoke a gpu hang.
> -  * If that doesn't work, we do bb start loop.
> -  * Note that the bb start aligment is illegal due this.
> -  * But hey, we are here to hang the gpu so whatever works.
> -  * We skip 0xfff on gen9 as it confuses hw in an such a way that
> -  * it will skip over the bb start, causing runaway head and
> -  * thus much slower hang detection.
> -  */

Daydreaming about MI_HALT,

Reviewed-by: Mika Kuoppala 

>   len = 2;
> - if (intel_gen(intel_get_drm_devid(fd)) >= 8) {
> - b[0] = MI_NOOP;
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
>   len++;
> - } else {
> - b[0] = 0x;
> - }
> -
> - b[1] = MI_BATCH_BUFFER_START | (len - 2);
> - b[1+len] = MI_BATCH_BUFFER_END;
> - b[2+len] = MI_NOOP;
> + b[0] = MI_BATCH_BUFFER_START | (len - 2);
> + b[len] = MI_BATCH_BUFFER_END;
> + b[len+1] = MI_NOOP;
>   gem_write(fd, exec.handle, 0, b, sizeof(b));
>  
> - reloc.offset = 8;
> - reloc.delta = 4;
> + reloc.offset = sizeof(uint32_t);
>   reloc.target_handle = exec.handle;
>   reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
>  
> -- 
> 2.13.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [Intel-gfx,1/2] igt/gem_exec_capture: Wait for batch to execute before triggering reset

[Patchwork]

== Series Details ==

Series: series starting with [Intel-gfx,1/2] igt/gem_exec_capture: Wait for 
batch to execute before triggering reset
URL   : https://patchwork.freedesktop.org/series/28452/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
79d6f77fa1ff33f198d954a3c7f1028322fcce52 tests/perf: follow up build fix

with latest DRM-Tip kernel build CI_DRM_2929
96c5eac5f202 drm-tip: 2017y-08m-07d-10h-55m-52s UTC integration manifest

Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:438s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:423s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:361s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:487s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:491s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:526s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:511s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:583s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:424s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:404s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:421s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:506s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:482s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:463s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:567s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:579s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:571s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:446s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:642s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:470s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:424s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:472s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:547s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:417s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_27/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Clear lost context-switch interrupts across reset

[Patchwork]

== Series Details ==

Series: drm/i915: Clear lost context-switch interrupts across reset
URL   : https://patchwork.freedesktop.org/series/28450/
State : success

== Summary ==

Series 28450v1 drm/i915: Clear lost context-switch interrupts across reset
https://patchwork.freedesktop.org/api/1.0/series/28450/revisions/1/mbox/

Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
pass   -> FAIL   (fi-snb-2600) fdo#100215

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:437s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:419s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:358s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:493s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:482s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:517s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:507s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:583s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:432s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:406s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:415s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:499s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:479s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:456s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:568s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:572s
fi-pnv-d510  total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  
time:568s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:444s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:640s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:464s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:426s
fi-skl-x1585ltotal:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:464s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:551s
fi-snb-2600  total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  
time:408s

96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC 
integration manifest
a7c5eee38b41 drm/i915: Clear lost context-switch interrupts across reset

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5334/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH igt 2/2] lib: Remove illegal instructions from hang injection

[Chris Wilson]

The idea behind using an illegal instruction was to hang the GPU must
faster than simply using the recursive batch. However, we stopped doing
so on gen8+ as the CS parser was much laxer and allowed the illegal
command through but still interpreted the packet length (jumping over
the recursive batch buffer start that followed). Sandybridge doesn't
just hang the GPU when it encounters an illegal command on the BLT
engine, it hangs the machine. That goes above and beyond testing our
hangcheck + reset, so remove the deadly instructions.

Signed-off-by: Chris Wilson 
---
 lib/igt_gt.c | 25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 6f7daa5e..d5e8b557 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -270,30 +270,15 @@ igt_hang_t igt_hang_ctx(int fd,
 
memset(b, 0xc5, sizeof(b));
 
-   /*
-* We emit invalid command to provoke a gpu hang.
-* If that doesn't work, we do bb start loop.
-* Note that the bb start aligment is illegal due this.
-* But hey, we are here to hang the gpu so whatever works.
-* We skip 0xfff on gen9 as it confuses hw in an such a way that
-* it will skip over the bb start, causing runaway head and
-* thus much slower hang detection.
-*/
len = 2;
-   if (intel_gen(intel_get_drm_devid(fd)) >= 8) {
-   b[0] = MI_NOOP;
+   if (intel_gen(intel_get_drm_devid(fd)) >= 8)
len++;
-   } else {
-   b[0] = 0x;
-   }
-
-   b[1] = MI_BATCH_BUFFER_START | (len - 2);
-   b[1+len] = MI_BATCH_BUFFER_END;
-   b[2+len] = MI_NOOP;
+   b[0] = MI_BATCH_BUFFER_START | (len - 2);
+   b[len] = MI_BATCH_BUFFER_END;
+   b[len+1] = MI_NOOP;
gem_write(fd, exec.handle, 0, b, sizeof(b));
 
-   reloc.offset = 8;
-   reloc.delta = 4;
+   reloc.offset = sizeof(uint32_t);
reloc.target_handle = exec.handle;
reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
 
-- 
2.13.3

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

[Intel-gfx] [PATCH igt 1/2] igt/gem_exec_capture: Wait for batch to execute before triggering reset

[Chris Wilson]

Signed-off-by: Chris Wilson 
---
 tests/gem_exec_capture.c | 65 
 1 file changed, 49 insertions(+), 16 deletions(-)

diff --git a/tests/gem_exec_capture.c b/tests/gem_exec_capture.c
index f8f43d29..a73ece5d 100644
--- a/tests/gem_exec_capture.c
+++ b/tests/gem_exec_capture.c
@@ -64,9 +64,9 @@ static void capture(int fd, int dir, unsigned ring)
 #define CAPTURE 1
 #define NOCAPTURE 2
 #define BATCH 3
-   struct drm_i915_gem_relocation_entry reloc;
+   struct drm_i915_gem_relocation_entry reloc[2];
struct drm_i915_gem_execbuffer2 execbuf;
-   uint32_t *batch;
+   uint32_t *batch, *seqno;
int i;
 
memset(obj, 0, sizeof(obj));
@@ -76,25 +76,50 @@ static void capture(int fd, int dir, unsigned ring)
obj[NOCAPTURE].handle = gem_create(fd, 4096);
 
obj[BATCH].handle = gem_create(fd, 4096);
-   obj[BATCH].relocs_ptr = (uintptr_t)
-   obj[BATCH].relocation_count = 1;
-
-   memset(, 0, sizeof(reloc));
-   reloc.target_handle = obj[BATCH].handle; /* recurse */
-   reloc.presumed_offset = 0;
-   reloc.offset = sizeof(uint32_t);
-   reloc.delta = 0;
-   reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
-   reloc.write_domain = 0;
+   obj[BATCH].relocs_ptr = (uintptr_t)reloc;
+   obj[BATCH].relocation_count = ARRAY_SIZE(reloc);
+
+   memset(reloc, 0, sizeof(reloc));
+   reloc[0].target_handle = obj[BATCH].handle; /* recurse */
+   reloc[0].presumed_offset = 0;
+   reloc[0].offset = 5*sizeof(uint32_t);
+   reloc[0].delta = 0;
+   reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
+   reloc[0].write_domain = 0;
+
+   reloc[1].target_handle = obj[SCRATCH].handle; /* breadcrumb */
+   reloc[1].presumed_offset = 0;
+   reloc[1].offset = sizeof(uint32_t);
+   reloc[1].delta = 0;
+   reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
+   reloc[1].write_domain = I915_GEM_DOMAIN_RENDER;
+
+   seqno = gem_mmap__wc(fd, obj[SCRATCH].handle, 0, 4096, PROT_READ);
+   gem_set_domain(fd, obj[SCRATCH].handle,
+   I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 
batch = gem_mmap__cpu(fd, obj[BATCH].handle, 0, 4096, PROT_WRITE);
gem_set_domain(fd, obj[BATCH].handle,
I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
 
i = 0;
-   batch[i++] = 0xdeadbeef; /* crashme */
-   batch[i++] = -1;
-   batch[i] = MI_BATCH_BUFFER_START; /* not crashed? try again! */
+   batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
+   if (gen >= 8) {
+   batch[++i] = 0;
+   batch[++i] = 0;
+   } else if (gen >= 4) {
+   batch[++i] = 0;
+   batch[++i] = 0;
+   reloc[1].offset += sizeof(uint32_t);
+   } else {
+   batch[i]--;
+   batch[++i] = 0;
+   }
+   batch[++i] = 0xc0ffee;
+   if (gen < 3)
+   batch[++i] = MI_NOOP;
+
+   batch[++i] = MI_BATCH_BUFFER_START; /* not crashed? try again! */
if (gen >= 8) {
batch[i] |= 1 << 8 | 1;
batch[++i] = 0;
@@ -107,7 +132,7 @@ static void capture(int fd, int dir, unsigned ring)
batch[++i] = 0;
if (gen < 4) {
batch[i] |= 1;
-   reloc.delta = 1;
+   reloc[0].delta = 1;
}
}
munmap(batch, 4096);
@@ -118,10 +143,17 @@ static void capture(int fd, int dir, unsigned ring)
execbuf.flags = ring;
gem_execbuf(fd, );
 
+   /* Wait for the request to start */
+   while (*(volatile uint32_t *)seqno != 0xc0ffee)
+   igt_assert(gem_bo_busy(fd, obj[SCRATCH].handle));
+   munmap(seqno, 4096);
+
/* Check that only the buffer we marked is reported in the error */
igt_force_gpu_reset(fd);
check_error_state(dir, [CAPTURE]);
 
+   gem_sync(fd, obj[BATCH].handle);
+
gem_close(fd, obj[BATCH].handle);
gem_close(fd, obj[NOCAPTURE].handle);
gem_close(fd, obj[CAPTURE].handle);
@@ -167,6 +199,7 @@ igt_main
 
igt_subtest_f("capture-%s", e->name) {
gem_require_ring(fd, e->exec_id | e->flags);
+   igt_require(gem_can_store_dword(fd, e->exec_id | 
e->flags));
capture(fd, dir, e->exec_id | e->flags);
}
}
-- 
2.13.3

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

[Intel-gfx] [PATCH i-g-t] intel_gpu_top: Use drm_open_driver, don't need drm master

[Petri Latvala]

Signed-off-by: Petri Latvala 
---
 tools/intel_gpu_top.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 7848876..3fe77f7 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -513,7 +513,7 @@ int main(int argc, char **argv)
}
 
/* Just to make sure we open the right debugfs files */
-   drm_fd = drm_open_driver_master(DRIVER_INTEL);
+   drm_fd = drm_open_driver(DRIVER_INTEL);
 
/* Grab access to the registers */
intel_register_access_init(pci_dev, 0, drm_fd);
-- 
2.9.3

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

[Intel-gfx] [PATCH] drm/i915: Clear lost context-switch interrupts across reset

[Chris Wilson]

During a global reset, we disable the irq. As we disable the irq, the
hardware may be raising a GT interrupt that we then ignore, leaving it
pending in the GTIIR. After the reset, we then re-enable the irq,
triggering the pending interrupt. However, that interrupt was for the
stale state from before the reset, and the contents of the CSB buffer
are now invalid.

Reported-by: "Dong, Chuanxiao" 
Signed-off-by: Chris Wilson 
Cc: "Dong, Chuanxiao" 
Cc: Tvrtko Ursulin 
Cc: Michal Winiarski 
Cc: Michel Thierry 
---
 drivers/gpu/drm/i915/intel_lrc.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b0738d2b2a7f..bc61948e2601 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1221,6 +1221,14 @@ static int intel_init_workaround_bb(struct 
intel_engine_cs *engine)
return ret;
 }
 
+static u8 gtiir[] = {
+   [RCS] = 0,
+   [BCS] = 0,
+   [VCS] = 1,
+   [VCS2] = 1,
+   [VECS] = 3,
+};
+
 static int gen8_init_common_ring(struct intel_engine_cs *engine)
 {
struct drm_i915_private *dev_priv = engine->i915;
@@ -1245,9 +1253,16 @@ static int gen8_init_common_ring(struct intel_engine_cs 
*engine)
 
DRM_DEBUG_DRIVER("Execlists enabled for %s\n", engine->name);
 
-   /* After a GPU reset, we may have requests to replay */
+   GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
+
+   /* Clear any pending interrupt state */
+   I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
+   I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
clear_bit(ENGINE_IRQ_EXECLIST, >irq_posted);
 
+   /* After a GPU reset, we may have requests to replay */
submit = false;
for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) {
if (!port_isset([n]))
-- 
2.13.3

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Convert gen4- watermarks to atomic. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Convert gen4- watermarks to atomic. (rev2)
URL   : https://patchwork.freedesktop.org/series/23954/
State : failure

== Summary ==

Series 23954v2 drm/i915: Convert gen4- watermarks to atomic.
https://patchwork.freedesktop.org/api/1.0/series/23954/revisions/2/mbox/

Test gem_exec_parallel:
Subgroup basic:
fail   -> PASS   (fi-ilk-650) fdo#101735
Test gem_ringfill:
Subgroup basic-default:
skip   -> PASS   (fi-bsw-n3050) fdo#101915
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass   -> FAIL   (fi-pnv-d510)
Subgroup basic-flip-after-cursor-legacy:
pass   -> FAIL   (fi-pnv-d510)
Subgroup basic-flip-after-cursor-varying-size:
pass   -> FAIL   (fi-pnv-d510)
Subgroup basic-flip-before-cursor-legacy:
pass   -> FAIL   (fi-pnv-d510)
Subgroup basic-flip-before-cursor-varying-size:
pass   -> FAIL   (fi-pnv-d510)
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781
Subgroup basic-flip-vs-wf_vblank:
pass   -> FAIL   (fi-pnv-d510)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#101735 https://bugs.freedesktop.org/show_bug.cgi?id=101735
fdo#101915 https://bugs.freedesktop.org/show_bug.cgi?id=101915
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  
time:439s
fi-bdw-gvtdvmtotal:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  
time:425s
fi-blb-e6850 total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  
time:365s
fi-bsw-n3050 total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  
time:489s
fi-bxt-j4205 total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:501s
fi-byt-j1900 total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  
time:521s
fi-byt-n2820 total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  
time:510s
fi-glk-2atotal:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  
time:583s
fi-hsw-4770  total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:431s
fi-hsw-4770r total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  
time:409s
fi-ilk-650   total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  
time:421s
fi-ivb-3520m total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:509s
fi-ivb-3770  total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:473s
fi-kbl-7500u total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:456s
fi-kbl-7560u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:565s
fi-kbl-r total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:578s
fi-pnv-d510  total:279  pass:215  dwarn:3   dfail:0   fail:6   skip:55  
time:605s
fi-skl-6260u total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:448s
fi-skl-6700k total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  
time:639s
fi-skl-6770hqtotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:458s
fi-skl-gvtdvmtotal:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  
time:427s
fi-skl-x1585ltotal:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  
time:489s
fi-snb-2520m total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  
time:553s
fi-snb-2600  total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  
time:407s

96c5eac5f202920356b34ef2da8a785bb8b4f320 drm-tip: 2017y-08m-07d-10h-55m-52s UTC 
integration manifest
1349492b8137 drm/i915: Rip out legacy watermark infrastructure
0ce836b1f4c0 drm/i915: Kill off intel_crtc_active.
dfbc9cf45b41 drm/i915: Program gen4 watermarks atomically
7c18d8bfbf7c drm/i915: Calculate gen4 watermarks semiatomically.
d9c8a8aea5bf drm/i915: Convert pineview watermarks to atomic, v2.
bcf55b93672f drm/i915: Program gen3- watermarks atomically
eaf653e8ffa8 drm/i915: Calculate gen3- watermarks semi-atomically.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5332/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Fix I915_EXEC_RING_MASK

[Chris Wilson]

This was supposed to be a mask of all known rings, but it is being used
by execbuffer to filter out invalid rings, and so is instead mapping high
unused values onto valid rings. Instead of a mask of all known rings,
we need it to be the mask of all possible rings.

Fixes: 549f7365820a ("drm/i915: Enable SandyBridge blitter ring")
Fixes: de1add360522 ("drm/i915: Decouple execbuf uAPI from internal 
implementation")
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc:  # v4.6+
---
 include/uapi/drm/i915_drm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e6b8ae712ae3..b20e41fd2062 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -907,7 +907,7 @@ struct drm_i915_gem_execbuffer2 {
  * struct drm_i915_gem_exec_fence *fences.
  */
__u64 cliprects_ptr;
-#define I915_EXEC_RING_MASK  (7<<0)
+#define I915_EXEC_RING_MASK  (0x3f)
 #define I915_EXEC_DEFAULT(0<<0)
 #define I915_EXEC_RENDER (1<<0)
 #define I915_EXEC_BSD(2<<0)
-- 
2.13.3

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

Re: [Intel-gfx] [PATCH] tests/kms_cursor_legacy: use 'enum pipe' type instead of 'int'

[Arkadiusz Hiler]

On Wed, Aug 02, 2017 at 07:54:17PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Signed-off-by: Gustavo Padovan 
Reviewed-by: Arkadiusz Hiler 

and pushed, thanks!

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

Re: [Intel-gfx] a potential dead loop in intel_lrc_irq_handler

[Dong, Chuanxiao]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Monday, August 7, 2017 7:13 PM
> To: Dong, Chuanxiao ; intel-
> g...@lists.freedesktop.org; Joonas Lahtinen
> 
> Subject: RE: a potential dead loop in intel_lrc_irq_handler
> 
> Quoting Dong, Chuanxiao (2017-08-07 11:31:57)
> > > -Original Message-
> > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] GPU reset ->
> > > clears CSB head/tail
> > But the GPU reset will make CSB_head = 0 and CSB_tail = 7.
> 
> Experience says otherwise, but the issue of the delayed interrupt is still a
> concern.
> 
> On the per-engine reset path, interrupts are not disabled, so once we disable
> the tasklet and cancel the pending execution across the reset, we should be
> fine.
> 
> It's the full reset path, where we disable the interrupt first, we need to be
> careful about the hw keeping the interrupt around. Certainly we have no
> fundamental reason to disable_irq there, the supposition is that we are safer
> without spurious interrupts we aren't ready to handle. But afaics to put your
> mind at rest all we need is an
> 
>   I915_WRITE(GEN8_GT_IIR[engine->irq_idx],
>  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
>   I915_WRITE(GEN8_GT_IIR[engine->irq_idx],
>  GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
> 
> as we process the reset.
> -Chris

Yes, if we cleared the pending context switch interrupt in IIR, it should be 
fine. Is there a patch which is trying to do so?

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

Re: [Intel-gfx] [PATCH i-g-t] tests/core_auth: set rlimit

[Arkadiusz Hiler]

On Fri, Aug 04, 2017 at 04:38:51PM +0200, Daniel Vetter wrote:
> Some distros have huge rlimits and then the test takes forever, or
> worse oom, or even worse, takse down the entire machine (which is
> shouldn't be able to, but oh well, oom handling in linux).
> 
> Make sure we have a consistent rlimit by adjusting it manually.
> 
> v2: Use the default of 1024 from everywhere except ubuntu.
> 
> Cc: Tomi Sarvela 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
Reviewed-by: Arkadiusz Hiler 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] a potential dead loop in intel_lrc_irq_handler

[Chris Wilson]

Quoting Dong, Chuanxiao (2017-08-07 11:31:57)
> > -Original Message-
> > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> > GPU reset -> clears CSB head/tail
> But the GPU reset will make CSB_head = 0 and CSB_tail = 7.

Experience says otherwise, but the issue of the delayed interrupt is
still a concern.

On the per-engine reset path, interrupts are not disabled, so once we
disable the tasklet and cancel the pending execution across the reset, we
should be fine.

It's the full reset path, where we disable the interrupt first, we need
to be careful about the hw keeping the interrupt around. Certainly we
have no fundamental reason to disable_irq there, the supposition is that
we are safer without spurious interrupts we aren't ready to handle. But
afaics to put your mind at rest all we need is an

I915_WRITE(GEN8_GT_IIR[engine->irq_idx],
   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
I915_WRITE(GEN8_GT_IIR[engine->irq_idx],
   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);

as we process the reset.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/7] drm/i915: Program gen4 watermarks atomically

[Maarten Lankhorst]

We're already calculating the watermarks correctly, now we have to
program them too.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/intel_pm.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f2062f589f41..3484ce40f2b0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2266,20 +2266,20 @@ static int i965_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
return 0;
 }
 
-static void i965_update_wm(struct intel_crtc *crtc)
+static void i965_program_watermarks(struct drm_i915_private *dev_priv)
 {
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   struct intel_crtc *crtc;
+   struct i9xx_wm_state *wm_state = NULL;
int srwm = 1;
int cursor_sr = 16;
bool cxsr_enabled = false;
 
-   crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
-
-   /* Calc sr entries for one plane configs */
crtc = single_enabled_crtc(dev_priv);
-   if (crtc && crtc->wm.active.i9xx.cxsr) {
-   struct i9xx_wm_state *wm_state = >wm.active.i9xx;
+   if (crtc)
+   wm_state = >wm.active.i9xx;
 
+   /* Calc sr entries for one plane configs */
+   if (wm_state && wm_state->cxsr) {
srwm = wm_state->sr.plane;
cursor_sr = wm_state->sr.cursor;
 
@@ -2569,8 +2569,10 @@ static void i9xx_initial_watermarks(struct 
intel_atomic_state *state,
pnv_program_watermarks(dev_priv);
else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
-   else
+   else if (INTEL_GEN(dev_priv) < 4)
i9xx_program_watermarks(dev_priv);
+   else
+   i965_program_watermarks(dev_priv);
mutex_unlock(_priv->wm.wm_mutex);
 }
 
@@ -2589,8 +2591,10 @@ static void i9xx_optimize_watermarks(struct 
intel_atomic_state *state,
pnv_program_watermarks(dev_priv);
else if (INTEL_INFO(dev_priv)->num_pipes == 1)
i845_program_watermarks(intel_crtc);
-   else
+   else if (INTEL_GEN(dev_priv) < 4)
i9xx_program_watermarks(dev_priv);
+   else
+   i965_program_watermarks(dev_priv);
mutex_unlock(_priv->wm.wm_mutex);
 }
 
@@ -9019,7 +9023,8 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
}
} else if (IS_GEN4(dev_priv)) {
dev_priv->display.compute_pipe_wm = i965_compute_pipe_wm;
-   dev_priv->display.update_wm = i965_update_wm;
+   dev_priv->display.initial_watermarks = i9xx_initial_watermarks;
+   dev_priv->display.optimize_watermarks = 
i9xx_optimize_watermarks;
} else if (IS_GEN3(dev_priv)) {
dev_priv->display.compute_pipe_wm = i9xx_compute_pipe_wm;
dev_priv->display.compute_intermediate_wm = 
i9xx_compute_intermediate_wm;
-- 
2.11.0

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

[Intel-gfx] [PATCH 1/7] drm/i915: Calculate gen3- watermarks semi-atomically.

[Maarten Lankhorst]

The gen3 watermark calculations are converted to atomic,
but the wm update calls are still done through the legacy
functions.

This will make it easier to bisect things if they go wrong.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/intel_display.c |   3 +-
 drivers/gpu/drm/i915/intel_drv.h |  14 +++
 drivers/gpu/drm/i915/intel_pm.c  | 231 +--
 3 files changed, 152 insertions(+), 96 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 974d4b577189..234b94cafc7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14709,7 +14709,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
skl_wm_get_hw_state(dev);
} else if (HAS_PCH_SPLIT(dev_priv)) {
ilk_wm_get_hw_state(dev);
-   }
+   } else if (INTEL_GEN(dev_priv) <= 3 && !IS_PINEVIEW(dev_priv))
+   i9xx_wm_get_hw_state(dev);
 
for_each_intel_crtc(dev, crtc) {
u64 put_domains;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f91de9cb9697..d53d34756048 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -547,6 +547,15 @@ struct g4x_wm_state {
bool fbc_en;
 };
 
+struct i9xx_wm_state {
+   uint16_t plane_wm;
+   bool cxsr;
+
+   struct {
+   uint16_t plane;
+   } sr;
+};
+
 struct intel_crtc_wm_state {
union {
struct {
@@ -591,6 +600,9 @@ struct intel_crtc_wm_state {
/* optimal watermarks */
struct g4x_wm_state optimal;
} g4x;
+   struct {
+   struct i9xx_wm_state optimal;
+   } i9xx;
};
 
/*
@@ -823,6 +835,7 @@ struct intel_crtc {
struct intel_pipe_wm ilk;
struct vlv_wm_state vlv;
struct g4x_wm_state g4x;
+   struct i9xx_wm_state i9xx;
} active;
} wm;
 
@@ -1845,6 +1858,7 @@ void gen6_rps_boost(struct drm_i915_gem_request *rq,
struct intel_rps_client *rps);
 void intel_queue_rps_boost_for_request(struct drm_i915_gem_request *req);
 void g4x_wm_get_hw_state(struct drm_device *dev);
+void i9xx_wm_get_hw_state(struct drm_device *dev);
 void vlv_wm_get_hw_state(struct drm_device *dev);
 void ilk_wm_get_hw_state(struct drm_device *dev);
 void skl_wm_get_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6e393b217450..807c9a730020 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2224,89 +2224,154 @@ static void i965_update_wm(struct intel_crtc 
*unused_crtc)
 
 #undef FW_WM
 
-static void i9xx_update_wm(struct intel_crtc *unused_crtc)
+static const struct intel_watermark_params *i9xx_get_wm_info(struct 
drm_i915_private *dev_priv,
+struct intel_crtc 
*crtc)
 {
-   struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
-   const struct intel_watermark_params *wm_info;
-   uint32_t fwater_lo;
-   uint32_t fwater_hi;
-   int cwm, srwm = 1;
-   int fifo_size;
-   int planea_wm, planeb_wm;
-   struct intel_crtc *crtc, *enabled = NULL;
+   struct intel_plane *plane = to_intel_plane(crtc->base.primary);
 
if (IS_I945GM(dev_priv))
-   wm_info = _wm_info;
+   return _wm_info;
else if (!IS_GEN2(dev_priv))
-   wm_info = _wm_info;
+   return _wm_info;
+   else if (plane->plane == PLANE_A)
+   return _a_wm_info;
else
-   wm_info = _a_wm_info;
+   return _bc_wm_info;
+}
 
-   fifo_size = dev_priv->display.get_fifo_size(dev_priv, 0);
-   crtc = intel_get_crtc_for_plane(dev_priv, 0);
-   if (intel_crtc_active(crtc)) {
+static int i9xx_compute_pipe_wm(struct intel_crtc_state *crtc_state)
+{
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   struct intel_atomic_state *state =
+   to_intel_atomic_state(crtc_state->base.state);
+   struct i9xx_wm_state *wm_state = _state->wm.i9xx.optimal;
+   struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+   const struct drm_plane_state *plane_state = NULL;
+   int fifo_size;
+   const struct intel_watermark_params *wm_info;
+
+   fifo_size = dev_priv->display.get_fifo_size(dev_priv, plane->plane);
+
+   wm_info = i9xx_get_wm_info(dev_priv, crtc);
+
+   wm_state->cxsr = false;
+   memset(_state->sr, 0, sizeof(wm_state->sr));
+
+   if (crtc_state->base.plane_mask & BIT(drm_plane_index(>base)))
+   

[Intel-gfx] [PATCH 3/7] drm/i915: Convert pineview watermarks to atomic, v2.

[Maarten Lankhorst]

Pineview seems to have different watermarks from the other
platforms and are calculated separately.

With the change to atomic, cxsr is automatically disabled for
intermediate watermarks when required. This will fix
fd.org bug #101597 as a nice side effect.

Changes since v1:
- Fix deadlock in pnv_program_watermarks.
- Add link to bug 101597

Signed-off-by: Maarten Lankhorst 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
---
 drivers/gpu/drm/i915/intel_drv.h |   3 +-
 drivers/gpu/drm/i915/intel_pm.c  | 138 ++-
 2 files changed, 94 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 176274d99ee6..d0f1704e1a17 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -553,7 +553,8 @@ struct i9xx_wm_state {
 
struct {
uint16_t plane;
-   } sr;
+   uint16_t cursor;
+   } sr, hpll;
 };
 
 struct intel_crtc_wm_state {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 08fd359307e6..7ecf39815ab5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -830,13 +830,17 @@ static struct intel_crtc *single_enabled_crtc(struct 
drm_i915_private *dev_priv)
return enabled;
 }
 
-static void pineview_update_wm(struct intel_crtc *unused_crtc)
+static int pnv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 {
-   struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
-   struct intel_crtc *crtc;
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   struct i9xx_wm_state *wm_state = _state->wm.i9xx.optimal;
+   struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+   struct intel_atomic_state *state = 
to_intel_atomic_state(crtc_state->base.state);
+   const struct drm_plane_state *primary_plane_state = NULL;
const struct cxsr_latency *latency;
-   u32 reg;
-   unsigned int wm;
+
+   memset(wm_state, 0, sizeof(*wm_state));
 
latency = intel_get_cxsr_latency(IS_PINEVIEW_G(dev_priv),
 dev_priv->is_ddr3,
@@ -844,60 +848,90 @@ static void pineview_update_wm(struct intel_crtc 
*unused_crtc)
 dev_priv->mem_freq);
if (!latency) {
DRM_DEBUG_KMS("Unknown FSB/MEM found, disable CxSR\n");
-   intel_set_memory_cxsr(dev_priv, false);
-   return;
+
+   return 0;
}
 
-   crtc = single_enabled_crtc(dev_priv);
-   if (crtc) {
-   const struct drm_display_mode *adjusted_mode =
-   >config->base.adjusted_mode;
+   if (crtc_state->base.plane_mask & BIT(drm_plane_index(>base)))
+   primary_plane_state = 
__drm_atomic_get_current_plane_state(>base, >base);
+
+   if (primary_plane_state) {
const struct drm_framebuffer *fb =
-   crtc->base.primary->state->fb;
+   primary_plane_state->fb;
int cpp = fb->format->cpp[0];
-   int clock = adjusted_mode->crtc_clock;
+   const struct drm_display_mode *adjusted_mode =
+   _state->base.adjusted_mode;
+   unsigned active_crtcs;
+
+   if (state->modeset)
+   active_crtcs = state->active_crtcs;
+   else
+   active_crtcs = dev_priv->active_crtcs;
+
+   wm_state->cxsr = active_crtcs == drm_crtc_mask(>base);
+
+   wm_state->sr.plane = 
intel_calculate_wm(adjusted_mode->crtc_clock,
+   _display_wm,
+   
pineview_display_wm.fifo_size,
+   cpp, 
latency->display_sr);
+
+   wm_state->sr.cursor = 
intel_calculate_wm(adjusted_mode->crtc_clock,
+_cursor_wm,
+
pineview_display_wm.fifo_size,
+4, latency->cursor_sr);
+
+   wm_state->hpll.plane = 
intel_calculate_wm(adjusted_mode->crtc_clock,
+
_display_hplloff_wm,
+
pineview_display_hplloff_wm.fifo_size,
+cpp, 
latency->display_hpll_disable);
+
+   wm_state->hpll.cursor = 
intel_calculate_wm(adjusted_mode->crtc_clock,
+ 
_cursor_hplloff_wm,
+   

[Intel-gfx] [PATCH 2/7] drm/i915: Program gen3- watermarks atomically

[Maarten Lankhorst]

With the atomic watermark calculations calculate intermediary watermark
values and update the watermarks atomically.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/i915_drv.h  |   5 ++
 drivers/gpu/drm/i915/intel_drv.h |   2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 103 +--
 3 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 907603cba447..3ac17f0f4053 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1774,6 +1774,10 @@ struct g4x_wm_values {
bool fbc_en;
 };
 
+struct i9xx_wm_values {
+   bool cxsr;
+};
+
 struct skl_ddb_entry {
uint16_t start, end;/* in number of blocks, 'end' is exclusive */
 };
@@ -2438,6 +2442,7 @@ struct drm_i915_private {
struct skl_wm_values skl_hw;
struct vlv_wm_values vlv;
struct g4x_wm_values g4x;
+   struct i9xx_wm_values i9xx;
};
 
uint8_t max_level;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53d34756048..176274d99ee6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -601,7 +601,7 @@ struct intel_crtc_wm_state {
struct g4x_wm_state optimal;
} g4x;
struct {
-   struct i9xx_wm_state optimal;
+   struct i9xx_wm_state optimal, intermediate;
} i9xx;
};
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 807c9a730020..08fd359307e6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -439,6 +439,8 @@ bool intel_set_memory_cxsr(struct drm_i915_private 
*dev_priv, bool enable)
dev_priv->wm.vlv.cxsr = enable;
else if (IS_G4X(dev_priv))
dev_priv->wm.g4x.cxsr = enable;
+   else if (INTEL_GEN(dev_priv) <= 4)
+   dev_priv->wm.i9xx.cxsr = enable;
mutex_unlock(_priv->wm.wm_mutex);
 
return ret;
@@ -2315,6 +2317,44 @@ static int i9xx_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
return 0;
 }
 
+static int i9xx_compute_intermediate_wm(struct drm_device *dev,
+  struct intel_crtc *intel_crtc,
+  struct intel_crtc_state *newstate)
+{
+   struct i9xx_wm_state *intermediate = >wm.i9xx.intermediate;
+   const struct drm_crtc_state *old_drm_state =
+   drm_atomic_get_old_crtc_state(newstate->base.state, 
_crtc->base);
+   const struct i9xx_wm_state *old = 
_intel_crtc_state(old_drm_state)->wm.i9xx.optimal;
+   const struct i9xx_wm_state *optimal = >wm.i9xx.optimal;
+
+   /*
+* Start with the final, target watermarks, then combine with the
+* currently active watermarks to get values that are safe both before
+* and after the vblank.
+*/
+   *intermediate = *optimal;
+   if (newstate->disable_cxsr)
+   intermediate->cxsr = false;
+
+   if (!newstate->base.active ||
+   drm_atomic_crtc_needs_modeset(>base))
+   goto out;
+
+   intermediate->plane_wm = min(old->plane_wm, optimal->plane_wm);
+   intermediate->sr.plane = min(old->sr.plane, optimal->sr.plane);
+
+out:
+   /*
+* If our intermediate WM are identical to the final WM, then we can
+* omit the post-vblank programming; only update if it's different.
+*/
+   if (newstate->base.active &&
+   memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
+   newstate->wm.need_postvbl_update = true;
+
+   return 0;
+}
+
 void i9xx_wm_get_hw_state(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -2343,17 +2383,15 @@ void i9xx_wm_get_hw_state(struct drm_device *dev)
}
 }
 
-static void i9xx_update_wm(struct intel_crtc *crtc)
+static void i9xx_program_watermarks(struct drm_i915_private *dev_priv)
 {
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   struct intel_crtc *crtc;
uint32_t fwater_lo;
uint32_t fwater_hi;
int cwm, srwm = -1;
int planea_wm, planeb_wm;
struct intel_crtc *enabled = NULL;
 
-   crtc->wm.active.i9xx = crtc->config->wm.i9xx.optimal;
-
crtc = intel_get_crtc_for_plane(dev_priv, 0);
planea_wm = crtc->wm.active.i9xx.plane_wm;
if (intel_crtc_active(crtc))
@@ -2379,7 +2417,7 @@ static void i9xx_update_wm(struct intel_crtc *crtc)
cwm = 2;
 
/* Play safe and disable self-refresh before adjusting watermarks. */
-   intel_set_memory_cxsr(dev_priv, false);
+   _intel_set_memory_cxsr(dev_priv, false);
 
/* Calc sr entries for one plane configs 

[Intel-gfx] [PATCH 4/7] drm/i915: Calculate gen4 watermarks semiatomically.

[Maarten Lankhorst]

Gen4 watermark is handled same as gen3-. Calculate
the optimal watermarks atomically first, and program
it in the legacy helper.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/intel_pm.c | 136 
 1 file changed, 95 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7ecf39815ab5..f2062f589f41 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2187,58 +2187,109 @@ static void vlv_optimize_watermarks(struct 
intel_atomic_state *state,
mutex_unlock(_priv->wm.wm_mutex);
 }
 
-static void i965_update_wm(struct intel_crtc *unused_crtc)
+static int i965_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 {
-   struct drm_i915_private *dev_priv = to_i915(unused_crtc->base.dev);
-   struct intel_crtc *crtc;
+   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+   struct intel_atomic_state *state =
+   to_intel_atomic_state(crtc_state->base.state);
+   struct i9xx_wm_state *wm_state = _state->wm.i9xx.optimal;
+   struct intel_plane *plane = to_intel_plane(crtc->base.primary);
+   const struct drm_plane_state *primary_plane_state = NULL;
+   const struct drm_plane_state *cursor_plane_state = NULL;
+
+   memset(wm_state, 0, sizeof(*wm_state));
+
+   if (crtc_state->base.plane_mask & BIT(drm_plane_index(>base)))
+   primary_plane_state = 
__drm_atomic_get_current_plane_state(>base, >base);
+
+   if (crtc_state->base.plane_mask & 
BIT(drm_plane_index(crtc->base.cursor)))
+   cursor_plane_state = 
__drm_atomic_get_current_plane_state(>base, crtc->base.cursor);
+
+   if (primary_plane_state) {
+   static const int sr_latency_ns = 12000;
+   const struct drm_display_mode *adjusted_mode =
+   _state->base.adjusted_mode;
+   unsigned active_crtcs;
+   unsigned long entries;
+   bool may_cxsr;
+
+   if (state->modeset)
+   active_crtcs = state->active_crtcs;
+   else
+   active_crtcs = dev_priv->active_crtcs;
+
+   may_cxsr = active_crtcs == drm_crtc_mask(>base);
+
+   if (may_cxsr && intel_wm_plane_visible(crtc_state, 
to_intel_plane_state(primary_plane_state))) {
+   struct drm_framebuffer *fb = primary_plane_state->fb;
+   unsigned cpp = fb->format->cpp[0];
+
+   entries = intel_wm_method2(adjusted_mode->crtc_clock,
+  adjusted_mode->crtc_htotal,
+  crtc_state->pipe_src_w, cpp,
+  sr_latency_ns / 100);
+   entries = DIV_ROUND_UP(entries, I915_FIFO_LINE_SIZE);
+   if (entries < I965_FIFO_SIZE)
+   wm_state->sr.plane = I965_FIFO_SIZE - entries;
+   else
+   may_cxsr = false;
+
+   DRM_DEBUG_KMS("self-refresh entries: %ld\n", entries);
+   }
+
+   /* No need to use intel_wm_plane_visible here, since cursor. */
+   if (may_cxsr && cursor_plane_state && crtc_state->base.active) {
+   entries = intel_wm_method2(adjusted_mode->crtc_clock,
+  adjusted_mode->crtc_htotal,
+  cursor_plane_state->crtc_w, 
4,
+  sr_latency_ns / 100);
+
+   entries = DIV_ROUND_UP(entries,
+ 
i965_cursor_wm_info.cacheline_size) +
+   i965_cursor_wm_info.guard_size;
+
+   if (entries < i965_cursor_wm_info.fifo_size)
+   wm_state->sr.cursor = 
min(i965_cursor_wm_info.fifo_size - entries,
+ (unsigned 
long)(i965_cursor_wm_info.max_wm));
+   else
+   may_cxsr = false;
+   } else if (may_cxsr)
+   wm_state->sr.cursor = 16;
+
+   wm_state->cxsr = may_cxsr;
+
+   DRM_DEBUG_KMS("FIFO watermarks - can cxsr: %s, display plane 
%d, cursor SR size: %d\n",
+ yesno(wm_state->cxsr), wm_state->sr.plane, 
wm_state->sr.cursor);
+   }
+
+   return 0;
+}
+
+static void i965_update_wm(struct intel_crtc *crtc)
+{
+   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
int srwm = 1;
int cursor_sr = 16;
-   bool cxsr_enabled;
+   bool cxsr_enabled = false;

[Intel-gfx] [PATCH 6/7] drm/i915: Kill off intel_crtc_active.

[Maarten Lankhorst]

Use crtc->active directly instead. This is still not completely
optimal and needs fixing, but it's about as good as using
intel_crtc_active.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/intel_display.c | 19 ---
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 drivers/gpu/drm/i915/intel_fbc.c |  2 +-
 drivers/gpu/drm/i915/intel_pm.c  |  6 +++---
 4 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 234b94cafc7d..0fa83895b8c4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -944,25 +944,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state 
*crtc_state, int target_clock,
  target_clock, refclk, NULL, best_clock);
 }
 
-bool intel_crtc_active(struct intel_crtc *crtc)
-{
-   /* Be paranoid as we can arrive here with only partial
-* state retrieved from the hardware during setup.
-*
-* We can ditch the adjusted_mode.crtc_clock check as soon
-* as Haswell has gained clock readout/fastboot support.
-*
-* We can ditch the crtc->primary->fb check as soon as we can
-* properly reconstruct framebuffers.
-*
-* FIXME: The intel_crtc->active here should be switched to
-* crtc->state->active once we have proper CRTC states wired up
-* for atomic.
-*/
-   return crtc->active && crtc->base.primary->state->fb &&
-   crtc->config->base.adjusted_mode.crtc_clock;
-}
-
 enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 enum pipe pipe)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d0f1704e1a17..1a70939c69c5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1464,7 +1464,6 @@ bool bxt_find_best_dpll(struct intel_crtc_state 
*crtc_state, int target_clock,
struct dpll *best_clock);
 int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
 
-bool intel_crtc_active(struct intel_crtc *crtc);
 void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
 enum intel_display_power_domain intel_port_to_power_domain(enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 860b8c26d29b..da83629fc38f 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1277,7 +1277,7 @@ void intel_fbc_init_pipe_state(struct drm_i915_private 
*dev_priv)
return;
 
for_each_intel_crtc(_priv->drm, crtc)
-   if (intel_crtc_active(crtc) &&
+   if (crtc->base.state->active &&
crtc->base.primary->state->visible)
dev_priv->fbc.visible_pipes_mask |= (1 << crtc->pipe);
 }
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3484ce40f2b0..3afaa6933c4b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -820,7 +820,7 @@ static struct intel_crtc *single_enabled_crtc(struct 
drm_i915_private *dev_priv)
struct intel_crtc *crtc, *enabled = NULL;
 
for_each_intel_crtc(_priv->drm, crtc) {
-   if (intel_crtc_active(crtc)) {
+   if (crtc->active) {
if (enabled)
return NULL;
enabled = crtc;
@@ -2484,11 +2484,11 @@ static void i9xx_program_watermarks(struct 
drm_i915_private *dev_priv)
 
crtc = intel_get_crtc_for_plane(dev_priv, 0);
planea_wm = crtc->wm.active.i9xx.plane_wm;
-   if (intel_crtc_active(crtc))
+   if (crtc->active)
enabled = crtc;
 
crtc = intel_get_crtc_for_plane(dev_priv, 1);
-   if (intel_crtc_active(crtc)) {
+   if (crtc->active) {
if (enabled == NULL)
enabled = crtc;
else
-- 
2.11.0

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

[Intel-gfx] [PATCH 7/7] drm/i915: Rip out legacy watermark infrastructure

[Maarten Lankhorst]

The legacy watermark infrastructure is now unused, so remove it.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_atomic.c  |  2 -
 drivers/gpu/drm/i915/intel_display.c | 74 ++--
 drivers/gpu/drm/i915/intel_drv.h |  2 -
 drivers/gpu/drm/i915/intel_pm.c  | 42 
 5 files changed, 3 insertions(+), 118 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ac17f0f4053..029587ccd803 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -692,7 +692,6 @@ struct drm_i915_display_funcs {
void (*optimize_watermarks)(struct intel_atomic_state *state,
struct intel_crtc_state *cstate);
int (*compute_global_watermarks)(struct drm_atomic_state *state);
-   void (*update_wm)(struct intel_crtc *crtc);
int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
 * fills out the pipe-config with the hw state. */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e635e4ce..e2b4ca518a04 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -173,8 +173,6 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state->update_pipe = false;
crtc_state->disable_lp_wm = false;
crtc_state->disable_cxsr = false;
-   crtc_state->update_wm_pre = false;
-   crtc_state->update_wm_post = false;
crtc_state->fb_changed = false;
crtc_state->fifo_changed = false;
crtc_state->wm.need_postvbl_update = false;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0fa83895b8c4..90c8a964c264 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4848,8 +4848,6 @@ static void intel_post_plane_update(struct 
intel_crtc_state *old_crtc_state)
 
intel_frontbuffer_flip(to_i915(crtc->base.dev), pipe_config->fb_bits);
 
-   if (pipe_config->update_wm_post && pipe_config->base.active)
-   intel_update_watermarks(crtc);
 
if (old_pri_state) {
struct intel_plane_state *primary_state =
@@ -4940,8 +4938,6 @@ static void intel_pre_plane_update(struct 
intel_crtc_state *old_crtc_state,
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
 pipe_config);
-   else if (pipe_config->update_wm_pre)
-   intel_update_watermarks(crtc);
 }
 
 static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned 
plane_mask)
@@ -5623,8 +5619,6 @@ static void i9xx_crtc_enable(struct intel_crtc_state 
*pipe_config,
if (dev_priv->display.initial_watermarks != NULL)
dev_priv->display.initial_watermarks(old_intel_state,
 intel_crtc->config);
-   else
-   intel_update_watermarks(intel_crtc);
intel_enable_pipe(intel_crtc);
 
assert_vblank_disabled(crtc);
@@ -5689,9 +5683,6 @@ static void i9xx_crtc_disable(struct intel_crtc_state 
*old_crtc_state,
if (!IS_GEN2(dev_priv))
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
 
-   if (!dev_priv->display.initial_watermarks)
-   intel_update_watermarks(intel_crtc);
-
/* clock the pipe down to 640x480@60 to potentially save power */
if (IS_I830(dev_priv))
i830_enable_pipe(dev_priv, pipe);
@@ -5752,7 +5743,6 @@ static void intel_crtc_disable_noatomic(struct drm_crtc 
*crtc,
encoder->base.crtc = NULL;
 
intel_fbc_disable(intel_crtc);
-   intel_update_watermarks(intel_crtc);
intel_disable_shared_dpll(intel_crtc);
 
domains = intel_crtc->enabled_power_domains;
@@ -10019,40 +10009,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
kfree(intel_crtc);
 }
 
-/**
- * intel_wm_need_update - Check whether watermarks need updating
- * @plane: drm plane
- * @state: new plane state
- *
- * Check current plane state versus the new one to determine whether
- * watermarks need to be recalculated.
- *
- * Returns true or false.
- */
-static bool intel_wm_need_update(struct drm_plane *plane,
-struct drm_plane_state *state)
-{
-   struct intel_plane_state *new = to_intel_plane_state(state);
-   struct intel_plane_state *cur = to_intel_plane_state(plane->state);
-
-   /* Update watermarks on tiling or size changes. */
-   if (new->base.visible != cur->base.visible)
-   return true;
-
-   if (!cur->base.fb || !new->base.fb)
-   return false;
-
-   if 

[Intel-gfx] [PATCH 0/7] drm/i915: Convert gen4- watermarks to atomic.

[Maarten Lankhorst]

Calculate watermarks for gen4 and lower atomically. This has been tested
on the pineview system in CI. It fixes bug 101597, which is a vblank wait
timed out when running kms_pipe_crc_basic. The changes to CXSR will allow
the test to pass.

Maarten Lankhorst (7):
  drm/i915: Calculate gen3- watermarks semi-atomically.
  drm/i915: Program gen3- watermarks atomically
  drm/i915: Convert pineview watermarks to atomic, v2.
  drm/i915: Calculate gen4 watermarks semiatomically.
  drm/i915: Program gen4 watermarks atomically
  drm/i915: Kill off intel_crtc_active.
  drm/i915: Rip out legacy watermark infrastructure

 drivers/gpu/drm/i915/i915_drv.h  |   6 +-
 drivers/gpu/drm/i915/intel_atomic.c  |   2 -
 drivers/gpu/drm/i915/intel_display.c |  96 +-
 drivers/gpu/drm/i915/intel_drv.h |  18 +-
 drivers/gpu/drm/i915/intel_fbc.c |   2 +-
 drivers/gpu/drm/i915/intel_pm.c  | 639 ++-
 6 files changed, 435 insertions(+), 328 deletions(-)

-- 
2.11.0

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

Re: [Intel-gfx] a potential dead loop in intel_lrc_irq_handler

[Dong, Chuanxiao]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Monday, August 7, 2017 5:56 PM
> To: Dong, Chuanxiao ; intel-
> g...@lists.freedesktop.org; Joonas Lahtinen
> 
> Subject: Re: a potential dead loop in intel_lrc_irq_handler
> 
> Quoting Dong, Chuanxiao (2017-08-07 10:41:29)
> > Hello,
> >
> > Found there might be a corner case for intel_lrc_irq_handler() in a dead
> loop, want to understand if this can be real or not.
> >
> > The scenario is like:
> 
> > 1. Write wedged to trigger a GPU reset;
> 
> This is dangerous full stop, but even with a hangcheck the scenario is still
> plausible.
> 
> > 2. meanwhile, there is one ongoing request in port[0], and its context
> > switch interrupt is generated from HW; 3. as interrupt line is
> > disabled during GPU reset, it is possible that this interrupt is not
> > handled by intel_lrc_irq_handler(); 4. during GPU reset, the CSB tail
> > is reset to 0x7 which is a default value;
> 
> In theory, yes. This prevents the delayed context switch interrupt from
> having any meaning.
> 
> > 5. i915 try to replay this request during GPU reset;
> 
> If the context-switch occurred (but still pending in IIR), the request is
> complete, it will not be replayed.
> 
> > 6. GPU reset completed;
> > 7. handling the pending interrupt of the step#2.
> >
> > Normally as in step#5 driver wrote the ELSP and replayed a request so the
> CSB tail should be updated to 0 in step#7. But if the CSB tail updating is not
> that quick, in step#7 when handling the last pending interrupt the CSB tail is
> still 0x7, the intel_lrc_irq_handler() will be in a dead loop then.
> >
> > If the CSB tail updating is not synchronized with the ELSP writing then my
> understanding is that it is possible to encounter this corner case. If so, 
> shall
> we clear the pending interrupts in IIR during i915_reset? Please correct me if
> anything wrong.
> 
> The CSB buf+tail is synchronized to the interrupt. Our goal is to make sure
> that the GPU is truly reset before we reset our state tracking so that we 
> don't
> have pending events on replay.
> 
> However, the CSB itself is a little bit of a black box as it is squirreled 
> away in a
> power context on reset, and it is only with a bit of handwaving that it is 
> reset
> to a default empty value on reset.
> 
> CSB interrupt -> pending
> GPU reset -> clears CSB head/tail
But the GPU reset will make CSB_head = 0 and CSB_tail = 7.

> post-reset, re-enable interrupts, raise CSB interrupt
> -> intel_lrc_irq_handler()
>   if (CSB_head == CSB_tail)
>   break;

So here intel_lrc_irq_handler() cannot break out. Looks like we are still stuck 
in intel_lrc_irq_handler(), right?

Thanks
Chuanxiao
> 
> Should be no problem. Similarly for a delayed tasklet, we haven't posted the
> CSB interrupt and so we don't even read the CSB_head/tail as they as still
> undefined (prior to the first CSB interrupt).
> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Chris Wilson]

Quoting Maarten Lankhorst (2017-08-07 10:45:38)
> Op 04-08-17 om 09:50 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2017-08-02 11:29:17)
> >> Export 2 functions, igt_signal_helper_get_num and
> >> igt_signal_helper_get_hz.
> >>
> >> This will allow tests to measure how much time in a test was spent
> >> in a uninterruptible state, which is useful when testing whether
> >> certain ioctl's can be interrupted or not.
> > Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> > was interrupted. Refine it to suit your purposes, it is the surgical
> > scalpel compared to the shotgun of signal_helper.
> > -Chris
> 
> I've been using the igt display helpers now so I don't have to worry about 
> the ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, 
> and that makes the code a lot more verbose..

You know that we igt uses igt_ioctl so that we can switch the ioctl
wrapper. What you have here is sig_ioctl so rather than painting the
shotgun signal helper a different colour, use the interface that
actually supports what you have in mind.
-Chris
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] a potential dead loop in intel_lrc_irq_handler

[Chris Wilson]

Quoting Dong, Chuanxiao (2017-08-07 10:41:29)
> Hello,
> 
> Found there might be a corner case for intel_lrc_irq_handler() in a dead 
> loop, want to understand if this can be real or not.
> 
> The scenario is like:

> 1. Write wedged to trigger a GPU reset;

This is dangerous full stop, but even with a hangcheck the scenario is
still plausible.

> 2. meanwhile, there is one ongoing request in port[0], and its context switch 
> interrupt is generated from HW;
> 3. as interrupt line is disabled during GPU reset, it is possible that this 
> interrupt is not handled by intel_lrc_irq_handler();
> 4. during GPU reset, the CSB tail is reset to 0x7 which is a default value;

In theory, yes. This prevents the delayed context switch interrupt from
having any meaning.

> 5. i915 try to replay this request during GPU reset;

If the context-switch occurred (but still pending in IIR), the request is
complete, it will not be replayed.

> 6. GPU reset completed;
> 7. handling the pending interrupt of the step#2.
> 
> Normally as in step#5 driver wrote the ELSP and replayed a request so the CSB 
> tail should be updated to 0 in step#7. But if the CSB tail updating is not 
> that quick, in step#7 when handling the last pending interrupt the CSB tail 
> is still 0x7, the intel_lrc_irq_handler() will be in a dead loop then.
> 
> If the CSB tail updating is not synchronized with the ELSP writing then my 
> understanding is that it is possible to encounter this corner case. If so, 
> shall we clear the pending interrupts in IIR during i915_reset? Please 
> correct me if anything wrong.

The CSB buf+tail is synchronized to the interrupt. Our goal is to make
sure that the GPU is truly reset before we reset our state tracking so
that we don't have pending events on replay.

However, the CSB itself is a little bit of a black box as it is
squirreled away in a power context on reset, and it is only with a bit
of handwaving that it is reset to a default empty value on reset.

CSB interrupt -> pending
GPU reset -> clears CSB head/tail
post-reset, re-enable interrupts, raise CSB interrupt
-> intel_lrc_irq_handler()
if (CSB_head == CSB_tail)
break;

Should be no problem. Similarly for a delayed tasklet, we haven't posted
the CSB interrupt and so we don't even read the CSB_head/tail as they as
still undefined (prior to the first CSB interrupt).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/3] lib/igt_aux: Export statistics of signal helper.

[Maarten Lankhorst]

Op 04-08-17 om 09:50 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2017-08-02 11:29:17)
>> Export 2 functions, igt_signal_helper_get_num and
>> igt_signal_helper_get_hz.
>>
>> This will allow tests to measure how much time in a test was spent
>> in a uninterruptible state, which is useful when testing whether
>> certain ioctl's can be interrupted or not.
> Use sig_ioctl, the purpose of that wrapper is to measure whether or not it
> was interrupted. Refine it to suit your purposes, it is the surgical
> scalpel compared to the shotgun of signal_helper.
> -Chris

I've been using the igt display helpers now so I don't have to worry about the 
ioctl's. Using sig_ioctl would mean having to perform the ioctl's myself, and 
that makes the code a lot more verbose..

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

[Intel-gfx] a potential dead loop in intel_lrc_irq_handler

[Dong, Chuanxiao]

Hello,

Found there might be a corner case for intel_lrc_irq_handler() in a dead loop, 
want to understand if this can be real or not.

The scenario is like:
1. Write wedged to trigger a GPU reset;
2. meanwhile, there is one ongoing request in port[0], and its context switch 
interrupt is generated from HW;
3. as interrupt line is disabled during GPU reset, it is possible that this 
interrupt is not handled by intel_lrc_irq_handler();
4. during GPU reset, the CSB tail is reset to 0x7 which is a default value;
5. i915 try to replay this request during GPU reset;
6. GPU reset completed;
7. handling the pending interrupt of the step#2.

Normally as in step#5 driver wrote the ELSP and replayed a request so the CSB 
tail should be updated to 0 in step#7. But if the CSB tail updating is not that 
quick, in step#7 when handling the last pending interrupt the CSB tail is still 
0x7, the intel_lrc_irq_handler() will be in a dead loop then.

If the CSB tail updating is not synchronized with the ELSP writing then my 
understanding is that it is possible to encounter this corner case. If so, 
shall we clear the pending interrupts in IIR during i915_reset? Please correct 
me if anything wrong.

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

Re: [Intel-gfx] [PATCH] drm: Wake up next in drm_read() chain if we are forced to putback the event

[Daniel Vetter]

On Fri, Aug 04, 2017 at 09:23:28AM +0100, Chris Wilson wrote:
> After an event is sent, we try to copy it into the user buffer of the
> first waiter in drm_read() and if the user buffer doesn't have enough
> room we put it back onto the list. However, we didn't wake up any
> subsequent waiter, so that event may sit on the list until either a new
> vblank event is sent or a new waiter appears. Rare, but in the worst
> case may lead to a stuck process.
> 
> Signed-off-by: Chris Wilson 
> Cc: Daniel Vetter 

New subtestcase in igt@drm_read?
-Daniel

> ---
>  drivers/gpu/drm/drm_file.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 59b75a974357..7e6db9f7a058 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -524,6 +524,7 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
>   file_priv->event_space -= length;
>   list_add(>link, _priv->event_list);
>   spin_unlock_irq(>event_lock);
> + wake_up_interruptible(_priv->event_wait);
>   break;
>   }
>  
> -- 
> 2.13.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/29] drm/i915: switch to drm_*{get, put} helpers

[Daniel Vetter]

On Thu, Aug 3, 2017 at 5:52 PM, Cihangir Akturk  wrote:
> On Thu, Aug 03, 2017 at 02:49:03PM +0200, Daniel Vetter wrote:
>> On Thu, Aug 03, 2017 at 03:26:01PM +0300, Jani Nikula wrote:
>> > On Thu, 03 Aug 2017, Cihangir Akturk  wrote:
>> > > drm_*_reference() and drm_*_unreference() functions are just
>> > > compatibility alias for drm_*_get() and drm_*_put() adn should not be
>> > > used by new code. So convert all users of compatibility functions to use
>> > > the new APIs.
>> >
>> > Please include the cocci script in the commit message. If you didn't use
>> > cocci, you should check it out! :)
>>
>> Yeah I assume you've created these with spatch/cocci, not with your own
>> script. I trust cocci a lot more than any kind of scripting, and the
>> coccie patch is already in there kernel (the commits you've cited in the
>> cover letter contain it iirc).
>
> I wrote a simple shell script, which you can see in the cover letter.
> But you are right I take function list from 
> scripts/coccinelle/api/drm-get-put.cocci
> file.
>
> Daniel Should I use coccinelle to generate patches and resend a v2?
> If so, do i need to include reviewed-by and acked-by tags to patches myself?

I think regenerating the patches using cocci would be great, I trust
cocci a lot more than random scripts. And cocci is a great tool to
learn anyway (if you don't know it yet). If the resulting patches
match, you can keep the r-b/a-b tags for v2.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] dim: symbolic link for the rr-cache

[Daniel Vetter]

After about a month and a few attempts it's clear that my rr-cache gc
logic is busted. When copying stuff back between the git branch
and git's rr-cache dir, something somewhere (or maybe it's git rerere
itself) touches all files and wreaks the timestamps.

End result is that over this month, every time when someone gc'ed a
few files, they've been resurrected a few days later by someone else.

I think the only way to fix this is by dropping the copying and
replacing git's rr-cache with a symbolic link. That way, when we
delete an rr-cache entry in the git branch, it won't be able to
resurrect. I hope at least ...

This also makes dim revert-rerere no longer needed, delete it.

v2: Unfunny the ln option placement (Jani).

Acked-by: Jani Nikula 
Signed-off-by: Daniel Vetter 
---
 dim | 22 +++---
 dim.rst |  9 -
 2 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/dim b/dim
index d52510529fa2..f8be76df4952 100755
--- a/dim
+++ b/dim
@@ -504,8 +504,13 @@ function update_rerere_cache
 
cd $DIM_PREFIX/drm-rerere/
git pull &> /dev/null
-   mkdir $(rr_cache_dir) &> /dev/null || true
-   cp rr-cache/* $(rr_cache_dir) -r --preserve=timestamps
+   if [ -d $(rr_cache_dir) ] ; then
+   rm -Rf $(rr_cache_dir)
+   fi
+   if [ ! -L $(rr_cache_dir) ] ; then
+   ln -s "$DIM_PREFIX/drm-rerere/rr-cache" $(dirname 
$(rr_cache_dir))
+   fi
+
cd - > /dev/null
 
echo "Done."
@@ -519,8 +524,6 @@ function commit_rerere_cache
if git_is_current_branch rerere-cache ; then
remote=$(branch_to_remote rerere-cache)
 
-   rm $(rr_cache_dir)/rr-cache -Rf &> /dev/null || true
-   cp $(rr_cache_dir)/* rr-cache -r --preserve=timestamps
git pull >& /dev/null
git add ./*.patch >& /dev/null || true
for file  in $(git ls-files); do
@@ -543,17 +546,6 @@ function commit_rerere_cache
fi
 }
 
-function dim_revert_rerere
-{
-   local commit
-
-   commit=${1:?$usage}
-
-   cd $DIM_PREFIX/drm-rerere/
-   git revert $commit
-   rm $(rr_cache_dir)/* -Rf
-}
-
 dim_alias_rebuild_nightly=rebuild-tip
 function dim_rebuild_tip
 {
diff --git a/dim.rst b/dim.rst
index f012a831da74..a2c110a054ee 100644
--- a/dim.rst
+++ b/dim.rst
@@ -247,15 +247,6 @@ Rebuild and push the integration tree.
 ADVANCED COMMANDS FOR COMMITTERS AND MAINTAINERS
 
 
-revert-rerere *rerere-cache-commit-ish*

-When a stored conflict resolution in the integration tree is wrong, this 
command
-can be used to fix up the mess. First figure out which commit in the
-*rerere-cache* branch contains the bogus conflict resolution, then revert it
-using this command. This ensures the resolution is also purged from any local
-caches, to make sure it doesn't get resurrected. Then run *rebuild-tip* to redo
-the merges, correctly.
-
 cat-to-fixup
 
 Pipes stdin into the fixup patch file for the current drm-tip merge.
-- 
2.13.3

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

Re: [Intel-gfx] [PATCH] dim: Continue also for dry runs

[Daniel Vetter]

On Thu, Aug 3, 2017 at 2:44 PM, Jani Nikula  wrote:
> On Thu, 27 Jul 2017, Daniel Vetter  wrote:
>> It's a bit silly to have to spec both -d and -f to see what dim would
>> all complain about. And dry-run should never cause bad side-effects.
>
> Ack.
>
> We don't do dry-run all that well in general, partly because you need to
> have one part actually succeed to make the rest succeed. And some things
> don't do dry-run at all. Those could bail out with an error right
> away. To the endless todo list...

Yeah, one of the biggest things I'd like to see is dim push-branch not
pushing until the dry-run (and entirely offline) rebuild-tip
succeeded. I think otherwise we're fairly good with dry-runs ...
-Daniel

>
> BR,
> Jani.
>
>>
>> Signed-off-by: Daniel Vetter 
>> ---
>>  dim | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/dim b/dim
>> index c0cbe352b165..96aaf7101d6b 100755
>> --- a/dim
>> +++ b/dim
>> @@ -126,6 +126,8 @@ function warn_or_fail
>>  {
>>   if [[ $FORCE ]] ; then
>>   echoerr "WARNING: $1, but continuing"
>> + elif [[ $DRY ]] ; then
>> + echoerr "WARNING: $1, but continuing dry-run"
>>   else
>>   echoerr "ERROR: $1, aborting"
>>   exit 1
>
> --
> Jani Nikula, Intel Open Source Technology Center



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [GIT PULL] GVT-g fixes for 4.13-rc5

[Jani Nikula]

On Mon, 07 Aug 2017, Zhenyu Wang  wrote:
> Hi, this is gvt fixes for 4.13-rc5, there're two regression fixes for
> MMIO handle 65f9f6febf12, and two important vGPU reset fixes.

Pulled to drm-intel-fixes, thanks.

BR,
Jani.

>
> Thanks
> --
> The following changes since commit 26a201a2ba82a801973ce29e1004b64742e81e7e:
>
>   drm/i915/gvt: Extend KBL platform support in GVT-g (2017-07-25 12:39:41 
> +0800)
>
> are available in the git repository at:
>
>   https://github.com/01org/gvt-linux.git tags/gvt-fixes-2017-08-07
>
> for you to fetch changes up to d6086598d34e1cf9091c7be201f5b2041dc6203e:
>
>   drm/i915/gvt: Change the max length of mmio_reg_rw from 4 to 8 (2017-08-07 
> 15:50:39 +0800)
>
> 
> gvt-fixes-2017-08-07
>
> - two regression fixes for 65f9f6febf12, one is for display MMIO
>   initial value (Tina), another for 64bit MMIO access (Xiong)
> - two reset fixes from Chuanxiao
>
> 
> Chuanxiao Dong (2):
>   drm/i915/gvt: change resetting to resetting_eng
>   drm/i915/gvt: clean workload queue if error happened
>
> Tina Zhang (1):
>   drm/i915/gvt: Initialize MMIO Block with HW state
>
> Xiong Zhang (1):
>   drm/i915/gvt: Change the max length of mmio_reg_rw from 4 to 8
>
>  drivers/gpu/drm/i915/gvt/execlist.c  | 27 -
>  drivers/gpu/drm/i915/gvt/firmware.c  | 11 ++-
>  drivers/gpu/drm/i915/gvt/gvt.h   | 14 -
>  drivers/gpu/drm/i915/gvt/handlers.c  | 38 
> +---
>  drivers/gpu/drm/i915/gvt/scheduler.c |  3 ++-
>  drivers/gpu/drm/i915/gvt/vgpu.c  |  8 +---
>  6 files changed, 70 insertions(+), 31 deletions(-)

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

Re: [Intel-gfx] [PATCH i-g-t 3/3] tests: Add kms_atomic_interruptible test.

[Maarten Lankhorst]

Op 04-08-17 om 09:50 schreef Mika Kahola:
> On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
>> To make sure that we have test exposure when allowing atomic ioctl's
>> to
>> fail interruptibly, we add a test that will fail to lock the
>> mutexes until the fence is signaled.
>>
>> If the locking is done interruptibly, our signal helper will
>> interrupt
>> often, and with the statistics we can ensure that we received
>> enough interrupts to pass.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  tests/Makefile.sources   |   1 +
>>  tests/kms_atomic_interruptible.c | 193
>> +++
>>  2 files changed, 194 insertions(+)
>>  create mode 100644 tests/kms_atomic_interruptible.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 5b98a5a371b8..9a6a846ab78a 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -171,6 +171,7 @@ TESTS_progs = \
>>  kms_3d \
>>  kms_addfb_basic \
>>  kms_atomic \
>> +kms_atomic_interruptible \
>>  kms_atomic_transition \
>>  kms_busy \
>>  kms_ccs \
>> diff --git a/tests/kms_atomic_interruptible.c
>> b/tests/kms_atomic_interruptible.c
>> new file mode 100644
>> index ..4015988c0e2c
>> --- /dev/null
>> +++ b/tests/kms_atomic_interruptible.c
>> @@ -0,0 +1,193 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
> Update a year here?
>> + *
>> + * 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"
>> +#include "drmtest.h"
>> +#include "sw_sync.h"
>> +
>> +enum plane_test_type
>> +{
>> +test_setmode,
>> +test_setplane,
>> +test_pageflip
>> +};
>> +
>> +static int block_plane(igt_display_t *display, igt_output_t *output,
>> enum plane_test_type test_type, igt_plane_t *plane)
>> +{
>> +int timeline = sw_sync_timeline_create();
>> +
>> +igt_fork(child, 1) {
>> +/* Ignore the signal helper, we need to block
>> indefinitely on the fence. */
>> +signal(SIGCONT, SIG_IGN);
>> +
>> +if (test_type == test_setmode) {
>> +igt_output_set_pipe(output, PIPE_NONE);
>> +igt_plane_set_fb(plane, NULL);
>> +}
>> +igt_plane_set_fence_fd(plane,
>> sw_sync_timeline_create_fence(timeline, 1));
>> +
>> +igt_display_commit2(display, COMMIT_ATOMIC);
>> +}
>> +
>> +return timeline;
>> +}
>> +
>> +static void unblock(int block)
>> +{
>> +sw_sync_timeline_inc(block, 1);
>> +close(block);
>> +}
>> +
>> +static void run_plane_test(igt_display_t *display, enum pipe pipe,
>> igt_output_t *output,
>> +   enum plane_test_type test_type, unsigned
>> plane_type, enum igt_commit_style style)
>> +{
>> +drmModeModeInfo *mode;
>> +igt_fb_t fb, fb2;
>> +igt_plane_t *primary, *plane;
>> +int block;
>> +
>> +/*
>> + * Make sure we start with everything disabled to force a
>> real modeset.
>> + * igt_display_init only sets sw state, and assumes the
>> first test doesn't care
>> + * about hw state.
>> + */
>> +igt_display_commit2(display, COMMIT_ATOMIC);
>> +
>> +igt_output_set_pipe(output, pipe);
>> +
>> +primary = igt_output_get_plane_type(output,
>> DRM_PLANE_TYPE_PRIMARY);
>> +plane = igt_output_get_plane_type(output, plane_type);
>> +mode = igt_output_get_mode(output);
>> +
>> +igt_create_fb(display->drm_fd, mode->hdisplay, mode-
>>> vdisplay,
>> +  DRM_FORMAT_XRGB,
>> LOCAL_DRM_FORMAT_MOD_NONE, );
>> +
>> +switch (plane_type) {
>> +case DRM_PLANE_TYPE_PRIMARY:
>> +igt_create_fb(display->drm_fd, mode->hdisplay, mode-
>>> vdisplay,
>> +  DRM_FORMAT_XRGB,
>> LOCAL_DRM_FORMAT_MOD_NONE, );
>> +

Re: [Intel-gfx] [PATCH i-g-t 2/3] lib/igt_kms: Remove vblank wait after plane update.

[Maarten Lankhorst]

Op 04-08-17 om 10:07 schreef Mika Kahola:
> On Wed, 2017-08-02 at 12:29 +0200, Maarten Lankhorst wrote:
>> With the conversion to atomic, this is already handled in the core.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>  lib/igt_kms.c | 15 ---
>>  lib/igt_kms.h |  1 -
>>  2 files changed, 16 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 6390229f1546..14e2701c3afd 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -2374,8 +2374,6 @@ static int
>> igt_primary_plane_commit_legacy(igt_plane_t *primary,
>>  
>>  CHECK_RETURN(ret, fail_on_error);
>>  
>> -primary->pipe->enabled = (fb_id != 0);
>> -
> How was this related to vblank wait removal?
There are no users of pipe->enabled left, so it's removed.
>>  return 0;
>>  }
>>  
>> @@ -2413,10 +2411,8 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>> enum igt_commit_style s,
>> bool fail_on_error)
>>  {
>> -igt_display_t *display = pipe->display;
>>  int i;
>>  int ret;
>> -bool need_wait_for_vblank = false;
>>  
>>  if (pipe->background_changed) {
>>  igt_crtc_set_property(pipe, pipe-
>>> background_property,
>> @@ -2435,21 +2431,10 @@ static int igt_pipe_commit(igt_pipe_t *pipe,
>>  for (i = 0; i < pipe->n_planes; i++) {
>>  igt_plane_t *plane = >planes[i];
>>  
>> -if (plane->fb_changed || plane->position_changed ||
>> plane->size_changed)
>> -need_wait_for_vblank = true;
>> -
>>  ret = igt_plane_commit(plane, pipe, s,
>> fail_on_error);
>>  CHECK_RETURN(ret, fail_on_error);
>>  }
>>  
>> -/*
>> - * If the crtc is enabled, wait until the next vblank before
>> returning
>> - * if we made changes to any of the planes.
>> - */
>> -if (need_wait_for_vblank && pipe->enabled) {
>> -igt_wait_for_vblank(display->drm_fd, pipe->pipe);
>> -}
>> -
>>  return 0;
>>  }
>>  
>> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
>> index 35428f3e9675..d18a92600933 100644
>> --- a/lib/igt_kms.h
>> +++ b/lib/igt_kms.h
>> @@ -323,7 +323,6 @@ typedef struct {
>>  struct igt_pipe {
>>  igt_display_t *display;
>>  enum pipe pipe;
>> -bool enabled;
>>  
>>  int n_planes;
>>  int plane_cursor;


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

Re: [Intel-gfx] [PATCH v13 6/7] drm/i915: Introduce GEM proxy

[Joonas Lahtinen]

On ti, 2017-07-25 at 17:28 +0800, Tina Zhang wrote:
> GEM proxy is a kind of GEM, whose backing physical memory is pinned
> and produced by guest VM and is used by host as read only. With GEM
> proxy, host is able to access guest physical memory through GEM object
> interface. As GEM proxy is such a special kind of GEM, a new flag
> I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the
> backing storage of GEM proxy.
> 

It is a good idea to add a changelog here to indicate what was changed
since the last patch revision. It'll be easier for the reviewer to spot
the differences.

It's also a good idea to add Cc: line for somebody who provided
previous review, this will, too, allow spotting the patch quicker.

> Signed-off-by: Tina Zhang 



> @@ -1730,7 +1744,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>    */
>   if (!obj->base.filp) {
>   i915_gem_object_put(obj);
> - return -EINVAL;
> + return -EPERM;
>   }

This hunk should be separate bugfix patch, but other than that the
patch looks OK to me.

I'll let Chris comment if he feels previous comments were adequately
addressed.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v13 5/7] vfio: ABI for mdev display dma-buf operation

[Zhang, Tina]

After going through the previous discussions, here are some summaries may be 
related to the current discussion:
1. How does user mode figure the device capabilities between region and dma-buf?
VFIO_DEVICE_GET_REGION_INFO could tell if the mdev supports region case. 
Otherwise, the mdev supports dma-buf.

2. For dma-buf, how to differentiate unsupported vs not initialized?
For dma-buf, when the mdev doesn't support some arguments, -EINVAL will be 
returned. And -errno will return when meeting other failures, like -ENOMEM.
If the mdev is not initialized, there won't be any returned err. Just zero all 
the fields in structure vfio_device_gfx_plane_info.

3. The id field in structure vfio_device_gfx_plane_info
So far we haven't figured out the usage of this field for dma-buf usage. So, 
this field is changed to "region_index" and only used for region usage.
In previous discussions, we thought this "id" field might be used for both 
dma-buf and region usages.
So, we proposed some ways, like adding flags field to the structure. Another 
way to do it was to add this:
enum vfio_device_gfx_type {
 VFIO_DEVICE_GFX_NONE,
 VFIO_DEVICE_GFX_DMABUF,
 VFIO_DEVICE_GFX_REGION,
 };
 
 struct vfio_device_gfx_query_caps {
 __u32 argsz;
 __u32 flags;
 enum vfio_device_gfx_type;
 };
Obviously, we don't need to consider this again, unless we find the 
"region_index" means something for dma-buf usage.

Thanks.

BR,
Tina

> -Original Message-
> From: Zhang, Tina
> Sent: Monday, August 7, 2017 11:23 AM
> To: 'Alex Williamson' 
> Cc: Tian, Kevin ; intel-gfx@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org; kwankh...@nvidia.com; kra...@redhat.com;
> intel-gvt-...@lists.freedesktop.org; Wang, Zhi A ; Lv,
> Zhiyuan 
> Subject: RE: [PATCH v13 5/7] vfio: ABI for mdev display dma-buf operation
> 
> 
> 
> > -Original Message-
> > From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On
> > Behalf Of Alex Williamson
> > Sent: Thursday, August 3, 2017 10:43 PM
> > To: Zhang, Tina 
> > Cc: Tian, Kevin ;
> > intel-gfx@lists.freedesktop.org; dri- de...@lists.freedesktop.org;
> > kwankh...@nvidia.com; kra...@redhat.com;
> > intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> > ; Lv, Zhiyuan 
> > Subject: Re: [PATCH v13 5/7] vfio: ABI for mdev display dma-buf
> > operation
> >
> > On Thu, 3 Aug 2017 07:08:15 +
> > "Zhang, Tina"  wrote:
> >
> > > > -Original Message-
> > > > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > > > Sent: Thursday, August 3, 2017 11:38 AM
> > > > To: Zhang, Tina 
> > > > Cc: Tian, Kevin ;
> > > > intel-gfx@lists.freedesktop.org; dri- de...@lists.freedesktop.org;
> > > > kwankh...@nvidia.com; kra...@redhat.com;
> > > > intel-gvt-...@lists.freedesktop.org; Wang, Zhi A
> > > > ; Lv, Zhiyuan 
> > > > Subject: Re: [PATCH v13 5/7] vfio: ABI for mdev display dma-buf
> > > > operation
> > > >
> > > > On Thu, 3 Aug 2017 03:17:09 +
> > > > "Zhang, Tina"  wrote:
> > > >
> > > > > > -Original Message-
> > > > > > From: dri-devel
> > > > > > [mailto:dri-devel-boun...@lists.freedesktop.org]
> > > > > > On Behalf Of Alex Williamson
> > > > > > Sent: Wednesday, August 2, 2017 5:18 AM
> > > > > > To: Zhang, Tina 
> > > > > > Cc: Tian, Kevin ; kra...@redhat.com;
> > > > > > intel- g...@lists.freedesktop.org; Wang, Zhi A
> > > > > > ; kwankh...@nvidia.com;
> > > > > > dri-de...@lists.freedesktop.org; intel-gvt-
> > > > > > d...@lists.freedesktop.org; Lv, Zhiyuan 
> > > > > > Subject: Re: [PATCH v13 5/7] vfio: ABI for mdev display
> > > > > > dma-buf operation
> > > > > >
> > > > > > On Tue, 25 Jul 2017 17:28:18 +0800 Tina Zhang
> > > > > >  wrote:
> > > > > >
> > > > > > > Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user
> > mode
> > > > > > > query and get the plan and its related information.
> > > > > > >
> > > > > > > The dma-buf's life cycle is handled by user mode and tracked by
> kernel.
> > > > > > > The returned fd in struct vfio_device_query_gfx_plane can be
> > > > > > > a new fd or an old fd of a re-exported dma-buf. Host User
> > > > > > > mode can check the value of fd and to see if it needs to
> > > > > > > create new resource according to the new fd or just use the
> > > > > > > existed resource related to the old
> > > > fd.
> > > > > > >
> > > > > > > Signed-off-by: Tina Zhang 
> > > > > > > ---
> > > > > > >  include/uapi/linux/vfio.h | 28 
> > > > > > >  1 file changed, 28 insertions(+)
> > > > > > >
> > > > > > > diff --git 

[Intel-gfx] [GIT PULL] GVT-g fixes for 4.13-rc5

[Zhenyu Wang]

Hi, this is gvt fixes for 4.13-rc5, there're two regression fixes for
MMIO handle 65f9f6febf12, and two important vGPU reset fixes.

Thanks
--
The following changes since commit 26a201a2ba82a801973ce29e1004b64742e81e7e:

  drm/i915/gvt: Extend KBL platform support in GVT-g (2017-07-25 12:39:41 +0800)

are available in the git repository at:

  https://github.com/01org/gvt-linux.git tags/gvt-fixes-2017-08-07

for you to fetch changes up to d6086598d34e1cf9091c7be201f5b2041dc6203e:

  drm/i915/gvt: Change the max length of mmio_reg_rw from 4 to 8 (2017-08-07 
15:50:39 +0800)


gvt-fixes-2017-08-07

- two regression fixes for 65f9f6febf12, one is for display MMIO
  initial value (Tina), another for 64bit MMIO access (Xiong)
- two reset fixes from Chuanxiao


Chuanxiao Dong (2):
  drm/i915/gvt: change resetting to resetting_eng
  drm/i915/gvt: clean workload queue if error happened

Tina Zhang (1):
  drm/i915/gvt: Initialize MMIO Block with HW state

Xiong Zhang (1):
  drm/i915/gvt: Change the max length of mmio_reg_rw from 4 to 8

 drivers/gpu/drm/i915/gvt/execlist.c  | 27 -
 drivers/gpu/drm/i915/gvt/firmware.c  | 11 ++-
 drivers/gpu/drm/i915/gvt/gvt.h   | 14 -
 drivers/gpu/drm/i915/gvt/handlers.c  | 38 +---
 drivers/gpu/drm/i915/gvt/scheduler.c |  3 ++-
 drivers/gpu/drm/i915/gvt/vgpu.c  |  8 +---
 6 files changed, 70 insertions(+), 31 deletions(-)


-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


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

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait timeout to 5s

[Lofstedt, Marta]

> -Original Message-
> From: Zanoni, Paulo R
> Sent: Friday, August 4, 2017 9:56 PM
> To: Lofstedt, Marta ; intel-
> g...@lists.freedesktop.org
> Cc: Latvala, Petri 
> Subject: Re: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC wait
> timeout to 5s
> 
> Em Sex, 2017-08-04 às 09:47 +, Lofstedt, Marta escreveu:
> > +Paolo
> >
> > > -Original Message-
> > > From: Lofstedt, Marta
> > > Sent: Wednesday, June 28, 2017 2:17 PM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Latvala, Petri ; Lofstedt, Marta
> > > 
> > > Subject: [PATCH i-g-t] tests/kms_frontbuffer_tracking: increase FBC
> > > wait timeout to 5s
> > >
> > > The subtests: igt@kms_frontbuffer_tracking@fbc-*draw*
> > > has non-consistent results, pending between fail and pass.
> > > The fails are always due to "FBC disabled".
> > > With this increase in timeout the flip-flop behavior is no longer
> > > reproducible.
> 
> This is a partial revert of:
> 
> 64590c7b768dc8d8dd962f812d5ff5a39e7e8b54
> kms_frontbuffer_tracking: reduce the FBC wait timeout to 2s
> 
> (but there's no need to make it a full revert if you don't need)
> 
> It would be nice to investigate why we're needing 5 seconds instead of
> 2 now, the document it in the commit message. Also document that this is a
> partial revert.
Paulo, do you have data backing up that 2 seconds was ever OK, I fail ~1/10 on 
various fbc subtests. 

/Marta
> 
> Acked-by: Paulo Zanoni 
> 
Thanks,

> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101623
> > > Signed-off-by: Marta Lofstedt 
> > > ---
> > >  tests/kms_frontbuffer_tracking.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > b/tests/kms_frontbuffer_tracking.c
> > > index c24e4a81..8bec5d5a 100644
> > > --- a/tests/kms_frontbuffer_tracking.c
> > > +++ b/tests/kms_frontbuffer_tracking.c
> > > @@ -923,7 +923,7 @@ static bool fbc_stride_not_supported(void)
> > >
> > >  static bool fbc_wait_until_enabled(void)  {
> > > - return igt_wait(fbc_is_enabled(), 2000, 1);
> > > + return igt_wait(fbc_is_enabled(), 5000, 1);
> > >  }
> > >
> > >  static bool psr_wait_until_enabled(void)
> > > --
> > > 2.11.0
> >
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx