Re: [PATCH v4 1/2] drm/i915/icl: new context descriptor support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Oscar Mateo (2018-02-09 23:48:38)
> 
> 
> On 02/09/2018 03:28 PM, Daniele Ceraolo Spurio wrote:
> > From: "Ceraolo Spurio, Daniele" <daniele.ceraolospurio@xxxxxxxxx>
> >
> > Starting from Gen11 the context descriptor format has been updated in
> > the HW. The hw_id field has been considerably reduced in size and engine
> > class and instance fields have been added.
> >
> > There is a slight name clashing issue because the field that we call
> > hw_id is actually called SW Context ID in the specs for Gen11+.
> >
> > With the current size of the hw_id field we can have a maximum of 2k
> > contexts at any time, but we could use the sw_counter field (which is sw
> > defined) to increase that because the HW requirement is that
> > engine_id + sw id + sw_counter is a unique number.
> > GuC uses a similar method to support more contexts but does its tracking
> > at lrc level. To avoid doing an implementation that will need to be
> > reworked once GuC support lands, defer it for now and mark it as TODO.
> >
> > v2: rebased, add documentation, fix GEN11_ENGINE_INSTANCE_SHIFT
> > v3: rebased, bring back lost code from i915_gem_context.c
> > v4: make TODO comment more generic
> >
> > Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> 
> Reviewed-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> 
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >   drivers/gpu/drm/i915/i915_gem_context.c | 11 +++++++++--
> >   drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
> >   drivers/gpu/drm/i915/intel_lrc.c        | 28 +++++++++++++++++++++++++++-
> >   4 files changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 7db3557b945c..acaa63f8237d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2093,6 +2093,7 @@ struct drm_i915_private {
> >                */
> >               struct ida hw_ida;
> >   #define MAX_CONTEXT_HW_ID (1<<21) /* exclusive */
> > +#define GEN11_MAX_CONTEXT_HW_ID (1<<11) /* exclusive */
> >       } contexts;
> >   
> >       u32 fdi_rx_config;
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 3d75f484f6e5..45b0b78aca3f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -211,9 +211,15 @@ static void context_close(struct i915_gem_context *ctx)
> >   static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
> >   {
> >       int ret;
> > +     unsigned int max;
> > +
> > +     if (INTEL_GEN(dev_priv) >= 11)
> > +             max = GEN11_MAX_CONTEXT_HW_ID;
> > +     else
> > +             max = MAX_CONTEXT_HW_ID;
> >   
> >       ret = ida_simple_get(&dev_priv->contexts.hw_ida,
> > -                          0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
> > +                          0, max, GFP_KERNEL);
> >       if (ret < 0) {
> >               /* Contexts are only released when no longer active.
> >                * Flush any pending retires to hopefully release some
> > @@ -221,7 +227,7 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
> >                */
> >               i915_gem_retire_requests(dev_priv);
> >               ret = ida_simple_get(&dev_priv->contexts.hw_ida,
> > -                                  0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
> > +                                  0, max, GFP_KERNEL);
> >               if (ret < 0)
> >                       return ret;
> >       }
> > @@ -463,6 +469,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> >   
> >       /* Using the simple ida interface, the max is limited by sizeof(int) */
> >       BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
> > +     BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > INT_MAX);
> >       ida_init(&dev_priv->contexts.hw_ida);
> >   
> >       /* lowest priority; idle task */
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e9c79b560823..bd84e29d5399 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3869,6 +3869,10 @@ enum {
> >   
> >   #define GEN8_CTX_ID_SHIFT 32
> >   #define GEN8_CTX_ID_WIDTH 21
> > +#define GEN11_SW_CTX_ID_SHIFT 37
> > +#define GEN11_SW_CTX_ID_WIDTH 11
> > +#define GEN11_ENGINE_CLASS_SHIFT 61
> > +#define GEN11_ENGINE_INSTANCE_SHIFT 48
> >   
> >   #define CHV_CLK_CTL1                        _MMIO(0x101100)
> >   #define VLV_CLK_CTL2                        _MMIO(0x101104)
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index c2c8380a0121..3305fbba65e9 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -187,6 +187,18 @@ static void execlists_init_reg_state(u32 *reg_state,
> >    *      bits 32-52:    ctx ID, a globally unique tag
> >    *      bits 53-54:    mbz, reserved for use by hardware
> >    *      bits 55-63:    group ID, currently unused and set to 0
> > + *
> > + * Starting from Gen11, the upper dword of the descriptor has a new format:
> > + *
> > + *      bits 32-36:    reserved
> > + *      bits 37-47:    SW context ID
> > + *      bits 48:53:    engine instance
> > + *      bit 54:        mbz, reserved for use by hardware
> > + *      bits 55-60:    SW counter
> > + *      bits 61-63:    engine class

Ascending ^

> > + *
> > + * engine info, SW context ID and SW counter need to form a unique number
> > + * (Context ID) per lrc.
> >    */
> >   static void
> >   intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
> > @@ -196,11 +208,25 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
> >       u64 desc;
> >   
> >       BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
> > +     BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > (1<<GEN11_SW_CTX_ID_WIDTH));
> >   
> >       desc = ctx->desc_template;                              /* bits  0-11 */

Should we add GEM_BUG_ON(desc & GENMASK_ULL(12, 63); ?

> >       desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
> >                                                               /* bits 12-31 */

GEM_BUG_ON(desc & GENMASK_ULL(32, 63);

Ascending ^

> > -     desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;           /* bits 32-52 */
> > +
> > +     if (INTEL_GEN(ctx->i915) >= 11) {

Descending v

GEM_BUG_ON(engine->class >= BIT(3)); ?

> > +             desc |= (u64)engine->class << GEN11_ENGINE_CLASS_SHIFT;
> > +                                                             /* bits 61-63 */
> > +
> > +             /* TODO: decide what to do with SW counter (bits 60-55) */
> > +
> > +             desc |= (u64)engine->instance << GEN11_ENGINE_INSTANCE_SHIFT;
> > +                                                             /* bits 53-48 */
> > +             desc |= (u64)ctx->hw_id << GEN11_SW_CTX_ID_SHIFT;
> > +                                                             /* bits 37-47 */

GEM_BUG_ON(ctx->hw_id >= BIT(GEN11_SW_CTX_ID_WIDTH)); etc?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux