Re: [Piglit] [RFC 2/3] framework: gbm: support for creating external buffers
On Fri, Mar 01, 2013 at 11:10:43AM -0800, Chad Versace wrote:
> On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> > Getting direct access to the gbm device through the waffle display
> > and creating the buffer is rather straightforward thing to do.
> > Writing to the buffer using CPU, however, requires priviledges for
> > the underlying gem-ioctl.
> >
> > Signed-off-by: Topi Pohjolainen
> > ---
> > .../piglit-framework-gl/piglit_gbm_framework.c | 78
> >
> > 1 file changed, 78 insertions(+)
>
> It's clear here that you're trying to test a difficult thing and there is no
> easy way to do it. I have several questions for you for which I don't know the
> answer.
>
>
> > diff --git a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> > b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> > index 4df3861..00e1425 100644
> > --- a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> > +++ b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> > @@ -24,10 +24,18 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> >
> > #include "piglit-util-gl-common.h"
> > #include "piglit_gbm_framework.h"
> >
> > +#define ALIGN(value, alignment) (((value) + alignment - 1) & ~(alignment -
> > 1))
> > +
> > static void
> > enter_event_loop(struct piglit_winsys_framework *winsys_fw)
> > {
> > @@ -51,6 +59,73 @@ destroy(struct piglit_gl_framework *gl_fw)
> > free(winsys_fw);
> > }
> >
> > +static void *
> > +map(struct gbm_device *gbm, struct gbm_bo *bo, unsigned n_bytes)
> > +{
> > +int res;
> > +struct drm_i915_gem_mmap mmap_arg;
> > +
> > +mmap_arg.handle = gbm_bo_get_handle(bo).u32;
> > +mmap_arg.offset = 0;
> > +mmap_arg.size = n_bytes;
> > +
> > +res = ioctl(gbm_device_get_fd(gbm), DRM_IOCTL_I915_GEM_MMAP,
> > &mmap_arg);
> > +
> > +if (res)
> > +return 0;
> > +
> > +return (void *)(uintptr_t)mmap_arg.addr_ptr;
> > +}
>
> This function is specific to i915, but gbm is specific only to drm. Is it
> possible
> to rewrite this function using only drm ioctls? If it's not possbile, or if
> it that's a bad idea,
> we should somehow detect here whether gbm is using i915 and skip the test if
> it's not.
Well, I'm still hoping that it would be possible, but I'm getting more and more
skeptical. I'm only aware of the MMAP and PWRITE which are both intel specific.
I'll take a look if the detection is possible.
>
>
> > +
> > +/* At the time of writing this, GBM did not have any alignment
> > + * restrictions on height or width.
> > + */
> > +static void *
> > +create_ext_420_buffer(struct piglit_gl_framework *gl_fw,
> > + unsigned w, unsigned h, bool swap_vu,
> > + const void *y, const void *u, const void *v)
> > +{
> > + struct waffle_gbm_display *native = waffle_display_get_native(
> > + piglit_wfl_framework(gl_fw)->display)->gbm;
> > + struct gbm_bo *bo = gbm_bo_create(native->gbm_device, w, h,
> > + swap_vu ? GBM_FORMAT_YVU420 : GBM_FORMAT_YUV420,
> > + GBM_BO_USE_RENDERING);
> > + unsigned char *p;
> > + unsigned y_off = 0;
> > + unsigned u_off = ALIGN(w * h, 4096);
> > + unsigned v_off = u_off + ALIGN((w / 2) * (h / 2), 4096);
> > + unsigned total = v_off + ALIGN((w / 2) * (h / 2), 4096);
>
> Why the 4096? Are you assuming that the buffer layout is aligned to 4096
> because that is the size of a tile on Intel gpus?
>
> I expected to see gbm_bo_get_stride() used in these calculations. Using it
> may allow you to remove the hardcoded 4096.
>
The problem here is that the buffer has two different strides (y has stride
based on full width and u/v-planes based on half), and get_stride() could return
only one.
I've chosen to use the page aligning after I saw Kristian's implementation
allowing each plane to be treated as a separate image itself - check the
YUV-allocation support I wrote for gbm-dri in mesa.
An alternative allowing one to get rid of this separate stride problem would be
to take advantage of Tom's dma_buf extension for EGL images and allocate Y and
U/V planes as separate buffers and allow the EGL extension to combine them
instead of doing it in GBM level.
One still has the former buffer-write-and-intel-specific-ioctl problem to solve,
but these are separate problems anyway.
Topi
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 2/3] framework: gbm: support for creating external buffers
On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> Getting direct access to the gbm device through the waffle display
> and creating the buffer is rather straightforward thing to do.
> Writing to the buffer using CPU, however, requires priviledges for
> the underlying gem-ioctl.
>
> Signed-off-by: Topi Pohjolainen
> ---
> .../piglit-framework-gl/piglit_gbm_framework.c | 78
>
> 1 file changed, 78 insertions(+)
It's clear here that you're trying to test a difficult thing and there is no
easy way to do it. I have several questions for you for which I don't know the
answer.
> diff --git a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> index 4df3861..00e1425 100644
> --- a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> +++ b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
> @@ -24,10 +24,18 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
>
> #include "piglit-util-gl-common.h"
> #include "piglit_gbm_framework.h"
>
> +#define ALIGN(value, alignment) (((value) + alignment - 1) & ~(alignment -
> 1))
> +
> static void
> enter_event_loop(struct piglit_winsys_framework *winsys_fw)
> {
> @@ -51,6 +59,73 @@ destroy(struct piglit_gl_framework *gl_fw)
> free(winsys_fw);
> }
>
> +static void *
> +map(struct gbm_device *gbm, struct gbm_bo *bo, unsigned n_bytes)
> +{
> +int res;
> +struct drm_i915_gem_mmap mmap_arg;
> +
> +mmap_arg.handle = gbm_bo_get_handle(bo).u32;
> +mmap_arg.offset = 0;
> +mmap_arg.size = n_bytes;
> +
> +res = ioctl(gbm_device_get_fd(gbm), DRM_IOCTL_I915_GEM_MMAP,
> &mmap_arg);
> +
> +if (res)
> +return 0;
> +
> +return (void *)(uintptr_t)mmap_arg.addr_ptr;
> +}
This function is specific to i915, but gbm is specific only to drm. Is it
possible
to rewrite this function using only drm ioctls? If it's not possbile, or if it
that's a bad idea,
we should somehow detect here whether gbm is using i915 and skip the test if
it's not.
> +
> +/* At the time of writing this, GBM did not have any alignment
> + * restrictions on height or width.
> + */
> +static void *
> +create_ext_420_buffer(struct piglit_gl_framework *gl_fw,
> + unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v)
> +{
> + struct waffle_gbm_display *native = waffle_display_get_native(
> + piglit_wfl_framework(gl_fw)->display)->gbm;
> + struct gbm_bo *bo = gbm_bo_create(native->gbm_device, w, h,
> + swap_vu ? GBM_FORMAT_YVU420 : GBM_FORMAT_YUV420,
> + GBM_BO_USE_RENDERING);
> + unsigned char *p;
> + unsigned y_off = 0;
> + unsigned u_off = ALIGN(w * h, 4096);
> + unsigned v_off = u_off + ALIGN((w / 2) * (h / 2), 4096);
> + unsigned total = v_off + ALIGN((w / 2) * (h / 2), 4096);
Why the 4096? Are you assuming that the buffer layout is aligned to 4096
because that is the size of a tile on Intel gpus?
I expected to see gbm_bo_get_stride() used in these calculations. Using it
may allow you to remove the hardcoded 4096.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [RFC 2/3] framework: gbm: support for creating external buffers
Getting direct access to the gbm device through the waffle display
and creating the buffer is rather straightforward thing to do.
Writing to the buffer using CPU, however, requires priviledges for
the underlying gem-ioctl.
Signed-off-by: Topi Pohjolainen
---
.../piglit-framework-gl/piglit_gbm_framework.c | 78
1 file changed, 78 insertions(+)
diff --git a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
index 4df3861..00e1425 100644
--- a/tests/util/piglit-framework-gl/piglit_gbm_framework.c
+++ b/tests/util/piglit-framework-gl/piglit_gbm_framework.c
@@ -24,10 +24,18 @@
#include
#include
#include
+#include
+#include
+#include
+#include
+#include
+#include
#include "piglit-util-gl-common.h"
#include "piglit_gbm_framework.h"
+#define ALIGN(value, alignment) (((value) + alignment - 1) & ~(alignment - 1))
+
static void
enter_event_loop(struct piglit_winsys_framework *winsys_fw)
{
@@ -51,6 +59,73 @@ destroy(struct piglit_gl_framework *gl_fw)
free(winsys_fw);
}
+static void *
+map(struct gbm_device *gbm, struct gbm_bo *bo, unsigned n_bytes)
+{
+int res;
+struct drm_i915_gem_mmap mmap_arg;
+
+mmap_arg.handle = gbm_bo_get_handle(bo).u32;
+mmap_arg.offset = 0;
+mmap_arg.size = n_bytes;
+
+res = ioctl(gbm_device_get_fd(gbm), DRM_IOCTL_I915_GEM_MMAP,
&mmap_arg);
+
+if (res)
+return 0;
+
+return (void *)(uintptr_t)mmap_arg.addr_ptr;
+}
+
+/* At the time of writing this, GBM did not have any alignment
+ * restrictions on height or width.
+ */
+static void *
+create_ext_420_buffer(struct piglit_gl_framework *gl_fw,
+ unsigned w, unsigned h, bool swap_vu,
+ const void *y, const void *u, const void *v)
+{
+ struct waffle_gbm_display *native = waffle_display_get_native(
+ piglit_wfl_framework(gl_fw)->display)->gbm;
+ struct gbm_bo *bo = gbm_bo_create(native->gbm_device, w, h,
+ swap_vu ? GBM_FORMAT_YVU420 : GBM_FORMAT_YUV420,
+ GBM_BO_USE_RENDERING);
+ unsigned char *p;
+ unsigned y_off = 0;
+ unsigned u_off = ALIGN(w * h, 4096);
+ unsigned v_off = u_off + ALIGN((w / 2) * (h / 2), 4096);
+ unsigned total = v_off + ALIGN((w / 2) * (h / 2), 4096);
+
+ if (!bo)
+ return NULL;
+
+ p = (unsigned char *)map(native->gbm_device, bo, total);
+
+ if (!p) {
+ gbm_bo_destroy(bo);
+ return NULL;
+ }
+
+ memcpy(p + y_off, y, w * h);
+
+ if (swap_vu) {
+ memcpy(p + u_off, v, (w / 2) * (h / 2));
+ memcpy(p + v_off, u, (w / 2) * (h / 2));
+ } else {
+ memcpy(p + u_off, u, (w / 2) * (h / 2));
+ memcpy(p + v_off, v, (w / 2) * (h / 2));
+ }
+
+ return bo;
+}
+
+static void
+destroy_ext_buffer(struct piglit_gl_framework *gl_fw, void *buf)
+{
+ (void)gl_fw;
+ gbm_bo_destroy((struct gbm_bo *)buf);
+}
+
struct piglit_gl_framework*
piglit_gbm_framework_create(const struct piglit_gl_test_config *test_config)
{
@@ -70,6 +145,9 @@ piglit_gbm_framework_create(const struct
piglit_gl_test_config *test_config)
winsys_fw->enter_event_loop = enter_event_loop;
gl_fw->destroy = destroy;
+ gl_fw->create_external_420_buffer = create_ext_420_buffer;
+ gl_fw->destroy_external_buffer = destroy_ext_buffer;
+
return gl_fw;
fail:
--
1.7.9.5
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
