On Wed, Jul 09, 2014 at 04:25:55PM +0200, Daniel Vetter wrote:
> On Wed, Jul 09, 2014 at 02:16:59PM +0100, Damien Lespiau wrote:
> > On Wed, Jul 09, 2014 at 02:08:08PM +0100, Tvrtko Ursulin wrote:
> > >
> > > On 06/19/2014 12:13 PM, Damien Lespiau wrote:
> > > >On Wed, Feb 26, 2014 at 04:41:41PM +0000, Tvrtko Ursulin wrote:
> > > >>From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > >>
> > > >>Allow userptr objects to be created and used via libdrm_intel.
> > > >>
> > > >>At the moment tiling and mapping to GTT aperture is not supported
> > > >>due hardware limitations across different generations and uncertainty
> > > >>about its usefulness.
> > > >>
> > > >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > >>---
> > > >> include/drm/i915_drm.h | 16 +++++
> > > >> intel/intel_bufmgr.c | 13 ++++
> > > >> intel/intel_bufmgr.h | 5 ++
> > > >> intel/intel_bufmgr_gem.c | 154 +++++++++++++++++++++++++++++++++++++++++++++-
> > > >> intel/intel_bufmgr_priv.h | 12 +++-
> > > >> 5 files changed, 198 insertions(+), 2 deletions(-)
> > > >
> > > >Apart from couple of remarks below I couldn't find anything that would
> > > >prevent merging this. Well, except maybe that it'd be very nice to have
> > > >some feedback from someone using it, we do have an API/ABI guarantee on
> > > >libdrm after all.
> > >
> > > Looks like I've forgotten to reply to this. I did address the other
> > > review comments and sent out a v2 back then.
> > >
> > > But for what users are concerned, apart from internal ones who have
> > > been using this API for some years now, I don't know of any.
> >
> > Well, considering this is only a wrapper of an ioctl() already
> > upstreamed, I'm inclined to just push it as is. Daniel any thoughts?
>
> Since both igt and sna have their own wrappers and the beinget patch for
> this hasn't surfaced yet we don't really have a public open-source user
> for this yet. My understanding of Dave's stance is that we should hold off
> with committing until this is requirement is fulfilled.
The interface is fairly ugly since it combines the tiling into the
create, which is just as convenient as a second step.
But if you want a user, just wire up an accelerated glReadPixels. Now,
this is only really beneficial as the current ReadPixels is not well
optimised and it assumes that userptr drm_intel_bo is automatically
synced on destruction:
diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c
index beb3152..1b1f7e4 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
@@ -161,6 +161,103 @@ do_blit_readpixels(struct gl_context * ctx,
return true;
}
+static bool
+do_userptr_readpixels(struct gl_context * ctx,
+ GLint x, GLint y, GLsizei width, GLsizei height,
+ GLenum format, GLenum type,
+ const struct gl_pixelstore_attrib *pack, GLvoid * pixels)
+{
+ struct brw_context *brw = brw_context(ctx);
+ GLuint dst_offset;
+ drm_intel_bo *dst_buffer;
+ GLint dst_x, dst_y;
+ GLuint dirty;
+
+ DBG("%s\n", __FUNCTION__);
+
+ assert(_mesa_is_bufferobj(pack->BufferObj));
+
+ struct gl_renderbuffer *rb = ctx->ReadBuffer->_ColorReadBuffer;
+ struct intel_renderbuffer *irb = intel_renderbuffer(rb);
+ uintptr_t start, end;
+
+ /* Currently this function only supports reading from color buffers. */
+ if (!_mesa_is_color_format(format))
+ return false;
+
+ assert(irb != NULL);
+
+ if (ctx->_ImageTransferState ||
+ !_mesa_format_matches_format_and_type(irb->mt->format, format, type,
+ false)) {
+ DBG("%s - bad format for blit\n", __FUNCTION__);
+ return false;
+ }
+
+ if (pack->SwapBytes || pack->LsbFirst) {
+ DBG("%s: bad packing params\n", __FUNCTION__);
+ return false;
+ }
+
+ int dst_stride = _mesa_image_row_stride(pack, width, format, type);
+ bool dst_flip = false;
+ /* Mesa flips the dst_stride for pack->Invert, but we want our mt to have a
+ * normal dst_stride.
+ */
+ struct gl_pixelstore_attrib uninverted_pack = *pack;
+ if (pack->Invert) {
+ dst_stride = -dst_stride;
+ dst_flip = true;
+ uninverted_pack.Invert = false;
+ }
+
+ dst_offset = (GLintptr)pixels;
+ dst_offset += _mesa_image_offset(2, &uninverted_pack, width, height,
+ format, type, 0, 0, 0);
+
+ if (!_mesa_clip_copytexsubimage(ctx,
+ &dst_x, &dst_y,
+ &x, &y,
+ &width, &height)) {
+ return true;
+ }
+
+ dirty = brw->front_buffer_dirty;
+ intel_prepare_render(brw);
+ brw->front_buffer_dirty = dirty;
+
+ start = (uintptr_t)(pixel + dst_offset) & ~4095;
+ end = (uintptr_t)(pixel + dst_offset + dst_stride * height + 4095) & ~4095;
+
+ dst_buffer = drm_intel_bo_alloc_userptr(bufmgr, "glReadPixels",
+ (void *)start,
+ I915_TILING_NONE, dst_stride,
+ end - start, 0);
+
+ struct intel_mipmap_tree *pbo_mt =
+ intel_miptree_create_for_bo(brw,
+ dst_buffer,
+ irb->mt->format,
+ (uintptr_t)(pixel + dst_offset) - start,
+ width, height,
+ dst_stride);
+
+ if (!intel_miptree_blit(brw,
+ irb->mt, irb->mt_level, irb->mt_layer,
+ x, y, _mesa_is_winsys_fbo(ctx->ReadBuffer),
+ pbo_mt, 0, 0,
+ 0, 0, dst_flip,
+ width, height, GL_COPY)) {
+ return false;
+ }
+
+ intel_miptree_release(&pbo_mt);
+ drm_intel_bo_unreference(dst_buffer);
+
+ DBG("%s - DONE\n", __FUNCTION__);
+
+ return true;
+}
+
void
intelReadPixels(struct gl_context * ctx,
GLint x, GLint y, GLsizei width, GLsizei height,
@@ -182,6 +280,13 @@ intelReadPixels(struct gl_context * ctx,
perf_debug("%s: fallback to CPU mapping in PBO case\n", __FUNCTION__);
}
+ if (do_userptr_readpixels(ctx,
+ x, y, width, height,
+ format, type,
+ pack, pixels)) {
+ return;
+ }
+
/* glReadPixels() wont dirty the front buffer, so reset the dirty
* flag after calling intel_prepare_render(). */
dirty = brw->front_buffer_dirty;
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
