| Message ID | 20190613151904.16256-4-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implicit dev_priv removal and GT compartmentalization | expand |
Quoting Tvrtko Ursulin (2019-06-13 16:19:03) > From: Tvrtko Ursulin <tvrtko.ursulin@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@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). > { > - struct i915_address_space *vm = &dev_priv->ggtt.vm; > + struct i915_address_space *vm = >->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. -Chris
On 13/06/2019 16:46, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-06-13 16:19:03) >> From: Tvrtko Ursulin <tvrtko.ursulin@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@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 = >->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
Quoting Tvrtko Ursulin (2019-06-13 16:56:37) > > On 13/06/2019 16:46, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2019-06-13 16:19:03) > >> @@ -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? I don't have this line in my kernel \o/ -Chris
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) { - struct i915_address_space *vm = &dev_priv->ggtt.vm; + struct i915_address_space *vm = >->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); ring = kzalloc(sizeof(*ring), GFP_KERNEL); if (!ring) @@ -1284,12 +1285,12 @@ intel_engine_create_ring(struct intel_engine_cs *engine, * of the buffer. */ ring->effective_size = size; - if (IS_I830(engine->i915) || IS_I845G(engine->i915)) + if (IS_I830(i915) || IS_I845G(i915)) ring->effective_size -= 2 * CACHELINE_BYTES; intel_ring_update_space(ring); - vma = intel_ring_create_vma(engine->i915, size); + vma = create_ring_vma(engine->gt, size); if (IS_ERR(vma)) { kfree(ring); return ERR_CAST(vma);