| Message ID | 1442937302-8211-7-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 22 Sep 2015 17:54:55 +0200 Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > This tests async processing of G2D jobs. A separate thread is spawned > to monitor the DRM fd for events and check whether a G2D job was > completed. > > v2: Add GPLv2 header, argument handling and documentation. > Test is only installed when requested. > v3: Allocate G2D jobs with calloc which fixes 'busy' being > potentially uninitialized. Also enable timeout for poll() > in the monitor thread. This fixes pthread_join() not working > because of poll() not returning. > > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > tests/exynos/Makefile.am | 11 +- > tests/exynos/exynos_fimg2d_event.c | 326 > +++++++++++++++++++++++++++++++++++++ 2 files changed, 335 > insertions(+), 2 deletions(-) create mode 100644 > tests/exynos/exynos_fimg2d_event.c > > diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am > index e82d199..357d6b8 100644 > --- a/tests/exynos/Makefile.am > +++ b/tests/exynos/Makefile.am > @@ -20,16 +20,23 @@ endif > > if HAVE_INSTALL_TESTS > bin_PROGRAMS += \ > - exynos_fimg2d_perf > + exynos_fimg2d_perf \ > + exynos_fimg2d_event > else > noinst_PROGRAMS += \ > - exynos_fimg2d_perf > + exynos_fimg2d_perf \ > + exynos_fimg2d_event > endif > > exynos_fimg2d_perf_LDADD = \ > $(top_builddir)/libdrm.la \ > $(top_builddir)/exynos/libdrm_exynos.la > > +exynos_fimg2d_event_LDADD = \ > + $(top_builddir)/libdrm.la \ > + $(top_builddir)/exynos/libdrm_exynos.la \ > + -lpthread > + > exynos_fimg2d_test_LDADD = \ > $(top_builddir)/libdrm.la \ > $(top_builddir)/libkms/libkms.la \ > diff --git a/tests/exynos/exynos_fimg2d_event.c > b/tests/exynos/exynos_fimg2d_event.c new file mode 100644 > index 0000000..c03dcff > --- /dev/null > +++ b/tests/exynos/exynos_fimg2d_event.c > @@ -0,0 +1,326 @@ > +/* > + * Copyright (C) 2015 - Tobias Jakobi > + * > + * This is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published > + * by the Free Software Foundation, either version 2 of the License, > + * or (at your option) any later version. > + * > + * It is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * You should have received a copy of the GNU General Public License > + * along with it. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <unistd.h> > +#include <poll.h> > + > +#include <stdlib.h> > +#include <stdio.h> > +#include <time.h> > +#include <getopt.h> > + > +#include <pthread.h> > + > +#include <xf86drm.h> > + > +#include "exynos_drm.h" > +#include "exynos_drmif.h" > +#include "exynos_fimg2d.h" > + > +struct g2d_job { > + unsigned int id; > + unsigned int busy; > +}; > + > +struct exynos_evhandler { > + struct pollfd fds; > + struct exynos_event_context evctx; > +}; > + > +struct threaddata { > + unsigned int stop; > + struct exynos_device *dev; > + struct exynos_evhandler evhandler; > +}; > + > +static void g2d_event_handler(int fd, unsigned int cmdlist_no, > unsigned int tv_sec, > + unsigned int > tv_usec, void *user_data) +{ > + struct g2d_job *job = user_data; > + > + fprintf(stderr, "info: g2d job (id = %u, cmdlist number = > %u) finished!\n", > + job->id, cmdlist_no); > + > + job->busy = 0; > +} > + > +static void setup_g2d_event_handler(struct exynos_evhandler > *evhandler, int fd) +{ > + evhandler->fds.fd = fd; > + evhandler->fds.events = POLLIN; > + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; > + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the versions are bumped, the event will contains wrong version info. Also, I think the type of event must be set here. Best regards, Hyungwon Hwang > + evhandler->evctx.g2d_event_handler = g2d_event_handler; > +} > + > +static void* threadfunc(void *arg) { > + const int timeout = 0; > + struct threaddata *data; > + > + data = arg; > + > + while (1) { > + if (data->stop) break; > + > + usleep(500); > + > + data->evhandler.fds.revents = 0; > + > + if (poll(&data->evhandler.fds, 1, timeout) < 0) > + continue; > + > + if (data->evhandler.fds.revents & (POLLHUP | > POLLERR)) > + continue; > + > + if (data->evhandler.fds.revents & POLLIN) > + exynos_handle_event(data->dev, > &data->evhandler.evctx); > + } > + > + pthread_exit(0); > +} > + > +/* > + * We need to wait until all G2D jobs are finished, otherwise we > + * potentially remove a BO which the engine still operates on. > + * This results in the following kernel message: > + * [drm:exynos_drm_gem_put_dma_addr] *ERROR* failed to lookup gem > object. > + * Also any subsequent BO allocations fail then with: > + * [drm:exynos_drm_alloc_buf] *ERROR* failed to allocate buffer. > + */ > +static void wait_all_jobs(struct g2d_job* jobs, unsigned num_jobs) > +{ > + unsigned i; > + > + for (i = 0; i < num_jobs; ++i) { > + while (jobs[i].busy) > + usleep(500); > + } > + > +} > + > +static struct g2d_job* free_job(struct g2d_job* jobs, unsigned > num_jobs) +{ > + unsigned i; > + > + for (i = 0; i < num_jobs; ++i) { > + if (jobs[i].busy == 0) > + return &jobs[i]; > + } > + > + return NULL; > +} > + > +static int g2d_work(struct g2d_context *ctx, struct g2d_image *img, > + unsigned num_jobs, unsigned > iterations) +{ > + struct g2d_job *jobs = calloc(num_jobs, sizeof(struct > g2d_job)); > + int ret; > + unsigned i; > + > + /* setup job ids */ > + for (i = 0; i < num_jobs; ++i) > + jobs[i].id = i; > + > + for (i = 0; i < iterations; ++i) { > + unsigned x, y, w, h; > + > + struct g2d_job *j = NULL; > + > + while (1) { > + j = free_job(jobs, num_jobs); > + > + if (j) > + break; > + else > + usleep(500); > + } > + > + x = rand() % img->width; > + y = rand() % img->height; > + > + if (x == (img->width - 1)) > + x -= 1; > + if (y == (img->height - 1)) > + y -= 1; > + > + w = rand() % (img->width - x); > + h = rand() % (img->height - y); > + > + if (w == 0) w = 1; > + if (h == 0) h = 1; > + > + img->color = rand(); > + > + j->busy = 1; > + g2d_config_event(ctx, j); > + > + ret = g2d_solid_fill(ctx, img, x, y, w, h); > + > + if (ret == 0) > + g2d_exec2(ctx, G2D_EXEC_FLAG_ASYNC); > + > + if (ret != 0) { > + fprintf(stderr, "error: iteration %u (x = > %u, x = %u, x = %u, x = %u) failed\n", > + i, x, y, w, h); > + break; > + } > + } > + > + wait_all_jobs(jobs, num_jobs); > + free(jobs); > + > + return 0; > +} > + > +static void usage(const char *name) > +{ > + fprintf(stderr, "usage: %s [-ijwh]\n\n", name); > + > + fprintf(stderr, "\t-i <number of iterations>\n"); > + fprintf(stderr, "\t-j <number of G2D jobs> (default = > 4)\n\n"); + > + fprintf(stderr, "\t-w <buffer width> (default = 4096)\n"); > + fprintf(stderr, "\t-h <buffer height> (default = 4096)\n"); > + > + exit(0); > +} > + > +int main(int argc, char **argv) > +{ > + int fd, ret, c, parsefail; > + > + pthread_t event_thread; > + struct threaddata event_data = {0}; > + > + struct exynos_device *dev; > + struct g2d_context *ctx; > + struct exynos_bo *bo; > + > + struct g2d_image img = {0}; > + > + unsigned int iters = 0, njobs = 4; > + unsigned int bufw = 4096, bufh = 4096; > + > + ret = 0; > + parsefail = 0; > + > + while ((c = getopt(argc, argv, "i:j:w:h:")) != -1) { > + switch (c) { > + case 'i': > + if (sscanf(optarg, "%u", &iters) != 1) > + parsefail = 1; > + break; > + case 'j': > + if (sscanf(optarg, "%u", &njobs) != 1) > + parsefail = 1; > + break; > + case 'w': > + if (sscanf(optarg, "%u", &bufw) != 1) > + parsefail = 1; > + break; > + case 'h': > + if (sscanf(optarg, "%u", &bufh) != 1) > + parsefail = 1; > + break; > + default: > + parsefail = 1; > + break; > + } > + } > + > + if (parsefail || (argc == 1) || (iters == 0)) > + usage(argv[0]); > + > + if (bufw > 4096 || bufh > 4096) { > + fprintf(stderr, "error: buffer width/height should > be less than 4096.\n"); > + ret = -1; > + > + goto out; > + } > + > + if (bufw == 0 || bufh == 0) { > + fprintf(stderr, "error: buffer width/height should > be non-zero.\n"); > + ret = -1; > + > + goto out; > + } > + > + fd = drmOpen("exynos", NULL); > + if (fd < 0) { > + fprintf(stderr, "error: failed to open drm\n"); > + ret = -1; > + > + goto out; > + } > + > + dev = exynos_device_create(fd); > + if (dev == NULL) { > + fprintf(stderr, "error: failed to create device\n"); > + ret = -2; > + > + goto fail; > + } > + > + ctx = g2d_init(fd); > + if (ctx == NULL) { > + fprintf(stderr, "error: failed to init G2D\n"); > + ret = -3; > + > + goto g2d_fail; > + } > + > + bo = exynos_bo_create(dev, bufw * bufh * 4, 0); > + if (bo == NULL) { > + fprintf(stderr, "error: failed to create bo\n"); > + ret = -4; > + > + goto bo_fail; > + } > + > + /* setup g2d image object */ > + img.width = bufw; > + img.height = bufh; > + img.stride = bufw * 4; > + img.color_mode = G2D_COLOR_FMT_ARGB8888 | G2D_ORDER_AXRGB; > + img.buf_type = G2D_IMGBUF_GEM; > + img.bo[0] = bo->handle; > + > + event_data.dev = dev; > + setup_g2d_event_handler(&event_data.evhandler, fd); > + > + pthread_create(&event_thread, NULL, threadfunc, &event_data); > + > + ret = g2d_work(ctx, &img, njobs, iters); > + if (ret != 0) > + fprintf(stderr, "error: g2d_work failed\n"); > + > + event_data.stop = 1; > + pthread_join(event_thread, NULL); > + > + exynos_bo_destroy(bo); > + > +bo_fail: > + g2d_fini(ctx); > + > +g2d_fail: > + exynos_device_destroy(dev); > + > +fail: > + drmClose(fd); > + > +out: > + return ret; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Hyungwon, first of all thanks for reviewing the series! Hyungwon Hwang wrote: > On Tue, 22 Sep 2015 17:54:55 +0200 > Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > >> This tests async processing of G2D jobs. A separate thread is spawned >> to monitor the DRM fd for events and check whether a G2D job was >> completed. >> >> v2: Add GPLv2 header, argument handling and documentation. >> Test is only installed when requested. >> v3: Allocate G2D jobs with calloc which fixes 'busy' being >> potentially uninitialized. Also enable timeout for poll() >> in the monitor thread. This fixes pthread_join() not working >> because of poll() not returning. >> >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >> --- >> tests/exynos/Makefile.am | 11 +- >> tests/exynos/exynos_fimg2d_event.c | 326 >> +++++++++++++++++++++++++++++++++++++ 2 files changed, 335 >> insertions(+), 2 deletions(-) create mode 100644 >> tests/exynos/exynos_fimg2d_event.c >> >> diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am >> index e82d199..357d6b8 100644 >> --- a/tests/exynos/Makefile.am >> +++ b/tests/exynos/Makefile.am >> @@ -20,16 +20,23 @@ endif >> >> if HAVE_INSTALL_TESTS >> bin_PROGRAMS += \ >> - exynos_fimg2d_perf >> + exynos_fimg2d_perf \ >> + exynos_fimg2d_event >> else >> noinst_PROGRAMS += \ >> - exynos_fimg2d_perf >> + exynos_fimg2d_perf \ >> + exynos_fimg2d_event >> endif >> >> exynos_fimg2d_perf_LDADD = \ >> $(top_builddir)/libdrm.la \ >> $(top_builddir)/exynos/libdrm_exynos.la >> >> +exynos_fimg2d_event_LDADD = \ >> + $(top_builddir)/libdrm.la \ >> + $(top_builddir)/exynos/libdrm_exynos.la \ >> + -lpthread >> + >> exynos_fimg2d_test_LDADD = \ >> $(top_builddir)/libdrm.la \ >> $(top_builddir)/libkms/libkms.la \ >> diff --git a/tests/exynos/exynos_fimg2d_event.c >> b/tests/exynos/exynos_fimg2d_event.c new file mode 100644 >> index 0000000..c03dcff >> --- /dev/null >> +++ b/tests/exynos/exynos_fimg2d_event.c >> @@ -0,0 +1,326 @@ >> +/* >> + * Copyright (C) 2015 - Tobias Jakobi >> + * >> + * This is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published >> + * by the Free Software Foundation, either version 2 of the License, >> + * or (at your option) any later version. >> + * >> + * It is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * You should have received a copy of the GNU General Public License >> + * along with it. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <unistd.h> >> +#include <poll.h> >> + >> +#include <stdlib.h> >> +#include <stdio.h> >> +#include <time.h> >> +#include <getopt.h> >> + >> +#include <pthread.h> >> + >> +#include <xf86drm.h> >> + >> +#include "exynos_drm.h" >> +#include "exynos_drmif.h" >> +#include "exynos_fimg2d.h" >> + >> +struct g2d_job { >> + unsigned int id; >> + unsigned int busy; >> +}; >> + >> +struct exynos_evhandler { >> + struct pollfd fds; >> + struct exynos_event_context evctx; >> +}; >> + >> +struct threaddata { >> + unsigned int stop; >> + struct exynos_device *dev; >> + struct exynos_evhandler evhandler; >> +}; >> + >> +static void g2d_event_handler(int fd, unsigned int cmdlist_no, >> unsigned int tv_sec, >> + unsigned int >> tv_usec, void *user_data) +{ >> + struct g2d_job *job = user_data; >> + >> + fprintf(stderr, "info: g2d job (id = %u, cmdlist number = >> %u) finished!\n", >> + job->id, cmdlist_no); >> + >> + job->busy = 0; >> +} >> + >> +static void setup_g2d_event_handler(struct exynos_evhandler >> *evhandler, int fd) +{ >> + evhandler->fds.fd = fd; >> + evhandler->fds.events = POLLIN; >> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; >> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; > > The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the > versions are bumped, the event will contains wrong version info. Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If the version in the public header is bumped, then it's also bumped here. So I don't see the issue. > Also, I think the type of event must be set here. What do you mean by 'type of event' here? I don't see a type field in the event context structures. With best wishes, Tobias > > Best regards, > Hyungwon Hwang > >> + evhandler->evctx.g2d_event_handler = g2d_event_handler; >> +} >> + >> +static void* threadfunc(void *arg) { >> + const int timeout = 0; >> + struct threaddata *data; >> + >> + data = arg; >> + >> + while (1) { >> + if (data->stop) break; >> + >> + usleep(500); >> + >> + data->evhandler.fds.revents = 0; >> + >> + if (poll(&data->evhandler.fds, 1, timeout) < 0) >> + continue; >> + >> + if (data->evhandler.fds.revents & (POLLHUP | >> POLLERR)) >> + continue; >> + >> + if (data->evhandler.fds.revents & POLLIN) >> + exynos_handle_event(data->dev, >> &data->evhandler.evctx); >> + } >> + >> + pthread_exit(0); >> +} >> + >> +/* >> + * We need to wait until all G2D jobs are finished, otherwise we >> + * potentially remove a BO which the engine still operates on. >> + * This results in the following kernel message: >> + * [drm:exynos_drm_gem_put_dma_addr] *ERROR* failed to lookup gem >> object. >> + * Also any subsequent BO allocations fail then with: >> + * [drm:exynos_drm_alloc_buf] *ERROR* failed to allocate buffer. >> + */ >> +static void wait_all_jobs(struct g2d_job* jobs, unsigned num_jobs) >> +{ >> + unsigned i; >> + >> + for (i = 0; i < num_jobs; ++i) { >> + while (jobs[i].busy) >> + usleep(500); >> + } >> + >> +} >> + >> +static struct g2d_job* free_job(struct g2d_job* jobs, unsigned >> num_jobs) +{ >> + unsigned i; >> + >> + for (i = 0; i < num_jobs; ++i) { >> + if (jobs[i].busy == 0) >> + return &jobs[i]; >> + } >> + >> + return NULL; >> +} >> + >> +static int g2d_work(struct g2d_context *ctx, struct g2d_image *img, >> + unsigned num_jobs, unsigned >> iterations) +{ >> + struct g2d_job *jobs = calloc(num_jobs, sizeof(struct >> g2d_job)); >> + int ret; >> + unsigned i; >> + >> + /* setup job ids */ >> + for (i = 0; i < num_jobs; ++i) >> + jobs[i].id = i; >> + >> + for (i = 0; i < iterations; ++i) { >> + unsigned x, y, w, h; >> + >> + struct g2d_job *j = NULL; >> + >> + while (1) { >> + j = free_job(jobs, num_jobs); >> + >> + if (j) >> + break; >> + else >> + usleep(500); >> + } >> + >> + x = rand() % img->width; >> + y = rand() % img->height; >> + >> + if (x == (img->width - 1)) >> + x -= 1; >> + if (y == (img->height - 1)) >> + y -= 1; >> + >> + w = rand() % (img->width - x); >> + h = rand() % (img->height - y); >> + >> + if (w == 0) w = 1; >> + if (h == 0) h = 1; >> + >> + img->color = rand(); >> + >> + j->busy = 1; >> + g2d_config_event(ctx, j); >> + >> + ret = g2d_solid_fill(ctx, img, x, y, w, h); >> + >> + if (ret == 0) >> + g2d_exec2(ctx, G2D_EXEC_FLAG_ASYNC); >> + >> + if (ret != 0) { >> + fprintf(stderr, "error: iteration %u (x = >> %u, x = %u, x = %u, x = %u) failed\n", >> + i, x, y, w, h); >> + break; >> + } >> + } >> + >> + wait_all_jobs(jobs, num_jobs); >> + free(jobs); >> + >> + return 0; >> +} >> + >> +static void usage(const char *name) >> +{ >> + fprintf(stderr, "usage: %s [-ijwh]\n\n", name); >> + >> + fprintf(stderr, "\t-i <number of iterations>\n"); >> + fprintf(stderr, "\t-j <number of G2D jobs> (default = >> 4)\n\n"); + >> + fprintf(stderr, "\t-w <buffer width> (default = 4096)\n"); >> + fprintf(stderr, "\t-h <buffer height> (default = 4096)\n"); >> + >> + exit(0); >> +} >> + >> +int main(int argc, char **argv) >> +{ >> + int fd, ret, c, parsefail; >> + >> + pthread_t event_thread; >> + struct threaddata event_data = {0}; >> + >> + struct exynos_device *dev; >> + struct g2d_context *ctx; >> + struct exynos_bo *bo; >> + >> + struct g2d_image img = {0}; >> + >> + unsigned int iters = 0, njobs = 4; >> + unsigned int bufw = 4096, bufh = 4096; >> + >> + ret = 0; >> + parsefail = 0; >> + >> + while ((c = getopt(argc, argv, "i:j:w:h:")) != -1) { >> + switch (c) { >> + case 'i': >> + if (sscanf(optarg, "%u", &iters) != 1) >> + parsefail = 1; >> + break; >> + case 'j': >> + if (sscanf(optarg, "%u", &njobs) != 1) >> + parsefail = 1; >> + break; >> + case 'w': >> + if (sscanf(optarg, "%u", &bufw) != 1) >> + parsefail = 1; >> + break; >> + case 'h': >> + if (sscanf(optarg, "%u", &bufh) != 1) >> + parsefail = 1; >> + break; >> + default: >> + parsefail = 1; >> + break; >> + } >> + } >> + >> + if (parsefail || (argc == 1) || (iters == 0)) >> + usage(argv[0]); >> + >> + if (bufw > 4096 || bufh > 4096) { >> + fprintf(stderr, "error: buffer width/height should >> be less than 4096.\n"); >> + ret = -1; >> + >> + goto out; >> + } >> + >> + if (bufw == 0 || bufh == 0) { >> + fprintf(stderr, "error: buffer width/height should >> be non-zero.\n"); >> + ret = -1; >> + >> + goto out; >> + } >> + >> + fd = drmOpen("exynos", NULL); >> + if (fd < 0) { >> + fprintf(stderr, "error: failed to open drm\n"); >> + ret = -1; >> + >> + goto out; >> + } >> + >> + dev = exynos_device_create(fd); >> + if (dev == NULL) { >> + fprintf(stderr, "error: failed to create device\n"); >> + ret = -2; >> + >> + goto fail; >> + } >> + >> + ctx = g2d_init(fd); >> + if (ctx == NULL) { >> + fprintf(stderr, "error: failed to init G2D\n"); >> + ret = -3; >> + >> + goto g2d_fail; >> + } >> + >> + bo = exynos_bo_create(dev, bufw * bufh * 4, 0); >> + if (bo == NULL) { >> + fprintf(stderr, "error: failed to create bo\n"); >> + ret = -4; >> + >> + goto bo_fail; >> + } >> + >> + /* setup g2d image object */ >> + img.width = bufw; >> + img.height = bufh; >> + img.stride = bufw * 4; >> + img.color_mode = G2D_COLOR_FMT_ARGB8888 | G2D_ORDER_AXRGB; >> + img.buf_type = G2D_IMGBUF_GEM; >> + img.bo[0] = bo->handle; >> + >> + event_data.dev = dev; >> + setup_g2d_event_handler(&event_data.evhandler, fd); >> + >> + pthread_create(&event_thread, NULL, threadfunc, &event_data); >> + >> + ret = g2d_work(ctx, &img, njobs, iters); >> + if (ret != 0) >> + fprintf(stderr, "error: g2d_work failed\n"); >> + >> + event_data.stop = 1; >> + pthread_join(event_thread, NULL); >> + >> + exynos_bo_destroy(bo); >> + >> +bo_fail: >> + g2d_fini(ctx); >> + >> +g2d_fail: >> + exynos_device_destroy(dev); >> + >> +fail: >> + drmClose(fd); >> + >> +out: >> + return ret; >> +} > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 October 2015 at 11:16, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > Hello Hyungwon, > > first of all thanks for reviewing the series! > > > > Hyungwon Hwang wrote: >> On Tue, 22 Sep 2015 17:54:55 +0200 >> Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: >> >>> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; >>> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; >> >> The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the >> versions are bumped, the event will contains wrong version info. > Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and > EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If the > version in the public header is bumped, then it's also bumped here. So I > don't see the issue. > The issue is that the public header defines the interface available, while you set the version that you implement. Currently those match, but if/when we expand the API (bumping the version in the header) and rebuild your program we will crash. Before you ask - yes current libdrm users are not doing the right thing and should be updated one of these days. -Emil P.S. Having some deja-vu here... I thought I mentioned this a while back. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Emil, Emil Velikov wrote: > On 30 October 2015 at 11:16, Tobias Jakobi > <tjakobi@math.uni-bielefeld.de> wrote: >> Hello Hyungwon, >> >> first of all thanks for reviewing the series! >> >> >> >> Hyungwon Hwang wrote: >>> On Tue, 22 Sep 2015 17:54:55 +0200 >>> Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: >>> > >>>> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; >>>> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; >>> >>> The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the >>> versions are bumped, the event will contains wrong version info. >> Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and >> EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If the >> version in the public header is bumped, then it's also bumped here. So I >> don't see the issue. >> > The issue is that the public header defines the interface available, > while you set the version that you implement. Currently those match, > but if/when we expand the API (bumping the version in the header) and > rebuild your program we will crash. Hmm, I'm still not sure I understand this. Do you mean rebuilding the tests out-of-tree and then running them against a newer/older libdrm version? Because from my understanding the tests are always build together with libdrm anyway. Or am I misunderstanding here something? > Before you ask - yes current libdrm users are not doing the right > thing and should be updated one of these days. Maybe a dumb question, but what would be the right thing to do in may case. Define my own MY_XZY_EVENT_CONTEXT_VERSION and use these? With best wishes, Tobias > -Emil > P.S. Having some deja-vu here... I thought I mentioned this a while back. > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 October 2015 at 11:28, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > Hello Emil, > > > Emil Velikov wrote: >> On 30 October 2015 at 11:16, Tobias Jakobi >> <tjakobi@math.uni-bielefeld.de> wrote: >>> Hello Hyungwon, >>> >>> first of all thanks for reviewing the series! >>> >>> >>> >>> Hyungwon Hwang wrote: >>>> On Tue, 22 Sep 2015 17:54:55 +0200 >>>> Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: >>>> >> >>>>> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; >>>>> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; >>>> >>>> The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the >>>> versions are bumped, the event will contains wrong version info. >>> Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and >>> EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If the >>> version in the public header is bumped, then it's also bumped here. So I >>> don't see the issue. >>> >> The issue is that the public header defines the interface available, >> while you set the version that you implement. Currently those match, >> but if/when we expand the API (bumping the version in the header) and >> rebuild your program we will crash. > Hmm, I'm still not sure I understand this. Do you mean rebuilding the > tests out-of-tree and then running them against a newer/older libdrm > version? > > Because from my understanding the tests are always build together with > libdrm anyway. Or am I misunderstanding here something? > We're not talking about building out of tree or anything like that here. The following example should illustrate what I have in mind. Greatly appreciated if you can explain it in your own words. Currently * xf86drm.h #define DRM_EVENT_CONTEXT_VERSION 2 struct drmEventContext { int version; ... void (*page_flip_handler)(...); } * user struct foo { .version = ...VERSION // 2 ... } API bump * xf86drm.h #define DRM_EVENT_CONTEXT_VERSION 3 struct drmEventContext { int version; ... void (*page_flip_handler)(...); void (*page_flip_handler2)(...); } * user (hasn't been updated, only rebuilt) struct foo { .version = ...VERSION // 3 ... .page_flip_handler2 = 0 // ... or garbage. } * xf86drmMode.c int drmHandleEvent(...) { ... if (foo.version >= 3) foo.page_flip_handler2(); // boom ! else foo.page_flip_handler(); ... } Also worth mentioning is that the issue is rather wide spread rather than limited to libdrm :'( > >> Before you ask - yes current libdrm users are not doing the right >> thing and should be updated one of these days. > Maybe a dumb question, but what would be the right thing to do in may case. > > Define my own MY_XZY_EVENT_CONTEXT_VERSION and use these? > No need for extra defines imho. Just set the versions to 2 and 1 for evctx.base.version and evctx.version respectively. Glad to see some of the Samsung/Exynos people looking this way :-) Regards, Emil -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hey Emil, Emil Velikov wrote: > On 30 October 2015 at 11:28, Tobias Jakobi > <tjakobi@math.uni-bielefeld.de> wrote: >> Hello Emil, >> >> >> Emil Velikov wrote: >>> On 30 October 2015 at 11:16, Tobias Jakobi >>> <tjakobi@math.uni-bielefeld.de> wrote: >>>> Hello Hyungwon, >>>> >>>> first of all thanks for reviewing the series! >>>> >>>> >>>> >>>> Hyungwon Hwang wrote: >>>>> On Tue, 22 Sep 2015 17:54:55 +0200 >>>>> Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: >>>>> >>> >>>>>> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; >>>>>> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; >>>>> >>>>> The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the >>>>> versions are bumped, the event will contains wrong version info. >>>> Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and >>>> EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If the >>>> version in the public header is bumped, then it's also bumped here. So I >>>> don't see the issue. >>>> >>> The issue is that the public header defines the interface available, >>> while you set the version that you implement. Currently those match, >>> but if/when we expand the API (bumping the version in the header) and >>> rebuild your program we will crash. >> Hmm, I'm still not sure I understand this. Do you mean rebuilding the >> tests out-of-tree and then running them against a newer/older libdrm >> version? >> >> Because from my understanding the tests are always build together with >> libdrm anyway. Or am I misunderstanding here something? >> > We're not talking about building out of tree or anything like that > here. The following example should illustrate what I have in mind. > Greatly appreciated if you can explain it in your own words. > > > Currently > > * xf86drm.h > #define DRM_EVENT_CONTEXT_VERSION 2 > struct drmEventContext { > int version; > ... > void (*page_flip_handler)(...); > } > > * user > struct foo { > .version = ...VERSION // 2 > ... > } > > > API bump > > * xf86drm.h > #define DRM_EVENT_CONTEXT_VERSION 3 > struct drmEventContext { > int version; > ... > void (*page_flip_handler)(...); > void (*page_flip_handler2)(...); > } > > * user (hasn't been updated, only rebuilt) > struct foo { > .version = ...VERSION // 3 > ... > .page_flip_handler2 = 0 // ... or garbage. > } > > > * xf86drmMode.c > int drmHandleEvent(...) > { > ... > if (foo.version >= 3) > foo.page_flip_handler2(); // boom ! > else > foo.page_flip_handler(); > ... > } > > > Also worth mentioning is that the issue is rather wide spread rather > than limited to libdrm :'( OK, I see what you mean. However this shouldn't happen as long as the user properly zero initializes the event context structures, right? >>> Before you ask - yes current libdrm users are not doing the right >>> thing and should be updated one of these days. >> Maybe a dumb question, but what would be the right thing to do in may case. >> >> Define my own MY_XZY_EVENT_CONTEXT_VERSION and use these? >> > No need for extra defines imho. Just set the versions to 2 and 1 for > evctx.base.version and evctx.version respectively. OK, going to do this then. In any case, can the user assume that the event structures only "grow", in the sense that new fields are added to it? With best wishes, Tobias > Glad to see some of the Samsung/Exynos people looking this way :-) > > Regards, > Emil > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 October 2015 at 14:28, Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > OK, I see what you mean. However this shouldn't happen as long as the > user properly zero initializes the event context structures, right? > It pains me to say it but in this day and age, not everyone zero initializes their structs (be that via C99 initailizers, memset or just {}). > OK, going to do this then. In any case, can the user assume that the > event structures only "grow", in the sense that new fields are added to it? > That is correct. Note that this approach is also used elsewhere - the DRI driver <> loader model, also the drm ioctls structs are extended in a similar way (minus the actual version field). With the current knowledge in hand, perhaps we could have done things differently... But that's another story :-) Regards, Emil -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 30 Oct 2015 12:16:47 +0100 Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > Hello Hyungwon, > > first of all thanks for reviewing the series! > > > > Hyungwon Hwang wrote: > > On Tue, 22 Sep 2015 17:54:55 +0200 > > Tobias Jakobi <tjakobi@math.uni-bielefeld.de> wrote: > > > >> This tests async processing of G2D jobs. A separate thread is > >> spawned to monitor the DRM fd for events and check whether a G2D > >> job was completed. > >> > >> v2: Add GPLv2 header, argument handling and documentation. > >> Test is only installed when requested. > >> v3: Allocate G2D jobs with calloc which fixes 'busy' being > >> potentially uninitialized. Also enable timeout for poll() > >> in the monitor thread. This fixes pthread_join() not working > >> because of poll() not returning. > >> > >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > >> --- > >> tests/exynos/Makefile.am | 11 +- > >> tests/exynos/exynos_fimg2d_event.c | 326 > >> +++++++++++++++++++++++++++++++++++++ 2 files changed, 335 > >> insertions(+), 2 deletions(-) create mode 100644 > >> tests/exynos/exynos_fimg2d_event.c > >> > >> diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am > >> index e82d199..357d6b8 100644 > >> --- a/tests/exynos/Makefile.am > >> +++ b/tests/exynos/Makefile.am > >> @@ -20,16 +20,23 @@ endif > >> > >> if HAVE_INSTALL_TESTS > >> bin_PROGRAMS += \ > >> - exynos_fimg2d_perf > >> + exynos_fimg2d_perf \ > >> + exynos_fimg2d_event > >> else > >> noinst_PROGRAMS += \ > >> - exynos_fimg2d_perf > >> + exynos_fimg2d_perf \ > >> + exynos_fimg2d_event > >> endif > >> > >> exynos_fimg2d_perf_LDADD = \ > >> $(top_builddir)/libdrm.la \ > >> $(top_builddir)/exynos/libdrm_exynos.la > >> > >> +exynos_fimg2d_event_LDADD = \ > >> + $(top_builddir)/libdrm.la \ > >> + $(top_builddir)/exynos/libdrm_exynos.la \ > >> + -lpthread > >> + > >> exynos_fimg2d_test_LDADD = \ > >> $(top_builddir)/libdrm.la \ > >> $(top_builddir)/libkms/libkms.la \ > >> diff --git a/tests/exynos/exynos_fimg2d_event.c > >> b/tests/exynos/exynos_fimg2d_event.c new file mode 100644 > >> index 0000000..c03dcff > >> --- /dev/null > >> +++ b/tests/exynos/exynos_fimg2d_event.c > >> @@ -0,0 +1,326 @@ > >> +/* > >> + * Copyright (C) 2015 - Tobias Jakobi > >> + * > >> + * This is free software: you can redistribute it and/or modify > >> + * it under the terms of the GNU General Public License as > >> published > >> + * by the Free Software Foundation, either version 2 of the > >> License, > >> + * or (at your option) any later version. > >> + * > >> + * It is distributed in the hope that it will be useful, but > >> + * WITHOUT ANY WARRANTY; without even the implied warranty of > >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> + * GNU General Public License for more details. > >> + * You should have received a copy of the GNU General Public > >> License > >> + * along with it. If not, see <http://www.gnu.org/licenses/>. > >> + */ > >> + > >> +#include <unistd.h> > >> +#include <poll.h> > >> + > >> +#include <stdlib.h> > >> +#include <stdio.h> > >> +#include <time.h> > >> +#include <getopt.h> > >> + > >> +#include <pthread.h> > >> + > >> +#include <xf86drm.h> > >> + > >> +#include "exynos_drm.h" > >> +#include "exynos_drmif.h" > >> +#include "exynos_fimg2d.h" > >> + > >> +struct g2d_job { > >> + unsigned int id; > >> + unsigned int busy; > >> +}; > >> + > >> +struct exynos_evhandler { > >> + struct pollfd fds; > >> + struct exynos_event_context evctx; > >> +}; > >> + > >> +struct threaddata { > >> + unsigned int stop; > >> + struct exynos_device *dev; > >> + struct exynos_evhandler evhandler; > >> +}; > >> + > >> +static void g2d_event_handler(int fd, unsigned int cmdlist_no, > >> unsigned int tv_sec, > >> + unsigned > >> int tv_usec, void *user_data) +{ > >> + struct g2d_job *job = user_data; > >> + > >> + fprintf(stderr, "info: g2d job (id = %u, cmdlist number = > >> %u) finished!\n", > >> + job->id, cmdlist_no); > >> + > >> + job->busy = 0; > >> +} > >> + > >> +static void setup_g2d_event_handler(struct exynos_evhandler > >> *evhandler, int fd) +{ > >> + evhandler->fds.fd = fd; > >> + evhandler->fds.events = POLLIN; > >> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; > >> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; > > > > The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After > > the versions are bumped, the event will contains wrong version info. > Hmm, I don't see how this is true. Both DRM_EVENT_CONTEXT_VERSION and > EXYNOS_EVENT_CONTEXT_VERSION come from the public libdrm header. If > the version in the public header is bumped, then it's also bumped > here. So I don't see the issue. > > > > Also, I think the type of event must be set here. > What do you mean by 'type of event' here? I don't see a type field in > the event context structures. Forget about it. I was confused about that. I think that it would be just OK if the issue related with version is fixed. Best regards, Hyungwon Hwang > > > With best wishes, > Tobias > > > > > > Best regards, > > Hyungwon Hwang > > > >> + evhandler->evctx.g2d_event_handler = g2d_event_handler; > >> +} > >> + > >> +static void* threadfunc(void *arg) { > >> + const int timeout = 0; > >> + struct threaddata *data; > >> + > >> + data = arg; > >> + > >> + while (1) { > >> + if (data->stop) break; > >> + > >> + usleep(500); > >> + > >> + data->evhandler.fds.revents = 0; > >> + > >> + if (poll(&data->evhandler.fds, 1, timeout) < 0) > >> + continue; > >> + > >> + if (data->evhandler.fds.revents & (POLLHUP | > >> POLLERR)) > >> + continue; > >> + > >> + if (data->evhandler.fds.revents & POLLIN) > >> + exynos_handle_event(data->dev, > >> &data->evhandler.evctx); > >> + } > >> + > >> + pthread_exit(0); > >> +} > >> + > >> +/* > >> + * We need to wait until all G2D jobs are finished, otherwise we > >> + * potentially remove a BO which the engine still operates on. > >> + * This results in the following kernel message: > >> + * [drm:exynos_drm_gem_put_dma_addr] *ERROR* failed to lookup gem > >> object. > >> + * Also any subsequent BO allocations fail then with: > >> + * [drm:exynos_drm_alloc_buf] *ERROR* failed to allocate buffer. > >> + */ > >> +static void wait_all_jobs(struct g2d_job* jobs, unsigned num_jobs) > >> +{ > >> + unsigned i; > >> + > >> + for (i = 0; i < num_jobs; ++i) { > >> + while (jobs[i].busy) > >> + usleep(500); > >> + } > >> + > >> +} > >> + > >> +static struct g2d_job* free_job(struct g2d_job* jobs, unsigned > >> num_jobs) +{ > >> + unsigned i; > >> + > >> + for (i = 0; i < num_jobs; ++i) { > >> + if (jobs[i].busy == 0) > >> + return &jobs[i]; > >> + } > >> + > >> + return NULL; > >> +} > >> + > >> +static int g2d_work(struct g2d_context *ctx, struct g2d_image > >> *img, > >> + unsigned num_jobs, > >> unsigned iterations) +{ > >> + struct g2d_job *jobs = calloc(num_jobs, sizeof(struct > >> g2d_job)); > >> + int ret; > >> + unsigned i; > >> + > >> + /* setup job ids */ > >> + for (i = 0; i < num_jobs; ++i) > >> + jobs[i].id = i; > >> + > >> + for (i = 0; i < iterations; ++i) { > >> + unsigned x, y, w, h; > >> + > >> + struct g2d_job *j = NULL; > >> + > >> + while (1) { > >> + j = free_job(jobs, num_jobs); > >> + > >> + if (j) > >> + break; > >> + else > >> + usleep(500); > >> + } > >> + > >> + x = rand() % img->width; > >> + y = rand() % img->height; > >> + > >> + if (x == (img->width - 1)) > >> + x -= 1; > >> + if (y == (img->height - 1)) > >> + y -= 1; > >> + > >> + w = rand() % (img->width - x); > >> + h = rand() % (img->height - y); > >> + > >> + if (w == 0) w = 1; > >> + if (h == 0) h = 1; > >> + > >> + img->color = rand(); > >> + > >> + j->busy = 1; > >> + g2d_config_event(ctx, j); > >> + > >> + ret = g2d_solid_fill(ctx, img, x, y, w, h); > >> + > >> + if (ret == 0) > >> + g2d_exec2(ctx, G2D_EXEC_FLAG_ASYNC); > >> + > >> + if (ret != 0) { > >> + fprintf(stderr, "error: iteration %u (x = > >> %u, x = %u, x = %u, x = %u) failed\n", > >> + i, x, y, w, h); > >> + break; > >> + } > >> + } > >> + > >> + wait_all_jobs(jobs, num_jobs); > >> + free(jobs); > >> + > >> + return 0; > >> +} > >> + > >> +static void usage(const char *name) > >> +{ > >> + fprintf(stderr, "usage: %s [-ijwh]\n\n", name); > >> + > >> + fprintf(stderr, "\t-i <number of iterations>\n"); > >> + fprintf(stderr, "\t-j <number of G2D jobs> (default = > >> 4)\n\n"); + > >> + fprintf(stderr, "\t-w <buffer width> (default = 4096)\n"); > >> + fprintf(stderr, "\t-h <buffer height> (default = > >> 4096)\n"); + > >> + exit(0); > >> +} > >> + > >> +int main(int argc, char **argv) > >> +{ > >> + int fd, ret, c, parsefail; > >> + > >> + pthread_t event_thread; > >> + struct threaddata event_data = {0}; > >> + > >> + struct exynos_device *dev; > >> + struct g2d_context *ctx; > >> + struct exynos_bo *bo; > >> + > >> + struct g2d_image img = {0}; > >> + > >> + unsigned int iters = 0, njobs = 4; > >> + unsigned int bufw = 4096, bufh = 4096; > >> + > >> + ret = 0; > >> + parsefail = 0; > >> + > >> + while ((c = getopt(argc, argv, "i:j:w:h:")) != -1) { > >> + switch (c) { > >> + case 'i': > >> + if (sscanf(optarg, "%u", &iters) != 1) > >> + parsefail = 1; > >> + break; > >> + case 'j': > >> + if (sscanf(optarg, "%u", &njobs) != 1) > >> + parsefail = 1; > >> + break; > >> + case 'w': > >> + if (sscanf(optarg, "%u", &bufw) != 1) > >> + parsefail = 1; > >> + break; > >> + case 'h': > >> + if (sscanf(optarg, "%u", &bufh) != 1) > >> + parsefail = 1; > >> + break; > >> + default: > >> + parsefail = 1; > >> + break; > >> + } > >> + } > >> + > >> + if (parsefail || (argc == 1) || (iters == 0)) > >> + usage(argv[0]); > >> + > >> + if (bufw > 4096 || bufh > 4096) { > >> + fprintf(stderr, "error: buffer width/height should > >> be less than 4096.\n"); > >> + ret = -1; > >> + > >> + goto out; > >> + } > >> + > >> + if (bufw == 0 || bufh == 0) { > >> + fprintf(stderr, "error: buffer width/height should > >> be non-zero.\n"); > >> + ret = -1; > >> + > >> + goto out; > >> + } > >> + > >> + fd = drmOpen("exynos", NULL); > >> + if (fd < 0) { > >> + fprintf(stderr, "error: failed to open drm\n"); > >> + ret = -1; > >> + > >> + goto out; > >> + } > >> + > >> + dev = exynos_device_create(fd); > >> + if (dev == NULL) { > >> + fprintf(stderr, "error: failed to create > >> device\n"); > >> + ret = -2; > >> + > >> + goto fail; > >> + } > >> + > >> + ctx = g2d_init(fd); > >> + if (ctx == NULL) { > >> + fprintf(stderr, "error: failed to init G2D\n"); > >> + ret = -3; > >> + > >> + goto g2d_fail; > >> + } > >> + > >> + bo = exynos_bo_create(dev, bufw * bufh * 4, 0); > >> + if (bo == NULL) { > >> + fprintf(stderr, "error: failed to create bo\n"); > >> + ret = -4; > >> + > >> + goto bo_fail; > >> + } > >> + > >> + /* setup g2d image object */ > >> + img.width = bufw; > >> + img.height = bufh; > >> + img.stride = bufw * 4; > >> + img.color_mode = G2D_COLOR_FMT_ARGB8888 | G2D_ORDER_AXRGB; > >> + img.buf_type = G2D_IMGBUF_GEM; > >> + img.bo[0] = bo->handle; > >> + > >> + event_data.dev = dev; > >> + setup_g2d_event_handler(&event_data.evhandler, fd); > >> + > >> + pthread_create(&event_thread, NULL, threadfunc, > >> &event_data); + > >> + ret = g2d_work(ctx, &img, njobs, iters); > >> + if (ret != 0) > >> + fprintf(stderr, "error: g2d_work failed\n"); > >> + > >> + event_data.stop = 1; > >> + pthread_join(event_thread, NULL); > >> + > >> + exynos_bo_destroy(bo); > >> + > >> +bo_fail: > >> + g2d_fini(ctx); > >> + > >> +g2d_fail: > >> + exynos_device_destroy(dev); > >> + > >> +fail: > >> + drmClose(fd); > >> + > >> +out: > >> + return ret; > >> +} > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am index e82d199..357d6b8 100644 --- a/tests/exynos/Makefile.am +++ b/tests/exynos/Makefile.am @@ -20,16 +20,23 @@ endif if HAVE_INSTALL_TESTS bin_PROGRAMS += \ - exynos_fimg2d_perf + exynos_fimg2d_perf \ + exynos_fimg2d_event else noinst_PROGRAMS += \ - exynos_fimg2d_perf + exynos_fimg2d_perf \ + exynos_fimg2d_event endif exynos_fimg2d_perf_LDADD = \ $(top_builddir)/libdrm.la \ $(top_builddir)/exynos/libdrm_exynos.la +exynos_fimg2d_event_LDADD = \ + $(top_builddir)/libdrm.la \ + $(top_builddir)/exynos/libdrm_exynos.la \ + -lpthread + exynos_fimg2d_test_LDADD = \ $(top_builddir)/libdrm.la \ $(top_builddir)/libkms/libkms.la \ diff --git a/tests/exynos/exynos_fimg2d_event.c b/tests/exynos/exynos_fimg2d_event.c new file mode 100644 index 0000000..c03dcff --- /dev/null +++ b/tests/exynos/exynos_fimg2d_event.c @@ -0,0 +1,326 @@ +/* + * Copyright (C) 2015 - Tobias Jakobi + * + * This is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + * + * It is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with it. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <unistd.h> +#include <poll.h> + +#include <stdlib.h> +#include <stdio.h> +#include <time.h> +#include <getopt.h> + +#include <pthread.h> + +#include <xf86drm.h> + +#include "exynos_drm.h" +#include "exynos_drmif.h" +#include "exynos_fimg2d.h" + +struct g2d_job { + unsigned int id; + unsigned int busy; +}; + +struct exynos_evhandler { + struct pollfd fds; + struct exynos_event_context evctx; +}; + +struct threaddata { + unsigned int stop; + struct exynos_device *dev; + struct exynos_evhandler evhandler; +}; + +static void g2d_event_handler(int fd, unsigned int cmdlist_no, unsigned int tv_sec, + unsigned int tv_usec, void *user_data) +{ + struct g2d_job *job = user_data; + + fprintf(stderr, "info: g2d job (id = %u, cmdlist number = %u) finished!\n", + job->id, cmdlist_no); + + job->busy = 0; +} + +static void setup_g2d_event_handler(struct exynos_evhandler *evhandler, int fd) +{ + evhandler->fds.fd = fd; + evhandler->fds.events = POLLIN; + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION; + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION; + evhandler->evctx.g2d_event_handler = g2d_event_handler; +} + +static void* threadfunc(void *arg) { + const int timeout = 0; + struct threaddata *data; + + data = arg; + + while (1) { + if (data->stop) break; + + usleep(500); + + data->evhandler.fds.revents = 0; + + if (poll(&data->evhandler.fds, 1, timeout) < 0) + continue; + + if (data->evhandler.fds.revents & (POLLHUP | POLLERR)) + continue; + + if (data->evhandler.fds.revents & POLLIN) + exynos_handle_event(data->dev, &data->evhandler.evctx); + } + + pthread_exit(0); +} + +/* + * We need to wait until all G2D jobs are finished, otherwise we + * potentially remove a BO which the engine still operates on. + * This results in the following kernel message: + * [drm:exynos_drm_gem_put_dma_addr] *ERROR* failed to lookup gem object. + * Also any subsequent BO allocations fail then with: + * [drm:exynos_drm_alloc_buf] *ERROR* failed to allocate buffer. + */ +static void wait_all_jobs(struct g2d_job* jobs, unsigned num_jobs) +{ + unsigned i; + + for (i = 0; i < num_jobs; ++i) { + while (jobs[i].busy) + usleep(500); + } + +} + +static struct g2d_job* free_job(struct g2d_job* jobs, unsigned num_jobs) +{ + unsigned i; + + for (i = 0; i < num_jobs; ++i) { + if (jobs[i].busy == 0) + return &jobs[i]; + } + + return NULL; +} + +static int g2d_work(struct g2d_context *ctx, struct g2d_image *img, + unsigned num_jobs, unsigned iterations) +{ + struct g2d_job *jobs = calloc(num_jobs, sizeof(struct g2d_job)); + int ret; + unsigned i; + + /* setup job ids */ + for (i = 0; i < num_jobs; ++i) + jobs[i].id = i; + + for (i = 0; i < iterations; ++i) { + unsigned x, y, w, h; + + struct g2d_job *j = NULL; + + while (1) { + j = free_job(jobs, num_jobs); + + if (j) + break; + else + usleep(500); + } + + x = rand() % img->width; + y = rand() % img->height; + + if (x == (img->width - 1)) + x -= 1; + if (y == (img->height - 1)) + y -= 1; + + w = rand() % (img->width - x); + h = rand() % (img->height - y); + + if (w == 0) w = 1; + if (h == 0) h = 1; + + img->color = rand(); + + j->busy = 1; + g2d_config_event(ctx, j); + + ret = g2d_solid_fill(ctx, img, x, y, w, h); + + if (ret == 0) + g2d_exec2(ctx, G2D_EXEC_FLAG_ASYNC); + + if (ret != 0) { + fprintf(stderr, "error: iteration %u (x = %u, x = %u, x = %u, x = %u) failed\n", + i, x, y, w, h); + break; + } + } + + wait_all_jobs(jobs, num_jobs); + free(jobs); + + return 0; +} + +static void usage(const char *name) +{ + fprintf(stderr, "usage: %s [-ijwh]\n\n", name); + + fprintf(stderr, "\t-i <number of iterations>\n"); + fprintf(stderr, "\t-j <number of G2D jobs> (default = 4)\n\n"); + + fprintf(stderr, "\t-w <buffer width> (default = 4096)\n"); + fprintf(stderr, "\t-h <buffer height> (default = 4096)\n"); + + exit(0); +} + +int main(int argc, char **argv) +{ + int fd, ret, c, parsefail; + + pthread_t event_thread; + struct threaddata event_data = {0}; + + struct exynos_device *dev; + struct g2d_context *ctx; + struct exynos_bo *bo; + + struct g2d_image img = {0}; + + unsigned int iters = 0, njobs = 4; + unsigned int bufw = 4096, bufh = 4096; + + ret = 0; + parsefail = 0; + + while ((c = getopt(argc, argv, "i:j:w:h:")) != -1) { + switch (c) { + case 'i': + if (sscanf(optarg, "%u", &iters) != 1) + parsefail = 1; + break; + case 'j': + if (sscanf(optarg, "%u", &njobs) != 1) + parsefail = 1; + break; + case 'w': + if (sscanf(optarg, "%u", &bufw) != 1) + parsefail = 1; + break; + case 'h': + if (sscanf(optarg, "%u", &bufh) != 1) + parsefail = 1; + break; + default: + parsefail = 1; + break; + } + } + + if (parsefail || (argc == 1) || (iters == 0)) + usage(argv[0]); + + if (bufw > 4096 || bufh > 4096) { + fprintf(stderr, "error: buffer width/height should be less than 4096.\n"); + ret = -1; + + goto out; + } + + if (bufw == 0 || bufh == 0) { + fprintf(stderr, "error: buffer width/height should be non-zero.\n"); + ret = -1; + + goto out; + } + + fd = drmOpen("exynos", NULL); + if (fd < 0) { + fprintf(stderr, "error: failed to open drm\n"); + ret = -1; + + goto out; + } + + dev = exynos_device_create(fd); + if (dev == NULL) { + fprintf(stderr, "error: failed to create device\n"); + ret = -2; + + goto fail; + } + + ctx = g2d_init(fd); + if (ctx == NULL) { + fprintf(stderr, "error: failed to init G2D\n"); + ret = -3; + + goto g2d_fail; + } + + bo = exynos_bo_create(dev, bufw * bufh * 4, 0); + if (bo == NULL) { + fprintf(stderr, "error: failed to create bo\n"); + ret = -4; + + goto bo_fail; + } + + /* setup g2d image object */ + img.width = bufw; + img.height = bufh; + img.stride = bufw * 4; + img.color_mode = G2D_COLOR_FMT_ARGB8888 | G2D_ORDER_AXRGB; + img.buf_type = G2D_IMGBUF_GEM; + img.bo[0] = bo->handle; + + event_data.dev = dev; + setup_g2d_event_handler(&event_data.evhandler, fd); + + pthread_create(&event_thread, NULL, threadfunc, &event_data); + + ret = g2d_work(ctx, &img, njobs, iters); + if (ret != 0) + fprintf(stderr, "error: g2d_work failed\n"); + + event_data.stop = 1; + pthread_join(event_thread, NULL); + + exynos_bo_destroy(bo); + +bo_fail: + g2d_fini(ctx); + +g2d_fail: + exynos_device_destroy(dev); + +fail: + drmClose(fd); + +out: + return ret; +}
This tests async processing of G2D jobs. A separate thread is spawned to monitor the DRM fd for events and check whether a G2D job was completed. v2: Add GPLv2 header, argument handling and documentation. Test is only installed when requested. v3: Allocate G2D jobs with calloc which fixes 'busy' being potentially uninitialized. Also enable timeout for poll() in the monitor thread. This fixes pthread_join() not working because of poll() not returning. Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> --- tests/exynos/Makefile.am | 11 +- tests/exynos/exynos_fimg2d_event.c | 326 +++++++++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 tests/exynos/exynos_fimg2d_event.c