[Mesa-dev] [PATCH] ilo: build pipe-loader driver]

[Steven Newbury]

On Fri, 2014-01-03 at 13:14 +0800, Chia-I Wu wrote:
> On Thu, Jan 2, 2014 at 10:39 PM, Steven Newbury <steve at 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 <steve at 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".

> 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
>