Hi Christian, Mostly minor suggestions I'm afraid. Things just look too good for anything serious.
On 11 October 2015 at 16:09, Christian Gmeiner <christian.gmei...@gmail.com> wrote: > This commit adds tegra support, which uses the renderonly driver > library. > > Signed-off-by: Christian Gmeiner <christian.gmei...@gmail.com> > --- > configure.ac | 19 +++++++- > src/gallium/Makefile.am | 6 +++ > .../auxiliary/target-helpers/inline_drm_helper.h | 29 ++++++++++++ > src/gallium/drivers/tegra/Automake.inc | 10 +++++ > src/gallium/drivers/tegra/Makefile.am | 9 ++++ > src/gallium/targets/dri/Makefile.am | 2 + > src/gallium/winsys/tegra/drm/Android.mk | 34 +++++++++++++++ > src/gallium/winsys/tegra/drm/Makefile.am | 33 ++++++++++++++ > src/gallium/winsys/tegra/drm/Makefile.sources | 3 ++ > src/gallium/winsys/tegra/drm/tegra_drm_public.h | 31 +++++++++++++ > src/gallium/winsys/tegra/drm/tegra_drm_winsys.c | 51 > ++++++++++++++++++++++ > 11 files changed, 226 insertions(+), 1 deletion(-) > create mode 100644 src/gallium/drivers/tegra/Automake.inc > create mode 100644 src/gallium/drivers/tegra/Makefile.am > create mode 100644 src/gallium/winsys/tegra/drm/Android.mk > create mode 100644 src/gallium/winsys/tegra/drm/Makefile.am > create mode 100644 src/gallium/winsys/tegra/drm/Makefile.sources > create mode 100644 src/gallium/winsys/tegra/drm/tegra_drm_public.h > create mode 100644 src/gallium/winsys/tegra/drm/tegra_drm_winsys.c > > diff --git a/configure.ac b/configure.ac > index ea485b1..9fb8244 100644 > --- a/configure.ac > +++ b/configure.ac [snip] > @@ -2166,6 +2167,12 @@ if test -n "$with_gallium_drivers"; then > HAVE_GALLIUM_LLVMPIPE=yes > fi > ;; > + xtegra) > + HAVE_GALLIUM_TEGRA=yes We need an extra NEED_GALLIUM_NOUVEAU conditional (set to yes here and in the xnouveau case). One will also need to duplicate (as a temporary workaround) the nouveau PKG_CHECK_MODULES here. Then update the src/gallium/Makefile.am to use it over HAVE_GALLIUM_NOUVEAU [snip] > +dnl We need to validate some needed dependencies for renderonly drivers. > + > +if test "x$HAVE_GALLIUM_NOUVEAU" != xyes -a "x$HAVE_GALLIUM_TEGRA" == xyes > ; then > + AC_ERROR([Building with tegra requires that nouveau]) > +fi > + > + And then you can drop this hunk. > --- a/src/gallium/auxiliary/target-helpers/inline_drm_helper.h > +++ b/src/gallium/auxiliary/target-helpers/inline_drm_helper.h > @@ -59,6 +59,10 @@ > #include "vc4/drm/vc4_drm_public.h" > #endif > > +#if GALLIUM_TEGRA > +#include "tegra/drm/tegra_drm_public.h" > +#endif > + FYI, I'm just testing some updates/rewrites of these target-helpers, so things might clash in the not so distant future. [snip] > --- /dev/null > +++ b/src/gallium/drivers/tegra/Automake.inc > @@ -0,0 +1,10 @@ > +if HAVE_GALLIUM_TEGRA > + > +TARGET_DRIVERS += tegra > +TARGET_CPPFLAGS += -DGALLIUM_TEGRA > +TARGET_LIB_DEPS += \ > + $(top_builddir)/src/gallium/drivers/renderonly/librenderonly.la \ > + $(top_builddir)/src/gallium/winsys/tegra/drm/libtegradrm.la \ > + $(LIBDRM_LIBS) This, perhaps, should be TEGRA_LIBS, yet we're not using anything from libdrm_tegra so we should be safe. [snip] > --- /dev/null > +++ b/src/gallium/winsys/tegra/drm/Android.mk I think we can drop this file for now. Android + tegra is quite incomplete as is. [snip] > --- /dev/null > +++ b/src/gallium/winsys/tegra/drm/tegra_drm_winsys.c [snip] > +#include "renderonly/renderonly_screen.h" > +#include "../winsys/tegra/drm/tegra_drm_public.h" > +#include "../winsys/nouveau/drm/nouveau_drm_public.h" > + Please rework things to avoid the ../'s > +#include <drm/tegra_drm.h> (as mentioned before) Please drop the path from the include. > +#include <xf86drm.h> > + Flip these two and move them to the top ? > +static int tegra_tiling(int fd, uint32_t handle) > +{ > + struct drm_tegra_gem_set_tiling args; > + > + memset(&args, 0, sizeof(args)); > + args.handle = handle; > + args.mode = DRM_TEGRA_GEM_TILING_MODE_BLOCK; > + args.value = 4; Worth adding a note wrt the magic number ? Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev