The trace driver is a debugging tool, so I don't mind it getting tainted with api specific code as long as it is useful to somebody.
But since we are it, it would be nice to start discussing what should be the way to address the problem for all drivers. So the problem is that although we've came a long way on gallium-0.3 in layering state tracker / pipe driver / winsys, we still have communication between the state tracker -> winsys. The reason for that communication is typically for 2D driver and OS integration. The evil consequences is that when we wrap pipe drivers on top of other drivers (for tracing, fallback, or whatever), the state tracker -> winsys communication is not handled, and we have to manually wrap/unwrap it for every case. Unfortunately absorving this state tracker -> winsys communication into the pipe driver implies relaxing the principle that a pipe driver is both API and OS independent to some extent. IMO this is an important principle, which should be preserved as much as possible, but not an untouchable one. I think that a pipe_driver interface like struct pipe_driver { void *(*query_interface)(struct pipe_driver *driver, uint32_t interfaceid); }; has been gathering some consensus, since it solves the problem of both version gallium itself, as well providing mechanism for additional apis. Most pipe drivers will handle a few interfaces, e.g., IID_GALLIUM_0_2, IID_GALLIUM_0_3, etc. For other interfaces they will just relay to the winsys, which needs a similar interface. struct winsys_driver { void *(*query_interface)(struct winsys_driver *driver, uint32_t interfaceid); }; A winsys driver will handle API specific interfaces, like IID_DRM, IID_WGL, and also driver speficic interfaces like IID_I915SIMPLE, IID_SOFTPIPE, etc. A trace driver will most likely handle everything. Each pipe driver would have a API-OS independent part, but also API-OS dependent parts, which would only be built for the particular platforms those API/OS exist. To make this divide clear, my suggesting is keeping each pipe_driver interface in a subdirectory: trace/gallium-0.3 trace/wgl trace/drm ... Jose On Thu, 2009-07-02 at 06:08 -0700, Jakob Bornecrantz wrote: > Module: Mesa > Branch: master > Commit: c0d7502a2cf994e635f1383f523653b92f4bd709 > URL: > http://cgit.freedesktop.org/mesa/mesa/commit/?id=c0d7502a2cf994e635f1383f523653b92f4bd709 > > Author: Jakob Bornecrantz <ja...@vmware.com> > Date: Thu Jul 2 14:57:55 2009 +0200 > > trace: Add drm api integration > > This is okay since drm_api.h doesn't have any external > dependancies, one could make it only compile on platforms > that support drm. > > --- > > src/gallium/drivers/trace/Makefile | 1 + > src/gallium/drivers/trace/tr_drm.c | 211 > +++++++++++++++++++++++++++++++++ > src/gallium/drivers/trace/tr_drm.h | 35 ++++++ > src/gallium/drivers/trace/tr_screen.c | 30 +++-- > 4 files changed, 265 insertions(+), 12 deletions(-) > > diff --git a/src/gallium/drivers/trace/Makefile > b/src/gallium/drivers/trace/Makefile > index 4aeb8e3..dd6831c 100644 > --- a/src/gallium/drivers/trace/Makefile > +++ b/src/gallium/drivers/trace/Makefile > @@ -11,6 +11,7 @@ C_SOURCES = \ > tr_screen.c \ > tr_state.c \ > tr_rbug.c \ > + tr_drm.c \ > tr_texture.c > > include ../../Makefile.template > diff --git a/src/gallium/drivers/trace/tr_drm.c > b/src/gallium/drivers/trace/tr_drm.c > new file mode 100644 > index 0000000..98ac75e > --- /dev/null > +++ b/src/gallium/drivers/trace/tr_drm.c > @@ -0,0 +1,211 @@ > +/************************************************************************** > + * > + * Copyright 2009 VMware, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. > + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR > + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, > + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE > + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > + > +#include "state_tracker/drm_api.h" > + > +#include "util/u_memory.h" > +#include "trace/tr_drm.h" > +#include "trace/tr_screen.h" > +#include "trace/tr_context.h" > +#include "trace/tr_buffer.h" > +#include "trace/tr_texture.h" > + > +struct trace_drm_api > +{ > + struct drm_api base; > + > + struct drm_api *api; > +}; > + > +static INLINE struct trace_drm_api * > +trace_drm_api(struct drm_api *_api) > +{ > + return (struct trace_drm_api *)_api; > +} > + > +static struct pipe_screen * > +trace_drm_create_screen(struct drm_api *_api, int fd, > + struct drm_create_screen_arg *arg) > +{ > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct drm_api *api = tr_api->api; > + struct pipe_screen *screen; > + > + /* TODO trace call */ > + > + if (arg && arg->mode != DRM_CREATE_NORMAL) > + return NULL; > + > + screen = api->create_screen(api, fd, arg); > + > + return trace_screen_create(screen); > +}; > + > +static struct pipe_context * > +trace_drm_create_context(struct drm_api *_api, > + struct pipe_screen *_screen) > +{ > + struct trace_screen *tr_screen = trace_screen(_screen); > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct pipe_screen *screen = tr_screen->screen; > + struct drm_api *api = tr_api->api; > + struct pipe_context *pipe; > + > + /* TODO trace call */ > + > + pipe = api->create_context(api, screen); > + > + pipe = trace_context_create(_screen, pipe); > + > + return pipe; > +}; > + > +static boolean > +trace_drm_buffer_from_texture(struct drm_api *_api, > + struct pipe_texture *_texture, > + struct pipe_buffer **_buffer, > + unsigned *stride) > +{ > + struct trace_texture *tr_texture = trace_texture(_texture); > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct pipe_texture *texture = tr_texture->texture; > + struct drm_api *api = tr_api->api; > + struct pipe_buffer *buffer = NULL; > + boolean result; > + > + /* TODO trace call */ > + > + result = api->buffer_from_texture(api, texture, &buffer, stride); > + > + if (result && _buffer) > + buffer = trace_buffer_create(trace_screen(texture->screen), buffer); > + > + if (_buffer) > + *_buffer = buffer; > + else > + pipe_buffer_reference(&buffer, NULL); > + > + return result; > +} > + > +static struct pipe_buffer * > +trace_drm_buffer_from_handle(struct drm_api *_api, > + struct pipe_screen *_screen, > + const char *name, > + unsigned handle) > +{ > + struct trace_screen *tr_screen = trace_screen(_screen); > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct pipe_screen *screen = tr_screen->screen; > + struct drm_api *api = tr_api->api; > + struct pipe_buffer *result; > + > + /* TODO trace call */ > + > + result = api->buffer_from_handle(api, screen, name, handle); > + > + result = trace_buffer_create(trace_screen(_screen), result); > + > + return result; > +} > + > +static boolean > +trace_drm_handle_from_buffer(struct drm_api *_api, > + struct pipe_screen *_screen, > + struct pipe_buffer *_buffer, > + unsigned *handle) > +{ > + struct trace_screen *tr_screen = trace_screen(_screen); > + struct trace_buffer *tr_buffer = trace_buffer(_buffer); > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct pipe_screen *screen = tr_screen->screen; > + struct pipe_buffer *buffer = tr_buffer->buffer; > + struct drm_api *api = tr_api->api; > + > + /* TODO trace call */ > + > + return api->handle_from_buffer(api, screen, buffer, handle); > +} > + > +static boolean > +trace_drm_global_handle_from_buffer(struct drm_api *_api, > + struct pipe_screen *_screen, > + struct pipe_buffer *_buffer, > + unsigned *handle) > +{ > + struct trace_screen *tr_screen = trace_screen(_screen); > + struct trace_buffer *tr_buffer = trace_buffer(_buffer); > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct pipe_screen *screen = tr_screen->screen; > + struct pipe_buffer *buffer = tr_buffer->buffer; > + struct drm_api *api = tr_api->api; > + > + /* TODO trace call */ > + > + return api->global_handle_from_buffer(api, screen, buffer, handle); > +} > + > +static void > +trace_drm_destroy(struct drm_api *_api) > +{ > + struct trace_drm_api *tr_api = trace_drm_api(_api); > + struct drm_api *api = tr_api->api; > + api->destroy(api); > + > + free(tr_api); > +} > + > +struct drm_api * > +trace_drm_create(struct drm_api *api) > +{ > + struct trace_drm_api *tr_api; > + > + if (!api) > + goto error; > + > + if (!trace_enabled()) > + goto error; > + > + tr_api = CALLOC_STRUCT(trace_drm_api); > + > + if (!tr_api) > + goto error; > + > + tr_api->base.create_screen = trace_drm_create_screen; > + tr_api->base.create_context = trace_drm_create_context; > + tr_api->base.buffer_from_texture = trace_drm_buffer_from_texture; > + tr_api->base.buffer_from_handle = trace_drm_buffer_from_handle; > + tr_api->base.handle_from_buffer = trace_drm_handle_from_buffer; > + tr_api->base.global_handle_from_buffer = > trace_drm_global_handle_from_buffer; > + tr_api->base.destroy = trace_drm_destroy; > + tr_api->api = api; > + > + return &tr_api->base; > + > +error: > + return api; > +} > diff --git a/src/gallium/drivers/trace/tr_drm.h > b/src/gallium/drivers/trace/tr_drm.h > new file mode 100644 > index 0000000..845c66a > --- /dev/null > +++ b/src/gallium/drivers/trace/tr_drm.h > @@ -0,0 +1,35 @@ > +/************************************************************************** > + * > + * Copyright 2009 VMware, Inc. > + * All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the > + * "Software"), to deal in the Software without restriction, including > + * without limitation the rights to use, copy, modify, merge, publish, > + * distribute, sub license, and/or sell copies of the Software, and to > + * permit persons to whom the Software is furnished to do so, subject to > + * the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portions > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. > + * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR > + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, > + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE > + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > + * > + **************************************************************************/ > + > +#ifndef TR_DRM_H > +#define TR_DRM_H > + > +struct drm_api; > + > +struct drm_api* trace_drm_create(struct drm_api *api); > + > +#endif /* ID_DRM_H */ > diff --git a/src/gallium/drivers/trace/tr_screen.c > b/src/gallium/drivers/trace/tr_screen.c > index 920f418..5b1e26a 100644 > --- a/src/gallium/drivers/trace/tr_screen.c > +++ b/src/gallium/drivers/trace/tr_screen.c > @@ -38,6 +38,7 @@ > > > static boolean trace = FALSE; > +static boolean rbug = FALSE; > > static const char * > trace_screen_get_name(struct pipe_screen *_screen) > @@ -837,18 +838,11 @@ trace_screen_destroy(struct pipe_screen *_screen) > boolean > trace_enabled(void) > { > - return trace; > -} > + static boolean firstrun = TRUE; > > -struct pipe_screen * > -trace_screen_create(struct pipe_screen *screen) > -{ > - struct trace_screen *tr_scr; > - struct pipe_winsys *winsys; > - boolean rbug = FALSE; > - > - if(!screen) > - goto error1; > + if (!firstrun) > + return trace; > + firstrun = FALSE; > > trace_dump_init(); > > @@ -862,7 +856,19 @@ trace_screen_create(struct pipe_screen *screen) > rbug = TRUE; > } > > - if (!trace) > + return trace; > +} > + > +struct pipe_screen * > +trace_screen_create(struct pipe_screen *screen) > +{ > + struct trace_screen *tr_scr; > + struct pipe_winsys *winsys; > + > + if(!screen) > + goto error1; > + > + if (!trace_enabled()) > goto error1; > > trace_dump_call_begin("", "pipe_screen_create"); > > _______________________________________________ > mesa-commit mailing list > mesa-com...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-commit ------------------------------------------------------------------------------ _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev