Execlists v5
Subsystems (3)
PATCH 34/43 drm/i915/bdw: Make sure gpu reset still works with Execlists
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
If we reset a ring after a hang, we have to make sure that we clear out all queued Execlists requests.
v2: The ring is, at this point, already being correctly re-programmed for Execlists, and the hangcheck counters cleared.
v3: Daniel suggests to drop the "if (execlists)" because the Execlists queue should be empty in legacy mode (which is true, if we do the INIT_LIST_HEAD).
v4: Do the pending intel_runtime_pm_put
--- drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + 2 files changed, 13 insertions(+)
drivers/gpu/drm/i915/i915_gem.c +12 -0
@@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,i915_gem_free_request (request); } +while (!list_empty (&ring->execlist_queue)) { +struct intel_ctx_submit_request *submit_req; + + submit_req =list_first_entry (&ring->execlist_queue, + struct intel_ctx_submit_request, + execlist_link); +list_del (&submit_req->execlist_link); +intel_runtime_pm_put (dev_priv); +i915_gem_context_unreference (submit_req->ctx); +kfree (submit_req); + } +/* These may not have been flush before the reset, do so now */ kfree (ring->preallocated_lazy_request); ring->preallocated_lazy_request =NULL ;
drivers/gpu/drm/i915/intel_ringbuffer.c +1 -0
@@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, ring->dev = dev;INIT_LIST_HEAD (&ring->active_list);INIT_LIST_HEAD (&ring->request_list); +INIT_LIST_HEAD (&ring->execlist_queue); ringbuf->size =32 *PAGE_SIZE ; ringbuf->ring = ring; ringbuf->ctx = ring->default_context;
Signature · 1 line
1.7.9.5
Re: [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with Execlists
Quoted 5 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
If we reset a ring after a hang, we have to make sure that we clear out all queued Execlists requests.
Quoted 8 lines
v2: The ring is, at this point, already being correctly re-programmed for Execlists, and the hangcheck counters cleared.
v3: Daniel suggests to drop the "if (execlists)" because the Execlists queue should be empty in legacy mode (which is true, if we do the
INIT_LIST_HEAD).v4: Do the pending
intel_runtime_pm_put
I don't see a intel_runtime_pm_get() that put() would correspond to.
Quoted 49 lines
Signed-off-by Oscar Mateo <[email protected]>--- drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + 2 files changed, 13 insertions(+)drivers/gpu/drm/i915/i915_gem.c +12
-0@@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,i915_gem_free_request (request); } +while (!list_empty (&ring->execlist_queue)) { +struct intel_ctx_submit_request *submit_req; + + submit_req =list_first_entry (&ring->execlist_queue, + struct intel_ctx_submit_request, + execlist_link); +list_del (&submit_req->execlist_link); +intel_runtime_pm_put (dev_priv); +i915_gem_context_unreference (submit_req->ctx); +kfree (submit_req); + } +/* These may not have been flush before the reset, do so now */ kfree (ring->preallocated_lazy_request); ring->preallocated_lazy_request =NULL ;drivers/gpu/drm/i915/intel_ringbuffer.c +1
-0@@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, ring->dev = dev;INIT_LIST_HEAD (&ring->active_list);INIT_LIST_HEAD (&ring->request_list); +INIT_LIST_HEAD (&ring->execlist_queue); ringbuf->size =32 *PAGE_SIZE ; ringbuf->ring = ring; ringbuf->ctx = ring->default_context;Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with Execlists
Quoted 17 lines
-----Original Message-----
From: Lespiau, Damien
Sent: Friday, August 01, 2014 3:42 PM
To: Daniel, Thomas
Cc: [email protected]
Subject: Re: [Intel-gfx] [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with ExeclistsQuoted 8 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
If we reset a ring after a hang, we have to make sure that we clear out all queued Execlists requests.
v2: The ring is, at this point, already being correctly re-programmed for Execlists, and the hangcheck counters cleared.
Quoted 7 lines
Quoted 5 lines
v3: Daniel suggests to drop the "if (execlists)" because the Execlists queue should be empty in legacy mode (which is true, if we do the
INIT_LIST_HEAD).v4: Do the pending
intel_runtime_pm_putI don't see a
intel_runtime_pm_get()thatput()would correspond to.
It is in execlists_context_queue() where the request is added to the queue.
Quoted 53 lines
drivers/gpu/drm/i915/i915_gem.c static void i915_gem_reset_ring_cleanup(struct
Quoted 13 lines
Signed-off-by Oscar Mateo <[email protected]>--- drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++ drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + 2 files changed, 13 insertions(+)(quoted diff) +0
-0
b/drivers/gpu/drm/i915/i915_gem.cindex 1c83b9c..143cff7 100644drivers/gpu/drm/i915/i915_gem.c +0
-0@@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct
drm_i915_private*dev_priv,drivers/gpu/drm/i915/intel_ringbuffer.c static int intel_init_ring_buffer(struct drm_device
Quoted 24 lines
drivers/gpu/drm/i915/i915_gem.c +12
-0i915_gem_free_request (request); } +while (!list_empty (&ring->execlist_queue)) { +struct intel_ctx_submit_request *submit_req; + + submit_req =list_first_entry (&ring->execlist_queue, + struct intel_ctx_submit_request, + execlist_link); +list_del (&submit_req->execlist_link); +intel_runtime_pm_put (dev_priv); +i915_gem_context_unreference (submit_req->ctx); +kfree (submit_req); + } +/* These may not have been flush before the reset, do so now */ kfree (ring->preallocated_lazy_request); ring->preallocated_lazy_request = NULL; diff --git
a/drivers/gpu/drm/i915/intel_ringbuffer.cb/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3188403..6e604c9 100644drivers/gpu/drm/i915/intel_ringbuffer.c +0
-0@@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device*dev,
Quoted 14 lines
drivers/gpu/drm/i915/intel_ringbuffer.c +1
-1ring->dev = dev;INIT_LIST_HEAD (&ring->active_list);INIT_LIST_HEAD (&ring->request_list); +INIT_LIST_HEAD (&ring->execlist_queue); ringbuf->size =32 *PAGE_SIZE ; ringbuf->ring = ring; ringbuf->ctx = ring->default_context; --1.7.9.5
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with Execlists
In reply to Thomas Danieldrivers/gpu/drm/i915/intel_ringbuffer.c static int intel_init_ring_buffer(struct drm_device *dev, · line 1590
Quoted 9 lines · Thomas Daniel
drivers/gpu/drm/i915/intel_ringbuffer.c +1
-0@@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, ring->dev = dev;INIT_LIST_HEAD (&ring->active_list);INIT_LIST_HEAD (&ring->request_list); +INIT_LIST_HEAD (&ring->execlist_queue);
It's also a bit weird to now have sites where we initialize this list. or I'm missing something?
Signature · 1 line
Damien
Re: [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with Execlists
Quoted 18 lines
-----Original Message-----
From: Lespiau, Damien
Sent: Friday, August 01, 2014 3:46 PM
To: Daniel, Thomas
Cc: [email protected]
Subject: Re: [Intel-gfx] [PATCH 34/43] drm/i915/bdw: Make sure gpu reset still works with ExeclistsReview
Quoted 1 line · Thomas Daniel
(quoted diff) +0
-0
b/drivers/gpu/drm/i915/intel_ringbuffer.cdrivers/gpu/drm/i915/intel_ringbuffer.c static int intel_init_ring_buffer(struct drm_device
Quoted 4 lines
index 3188403..6e604c9 100644
drivers/gpu/drm/i915/intel_ringbuffer.c +0
-0@@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device*dev,
Quoted 7 lines
drivers/gpu/drm/i915/intel_ringbuffer.c static int intel_init_ring_buffer(struct drm_device *dev, · line 1590
Quoted 4 lines
drivers/gpu/drm/i915/intel_ringbuffer.c +1
-0ring->dev = dev;INIT_LIST_HEAD (&ring->active_list);INIT_LIST_HEAD (&ring->request_list); +INIT_LIST_HEAD (&ring->execlist_queue);It's also a bit weird to now have sites where we initialize this list. or I'm missing something?
The legacy ringbuffer init path now also has to initialize the queue because I915_gem_reset_ring_cleanup() now assumes that the list is always valid, even though it will always be empty in legacy mode.
Quoted 3 lines
--
Damien
PATCH 35/43 drm/i915/bdw: Make sure error capture keeps working with Execlists
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
Since the ringbuffer does not belong per engine anymore, we have to make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but we need to add more information for it to be useful (e.g. dump the context being executed).
v2: Reorder how the ringbuffer is chosen to clarify the change and rename the variable, both changes suggested by Chris Wilson. Also, add the TODO comment to the code, as suggested by Daniel.
--- drivers/gpu/drm/i915/i915_gpu_error.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/i915_gpu_error.c +18 -4
@@ -874,9 +874,6 @@ static void i915_record_ring_state(struct drm_device *dev, ering->hws =I915_READ (mmio); } - ering->cpu_ring_head = ring->buffer->head; - ering->cpu_ring_tail = ring->buffer->tail; - ering->hangcheck_score = ring->hangcheck.score; ering->hangcheck_action = ring->hangcheck.action;
@@ -936,6 +933,7 @@ static void i915_gem_record_rings(struct drm_device *dev,for (i =0 ; i <I915_NUM_RINGS ; i++) {struct intel_engine_cs *ring = &dev_priv->ring[i]; +struct intel_ringbuffer *rbuf; error->ring[i].pid =-1 ;
@@ -979,8 +977,24 @@ static void i915_gem_record_rings(struct drm_device *dev, } } +if (i915.enable_execlists) { +/* TODO: This is only a small fix to keep basic error +* capture working, but we need to add more information +* for it to be useful (e.g. dump the context being +* executed). +*/ +if (request) + rbuf = request->ctx->engine[ring->id].ringbuf; +else + rbuf = ring->default_context->engine[ring->id].ringbuf; + }else + rbuf = ring->buffer; + + error->ring[i].cpu_ring_head = rbuf->head; + error->ring[i].cpu_ring_tail = rbuf->tail; + error->ring[i].ringbuffer = -i915_error_ggtt_object_create (dev_priv, ring->buffer->obj); +i915_error_ggtt_object_create (dev_priv, rbuf->obj);if (ring->status_page.obj) error->ring[i].hws_page =
Signature · 1 line
1.7.9.5
Re: [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with Execlists
Quoted 9 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
Since the ringbuffer does not belong per engine anymore, we have to make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but we need to add more information for it to be useful (e.g. dump the context being executed).
Quoted 5 lines
v2: Reorder how the ringbuffer is chosen to clarify the change and rename the variable, both changes suggested by Chris Wilson. Also, add the TODO comment to the code, as suggested by Daniel.
Signed-off-by Oscar Mateo <[email protected]>
There's a bit too much stuff in-flight to fix up error capture for ppgtt. I think it's better to stall this patch here until that work is completed. Please coordinate with Mika here.
-Daniel
Quoted 59 lines
--- drivers/gpu/drm/i915/i915_gpu_error.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)drivers/gpu/drm/i915/i915_gpu_error.c +18
-4@@ -874,9 +874,6 @@ static void i915_record_ring_state(struct drm_device *dev, ering->hws =I915_READ (mmio); } - ering->cpu_ring_head = ring->buffer->head; - ering->cpu_ring_tail = ring->buffer->tail; - ering->hangcheck_score = ring->hangcheck.score; ering->hangcheck_action = ring->hangcheck.action;@@ -936,6 +933,7 @@ static void i915_gem_record_rings(struct drm_device *dev,for (i =0 ; i <I915_NUM_RINGS ; i++) {struct intel_engine_cs *ring = &dev_priv->ring[i]; +struct intel_ringbuffer *rbuf; error->ring[i].pid =-1 ;@@ -979,8 +977,24 @@ static void i915_gem_record_rings(struct drm_device *dev, } } +if (i915.enable_execlists) { +/* TODO: This is only a small fix to keep basic error +* capture working, but we need to add more information +* for it to be useful (e.g. dump the context being +* executed). +*/ +if (request) + rbuf = request->ctx->engine[ring->id].ringbuf; +else + rbuf = ring->default_context->engine[ring->id].ringbuf; + }else + rbuf = ring->buffer; + + error->ring[i].cpu_ring_head = rbuf->head; + error->ring[i].cpu_ring_tail = rbuf->tail; + error->ring[i].ringbuffer = -i915_error_ggtt_object_create (dev_priv, ring->buffer->obj); +i915_error_ggtt_object_create (dev_priv, rbuf->obj);if (ring->status_page.obj) error->ring[i].hws_page =Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with Execlists
Quoted 25 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, August 15, 2014 1:14 PM
To: Daniel, Thomas
Cc: [email protected]; Mika Kuoppala
Subject: Re: [Intel-gfx] [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with ExeclistsQuoted 13 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
Since the ringbuffer does not belong per engine anymore, we have to make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but we need to add more information for it to be useful (e.g. dump the context being executed).
v2: Reorder how the ringbuffer is chosen to clarify the change and rename the variable, both changes suggested by Chris Wilson. Also, add the TODO comment to the code, as suggested by Daniel.
Quoted 6 lines
Quoted 1 line
Signed-off-by Oscar Mateo <[email protected]>There's a bit too much stuff in-flight to fix up error capture for ppgtt. I think it's better to stall this patch here until that work is completed. Please coordinate with Mika here.
-Daniel
Mika has now closed the Jira issue. This patch still applies and looks correct. Is it OK to be merged as-is?
Thomas.
Quoted 73 lines
drivers/gpu/drm/i915/i915_gpu_error.c static void i915_record_ring_state(struct
Quoted 11 lines
--- drivers/gpu/drm/i915/i915_gpu_error.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)drivers/gpu/drm/i915/i915_gpu_error.c +0
-0@@ -874,9 +874,6 @@ static void i915_record_ring_state(struct
drm_device*dev,drivers/gpu/drm/i915/i915_gpu_error.c static void i915_gem_record_rings(struct
Quoted 19 lines
drivers/gpu/drm/i915/i915_gpu_error.c +1
-3ering->hws =I915_READ (mmio); } - ering->cpu_ring_head = ring->buffer->head; - ering->cpu_ring_tail = ring->buffer->tail; - ering->hangcheck_score = ring->hangcheck.score; ering->hangcheck_action = ring->hangcheck.action;@@ -936,6 +933,7 @@ static void i915_gem_record_rings(struct drm_device *dev, for (i = 0; i < I915_NUM_RINGS; i++) {struct intel_engine_cs *ring = &dev_priv->ring[i]; +struct intel_ringbuffer *rbuf; error->ring[i].pid =-1 ;@@ -979,8 +977,24 @@ static void i915_gem_record_rings(struct
drm_device*dev,drivers/gpu/drm/i915/i915_gpu_error.c static void i915_gem_record_rings(struct drm_device *dev,
Quoted 6 lines
drivers/gpu/drm/i915/i915_gpu_error.c +3
-0} } +if (i915.enable_execlists) { +/* TODO: This is only a small fix to keep basic error +* capture working, but we need to add more information
Quoted 25 lines
drivers/gpu/drm/i915/i915_gpu_error.c +14
-1+* for it to be useful (e.g. dump the context being +* executed). +*/ +if (request) + rbuf = request->ctx->engine[ring- id].ringbuf; +else + rbuf = ring->default_context->engine[ring- id].ringbuf; + }else + rbuf = ring->buffer; + + error->ring[i].cpu_ring_head = rbuf->head; + error->ring[i].cpu_ring_tail = rbuf->tail; + error->ring[i].ringbuffer = -i915_error_ggtt_object_create (dev_priv, ring- buffer->obj); +i915_error_ggtt_object_create (dev_priv, rbuf->obj);drivers/gpu/drm/i915/i915_gpu_error.c +0
-1if (ring->status_page.obj) error->ring[i].hws_page = --1.7.9.5
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with Execlists
Quoted 25 lines · Thomas
Quoted 25 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, August 15, 2014 1:14 PM
To: Daniel, Thomas
Cc: [email protected]; Mika Kuoppala
Subject: Re: [Intel-gfx] [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with ExeclistsQuoted 13 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
Since the ringbuffer does not belong per engine anymore, we have to make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but we need to add more information for it to be useful (e.g. dump the context being executed).
v2: Reorder how the ringbuffer is chosen to clarify the change and rename the variable, both changes suggested by Chris Wilson. Also, add the TODO comment to the code, as suggested by Daniel.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Oscar Mateo <[email protected]>There's a bit too much stuff in-flight to fix up error capture for ppgtt. I think it's better to stall this patch here until that work is completed. Please coordinate with Mika here.
-DanielMika has now closed the Jira issue. This patch still applies and looks correct. Is it OK to be merged as-is?
Apparently Mika closed the Jira before I've merged all the patches. I'd like to do that first, then come back to this patch.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Daniel Vetter
AppliedRe: [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with Execlists
In reply to Daniel, ThomasQuoted 25 lines · Thomas
Quoted 25 lines
-----Original Message-----
From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Friday, August 15, 2014 1:14 PM
To: Daniel, Thomas
Cc: [email protected]; Mika Kuoppala
Subject: Re: [Intel-gfx] [PATCH 35/43] drm/i915/bdw: Make sure error capture keeps working with ExeclistsQuoted 13 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
Since the ringbuffer does not belong per engine anymore, we have to make sure that we are always recording the correct ringbuffer.
TODO: This is only a small fix to keep basic error capture working, but we need to add more information for it to be useful (e.g. dump the context being executed).
v2: Reorder how the ringbuffer is chosen to clarify the change and rename the variable, both changes suggested by Chris Wilson. Also, add the TODO comment to the code, as suggested by Daniel.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Oscar Mateo <[email protected]>There's a bit too much stuff in-flight to fix up error capture for ppgtt. I think it's better to stall this patch here until that work is completed. Please coordinate with Mika here.
-DanielMika has now closed the Jira issue. This patch still applies and looks correct. Is it OK to be merged as-is?
Queued for -next, thanks for the patch.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
PATCH 36/43 drm/i915/bdw: Disable semaphores for Execlists
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
Up until recently, semaphores weren't enabled in BDW so we didn't care about them. But then Rodrigo came and enabled them:
commit 521e62e49a42661a4ee0102644517dbe2f100a23
Author: Rodrigo Vivi <[email protected]>
drm/i915: Enable semaphores on BDW
So now we have to explicitly disable them for Execlists until both features play nicely.
--- drivers/gpu/drm/i915/i915_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/gpu/drm/i915/i915_drv.c +4 -0
@@ -477,6 +477,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)if (i915.semaphores >=0 )return i915.semaphores; +/* TODO: make semaphores and Execlists play nicely together */ +if (i915.enable_execlists) +return false; +#ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ if (INTEL_INFO (dev)->gen ==6 && intel_iommu_gfx_mapped)
Signature · 1 line
1.7.9.5
PATCH 37/43 drm/i915/bdw: Display execlists info in debugfs
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
--- drivers/gpu/drm/i915/i915_debugfs.c | 73 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 6 --- drivers/gpu/drm/i915/intel_lrc.h | 7 ++++ 3 files changed, 80 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +73 -0
@@ -1674,6 +1674,78 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_execlists (struct seq_file *m,void *data) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +u32 status_pointer; +u8 read_pointer; +u8 write_pointer; +u32 status; +u32 ctx_id; +struct list_head *cursor; +int ring_id, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + +for_each_ring (ring, dev_priv, ring_id) { +struct intel_ctx_submit_request *head_req =NULL ; +int count =0 ; +unsigned long flags; + +seq_printf (m,"%s\n" , ring->name); + + status =I915_READ (RING_EXECLIST_STATUS (ring)); + ctx_id =I915_READ (RING_EXECLIST_STATUS (ring) +4 ); +seq_printf (m,"\tExeclist status: 0x%08X, context: %u\n" , + status, ctx_id); + + status_pointer =I915_READ (RING_CONTEXT_STATUS_PTR (ring)); +seq_printf (m,"\tStatus pointer: 0x%08X\n" , status_pointer); + + read_pointer = ring->next_context_status_buffer; + write_pointer = status_pointer &0x07 ; +if (read_pointer > write_pointer) + write_pointer +=6 ; +seq_printf (m,"\tRead pointer: 0x%08X, write pointer 0x%08X\n" , + read_pointer, write_pointer); + +for (i =0 ; i <6 ; i++) { + status =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i); + ctx_id =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i +4 ); + +seq_printf (m,"\tStatus buffer %d: 0x%08X, context: %u\n" , + i, status, ctx_id); + } + +spin_lock_irqsave (&ring->execlist_lock, flags); +list_for_each (cursor, &ring->execlist_queue) + count++; + head_req =list_first_entry_or_null (&ring->execlist_queue, + struct intel_ctx_submit_request, execlist_link); +spin_unlock_irqrestore (&ring->execlist_lock, flags); + +seq_printf (m,"\t%d requests in queue\n" , count); +if (head_req) { +struct drm_i915_gem_object *ctx_obj; + + ctx_obj = head_req->ctx->engine[ring_id].state; +seq_printf (m,"\tHead request id: %u\n" , +intel_execlists_ctx_id (ctx_obj)); +seq_printf (m,"\tHead request tail: %u\n" , head_req->tail); + } + +seq_putc (m,'\n' ); + } + +return 0 ; +} +static int i915_gen6_forcewake_count_info (struct seq_file *m,void *data) {struct drm_info_node *node = m->private;
@@ -3899,6 +3971,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0}, {"i915_ppgtt_info", i915_ppgtt_info, 0},
drivers/gpu/drm/i915/intel_lrc.c +0 -6
@@ -46,12 +46,6 @@#define GEN8_LR_CONTEXT_ALIGN 4096 -#define RING_ELSP (ring) ((ring)->mmio_base+0x230) -#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) -#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) -#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) -#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) -#define RING_EXECLIST_QFULL (1 << 0x2)#define RING_EXECLIST1_VALID (1 << 0x3)#define RING_EXECLIST0_VALID (1 << 0x4)
drivers/gpu/drm/i915/intel_lrc.h +7 -0
@@ -24,6 +24,13 @@#ifndef _INTEL_LRC_H_#define _INTEL_LRC_H_ +/* Execlists regs */ +#define RING_ELSP (ring) ((ring)->mmio_base+0x230) +#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) +#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) +#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) +#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) +/* Logical Rings */ void intel_logical_ring_stop (struct intel_engine_cs *ring);void intel_logical_ring_cleanup (struct intel_engine_cs *ring);
Signature · 1 line
1.7.9.5
Re: [PATCH 37/43] drm/i915/bdw: Display execlists info in debugfs
Quoted 7 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
Signed-off-by Oscar Mateo <[email protected]>
This looks like it may be missing a struct_mutex lock to grab a state atomically.
Signature · 1 line
Damien
Quoted 139 lines
--- drivers/gpu/drm/i915/i915_debugfs.c | 73 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 6 --- drivers/gpu/drm/i915/intel_lrc.h | 7 ++++ 3 files changed, 80 insertions(+), 6 deletions(-)drivers/gpu/drm/i915/i915_debugfs.c +73
-0@@ -1674,6 +1674,78 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_execlists (struct seq_file *m,void *data) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +u32 status_pointer; +u8 read_pointer; +u8 write_pointer; +u32 status; +u32 ctx_id; +struct list_head *cursor; +int ring_id, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + +for_each_ring (ring, dev_priv, ring_id) { +struct intel_ctx_submit_request *head_req =NULL ; +int count =0 ; +unsigned long flags; + +seq_printf (m,"%s\n" , ring->name); + + status =I915_READ (RING_EXECLIST_STATUS (ring)); + ctx_id =I915_READ (RING_EXECLIST_STATUS (ring) +4 ); +seq_printf (m,"\tExeclist status: 0x%08X, context: %u\n" , + status, ctx_id); + + status_pointer =I915_READ (RING_CONTEXT_STATUS_PTR (ring)); +seq_printf (m,"\tStatus pointer: 0x%08X\n" , status_pointer); + + read_pointer = ring->next_context_status_buffer; + write_pointer = status_pointer &0x07 ; +if (read_pointer > write_pointer) + write_pointer +=6 ; +seq_printf (m,"\tRead pointer: 0x%08X, write pointer 0x%08X\n" , + read_pointer, write_pointer); + +for (i =0 ; i <6 ; i++) { + status =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i); + ctx_id =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i +4 ); + +seq_printf (m,"\tStatus buffer %d: 0x%08X, context: %u\n" , + i, status, ctx_id); + } + +spin_lock_irqsave (&ring->execlist_lock, flags); +list_for_each (cursor, &ring->execlist_queue) + count++; + head_req =list_first_entry_or_null (&ring->execlist_queue, + struct intel_ctx_submit_request, execlist_link); +spin_unlock_irqrestore (&ring->execlist_lock, flags); + +seq_printf (m,"\t%d requests in queue\n" , count); +if (head_req) { +struct drm_i915_gem_object *ctx_obj; + + ctx_obj = head_req->ctx->engine[ring_id].state; +seq_printf (m,"\tHead request id: %u\n" , +intel_execlists_ctx_id (ctx_obj)); +seq_printf (m,"\tHead request tail: %u\n" , head_req->tail); + } + +seq_putc (m,'\n' ); + } + +return 0 ; +} +static int i915_gen6_forcewake_count_info (struct seq_file *m,void *data) {struct drm_info_node *node = m->private;@@ -3899,6 +3971,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0}, {"i915_ppgtt_info", i915_ppgtt_info, 0},drivers/gpu/drm/i915/intel_lrc.c +0
-6@@ -46,12 +46,6 @@#define GEN8_LR_CONTEXT_ALIGN 4096 -#define RING_ELSP (ring) ((ring)->mmio_base+0x230) -#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) -#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) -#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) -#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) -#define RING_EXECLIST_QFULL (1 << 0x2)#define RING_EXECLIST1_VALID (1 << 0x3)#define RING_EXECLIST0_VALID (1 << 0x4)drivers/gpu/drm/i915/intel_lrc.h +7
-0@@ -24,6 +24,13 @@#ifndef _INTEL_LRC_H_#define _INTEL_LRC_H_ +/* Execlists regs */ +#define RING_ELSP (ring) ((ring)->mmio_base+0x230) +#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) +#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) +#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) +#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) +/* Logical Rings */ void intel_logical_ring_stop (struct intel_engine_cs *ring);void intel_logical_ring_cleanup (struct intel_engine_cs *ring);Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 37/43 drm/i915/bdw: Display execlists info in debugfs
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
v4: Lock the struct mutex for atomic state capture
--- drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 6 --- drivers/gpu/drm/i915/intel_lrc.h | 7 +++ 3 files changed, 87 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +80 -0
@@ -1674,6 +1674,85 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_execlists (struct seq_file *m,void *data) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +u32 status_pointer; +u8 read_pointer; +u8 write_pointer; +u32 status; +u32 ctx_id; +struct list_head *cursor; +int ring_id, i; +int ret; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + + ret =mutex_lock_interruptible (&dev->struct_mutex); +if (ret) +return ret; + +for_each_ring (ring, dev_priv, ring_id) { +struct intel_ctx_submit_request *head_req =NULL ; +int count =0 ; +unsigned long flags; + +seq_printf (m,"%s\n" , ring->name); + + status =I915_READ (RING_EXECLIST_STATUS (ring)); + ctx_id =I915_READ (RING_EXECLIST_STATUS (ring) +4 ); +seq_printf (m,"\tExeclist status: 0x%08X, context: %u\n" , + status, ctx_id); + + status_pointer =I915_READ (RING_CONTEXT_STATUS_PTR (ring)); +seq_printf (m,"\tStatus pointer: 0x%08X\n" , status_pointer); + + read_pointer = ring->next_context_status_buffer; + write_pointer = status_pointer &0x07 ; +if (read_pointer > write_pointer) + write_pointer +=6 ; +seq_printf (m,"\tRead pointer: 0x%08X, write pointer 0x%08X\n" , + read_pointer, write_pointer); + +for (i =0 ; i <6 ; i++) { + status =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i); + ctx_id =I915_READ (RING_CONTEXT_STATUS_BUF (ring) +8 *i +4 ); + +seq_printf (m,"\tStatus buffer %d: 0x%08X, context: %u\n" , + i, status, ctx_id); + } + +spin_lock_irqsave (&ring->execlist_lock, flags); +list_for_each (cursor, &ring->execlist_queue) + count++; + head_req =list_first_entry_or_null (&ring->execlist_queue, + struct intel_ctx_submit_request, execlist_link); +spin_unlock_irqrestore (&ring->execlist_lock, flags); + +seq_printf (m,"\t%d requests in queue\n" , count); +if (head_req) { +struct drm_i915_gem_object *ctx_obj; + + ctx_obj = head_req->ctx->engine[ring_id].state; +seq_printf (m,"\tHead request id: %u\n" , +intel_execlists_ctx_id (ctx_obj)); +seq_printf (m,"\tHead request tail: %u\n" , head_req->tail); + } + +seq_putc (m,'\n' ); + } + +mutex_unlock (&dev->struct_mutex); + +return 0 ; +} +static int i915_gen6_forcewake_count_info (struct seq_file *m,void *data) {struct drm_info_node *node = m->private;
@@ -3899,6 +3978,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0}, {"i915_ppgtt_info", i915_ppgtt_info, 0},
drivers/gpu/drm/i915/intel_lrc.c +0 -6
@@ -46,12 +46,6 @@#define GEN8_LR_CONTEXT_ALIGN 4096 -#define RING_ELSP (ring) ((ring)->mmio_base+0x230) -#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) -#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) -#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) -#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) -#define RING_EXECLIST_QFULL (1 << 0x2)#define RING_EXECLIST1_VALID (1 << 0x3)#define RING_EXECLIST0_VALID (1 << 0x4)
drivers/gpu/drm/i915/intel_lrc.h +7 -0
@@ -24,6 +24,13 @@#ifndef _INTEL_LRC_H_#define _INTEL_LRC_H_ +/* Execlists regs */ +#define RING_ELSP (ring) ((ring)->mmio_base+0x230) +#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234) +#define RING_CONTEXT_CONTROL (ring) ((ring)->mmio_base+0x244) +#define RING_CONTEXT_STATUS_BUF (ring) ((ring)->mmio_base+0x370) +#define RING_CONTEXT_STATUS_PTR (ring) ((ring)->mmio_base+0x3a0) +/* Logical Rings */ void intel_logical_ring_stop (struct intel_engine_cs *ring);void intel_logical_ring_cleanup (struct intel_engine_cs *ring);
Signature · 1 line
1.7.9.5
Re: [PATCH 37/43] drm/i915/bdw: Display execlists info in debugfs
Quoted 34 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
Signed-off-by Oscar Mateo <[email protected]>v4: Lock the struct mutex for atomic state capture
Signed-off-by Thomas Daniel <[email protected]>--- drivers/gpu/drm/i915/i915_debugfs.c | 80 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 6 --- drivers/gpu/drm/i915/intel_lrc.h | 7 +++ 3 files changed, 87 insertions(+), 6 deletions(-)drivers/gpu/drm/i915/i915_debugfs.c +9
-0@@ -1674,6 +1674,85 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_execlists (struct seq_file *m,void *data) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +u32 status_pointer; +u8 read_pointer; +u8 write_pointer;
drivers/gpu/drm/i915/i915_debugfs.c static int i915_context_status(struct seq_file *m, void *unused) · line 1695
Quoted 10 lines
drivers/gpu/drm/i915/i915_debugfs.c +10
-0+u32 status; +u32 ctx_id; +struct list_head *cursor; +int ring_id, i; +int ret; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + }
checkpatch.pl will tell you seq_puts() should be used here I guess.
Signature · 1 line
Damien
PATCH 38/43 drm/i915/bdw: Display context backing obj & ringbuffer info in debugfs
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
--- drivers/gpu/drm/i915/i915_debugfs.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +23 -2
@@ -1629,6 +1629,12 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)return 0 ; } +static void describe_ctx_ringbuf (struct seq_file *m,struct intel_ringbuffer *ringbuf) +{ +seq_printf (m," (ringbuffer, space: %d, head: %u, tail: %u, last head: %d)" , + ringbuf->space, ringbuf->head, ringbuf->tail, + ringbuf->last_retired_head); +}static int i915_context_status (struct seq_file *m,void *unused) {
@@ -1656,7 +1662,7 @@ static int i915_context_status(struct seq_file *m, void *unused) }list_for_each_entry (ctx, &dev_priv->context_list, link) { -if (ctx->legacy_hw_ctx.rcs_state ==NULL ) +if (!i915.enable_execlists && ctx->legacy_hw_ctx.rcs_state ==NULL )continue ;seq_puts (m,"HW context " );
@@ -1665,7 +1671,22 @@ static int i915_context_status(struct seq_file *m, void *unused)if (ring->default_context == ctx)seq_printf (m,"(default context %s) " , ring->name); -describe_obj (m, ctx->legacy_hw_ctx.rcs_state); +if (i915.enable_execlists) { +seq_putc (m,'\n' ); +for_each_ring (ring, dev_priv, i) { +struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; +struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf; + +seq_printf (m,"%s: " , ring->name); +if (ctx_obj) +describe_obj (m, ctx_obj); +if (ringbuf) +describe_ctx_ringbuf (m, ringbuf); +seq_putc (m,'\n' ); + } + }else +describe_obj (m, ctx->legacy_hw_ctx.rcs_state); +seq_putc (m,'\n' ); }
Signature · 1 line
1.7.9.5
PATCH 39/43 drm/i915/bdw: Print context state in debugfs
In reply to Thomas DanielFrom: Ben Widawsky <[email protected]>
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error state. I've used it quite a bit in debugfs however, and I'd like to keep it here at least until preemption is working.
This patch was accidentally dropped in the first Execlists version, and it has been very useful indeed. Put it back again, but as a standalone debugfs file.
--- drivers/gpu/drm/i915/i915_debugfs.c | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
drivers/gpu/drm/i915/i915_debugfs.c +52 -0
@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_dump_lrc (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +struct intel_context *ctx; +int ret, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + + ret =mutex_lock_interruptible (&dev->mode_config.mutex); +if (ret) +return ret; + +list_for_each_entry (ctx, &dev_priv->context_list, link) { +for_each_ring (ring, dev_priv, i) { +struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; + +if (ring->default_context == ctx) +continue ; + +if (ctx_obj) { +struct page *page =i915_gem_object_get_page (ctx_obj,1 ); +uint32_t *reg_state =kmap_atomic (page); +int j; + +seq_printf (m,"CONTEXT: %s %u\n" , ring->name, +intel_execlists_ctx_id (ctx_obj)); + +for (j =0 ; j <0x600 /sizeof (u32) /4 ; j +=4 ) { +seq_printf (m,"\t[0x%08lx] 0x%08x 0x%08x 0x%08x 0x%08x\n" , +i915_gem_obj_ggtt_offset (ctx_obj) +4096 + (j *4 ), + reg_state[j], reg_state[j +1 ], + reg_state[j +2 ], reg_state[j +3 ]); + } +kunmap_atomic (reg_state); + +seq_putc (m,'\n' ); + } + } + } + +mutex_unlock (&dev->mode_config.mutex); + +return 0 ; +} +static int i915_execlists (struct seq_file *m,void *data) {struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -3992,6 +4043,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_dump_lrc", i915_dump_lrc, 0}, {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0},
Signature · 1 line
1.7.9.5
Re: [PATCH 39/43] drm/i915/bdw: Print context state in debugfs
Quoted 9 lines · Thomas Daniel
From: Ben Widawsky <[email protected]>
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error state. I've used it quite a bit in debugfs however, and I'd like to keep it here at least until preemption is working.
Quoted 7 lines
Signed-off-by Ben Widawsky <[email protected]>This patch was accidentally dropped in the first Execlists version, and it has been very useful indeed. Put it back again, but as a standalone debugfs file.
Signed-off-by Oscar Mateo <[email protected]>
It looks like the locking should be on struct_mutex and not mode_config.mutex?
Signature · 1 line
Damien
Quoted 81 lines
--- drivers/gpu/drm/i915/i915_debugfs.c | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)drivers/gpu/drm/i915/i915_debugfs.c +52
-0@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_dump_lrc (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +struct intel_context *ctx; +int ret, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + + ret =mutex_lock_interruptible (&dev->mode_config.mutex); +if (ret) +return ret; + +list_for_each_entry (ctx, &dev_priv->context_list, link) { +for_each_ring (ring, dev_priv, i) { +struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; + +if (ring->default_context == ctx) +continue ; + +if (ctx_obj) { +struct page *page =i915_gem_object_get_page (ctx_obj,1 ); +uint32_t *reg_state =kmap_atomic (page); +int j; + +seq_printf (m,"CONTEXT: %s %u\n" , ring->name, +intel_execlists_ctx_id (ctx_obj)); + +for (j =0 ; j <0x600 /sizeof (u32) /4 ; j +=4 ) { +seq_printf (m,"\t[0x%08lx] 0x%08x 0x%08x 0x%08x 0x%08x\n" , +i915_gem_obj_ggtt_offset (ctx_obj) +4096 + (j *4 ), + reg_state[j], reg_state[j +1 ], + reg_state[j +2 ], reg_state[j +3 ]); + } +kunmap_atomic (reg_state); + +seq_putc (m,'\n' ); + } + } + } + +mutex_unlock (&dev->mode_config.mutex); + +return 0 ; +} +static int i915_execlists (struct seq_file *m,void *data) {struct drm_info_node *node = (struct drm_info_node *) m->private;@@ -3992,6 +4043,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_dump_lrc", i915_dump_lrc, 0}, {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0},Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 39/43 drm/i915/bdw: Print context state in debugfs
In reply to Thomas DanielFrom: Ben Widawsky <[email protected]>
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error state. I've used it quite a bit in debugfs however, and I'd like to keep it here at least until preemption is working.
This patch was accidentally dropped in the first Execlists version, and it has been very useful indeed. Put it back again, but as a standalone debugfs file.
v2: Take the device struct_mutex rather than mode_config mutex for atomic state capture.
--- drivers/gpu/drm/i915/i915_debugfs.c | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
drivers/gpu/drm/i915/i915_debugfs.c +52 -0
@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_dump_lrc (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +struct intel_context *ctx; +int ret, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + + ret =mutex_lock_interruptible (&dev->struct_mutex); +if (ret) +return ret; + +list_for_each_entry (ctx, &dev_priv->context_list, link) { +for_each_ring (ring, dev_priv, i) { +struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; + +if (ring->default_context == ctx) +continue ; + +if (ctx_obj) { +struct page *page =i915_gem_object_get_page (ctx_obj,1 ); +uint32_t *reg_state =kmap_atomic (page); +int j; + +seq_printf (m,"CONTEXT: %s %u\n" , ring->name, +intel_execlists_ctx_id (ctx_obj)); + +for (j =0 ; j <0x600 /sizeof (u32) /4 ; j +=4 ) { +seq_printf (m,"\t[0x%08lx] 0x%08x 0x%08x 0x%08x 0x%08x\n" , +i915_gem_obj_ggtt_offset (ctx_obj) +4096 + (j *4 ), + reg_state[j], reg_state[j +1 ], + reg_state[j +2 ], reg_state[j +3 ]); + } +kunmap_atomic (reg_state); + +seq_putc (m,'\n' ); + } + } + } + +mutex_unlock (&dev->struct_mutex); + +return 0 ; +} +static int i915_execlists (struct seq_file *m,void *data) {struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -3999,6 +4050,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_dump_lrc", i915_dump_lrc, 0}, {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0},
Signature · 1 line
1.7.9.5
Re: [PATCH 39/43] drm/i915/bdw: Print context state in debugfs
Quoted 15 lines · Thomas Daniel
From: Ben Widawsky <[email protected]>
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error state. I've used it quite a bit in debugfs however, and I'd like to keep it here at least until preemption is working.Signed-off-by Ben Widawsky <[email protected]>This patch was accidentally dropped in the first Execlists version, and it has been very useful indeed. Put it back again, but as a standalone debugfs file.
Quoted 6 lines
Signed-off-by Oscar Mateo <[email protected]>v2: Take the device
struct_mutexrather thanmode_configmutex for atomic state capture.Signed-off-by Thomas Daniel <[email protected]>
Signature · 1 line
Damien
Quoted 81 lines
--- drivers/gpu/drm/i915/i915_debugfs.c | 52 +++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)drivers/gpu/drm/i915/i915_debugfs.c +52
-0@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m, void *unused)return 0 ; } +static int i915_dump_lrc (struct seq_file *m,void *unused) +{ +struct drm_info_node *node = (struct drm_info_node *) m->private; +struct drm_device *dev = node->minor->dev; +struct drm_i915_private *dev_priv = dev->dev_private; +struct intel_engine_cs *ring; +struct intel_context *ctx; +int ret, i; + +if (!i915.enable_execlists) { +seq_printf (m,"Logical Ring Contexts are disabled\n" ); +return 0 ; + } + + ret =mutex_lock_interruptible (&dev->struct_mutex); +if (ret) +return ret; + +list_for_each_entry (ctx, &dev_priv->context_list, link) { +for_each_ring (ring, dev_priv, i) { +struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; + +if (ring->default_context == ctx) +continue ; + +if (ctx_obj) { +struct page *page =i915_gem_object_get_page (ctx_obj,1 ); +uint32_t *reg_state =kmap_atomic (page); +int j; + +seq_printf (m,"CONTEXT: %s %u\n" , ring->name, +intel_execlists_ctx_id (ctx_obj)); + +for (j =0 ; j <0x600 /sizeof (u32) /4 ; j +=4 ) { +seq_printf (m,"\t[0x%08lx] 0x%08x 0x%08x 0x%08x 0x%08x\n" , +i915_gem_obj_ggtt_offset (ctx_obj) +4096 + (j *4 ), + reg_state[j], reg_state[j +1 ], + reg_state[j +2 ], reg_state[j +3 ]); + } +kunmap_atomic (reg_state); + +seq_putc (m,'\n' ); + } + } + } + +mutex_unlock (&dev->struct_mutex); + +return 0 ; +} +static int i915_execlists (struct seq_file *m,void *data) {struct drm_info_node *node = (struct drm_info_node *) m->private;@@ -3999,6 +4050,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {"i915_opregion", i915_opregion, 0}, {"i915_gem_framebuffer", i915_gem_framebuffer_info, 0}, {"i915_context_status", i915_context_status, 0}, + {"i915_dump_lrc", i915_dump_lrc, 0}, {"i915_execlists", i915_execlists, 0}, {"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0}, {"i915_swizzle_info", i915_swizzle_info, 0},Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
PATCH 40/43 drm/i915/bdw: Document Logical Rings, LR contexts and Execlists
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
Add theory of operation notes to intel_lrc.c and comments to externally visible functions.
v2: Add notes on logical ring context creation.
v3: Use kerneldoc.
v4: Integrate it in the DocBook template.
--- Documentation/DocBook/drm.tmpl | 5 + drivers/gpu/drm/i915/intel_lrc.c | 215 +++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_lrc.h | 30 ++++++ 3 files changed, 249 insertions(+), 1 deletion(-)
Documentation/DocBook/drm.tmpl +5 -0
@@ -3909,6 +3909,11 @@ int num_ioctls;</synopsis> !Pdrivers/gpu/drm/i915/i915_cmd_parser.c batch buffer command parser !Idrivers/gpu/drm/i915/i915_cmd_parser.c </sect2> + <sect2> + <title>Logical Rings, Logical Ring Contexts and Execlists</title> +!Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists +!Idrivers/gpu/drm/i915/intel_lrc.c + </sect2> </sect1> </chapter> </part>
drivers/gpu/drm/i915/intel_lrc.c +214 -1
293 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.h +30 -0
@@ -38,10 +38,21 @@ int intel_logical_rings_init(struct drm_device *dev);int logical_ring_flush_all_caches (struct intel_ringbuffer *ringbuf);void intel_logical_ring_advance_and_submit (struct intel_ringbuffer *ringbuf); +/** +* intel_logical_ring_advance() - advance the ringbuffer tail +* @ringbuf: Ringbuffer to advance. +* +* The tail is only updated in our logical ringbuffer struct. +*/ static inline void intel_logical_ring_advance (struct intel_ringbuffer *ringbuf) { ringbuf->tail &= ringbuf->size -1 ; } +/** +* intel_logical_ring_emit() - write a DWORD to the ringbuffer. +* @ringbuf: Ringbuffer to write to. +* @data: DWORD to write. +*/ static inline void intel_logical_ring_emit (struct intel_ringbuffer *ringbuf,u32 data) {iowrite32 (data, ringbuf->virtual_start + ringbuf->tail);
@@ -67,6 +78,25 @@ int intel_execlists_submission(struct drm_device *dev, struct drm_file *file, u64 exec_start, u32 flags);u32 intel_execlists_ctx_id (struct drm_i915_gem_object *ctx_obj); +/** +* struct intel_ctx_submit_request - queued context submission request +* @ctx: Context to submit to the ELSP. +* @ring: Engine to submit it to. +* @tail: how far in the context's ringbuffer this request goes to. +* @execlist_link: link in the submission queue. +* @work: workqueue for processing this request in a bottom half. +* @elsp_submitted: no. of times this request has been sent to the ELSP. +* +* The ELSP only accepts two elements at a time, so we queue context/tail +* pairs on a given queue (ring->execlist_queue) until the hardware is +* available. The queue serves a double purpose: we also use it to keep track +* of the up to 2 contexts currently in the hardware (usually one in execution +* and the other queued up by the GPU): We only remove elements from the head +* of the queue when the hardware informs us that an element has been +* completed. +* +* All accesses to the queue are mediated by a spinlock (ring->execlist_lock). +*/ struct intel_ctx_submit_request {struct intel_context *ctx;struct intel_engine_cs *ring;
Signature · 1 line
1.7.9.5
Re: [PATCH 40/43] drm/i915/bdw: Document Logical Rings, LR contexts and Execlists
Quoted 4 lines · Thomas Daniel
+/**
+ *intel_lr_context_render_state_init()- render state init for Execlists
+ * @ring: Engine Command Streamer.
+ * @ctx: Context to initialize.
Quoted 8 lines
+ *
+ * A.K.A. null-context, A.K.A. golden-context. In a word, the render engine
+ * contexts require to always have a valid 3d pipeline state. As this is
+ * achieved with the submission of a batchbuffer, we require an alternative
+ * entry point to the legacy ringbuffer submission one (i915_gem_render_state_init).
+ *
+ * Return: non-zero if the initialization failed.
+ */
I've dropped this hunk here since that part isn't merged yet. -Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
PATCH 41/43 drm/i915/bdw: Enable Logical Ring Contexts (hence, Execlists)
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
The time has come, the Walrus said, to talk of many things.
--- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/i915/i915_drv.h +1 -1
@@ -2061,7 +2061,7 @@ struct drm_i915_cmd_table {#define I915_NEED_GFX_HWS (dev) (INTEL_INFO(dev)->need_gfx_hws)#define HAS_HW_CONTEXTS (dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_LOGICAL_RING_CONTEXTS (dev) 0 +#define HAS_LOGICAL_RING_CONTEXTS (dev) (INTEL_INFO(dev)->gen >= 8)#define HAS_ALIASING_PPGTT (dev) (INTEL_INFO(dev)->gen >= 6)#define HAS_PPGTT (dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))#define USES_PPGTT (dev) intel_enable_ppgtt(dev, false)
Signature · 1 line
1.7.9.5
Jani Nikula
maintainerRe: [PATCH 41/43] drm/i915/bdw: Enable Logical Ring Contexts (hence, Execlists)
Quoted 3 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
The time has come, the Walrus said, to talk of many things.
FYI this causes https://bugs.freedesktop.org/show_bug.cgi?id=82740
Quoted 25 lines
Signed-off-by Oscar Mateo <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpu/drm/i915/i915_drv.h +1
-1@@ -2061,7 +2061,7 @@ struct drm_i915_cmd_table {#define I915_NEED_GFX_HWS (dev) (INTEL_INFO(dev)->need_gfx_hws)#define HAS_HW_CONTEXTS (dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_LOGICAL_RING_CONTEXTS (dev) 0 +#define HAS_LOGICAL_RING_CONTEXTS (dev) (INTEL_INFO(dev)->gen >= 8)#define HAS_ALIASING_PPGTT (dev) (INTEL_INFO(dev)->gen >= 6)#define HAS_PPGTT (dev) (INTEL_INFO(dev)->gen >= 7 && !IS_GEN8(dev))#define USES_PPGTT (dev) intel_enable_ppgtt(dev, false)Signature · 6 lines
1.7.9.5 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 1 line
Jani Nikula, Intel Open Source Technology Center
Re: [PATCH 41/43] drm/i915/bdw: Enable Logical Ring Contexts (hence, Execlists)
Quoted 9 lines
-----Original Message-----
From: Jani Nikula [mailto:[email protected]]
Sent: Monday, August 18, 2014 9:33 AM
To: Daniel, Thomas; [email protected]
Subject: Re: [Intel-gfx] [PATCH 41/43] drm/i915/bdw: Enable Logical Ring Contexts (hence, Execlists)
Quoted 6 lines
Quoted 3 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
The time has come, the Walrus said, to talk of many things.
FYI this causes https://bugs.freedesktop.org/show_bug.cgi?id=82740
Hmm, this seems to be an interaction between execlists and PPGTT patches, specifically:
http://patchwork.freedesktop.org/patch/31150/
which breaks aliasing PPGTT when execlists are enabled, resulting in this oops on boot.
I will post a patch which avoids this but I don't know if the intention is to drop aliasing PPGTT support when execlists are enabled.
Thomas.
Quoted 30 lines
Quoted 10 lines
Signed-off-by Oscar Mateo <[email protected]>--- drivers/gpu/drm/i915/i915_drv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/gpu/drm/i915/i915_drv.hindex b7cf0ec..1ce51d6 100644drivers/gpu/drm/i915/i915_drv.h struct drm_i915_cmd_table { · line 2067
Quoted 8 lines
drivers/gpu/drm/i915/i915_drv.h +1
-1@@ -2061,7 +2061,7 @@ struct drm_i915_cmd_table {#define I915_NEED_GFX_HWS (dev) (INTEL_INFO(dev)->need_gfx_hws)#define HAS_HW_CONTEXTS (dev) (INTEL_INFO(dev)->gen >= 6) -#define HAS_LOGICAL_RING_CONTEXTS (dev) 0 +#define HAS_LOGICAL_RING_CONTEXTS (dev) (INTEL_INFO(dev)- gen >= 8)#define HAS_ALIASING_PPGTT (dev) (INTEL_INFO(dev)->gen >= 6)#define HAS_PPGTT (dev) (INTEL_INFO(dev)->gen >= 7 &&!IS_GEN8(dev))
Quoted 8 lines
#define
USES_PPGTT(dev)intel_enable_ppgtt(dev, false)
--
1.7.9.5_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Jani Nikula, Intel Open Source Technology Center
PATCH 42/43 drm/i915/bdw: Pin the context backing objects to GGTT on-demand
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
Up until now, we have pinned every logical ring context backing object during creation, and left it pinned until destruction. This made my life easier, but it's a harmful thing to do, because we cause fragmentation of the GGTT (and, eventually, we would run out of space).
This patch makes the pinning on-demand: the backing objects of the two contexts that are written to the ELSP are pinned right before submission and unpinned once the hardware is done with them. The only context that is still pinned regardless is the global default one, so that the HWS can still be accessed in the same way (ring->status_page).
v2: In the early version of this patch, we were pinning the context as we put it into the ELSP: on the one hand, this is very efficient because only a maximum two contexts are pinned at any given time, but on the other hand, we cannot really pin in interrupt time :(
--- drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 44 ++++++++++++++++++++++++----------- drivers/gpu/drm/i915/intel_lrc.c | 42 ++++++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.h | 2 ++ 5 files changed, 73 insertions(+), 27 deletions(-)
drivers/gpu/drm/i915/i915_debugfs.c +9 -2
@@ -1721,10 +1721,15 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)continue ;if (ctx_obj) { -struct page *page =i915_gem_object_get_page (ctx_obj,1 ); -uint32_t *reg_state =kmap_atomic (page); +struct page *page ; +uint32_t *reg_state;int j; +i915_gem_obj_ggtt_pin (ctx_obj,GEN8_LR_CONTEXT_ALIGN ,0 ); + + page =i915_gem_object_get_page (ctx_obj,1 ); + reg_state =kmap_atomic (page); +seq_printf (m,"CONTEXT: %s %u\n" , ring->name,intel_execlists_ctx_id (ctx_obj));
@@ -1736,6 +1741,8 @@ static int i915_dump_lrc(struct seq_file *m, void *unused) }kunmap_atomic (reg_state); +i915_gem_object_ggtt_unpin (ctx_obj); +seq_putc (m,'\n' ); } }
drivers/gpu/drm/i915/i915_drv.h +1 -0
@@ -628,6 +628,7 @@ struct intel_context {struct {struct drm_i915_gem_object *state;struct intel_ringbuffer *ringbuf; +atomic_t unpin_count; } engine[I915_NUM_RINGS ];struct list_head link;
drivers/gpu/drm/i915/i915_gem.c +30 -14
66 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.c +31 -11
98 lines not on this page · raw message
drivers/gpu/drm/i915/intel_lrc.h +2 -0
@@ -24,6 +24,8 @@#ifndef _INTEL_LRC_H_#define _INTEL_LRC_H_ +#define GEN8_LR_CONTEXT_ALIGN 4096 +/* Execlists regs */ #define RING_ELSP (ring) ((ring)->mmio_base+0x230)#define RING_EXECLIST_STATUS (ring) ((ring)->mmio_base+0x234)
Signature · 1 line
1.7.9.5
Re: [PATCH 42/43] drm/i915/bdw: Pin the context backing objects to GGTT on-demand
Quoted 32 lines · Thomas Daniel
From: Oscar Mateo <[email protected]>
Up until now, we have pinned every logical ring context backing object during creation, and left it pinned until destruction. This made my life easier, but it's a harmful thing to do, because we cause fragmentation of the GGTT (and, eventually, we would run out of space).
This patch makes the pinning on-demand: the backing objects of the two contexts that are written to the ELSP are pinned right before submission and unpinned once the hardware is done with them. The only context that is still pinned regardless is the global default one, so that the HWS can still be accessed in the same way (
ring->status_page).v2: In the early version of this patch, we were pinning the context as we put it into the ELSP: on the one hand, this is very efficient because only a maximum two contexts are pinned at any given time, but on the other hand, we cannot really pin in interrupt time :(
Signed-off-by Oscar Mateo <[email protected]>--- drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++-- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 44 ++++++++++++++++++++++++----------- drivers/gpu/drm/i915/intel_lrc.c | 42 ++++++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_lrc.h | 2 ++ 5 files changed, 73 insertions(+), 27 deletions(-)drivers/gpu/drm/i915/i915_debugfs.c +0
-0
drivers/gpu/drm/i915/i915_debugfs.c static int i915_dump_lrc(struct seq_file *m, void *unused) · line 1728
Quoted 10 lines
drivers/gpu/drm/i915/i915_debugfs.c +3
-2@@ -1721,10 +1721,15 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)continue ;if (ctx_obj) { -struct page *page =i915_gem_object_get_page (ctx_obj,1 ); -uint32_t *reg_state =kmap_atomic (page); +struct page *page ; +uint32_t *reg_state;int j; +i915_gem_obj_ggtt_pin (ctx_obj,GEN8_LR_CONTEXT_ALIGN ,0 );
This just needs a get/put_pages, no pinning required.
Quoted 21 lines
drivers/gpu/drm/i915/i915_debugfs.c +6
-0+ + page =i915_gem_object_get_page (ctx_obj,1 ); + reg_state =kmap_atomic (page); +seq_printf (m,"CONTEXT: %s %u\n" , ring->name,intel_execlists_ctx_id (ctx_obj));@@ -1736,6 +1741,8 @@ static int i915_dump_lrc(struct seq_file *m, void *unused) }kunmap_atomic (reg_state); +i915_gem_object_ggtt_unpin (ctx_obj); +seq_putc (m,'\n' ); } }drivers/gpu/drm/i915/i915_drv.h +0
-0
drivers/gpu/drm/i915/i915_drv.h struct intel_context { · line 631
Quoted 4 lines
drivers/gpu/drm/i915/i915_drv.h +1
-0@@ -628,6 +628,7 @@ struct intel_context {struct {struct drm_i915_gem_object *state;struct intel_ringbuffer *ringbuf; +atomic_t unpin_count;
No need to reinvent wheels for this. We should be able to do exactly what we've done for the legacy ctx objects, namely:
- Always pin the default system context so that we can always switch to that.
- Shovel all context releated objects through the active queue and obj management. This might depend upon the reworked exec list item.
- In the shrinker have a last-ditch effort to switch to the default context in case we run out of space.
- igt testcase. For legacy rings this code here resulted in some _really_ expensive bugs and regressions.
I'll do another jira for this.
Otherwise I think I've pulled pretty much everything in except those places where I think directly reworking the patches makes more sense. And we should have JIRA tasks for all the outstanding work (plus ofc getting ppgtt ready since execlists requires that).
If there's a patch I've missed or where people think it makes more sense to merge the wip version now, please pipe up.
Thanks, Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
PATCH 43/43 drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
In reply to Thomas DanielFrom: Oscar Mateo <[email protected]>
Same as with the context, pinning to GGTT regardless is harmful (it badly fragments the GGTT and can even exhaust it).
Unfortunately, this case is also more complex than the previous one because we need to map and access the ringbuffer in several places along the execbuffer path (and we cannot make do by leaving the default ringbuffer pinned, as before). Also, the context object itself contains a pointer to the ringbuffer address that we have to keep updated if we are going to allow the ringbuffer to move around.
v2: Same as with the context pinning, we cannot really do it during an interrupt. Also, pin the default ringbuffers objects regardless (makes error capture a lot easier).
--- drivers/gpu/drm/i915/i915_gem.c | 5 +- drivers/gpu/drm/i915/intel_lrc.c | 80 ++++++++++++++++++++--------- drivers/gpu/drm/i915/intel_ringbuffer.c | 83 ++++++++++++++++++------------- drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++ 4 files changed, 111 insertions(+), 60 deletions(-)
drivers/gpu/drm/i915/i915_gem.c +4 -1
@@ -2498,13 +2498,16 @@ static void i915_gem_free_request(struct drm_i915_gem_request *request)if (ctx) {struct intel_engine_cs *ring = request->ring; +struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;atomic_t *unpin_count = &ctx->engine[ring->id].unpin_count;if (ctx_obj) {if (atomic_dec_return (unpin_count) ==0 && - ctx != ring->default_context) + ctx != ring->default_context) { +intel_unpin_ringbuffer_obj (ringbuf);i915_gem_object_ggtt_unpin (ctx_obj); + } }i915_gem_context_unreference (ctx); }
drivers/gpu/drm/i915/intel_lrc.c +56 -24
179 lines not on this page · raw message
drivers/gpu/drm/i915/intel_ringbuffer.c +48 -35
119 lines not on this page · raw message
drivers/gpu/drm/i915/intel_ringbuffer.h +3 -0
@@ -371,6 +371,9 @@ intel_write_status_page(struct intel_engine_cs *ring,#define I915_GEM_HWS_SCRATCH_INDEX 0x30#define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX << MI_STORE_DWORD_INDEX_SHIFT) +void intel_unpin_ringbuffer_obj (struct intel_ringbuffer *ringbuf); +int intel_pin_and_map_ringbuffer_obj (struct drm_device *dev, +struct intel_ringbuffer *ringbuf);void intel_destroy_ringbuffer_obj (struct intel_ringbuffer *ringbuf);int intel_alloc_ringbuffer_obj (struct drm_device *dev,struct intel_ringbuffer *ringbuf);
Signature · 1 line
1.7.9.5
Please format mails to a width of 75 chars or so. Decent mailers should do that for you when hitting send.
Quoted 4 lines · Thomas Daniel
From: Thomas Daniel <[email protected]>
The previous comment about the WAs still applies. I reproduce it here for completeness:
Quoted 12 lines
"One other caveat I have noticed is that many WAs in
gen8_init_clock_gating(those that affect registers that now exist per-context) can get lost in the render default context. The reason is, in Execlists, a context is saved as soon as head = tail (withMI_SET_CONTEXT, however, the context wouldn't be saved until you tried to restore a different context). As we are sending the golden state batchbuffer to the render ring as soon as the rings are initialized, we are effectively saving the default context beforegen8_init_clock_gatinghas an opportunity to set the WAs. I haven't noticed any ill-effect from this (yet) but it would be a good idea to move the WAs somewhere else (ring init looks like a good place). I believe there is already work in progress to create a new WA architecture, so this can be tackled there."
This sounds like the w/a test patch to compare wa reg state after system s/r, runtime pm, gpu reset and driver reload should also have a test for multiple contexts. I'll add it to the wishlist.
-Daniel
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, Jul 24, 2014 at 05:04:08PM +0100, Thomas Daniel wrote:
All patches can have my r-b tag but patches 12, 34, 37, 39 which have minor comments (in terms of code changes) to address. I did look more at the low-level stuff (Vs the higher level abstractions).
At this point, I believe the way forward is to merge that series to allow more people to beat on it. Step 1 is to make sure we don't regress the legacy ring buffers and now is a good time to land it as we start a new kernel cycle.
Whether to enable it by default for BDW is an interesting question that may depend on the first round of QA.
Signature · 1 line
Damien
Quoted 6 lines · On Fri, 1 Aug 2014 17:09:50 +0100 Damien Lespiau <[email protected]>…
On Thu, Jul 24, 2014 at 05:04:08PM +0100, Thomas Daniel wrote:
All patches can have my r-b tag but patches 12, 34, 37, 39 which have minor comments (in terms of code changes) to address. I did look more at the low-level stuff (Vs the higher level abstractions).
Quoted 7 lines
At this point, I believe the way forward is to merge that series to allow more people to beat on it. Step 1 is to make sure we don't regress the legacy ring buffers and now is a good time to land it as we start a new kernel cycle.
Whether to enable it by default for BDW is an interesting question that may depend on the first round of QA.
Yeah I think we want to enable it on BDW too after getting some testing and sanity checking. The legacy ring buffers aren't getting much testing elsewhere and I'm afraid we'll run into issues that don't exist with the execlist path if we stick with the legacy submission path (we may have already hit one in fact).
Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center