<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 7, 2017 at 2:31 PM, Ben Widawsky <<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 17-03-07 08:28:09, Jason Ekstrand wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Mon, Mar 6, 2017 at 6:37 PM, Ben Widawsky <<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> v2: Make the error return be -1 instead of 0 because I think 0 is actually valid. v3: Set errno to EINVAL when the specified plane is above the total planes. (Jason Ekstrand) Return the bo's handle if there is no image ie. for dumb images like cursor (Daniel) Signed-off-by: Ben Widawsky <<a href="mailto:ben@bwidawsk.net" target="_blank">ben@bwidawsk.net</a>> Acked-by: Daniel Stone <<a href="mailto:daniels@collabora.com" target="_blank">daniels@collabora.com</a>> --- src/gbm/backends/dri/gbm_dri.c | 35 +++++++++++++++++++++++++++++++++++ src/gbm/gbm-symbols-check | 1 + src/gbm/main/gbm.c | 18 ++++++++++++++++++ src/gbm/main/gbm.h | 3 +++ src/gbm/main/gbmint.h | 1 + 5 files changed, 58 insertions(+) diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_ dri.c index 0b75e411df..c3704e505b 100644 --- a/src/gbm/backends/dri/gbm_dri.c +++ b/src/gbm/backends/dri/gbm_dri.c @@ -624,6 +624,40 @@ gbm_dri_bo_get_planes(struct gbm_bo *_bo) return get_number_planes(dri, bo->image); } +static union gbm_bo_handle +gbm_dri_bo_get_handle_for_plane(struct gbm_bo *_bo, int plane) +{ + struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm); + struct gbm_dri_bo *bo = gbm_dri_bo(_bo); + union gbm_bo_handle ret; + ret.s32 = -1; + + if (!dri->image || dri->image->base.version < 13 || !dri->image->fromPlanar) { + errno = ENOSYS; + return ret; + } + + if (plane >= get_number_planes(dri, bo->image)) { + errno = EINVAL; + return ret; + } + + if (!bo->image) { + ret.s32 = bo->handle; + return ret; + } + + __DRIimage *image = dri->image->fromPlanar(bo->image, plane, NULL); + if (!image) { + /* Use the parent's handle */ + image = bo->image; </blockquote> Assuming get_number_of_planes does the right thing, I think this is correct. Would it make sense to add an assert(plane == 0) to this error case and the one above? </blockquote> </div></div> Let me claim ignorance... For "the one above": I thought it might be possible to have a planar dumb bo, in which case, the assertion wouldn't always hold. However, I have other code here which prevents that, so I think I'll modify the commit message, add some comments, and most importantly make a patch before this which prevents dumb BOs from being planar formats. For the !image case: this assertion should also hold, but since fromPlanar can be implemented however by the DRI implementation, I don't know if it's the right thing to add assertions in that path. I'm fine with adding it, I just claim some level of ignorance. </blockquote><div> </div><div>Yes, the top one is definitely assert-worthy. The bottom one maybe not? For the bottom one, I guess I could see someone making fromPlanar as just return ref(bo->image) in which case maybe that's not safe? Then again, if the implemented it that way, all of the planes would have to have the same offset/stride so the next two entrypoints would be toast. It seems somewhat reasonable to assert for the second one too but I also must claim ignorance at this point. :-) </div></div></div></div>