dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox