Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-26 Thread Chris Wilson
On Mon, Aug 25, 2014 at 10:39:39PM +0200, Daniel Vetter wrote:
 On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote:
  On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote:
   These two functions make no sense in an Logical Ring Context  Execlists
   world.
   
   v2: We got rid of lrc_enabled and centralized everything in the sanitized
   i915.enable_execlists instead.
   
   Signed-off-by: Oscar Mateo oscar.ma...@intel.com
   
   v3: Rebased.  Corrected a typo in comment for i915_switch_context and
   added a comment that it should not be called in execlist mode. Added
   WARN_ON if i915_switch_context is called in execlist mode. Moved check
   for execlist mode out of i915_switch_context and into callers. Added
   comment in context_reset explaining why nothing is done in execlist
   mode.
  
  No, this is not the way. The requirement is to reduce the number of
  special cases not increase them. These should be evaluated to be no-ops
  when execlists is used.
 
 I think it's ok-ish for now. Maybe we need to reconsider when we wire up
 lrc reclaim - which is the real user of the switch_context in gpu_idle.
 The problem I have though is that I can't parse the subject of the patch,
 someone please translate that to simplified English for me. I can do the
 replacement while applying.

No, it is not. execlists is badly designed and this is a further symptom
of that.
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-26 Thread Siluvery, Arun

On 26/08/2014 06:59, Chris Wilson wrote:

On Mon, Aug 25, 2014 at 10:39:39PM +0200, Daniel Vetter wrote:

On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote:

On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote:

These two functions make no sense in an Logical Ring Context  Execlists
world.

v2: We got rid of lrc_enabled and centralized everything in the sanitized
i915.enable_execlists instead.

Signed-off-by: Oscar Mateo oscar.ma...@intel.com

v3: Rebased.  Corrected a typo in comment for i915_switch_context and
added a comment that it should not be called in execlist mode. Added
WARN_ON if i915_switch_context is called in execlist mode. Moved check
for execlist mode out of i915_switch_context and into callers. Added
comment in context_reset explaining why nothing is done in execlist
mode.


No, this is not the way. The requirement is to reduce the number of
special cases not increase them. These should be evaluated to be no-ops
when execlists is used.


I think it's ok-ish for now. Maybe we need to reconsider when we wire up
lrc reclaim - which is the real user of the switch_context in gpu_idle.
The problem I have though is that I can't parse the subject of the patch,
someone please translate that to simplified English for me. I can do the
replacement while applying.


No, it is not. execlists is badly designed and this is a further symptom
of that.
-Chris


Thomas is not available and I am replying on his behalf.
Is the following subject is good for this patch?

Don't execute context reset and switch when using Execlists

regards
Arun


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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-25 Thread Daniel Vetter
On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote:
 On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote:
  These two functions make no sense in an Logical Ring Context  Execlists
  world.
  
  v2: We got rid of lrc_enabled and centralized everything in the sanitized
  i915.enable_execlists instead.
  
  Signed-off-by: Oscar Mateo oscar.ma...@intel.com
  
  v3: Rebased.  Corrected a typo in comment for i915_switch_context and
  added a comment that it should not be called in execlist mode. Added
  WARN_ON if i915_switch_context is called in execlist mode. Moved check
  for execlist mode out of i915_switch_context and into callers. Added
  comment in context_reset explaining why nothing is done in execlist
  mode.
 
 No, this is not the way. The requirement is to reduce the number of
 special cases not increase them. These should be evaluated to be no-ops
 when execlists is used.

I think it's ok-ish for now. Maybe we need to reconsider when we wire up
lrc reclaim - which is the real user of the switch_context in gpu_idle.
The problem I have though is that I can't parse the subject of the patch,
someone please translate that to simplified English for me. I can do the
replacement while applying.
-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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-25 Thread Scot Doyle

On Mon, 25 Aug 2014, Daniel Vetter wrote:

On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote:

On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote:

These two functions make no sense in an Logical Ring Context  Execlists
world.

v2: We got rid of lrc_enabled and centralized everything in the sanitized
i915.enable_execlists instead.

Signed-off-by: Oscar Mateo oscar.ma...@intel.com

v3: Rebased.  Corrected a typo in comment for i915_switch_context and
added a comment that it should not be called in execlist mode. Added
WARN_ON if i915_switch_context is called in execlist mode. Moved check
for execlist mode out of i915_switch_context and into callers. Added
comment in context_reset explaining why nothing is done in execlist
mode.


No, this is not the way. The requirement is to reduce the number of
special cases not increase them. These should be evaluated to be no-ops
when execlists is used.


I think it's ok-ish for now. Maybe we need to reconsider when we wire up
lrc reclaim - which is the real user of the switch_context in gpu_idle.
The problem I have though is that I can't parse the subject of the patch,
someone please translate that to simplified English for me. I can do the
replacement while applying.
-Daniel


Render moot usually means something like make obsolete, not sure about 
the rest.

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


Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-20 Thread Chris Wilson
On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote:
 These two functions make no sense in an Logical Ring Context  Execlists
 world.
 
 v2: We got rid of lrc_enabled and centralized everything in the sanitized
 i915.enable_execlists instead.
 
 Signed-off-by: Oscar Mateo oscar.ma...@intel.com
 
 v3: Rebased.  Corrected a typo in comment for i915_switch_context and
 added a comment that it should not be called in execlist mode. Added
 WARN_ON if i915_switch_context is called in execlist mode. Moved check
 for execlist mode out of i915_switch_context and into callers. Added
 comment in context_reset explaining why nothing is done in execlist
 mode.

No, this is not the way. The requirement is to reduce the number of
special cases not increase them. These should be evaluated to be no-ops
when execlists is used.
-Chris

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