Re: [Intel-gfx] [PATCH 5/9] drm/i915: Use a define for the default priority [0]

2017-05-05 Thread Chris Wilson
On Fri, May 05, 2017 at 10:21:32AM +0100, Tvrtko Ursulin wrote:
> 
> On 05/05/2017 10:13, Chris Wilson wrote:
> >On Fri, May 05, 2017 at 11:31:14AM +0300, Mika Kuoppala wrote:
> >>Chris Wilson  writes:
> >>
> >>>On Thu, May 04, 2017 at 04:32:34PM +0300, Joonas Lahtinen wrote:
> On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:
> >Explicitly assign the default priority, and give it a name (macro).
> >
> >Signed-off-by: Chris Wilson 
> 
> 
> 
> > kref_init(>ref);
> > list_add_tail(>link, _priv->context_list);
> > ctx->i915 = dev_priv;
> >+ctx->priority = I915_PRIORITY_DFL;
> 
> I915_PRIORITY_DEFAULT would work better.
> >>>
> >>>On the one hand I have the symmetry with MIN, DFL, MAX, on the other
> >>>hand DFL is plain bizarre.
> >>
> >>DEF?
> >
> >I915_PRIORITY_DEFEAT. I'm perfectly happy just to 0, pesky Tvrtko.
> 
> Will to argue deflated. :) I suggested it for benefit in one of the
> later patches which explicitly compared against zero. if < 0 &&
> !cap_sys_admin or something.. I thought being explicit what zero
> means there would be a good thing.
> 
> DEFAULT or DEF both sounds good to me. Or NORMAL. DFL is not
> entirely new (SIG_DFL) but it does look very weird.

I like I915_PRIORITY_NORMAL.
-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 5/9] drm/i915: Use a define for the default priority [0]

2017-05-05 Thread Tvrtko Ursulin


On 05/05/2017 10:13, Chris Wilson wrote:

On Fri, May 05, 2017 at 11:31:14AM +0300, Mika Kuoppala wrote:

Chris Wilson  writes:


On Thu, May 04, 2017 at 04:32:34PM +0300, Joonas Lahtinen wrote:

On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:

Explicitly assign the default priority, and give it a name (macro).

Signed-off-by: Chris Wilson 





kref_init(>ref);
list_add_tail(>link, _priv->context_list);
ctx->i915 = dev_priv;
+   ctx->priority = I915_PRIORITY_DFL;


I915_PRIORITY_DEFAULT would work better.


On the one hand I have the symmetry with MIN, DFL, MAX, on the other
hand DFL is plain bizarre.


DEF?


I915_PRIORITY_DEFEAT. I'm perfectly happy just to 0, pesky Tvrtko.


Will to argue deflated. :) I suggested it for benefit in one of the 
later patches which explicitly compared against zero. if < 0 && 
!cap_sys_admin or something.. I thought being explicit what zero means 
there would be a good thing.


DEFAULT or DEF both sounds good to me. Or NORMAL. DFL is not entirely 
new (SIG_DFL) but it does look very weird.


Regards,

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


Re: [Intel-gfx] [PATCH 5/9] drm/i915: Use a define for the default priority [0]

2017-05-05 Thread Chris Wilson
On Fri, May 05, 2017 at 11:31:14AM +0300, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > On Thu, May 04, 2017 at 04:32:34PM +0300, Joonas Lahtinen wrote:
> >> On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:
> >> > Explicitly assign the default priority, and give it a name (macro).
> >> > 
> >> > Signed-off-by: Chris Wilson 
> >> 
> >> 
> >> 
> >> >  kref_init(>ref);
> >> >  list_add_tail(>link, _priv->context_list);
> >> >  ctx->i915 = dev_priv;
> >> > +ctx->priority = I915_PRIORITY_DFL;
> >> 
> >> I915_PRIORITY_DEFAULT would work better.
> >
> > On the one hand I have the symmetry with MIN, DFL, MAX, on the other
> > hand DFL is plain bizarre.
> 
> DEF?

I915_PRIORITY_DEFEAT. I'm perfectly happy just to 0, pesky Tvrtko.
-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 5/9] drm/i915: Use a define for the default priority [0]

2017-05-05 Thread Mika Kuoppala
Chris Wilson  writes:

> On Thu, May 04, 2017 at 04:32:34PM +0300, Joonas Lahtinen wrote:
>> On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:
>> > Explicitly assign the default priority, and give it a name (macro).
>> > 
>> > Signed-off-by: Chris Wilson 
>> 
>> 
>> 
>> >    kref_init(>ref);
>> >    list_add_tail(>link, _priv->context_list);
>> >    ctx->i915 = dev_priv;
>> > +  ctx->priority = I915_PRIORITY_DFL;
>> 
>> I915_PRIORITY_DEFAULT would work better.
>
> On the one hand I have the symmetry with MIN, DFL, MAX, on the other
> hand DFL is plain bizarre.

DEF?

[1]: http://www.abbreviations.com/abbreviation/default

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


Re: [Intel-gfx] [PATCH 5/9] drm/i915: Use a define for the default priority [0]

2017-05-04 Thread Chris Wilson
On Thu, May 04, 2017 at 04:32:34PM +0300, Joonas Lahtinen wrote:
> On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:
> > Explicitly assign the default priority, and give it a name (macro).
> > 
> > Signed-off-by: Chris Wilson 
> 
> 
> 
> >     kref_init(>ref);
> >     list_add_tail(>link, _priv->context_list);
> >     ctx->i915 = dev_priv;
> > +   ctx->priority = I915_PRIORITY_DFL;
> 
> I915_PRIORITY_DEFAULT would work better.

On the one hand I have the symmetry with MIN, DFL, MAX, on the other
hand DFL is plain bizarre.
-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 5/9] drm/i915: Use a define for the default priority [0]

2017-05-04 Thread Joonas Lahtinen
On ke, 2017-05-03 at 12:37 +0100, Chris Wilson wrote:
> Explicitly assign the default priority, and give it a name (macro).
> 
> Signed-off-by: Chris Wilson 



>   kref_init(>ref);
>   list_add_tail(>link, _priv->context_list);
>   ctx->i915 = dev_priv;
> + ctx->priority = I915_PRIORITY_DFL;

I915_PRIORITY_DEFAULT would work better.

Reviewed-by: Joonas Lahtinen 

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


[Intel-gfx] [PATCH 5/9] drm/i915: Use a define for the default priority [0]

2017-05-03 Thread Chris Wilson
Explicitly assign the default priority, and give it a name (macro).

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 1 +
 drivers/gpu/drm/i915/i915_gem_request.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 31a73c39239f..c0ae5cd3cf77 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -199,6 +199,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
kref_init(>ref);
list_add_tail(>link, _priv->context_list);
ctx->i915 = dev_priv;
+   ctx->priority = I915_PRIORITY_DFL;
 
/* Default context will never have a file_priv */
ret = DEFAULT_CONTEXT_HANDLE;
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h 
b/drivers/gpu/drm/i915/i915_gem_request.h
index 4ccab5affd3c..feb81463abc9 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -70,6 +70,7 @@ struct i915_priotree {
struct rb_node node;
int priority;
 #define I915_PRIORITY_MAX 1024
+#define I915_PRIORITY_DFL 0
 #define I915_PRIORITY_MIN (-I915_PRIORITY_MAX)
 };
 
-- 
2.11.0

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