On Tue, Mar 11, 2014 at 10:18:47AM +0200, Pohjolainen, Topi wrote:
> On Tue, Mar 11, 2014 at 08:59:46AM +0100, Gwenole Beauchesne wrote:
> > Hi,
> >
> > 2014-03-11 7:37 GMT+01:00 Pohjolainen, Topi <topi.pohjolai...@intel.com>:
> > > On Mon, Mar 10, 2014 at 05:36:17PM +0100, Gwenole Beauchesne wrote:
> > >> Fix eglCreateImage() from a packed dma_buf surface with a non-zero offset
> > >> to pixels data. In particular, this fixes support for planar YUV surfaces
> > >> when they are individually mapped on a per-plane basis, i.e. when the
> > >> OES_EGL_image_external is not used and user application wants to use its
> > >> own shader code for composition, or processing on individual plane (OCL).
> > >>
> > >> Signed-off-by: Gwenole Beauchesne <gwenole.beauche...@intel.com>
> > >> ---
> > >> src/mesa/drivers/dri/i965/intel_screen.c | 9 +++++++++
> > >> 1 file changed, 9 insertions(+)
> > >>
> > >> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> > >> b/src/mesa/drivers/dri/i965/intel_screen.c
> > >> index 05cf6b1..81a8330 100644
> > >> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > >> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > >> @@ -651,6 +651,7 @@ intel_create_image_from_fds(__DRIscreen *screen,
> > >> {
> > >> struct intel_screen *intelScreen = screen->driverPrivate;
> > >> struct intel_image_format *f;
> > >> + uint32_t mask_x, mask_y;
> > >> __DRIimage *image;
> > >> int i, index;
> > >>
> > >> @@ -684,6 +685,14 @@ intel_create_image_from_fds(__DRIscreen *screen,
> > >> image->strides[index] = strides[index];
> > >> }
> > >>
> > >> + if (f->nplanes == 1) {
> > >> + image->offset = image->offsets[0];
> > >
> > > I haven't thought through all the uses of '__DRIimage' but I'm a bit
> > > worried
> > > that there is now the offset possibly twice - in the base and again in the
> > > array describing the planes. I wonder if the real bug is that the offsets
> > > in
> > > the array are not used in the right place(s):
> > >
> > > In intel_image_target_texture_2d() the call:
> > >
> > > intel_set_texture_image_region(ctx, texImage, image->region,
> > > target, image->internal_format,
> > > image->format, image->offset,
> > > image->width, image->height,
> > > image->tile_x, image->tile_y);
> > >
> > > should probably take the plane offsets into account even now when only
> > > single
> > > plane images are accepted.
> > >
> > > Would this make sense?
> > >
> >
> > Yes, or removing image->offset altogether and exclusively use
> > image->offsets[]?
>
> And now muddying the waters even more. Maybe we should just throw away the
> plane array until there is really some support for using it. I added it when I
> was working on the image external extension and planar YUV support. In the end
> there was no users for that, and the common opinion was not to introduce any
> planar support that the GPU couldn't handle natively.
> Hence I start to feel that you did the right thing - only in addition we
> should
> get rid of the planar support in '__DRIimage' as there is no backend for it
> anyway. And if and when any GPU native support is introduced in the backend
> then this could be re-introduced properly.
Just checked this a bit more carefully. The original user of planar support is
of course there (wayland and fromPlanar()), and the dma-buf extension needs to
co-exist along side with it. So the last idea is just noise, sorry for that.
>
> Eric, Ian, do you have any take on this?
>
> >
> > >> + intel_region_get_tile_masks(image->region, &mask_x, &mask_y,
> > >> false);
> > >> + if (image->offset & mask_x)
> > >> + _mesa_warning(NULL,
> > >> + "intel_create_image_from_fds: offset not on tile
> > >> boundary");
> > >> + }
> > >> +
> > >> intel_setup_image_from_dimensions(image);
> > >>
> > >> return image;
> > >> --
> > >> 1.7.9.5
> > >>
> > >> _______________________________________________
> > >> mesa-dev mailing list
> > >> mesa-dev@lists.freedesktop.org
> > >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
