diff mbox

[RFC,024/111] staging: etnaviv: fix multiple command buffer submission in etnaviv_buffer_queue()

Message ID 1427988653-754-25-git-send-email-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lucas Stach April 2, 2015, 3:29 p.m. UTC 
From: Russell King <rmk+kernel@arm.linux.org.uk>

etnaviv_buffer_queue() could not handle multiple command buffers.  We
can handle this trivially by adapting the existing code - we record
where we want to link to, and walk the submitted command buffers in
reverse order, appending a LINK command to the previous target.

This also means that we conveniently end up with the address and size
to link to when changing the previous WAIT command.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/staging/etnaviv/etnaviv_buffer.c | 45 +++++++++++++++++---------------
 1 file changed, 24 insertions(+), 21 deletions(-)

Comments

Christian Gmeiner April 5, 2015, 6:36 p.m. UTC | #1 
Is there a real use case to make use of multiple command buffers per
one submit ioctl? I have removed
it from my kernel tree.

2015-04-02 17:29 GMT+02:00 Lucas Stach <l.stach@pengutronix.de>:
> From: Russell King <rmk+kernel@arm.linux.org.uk>
>
> etnaviv_buffer_queue() could not handle multiple command buffers.  We
> can handle this trivially by adapting the existing code - we record
> where we want to link to, and walk the submitted command buffers in
> reverse order, appending a LINK command to the previous target.
>
> This also means that we conveniently end up with the address and size
> to link to when changing the previous WAIT command.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  drivers/staging/etnaviv/etnaviv_buffer.c | 45 +++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/etnaviv/etnaviv_buffer.c b/drivers/staging/etnaviv/etnaviv_buffer.c
> index 945af22db3f1..be2b02ce9a46 100644
> --- a/drivers/staging/etnaviv/etnaviv_buffer.c
> +++ b/drivers/staging/etnaviv/etnaviv_buffer.c
> @@ -148,7 +148,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
>         struct etnaviv_gem_object *buffer = to_etnaviv_bo(gpu->buffer);
>         struct etnaviv_gem_object *cmd;
>         u32 *lw = buffer->vaddr + ((buffer->offset - 4) * 4);
> -       u32 back;
> +       u32 back, link_target, link_size;
>         u32 i;
>
>         if (drm_debug & DRM_UT_DRIVER)
> @@ -156,6 +156,8 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
>
>         /* save offset back into main buffer */
>         back = buffer->offset;
> +       link_target = buffer->paddr + buffer->offset * 4;
> +       link_size = 6;
>
>         /* trigger event */
>         CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) | VIVS_GL_EVENT_FROM_PE);
> @@ -165,27 +167,28 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
>         CMD_LINK(buffer, 2, buffer->paddr + ((buffer->offset - 1) * 4));
>
>         /* update offset for every cmd stream */
> -       for (i = 0; i < submit->nr_cmds; i++)
> -               submit->cmd[i].obj->offset = submit->cmd[i].offset +
> -                                            submit->cmd[i].size;
> +       for (i = submit->nr_cmds; i--; ) {
> +               cmd = submit->cmd[i].obj;
>
> -       /* TODO: inter-connect all cmd buffers */
> +               cmd->offset = submit->cmd[i].offset + submit->cmd[i].size;
>
> -       /* jump back from last cmd to main buffer */
> -       cmd = submit->cmd[submit->nr_cmds - 1].obj;
> -       CMD_LINK(cmd, 4, buffer->paddr + (back * 4));
> +               if (drm_debug & DRM_UT_DRIVER)
> +                       pr_info("stream link from buffer %u to 0x%08x @ 0x%08x %p\n",
> +                               i, link_target,
> +                               cmd->paddr + cmd->offset * 4,
> +                               cmd->vaddr + cmd->offset * 4);
>
> -       /* update the size */
> -       for (i = 0; i < submit->nr_cmds; i++)
> -               submit->cmd[i].size = submit->cmd[i].obj->offset -
> -                                     submit->cmd[i].offset;
> +               /* jump back from last cmd to main buffer */
> +               CMD_LINK(cmd, link_size, link_target);
>
> -       if (drm_debug & DRM_UT_DRIVER) {
> -               pr_info("stream link @ 0x%08x\n",
> -                       cmd->paddr + ((cmd->offset - 1) * 4));
> -               pr_info("stream link @ %p\n",
> -                       cmd->vaddr + ((cmd->offset - 1) * 4));
> +               /* update the size */
> +               submit->cmd[i].size = cmd->offset - submit->cmd[i].offset;
> +
> +               link_target = cmd->paddr + submit->cmd[i].offset * 4;
> +               link_size = submit->cmd[i].size * 2;
> +       }
>
> +       if (drm_debug & DRM_UT_DRIVER) {
>                 for (i = 0; i < submit->nr_cmds; i++) {
>                         struct etnaviv_gem_object *obj = submit->cmd[i].obj;
>
> @@ -195,16 +198,16 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
>
>                 pr_info("link op: %p\n", lw);
>                 pr_info("link addr: %p\n", lw + 1);
> -               pr_info("addr: 0x%08x\n", submit->cmd[0].obj->paddr);
> +               pr_info("addr: 0x%08x\n", link_target);
>                 pr_info("back: 0x%08x\n", buffer->paddr + (back * 4));
>                 pr_info("event: %d\n", event);
>         }
>
>         /* Change WAIT into a LINK command; write the address first. */
> -       i = VIV_FE_LINK_HEADER_OP_LINK | VIV_FE_LINK_HEADER_PREFETCH(submit->cmd[0].size * 2);
> -       *(lw + 1) = submit->cmd[0].obj->paddr + submit->cmd[0].offset * 4;
> +       *(lw + 1) = link_target;
>         mb();
> -       *(lw)= i;
> +       *(lw) = VIV_FE_LINK_HEADER_OP_LINK |
> +               VIV_FE_LINK_HEADER_PREFETCH(link_size);
>         mb();
>
>         if (drm_debug & DRM_UT_DRIVER)
> --
> 2.1.4
>

--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner
diff mbox

Patch

diff --git a/drivers/staging/etnaviv/etnaviv_buffer.c b/drivers/staging/etnaviv/etnaviv_buffer.c
index 945af22db3f1..be2b02ce9a46 100644
--- a/drivers/staging/etnaviv/etnaviv_buffer.c
+++ b/drivers/staging/etnaviv/etnaviv_buffer.c
@@ -148,7 +148,7 @@  void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
 	struct etnaviv_gem_object *buffer = to_etnaviv_bo(gpu->buffer);
 	struct etnaviv_gem_object *cmd;
 	u32 *lw = buffer->vaddr + ((buffer->offset - 4) * 4);
-	u32 back;
+	u32 back, link_target, link_size;
 	u32 i;
 
 	if (drm_debug & DRM_UT_DRIVER)
@@ -156,6 +156,8 @@  void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
 
 	/* save offset back into main buffer */
 	back = buffer->offset;
+	link_target = buffer->paddr + buffer->offset * 4;
+	link_size = 6;
 
 	/* trigger event */
 	CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) | VIVS_GL_EVENT_FROM_PE);
@@ -165,27 +167,28 @@  void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
 	CMD_LINK(buffer, 2, buffer->paddr + ((buffer->offset - 1) * 4));
 
 	/* update offset for every cmd stream */
-	for (i = 0; i < submit->nr_cmds; i++)
-		submit->cmd[i].obj->offset = submit->cmd[i].offset +
-					     submit->cmd[i].size;
+	for (i = submit->nr_cmds; i--; ) {
+		cmd = submit->cmd[i].obj;
 
-	/* TODO: inter-connect all cmd buffers */
+		cmd->offset = submit->cmd[i].offset + submit->cmd[i].size;
 
-	/* jump back from last cmd to main buffer */
-	cmd = submit->cmd[submit->nr_cmds - 1].obj;
-	CMD_LINK(cmd, 4, buffer->paddr + (back * 4));
+		if (drm_debug & DRM_UT_DRIVER)
+			pr_info("stream link from buffer %u to 0x%08x @ 0x%08x %p\n",
+				i, link_target,
+				cmd->paddr + cmd->offset * 4,
+				cmd->vaddr + cmd->offset * 4);
 
-	/* update the size */
-	for (i = 0; i < submit->nr_cmds; i++)
-		submit->cmd[i].size = submit->cmd[i].obj->offset -
-				      submit->cmd[i].offset;
+		/* jump back from last cmd to main buffer */
+		CMD_LINK(cmd, link_size, link_target);
 
-	if (drm_debug & DRM_UT_DRIVER) {
-		pr_info("stream link @ 0x%08x\n",
-			cmd->paddr + ((cmd->offset - 1) * 4));
-		pr_info("stream link @ %p\n",
-			cmd->vaddr + ((cmd->offset - 1) * 4));
+		/* update the size */
+		submit->cmd[i].size = cmd->offset - submit->cmd[i].offset;
+
+		link_target = cmd->paddr + submit->cmd[i].offset * 4;
+		link_size = submit->cmd[i].size * 2;
+	}
 
+	if (drm_debug & DRM_UT_DRIVER) {
 		for (i = 0; i < submit->nr_cmds; i++) {
 			struct etnaviv_gem_object *obj = submit->cmd[i].obj;
 
@@ -195,16 +198,16 @@  void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, unsigned int event, struct et
 
 		pr_info("link op: %p\n", lw);
 		pr_info("link addr: %p\n", lw + 1);
-		pr_info("addr: 0x%08x\n", submit->cmd[0].obj->paddr);
+		pr_info("addr: 0x%08x\n", link_target);
 		pr_info("back: 0x%08x\n", buffer->paddr + (back * 4));
 		pr_info("event: %d\n", event);
 	}
 
 	/* Change WAIT into a LINK command; write the address first. */
-	i = VIV_FE_LINK_HEADER_OP_LINK | VIV_FE_LINK_HEADER_PREFETCH(submit->cmd[0].size * 2);
-	*(lw + 1) = submit->cmd[0].obj->paddr + submit->cmd[0].offset * 4;
+	*(lw + 1) = link_target;
 	mb();
-	*(lw)= i;
+	*(lw) = VIV_FE_LINK_HEADER_OP_LINK |
+		VIV_FE_LINK_HEADER_PREFETCH(link_size);
 	mb();
 
 	if (drm_debug & DRM_UT_DRIVER)