Re: [Intel-gfx] [PATCH v2 09/11] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask

[Chris Wilson]

Quoting Joonas Lahtinen (2017-09-28 14:07:35)
> On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> > In the next few patches, we wish to enable different features for the
> > scheduler, some which may subtlety change ABI (e.g. allow requests to be
> > reordered under different circumstances). So we need to make sure
> > userspace is cognizant of the changes (if they care), by which we employ
> > the usual method of a GETPARAM. We already have an
> > I915_PARAM_HAS_SCHEDULER (which notes the existing ability to reorder
> > requests to avoid bubbles), and now we wish to extend that to be a
> > bitmask to describe the different capabilities implemented.
> 
> Please link the last Mesa series here, as this is the patch bringing in
> the ABI.
> 
> > Signed-off-by: Chris Wilson 
> 
> 
> 
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -397,10 +397,17 @@ typedef struct drm_i915_irq_wait {
> >  #define I915_PARAM_MIN_EU_IN_POOL 39
> >  #define I915_PARAM_MMAP_GTT_VERSION   40
> >  
> > -/* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> > +/*
> > + * Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> >   * priorities and the driver will attempt to execute batches in priority 
> > order.
> > + * The param returns a capability bitmask, nonzero implies that the 
> > scheduler
> > + * is enabled, with different features present according to the mask.
> >   */
> >  #define I915_PARAM_HAS_SCHEDULER  41
> > +#define   I915_SCHEDULER_CAP_ENABLED (1ul << 0)
> > +#define   I915_SCHEDULER_CAP_PRIORITY(1ul << 1)
> > +#define   I915_SCHEDULER_CAP_PREEMPTION  (1ul << 2)
> 
> There seems to be the occasional BIT() hanging under uapi (I seem to
> have added the first for i915.h). So maybe we need to revert that or
> convert these.

We don't have BIT() in uapi yet, afaik. Plenty of places where we could
benefit.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 09/11] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask

[Joonas Lahtinen]

On Wed, 2017-09-27 at 17:44 +0100, Chris Wilson wrote:
> In the next few patches, we wish to enable different features for the
> scheduler, some which may subtlety change ABI (e.g. allow requests to be
> reordered under different circumstances). So we need to make sure
> userspace is cognizant of the changes (if they care), by which we employ
> the usual method of a GETPARAM. We already have an
> I915_PARAM_HAS_SCHEDULER (which notes the existing ability to reorder
> requests to avoid bubbles), and now we wish to extend that to be a
> bitmask to describe the different capabilities implemented.

Please link the last Mesa series here, as this is the patch bringing in
the ABI.

> Signed-off-by: Chris Wilson 



> +++ b/include/uapi/drm/i915_drm.h
> @@ -397,10 +397,17 @@ typedef struct drm_i915_irq_wait {
>  #define I915_PARAM_MIN_EU_IN_POOL 39
>  #define I915_PARAM_MMAP_GTT_VERSION   40
>  
> -/* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
> +/*
> + * Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
>   * priorities and the driver will attempt to execute batches in priority 
> order.
> + * The param returns a capability bitmask, nonzero implies that the scheduler
> + * is enabled, with different features present according to the mask.
>   */
>  #define I915_PARAM_HAS_SCHEDULER  41
> +#define   I915_SCHEDULER_CAP_ENABLED (1ul << 0)
> +#define   I915_SCHEDULER_CAP_PRIORITY(1ul << 1)
> +#define   I915_SCHEDULER_CAP_PREEMPTION  (1ul << 2)

There seems to be the occasional BIT() hanging under uapi (I seem to
have added the first for i915.h). So maybe we need to revert that or
convert these.

Either way;

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 v2 09/11] drm/i915: Expand I915_PARAM_HAS_SCHEDULER into a capability bitmask

[Chris Wilson]

In the next few patches, we wish to enable different features for the
scheduler, some which may subtlety change ABI (e.g. allow requests to be
reordered under different circumstances). So we need to make sure
userspace is cognizant of the changes (if they care), by which we employ
the usual method of a GETPARAM. We already have an
I915_PARAM_HAS_SCHEDULER (which notes the existing ability to reorder
requests to avoid bubbles), and now we wish to extend that to be a
bitmask to describe the different capabilities implemented.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.c | 5 +++--
 include/uapi/drm/i915_drm.h | 9 -
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7056bb299dc6..db3438ba92fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -367,8 +367,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
value = i915_gem_mmap_gtt_version();
break;
case I915_PARAM_HAS_SCHEDULER:
-   value = dev_priv->engine[RCS] &&
-   dev_priv->engine[RCS]->schedule;
+   value = 0;
+   if (dev_priv->engine[RCS] && dev_priv->engine[RCS]->schedule)
+   value |= I915_SCHEDULER_CAP_ENABLED;
break;
case I915_PARAM_MMAP_VERSION:
/* Remember to bump this if the version changes! */
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fe25a01c81f2..aa4a3b20ef6b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -397,10 +397,17 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_MIN_EU_IN_POOL   39
 #define I915_PARAM_MMAP_GTT_VERSION 40
 
-/* Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
+/*
+ * Query whether DRM_I915_GEM_EXECBUFFER2 supports user defined execution
  * priorities and the driver will attempt to execute batches in priority order.
+ * The param returns a capability bitmask, nonzero implies that the scheduler
+ * is enabled, with different features present according to the mask.
  */
 #define I915_PARAM_HAS_SCHEDULER41
+#define   I915_SCHEDULER_CAP_ENABLED   (1ul << 0)
+#define   I915_SCHEDULER_CAP_PRIORITY  (1ul << 1)
+#define   I915_SCHEDULER_CAP_PREEMPTION(1ul << 2)
+
 #define I915_PARAM_HUC_STATUS   42
 
 /* Query whether DRM_I915_GEM_EXECBUFFER2 supports the ability to opt-out of
-- 
2.14.1

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