Attachment 43454 Details for Bug 34332 – possible fix

[patch] possible fix

0001-r300g-fix-a-race-between-CS-and-SET_TILING-ioctls.patch (text/plain), 7.18 KB, created by Marek Olšák on 2011-02-16 13:25:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marek Olšák

Created: 2011-02-16 13:25:45 UTC

Size: 7.18 KB

patch

obsolete

>From fa3f1348e49feeac511dbe5b22bbddc47f56ba81 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <maraeo@gmail.com>
>Date: Wed, 16 Feb 2011 22:23:23 +0100
>Subject: [PATCH] r300g: fix a race between CS and SET_TILING ioctls
>
>---
> src/gallium/drivers/r300/r300_state.c         |    7 +------
> src/gallium/drivers/r300/r300_texture.c       |    2 +-
> src/gallium/drivers/r300/r300_winsys.h        |    2 ++
> src/gallium/winsys/radeon/drm/radeon_drm_bo.c |   13 +++++++++++++
> src/gallium/winsys/radeon/drm/radeon_drm_cs.c |   12 ++++++------
> src/gallium/winsys/radeon/drm/radeon_drm_cs.h |    2 ++
> 6 files changed, 25 insertions(+), 13 deletions(-)
>
>diff --git a/src/gallium/drivers/r300/r300_state.c b/src/gallium/drivers/r300/r300_state.c
>index aa18ab7..5d82983 100644
>--- a/src/gallium/drivers/r300/r300_state.c
>+++ b/src/gallium/drivers/r300/r300_state.c
>@@ -624,12 +624,7 @@ static void r300_tex_set_tiling_flags(struct r300_context *r300,
>      * Skip changing the flags otherwise. */
>     if (tex->tex.macrotile[tex->surface_level] !=
>         tex->tex.macrotile[level]) {
>-        /* Tiling determines how DRM treats the buffer data.
>-         * We must flush CS when changing it if the buffer is referenced. */
>-        if (r300->rws->cs_is_buffer_referenced(r300->cs, tex->cs_buf))
>-            r300->context.flush(&r300->context, 0, NULL);
>-
>-        r300->rws->buffer_set_tiling(tex->buf,
>+        r300->rws->buffer_set_tiling(tex->buf, r300->cs,
>                 tex->tex.microtile, tex->tex.macrotile[level],
>                 tex->tex.stride_in_bytes[0]);
> 
>diff --git a/src/gallium/drivers/r300/r300_texture.c b/src/gallium/drivers/r300/r300_texture.c
>index 45a896d..354144c 100644
>--- a/src/gallium/drivers/r300/r300_texture.c
>+++ b/src/gallium/drivers/r300/r300_texture.c
>@@ -800,7 +800,7 @@ r300_texture_create_object(struct r300_screen *rscreen,
> 
>     tex->cs_buf = rws->buffer_get_cs_handle(tex->buf);
> 
>-    rws->buffer_set_tiling(tex->buf,
>+    rws->buffer_set_tiling(tex->buf, NULL,
>             tex->tex.microtile, tex->tex.macrotile[0],
>             tex->tex.stride_in_bytes[0]);
> 
>diff --git a/src/gallium/drivers/r300/r300_winsys.h b/src/gallium/drivers/r300/r300_winsys.h
>index 6733253..d5c7358 100644
>--- a/src/gallium/drivers/r300/r300_winsys.h
>+++ b/src/gallium/drivers/r300/r300_winsys.h
>@@ -168,6 +168,7 @@ struct r300_winsys_screen {
>      * Set tiling flags describing a memory layout of a buffer object.
>      *
>      * \param buf       A winsys buffer object to set the flags for.
>+     * \param cs        A command stream to flush if the buffer is referenced by it.
>      * \param macrotile A macrotile flag.
>      * \param microtile A microtile flag.
>      * \param stride    A stride of the buffer in bytes, for texturing.
>@@ -175,6 +176,7 @@ struct r300_winsys_screen {
>      * \note microtile and macrotile are not bitmasks!
>      */
>     void (*buffer_set_tiling)(struct r300_winsys_bo *buf,
>+                              struct r300_winsys_cs *cs,
>                               enum r300_buffer_tiling microtile,
>                               enum r300_buffer_tiling macrotile,
>                               unsigned stride);
>diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>index afb8131..3094337 100644
>--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
>@@ -394,13 +394,26 @@ static void radeon_bo_get_tiling(struct r300_winsys_bo *_buf,
> }
> 
> static void radeon_bo_set_tiling(struct r300_winsys_bo *_buf,
>+                                 struct r300_winsys_cs *rcs,
>                                  enum r300_buffer_tiling microtiled,
>                                  enum r300_buffer_tiling macrotiled,
>                                  uint32_t pitch)
> {
>     struct radeon_bo *bo = get_radeon_bo(pb_buffer(_buf));
>+    struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>     struct drm_radeon_gem_set_tiling args = {};
> 
>+    /* Tiling determines how DRM treats the buffer data.
>+     * We must flush CS when changing it if the buffer is referenced. */
>+    if (cs && radeon_bo_is_referenced_by_cs(cs, bo)) {
>+        radeon_drm_cs_flush(rcs);
>+        radeon_drm_cs_sync_flush(rcs);
>+    }
>+
>+    while (p_atomic_read(&bo->num_active_ioctls)) {
>+        sched_yield();
>+    }
>+
>     if (microtiled == R300_BUFFER_TILED)
>         args.tiling_flags |= RADEON_BO_FLAGS_MICRO_TILE;
>     else if (microtiled == R300_BUFFER_SQUARETILED)
>diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>index b4f5c9f..8f6f4a1 100644
>--- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>+++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c
>@@ -330,7 +330,7 @@ static void radeon_drm_cs_write_reloc(struct r300_winsys_cs *rcs,
>     OUT_CS(&cs->base, index * RELOC_DWORDS);
> }
> 
>-static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_async, param)
>+static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_ioctl, param)
> {
>     struct radeon_cs_context *csc = (struct radeon_cs_context*)param;
>     unsigned i;
>@@ -355,7 +355,7 @@ static PIPE_THREAD_ROUTINE(radeon_drm_cs_emit_async, param)
>     return NULL;
> }
> 
>-static void radeon_drm_cs_sync_flush(struct r300_winsys_cs *rcs)
>+void radeon_drm_cs_sync_flush(struct r300_winsys_cs *rcs)
> {
>     struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
> 
>@@ -368,7 +368,7 @@ static void radeon_drm_cs_sync_flush(struct r300_winsys_cs *rcs)
> 
> DEBUG_GET_ONCE_BOOL_OPTION(thread, "RADEON_THREAD", TRUE)
> 
>-static void radeon_drm_cs_emit(struct r300_winsys_cs *rcs)
>+void radeon_drm_cs_flush(struct r300_winsys_cs *rcs)
> {
>     struct radeon_drm_cs *cs = radeon_drm_cs(rcs);
>     struct radeon_cs_context *tmp;
>@@ -385,10 +385,10 @@ static void radeon_drm_cs_emit(struct r300_winsys_cs *rcs)
>             p_atomic_inc(&cs->csc->relocs_bo[i]->num_active_ioctls);
> 
>         if (debug_get_option_thread()) {
>-            cs->thread = pipe_thread_create(radeon_drm_cs_emit_async, cs->csc);
>+            cs->thread = pipe_thread_create(radeon_drm_cs_emit_ioctl, cs->csc);
>             assert(cs->thread);
>         } else {
>-            radeon_drm_cs_emit_async(cs->csc);
>+            radeon_drm_cs_emit_ioctl(cs->csc);
>         }
>     }
> 
>@@ -440,7 +440,7 @@ void radeon_drm_cs_init_functions(struct radeon_drm_winsys *ws)
>     ws->base.cs_add_reloc = radeon_drm_cs_add_reloc;
>     ws->base.cs_validate = radeon_drm_cs_validate;
>     ws->base.cs_write_reloc = radeon_drm_cs_write_reloc;
>-    ws->base.cs_flush = radeon_drm_cs_emit;
>+    ws->base.cs_flush = radeon_drm_cs_flush;
>     ws->base.cs_sync_flush = radeon_drm_cs_sync_flush;
>     ws->base.cs_set_flush = radeon_drm_cs_set_flush;
>     ws->base.cs_is_buffer_referenced = radeon_bo_is_referenced;
>diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.h b/src/gallium/winsys/radeon/drm/radeon_drm_cs.h
>index 486fd23..4cc97f3 100644
>--- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.h
>+++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.h
>@@ -96,6 +96,8 @@ static INLINE boolean radeon_bo_is_referenced_by_any_cs(struct radeon_bo *bo)
>     return bo->num_cs_references;
> }
> 
>+void radeon_drm_cs_flush(struct r300_winsys_cs *rcs);
>+void radeon_drm_cs_sync_flush(struct r300_winsys_cs *rcs);
> void radeon_drm_cs_init_functions(struct radeon_drm_winsys *ws);
> 
> #endif
>-- 
>1.7.1
>

Actions: View | Splinter Review

Attachments on bug 34332: 43425 | 43454

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.