[PATCH 2/2] compositor-drm: Add support for Tegra Jetson TK1
Thierry Reding
treding at nvidia.com
Sun Jun 22 23:40:01 PDT 2014
On Sat, Jun 21, 2014 at 01:37:16PM +0200, Rob Clark wrote:
> On Fri, Jun 20, 2014 at 12:12 PM, James Thomas
> <james.thomas at codethink.co.uk> wrote:
> > Update configure.ac to add check for libdrm_tegra
> > ---
> > configure.ac | 18 ++++++++++
> > src/compositor-drm.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 111 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index b4511fc..ece4a0c 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -110,6 +110,23 @@ fi
> > PKG_CHECK_MODULES(LIBDRM, [libdrm],
> > [AC_DEFINE(HAVE_LIBDRM, 1, [Define if libdrm is available]) have_libdrm=yes], have_libdrm=no)
> >
> > +AC_ARG_ENABLE(libdrm-tegra,
> > + AS_HELP_STRING([--disable-libdrm-tegra],
> > + [do not build support for nvidia Jetson TK1]),,
> > + enable_libdrm_tegra=auto)
> > +
> > +if test "x$enable_libdrm_tegra" != "xno"; then
> > + PKG_CHECK_MODULES(LIBDRM_TEGRA,
> > + libdrm_tegra,
> > + have_libdrm_tegra=yes,
> > + have_libdrm_tegra=no)
> > + if test "x$have_libdrm_tegra" = "xno" -a "x$enable_libdrm_tegra" = "xyes"; then
> > + AC_MSG_ERROR([Tegra support explicitly requested, but libdrm_tegra not found])
> > + fi
> > + AS_IF([test "x$have_libdrm_tegra" = "xyes"],
> > + AC_DEFINE([HAVE_DRM_TEGRA], [1], [Enable tegra support in drm backend]))
> > +fi
> > +
> > AC_ARG_ENABLE(x11-compositor, [ --enable-x11-compositor],,
> > enable_x11_compositor=yes)
> > AM_CONDITIONAL(ENABLE_X11_COMPOSITOR, test x$enable_x11_compositor = xyes)
> > @@ -555,4 +572,5 @@ AC_MSG_RESULT([
> > libwebp Support ${have_webp}
> > libunwind Support ${have_libunwind}
> > VA H.264 encoding Support ${have_libva}
> > + Tegra DRM Support ${have_libdrm_tegra}
> > ])
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 19d1e47..89eed71 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -42,6 +42,9 @@
> >
> > #include <gbm.h>
> > #include <libudev.h>
> > +#ifdef HAVE_DRM_TEGRA
> > +#include <tegra_drm.h>
> > +#endif
> >
> > #include "libbacklight.h"
> > #include "compositor.h"
> > @@ -211,6 +214,32 @@ static struct gl_renderer_interface *gl_renderer;
> >
> > static const char default_seat[] = "seat0";
> >
> > +static int
> > +drm_tegra_import(int fd, uint32_t handle)
> > +{
> > + #ifdef HAVE_DRM_TEGRA
> > + struct drm_tegra_gem_set_tiling args;
> > + int err;
> > +
> > + memset(&args, 0, sizeof(args));
> > + args.handle = handle;
> > + args.mode = DRM_TEGRA_GEM_TILING_MODE_BLOCK;
> > + args.value = 4;
> > +
> > + err = ioctl(fd, DRM_IOCTL_TEGRA_GEM_SET_TILING, &args);
> > + if (err < 0) {
> > + weston_log("failed to set tiling parameters: %m\n");
> > + return -errno;
> > + }
> > + return 0;
> > + #else
> > + weston_log("DRM device is a tegra but weston compiled without "
> > + "libdrm tegra");
> > +
> > + return -1;
> > + #endif
> > +}
> > +
> > static void
> > drm_output_set_cursor(struct drm_output *output);
> >
> > @@ -351,6 +380,32 @@ drm_fb_get_from_bo(struct gbm_bo *bo,
> > fb->size = fb->stride * height;
> > fb->fd = compositor->drm.fd;
> >
> > + if (compositor->drm.fd != compositor->gbm.fd) {
> > + int fd;
> > +
> > + ret = drmPrimeHandleToFD(compositor->gbm.fd, fb->handle, 0,
> > + &fd);
> > + if (ret) {
> > + weston_log("failed to export bo: %m\n");
> > + goto err_free;
> > + }
> > +
> > + ret = drmPrimeFDToHandle(compositor->drm.fd, fd, &fb->handle);
> > + if (ret) {
> > + weston_log("failed to import bo: %m\n");
> > + goto err_free;
> > + }
> > +
> > + close(fd);
> > +
> > + ret = drm_tegra_import(compositor->drm.fd, fb->handle);
> > + if (ret < 0) {
> > + weston_log("failed to import handle: %s\n",
> > + strerror(-ret));
> > + goto err_free;
> > + }
> > + }
> > +
> > if (compositor->min_width > width || width > compositor->max_width ||
> > compositor->min_height > height ||
> > height > compositor->max_height) {
> > @@ -1265,6 +1320,8 @@ on_drm_input(int fd, uint32_t mask, void *data)
> > return 1;
> > }
> >
> > +
> > +
> > static int
> > init_drm(struct drm_compositor *ec, struct udev_device *device)
> > {
> > @@ -1294,8 +1351,14 @@ init_drm(struct drm_compositor *ec, struct udev_device *device)
> > ec->drm.fd = fd;
> > ec->drm.filename = strdup(filename);
> >
> > - ec->gbm.fd = fd;
> > - ec->gbm.filename = ec->drm.filename;
> > + if (ec->gbm.filename) {
> > + fd = weston_launcher_open(ec->base.launcher, ec->gbm.filename,
> > + O_RDWR);
> > + ec->gbm.fd = fd;
> > + } else {
> > + ec->gbm.fd = fd;
> > + ec->gbm.filename = ec->drm.filename;
> > + }
> >
> > ret = drmGetCap(fd, DRM_CAP_TIMESTAMP_MONOTONIC, &cap);
> > if (ret == 0 && cap == 1)
> > @@ -2500,7 +2563,7 @@ find_primary_gpu(struct drm_compositor *ec, const char *seat)
> > struct udev_enumerate *e;
> > struct udev_list_entry *entry;
> > const char *path, *device_seat, *id;
> > - struct udev_device *device, *drm_device, *pci;
> > + struct udev_device *device, *drm_device, *pci, *soc = NULL;
> >
> > e = udev_enumerate_new(ec->udev);
> > udev_enumerate_add_match_subsystem(e, "drm");
> > @@ -2510,6 +2573,7 @@ find_primary_gpu(struct drm_compositor *ec, const char *seat)
> > drm_device = NULL;
> > udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) {
> > path = udev_list_entry_get_name(entry);
> > + weston_log("udev path = %s\n", path);
> > device = udev_device_new_from_syspath(ec->udev, path);
> > if (!device)
> > continue;
> > @@ -2531,6 +2595,32 @@ find_primary_gpu(struct drm_compositor *ec, const char *seat)
> > drm_device = device;
> > break;
> > }
> > + } else {
> > + soc = udev_device_get_parent_with_subsystem_devtype(
> > + device,
> > + "soc",
> > + NULL);
> > + if (soc) {
> > + id = udev_device_get_sysattr_value(soc,
> > + "family");
> > + if (id && !strcmp(id, "Tegra")) {
>
>
> Just one minor, perhaps not very helpful comment...
>
> This separate display vs gpu device is actually going to be the more
> common case for SoC devices. It would be a very good candidate to
> *somehow* extract this sort of logic out into some helper library so
> it can be shared between different wayland compositors. No idea if
> that should be libdrm or something new? Or perhaps we could somehow
> expose some attributes/flags from kernel or gbm to hint about what
> device to use for rendering vs display so that the compositor could
> automagically do the right thing?
>
> But we'll need the same sort of thing for etnaviv and someday lima.
> And would be nice if each different wayland compositor did not have to
> be patched every time there was a new combination of SoC (display
> controller) vs. gpu.
I had a discussion with Kristian on IRC and he suggested that this be
hidden within GBM. If I understand correctly the goal would be to add
support to GBM for Tegra K1 so that it would do the right thing and
simply open the two devices (or take two file descriptors). For etnaviv
(and others) this could be done in a similar way. The current API can't
cope with it, so I'm currently investigating how it could possibly be
changed to support this.
One of the issues is that we need two file descriptors to pass in but
gbm_create_device() only takes one. To support more than one we also
need a way to tell GBM which is the modesetting one and which is the
render node one. The other problem is that a struct gbm_bo would need to
be associated with both the display and render devices, which I think we
can easily deal with internally, but the problem arises when you want to
use its handle in an external API (such as DRM/KMS). gbm_bo_get_handle()
doesn't know which one to return. We discussed that a bit on IRC and
Kristian proposed to add a way for GBM to export a DRM framebuffer
handle.
One other idea that was brought up in an offline discussion was to wrap
this at the kernel level. I'm not sure if this could be made to work,
but the general concept would be to have a proxy DRM device that could
forward modesetting IOCTLs to one device and rendering IOCTLs to
another. That's problematic because both DRM device share the same IOCTL
number space, so there will be overlap. But I wanted to bring it up just
in case somebody had an idea to work around this.
Thierry
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140623/1d056d90/attachment.sig>
More information about the wayland-devel
mailing list