<div dir="ltr">Thx for pointing that.<div> </div><div>Gentle ping, anyone wants to mark 'reviewed' this patch ?</div><div><div class="gmail_extra"> <div class="gmail_quote">On 10 November 2017 at 16:18, Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com" target="_blank">rafael.antognolli@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Nov 10, 2017 at 10:46:03AM +0000, Julien Isorce wrote: > Thx for the suggestions. > > Anyone familiar with _mesa_get_format_block_size and _mesa_get_format_bytes > wants to review this patch ? > > On 9 November 2017 at 17:10, Eric Engestrom <<a href="mailto:eric.engestrom@imgtec.com">eric.engestrom@imgtec.com</a>> wrote: > > On Thursday, 2017-11-09 17:03:13 +0000, Julien Isorce wrote: > > v2: add early return if (flag & MAP_INTERNAL_MASK) > > v3: take input rect into account and test with kmscube and piglit. > > v4: handle wraparound and bo reference. Just nitpick, but in case you need to respin a new version of this patch, please consider adding the changelog (v2-n) after the commit description. See other commits for some examples. Rafael <div><div class="h5"> > > Already implemented for Gallium drivers. > > > > Useful for gbm_bo_(un)map. > > > > Tests: > > By porting wayland/weston/clients/simple-dmabuf-drm.c to GBM. > > kmscube --mode=rgba > > kmscube --mode=nv12-1img > > kmscube --mode=nv12-2img > > piglit ext_image_dma_buf_import-refcount -auto > > piglit ext_image_dma_buf_import-transcode-nv12-as-r8-gr88 -auto > > piglit ext_image_dma_buf_import-sample_rgb -fmt=AR24 -alpha-one -auto > > piglit ext_image_dma_buf_import-sample_rgb -fmt=XR24 -auto > > piglit ext_image_dma_buf_import-sample_yuv -fmt=NV12 -auto > > piglit ext_image_dma_buf_import-sample_yuv -fmt=YU12 -auto > > piglit ext_image_dma_buf_import-sample_yuv -fmt=YV12 -auto > > > > Signed-off-by: Julien Isorce <<a href="mailto:jisorce@oblong.com">jisorce@oblong.com</a>> > > --- > > src/mesa/drivers/dri/i965/intel_screen.c | 75 > +++++++++++++++++++++++++++++++- > > 1 file changed, 73 insertions(+), 2 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/ > dri/i965/intel_screen.c > > index cdc36ad..6074ee8 100644 > > --- a/src/mesa/drivers/dri/i965/intel_screen.c > > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > > @@ -755,6 +755,77 @@ intel_create_image(__DRIscreen *dri_screen, > > loaderPrivate); > > } > > > > +static void * > > +intel_map_image(__DRIcontext *context, __DRIimage *image, > > + int x0, int y0, int width, int height, > > + unsigned int flags, int *stride, void **map_info) > > +{ > > + struct brw_context *brw = NULL; > > + struct brw_bo *bo = NULL; > > + void *raw_data = NULL; > > + > > + if (!context || !image || !stride || !map_info || *map_info) > > + return NULL; > > + > > + if (x0 < 0 || x0 > image->width || width > image->width - x0) > > + return NULL; > > + > > + if (y0 < 0 || y0 > image->height || height > image->height - y0) > > + return NULL; > > + > > + if (flags & MAP_INTERNAL_MASK) > > + return NULL; > > + > > + brw = context->driverPrivate; > > + bo = image->bo; > > + > > + assert(brw); > > + assert(bo); > > + > > + brw_bo_reference(bo); > > + > > + /* DRI flags and GL_MAP.*_BIT flags are the same, so just pass them > on. */ > > + raw_data = brw_bo_map(brw, bo, flags); > > + > > + if (raw_data) { > > + GLuint pix_w = 1; > > + GLuint pix_h = 1; > > + GLint pix_bytes = 1; > > + > > + *map_info = raw_data; > > + *stride = image->pitch; > > + > > + _mesa_get_format_block_size(image->format, &pix_w, &pix_h); > > + pix_bytes = _mesa_get_format_bytes(image->format); > > + > > + assert(pix_w); > > + assert(pix_h); > > + assert(pix_bytes > 0); > > + > > + raw_data += ((x0 / pix_w) * pix_bytes) + (y0 / pix_h) * image-> > pitch; > > + } else { > > + brw_bo_unreference(bo); > > + } > > Feels really nit-picky, so don't send a new rev just for that, but: > > if (!raw_data) { > brw_bo_unreference(bo); > return NULL; > } > > /* code from the `if (raw_data)` branch */ > > that way you don't have to carry around the indentation :) > > > + > > + return raw_data; > > +} > > + > > +static void > > +intel_unmap_image(__DRIcontext *context, __DRIimage *image, void > *map_info) > > +{ > > + struct brw_bo *bo = NULL; > > + > > + if (!context || !image || !map_info) > > + return; > > + > > + bo = image->bo; > > + > > + assert(bo); > > + > > + brw_bo_unmap(bo); > > + brw_bo_unreference(bo); > > +} > > + > > static __DRIimage * > > intel_create_image_with_modifiers(__DRIscreen *dri_screen, > > int width, int height, int format, > > @@ -1305,8 +1376,8 @@ static const __DRIimageExtension > intelImageExtension = { > > .createImageFromDmaBufs = intel_create_image_from_dma_ > bufs, > > .blitImage = NULL, > > .getCapabilities = NULL, > > - .mapImage = NULL, > > - .unmapImage = NULL, > > + .mapImage = intel_map_image, > > + .unmapImage = intel_unmap_image, > > .createImageWithModifiers = intel_create_image_with_ > modifiers, > > .createImageFromDmaBufs2 = intel_create_image_from_dma_ > bufs2, > > .queryDmaBufFormats = intel_query_dma_buf_formats, > > -- > > 2.7.4 > > > > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></div>