On 22 October 2015 at 15:07, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 18.10.2015 00:57, Emil Velikov wrote:
>>
>> Add a list of driver descriptors and select one from the list, during
>> probe time.
>>
>> As we'll need to have all the driver pipe_foo_screen_create() functions
>> provided externally (i.e. from another static lib) we need a separate
>> (non-inline) drm_helper, which contains the function declarations.
>>
>> XXX: More than happy to rename things - header/functions/etc.
>>
>> Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
>> ---
>> src/gallium/auxiliary/pipe-loader/Makefile.am | 6 +-
>> .../auxiliary/pipe-loader/pipe_loader_drm.c | 119
>> ++++++++++++++++++++-
>> .../auxiliary/target-helpers/drm_helper_public.h | 34 ++++++
>> 3 files changed, 154 insertions(+), 5 deletions(-)
>> create mode 100644
>> src/gallium/auxiliary/target-helpers/drm_helper_public.h
>>
>> diff --git a/src/gallium/auxiliary/pipe-loader/Makefile.am
>> b/src/gallium/auxiliary/pipe-loader/Makefile.am
>> index 6a4a667..7db4190 100644
>> --- a/src/gallium/auxiliary/pipe-loader/Makefile.am
>> +++ b/src/gallium/auxiliary/pipe-loader/Makefile.am
>> @@ -34,12 +34,12 @@ AM_CFLAGS += \
>> libpipe_loader_static_la_SOURCES += \
>> $(DRM_SOURCES)
>>
>> -libpipe_loader_dynamic_la_SOURCES += \
>> - $(DRM_SOURCES)
>> -
>> libpipe_loader_static_la_LIBADD = \
>> $(top_builddir)/src/loader/libloader.la
>>
>> +libpipe_loader_dynamic_la_SOURCES += \
>> + $(DRM_SOURCES)
>> +
>> libpipe_loader_dynamic_la_LIBADD = \
>> $(top_builddir)/src/loader/libloader.la
>>
>> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> index 33274de..97e9dcb 100644
>> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> @@ -36,6 +36,7 @@
>> #include <unistd.h>
>>
>> #include "loader.h"
>> +#include "target-helpers/drm_helper_public.h"
>> #include "state_tracker/drm_driver.h"
>> #include "pipe_loader_priv.h"
>>
>> @@ -51,7 +52,9 @@
>> struct pipe_loader_drm_device {
>> struct pipe_loader_device base;
>> const struct drm_driver_descriptor *dd;
>> +#ifndef GALLIUM_STATIC_TARGETS
>> struct util_dl_library *lib;
>> +#endif
>> int fd;
>> };
>>
>> @@ -59,6 +62,103 @@ struct pipe_loader_drm_device {
>>
>> static const struct pipe_loader_ops pipe_loader_drm_ops;
>>
>> +#ifdef GALLIUM_STATIC_TARGETS
>> +static const struct drm_conf_ret throttle_ret = {
>> + DRM_CONF_INT,
>> + {2},
>> +};
>> +
>> +static const struct drm_conf_ret share_fd_ret = {
>> + DRM_CONF_BOOL,
>> + {true},
>> +};
>> +
>> +static inline const struct drm_conf_ret *
>> +configuration_query(enum drm_conf conf)
>> +{
>> + switch (conf) {
>> + case DRM_CONF_THROTTLE:
>> + return &throttle_ret;
>> + case DRM_CONF_SHARE_FD:
>> + return &share_fd_ret;
>> + default:
>> + break;
>> + }
>> + return NULL;
>> +}
>> +
>> +static const struct drm_driver_descriptor driver_descriptors[] = {
>> + {
>> + .name = "i915",
>> + .driver_name = "i915",
>> + .create_screen = pipe_i915_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "i965",
>> + .driver_name = "i915",
>> + .create_screen = pipe_ilo_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "nouveau",
>> + .driver_name = "nouveau",
>> + .create_screen = pipe_nouveau_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "r300",
>> + .driver_name = "radeon",
>> + .create_screen = pipe_r300_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "r600",
>> + .driver_name = "radeon",
>> + .create_screen = pipe_r600_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "radeonsi",
>> + .driver_name = "radeon",
>> + .create_screen = pipe_radeonsi_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "vmwgfx",
>> + .driver_name = "vmwgfx",
>> + .create_screen = pipe_vmwgfx_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "kgsl",
>> + .driver_name = "freedreno",
>> + .create_screen = pipe_freedreno_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "msm",
>> + .driver_name = "freedreno",
>> + .create_screen = pipe_freedreno_create_screen,
>> + .configuration = configuration_query,
>> + },
>> + {
>> + .name = "vc4",
>> + .driver_name = "vc4",
>> + .create_screen = pipe_vc4_create_screen,
>> + .configuration = configuration_query,
>> + },
>
>
> I believe these should be guarded by the respective #if
> defined(GALLIUM_xxx).
>
> I see that in patch 25 (target-helpers: add a non-inline drm_helper.h) you
> change the pipe_XXX_create_screen functions so that they return NULL if the
> corresponding driver has not been configured.
>
I'm afraid that adding the GALLIUM_$DRIVER define guards is not what
we want here. The idea being that the pipe-loader is a component
linked alongside the state-tracker, thus all the pipe_*_create_screen
should be available, with the appropriate implementation for each
function dependant on the target.
For example: pipe-loader should always list pipe_r300_create_screen
symbol as 'requirement', which will be resolved as { return NULL } for
OMX or { radeon_specifics } for DRI.
> However, using #if guards here instead is bound to provide a clearer
> distinction between the "create_screen failed" and "driver missing" failure
> modes.
>
If you want I can add the fprintf(stderr, "foo: driver missing\n") in
the dummy (return NULL) cases for patch 25 ?
Thanks for having a look Nicolai and welcome back to mesa :-)
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
