Re: [RFC] libdrm_intel: Add support for userptr objects

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux