diff mbox

drm/i915: Special case kernel_context switch request

Message ID 20180522124924.5784-1-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala May 22, 2018, 12:49 p.m. UTC 
From: Mika Kuoppala <mika.kuoppala@intel.com>

When checking if engine is idling on a kernel context,
the last request emitted to it could have been the exact
request to switch into kernel context.

Do not bail out early even if engine has requests,
if the last request was for kernel context.

Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Chris Wilson May 22, 2018, 12:59 p.m. UTC | #1 
Quoting Mika Kuoppala (2018-05-22 13:49:24)
> From: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> When checking if engine is idling on a kernel context,
> the last request emitted to it could have been the exact
> request to switch into kernel context.
> 
> Do not bail out early even if engine has requests,
> if the last request was for kernel context.
> 
> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

I feel there's some redundancy here (in the checks at different levels),
but that's my fault.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Chris Wilson May 22, 2018, 11:46 p.m. UTC | #2 
Quoting Mika Kuoppala (2018-05-22 13:49:24)
> From: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> When checking if engine is idling on a kernel context,
> the last request emitted to it could have been the exact
> request to switch into kernel context.
> 
> Do not bail out early even if engine has requests,
> if the last request was for kernel context.
> 
> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b69b18ef8120..3fe1212b0f7e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -595,7 +595,10 @@ static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
>         lockdep_assert_held(&engine->i915->drm.struct_mutex);
>  
>         list_for_each_entry(ring, active_rings, active_link) {
> -               if (last_request_on_engine(ring->timeline, engine))
> +               struct i915_request *rq =
> +                       last_request_on_engine(ring->timeline, engine);
> +
> +               if (rq && rq->gem_context != engine->i915->kernel_context)
>                         return false;

Ah, this isn't enough yet. The challenge is that we don't want to report
false when there is an active batch followed by the kernel context.

I think we can rely on the kernel request priority being the lowest
here...
-Chris
kernel test robot May 27, 2018, 4:24 p.m. UTC | #3 
Hi Mika,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20180517]
[cannot apply to drm-intel/for-linux-next v4.17-rc6 v4.17-rc5 v4.17-rc4 v4.17-rc6]
[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/Mika-Kuoppala/drm-i915-Special-case-kernel_context-switch-request/20180524-233610
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/i915_gem_context.c: In function 'engine_has_idle_kernel_context':
>> drivers/gpu/drm/i915/i915_gem_context.c:608:15: error: 'struct i915_request' has no member named 'gem_context'
      if (rq && rq->gem_context != engine->i915->kernel_context)
                  ^~

vim +608 drivers/gpu/drm/i915/i915_gem_context.c

   596	
   597	static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
   598	{
   599		struct list_head * const active_rings = &engine->i915->gt.active_rings;
   600		struct intel_ring *ring;
   601	
   602		lockdep_assert_held(&engine->i915->drm.struct_mutex);
   603	
   604		list_for_each_entry(ring, active_rings, active_link) {
   605			struct i915_request *rq =
   606				last_request_on_engine(ring->timeline, engine);
   607	
 > 608			if (rq && rq->gem_context != engine->i915->kernel_context)
   609				return false;
   610		}
   611	
   612		return intel_engine_has_kernel_context(engine);
   613	}
   614	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b69b18ef8120..3fe1212b0f7e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -595,7 +595,10 @@  static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
 	lockdep_assert_held(&engine->i915->drm.struct_mutex);
 
 	list_for_each_entry(ring, active_rings, active_link) {
-		if (last_request_on_engine(ring->timeline, engine))
+		struct i915_request *rq =
+			last_request_on_engine(ring->timeline, engine);
+
+		if (rq && rq->gem_context != engine->i915->kernel_context)
 			return false;
 	}