<div dir="ltr"><div>I made a few trivial comments (mostly just asking for plane == 0 assertions more places).  With that, these 5 are<br><br></div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 8, 2017 at 3:09 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Tue, Mar 7, 2017 at 9:31 PM, Ben Widawsky <span dir="ltr"><<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-3582020962758831698m_848714118394336865gmail-">Unlike stride, there was no previous offset getter, so it can be right<br>
on the first try.<br>
<br>
v2: Return EINVAL when plane is greater than total planes to make it<br>
match the similar APIs.<br>
Avoid leak after fromPlanar (Daniel)<br>
Make sure when getting offsets we consider dumb images (Daniel)<br>
<br>
v3: Use Jason's recommendation for handling the non-planar case.<br>
<br>
</span>v4: Return int64_t so we can get real errors<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-"><br>
Signed-off-by: Ben Widawsky <<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>><br>
Reviewed-by: Eric Engestrom <<a href="mailto:eric.engestrom@imgtec.com" target="_blank">eric.engestrom@imgtec.com</a>><br>
Acked-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>><br>
---<br>
 src/gbm/backends/dri/gbm_dri.<wbr>c | 33 ++++++++++++++++++++++++++++++<wbr>+++<br>
 src/gbm/gbm-symbols-check      |  1 +<br>
 src/gbm/main/gbm.c             | 15 +++++++++++++++<br>
 src/gbm/main/gbm.h             |  3 +++<br>
 src/gbm/main/gbmint.h          |  1 +<br>
 5 files changed, 53 insertions(+)<br>
<br>
diff --git a/src/gbm/backends/dri/gbm_dri<wbr>.c b/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
</span>index 56a976fe78..b53f52f8df 100644<br>
--- a/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
+++ b/src/gbm/backends/dri/gbm_dri<wbr>.c<br>
@@ -699,6 +699,38 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)<br>
    return (uint32_t)stride;<br>
 }<br>
<br>
+static int64_t<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">+gbm_dri_bo_get_offset(struct gbm_bo *_bo, int plane)<br>
+{<br>
+   struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);<br>
+   struct gbm_dri_bo *bo = gbm_dri_bo(_bo);<br>
+   int offset = 0;<br>
+<br>
+   if (!dri->image || dri->image->base.version < 13 || !dri->image->fromPlanar) {<br>
+      errno = ENOSYS;<br>
</span>+      return -1;<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">+   }<br>
+<br>
+   if (plane >= get_number_planes(dri, bo->image)) {<br>
+      errno = EINVAL;<br>
</span>+      return -2;<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">+   }<br>
+<br>
+    /* Dumb images have no offset */<br>
+   if (!bo->image)<br></span></blockquote><div><br></div></div></div><div>assert(plane == 0);<br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-3582020962758831698m_848714118394336865gmail-">
+      return 0;<br>
</span><span class="m_-3582020962758831698m_848714118394336865gmail-">+<br>
+   __DRIimage *image = dri->image->fromPlanar(bo->ima<wbr>ge, plane, NULL);<br>
+   if (image) {<br>
+      dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_OFFSET, &offset);<br>
+      dri->image->destroyImage(image<wbr>);<br>
+   } else {<br></span></blockquote></span><div><br>assert(plane == 0);<br> </div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-3582020962758831698m_848714118394336865gmail-">
+      dri->image->queryImage(bo->ima<wbr>ge, __DRI_IMAGE_ATTRIB_OFFSET, &offset);<br>
+   }<br>
+<br>
+   return (uint32_t)offset;<br>
+}<br>
+<br>
 static void<br>
 gbm_dri_bo_destroy(struct gbm_bo *_bo)<br>
 {<br>
</span>@@ -1194,6 +1226,7 @@ dri_device_create(int fd)<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">    dri->base.base.bo_get_planes = gbm_dri_bo_get_planes;<br>
    dri->base.base.bo_get_handle = gbm_dri_bo_get_handle_for_plan<wbr>e;<br>
    dri->base.base.bo_get_stride = gbm_dri_bo_get_stride;<br>
+   dri->base.base.bo_get_offset = gbm_dri_bo_get_offset;<br>
    dri->base.base.bo_destroy = gbm_dri_bo_destroy;<br>
    dri->base.base.destroy = dri_destroy;<br>
    dri->base.base.surface_create = gbm_dri_surface_create;<br>
diff --git a/src/gbm/gbm-symbols-check b/src/gbm/gbm-symbols-check<br>
index 459006a63f..7ff78ab400 100755<br>
--- a/src/gbm/gbm-symbols-check<br>
+++ b/src/gbm/gbm-symbols-check<br>
@@ -16,6 +16,7 @@ gbm_bo_get_height<br>
 gbm_bo_get_stride<br>
 gbm_bo_get_stride_for_plane<br>
 gbm_bo_get_format<br>
+gbm_bo_get_offset<br>
 gbm_bo_get_device<br>
 gbm_bo_get_handle<br>
 gbm_bo_get_fd<br>
diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c<br>
</span>index 0a9f0bef7e..afcca63da3 100644<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">--- a/src/gbm/main/gbm.c<br>
+++ b/src/gbm/main/gbm.c<br>
@@ -194,6 +194,21 @@ gbm_bo_get_format(struct gbm_bo *bo)<br>
    return bo->format;<br>
 }<br>
<br>
+/** Get the offset for the data of the specified plane<br>
+ *<br>
+ * Extra planes, and even the first plane, may have an offset from the start of<br>
+ * the buffer object. This function will provide the offset for the given plane<br>
+ * to be used in various KMS APIs.<br>
+ *<br>
+ * \param bo The buffer object<br>
+ * \return The offset<br></span></blockquote><div><br></div></div></div><div>Maybe a comment about < 0 on error?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_-3582020962758831698m_848714118394336865gmail-">
+ */<br>
</span>+GBM_EXPORT int64_t<br></blockquote><div><br></div><div>I find the int64_t a bit ugly given the rest of the API, but I also can't really see any better options.  I guess we could say (uint32_t)-1 is an invalid offset (it almost certainly is) and use that.  In any case, int64_t is just fine.<br></div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="m_-3582020962758831698m_848714118394336865gmail-">+gbm_bo_get_offset(struct gbm_bo *bo, int plane)<br>
+{<br>
+   return bo->gbm->bo_get_offset(bo, plane);<br>
+}<br>
+<br>
 /** Get the gbm device used to create the buffer object<br>
  *<br>
  * \param bo The buffer object<br>
diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h<br>
</span>index 1719c5312a..e3e5d34d97 100644<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">--- a/src/gbm/main/gbm.h<br>
+++ b/src/gbm/main/gbm.h<br>
@@ -309,6 +309,9 @@ gbm_bo_get_stride_for_plane(st<wbr>ruct gbm_bo *bo, int plane);<br>
 uint32_t<br>
 gbm_bo_get_format(struct gbm_bo *bo);<br>
<br>
</span>+int64_t<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">+gbm_bo_get_offset(struct gbm_bo *bo, int plane);<br>
+<br>
 struct gbm_device *<br>
 gbm_bo_get_device(struct gbm_bo *bo);<br>
<br>
diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h<br>
</span>index 26d18bab6b..a6541d91c5 100644<br>
<span class="m_-3582020962758831698m_848714118394336865gmail-">--- a/src/gbm/main/gbmint.h<br>
+++ b/src/gbm/main/gbmint.h<br>
@@ -79,6 +79,7 @@ struct gbm_device {<br>
    int (*bo_get_planes)(struct gbm_bo *bo);<br>
    union gbm_bo_handle (*bo_get_handle)(struct gbm_bo *bo, int plane);<br>
    uint32_t (*bo_get_stride)(struct gbm_bo *bo, int plane);<br>
</span>+   int64_t (*bo_get_offset)(struct gbm_bo *bo, int plane);<br>
<div class="m_-3582020962758831698m_848714118394336865gmail-HOEnZb"><div class="m_-3582020962758831698m_848714118394336865gmail-h5">    void (*bo_destroy)(struct gbm_bo *bo);<br>
<br>
    struct gbm_surface *(*surface_create)(struct gbm_device *gbm,<br>
--<br>
2.12.0<br>
<br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>