Re: [Intel-gfx] [PATCH 02/13] drm/i915: Copy user requested buffers into the error state

[Matt Turner]

On Wed, Apr 12, 2017 at 2:43 PM, Chris Wilson  wrote:
> On Wed, Mar 29, 2017 at 04:56:24PM +0100, Chris Wilson wrote:
>> Introduce a new execobject.flag (EXEC_OBJECT_CAPTURE) that userspace may
>> use to indicate that it wants the contents of this buffer preserved in
>> the error state (/sys/class/drm/cardN/error) following a GPU hang
>> involving this batch.
>>
>> Use this at your discretion, the contents of the error state. although
>> compressed, are allocated with GFP_ATOMIC (i.e. limited) and kept for all
>> eternity (until the error state is destroyed).
>>
>> Based on an earlier patch by Ben Widawsky 
>> Signed-off-by: Chris Wilson 
>> Cc: Ben Widawsky 
>> Cc: Matt Turner 
>> Acked-by: Ben Widawsky 
>> Reviewed-by: Joonas Lahtinen 
>
> I believe Matt has userspace ready to make use of this flag and is happy
> with the current ABI. Matt, are we ready to commit ourselves to this
> interface?

Yes, from my end this interface works well.

I'm able to capture the instruction buffer, and recognize it by
matching the "user" buffer's address with that specified by
STATE_BASE_ADDRESS, and then disassemble the various programs
contained within by inspecting the kernel start pointers.

Thanks for handling the kernel side of things!
___
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: uninitialized value on error path (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: uninitialized value on error path (rev3)
URL   : https://patchwork.freedesktop.org/series/23038/
State : success

== Summary ==

Series 23038v3 drm/i915: uninitialized value on error path
https://patchwork.freedesktop.org/api/1.0/series/23038/revisions/3/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#100652

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

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:427s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:432s
fi-bsw-n3050 total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  
time:571s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:516s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:549s
fi-byt-j1900 total:278  pass:253  dwarn:1   dfail:0   fail:0   skip:24  
time:483s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:479s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:415s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:404s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:431s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:496s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:487s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:458s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:562s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time:572s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:463s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:500s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:433s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:535s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time:405s

4079da1bee731e2bf185411abd9ecda25f247890 drm-tip: 2017y-04m-13d-21h-13m-19s UTC 
integration manifest
8f25cd1 drm/i915: set "ret" correctly on error paths

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4507/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915: set "ret" correctly on error paths

[Dan Carpenter]

If "crtc" is NULL, then my static checker complains that "ret" isn't
initialized on that path.  It doesn't really cause a problem unless
"ret" is somehow set to -EDEADLK which is not likely.

Chris Wilson also noticed another error path where "ret" isn't set
correctly.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 48a546210d8b..2c66de875d3d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9563,6 +9563,7 @@ int intel_get_load_detect_pipe(struct drm_connector 
*connector,
 */
if (!crtc) {
DRM_DEBUG_KMS("no pipe available for load-detect\n");
+   ret = -ENODEV;
goto fail;
}
 
@@ -9619,6 +9620,7 @@ int intel_get_load_detect_pipe(struct drm_connector 
*connector,
DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n");
if (IS_ERR(fb)) {
DRM_DEBUG_KMS("failed to allocate framebuffer for 
load-detection\n");
+   ret = PTR_ERR(fb);
goto fail;
}
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] aubdump: Don't bail if a GEM handle of 0 is passed into execbuf

[Jason Ekstrand]

On Fri, Apr 14, 2017 at 11:34 AM, Rafael Antognolli <
rafael.antogno...@intel.com> wrote:

> Patch is
> Reviewed-by: Rafael Antognolli 
>

Thanks!  Pushed.


> On Fri, Mar 24, 2017 at 04:45:01PM -0700, Jason Ekstrand wrote:
> > A gem handle of 0 can be used to check for whether or not 48-bit
> > addressing is available.  This keeps aubdump from failing on you if
> > you try to do the check.
> > ---
> >  tools/aubdump.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/aubdump.c b/tools/aubdump.c
> > index 8a89b8c..3aca7eb 100644
> > --- a/tools/aubdump.c
> > +++ b/tools/aubdump.c
> > @@ -131,7 +131,6 @@ get_bo(uint32_t handle)
> >
> >   fail_if(handle >= MAX_BO_COUNT, "bo handle too large\n");
> >   bo = [handle];
> > - fail_if(bo->size == 0, "invalid bo handle (%d) in execbuf\n",
> handle);
> >
> >   return bo;
> >  }
> > @@ -442,7 +441,7 @@ dump_execbuffer2(int fd, struct
> drm_i915_gem_execbuffer2 *execbuffer2)
> >   offset = align_u32(offset + bo->size + 4095, 4096);
> >   }
> >
> > - if (bo->map == NULL)
> > + if (bo->map == NULL && bo->size > 0)
> >   bo->map = gem_mmap(fd, obj->handle, 0, bo->size);
> >   fail_if(bo->map == MAP_FAILED, "intel_aubdump: bo mmap
> failed\n");
> >   }
> > @@ -583,7 +582,7 @@ maybe_init(void)
> >   }
> >   fclose(config);
> >
> > - bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
> > + bos = calloc(MAX_BO_COUNT, sizeof(bos[0]));
> >   fail_if(bos == NULL, "intel_aubdump: out of memory\n");
> >  }
> >
> > --
> > 2.5.0.400.gff86faf
> >
> > ___
> > 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] aubdump: Don't bail if a GEM handle of 0 is passed into execbuf

[Rafael Antognolli]

Patch is
Reviewed-by: Rafael Antognolli 

On Fri, Mar 24, 2017 at 04:45:01PM -0700, Jason Ekstrand wrote:
> A gem handle of 0 can be used to check for whether or not 48-bit
> addressing is available.  This keeps aubdump from failing on you if
> you try to do the check.
> ---
>  tools/aubdump.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/aubdump.c b/tools/aubdump.c
> index 8a89b8c..3aca7eb 100644
> --- a/tools/aubdump.c
> +++ b/tools/aubdump.c
> @@ -131,7 +131,6 @@ get_bo(uint32_t handle)
>  
>   fail_if(handle >= MAX_BO_COUNT, "bo handle too large\n");
>   bo = [handle];
> - fail_if(bo->size == 0, "invalid bo handle (%d) in execbuf\n", handle);
>  
>   return bo;
>  }
> @@ -442,7 +441,7 @@ dump_execbuffer2(int fd, struct drm_i915_gem_execbuffer2 
> *execbuffer2)
>   offset = align_u32(offset + bo->size + 4095, 4096);
>   }
>  
> - if (bo->map == NULL)
> + if (bo->map == NULL && bo->size > 0)
>   bo->map = gem_mmap(fd, obj->handle, 0, bo->size);
>   fail_if(bo->map == MAP_FAILED, "intel_aubdump: bo mmap 
> failed\n");
>   }
> @@ -583,7 +582,7 @@ maybe_init(void)
>   }
>   fclose(config);
>  
> - bos = malloc(MAX_BO_COUNT * sizeof(bos[0]));
> + bos = calloc(MAX_BO_COUNT, sizeof(bos[0]));
>   fail_if(bos == NULL, "intel_aubdump: out of memory\n");
>  }
>  
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 v5 17/18] drm/i915: Watchdog timeout: DRM kernel interface to set the timeout

[Michel Thierry]

On 14/04/17 09:05, Daniele Ceraolo Spurio wrote:



On 24/03/17 18:30, Michel Thierry wrote:

Final enablement patch for GPU hang detection using watchdog timeout.
Using the gem_context_setparam ioctl, users can specify the desired
timeout value in microseconds, and the driver will do the conversion to
'timestamps'.

The recommended default watchdog threshold for video engines is 6 us,
since this has been _empirically determined_ to be a good compromise for
low-latency requirements and low rate of false positives. The default
register value is ~106000us and the theoretical max value (all 1s) is
353 seconds.

v2: Fixed get api to return values in microseconds. Threshold updated to
be per context engine. Check for u32 overflow. Capture ctx threshold
value in error state.

Signed-off-by: Tomas Elf 
Signed-off-by: Arun Siluvery 
Signed-off-by: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 78
+
 drivers/gpu/drm/i915/i915_gem_context.h | 20 +
 drivers/gpu/drm/i915/i915_gpu_error.c   | 11 +++--
 drivers/gpu/drm/i915/intel_lrc.c|  2 +-
 include/uapi/drm/i915_drm.h |  1 +
 6 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index b43c37a911bb..1741584d858f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1039,6 +1039,7 @@ struct i915_gpu_state {
 int ban_score;
 int active;
 int guilty;
+int watchdog_threshold;
 } context;

 struct drm_i915_error_object {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index edbed85a1c88..f5c126c0c681 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -422,6 +422,78 @@ i915_gem_context_create_gvt(struct drm_device *dev)
 return ctx;
 }

+/* Return the timer count threshold in microseconds. */
+int i915_gem_context_get_watchdog(struct i915_gem_context *ctx,
+  struct drm_i915_gem_context_param *args)
+{
+struct drm_i915_private *dev_priv = ctx->i915;
+struct intel_engine_cs *engine;
+enum intel_engine_id id;
+u32 threshold_in_us[I915_NUM_ENGINES];
+
+if (!dev_priv->engine[VCS]->emit_start_watchdog)
+return -ENODEV;
+
+for_each_engine(engine, dev_priv, id) {
+struct intel_context *ce = >engine[id];
+
+threshold_in_us[id] = watchdog_to_us(ce->watchdog_threshold);
+}
+
+mutex_unlock(_priv->drm.struct_mutex);
+if (__copy_to_user(u64_to_user_ptr(args->value),
+   _in_us,
+   sizeof(threshold_in_us))) {
+mutex_lock(_priv->drm.struct_mutex);
+return -EFAULT;
+}
+mutex_lock(_priv->drm.struct_mutex);
+args->size = sizeof(threshold_in_us);
+
+return 0;
+}
+
+/*
+ * Based on time out value in microseconds (us) calculate
+ * timer count thresholds needed based on core frequency.
+ * Watchdog can be disabled by setting it to 0.
+ */
+int i915_gem_context_set_watchdog(struct i915_gem_context *ctx,
+  struct drm_i915_gem_context_param *args)
+{
+struct drm_i915_private *dev_priv = ctx->i915;
+struct intel_engine_cs *engine;
+enum intel_engine_id id;
+u32 threshold_in_us[I915_NUM_ENGINES];
+
+if (!dev_priv->engine[VCS]->emit_start_watchdog)
+return -ENODEV;
+else if (args->size < sizeof(threshold_in_us))
+return -EINVAL;


won't we break userspace with this check if we ever get more engines on
a new platform and thus bump I915_NUM_ENGINES?

Thanks,
Daniele



There's a v3 of this patch with Chris feedback,
https://patchwork.freedesktop.org/patch/148805/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [maintainer-tools PATCH v3 2/2] dim: Curate and insert tags into patch(es)

[Sean Paul]

Launch $EDITOR when extracting tags to curate the tags immediately. Once the
tags are proper, automatically add them before the committer's Signed-off-by 
line
to all patches in the range.

Changes in v2:
- Append the tags before the committer's SoB (Ville)
- Make launching $EDITOR contingent on -i flag (Ville/Jani)
- Fix tty issues when launching editor

Changes in v3:
- Don't overload interactive mode (add new edit mode) (Jani)
- Don't default to vi, use non-edit mode if EDITOR not set (Jani)
- Fix newline escaping in insert_extracted_tags

Signed-off-by: Sean Paul 
---
 dim | 49 +++--
 dim.rst |  5 +
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/dim b/dim
index 750f737..d791df3 100755
--- a/dim
+++ b/dim
@@ -144,13 +144,17 @@ INTERACTIVE=
 DRY=
 FORCE=
 HELP=
+EDIT=
 
-while getopts hdfis opt; do
+while getopts hdefis opt; do
case "$opt" in
d)
DRY_RUN=--dry-run
DRY=echo
;;
+   e)
+   EDIT=1
+   ;;
f)
FORCE=1
;;
@@ -670,13 +674,23 @@ function dim_push_fixes
dim_push_branch drm-intel-fixes "$@"
 }
 
+function get_committer_email
+{
+   local committer_email
+
+   if ! committer_email=$(git config --get user.email) ; then
+   committer_email=$EMAIL
+   fi
+   echo -n $committer_email
+}
+
 # ensure we're on branch $1, and apply patches. the rest of the arguments are
 # passed to git am.
 dim_alias_ab=apply-branch
 dim_alias_sob=apply-branch
 function dim_apply_branch
 {
-   local branch file message_id commiter_email patch_from sob rv
+   local branch file message_id committer_email patch_from sob rv
 
branch=${1:?$usage}
shift
@@ -688,13 +702,10 @@ function dim_apply_branch
cat > $file
 
message_id=$(message_get_id $file)
-
-   if ! commiter_email=$(git config --get user.email) ; then
-   commiter_email=$EMAIL
-   fi
+   committer_email=$(get_committer_email)
 
patch_from=$(grep "From:" "$file" | head -1)
-   if [[ "$patch_from" != *"$commiter_email"* ]] ; then
+   if [[ "$patch_from" != *"$committer_email"* ]] ; then
sob=-s
fi
 
@@ -1213,6 +1224,15 @@ function rangeish()
fi
 }
 
+function insert_extracted_tags
+{
+   local committer_email new_tags sob
+   committer_email=$(get_committer_email)
+   new_tags=$(awk '{ORS="\\n"} {print $0}' $1 | head -c-3)
+   sob="Signed-off-by: .*<$committer_email>"
+   awk "/$sob/{p++} p==1{print \"$new_tags\"; p++} p!=1{print}"
+}
+
 function dim_extract_tags
 {
local branch range file tags
@@ -1234,9 +1254,18 @@ function dim_extract_tags
return 0
fi
 
-   tags=$(printf -- "# *** extracted tags ***\n%s" "$tags")
-
-   git filter-branch -f --msg-filter "cat ; echo \"$tags\"" $range
+   # If edit is selected, launch an editor to allow tag editing
+   # If it's not, just append the tags at the bottom of the commit
+   if [ "$EDIT" ] && [ -n $EDITOR ]; then
+   echo "$tags" > $file
+   $EDITOR $file >/dev/ttyhttps://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/15] drm: Remove drm_modeset_(un)lock_crtc

[Mike Lothian]

Hi

X no longer starts for me and I've bisected it back to this commit

During bisect I ended up on commits where my laptop would lockup during boot

Regards

Mike

On Tue, 4 Apr 2017 at 06:39 Daniel Vetter  wrote:

> On Tue, Apr 4, 2017 at 12:13 AM, kbuild test robot  wrote:
> > [if your patch is applied to the wrong git tree, please drop us a note
> to help improve the system]
>
> It should compile just fine on latest linux-next (if there is one)
> where this code in vmwgfx is already removed. Well you just need the
> latest drm-next from Dave Airlie.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 <+41%2079%20365%2057%2048> - http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 17/18] drm/i915: Watchdog timeout: DRM kernel interface to set the timeout

[Chris Wilson]

On Fri, Apr 14, 2017 at 09:05:06AM -0700, Daniele Ceraolo Spurio wrote:
> On 24/03/17 18:30, Michel Thierry wrote:
> >+/*
> >+ * Based on time out value in microseconds (us) calculate
> >+ * timer count thresholds needed based on core frequency.
> >+ * Watchdog can be disabled by setting it to 0.
> >+ */
> >+int i915_gem_context_set_watchdog(struct i915_gem_context *ctx,
> >+  struct drm_i915_gem_context_param *args)
> >+{
> >+struct drm_i915_private *dev_priv = ctx->i915;
> >+struct intel_engine_cs *engine;
> >+enum intel_engine_id id;
> >+u32 threshold_in_us[I915_NUM_ENGINES];
> >+
> >+if (!dev_priv->engine[VCS]->emit_start_watchdog)
> >+return -ENODEV;
> >+else if (args->size < sizeof(threshold_in_us))
> >+return -EINVAL;
> 
> won't we break userspace with this check if we ever get more engines
> on a new platform and thus bump I915_NUM_ENGINES?

Not really. Userspace uses a 2 step process to first determine the array
length it needs to pass to the kernel. It will just fill those rings it
doesn't know about with 0.

The alternative to using a fixed length array is using an array of
(engine-id, threshold) pairs. Which is probably going to be more
convenient to userspace.
-Chris

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

Re: [Intel-gfx] [PATCH v5 17/18] drm/i915: Watchdog timeout: DRM kernel interface to set the timeout

[Daniele Ceraolo Spurio]

On 24/03/17 18:30, Michel Thierry wrote:

Final enablement patch for GPU hang detection using watchdog timeout.
Using the gem_context_setparam ioctl, users can specify the desired
timeout value in microseconds, and the driver will do the conversion to
'timestamps'.

The recommended default watchdog threshold for video engines is 6 us,
since this has been _empirically determined_ to be a good compromise for
low-latency requirements and low rate of false positives. The default
register value is ~106000us and the theoretical max value (all 1s) is
353 seconds.

v2: Fixed get api to return values in microseconds. Threshold updated to
be per context engine. Check for u32 overflow. Capture ctx threshold
value in error state.

Signed-off-by: Tomas Elf 
Signed-off-by: Arun Siluvery 
Signed-off-by: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 78 +
 drivers/gpu/drm/i915/i915_gem_context.h | 20 +
 drivers/gpu/drm/i915/i915_gpu_error.c   | 11 +++--
 drivers/gpu/drm/i915/intel_lrc.c|  2 +-
 include/uapi/drm/i915_drm.h |  1 +
 6 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b43c37a911bb..1741584d858f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1039,6 +1039,7 @@ struct i915_gpu_state {
int ban_score;
int active;
int guilty;
+   int watchdog_threshold;
} context;

struct drm_i915_error_object {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index edbed85a1c88..f5c126c0c681 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -422,6 +422,78 @@ i915_gem_context_create_gvt(struct drm_device *dev)
return ctx;
 }

+/* Return the timer count threshold in microseconds. */
+int i915_gem_context_get_watchdog(struct i915_gem_context *ctx,
+ struct drm_i915_gem_context_param *args)
+{
+   struct drm_i915_private *dev_priv = ctx->i915;
+   struct intel_engine_cs *engine;
+   enum intel_engine_id id;
+   u32 threshold_in_us[I915_NUM_ENGINES];
+
+   if (!dev_priv->engine[VCS]->emit_start_watchdog)
+   return -ENODEV;
+
+   for_each_engine(engine, dev_priv, id) {
+   struct intel_context *ce = >engine[id];
+
+   threshold_in_us[id] = watchdog_to_us(ce->watchdog_threshold);
+   }
+
+   mutex_unlock(_priv->drm.struct_mutex);
+   if (__copy_to_user(u64_to_user_ptr(args->value),
+  _in_us,
+  sizeof(threshold_in_us))) {
+   mutex_lock(_priv->drm.struct_mutex);
+   return -EFAULT;
+   }
+   mutex_lock(_priv->drm.struct_mutex);
+   args->size = sizeof(threshold_in_us);
+
+   return 0;
+}
+
+/*
+ * Based on time out value in microseconds (us) calculate
+ * timer count thresholds needed based on core frequency.
+ * Watchdog can be disabled by setting it to 0.
+ */
+int i915_gem_context_set_watchdog(struct i915_gem_context *ctx,
+ struct drm_i915_gem_context_param *args)
+{
+   struct drm_i915_private *dev_priv = ctx->i915;
+   struct intel_engine_cs *engine;
+   enum intel_engine_id id;
+   u32 threshold_in_us[I915_NUM_ENGINES];
+
+   if (!dev_priv->engine[VCS]->emit_start_watchdog)
+   return -ENODEV;
+   else if (args->size < sizeof(threshold_in_us))
+   return -EINVAL;


won't we break userspace with this check if we ever get more engines on 
a new platform and thus bump I915_NUM_ENGINES?


Thanks,
Daniele


+
+   mutex_unlock(_priv->drm.struct_mutex);
+   if (copy_from_user(_in_us,
+  u64_to_user_ptr(args->value),
+  sizeof(threshold_in_us))) {
+   mutex_lock(_priv->drm.struct_mutex);
+   return -EFAULT;
+   }
+   mutex_lock(_priv->drm.struct_mutex);
+
+   /* not supported in blitter engine */
+   if (threshold_in_us[BCS] != 0)
+   return -EINVAL;
+
+   for_each_engine(engine, dev_priv, id) {
+   struct intel_context *ce = >engine[id];
+
+   ce->watchdog_threshold =
+   watchdog_to_clock_counts((u64)threshold_in_us[id]);
+   }
+
+   return 0;
+}
+
 int i915_gem_context_init(struct drm_i915_private *dev_priv)
 {
struct i915_gem_context *ctx;
@@ -1061,6 +1133,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device 
*dev, void *data,
case I915_CONTEXT_PARAM_BANNABLE:
args->value = i915_gem_context_is_bannable(ctx);
 

Re: [Intel-gfx] [maintainer-tools PATCH] dim: Expand drm-misc branch explanations

[Sean Paul]

On Mon, Apr 10, 2017 at 12:05:43PM -0400, Sean Paul wrote:
> Add a bit more colour to the -misc branch explanations, and add a merge 
> timeline
> similar to the chart used in drm-intel.
> 
> Signed-off-by: Sean Paul 
> ---

Friendly review ping.

Sean

> 
> 
> I've compiled the with this patch and hosted the output here:
>   https://people.freedesktop.org/~seanpaul/drm-misc.html
> 
> 
> 
> 
>  Makefile   |  5 
>  drm-misc-timeline.json | 67 
> ++
>  drm-misc-timeline.rst  | 29 ++
>  drm-misc.rst   | 35 +++---
>  4 files changed, 127 insertions(+), 9 deletions(-)
>  create mode 100644 drm-misc-timeline.json
>  create mode 100644 drm-misc-timeline.rst
> 
> diff --git a/Makefile b/Makefile
> index 4291049..e079e35 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -17,6 +17,11 @@ drm-intel.html: drm-intel.rst drm-intel-flow.svg 
> drm-intel-timeline.rst drm-inte
>   rst2html $< > $@
>   sed -i 's/  
> +# the sed bit here is a hack to make wavedrom process the timeline
> +drm-misc.html: drm-misc.rst drm-misc-timeline.rst drm-misc-timeline.json
> + rst2html $< > $@
> + sed -i 's/ +
>  dim.html: dim.rst
>  
>  SC_EXCLUDE := \
> diff --git a/drm-misc-timeline.json b/drm-misc-timeline.json
> new file mode 100644
> index 000..7b039ce
> --- /dev/null
> +++ b/drm-misc-timeline.json
> @@ -0,0 +1,67 @@
> +{
> + head: {
> + text: 'drm-misc upstream merge timeline',
> + tock: -2,
> + },
> +
> + foot: {
> + text: 'time in weeks, releases/rc at the end of each week',
> + tock: -2,
> + },
> +
> + signal: [
> + [ 'upstreams',
> + { name: 'linus',
> +   wave: 'x|==.=|===.',
> +   data: ['release', 'merge window, -rc1', '-rc2',
> +  '-rc3', '-rc4', '-rc5', '', '-rcN',
> +  'release + 1', 'merge window'],
> +   node: '123456.8.90' },
> +
> + { name: 'drm-fixes',
> +   wave: '0|..=.0...=',
> +   data: ['drm fixes for release + 1'],
> +   node: 'abcde.f...p' },
> +
> + { name: 'drm-next',
> +   wave: '=..0.=...0.',
> +   data: ['drm features for release + 1',
> +  'drm features for release + 2'],
> +   node: '...g.stuvw"..h.' }
> + ],
> +
> + { name: 'drm-misc-fixes',
> +   wave: '0|.=.0.',
> +   data: ['drm-misc fixes for release + 1'],
> +   node: '...ijklm.n.' },
> +
> + { name: 'drm-misc-next-fixes',
> +   wave: '=.xxx=..xxx',
> +   data: ['drm-misc fixes for release + 1',
> +  'drm-misc fixes for release + 2'],
> +   node: '..o.q..' },
> +
> + { name: 'drm-misc-next',
> +   wave: '==.',
> +   data: ['drm-misc features for release + 2',
> +  'drm-misc features for release + 3'],
> +   node: 'rxyz;:.' },
> + ],
> +
> + edge: [
> + /* -fixes to linus */
> + 'a~>2', 'b~>3', 'c~>4', 'd~>5', 'e~>6', 'f~>8',
> + /* -next to linus */
> + 'g~>1 feature merge', 'h~>0 feature merge',
> + /* misc-fixes to -next */
> + 'i~>a', 'j~>b', 'k~>c', 'l~>d', 'm~>e', 'n~>f',
> + /* misc-next-fixes to -next */
> + 'o~>g', 'q~>p',
> + /* misc-next to -next */
> + 'r~>s', 'x~>t', 'y~>u', 'z~>v', ';~>w', ':~>"'
> +
> + ],
> +
> + config: { hscale:3 },
> +}
> +
> diff --git a/drm-misc-timeline.rst b/drm-misc-timeline.rst
> new file mode 100644
> index 000..6972777
> --- /dev/null
> +++ b/drm-misc-timeline.rst
> @@ -0,0 +1,29 @@
> +.. raw:: html
> +
> + 
> + /* Embedded WaveDrom skin from http://wavedrom.com/skins/default.js */
> +
> +.. raw:: html
> + :url: http://wavedrom.com/skins/default.js
> +
> +.. raw:: html
> +
> + 
> + 
> + /* Embedded WaveDrom engine from http://wavedrom.com/WaveDrom.js */
> +
> +.. raw:: html
> + :url: http://wavedrom.com/WaveDrom.js
> +
> +.. raw:: html
> +
> + 
> + 
> +
> +.. raw:: html
> + :file: drm-misc-timeline.json
> +
> +.. raw:: html
> +
> + 
> +
> diff --git a/drm-misc.rst b/drm-misc.rst
> index 45ea795..caba8dc 100644
> --- a/drm-misc.rst
> +++ b/drm-misc.rst
> @@ -38,20 

Re: [Intel-gfx] [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)

[Julia Lawall]

It looks like >cmdr_lock should be released at line 512 if it has
not been released otherwise.  The lock was taken at line 438.

julia

-- Forwarded message --
Date: Fri, 14 Apr 2017 22:21:44 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
call sites

Hi Logan,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc6]
[cannot apply to next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago

>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
   drivers/target/target_core_user.c:512:2-8: preceding lock on line 471

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
vim +512 drivers/target/target_core_user.c

7c9e7a6f Andy Grover  2014-10-01  432   
sizeof(struct tcmu_cmd_entry));
7c9e7a6f Andy Grover  2014-10-01  433   command_size = base_command_size
7c9e7a6f Andy Grover  2014-10-01  434   + 
round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
7c9e7a6f Andy Grover  2014-10-01  435
7c9e7a6f Andy Grover  2014-10-01  436   WARN_ON(command_size & 
(TCMU_OP_ALIGN_SIZE-1));
7c9e7a6f Andy Grover  2014-10-01  437
7c9e7a6f Andy Grover  2014-10-01 @438   spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  439
7c9e7a6f Andy Grover  2014-10-01  440   mb = udev->mb_addr;
7c9e7a6f Andy Grover  2014-10-01  441   cmd_head = mb->cmd_head % 
udev->cmdr_size; /* UAM */
26418649 Sheng Yang   2016-02-26  442   data_length = 
se_cmd->data_length;
26418649 Sheng Yang   2016-02-26  443   if (se_cmd->se_cmd_flags & 
SCF_BIDI) {
26418649 Sheng Yang   2016-02-26  444   
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
26418649 Sheng Yang   2016-02-26  445   data_length += 
se_cmd->t_bidi_data_sg->length;
26418649 Sheng Yang   2016-02-26  446   }
554617b2 Andy Grover  2016-08-25  447   if ((command_size > 
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover  2016-08-25  448   data_length > 
udev->data_size) {
554617b2 Andy Grover  2016-08-25  449   pr_warn("TCMU: Request 
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover  2016-08-25  450   "cmd ring/data 
area\n", command_size, data_length,
7c9e7a6f Andy Grover  2014-10-01  451   
udev->cmdr_size, udev->data_size);
554617b2 Andy Grover  2016-08-25  452   
spin_unlock_irq(>cmdr_lock);
554617b2 Andy Grover  2016-08-25  453   return 
TCM_INVALID_CDB_FIELD;
554617b2 Andy Grover  2016-08-25  454   }
7c9e7a6f Andy Grover  2014-10-01  455
26418649 Sheng Yang   2016-02-26  456   while 
(!is_ring_space_avail(udev, command_size, data_length)) {
7c9e7a6f Andy Grover  2014-10-01  457   int ret;
7c9e7a6f Andy Grover  2014-10-01  458   DEFINE_WAIT(__wait);
7c9e7a6f Andy Grover  2014-10-01  459
7c9e7a6f Andy Grover  2014-10-01  460   
prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
7c9e7a6f Andy Grover  2014-10-01  461
7c9e7a6f Andy Grover  2014-10-01  462   pr_debug("sleeping for 
ring space\n");
7c9e7a6f Andy Grover  2014-10-01  463   
spin_unlock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  464   ret = 
schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
7c9e7a6f Andy Grover  2014-10-01  465   
finish_wait(>wait_cmdr, &__wait);
7c9e7a6f Andy Grover  2014-10-01  466   if (!ret) {
7c9e7a6f Andy Grover  2014-10-01  467   pr_warn("tcmu: 
command timed out\n");
02eb924f Andy Grover  2016-10-06  468   return 
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover  2014-10-01  469   }
7c9e7a6f Andy Grover  2014-10-01  470
7c9e7a6f Andy Grover  2014-10-01  471   
spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  472
7c9e7a6f Andy Grover  2014-10-01  473   /* We dropped 
cmdr_lock, cmd_head is stale */
7c9e7a6f Andy Grover  2014-10-01  474   cmd_head = mb->cmd_head 
% udev->cmdr_size; /* UAM */
7c9e7a6f Andy Grover

Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual aperture size under gvt environment

[Li, Weinan Z]

> -Original Message-
> From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com]
> Sent: Thursday, April 13, 2017 6:11 PM
> To: Li, Weinan Z ; Chris Wilson  wilson.co.uk>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual aperture size
> under gvt environment
> 
> On to, 2017-04-13 at 01:01 +, Li, Weinan Z wrote:
> > >
> > > -Original Message-
> > > From: Joonas Lahtinen [mailto:joonas.lahti...@linux.intel.com]
> > > Sent: Wednesday, April 12, 2017 6:19 PM
> > > To: Chris Wilson ; Li, Weinan Z
> > > 
> > > Cc: intel-gfx@lists.freedesktop.org;
> > > intel-gvt-...@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: return the actual
> > > aperture size under gvt environment
> > >
> > > On ke, 2017-04-12 at 09:53 +0100, Chris Wilson wrote:
> > > >
> > > > On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> > > > >
> > > > >
> > > > > I915_GEM_GET_APERTURE ioctl is used to probe aperture size from
> > > userspace.
> > > >
> > > > >
> > > > > Some applications like OpenCL use this information to know how
> > > > > much GM resource can it use.
> > > >
> > > > That's a userspace bug.
> > >
> > > Yes, a new property might be in place. I don't think we can go and
> > > change the meaning of a parameter just like that.
> > >
> > > 
> > >
> > Here I don’t want to change the meaning of I915_GEM_GET_APERTURE, but
> > for the ioctl, We need to return the actual available aperture size exclude 
> > the
> reserved space by GVT balloon.
> 
> IOCTLs represent the ABI contract we have with userspace. It has previously
> returned size of the aperture, so we can't change it to be something else 
> (like
> the usable size of aperture as proposed here).
> 
> Somebody might be doing an assert that any address in aperture is below
> I915_GEM_GET_APERTURE returned value, which has previously been correct,
> but would be broken after this change. There are also potentially other things
> consuming the aperture than VGT ballooning, so the UMDs would still be
> misbehaving.
> 
> Shouldn't they rather be doing these decisions based on aper_available_size?
> 
Known your mean, if we return the value as below:
-   args->aper_size = ggtt->base.total;
+   args->aper_size = ggtt->base.total - ggtt->base.reserved;
Then userspace may use 'args->aper_size' as the MAX aperture addr, it may cause 
other issues.
In GVT with balloon the aperture addr still be from 0 to ggtt->base.total.
If it's expected behavior, change the available aperture size may avoid this.
args->aper_size = ggtt->base.total;
args->aper_available_size = args->aper_size - ggtt->base.reserved - 
pinned;
> 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 V5] drm/i915: Disable stolen memory when i915 runs on qemu

[Zhang, Xiong Y]

> On Thu, 13 Apr 2017 05:44:18 +
> "Zhang, Xiong Y"  wrote:
> 
> > > On Wed, 12 Apr 2017 20:20:00 +0800
> > > Xiong Zhang  wrote:
> > >
> > > > Stolen memory isn't a standard pci resource and exists in RMRR which
> has
> > > > identity mapping in iommu table, IGD could access stolen memory in
> host
> > > OS.
> > > > While according to 'commit c875d2c1b808 ("iommu/vt-d: Exclude
> devices
> > > using
> > > > RMRRs from IOMMU API domains")',RMRR isn't supported by kvm, then
> > > both EPT
> > > > and guest iommu domain table lack of maaping for stolen memory in
> kvm
> > > IGD
> > > > passthrough environment. If IGD access stolen memory in such
> environment,
> > > > many iommu exceptions exist in host dmesg and gpu hang exists also.
> > > > DMAR: [DMA Read] Request device [00:02.0] fault addr da012000
> > > > [fault reason 05] PTE Write access is not set
> > > > DMAR: [DMA Read] Request device [00:02.0] fault addr da2df000
> > > > [fault reason 06] PTE Read access is not set
> > > >
> > > > So stolen memory should be disabled in KVM IGD passthrough
> environment,
> > > > this patch detects such environment through the existence of qemu
> > > emulated
> > > > isa bridge.
> > > >
> > > > When the real ISA bridge is also passed through to guest, guest will 
> > > > have
> > > > two isa bridges: emulated and real. Qemu guarantees the
> busnum:devnum.
> > > > funcnum of emulated isa bridge is always less than the real one. Then
> > > > emulated isa bridge is always detected first by pci_get_class(ISA). So
> > > > stolen memory will be disabled in this case also.
> > >
> > > Where does QEMU make this guarantee or any sort of guarantee wrt the
> > > ISA bridge?  Thanks,
> > >
> > > Alex
> > >
> > [Zhang, Xiong Y] In my guest environment I always see emulated devices
> > are at head of pci device list, the passed through devices are at tail. 
> > Even if
> > I want to assign the passed IGD to 00:02.0, the qemu tell me 00:02.0 has
> already
> > occupied by emulated graphic card.
> > If I pass through real ISA bridge to guest, the emulated ISA bridge is at
> 00:01.0,
> > While real ISA bridge is at 00:04.0.
> > Then I checked the code: emulated devices are created in pc_init1() 
> > function,
> it
> > creates host_bridge firstly, create isa_bridge secondly, create all other
> devices following.
> > So I think Qemu could guarantee. Now I'm suspect it, and need your coach.
> 
> So you're calling the current default behavior a guarantee.  That's not
> valid, it ignores that we might have future chipsets that do things
> differently and it ignores that the user can override some of those
> defaults and specify addresses for devices that may not match your
> expectations.  There is no agreement with the QEMU community to make
> this a stable feature of the VM.  What about using smbios information
> or detecting kvm (or any hypervisor) via the hypervisor MSRs?  You could
> maybe use the VM PCI host bridge and figure out that this version of
> IGD never shipped on 440fx or q35, but then you'll have the maintenance
> headache of updating the code for any new chipset QEMU decides to
> implement. Thanks,
> 
[Zhang, Xiong Y] Thanks for your teach and propose.
For smbios, could you teach me which type and field could be used ?
For hypervisor MSRs, from 
https://www.kernel.org/doc/Documentation/virtual/kvm/msr.txt,
We should use cupid(0x4001) first, then use rdmsr(), in this case we could 
use
cupid(0x4000) directly to detect kvm. But I don't know whether community 
could accept
it or not ?

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