I have few doubts/questions.
On 05/03/2019 23:57, Brian Paul wrote:
This will allow us to query whether a context is valid.
In addition to keeping a list of contexts, we need to give each
context we create a unique ID which is never re-used. The screen
also contains a bitmask to track which IDs are valid. We need the
ID since a context pointer could be recycled by the memory allocator.
---
src/gallium/drivers/svga/svga_context.c | 7 +++
src/gallium/drivers/svga/svga_context.h | 6 +++
src/gallium/drivers/svga/svga_screen.c | 86 +++++++++++++++++++++++++++++++++
src/gallium/drivers/svga/svga_screen.h | 23 +++++++++
4 files changed, 122 insertions(+)
diff --git a/src/gallium/drivers/svga/svga_context.c
b/src/gallium/drivers/svga/svga_context.c
index 7b3e9e8..1284d2f 100644
--- a/src/gallium/drivers/svga/svga_context.c
+++ b/src/gallium/drivers/svga/svga_context.c
@@ -57,6 +57,7 @@ DEBUG_GET_ONCE_BOOL_OPTION(force_hw_line_stipple,
"SVGA_FORCE_HW_LINE_STIPPLE",
static void
svga_destroy(struct pipe_context *pipe)
{
+ struct svga_screen *svgascreen = svga_screen(pipe->screen);
struct svga_context *svga = svga_context(pipe);
unsigned shader, i;
@@ -97,6 +98,9 @@ svga_destroy(struct pipe_context *pipe)
svga->swc->destroy(svga->swc);
+ /* remove this context from the screen's list */
+ svga_screen_remove_context(svgascreen, svga);
+
util_bitmask_destroy(svga->blend_object_id_bm);
util_bitmask_destroy(svga->ds_object_id_bm);
util_bitmask_destroy(svga->input_element_object_id_bm);
@@ -300,6 +304,9 @@ svga_context_create(struct pipe_screen *screen, void *priv,
unsigned flags)
svga->pred.query_id = SVGA3D_INVALID_ID;
svga->disable_rasterizer = FALSE;
+ /* add this context to the screen's list */
+ svga_screen_save_context(svgascreen, svga);
+
goto done;
cleanup:
diff --git a/src/gallium/drivers/svga/svga_context.h
b/src/gallium/drivers/svga/svga_context.h
index fc63ec3..2ec6b3f 100644
--- a/src/gallium/drivers/svga/svga_context.h
+++ b/src/gallium/drivers/svga/svga_context.h
@@ -441,6 +441,12 @@ struct svga_context
struct u_upload_mgr *const0_upload;
struct u_upload_mgr *tex_upload;
+ /** used for svga_screen's list of contexts */
+ struct list_head context_node;
+
+ /** A per-context ID which is never reused */
+ unsigned context_id;
+
struct {
boolean no_swtnl;
boolean force_swtnl;
diff --git a/src/gallium/drivers/svga/svga_screen.c
b/src/gallium/drivers/svga/svga_screen.c
index 6cb5a14..6f4e8fc 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -24,6 +24,7 @@
**********************************************************/
#include "git_sha1.h" /* For MESA_GIT_SHA1 */
+#include "util/u_bitmask.h"
#include "util/u_format.h"
#include "util/u_memory.h"
#include "util/u_inlines.h"
@@ -1129,6 +1130,11 @@ svga_screen_create(struct svga_winsys_screen *sws)
debug_printf("svga: msaa samples mask: 0x%x\n", svgascreen->ms_samples);
}
+ LIST_INITHEAD(&svgascreen->contexts);
+ mtx_init(&svgascreen->contexts_mutex, mtx_plain);
+
+ svgascreen->context_id_bm = util_bitmask_create();
+
(void) mtx_init(&svgascreen->tex_mutex, mtx_plain);
(void) mtx_init(&svgascreen->swc_mutex, mtx_recursive);
@@ -1144,6 +1150,86 @@ error1:
}
+/*
+ * Add the given context to the screen's list.
+ * This should be done once when a context is created.
+ */
+void
+svga_screen_save_context(struct svga_screen *svgascreen,
+ struct svga_context *svga)
+{
+ /* This context should not already be in the list */
+ assert(!svga_screen_context_exists(svgascreen, svga));
+
+ /* the context ID should not be set yet */
+ assert(svga->context_id == 0);
+
+ mtx_lock(&svgascreen->contexts_mutex);
+
+ /* Assign a unique ID to the svga context. The ID is never reused */
+ svga->context_id = svgascreen->context_counter++;
post-increment doesn't look right, because context_counter is never
initialied to 1 anywhere. So it seems to me the first context will
have a context_id of zero, which shouldn't happen.
I think we should rename `context_counter` to `last_context_id` and use
pre-increment.
+
+ util_bitmask_set(svgascreen->context_id_bm, svga->context_id);
+
+ LIST_ADDTAIL(&svga->context_node, &svgascreen->contexts);
+
+ mtx_unlock(&svgascreen->contexts_mutex);
+}
+
+
+/*
+ * Remove the given context from the screen's list.
+ * This should be done once when a context is destroyed;
+ */
+void
+svga_screen_remove_context(struct svga_screen *svgascreen,
+ struct svga_context *svga)
+{
+ /* This context should be in the list */
+ assert(svga_screen_context_exists(svgascreen, svga));
+
+ mtx_lock(&svgascreen->contexts_mutex);
+
+ /* remove the ID from the bitmask */
+ util_bitmask_clear(svgascreen->context_id_bm, svga->context_id);
+
+ LIST_DEL(&svga->context_node);
+
+ mtx_unlock(&svgascreen->contexts_mutex);
+}
+
+
+/*
+ * Return true if the context exists in the screen's list and its ID
+ * is valid, false otherwise.
+ */
+boolean
+svga_screen_context_exists(struct svga_screen *svgascreen,
+ const struct svga_context *svga)
+{
+ struct svga_context *ctx;
+ boolean found = FALSE;
+
+ mtx_lock(&svgascreen->contexts_mutex);
+
+ LIST_FOR_EACH_ENTRY(ctx, &svgascreen->contexts, context_node) {
+ if (ctx == svga) {
+ /* Also check that the context ID is valid since a context
+ * pointer could be recycled by the memory allocator.
+ */
+ if (util_bitmask_get(svgascreen->context_id_bm, svga->context_id)) {
At a glance context_id_bm seems overkill. Why not remove the context
from svgascreen->contexts immediately?
It's not clear what these three things mean:
- belonging to svgascreen->contexts
- belonging to svgascreen->context_id_bm
- have non zero svga->context_id
+ found = TRUE;
+ break;
+ }
+ }
+ }
+
+ mtx_unlock(&svgascreen->contexts_mutex);
+
+ return found;
+}
+
+
struct svga_winsys_screen *
svga_winsys_screen(struct pipe_screen *screen)
{
diff --git a/src/gallium/drivers/svga/svga_screen.h
b/src/gallium/drivers/svga/svga_screen.h
index 12b9346..5ada452 100644
--- a/src/gallium/drivers/svga/svga_screen.h
+++ b/src/gallium/drivers/svga/svga_screen.h
@@ -45,6 +45,15 @@ struct svga_screen
struct pipe_screen screen;
struct svga_winsys_screen *sws;
+ /** list of svga_contexts */
+ struct list_head contexts;
+ mtx_t contexts_mutex;
+
I'd put everything that needs to be protected by the mutex into a
structure, like
struct {
mtx_t mutex;
struct list_head list;
...
} contexts;
Because it's not immedaitely obvious that all recently added members
need the mutex.
+ /** Bitmask used to allocate context IDs */
+ struct util_bitmask *context_id_bm;
I don't see context_id_bm being freed anywhere.
+
+ unsigned context_counter;
+
SVGA3dHardwareVersion hw_version;
/** Device caps */
@@ -102,4 +111,18 @@ struct svga_screen *
svga_screen(struct pipe_screen *screen);
#endif
+
+void
+svga_screen_save_context(struct svga_screen *screen,
+ struct svga_context *svga);
+
+void
+svga_screen_remove_context(struct svga_screen *screen,
+ struct svga_context *svga);
+
+boolean
+svga_screen_context_exists(struct svga_screen *screen,
+ const struct svga_context *svga);
+
+
#endif /* SVGA_SCREEN_H */
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
