Re: [PATCH v2] drm/i915: Sanitize engine context sizes

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 26/04/2017 14:16, Tvrtko Ursulin wrote:
> On 26/04/2017 13:20, Joonas Lahtinen wrote:
>> Pre-calculate engine context size based on engine class and device
>> generation and store it in the engine instance.
>>
>> v2:
>>     - Squash and get rid of hw_context_size (Chris)
>>
>> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: intel-gvt-dev@lists.freedesktop.org
>> ---
>>  drivers/gpu/drm/i915/gvt/scheduler.c       |  6 +--
>>  drivers/gpu/drm/i915/i915_drv.h            |  1 -
>>  drivers/gpu/drm/i915/i915_gem_context.c    | 59 +++-------------------
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  3 +-
>>  drivers/gpu/drm/i915/i915_reg.h            | 10 ----
>>  drivers/gpu/drm/i915/intel_engine_cs.c     | 78
>> ++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_lrc.c           | 54 +--------------------
>>  drivers/gpu/drm/i915/intel_lrc.h           |  2 -
>>  drivers/gpu/drm/i915/intel_ringbuffer.h    |  7 +--
>>  9 files changed, 93 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
>> b/drivers/gpu/drm/i915/gvt/scheduler.c
>> index a77db23..ac538dc 100644
>> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
>> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
>> @@ -69,8 +69,7 @@ static int populate_shadow_context(struct
>> intel_vgpu_workload *workload)
>>      gvt_dbg_sched("ring id %d workload lrca %x", ring_id,
>>              workload->ctx_desc.lrca);
>>
>> -    context_page_num = intel_lr_context_size(
>> -            gvt->dev_priv->engine[ring_id]);
>> +    context_page_num = gvt->dev_priv->engine[ring_id]->context_size;
>>
>>      context_page_num = context_page_num >> PAGE_SHIFT;
>>
>> @@ -333,8 +332,7 @@ static void update_guest_context(struct
>> intel_vgpu_workload *workload)
>>      gvt_dbg_sched("ring id %d workload lrca %x\n", ring_id,
>>              workload->ctx_desc.lrca);
>>
>> -    context_page_num = intel_lr_context_size(
>> -            gvt->dev_priv->engine[ring_id]);
>> +    context_page_num = gvt->dev_priv->engine[ring_id]->context_size;
>>
>>      context_page_num = context_page_num >> PAGE_SHIFT;
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 357b6c6..4b54b92 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2359,7 +2359,6 @@ struct drm_i915_private {
>>       */
>>      struct mutex av_mutex;
>>
>> -    uint32_t hw_context_size;
>>      struct list_head context_list;
>>
>>      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 8bd0c49..b69a026 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -92,33 +92,6 @@
>>
>>  #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
>>
>> -static int get_context_size(struct drm_i915_private *dev_priv)
>> -{
>> -    int ret;
>> -    u32 reg;
>> -
>> -    switch (INTEL_GEN(dev_priv)) {
>> -    case 6:
>> -        reg = I915_READ(CXT_SIZE);
>> -        ret = GEN6_CXT_TOTAL_SIZE(reg) * 64;
>> -        break;
>> -    case 7:
>> -        reg = I915_READ(GEN7_CXT_SIZE);
>> -        if (IS_HASWELL(dev_priv))
>> -            ret = HSW_CXT_TOTAL_SIZE;
>> -        else
>> -            ret = GEN7_CXT_TOTAL_SIZE(reg) * 64;
>> -        break;
>> -    case 8:
>> -        ret = GEN8_CXT_TOTAL_SIZE;
>> -        break;
>> -    default:
>> -        BUG();
>> -    }
>> -
>> -    return ret;
>> -}
>> -
>>  void i915_gem_context_free(struct kref *ctx_ref)
>>  {
>>      struct i915_gem_context *ctx = container_of(ctx_ref,
>> typeof(*ctx), ref);
>> @@ -266,11 +239,12 @@ __create_hw_context(struct drm_i915_private
>> *dev_priv,
>>      list_add_tail(&ctx->link, &dev_priv->context_list);
>>      ctx->i915 = dev_priv;
>>
>> -    if (dev_priv->hw_context_size) {
>> +    if (dev_priv->engine[RCS]->context_size) {
>>          struct drm_i915_gem_object *obj;
>>          struct i915_vma *vma;
>>
>> -        obj = alloc_context_obj(dev_priv, dev_priv->hw_context_size);
>> +        obj = alloc_context_obj(dev_priv,
>> +                    dev_priv->engine[RCS]->context_size);
>>          if (IS_ERR(obj)) {
>>              ret = PTR_ERR(obj);
>>              goto err_out;
>> @@ -443,21 +417,6 @@ int i915_gem_context_init(struct drm_i915_private
>> *dev_priv)
>>      BUILD_BUG_ON(MAX_CONTEXT_HW_ID > INT_MAX);
>>      ida_init(&dev_priv->context_hw_ida);
>>
>> -    if (i915.enable_execlists) {
>> -        /* NB: intentionally left blank. We will allocate our own
>> -         * backing objects as we need them, thank you very much */
>> -        dev_priv->hw_context_size = 0;
>> -    } else if (HAS_HW_CONTEXTS(dev_priv)) {
>> -        dev_priv->hw_context_size =
>> -            round_up(get_context_size(dev_priv),
>> -                 I915_GTT_PAGE_SIZE);
>
> Is this rounding up lost when used from __create_hw_context?

Also lost seems keeping hw_context_size at zero when !HAS_HW_CONTEXT, 
deliberate? Looks like the replacement of contexts_enabled will not work 
as expected.

Regards,

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