On 22 October 2015 at 16:32, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> 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.
>
Or on other words - as is you'll build a static/dynamic combo once.
Following your suggestion one will need to build one pair for each
state-tracker/target.
And yes, it's not that too obvious of a design as is. Suggestions on
the commit message wording and/or documentation are greatly
appreciated.
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
