| Message ID | 1448023432-10726-8-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 20, 2015 at 12:43:48PM +0000, Chris Wilson wrote: > Having ringbuf->ring point to an engine is confusing, so rename it once > again to ring->engine. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> This massively conflicts with commit e84fe80337dc85cca07d0417ea97edbec4789d8b Author: Nick Hoath <nicholas.hoath@intel.com> Date: Fri Sep 11 12:53:46 2015 +0100 drm/i915: Split alloc from init for lrc since this patch touches piles of old code now removed. But no functional conflict afaics, so Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Maybe rebase before sending ;-) -Daniel > --- > drivers/gpu/drm/i915/intel_lrc.c | 46 ++++++++++++++++----------------- > drivers/gpu/drm/i915/intel_ringbuffer.c | 36 +++++++++++++------------- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > 3 files changed, 42 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 5c37922c3cde..346f5889738e 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -2089,13 +2089,13 @@ void intel_lr_context_free(struct intel_context *ctx) > if (ctx_obj) { > struct intel_ringbuffer *ringbuf = > ctx->engine[i].ringbuf; > - struct intel_engine_cs *ring = ringbuf->ring; > + struct intel_engine_cs *engine = ringbuf->engine; > > - if (ctx == ring->default_context) { > + if (ctx == engine->default_context) { > intel_unpin_ringbuffer_obj(ringbuf); > i915_gem_object_ggtt_unpin(ctx_obj); > } > - WARN_ON(ctx->engine[ring->id].pin_count); > + WARN_ON(ctx->engine[engine->id].pin_count); > intel_destroy_ringbuffer_obj(ringbuf); > kfree(ringbuf); > drm_gem_object_unreference(&ctx_obj->base); > @@ -2158,19 +2158,19 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring, > * Return: non-zero on error. > */ > int intel_lr_context_deferred_create(struct intel_context *ctx, > - struct intel_engine_cs *ring) > + struct intel_engine_cs *engine) > { > - const bool is_global_default_ctx = (ctx == ring->default_context); > - struct drm_device *dev = ring->dev; > + const bool is_global_default_ctx = (ctx == engine->default_context); > + struct drm_device *dev = engine->dev; > struct drm_i915_gem_object *ctx_obj; > uint32_t context_size; > struct intel_ringbuffer *ringbuf; > int ret; > > WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL); > - WARN_ON(ctx->engine[ring->id].state); > + WARN_ON(ctx->engine[engine->id].state); > > - context_size = round_up(get_lr_context_size(ring), 4096); > + context_size = round_up(get_lr_context_size(engine), 4096); > > ctx_obj = i915_gem_alloc_object(dev, context_size); > if (!ctx_obj) { > @@ -2191,12 +2191,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL); > if (!ringbuf) { > DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n", > - ring->name); > + engine->name); > ret = -ENOMEM; > goto error_unpin_ctx; > } > > - ringbuf->ring = ring; > + ringbuf->engine = engine; > > ringbuf->size = 32 * PAGE_SIZE; > ringbuf->effective_size = ringbuf->size; > @@ -2210,7 +2210,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > if (ret) { > DRM_DEBUG_DRIVER( > "Failed to allocate ringbuffer obj %s: %d\n", > - ring->name, ret); > + engine->name, ret); > goto error_free_rbuf; > } > > @@ -2219,38 +2219,38 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > if (ret) { > DRM_ERROR( > "Failed to pin and map ringbuffer %s: %d\n", > - ring->name, ret); > + engine->name, ret); > goto error_destroy_rbuf; > } > } > > } > > - ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf); > + ret = populate_lr_context(ctx, ctx_obj, engine, ringbuf); > if (ret) { > DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret); > goto error; > } > > - ctx->engine[ring->id].ringbuf = ringbuf; > - ctx->engine[ring->id].state = ctx_obj; > + ctx->engine[engine->id].ringbuf = ringbuf; > + ctx->engine[engine->id].state = ctx_obj; > > - if (ctx == ring->default_context) > - lrc_setup_hardware_status_page(ring, ctx_obj); > - else if (ring->id == RCS && !ctx->rcs_initialized) { > - if (ring->init_context) { > + if (ctx == engine->default_context) > + lrc_setup_hardware_status_page(engine, ctx_obj); > + else if (engine->id == RCS && !ctx->rcs_initialized) { > + if (engine->init_context) { > struct drm_i915_gem_request *req; > > - ret = i915_gem_request_alloc(ring, ctx, &req); > + ret = i915_gem_request_alloc(engine, ctx, &req); > if (ret) > return ret; > > - ret = ring->init_context(req); > + ret = engine->init_context(req); > if (ret) { > DRM_ERROR("ring init context: %d\n", ret); > i915_gem_request_cancel(req); > - ctx->engine[ring->id].ringbuf = NULL; > - ctx->engine[ring->id].state = NULL; > + ctx->engine[engine->id].ringbuf = NULL; > + ctx->engine[engine->id].state = NULL; > goto error; > } > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index c4610c727c49..1d43a24b6268 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -2043,36 +2043,36 @@ int intel_alloc_ringbuffer_obj(struct drm_device *dev, > } > > static int intel_init_ring_buffer(struct drm_device *dev, > - struct intel_engine_cs *ring) > + struct intel_engine_cs *engine) > { > struct intel_ringbuffer *ringbuf; > int ret; > > - WARN_ON(ring->buffer); > + WARN_ON(engine->buffer); > > ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL); > if (!ringbuf) > return -ENOMEM; > - ring->buffer = ringbuf; > + engine->buffer = ringbuf; > > - ring->dev = dev; > - INIT_LIST_HEAD(&ring->active_list); > - INIT_LIST_HEAD(&ring->request_list); > - INIT_LIST_HEAD(&ring->execlist_queue); > - i915_gem_batch_pool_init(dev, &ring->batch_pool); > + engine->dev = dev; > + INIT_LIST_HEAD(&engine->active_list); > + INIT_LIST_HEAD(&engine->request_list); > + INIT_LIST_HEAD(&engine->execlist_queue); > + i915_gem_batch_pool_init(dev, &engine->batch_pool); > ringbuf->size = 32 * PAGE_SIZE; > - ringbuf->ring = ring; > - memset(ring->semaphore.sync_seqno, 0, sizeof(ring->semaphore.sync_seqno)); > + ringbuf->engine = engine; > + memset(engine->semaphore.sync_seqno, 0, sizeof(engine->semaphore.sync_seqno)); > > - init_waitqueue_head(&ring->irq_queue); > + init_waitqueue_head(&engine->irq_queue); > > if (I915_NEED_GFX_HWS(dev)) { > - ret = init_status_page(ring); > + ret = init_status_page(engine); > if (ret) > goto error; > } else { > - BUG_ON(ring->id != RCS); > - ret = init_phys_status_page(ring); > + BUG_ON(engine->id != RCS); > + ret = init_phys_status_page(engine); > if (ret) > goto error; > } > @@ -2082,14 +2082,14 @@ static int intel_init_ring_buffer(struct drm_device *dev, > ret = intel_alloc_ringbuffer_obj(dev, ringbuf); > if (ret) { > DRM_ERROR("Failed to allocate ringbuffer %s: %d\n", > - ring->name, ret); > + engine->name, ret); > goto error; > } > > ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); > if (ret) { > DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", > - ring->name, ret); > + engine->name, ret); > intel_destroy_ringbuffer_obj(ringbuf); > goto error; > } > @@ -2102,7 +2102,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, > if (IS_I830(dev) || IS_845G(dev)) > ringbuf->effective_size -= 2 * CACHELINE_BYTES; > > - ret = i915_cmd_parser_init_ring(ring); > + ret = i915_cmd_parser_init_ring(engine); > if (ret) > goto error; > > @@ -2110,7 +2110,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, > > error: > kfree(ringbuf); > - ring->buffer = NULL; > + engine->buffer = NULL; > return ret; > } > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 368d9b0454ed..6a9056e3ac46 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -99,7 +99,7 @@ struct intel_ringbuffer { > struct drm_i915_gem_object *obj; > void *virtual_start; > > - struct intel_engine_cs *ring; > + struct intel_engine_cs *engine; > > u32 head; > u32 tail; > -- > 2.6.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Nov 24, 2015 at 03:58:13PM +0100, Daniel Vetter wrote: > On Fri, Nov 20, 2015 at 12:43:48PM +0000, Chris Wilson wrote: > > Having ringbuf->ring point to an engine is confusing, so rename it once > > again to ring->engine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > This massively conflicts with > > commit e84fe80337dc85cca07d0417ea97edbec4789d8b > Author: Nick Hoath <nicholas.hoath@intel.com> > Date: Fri Sep 11 12:53:46 2015 +0100 > > drm/i915: Split alloc from init for lrc > > since this patch touches piles of old code now removed. But no functional > conflict afaics, so Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Maybe rebase before sending ;-) Depends on how much you value testing. -Chris
On Tue, Nov 24, 2015 at 03:58:13PM +0100, Daniel Vetter wrote: > On Fri, Nov 20, 2015 at 12:43:48PM +0000, Chris Wilson wrote: > > Having ringbuf->ring point to an engine is confusing, so rename it once > > again to ring->engine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > This massively conflicts with > > commit e84fe80337dc85cca07d0417ea97edbec4789d8b > Author: Nick Hoath <nicholas.hoath@intel.com> > Date: Fri Sep 11 12:53:46 2015 +0100 > > drm/i915: Split alloc from init for lrc That patch still didn't implement what I suggested in review to remove more code and reduce the layering violation. What I was trying to get Nick to achieve was http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=210680144b780b491de1de9b49125d9042054eb6 -Chris
On Tue, Nov 24, 2015 at 03:15:57PM +0000, Chris Wilson wrote: > On Tue, Nov 24, 2015 at 03:58:13PM +0100, Daniel Vetter wrote: > > On Fri, Nov 20, 2015 at 12:43:48PM +0000, Chris Wilson wrote: > > > Having ringbuf->ring point to an engine is confusing, so rename it once > > > again to ring->engine. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > This massively conflicts with > > > > commit e84fe80337dc85cca07d0417ea97edbec4789d8b > > Author: Nick Hoath <nicholas.hoath@intel.com> > > Date: Fri Sep 11 12:53:46 2015 +0100 > > > > drm/i915: Split alloc from init for lrc > > That patch still didn't implement what I suggested in review to remove > more code and reduce the layering violation. What I was trying to get > Nick to achieve was > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=210680144b780b491de1de9b49125d9042054eb6 Yeah that's even prettier. But Nick's patch did at least get some of the init code flow maze sorted out. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 5c37922c3cde..346f5889738e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -2089,13 +2089,13 @@ void intel_lr_context_free(struct intel_context *ctx) if (ctx_obj) { struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf; - struct intel_engine_cs *ring = ringbuf->ring; + struct intel_engine_cs *engine = ringbuf->engine; - if (ctx == ring->default_context) { + if (ctx == engine->default_context) { intel_unpin_ringbuffer_obj(ringbuf); i915_gem_object_ggtt_unpin(ctx_obj); } - WARN_ON(ctx->engine[ring->id].pin_count); + WARN_ON(ctx->engine[engine->id].pin_count); intel_destroy_ringbuffer_obj(ringbuf); kfree(ringbuf); drm_gem_object_unreference(&ctx_obj->base); @@ -2158,19 +2158,19 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring, * Return: non-zero on error. */ int intel_lr_context_deferred_create(struct intel_context *ctx, - struct intel_engine_cs *ring) + struct intel_engine_cs *engine) { - const bool is_global_default_ctx = (ctx == ring->default_context); - struct drm_device *dev = ring->dev; + const bool is_global_default_ctx = (ctx == engine->default_context); + struct drm_device *dev = engine->dev; struct drm_i915_gem_object *ctx_obj; uint32_t context_size; struct intel_ringbuffer *ringbuf; int ret; WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL); - WARN_ON(ctx->engine[ring->id].state); + WARN_ON(ctx->engine[engine->id].state); - context_size = round_up(get_lr_context_size(ring), 4096); + context_size = round_up(get_lr_context_size(engine), 4096); ctx_obj = i915_gem_alloc_object(dev, context_size); if (!ctx_obj) { @@ -2191,12 +2191,12 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL); if (!ringbuf) { DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n", - ring->name); + engine->name); ret = -ENOMEM; goto error_unpin_ctx; } - ringbuf->ring = ring; + ringbuf->engine = engine; ringbuf->size = 32 * PAGE_SIZE; ringbuf->effective_size = ringbuf->size; @@ -2210,7 +2210,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, if (ret) { DRM_DEBUG_DRIVER( "Failed to allocate ringbuffer obj %s: %d\n", - ring->name, ret); + engine->name, ret); goto error_free_rbuf; } @@ -2219,38 +2219,38 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, if (ret) { DRM_ERROR( "Failed to pin and map ringbuffer %s: %d\n", - ring->name, ret); + engine->name, ret); goto error_destroy_rbuf; } } } - ret = populate_lr_context(ctx, ctx_obj, ring, ringbuf); + ret = populate_lr_context(ctx, ctx_obj, engine, ringbuf); if (ret) { DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret); goto error; } - ctx->engine[ring->id].ringbuf = ringbuf; - ctx->engine[ring->id].state = ctx_obj; + ctx->engine[engine->id].ringbuf = ringbuf; + ctx->engine[engine->id].state = ctx_obj; - if (ctx == ring->default_context) - lrc_setup_hardware_status_page(ring, ctx_obj); - else if (ring->id == RCS && !ctx->rcs_initialized) { - if (ring->init_context) { + if (ctx == engine->default_context) + lrc_setup_hardware_status_page(engine, ctx_obj); + else if (engine->id == RCS && !ctx->rcs_initialized) { + if (engine->init_context) { struct drm_i915_gem_request *req; - ret = i915_gem_request_alloc(ring, ctx, &req); + ret = i915_gem_request_alloc(engine, ctx, &req); if (ret) return ret; - ret = ring->init_context(req); + ret = engine->init_context(req); if (ret) { DRM_ERROR("ring init context: %d\n", ret); i915_gem_request_cancel(req); - ctx->engine[ring->id].ringbuf = NULL; - ctx->engine[ring->id].state = NULL; + ctx->engine[engine->id].ringbuf = NULL; + ctx->engine[engine->id].state = NULL; goto error; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c4610c727c49..1d43a24b6268 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -2043,36 +2043,36 @@ int intel_alloc_ringbuffer_obj(struct drm_device *dev, } static int intel_init_ring_buffer(struct drm_device *dev, - struct intel_engine_cs *ring) + struct intel_engine_cs *engine) { struct intel_ringbuffer *ringbuf; int ret; - WARN_ON(ring->buffer); + WARN_ON(engine->buffer); ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL); if (!ringbuf) return -ENOMEM; - ring->buffer = ringbuf; + engine->buffer = ringbuf; - ring->dev = dev; - INIT_LIST_HEAD(&ring->active_list); - INIT_LIST_HEAD(&ring->request_list); - INIT_LIST_HEAD(&ring->execlist_queue); - i915_gem_batch_pool_init(dev, &ring->batch_pool); + engine->dev = dev; + INIT_LIST_HEAD(&engine->active_list); + INIT_LIST_HEAD(&engine->request_list); + INIT_LIST_HEAD(&engine->execlist_queue); + i915_gem_batch_pool_init(dev, &engine->batch_pool); ringbuf->size = 32 * PAGE_SIZE; - ringbuf->ring = ring; - memset(ring->semaphore.sync_seqno, 0, sizeof(ring->semaphore.sync_seqno)); + ringbuf->engine = engine; + memset(engine->semaphore.sync_seqno, 0, sizeof(engine->semaphore.sync_seqno)); - init_waitqueue_head(&ring->irq_queue); + init_waitqueue_head(&engine->irq_queue); if (I915_NEED_GFX_HWS(dev)) { - ret = init_status_page(ring); + ret = init_status_page(engine); if (ret) goto error; } else { - BUG_ON(ring->id != RCS); - ret = init_phys_status_page(ring); + BUG_ON(engine->id != RCS); + ret = init_phys_status_page(engine); if (ret) goto error; } @@ -2082,14 +2082,14 @@ static int intel_init_ring_buffer(struct drm_device *dev, ret = intel_alloc_ringbuffer_obj(dev, ringbuf); if (ret) { DRM_ERROR("Failed to allocate ringbuffer %s: %d\n", - ring->name, ret); + engine->name, ret); goto error; } ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); if (ret) { DRM_ERROR("Failed to pin and map ringbuffer %s: %d\n", - ring->name, ret); + engine->name, ret); intel_destroy_ringbuffer_obj(ringbuf); goto error; } @@ -2102,7 +2102,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, if (IS_I830(dev) || IS_845G(dev)) ringbuf->effective_size -= 2 * CACHELINE_BYTES; - ret = i915_cmd_parser_init_ring(ring); + ret = i915_cmd_parser_init_ring(engine); if (ret) goto error; @@ -2110,7 +2110,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, error: kfree(ringbuf); - ring->buffer = NULL; + engine->buffer = NULL; return ret; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 368d9b0454ed..6a9056e3ac46 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -99,7 +99,7 @@ struct intel_ringbuffer { struct drm_i915_gem_object *obj; void *virtual_start; - struct intel_engine_cs *ring; + struct intel_engine_cs *engine; u32 head; u32 tail;
Having ringbuf->ring point to an engine is confusing, so rename it once again to ring->engine. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_lrc.c | 46 ++++++++++++++++----------------- drivers/gpu/drm/i915/intel_ringbuffer.c | 36 +++++++++++++------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 3 files changed, 42 insertions(+), 42 deletions(-)