On Wed, Jan 8, 2014 at 8:23 PM, Steven Newbury <st...@snewbury.org.uk> wrote:
> On Fri, 2014-01-03 at 13:14 +0800, Chia-I Wu wrote:
>> On Thu, Jan 2, 2014 at 10:39 PM, Steven Newbury <st...@snewbury.org.uk>
>> wrote:
>> > Forgot to add signed-off-by...
>> >
>> > In trying to get gallium-nine working with the ilo Gallium driver I
>> > noticed there's no ilo pipe-loader driver being built.
>> >
>> > This patch simply puts in place the missing pieces.
>> >
>> > The driver descriptor is named "ilo", rather than "i965" as the ilo DRI
>> > driver currently names itself, this is necessary as otherwise the
>> > pipe-loader refuses to load as it has a sanity check verifying the name
>> > matches. A follow-up patch renames the ilo DRI driver descriptor to
>> > match.
>> >
>> > Signed-off-by: Steven Newbury <st...@snewbury.org.uk>
>> > ---
>> > include/pci_ids/pci_id_driver_map.h | 4 +++-
>> > src/gallium/targets/pipe-loader/Makefile.am | 17 +++++++++++++++++
>> > src/gallium/targets/pipe-loader/pipe_ilo.c | 27
>> > +++++++++++++++++++++++++++
>> > 3 files changed, 47 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/include/pci_ids/pci_id_driver_map.h
>> > b/include/pci_ids/pci_id_driver_map.h
>> > index 8a97c6f..1fb0467 100644
>> > --- a/include/pci_ids/pci_id_driver_map.h
>> > +++ b/include/pci_ids/pci_id_driver_map.h
>> > @@ -64,10 +64,12 @@ static const struct {
>> > int num_chips_ids;
>> > } driver_map[] = {
>> > { 0x8086, "i915", i915_chip_ids, ARRAY_SIZE(i915_chip_ids) },
>> > - { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
>> > #ifndef DRIVER_MAP_GALLIUM_ONLY
>> > + { 0x8086, "i965", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
>> > { 0x1002, "radeon", r100_chip_ids, ARRAY_SIZE(r100_chip_ids) },
>> > { 0x1002, "r200", r200_chip_ids, ARRAY_SIZE(r200_chip_ids) },
>> > +#else
>> > + { 0x8086, "ilo", i965_chip_ids, ARRAY_SIZE(i965_chip_ids) },
>> > #endif
>> Moving "i965" into the #ifndef looks correct to me, but having "ilo"
>> in the #else looks hacky. For in this map, "ilo" should be always
>> defined by definition, and supports a subset of i965_chip_ids.
>>
> I guess it is hacky in the sense that without DRIVER_MAP_GALLIUM_ONLY it
> should return all drivers, but there is no provision (nor use) for
> returning two drivers for an opened drm device. You're absolutely right
> though, I should check which devices are actually supported and create a
> new array for "ilo_chip_ids".
There is no rule as to what to do when two drivers support the same
devices. In practice, if you list ilo after i965, and move i965 into
the #ifdef, things may work for your need. But it should not be
relied on.
IMO, pipe loader uses the driver map to auto-detect the driver.
Having an environment variable to skip auto-detection makes sense
(e.g., to load the driver from a different path, or to force loading
the driver for an unknown device, and etc.).
>
>> I am actually in favor of an environment variable that overrides the
>> auto-detection of the driver in the pipe loader, thus skipping this
>> map.
> This does remind me of related, perhaps overlapping issue. From the
> point of view of the DRI extension there's already a specified (or
> default) DRI driver which gets returned in DRI2Connect(). Perhaps this
> is the place to read the driver from except as far as I can tell it
> provides no override from a running Xserver.
>
> The ability to be able to use an alternative driver is really useful for
> ilo. An environment variable override is definitely a good idea, I'm
> not sure about skipping the map entirely and not having a default
> though.
>
>>
>> > { 0x1002, "r300", r300_chip_ids, ARRAY_SIZE(r300_chip_ids) },
>> > { 0x1002, "r600", r600_chip_ids, ARRAY_SIZE(r600_chip_ids) },
>> > diff --git a/src/gallium/targets/pipe-loader/Makefile.am
>> > b/src/gallium/targets/pipe-loader/Makefile.am
>> > index 6875453..8fa3873 100644
>> > --- a/src/gallium/targets/pipe-loader/Makefile.am
>> > +++ b/src/gallium/targets/pipe-loader/Makefile.am
>> > @@ -47,6 +47,23 @@ PIPE_LIBS = \
>> > -lpthread \
>> > -lm
>> >
>> > +if HAVE_GALLIUM_ILO
>> > +pipe_LTLIBRARIES += pipe_ilo.la
>> > +pipe_ilo_la_SOURCES = pipe_ilo.c
>> > +pipe_ilo_la_LIBADD = \
>> > + $(PIPE_LIBS) \
>> > + $(top_builddir)/src/gallium/winsys/intel/drm/libintelwinsys.la \
>> > + $(top_builddir)/src/gallium/drivers/ilo/libilo.la \
>> > + $(LIBDRM_LIBS) \
>> > + $(INTEL_LIBS)
>> > +pipe_ilo_la_LDFLAGS = -no-undefined -avoid-version -module
>> > +if HAVE_MESA_LLVM
>> > +nodist_EXTRA_pipe_ilo_la_SOURCES = dummy.cpp
>> > +pipe_ilo_la_LIBADD += $(LLVM_LIBS)
>> > +pipe_ilo_la_LDFLAGS += $(LLVM_LDFLAGS)
>> > +endif
>> > +endif
>> > +
>> > if HAVE_GALLIUM_I915
>> > pipe_LTLIBRARIES += pipe_i915.la
>> > pipe_i915_la_SOURCES = pipe_i915.c
>> > diff --git a/src/gallium/targets/pipe-loader/pipe_ilo.c
>> > b/src/gallium/targets/pipe-loader/pipe_ilo.c
>> > new file mode 100644
>> > index 0000000..11be2d1
>> > --- /dev/null
>> > +++ b/src/gallium/targets/pipe-loader/pipe_ilo.c
>> > @@ -0,0 +1,27 @@
>> > +
>> > +#include "target-helpers/inline_debug_helper.h"
>> > +#include "state_tracker/drm_driver.h"
>> > +#include "intel/intel_winsys.h"
>> > +#include "ilo/ilo_public.h"
>> > +
>> > +static struct pipe_screen *
>> > +create_screen(int fd)
>> > +{
>> > + struct intel_winsys *iws;
>> > + struct pipe_screen *screen;
>> > +
>> > + iws = intel_winsys_create_for_fd(fd);
>> > + if (!iws)
>> > + return NULL;
>> > +
>> > + screen = ilo_screen_create(iws);
>> > + if (!screen)
>> > + return NULL;
>> > +
>> > + screen = debug_screen_wrap(screen);
>> > +
>> > + return screen;
>> > +}
>> > +
>> > +PUBLIC
>> > +DRM_DRIVER_DESCRIPTOR("ilo", "i915", create_screen, NULL)
>> >
>> >
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>>
>
>
>
--
o...@lunarg.com
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
