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

Reply via email to