From: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
To: "Kristian Høgsberg" <hoegsberg@gmail.com>
Cc: mesa3d-dev@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 3/8] dri/intel: Add explicit size parameter to intel_region_alloc_for_fd
Date: Thu, 07 Nov 2013 16:17:47 +1100 [thread overview]
Message-ID: <1383801467.26497.16.camel@RedTail> (raw)
In-Reply-To: <20131105222300.GA5220@tokamak.local>
[-- Attachment #1.1: Type: text/plain, Size: 6622 bytes --]
On Tue, 2013-11-05 at 14:23 -0800, Kristian Høgsberg wrote:
> On Mon, Nov 04, 2013 at 06:23:23PM -0800, Keith Packard wrote:
> > Instead of assuming that the size will be height * pitch, have the caller pass
> > in the size explicitly.
> >
> > Signed-off-by: Keith Packard <keithp@keithp.com>
> > ---
> > src/mesa/drivers/dri/i915/intel_regions.c | 4 ++--
> > src/mesa/drivers/dri/i915/intel_regions.h | 2 +-
> > src/mesa/drivers/dri/i915/intel_screen.c | 2 +-
> > src/mesa/drivers/dri/i965/intel_regions.c | 4 ++--
> > src/mesa/drivers/dri/i965/intel_regions.h | 1 +
> > src/mesa/drivers/dri/i965/intel_screen.c | 2 +-
> > 6 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i915/intel_regions.c b/src/mesa/drivers/dri/i915/intel_regions.c
> > index 44f7030..9f5b89e 100644
> > --- a/src/mesa/drivers/dri/i915/intel_regions.c
> > +++ b/src/mesa/drivers/dri/i915/intel_regions.c
> > @@ -209,6 +209,7 @@ struct intel_region *
> > intel_region_alloc_for_fd(struct intel_screen *screen,
> > GLuint cpp,
> > GLuint width, GLuint height, GLuint pitch,
> > + GLuint size,
> > int fd, const char *name)
> > {
> > struct intel_region *region;
> > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen,
> > int ret;
> > uint32_t bit_6_swizzle, tiling;
> >
> > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr,
> > - fd, height * pitch);
> > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size);
>
> The 3.12 kernel let's you get the bo size from lseek on the dma_buf
> fd. I added libdrm support for getting the size that, and if that
> works, it overrides the user provided size:
>
> http://cgit.freedesktop.org/mesa/drm/commit/?id=9c52c3dc4763336884277d8005eac7e6efb77600
>
> 3.12 is the first kernel where dma_buf fd passing works reliably
> anyway and the first kernel with render-nodes, so it's not worth the
> trouble to try to make this work for older kernels.
>
> Regardless, this patchs looks good.
>
> Reviewed-by: Kristian Høgsberg <krh@bitplanet.net>
>
> > if (buffer == NULL)
> > return NULL;
> > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle);
> > diff --git a/src/mesa/drivers/dri/i915/intel_regions.h b/src/mesa/drivers/dri/i915/intel_regions.h
> > index 5c612a9..6bc4a42 100644
> > --- a/src/mesa/drivers/dri/i915/intel_regions.h
> > +++ b/src/mesa/drivers/dri/i915/intel_regions.h
> > @@ -91,7 +91,7 @@ struct intel_region *
> > intel_region_alloc_for_fd(struct intel_screen *screen,
> > GLuint cpp,
> > GLuint width, GLuint height, GLuint pitch,
> > - int fd, const char *name);
> > + GLuint size, int fd, const char *name);
> >
> > bool
> > intel_region_flink(struct intel_region *region, uint32_t *name);
> > diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c
> > index 3f54752..085e894 100644
> > --- a/src/mesa/drivers/dri/i915/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i915/intel_screen.c
> > @@ -653,7 +653,7 @@ intel_create_image_from_fds(__DRIscreen *screen,
> > return NULL;
> >
> > image->region = intel_region_alloc_for_fd(intelScreen,
> > - 1, width, height,
> > + 1, width, height, height * strides[0],
> > strides[0], fds[0], "image");
You've presumably noticed this already, but this is the wrong way round
- you're passing height * stride as pitch, and stride as size. This
makes for awesome rendering.
> > if (image->region == NULL) {
> > free(image);
> > diff --git a/src/mesa/drivers/dri/i965/intel_regions.c b/src/mesa/drivers/dri/i965/intel_regions.c
> > index a6b80fd..3920f4f 100644
> > --- a/src/mesa/drivers/dri/i965/intel_regions.c
> > +++ b/src/mesa/drivers/dri/i965/intel_regions.c
> > @@ -209,6 +209,7 @@ struct intel_region *
> > intel_region_alloc_for_fd(struct intel_screen *screen,
> > GLuint cpp,
> > GLuint width, GLuint height, GLuint pitch,
> > + GLuint size,
> > int fd, const char *name)
> > {
> > struct intel_region *region;
> > @@ -216,8 +217,7 @@ intel_region_alloc_for_fd(struct intel_screen *screen,
> > int ret;
> > uint32_t bit_6_swizzle, tiling;
> >
> > - buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr,
> > - fd, height * pitch);
> > + buffer = drm_intel_bo_gem_create_from_prime(screen->bufmgr, fd, size);
> > if (buffer == NULL)
> > return NULL;
> > ret = drm_intel_bo_get_tiling(buffer, &tiling, &bit_6_swizzle);
> > diff --git a/src/mesa/drivers/dri/i965/intel_regions.h b/src/mesa/drivers/dri/i965/intel_regions.h
> > index f08a113..05dfef3 100644
> > --- a/src/mesa/drivers/dri/i965/intel_regions.h
> > +++ b/src/mesa/drivers/dri/i965/intel_regions.h
> > @@ -92,6 +92,7 @@ struct intel_region *
> > intel_region_alloc_for_fd(struct intel_screen *screen,
> > GLuint cpp,
> > GLuint width, GLuint height, GLuint pitch,
> > + GLuint size,
> > int fd, const char *name);
> >
> > bool
> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> > index ce8124b..b89b1a5 100644
> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> > @@ -718,7 +718,7 @@ intel_create_image_from_fds(__DRIscreen *screen,
> > return NULL;
> >
> > image->region = intel_region_alloc_for_fd(intelScreen,
> > - 1, width, height,
> > + 1, width, height, height * strides[0],
> > strides[0], fds[0], "image");
> > if (image->region == NULL) {
> > free(image);
> > --
> > 1.8.4.2
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2013-11-07 5:18 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-05 2:23 [PATCH 0/8] Add DRIimage-based DRI3/Present loader Keith Packard
2013-11-05 2:23 ` [PATCH 1/8] drivers/dri/common: A few dri2 functions are not actually DRI2 specific Keith Packard
2013-11-05 2:23 ` [PATCH 2/8] dri/intel: Split out DRI2 buffer update code to separate function Keith Packard
2013-11-05 2:23 ` [PATCH 3/8] dri/intel: Add explicit size parameter to intel_region_alloc_for_fd Keith Packard
2013-11-05 22:23 ` Kristian Høgsberg
2013-11-06 0:52 ` Keith Packard
2013-11-07 5:17 ` Christopher James Halse Rogers [this message]
2013-11-07 5:42 ` Keith Packard
2013-11-05 2:23 ` [PATCH 4/8] Define __DRI_IMAGE_FORMAT_SARGB8 Keith Packard
2013-11-05 2:23 ` [PATCH 5/8] dri/common: Add functions mapping MESA_FORMAT_* <-> __DRI_IMAGE_FORMAT_* Keith Packard
2013-11-05 3:01 ` Jordan Justen
2013-11-05 4:11 ` Keith Packard
2013-11-05 22:53 ` Jordan Justen
2013-11-05 22:35 ` Kristian Høgsberg
2013-11-06 0:54 ` Keith Packard
2013-11-05 2:23 ` [PATCH 6/8] dri/i915, dri/i965: Use driGLFormatToImageFormat and driImageFormatToGLFormat Keith Packard
2013-11-05 22:37 ` [PATCH 6/8] dri/i915,dri/i965: " Kristian Høgsberg
2013-11-05 2:23 ` [PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension Keith Packard
2013-11-05 20:05 ` Eric Anholt
2013-11-05 23:47 ` Keith Packard
2013-11-05 22:59 ` Kristian Høgsberg
2013-11-06 0:59 ` Keith Packard
2013-11-06 2:48 ` Kristian Høgsberg
2013-11-06 6:25 ` Kristian Høgsberg
2013-11-06 14:55 ` Keith Packard
2013-11-06 16:17 ` Kristian Høgsberg
2013-11-06 18:09 ` Keith Packard
2013-11-06 19:06 ` Kristian Høgsberg
2013-11-06 19:29 ` Keith Packard
2013-11-05 2:23 ` [PATCH 8/8] Add DRI3+Present loader Keith Packard
2013-11-05 23:10 ` Eric Anholt
2013-11-06 2:32 ` Keith Packard
2013-11-05 16:40 ` [PATCH 0/8] Add DRIimage-based DRI3/Present loader Keith Packard
2013-11-05 20:04 ` Eric Anholt
2013-11-05 22:09 ` Kristian Høgsberg
2013-11-05 23:54 ` Keith Packard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1383801467.26497.16.camel@RedTail \
--to=christopher.halse.rogers@canonical.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hoegsberg@gmail.com \
--cc=mesa3d-dev@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.