[PATCH v2 libdrm 5/7] tegra: Add helper library for tests

Erik Faye-Lund kusmabite at gmail.com
Thu Apr 10 10:33:33 PDT 2014


On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding at gmail.com> wrote:
> From: Thierry Reding <treding at nvidia.com>
>
> This library provides helpers for common functionality needed by test
> programs.
>
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
> Changes in v2:
> - fix a couple of memory leaks and get rid of some unneeded code
>
>  tests/tegra/Makefile.am      |  10 +-
>  tests/tegra/drm-test-tegra.c | 137 ++++++++++++++++++++++++
>  tests/tegra/drm-test-tegra.h |  55 ++++++++++
>  tests/tegra/drm-test.c       | 248 +++++++++++++++++++++++++++++++++++++++++++
>  tests/tegra/drm-test.h       |  72 +++++++++++++
>  5 files changed, 521 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tegra/drm-test-tegra.c
>  create mode 100644 tests/tegra/drm-test-tegra.h
>  create mode 100644 tests/tegra/drm-test.c
>  create mode 100644 tests/tegra/drm-test.h

This isn't really important at this point, but it looks to me like
tests/tegra/drm-test.[ch] isn't really tegra-specific. If so, perhaps
some other tests can benefit from it? Doing so is of course something
whoever writes those tests should do, though. Leaving them in the
tegra-subdir is probably fine.

> +int drm_tegra_gr2d_fill(struct drm_tegra_gr2d *gr2d, struct drm_framebuffer *fb,
> +                       unsigned int x, unsigned int y, unsigned int width,
> +                       unsigned int height, uint32_t color)
> +{
> +       struct drm_tegra_bo *fbo = fb->data;
> +       struct drm_tegra_pushbuf *pushbuf;
> +       struct drm_tegra_fence *fence;
> +       struct drm_tegra_job *job;
> +       int err;
> +
> +       err = drm_tegra_job_new(&job, gr2d->channel);
> +       if (err < 0)
> +               return err;
> +
> +       err = drm_tegra_pushbuf_new(&pushbuf, job);
> +       if (err < 0)
> +               return err;
> +

I think this helper would be generally more useful if it skipped the
above two, and required the call-sites to do them instead.

> +       err = drm_tegra_pushbuf_prepare(pushbuf, 32);
> +       if (err < 0)
> +               return err;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_SETCL(0, HOST1X_CLASS_GR2D, 0);
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_MASK(0x9, 0x9);
> +       *pushbuf->ptr++ = 0x0000003a;
> +       *pushbuf->ptr++ = 0x00000000;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_MASK(0x1e, 0x7);
> +       *pushbuf->ptr++ = 0x00000000;
> +       *pushbuf->ptr++ = (2 << 16) | (1 << 6) | (1 << 2);
> +       *pushbuf->ptr++ = 0x000000cc;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_MASK(0x2b, 0x9);
> +
> +       /* relocate destination buffer */
> +       err = drm_tegra_pushbuf_relocate(pushbuf, fbo, 0, 0);
> +       if (err < 0) {
> +               fprintf(stderr, "failed to relocate buffer object: %d\n", err);
> +               return err;
> +       }
> +
> +       *pushbuf->ptr++ = fb->pitch;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_NONINCR(0x35, 1);
> +       *pushbuf->ptr++ = color;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_NONINCR(0x46, 1);
> +       *pushbuf->ptr++ = 0x00000000;
> +
> +       *pushbuf->ptr++ = HOST1X_OPCODE_MASK(0x38, 0x5);
> +       *pushbuf->ptr++ = height << 16 | width;
> +       *pushbuf->ptr++ = y << 16 | x;
> +

...and stop here.

That way we can use it for tests that perform multiple fills in one submit etc.

> +#define HOST1X_OPCODE_SETCL(offset, classid, mask) \
> +    ((0x0 << 28) | (((offset) & 0xfff) << 16) | (((classid) & 0x3ff) << 6) | ((mask) & 0x3f))
> +#define HOST1X_OPCODE_INCR(offset, count) \
> +    ((0x1 << 28) | (((offset) & 0xfff) << 16) | ((count) & 0xffff))
> +#define HOST1X_OPCODE_NONINCR(offset, count) \
> +    ((0x2 << 28) | (((offset) & 0xfff) << 16) | ((count) & 0xffff))
> +#define HOST1X_OPCODE_MASK(offset, mask) \
> +    ((0x3 << 28) | (((offset) & 0xfff) << 16) | ((mask) & 0xffff))
> +#define HOST1X_OPCODE_IMM(offset, data) \
> +    ((0x4 << 28) | (((offset) & 0xfff) << 16) | ((data) & 0xffff))
> +#define HOST1X_OPCODE_EXTEND(subop, value) \
> +    ((0xe << 28) | (((subop) & 0xf) << 24) | ((value) & 0xffffff))
> +
> +#define HOST1X_CLASS_GR2D 0x51

Hmm, shouldn't these be available from somewhere else already? No
point in repeating the same macros over and over again, no?

> diff --git a/tests/tegra/drm-test.c b/tests/tegra/drm-test.c
> new file mode 100644
> index 000000000000..1f91d17f61bb
> --- /dev/null
> +++ b/tests/tegra/drm-test.c
> @@ -0,0 +1,248 @@
> +/*
> + * Copyright © 2014 NVIDIA Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#  include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
> +
> +#include "xf86drm.h"
> +#include "xf86drmMode.h"
> +#include "drm_fourcc.h"
> +
> +#include "drm-test.h"
> +
> +static int drm_screen_probe_connector(struct drm_screen *screen,
> +                                     drmModeConnectorPtr connector)
> +{
> +       drmModeEncoderPtr encoder;
> +       drmModeCrtcPtr crtc;
> +       drmModeFBPtr fb;
> +
> +       encoder = drmModeGetEncoder(screen->fd, connector->encoder_id);
> +       if (!encoder)
> +               return -ENODEV;
> +
> +       crtc = drmModeGetCrtc(screen->fd, encoder->crtc_id);
> +       if (!crtc) {
> +               drmModeFreeEncoder(encoder);
> +               return -ENODEV;
> +       }
> +
> +       screen->old_fb = crtc->buffer_id;
> +
> +       fb = drmModeGetFB(screen->fd, crtc->buffer_id);
> +       if (!fb) {
> +               /* TODO: create new framebuffer */

What's the implications of not doing what this TODO says?


More information about the dri-devel mailing list