[Mesa-dev] [PATCH 5/5] tegra: Initial support

[Emil Velikov]

Hi Thierry,

On 21 February 2018 at 15:30, Thierry Reding <thierry.reding at gmail.com> wrote:

> @@ -2595,6 +2596,11 @@ if test -n "$with_gallium_drivers"; then
>         ximx)
>              HAVE_GALLIUM_IMX=yes
>              ;;
> +        xtegra)
> +            HAVE_GALLIUM_TEGRA=yes
> +            PKG_CHECK_MODULES([TEGRA], [libdrm_tegra >= $LIBDRM_TEGRA_REQUIRED])
> +            require_libdrm "tegra"
> +            ;;

Could use the following hunk just after the "... needed dependencies
for renderonly drivers." comment.

if test "x$HAVE_GALLIUM_NOUVEAU" != xyes -a "x$HAVE_GALLIUM_TEGRA" =
xyes  ; then
    AC_MSG_ERROR([Building with tegra requires nouveau])
fi

In a future patch we can:
- add tegra to the Makefile.am DISTCHECK list
- wire up the driver with .travis


> index 000000000000..910cbe02d873
> --- /dev/null
> +++ b/include/drm-uapi/tegra_drm.h

Haven't checked this one, but I trust you picked the latest and greatest.


> --- /dev/null
> +++ b/src/gallium/drivers/tegra/Makefile.am
> @@ -0,0 +1,18 @@
> +AUTOMAKE_OPTIONS = subdir-objects
> +
Not needed - already set at global scope.

> +include Makefile.sources
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CFLAGS = \
> +       -I$(top_srcdir)/include/drm-uapi \
> +       $(GALLIUM_DRIVER_CFLAGS) \
> +       $(LIBUDEV_CFLAGS) \
libudev is gone.

> +
> +noinst_LTLIBRARIES = libtegra.la
> +
> +libtegra_la_SOURCES = \
> +       $(C_SOURCES)
> +
> +libtegra_la_LIBADD = \
> +       $(LIBUDEV_LIBS)
Ditto.


> --- /dev/null
> +++ b/src/gallium/drivers/tegra/Makefile.sources
> @@ -0,0 +1,3 @@
> +C_SOURCES := \
> +       tegra_context.c \
> +       tegra_screen.c

Please include the following to as well.
tegra_context.h
tegra_resource.h

> index 000000000000..feaa5138c95d
> --- /dev/null
> +++ b/src/gallium/drivers/tegra/tegra_context.c

> +#include "tegra/tegra_context.h"
> +#include "tegra/tegra_resource.h"
> +#include "tegra/tegra_screen.h"
> +
The "tegra/" prefix isn't needed - it doesn't hurt though.


> --- /dev/null
> +++ b/src/gallium/drivers/tegra/tegra_screen.c

> +#include "tegra/tegra_context.h"
> +#include "tegra/tegra_resource.h"
> +#include "tegra/tegra_screen.h"
> +
Ditto.

> +static const char *
> +tegra_screen_get_name(struct pipe_screen *pscreen)
> +{
> +   return "tegra";
> +}
> +
> +static const char *
> +tegra_screen_get_vendor(struct pipe_screen *pscreen)
> +{
> +   return "NVIDIA";
> +}
> +
> +static const char *
> +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
> +{
> +   return "NVIDIA";
> +}
> +
As-is these might be a bit fiddly, but will do for now.
For example - how will devs distinguish between the closed-source
driver and Mesa.


> +static int tegra_open_render_node(void)
> +{
> +   drmDevicePtr *devices, device;
> +   int err, fd = -ENODEV;
> +   unsigned int num, i;
> +
> +   err = drmGetDevices2(0, NULL, 0);
> +   if (err < 0)
> +      return err;
> +
> +   num = err;
> +
> +   devices = calloc(num, sizeof(*devices));
> +   if (!devices)
> +      return -ENOMEM;
> +
> +   err = drmGetDevices2(0, devices, num);
> +   if (err < 0) {
> +      free(devices);
> +      return err;
> +   }
> +
> +   for (i = 0; i < num; i++) {
> +      device = devices[i];
> +
> +      if ((device->available_nodes & (1 << DRM_NODE_RENDER)) &&
> +          (device->bustype == DRM_BUS_PLATFORM)) {
Worth checking for any instances of "tegra" in the platform/deviceinfo strings?

> +         fd = open(device->nodes[DRM_NODE_RENDER], O_RDWR);
open(.... O_RDWR | O_CLOEXEC) + check for failure.



> +struct pipe_screen *
> +tegra_screen_create(int fd)
> +{
> +   struct tegra_screen *screen;
> +
> +   screen = calloc(1, sizeof(*screen));
> +   if (!screen)
> +      return NULL;
> +
> +   screen->fd = fd;
IIRC there were some fallouts for xinerama setups that required a dup().
Don't recall the details and how applicable those are here.


-Emil