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 <topi.pohjolai...@intel.com>
> > ---
> > .../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 <assert.h>
> > #include <stdlib.h>
> > #include <unistd.h>
> > +#include <waffle.h>
> > +#include <waffle_gbm.h>
> > +#include <gbm.h>
> > +#include <libdrm/drm.h>
> > +#include <libdrm/i915_drm.h>
> > +#include <sys/ioctl.h>
> >
> > #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
