[Intel-gfx] [RFC 27/28] drm/i915: Compartmentalize ring buffer creation

[Tvrtko Ursulin]

On 13/06/2019 16:46, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-06-13 16:19:03)
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Continuing on the theme of compartmentalizing the code better to make
>> future split between gt and display in global i915 clearer.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
>> index b3bf47e8162f..0a1d952ad160 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
>> @@ -1226,16 +1226,16 @@ void intel_ring_unpin(struct intel_ring *ring)
>>          i915_timeline_unpin(ring->timeline);
>>   }
>>   
>> -static struct i915_vma *
>> -intel_ring_create_vma(struct drm_i915_private *dev_priv, int size)
>> +static struct i915_vma *create_ring_vma(struct intel_gt *gt, int size)
> 
> Here, it probably makes more sense to be passing i915_ggtt (since this
> is about the i915_vma itself).

Yep, good point.

>>   {
>> -       struct i915_address_space *vm = &dev_priv->ggtt.vm;
>> +       struct i915_address_space *vm = &gt->ggtt->vm;
>> +       struct drm_i915_private *i915 = gt->i915;
>>          struct drm_i915_gem_object *obj;
>>          struct i915_vma *vma;
>>   
>> -       obj = i915_gem_object_create_stolen(dev_priv, size);
>> +       obj = i915_gem_object_create_stolen(i915, size);
>>          if (!obj)
>> -               obj = i915_gem_object_create_internal(dev_priv, size);
>> +               obj = i915_gem_object_create_internal(i915, size);
>>          if (IS_ERR(obj))
>>                  return ERR_CAST(obj);
>>   
>> @@ -1262,13 +1262,14 @@ intel_engine_create_ring(struct intel_engine_cs *engine,
>>                           struct i915_timeline *timeline,
>>                           int size)
>>   {
>> +       struct drm_i915_private *i915 = engine->i915;
>>          struct intel_ring *ring;
>>          struct i915_vma *vma;
>>   
>>          GEM_BUG_ON(!is_power_of_2(size));
>>          GEM_BUG_ON(RING_CTL_SIZE(size) & ~RING_NR_PAGES);
>>          GEM_BUG_ON(timeline == &engine->timeline);
>> -       lockdep_assert_held(&engine->i915->drm.struct_mutex);
>> +       lockdep_assert_held(&i915->drm.struct_mutex);
> 
> Heresy.

What why?

Regards,

Tvrtko