diff mbox

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

Message ID 1397043630-13972-6-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding April 9, 2014, 11:40 a.m. UTC
From: Thierry Reding <treding@nvidia.com>

This library provides helpers for common functionality needed by test
programs.

Signed-off-by: Thierry Reding <treding@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

Comments

Erik Faye-Lund April 10, 2014, 5:33 p.m. UTC | #1
On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> This library provides helpers for common functionality needed by test
> programs.
>
> Signed-off-by: Thierry Reding <treding@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?
Thierry Reding May 2, 2014, 2:42 p.m. UTC | #2
On Thu, Apr 10, 2014 at 07:33:33PM +0200, Erik Faye-Lund wrote:
> On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > This library provides helpers for common functionality needed by test
> > programs.
> >
> > Signed-off-by: Thierry Reding <treding@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.

Daniel Vetter and I have been "discussing" this for a while now. There
are a bunch of tests in the intel-gpu-tools repository that aren't Intel
specific either and the idea is to eventually collect all those test
cases in a common location so that they can be reused by other drivers
too, but so far nobody's had time to do that yet.

> > +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.

How about we make drm_tegra_gr2d_fill() take a drm_tegra_pushbuf object
and push the commands as you suggest and then add a wrapper, say
drm_tegra_gr2d_fill_simple() for convenience when a single fill per
submit is good enough?

> > +#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?

I don't think we have these anywhere else. It seems to be custom to have
numerous redefinitions of this kind of macro throughout various drivers.
I suspect that the reason is that these can vary depending on chipset
revision and keeping a global list would require a revision prefix to be
prepended to all of them.

Even if we want them somewhere else, I wouldn't know where they'd be
best kept to be honest.

> > diff --git a/tests/tegra/drm-test.c b/tests/tegra/drm-test.c
[...]
> > +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?

It currently just means that we don't want to deal with this situation,
which I think shouldn't be happening in the first place anyway. So the
TODO is there mostly as a reminder that this could happen and that there
*might* be something more useful than returning an error that could be
done.

Thierry
Erik Faye-Lund May 2, 2014, 3:13 p.m. UTC | #3
On Fri, May 2, 2014 at 4:42 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Apr 10, 2014 at 07:33:33PM +0200, Erik Faye-Lund wrote:
>> On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > From: Thierry Reding <treding@nvidia.com>
>> >
>> > This library provides helpers for common functionality needed by test
>> > programs.
>> >
>> > Signed-off-by: Thierry Reding <treding@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.
>
> Daniel Vetter and I have been "discussing" this for a while now. There
> are a bunch of tests in the intel-gpu-tools repository that aren't Intel
> specific either and the idea is to eventually collect all those test
> cases in a common location so that they can be reused by other drivers
> too, but so far nobody's had time to do that yet.

Right. Let's just leave it for later, then.

>> > +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.
>
> How about we make drm_tegra_gr2d_fill() take a drm_tegra_pushbuf object
> and push the commands as you suggest and then add a wrapper, say
> drm_tegra_gr2d_fill_simple() for convenience when a single fill per
> submit is good enough?

I wouldn't be against such a convenience-wrapper, no. I would
personally leave that for the person who added more such tests,
though. I guess that could be said about my suggestion as well ;-)

>> > +#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?
>
> I don't think we have these anywhere else. It seems to be custom to have
> numerous redefinitions of this kind of macro throughout various drivers.
> I suspect that the reason is that these can vary depending on chipset
> revision and keeping a global list would require a revision prefix to be
> prepended to all of them.
>
> Even if we want them somewhere else, I wouldn't know where they'd be
> best kept to be honest.

OK. That's a bit of a shame though, as it means "everyone"
(xf86-video-opentegra and mesa + grate tests) must carry these
definitions. But it's not important, and can be fixed later if it
turns out to be a nuisance.

>> > diff --git a/tests/tegra/drm-test.c b/tests/tegra/drm-test.c
> [...]
>> > +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?
>
> It currently just means that we don't want to deal with this situation,
> which I think shouldn't be happening in the first place anyway. So the
> TODO is there mostly as a reminder that this could happen and that there
> *might* be something more useful than returning an error that could be
> done.

Right. Perhaps it could happen if no monitor is connected? That's
probably not important, though.
Erik Faye-Lund May 2, 2014, 3:18 p.m. UTC | #4
On Wed, Apr 9, 2014 at 1:40 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> +int drm_open(const char *path)
> +{
> +       int fd, err;
> +
> +       fd = open(path, O_RDWR);
> +       if (fd < 0)
> +               return -errno;
> +
> +       err = drmSetMaster(fd);
> +       if (err < 0) {
> +               close(fd);
> +               return -errno;
> +       }

Could we make this opt-in? Otherwise "make check" needs to be run as
root to pass, which is a bit iffy IMO as it also invokes the compiler
and all tools as root. I'd prefer not having to give my dev-tools (and
potentially buggy in-tree scripts) that much privilege...
diff mbox

Patch

diff --git a/tests/tegra/Makefile.am b/tests/tegra/Makefile.am
index 8b481bde4f11..e468029d152e 100644
--- a/tests/tegra/Makefile.am
+++ b/tests/tegra/Makefile.am
@@ -5,9 +5,17 @@  AM_CPPFLAGS = \
 
 AM_CFLAGS = -Wall -Werror
 
+noinst_LTLIBRARIES = libdrm-test.la
+libdrm_test_la_SOURCES = \
+	drm-test.c \
+	drm-test.h \
+	drm-test-tegra.c \
+	drm-test-tegra.h
+
 LDADD = \
 	../../tegra/libdrm_tegra.la \
-	../../libdrm.la
+	../../libdrm.la \
+	libdrm-test.la
 
 TESTS = \
 	openclose \
diff --git a/tests/tegra/drm-test-tegra.c b/tests/tegra/drm-test-tegra.c
new file mode 100644
index 000000000000..aed353af739b
--- /dev/null
+++ b/tests/tegra/drm-test-tegra.c
@@ -0,0 +1,137 @@ 
+/*
+ * 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 <stdio.h>
+
+#include "drm-test-tegra.h"
+#include "tegra.h"
+
+int drm_tegra_gr2d_open(struct drm_tegra_gr2d **gr2dp, struct drm_tegra *drm)
+{
+	struct drm_tegra_gr2d *gr2d;
+	int err;
+
+	gr2d = calloc(1, sizeof(*gr2d));
+	if (!gr2d)
+		return -ENOMEM;
+
+	gr2d->drm = drm;
+
+	err = drm_tegra_channel_open(&gr2d->channel, drm, DRM_TEGRA_GR2D);
+	if (err < 0) {
+		free(gr2d);
+		return err;
+	}
+
+	*gr2dp = gr2d;
+
+	return 0;
+}
+
+int drm_tegra_gr2d_close(struct drm_tegra_gr2d *gr2d)
+{
+	if (!gr2d)
+		return -EINVAL;
+
+	drm_tegra_channel_close(gr2d->channel);
+	free(gr2d);
+
+	return 0;
+}
+
+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;
+
+	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;
+
+	err = drm_tegra_job_submit(job, &fence);
+	if (err < 0) {
+		fprintf(stderr, "failed to submit job: %d\n", err);
+		return err;
+	}
+
+	err = drm_tegra_fence_wait(fence);
+	if (err < 0) {
+		fprintf(stderr, "failed to wait for fence: %d\n", err);
+		return err;
+	}
+
+	drm_tegra_fence_free(fence);
+	drm_tegra_pushbuf_free(pushbuf);
+	drm_tegra_job_free(job);
+
+	return 0;
+}
diff --git a/tests/tegra/drm-test-tegra.h b/tests/tegra/drm-test-tegra.h
new file mode 100644
index 000000000000..d1cb6b1ee440
--- /dev/null
+++ b/tests/tegra/drm-test-tegra.h
@@ -0,0 +1,55 @@ 
+/*
+ * 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.
+ */
+
+#ifndef TEGRA_DRM_TEST_TEGRA_H
+#define TEGRA_DRM_TEST_TEGRA_H
+
+#include "drm-test.h"
+#include "tegra.h"
+
+#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
+
+struct drm_tegra_gr2d {
+	struct drm_tegra *drm;
+	struct drm_tegra_channel *channel;
+};
+
+int drm_tegra_gr2d_open(struct drm_tegra_gr2d **gr2dp, struct drm_tegra *drm);
+int drm_tegra_gr2d_close(struct drm_tegra_gr2d *gr2d);
+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);
+
+#endif
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 */
+		drmModeFreeEncoder(encoder);
+		drmModeFreeCrtc(crtc);
+		return -ENOSYS;
+	}
+
+	screen->connector = connector->connector_id;
+	screen->old_fb = crtc->buffer_id;
+	screen->crtc = encoder->crtc_id;
+	/* TODO: check crtc->mode_valid */
+	screen->mode = crtc->mode;
+
+	screen->width = fb->width;
+	screen->height = fb->height;
+	screen->pitch = fb->pitch;
+	screen->depth = fb->depth;
+	screen->bpp = fb->bpp;
+
+	drmModeFreeEncoder(encoder);
+	drmModeFreeCrtc(crtc);
+	drmModeFreeFB(fb);
+
+	return 0;
+}
+
+int drm_screen_open(struct drm_screen **screenp, int fd)
+{
+	drmModeConnectorPtr connector;
+	struct drm_screen *screen;
+	bool found = false;
+	drmModeResPtr res;
+	unsigned int i;
+	int err;
+
+	if (!screenp || fd < 0)
+		return -EINVAL;
+
+	screen = calloc(1, sizeof(*screen));
+	if (!screen)
+		return -ENOMEM;
+
+	screen->format = DRM_FORMAT_XRGB8888;
+	screen->fd = fd;
+
+	res = drmModeGetResources(fd);
+	if (!res) {
+		free(screen);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < res->count_connectors; i++) {
+		connector = drmModeGetConnector(fd, res->connectors[i]);
+		if (!connector)
+			continue;
+
+		if (connector->connection != DRM_MODE_CONNECTED) {
+			drmModeFreeConnector(connector);
+			continue;
+		}
+
+		err = drm_screen_probe_connector(screen, connector);
+		if (err < 0) {
+			drmModeFreeConnector(connector);
+			continue;
+		}
+
+		drmModeFreeConnector(connector);
+		found = true;
+		break;
+	}
+
+	drmModeFreeResources(res);
+
+	if (!found) {
+		free(screen);
+		return -ENODEV;
+	}
+
+	*screenp = screen;
+
+	return 0;
+}
+
+int drm_screen_close(struct drm_screen *screen)
+{
+	int err;
+
+	err = drmModeSetCrtc(screen->fd, screen->crtc, screen->old_fb, 0, 0,
+			     &screen->connector, 1, &screen->mode);
+	if (err < 0) {
+		fprintf(stderr, "drmModeSetCrtc() failed: %m\n");
+		return -errno;
+	}
+
+	free(screen);
+
+	return 0;
+}
+
+int drm_framebuffer_new(struct drm_framebuffer **fbp,
+			struct drm_screen *screen, uint32_t handle,
+			unsigned int width, unsigned int height,
+			unsigned int pitch, uint32_t format,
+			void *data)
+{
+	struct drm_framebuffer *fb;
+	uint32_t handles[4];
+	uint32_t pitches[4];
+	uint32_t offsets[4];
+	int err;
+
+	fb = calloc(1, sizeof(*fb));
+	if (!fb)
+		return -ENOMEM;
+
+	fb->fd = screen->fd;
+	fb->width = width;
+	fb->height = height;
+	fb->pitch = pitch;
+	fb->format = format;
+	fb->data = data;
+
+	handles[0] = handle;
+	pitches[0] = pitch;
+	offsets[0] = 0;
+
+	err = drmModeAddFB2(screen->fd, width, height, format, handles,
+			    pitches, offsets, &fb->handle, 0);
+	if (err < 0)
+		return -errno;
+
+	*fbp = fb;
+
+	return 0;
+}
+
+int drm_framebuffer_free(struct drm_framebuffer *fb)
+{
+	int err;
+
+	err = drmModeRmFB(fb->fd, fb->handle);
+	if (err < 0)
+		return -errno;
+
+	free(fb);
+
+	return 0;
+}
+
+int drm_screen_set_framebuffer(struct drm_screen *screen,
+			       struct drm_framebuffer *fb)
+{
+	int err;
+
+	err = drmModeSetCrtc(screen->fd, screen->crtc, fb->handle, 0, 0,
+			     &screen->connector, 1, &screen->mode);
+	if (err < 0)
+		return -errno;
+
+	return 0;
+}
+
+int drm_open(const char *path)
+{
+	int fd, err;
+
+	fd = open(path, O_RDWR);
+	if (fd < 0)
+		return -errno;
+
+	err = drmSetMaster(fd);
+	if (err < 0) {
+		close(fd);
+		return -errno;
+	}
+
+	return fd;
+}
+
+void drm_close(int fd)
+{
+	drmDropMaster(fd);
+	close(fd);
+}
diff --git a/tests/tegra/drm-test.h b/tests/tegra/drm-test.h
new file mode 100644
index 000000000000..0f3d1ec26e2b
--- /dev/null
+++ b/tests/tegra/drm-test.h
@@ -0,0 +1,72 @@ 
+/*
+ * 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.
+ */
+
+#ifndef TEGRA_DRM_TEST_H
+#define TEGRA_DRM_TEST_H
+
+#include <stdint.h>
+#include <stdlib.h>
+
+#include "xf86drmMode.h"
+
+struct drm_screen {
+	int fd;
+
+	unsigned int width;
+	unsigned int height;
+	unsigned int pitch;
+	unsigned int depth;
+	unsigned int bpp;
+
+	drmModeModeInfo mode;
+	uint32_t connector;
+	uint32_t old_fb;
+	uint32_t format;
+	uint32_t crtc;
+};
+
+struct drm_framebuffer {
+	unsigned int width;
+	unsigned int height;
+	unsigned int pitch;
+	uint32_t format;
+	uint32_t handle;
+	void *data;
+	int fd;
+};
+
+int drm_screen_open(struct drm_screen **screenp, int fd);
+int drm_screen_close(struct drm_screen *screen);
+int drm_screen_set_framebuffer(struct drm_screen *screen,
+			       struct drm_framebuffer *fb);
+
+int drm_framebuffer_new(struct drm_framebuffer **fbp,
+			struct drm_screen *screen, uint32_t handle,
+			unsigned int width, unsigned int height,
+			unsigned int pitch, uint32_t format,
+			void *data);
+int drm_framebuffer_free(struct drm_framebuffer *fb);
+
+int drm_open(const char *path);
+void drm_close(int fd);
+
+#endif