[PATCH 1/2] drm/i915: Switch to fake context on older gens

[PATCH 1/2] drm/i915: Switch to fake context on older gens

[Mika Kuoppala]

We used to have per file descriptor hang stats for the
i915_get_reset_stats_ioctl() and for default context banning.

commit 0eea67eb26000657079b7fc41079097942339452
Author: Ben Widawsky <ben@bwidawsk.net>
Date:   Fri Dec 6 14:11:19 2013 -0800

    drm/i915: Create a per file_priv default context

made having separate hangstats in file_private redundant
as i915_hw_context already contained hangstats. So

commit c482972a086e03e6a6d27e4f7af2d868bf659648
Author: Ben Widawsky <benjamin.widawsky@intel.com>
Date:   Fri Dec 6 14:11:20 2013 -0800

    drm/i915: Piggy back hangstats off of contexts

consolidated the hangstats and enabled further improvements.

commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e
Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Date:   Thu Jan 30 16:01:15 2014 +0200

    drm/i915: Use i915_hw_context to set reset stats

tried to reap full benefits of consolidation but fell short
as we never 'switch' to the fake private context on gens
that don't have hw_contexts, so request->ctx remained NULL
on those.

Fix this by 'switching' to fake context so that when
request is submitted to ring, proper context gets assigned
to it.

References: https://bugs.freedesktop.org/show_bug.cgi?id=76055
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ce41cff..b5a5837 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -775,9 +775,11 @@ int i915_switch_context(struct intel_ring_buffer *ring,
 
 	BUG_ON(file && to == NULL);
 
-	/* We have the fake context, but don't supports switching. */
-	if (!HAS_HW_CONTEXTS(ring->dev))
+	/* We have the fake context */
+	if (!HAS_HW_CONTEXTS(ring->dev)) {
+		ring->last_context = to;
 		return 0;
+	}
 
 	return do_switch(ring, to);
 }
-- 
1.7.9.5

[PATCH 2/2] drm/i915: Return -ENOENT for unknown contexts

[Mika Kuoppala]

If hw_contexts are disabled, we always return the per file
descriptor default context stats. Make sure that the context
is correctly given and fail accordingly if not.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b5a5837..a8e625d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -573,8 +573,12 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
 {
 	struct i915_hw_context *ctx;
 
-	if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev))
+	if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev)) {
+		if (id != DEFAULT_CONTEXT_ID)
+			return ERR_PTR(-ENOENT);
+
 		return file_priv->private_default_ctx;
+	}
 
 	ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
 	if (!ctx)
-- 
1.7.9.5

Re: [PATCH 2/2] drm/i915: Return -ENOENT for unknown contexts

[Daniel Vetter]

On Fri, Mar 14, 2014 at 04:22:11PM +0200, Mika Kuoppala wrote:
> If hw_contexts are disabled, we always return the per file
> descriptor default context stats. Make sure that the context
> is correctly given and fail accordingly if not.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Hm, does this fix a testcase somewhere? If sou can you specify which one?
Of there's no such thing I guess we should try to add a new testcase for
this for pre-gen6 ...
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b5a5837..a8e625d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -573,8 +573,12 @@ i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
>  {
>  	struct i915_hw_context *ctx;
>  
> -	if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev))
> +	if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev)) {
> +		if (id != DEFAULT_CONTEXT_ID)
> +			return ERR_PTR(-ENOENT);
> +
>  		return file_priv->private_default_ctx;
> +	}
>  
>  	ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
>  	if (!ctx)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] tests/gem_reset_stats: run non hw context tests also on older gens

[Mika Kuoppala]

To gain more coverage on interface, default context and banning.
As there is no proper reset support for gen <= 3, we only
do limited interface testing on those.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/gem_reset_stats.c |   91 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 75 insertions(+), 16 deletions(-)

diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 3719f40..2bb4681 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -51,6 +51,9 @@
 #define RS_BATCH_PENDING (1 << 1)
 #define RS_UNKNOWN       (1 << 2)
 
+static uint32_t devid;
+static bool hw_contexts;
+
 struct local_drm_i915_reset_stats {
 	__u32 ctx_id;
 	__u32 flags;
@@ -101,6 +104,9 @@ static const struct target_ring {
 
 static bool has_context(const struct target_ring *ring)
 {
+	if (!hw_contexts)
+		return false;
+
 	if(ring->exec == I915_EXEC_RENDER)
 		return true;
 
@@ -277,7 +283,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
 
 	srandom(time(NULL));
 
-	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
+	if (intel_gen(devid) >= 8)
 		cmd_len = 3;
 
 	buf = malloc(BUFSIZE);
@@ -960,7 +966,7 @@ static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad)
 
 typedef enum { root = 0, user } cap_t;
 
-static void test_param_ctx(const int fd, const int ctx, const cap_t cap)
+static void _check_param_ctx(const int fd, const int ctx, const cap_t cap)
 {
 	const uint32_t bad = rand() + 1;
 
@@ -981,8 +987,7 @@ static void check_params(const int fd, const int ctx, cap_t cap)
 	igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
 	igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
 
-	test_param_ctx(fd, 0, cap);
-	test_param_ctx(fd, ctx, cap);
+	_check_param_ctx(fd, ctx, cap);
 }
 
 static void _test_param(const int fd, const int ctx)
@@ -1002,7 +1007,7 @@ static void _test_param(const int fd, const int ctx)
 	igt_waitchildren();
 }
 
-static void test_params(void)
+static void test_params_ctx(void)
 {
 	int fd, ctx;
 
@@ -1015,29 +1020,76 @@ static void test_params(void)
 	close(fd);
 }
 
-#define RING_HAS_CONTEXTS current_ring->contexts(current_ring)
-#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false); __VA_ARGS__; } while (0)
+static void test_params(void)
+{
+	int fd;
 
-int fd;
+	fd = drm_open_any();
+	igt_assert(fd >= 0);
 
-igt_main
+	_test_param(fd, 0);
+
+	close(fd);
+
+}
+
+static bool gem_has_hw_contexts(int fd)
 {
 	struct local_drm_i915_gem_context_create create;
-	uint32_t devid;
 	int ret;
 
+	memset(&create, 0, sizeof(create));
+	ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
+
+	if (ret == 0) {
+		drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &create);
+		return true;
+	}
+
+	return false;
+}
+
+static bool gem_has_reset_stats(int fd)
+{
+	struct local_drm_i915_reset_stats rs;
+	int ret;
+
+	/* Carefully set flags and pad to zero, otherwise
+	   we get -EINVAL
+	*/
+	memset(&rs, 0, sizeof(rs));
+
+	ret = drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs);
+	if (ret == 0)
+		return true;
+
+	/* If we get EPERM, we have support but did not
+	   have CAP_SYSADM */
+	if (ret == -1 && errno == EPERM)
+		return true;
+
+	return false;
+}
+
+#define RING_HAS_CONTEXTS (current_ring->contexts(current_ring))
+#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false); __VA_ARGS__; } while (0)
+
+static int fd;
+
+igt_main
+{
 	igt_skip_on_simulation();
 
 	igt_fixture {
+		bool has_reset_stats;
 		fd = drm_open_any();
 		devid = intel_get_drm_devid(fd);
-		igt_require_f(intel_gen(devid) >= 4,
-			      "Architecture %d too old\n", intel_gen(devid));
 
-		ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
-		igt_skip_on_f(ret != 0 && (errno == ENODEV || errno == EINVAL),
-			      "Kernel is too old, or contexts not supported: %s\n",
-			      strerror(errno));
+		hw_contexts = gem_has_hw_contexts(fd);
+		has_reset_stats = gem_has_reset_stats(fd);
+
+		igt_require_f(has_reset_stats,
+			      "No reset stats ioctl support. Too old kernel?\n");
 	}
 
 	igt_subtest("params")
@@ -1052,6 +1104,13 @@ igt_main
 		igt_fixture
 			gem_require_ring(fd, current_ring->exec);
 
+		igt_fixture
+			igt_require_f(intel_gen(devid) >= 4,
+				      "gen %d doesn't support reset\n", intel_gen(devid));
+
+		igt_subtest_f("params-ctx-%s", name)
+			RUN_CTX_TEST(test_params_ctx());
+
 		igt_subtest_f("reset-stats-%s", name)
 			test_rs(4, 1, 0);
 
-- 
1.7.9.5

Re: [PATCH] tests/gem_reset_stats: run non hw context tests also on older gens

[Daniel Vetter]

On Mon, Mar 17, 2014 at 07:09:23PM +0200, Mika Kuoppala wrote:
> To gain more coverage on interface, default context and banning.
> As there is no proper reset support for gen <= 3, we only
> do limited interface testing on those.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Applied, thanks for the test. And can you please list all your outstanding
reset patches on our review board and sign someone up?

Thanks, Daniel

> ---
>  tests/gem_reset_stats.c |   91 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 75 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
> index 3719f40..2bb4681 100644
> --- a/tests/gem_reset_stats.c
> +++ b/tests/gem_reset_stats.c
> @@ -51,6 +51,9 @@
>  #define RS_BATCH_PENDING (1 << 1)
>  #define RS_UNKNOWN       (1 << 2)
>  
> +static uint32_t devid;
> +static bool hw_contexts;
> +
>  struct local_drm_i915_reset_stats {
>  	__u32 ctx_id;
>  	__u32 flags;
> @@ -101,6 +104,9 @@ static const struct target_ring {
>  
>  static bool has_context(const struct target_ring *ring)
>  {
> +	if (!hw_contexts)
> +		return false;
> +
>  	if(ring->exec == I915_EXEC_RENDER)
>  		return true;
>  
> @@ -277,7 +283,7 @@ static int inject_hang_ring(int fd, int ctx, int ring)
>  
>  	srandom(time(NULL));
>  
> -	if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> +	if (intel_gen(devid) >= 8)
>  		cmd_len = 3;
>  
>  	buf = malloc(BUFSIZE);
> @@ -960,7 +966,7 @@ static int _test_params(int fd, int ctx, uint32_t flags, uint32_t pad)
>  
>  typedef enum { root = 0, user } cap_t;
>  
> -static void test_param_ctx(const int fd, const int ctx, const cap_t cap)
> +static void _check_param_ctx(const int fd, const int ctx, const cap_t cap)
>  {
>  	const uint32_t bad = rand() + 1;
>  
> @@ -981,8 +987,7 @@ static void check_params(const int fd, const int ctx, cap_t cap)
>  	igt_assert(ioctl(fd, GET_RESET_STATS_IOCTL, 0) == -1);
>  	igt_assert(_test_params(fd, 0xbadbad, 0, 0) == -ENOENT);
>  
> -	test_param_ctx(fd, 0, cap);
> -	test_param_ctx(fd, ctx, cap);
> +	_check_param_ctx(fd, ctx, cap);
>  }
>  
>  static void _test_param(const int fd, const int ctx)
> @@ -1002,7 +1007,7 @@ static void _test_param(const int fd, const int ctx)
>  	igt_waitchildren();
>  }
>  
> -static void test_params(void)
> +static void test_params_ctx(void)
>  {
>  	int fd, ctx;
>  
> @@ -1015,29 +1020,76 @@ static void test_params(void)
>  	close(fd);
>  }
>  
> -#define RING_HAS_CONTEXTS current_ring->contexts(current_ring)
> -#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false); __VA_ARGS__; } while (0)
> +static void test_params(void)
> +{
> +	int fd;
>  
> -int fd;
> +	fd = drm_open_any();
> +	igt_assert(fd >= 0);
>  
> -igt_main
> +	_test_param(fd, 0);
> +
> +	close(fd);
> +
> +}
> +
> +static bool gem_has_hw_contexts(int fd)
>  {
>  	struct local_drm_i915_gem_context_create create;
> -	uint32_t devid;
>  	int ret;
>  
> +	memset(&create, 0, sizeof(create));
> +	ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
> +
> +	if (ret == 0) {
> +		drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &create);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool gem_has_reset_stats(int fd)
> +{
> +	struct local_drm_i915_reset_stats rs;
> +	int ret;
> +
> +	/* Carefully set flags and pad to zero, otherwise
> +	   we get -EINVAL
> +	*/
> +	memset(&rs, 0, sizeof(rs));
> +
> +	ret = drmIoctl(fd, GET_RESET_STATS_IOCTL, &rs);
> +	if (ret == 0)
> +		return true;
> +
> +	/* If we get EPERM, we have support but did not
> +	   have CAP_SYSADM */
> +	if (ret == -1 && errno == EPERM)
> +		return true;
> +
> +	return false;
> +}
> +
> +#define RING_HAS_CONTEXTS (current_ring->contexts(current_ring))
> +#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false); __VA_ARGS__; } while (0)
> +
> +static int fd;
> +
> +igt_main
> +{
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
> +		bool has_reset_stats;
>  		fd = drm_open_any();
>  		devid = intel_get_drm_devid(fd);
> -		igt_require_f(intel_gen(devid) >= 4,
> -			      "Architecture %d too old\n", intel_gen(devid));
>  
> -		ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
> -		igt_skip_on_f(ret != 0 && (errno == ENODEV || errno == EINVAL),
> -			      "Kernel is too old, or contexts not supported: %s\n",
> -			      strerror(errno));
> +		hw_contexts = gem_has_hw_contexts(fd);
> +		has_reset_stats = gem_has_reset_stats(fd);
> +
> +		igt_require_f(has_reset_stats,
> +			      "No reset stats ioctl support. Too old kernel?\n");
>  	}
>  
>  	igt_subtest("params")
> @@ -1052,6 +1104,13 @@ igt_main
>  		igt_fixture
>  			gem_require_ring(fd, current_ring->exec);
>  
> +		igt_fixture
> +			igt_require_f(intel_gen(devid) >= 4,
> +				      "gen %d doesn't support reset\n", intel_gen(devid));
> +
> +		igt_subtest_f("params-ctx-%s", name)
> +			RUN_CTX_TEST(test_params_ctx());
> +
>  		igt_subtest_f("reset-stats-%s", name)
>  			test_rs(4, 1, 0);
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] drm/i915: Switch to fake context on older gens

[Jani Nikula]

On Fri, 14 Mar 2014, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> We used to have per file descriptor hang stats for the
> i915_get_reset_stats_ioctl() and for default context banning.
>
> commit 0eea67eb26000657079b7fc41079097942339452
> Author: Ben Widawsky <ben@bwidawsk.net>
> Date:   Fri Dec 6 14:11:19 2013 -0800
>
>     drm/i915: Create a per file_priv default context
>
> made having separate hangstats in file_private redundant
> as i915_hw_context already contained hangstats. So
>
> commit c482972a086e03e6a6d27e4f7af2d868bf659648
> Author: Ben Widawsky <benjamin.widawsky@intel.com>
> Date:   Fri Dec 6 14:11:20 2013 -0800
>
>     drm/i915: Piggy back hangstats off of contexts
>
> consolidated the hangstats and enabled further improvements.
>
> commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e
> Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Date:   Thu Jan 30 16:01:15 2014 +0200
>
>     drm/i915: Use i915_hw_context to set reset stats
>
> tried to reap full benefits of consolidation but fell short
> as we never 'switch' to the fake private context on gens
> that don't have hw_contexts, so request->ctx remained NULL
> on those.
>
> Fix this by 'switching' to fake context so that when
> request is submitted to ring, proper context gets assigned
> to it.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76055

I'd say this still applies:

Tested-by: Lu Hua <huax.lu@intel.com>

> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index ce41cff..b5a5837 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -775,9 +775,11 @@ int i915_switch_context(struct intel_ring_buffer *ring,
>  
>  	BUG_ON(file && to == NULL);
>  
> -	/* We have the fake context, but don't supports switching. */
> -	if (!HAS_HW_CONTEXTS(ring->dev))
> +	/* We have the fake context */
> +	if (!HAS_HW_CONTEXTS(ring->dev)) {
> +		ring->last_context = to;
>  		return 0;
> +	}
>  
>  	return do_switch(ring, to);
>  }
> -- 
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 1/2] drm/i915: Switch to fake context on older gens

[Daniel Vetter]

On Fri, Mar 14, 2014 at 04:43:16PM +0200, Jani Nikula wrote:
> On Fri, 14 Mar 2014, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> > We used to have per file descriptor hang stats for the
> > i915_get_reset_stats_ioctl() and for default context banning.
> >
> > commit 0eea67eb26000657079b7fc41079097942339452
> > Author: Ben Widawsky <ben@bwidawsk.net>
> > Date:   Fri Dec 6 14:11:19 2013 -0800
> >
> >     drm/i915: Create a per file_priv default context
> >
> > made having separate hangstats in file_private redundant
> > as i915_hw_context already contained hangstats. So
> >
> > commit c482972a086e03e6a6d27e4f7af2d868bf659648
> > Author: Ben Widawsky <benjamin.widawsky@intel.com>
> > Date:   Fri Dec 6 14:11:20 2013 -0800
> >
> >     drm/i915: Piggy back hangstats off of contexts
> >
> > consolidated the hangstats and enabled further improvements.
> >
> > commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e
> > Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Date:   Thu Jan 30 16:01:15 2014 +0200
> >
> >     drm/i915: Use i915_hw_context to set reset stats
> >
> > tried to reap full benefits of consolidation but fell short
> > as we never 'switch' to the fake private context on gens
> > that don't have hw_contexts, so request->ctx remained NULL
> > on those.
> >
> > Fix this by 'switching' to fake context so that when
> > request is submitted to ring, proper context gets assigned
> > to it.
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76055
> 
> I'd say this still applies:
> 
> Tested-by: Lu Hua <huax.lu@intel.com>

Also if your patch intends to fix a bug, please use a Bugzilla: reference,
I tend to use References only when there's an issue seen in a report, but
it's not the main issue.
-Daniel

> 
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index ce41cff..b5a5837 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -775,9 +775,11 @@ int i915_switch_context(struct intel_ring_buffer *ring,
> >  
> >  	BUG_ON(file && to == NULL);
> >  
> > -	/* We have the fake context, but don't supports switching. */
> > -	if (!HAS_HW_CONTEXTS(ring->dev))
> > +	/* We have the fake context */
> > +	if (!HAS_HW_CONTEXTS(ring->dev)) {
> > +		ring->last_context = to;
> >  		return 0;
> > +	}
> >  
> >  	return do_switch(ring, to);
> >  }
> > -- 
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] drm/i915: Switch to fake context on older gens

[Daniel Vetter]

On Fri, Mar 14, 2014 at 04:42:21PM +0100, Daniel Vetter wrote:
> On Fri, Mar 14, 2014 at 04:43:16PM +0200, Jani Nikula wrote:
> > On Fri, 14 Mar 2014, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> > > We used to have per file descriptor hang stats for the
> > > i915_get_reset_stats_ioctl() and for default context banning.
> > >
> > > commit 0eea67eb26000657079b7fc41079097942339452
> > > Author: Ben Widawsky <ben@bwidawsk.net>
> > > Date:   Fri Dec 6 14:11:19 2013 -0800
> > >
> > >     drm/i915: Create a per file_priv default context
> > >
> > > made having separate hangstats in file_private redundant
> > > as i915_hw_context already contained hangstats. So
> > >
> > > commit c482972a086e03e6a6d27e4f7af2d868bf659648
> > > Author: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Date:   Fri Dec 6 14:11:20 2013 -0800
> > >
> > >     drm/i915: Piggy back hangstats off of contexts
> > >
> > > consolidated the hangstats and enabled further improvements.
> > >
> > > commit 44e2c0705a19e09d7b0f30a591f92e473e5ef89e
> > > Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > Date:   Thu Jan 30 16:01:15 2014 +0200
> > >
> > >     drm/i915: Use i915_hw_context to set reset stats
> > >
> > > tried to reap full benefits of consolidation but fell short
> > > as we never 'switch' to the fake private context on gens
> > > that don't have hw_contexts, so request->ctx remained NULL
> > > on those.
> > >
> > > Fix this by 'switching' to fake context so that when
> > > request is submitted to ring, proper context gets assigned
> > > to it.
> > >
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76055
> > 
> > I'd say this still applies:
> > 
> > Tested-by: Lu Hua <huax.lu@intel.com>
> 
> Also if your patch intends to fix a bug, please use a Bugzilla: reference,
> I tend to use References only when there's an issue seen in a report, but
> it's not the main issue.

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch