[Mesa-dev] [PATCH 1/3] drm_driver: Add a configuration function to the driver descriptor.
Adds a possibility for the state tracker manager to query the target for a specific configuration. Signed-off-by: Thomas Hellstrom --- src/gallium/include/state_tracker/drm_driver.h| 47 - src/gallium/targets/dri-i915/target.c |2 +- src/gallium/targets/dri-i965/target.c |2 +- src/gallium/targets/dri-nouveau/target.c |2 +- src/gallium/targets/dri-r300/target.c |2 +- src/gallium/targets/dri-r600/target.c |2 +- src/gallium/targets/dri-vmwgfx/target.c |2 +- src/gallium/targets/gbm/pipe_i915.c |2 +- src/gallium/targets/gbm/pipe_i965.c |2 +- src/gallium/targets/gbm/pipe_nouveau.c|2 +- src/gallium/targets/gbm/pipe_r300.c |2 +- src/gallium/targets/gbm/pipe_r600.c |2 +- src/gallium/targets/gbm/pipe_swrast.c |2 +- src/gallium/targets/gbm/pipe_vmwgfx.c |2 +- src/gallium/targets/va-r300/target.c |2 +- src/gallium/targets/va-r600/target.c |2 +- src/gallium/targets/vdpau-r300/target.c |2 +- src/gallium/targets/vdpau-r600/target.c |2 +- src/gallium/targets/xa-vmwgfx/vmw_target.c|2 +- src/gallium/targets/xorg-i915/intel_target.c |2 +- src/gallium/targets/xorg-i965/intel_target.c |2 +- src/gallium/targets/xorg-nouveau/nouveau_target.c |2 +- src/gallium/targets/xorg-r300/target.c|2 +- src/gallium/targets/xorg-r600/target.c|2 +- src/gallium/targets/xvmc-nouveau/target.c |2 +- src/gallium/targets/xvmc-r300/target.c|2 +- src/gallium/targets/xvmc-r600/target.c|2 +- 27 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/gallium/include/state_tracker/drm_driver.h b/src/gallium/include/state_tracker/drm_driver.h index d94c1e6..2df2859 100644 --- a/src/gallium/include/state_tracker/drm_driver.h +++ b/src/gallium/include/state_tracker/drm_driver.h @@ -35,6 +35,40 @@ struct winsys_handle unsigned stride; }; + + +/** + * Configuration queries. + */ +enum drm_conf { + /* How many frames to allow before throttling. Or -1 to indicate any number */ + DRM_CONF_THROTTLE, /* DRM_CONF_INT. */ + DRM_CONF_MAX +}; + +/** + * Type of configuration answer + */ +enum drm_conf_type { + DRM_CONF_INT, + DRM_CONF_BOOL, + DRM_CONF_FLOAT, + DRM_CONF_POINTER +}; + +/** + * Return value from the configuration function. + */ +struct drm_conf_ret { + enum drm_conf_type type; + union { + int val_int; + bool val_bool; + float val_float; + void *val_pointer; + } val; +}; + struct drm_driver_descriptor { /** @@ -54,6 +88,16 @@ struct drm_driver_descriptor * For example wrapping trace or rbug debugging drivers around it. */ struct pipe_screen* (*create_screen)(int drm_fd); + + + /** +* Return a configuration value. +* +* If this function is NULL, or if it returns NULL +* the state tracker- or state +* tracker manager should provide a reasonable default value. +*/ + const struct drm_conf_ret *(*configuration) (enum drm_conf conf); }; extern struct drm_driver_descriptor driver_descriptor; @@ -61,11 +105,12 @@ extern struct drm_driver_descriptor driver_descriptor; /** * Instantiate a drm_driver_descriptor struct. */ -#define DRM_DRIVER_DESCRIPTOR(name_str, driver_name_str, func) \ +#define DRM_DRIVER_DESCRIPTOR(name_str, driver_name_str, func, conf) \ struct drm_driver_descriptor driver_descriptor = { \ .name = name_str, \ .driver_name = driver_name_str, \ .create_screen = func, \ + .configuration = (conf), \ }; #endif diff --git a/src/gallium/targets/dri-i915/target.c b/src/gallium/targets/dri-i915/target.c index a27b7bd..935eb0e 100644 --- a/src/gallium/targets/dri-i915/target.c +++ b/src/gallium/targets/dri-i915/target.c @@ -26,4 +26,4 @@ create_screen(int fd) return screen; } -DRM_DRIVER_DESCRIPTOR("i915", "i915", create_screen) +DRM_DRIVER_DESCRIPTOR("i915", "i915", create_screen, NULL) diff --git a/src/gallium/targets/dri-i965/target.c b/src/gallium/targets/dri-i965/target.c index 0632b97..0434063 100644 --- a/src/gallium/targets/dri-i965/target.c +++ b/src/gallium/targets/dri-i965/target.c @@ -26,4 +26,4 @@ create_screen(int fd) return screen; } -DRM_DRIVER_DESCRIPTOR("i915", "i965", create_screen) +DRM_DRIVER_DESCRIPTOR("i915", "i965", create_screen, NULL) diff --git a/src/gallium/targets/dri-nouveau/target.c b/src/gallium/targets/dri-nouveau/target.c index e725a4d..c0d7f92 100644 --- a/src/gallium/targets/dri-nouveau/target.c +++ b/src/gallium/targets/dri-nouveau/target.c @@ -17,4 +17,4 @@ create_screen(int fd) return s
[Mesa-dev] [PATCH 2/3] st/dri: Hook up throttling based on the drm driver_descriptor configuration
Hooks up throttling if there is a configuration function present and it indicates that throttling is desired. Signed-off-by: Thomas Hellstrom --- .../state_trackers/dri/common/dri_drawable.c |4 ++- src/gallium/state_trackers/dri/common/dri_screen.h |1 + src/gallium/state_trackers/dri/drm/dri2.c | 24 +++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/common/dri_drawable.c b/src/gallium/state_trackers/dri/common/dri_drawable.c index ca9e130..340404e 100644 --- a/src/gallium/state_trackers/dri/common/dri_drawable.c +++ b/src/gallium/state_trackers/dri/common/dri_drawable.c @@ -137,7 +137,9 @@ dri_create_buffer(__DRIscreen * sPriv, drawable->screen = screen; drawable->sPriv = sPriv; drawable->dPriv = dPriv; - drawable->desired_fences = 2; + drawable->desired_fences = screen->default_throttle_frames; + if (drawable->desired_fences > DRI_SWAP_FENCES_MAX) + drawable->desired_fences = DRI_SWAP_FENCES_MAX; dPriv->driverPrivate = (void *)drawable; p_atomic_set(&drawable->base.stamp, 1); diff --git a/src/gallium/state_trackers/dri/common/dri_screen.h b/src/gallium/state_trackers/dri/common/dri_screen.h index 8cb0a10..4e865b9 100644 --- a/src/gallium/state_trackers/dri/common/dri_screen.h +++ b/src/gallium/state_trackers/dri/common/dri_screen.h @@ -54,6 +54,7 @@ struct dri_screen /* dri */ __DRIscreen *sPriv; + int default_throttle_frames; /** * Configuration cache with default values for all contexts diff --git a/src/gallium/state_trackers/dri/drm/dri2.c b/src/gallium/state_trackers/dri/drm/dri2.c index 2b6919d..f3c9e10 100644 --- a/src/gallium/state_trackers/dri/drm/dri2.c +++ b/src/gallium/state_trackers/dri/drm/dri2.c @@ -623,6 +623,19 @@ static const __DRIextension *dri_screen_extensions[] = { NULL }; +static const __DRIextension *dri_screen_extensions_throttle[] = { + &driReadDrawableExtension, + &driCopySubBufferExtension.base, + &driSwapControlExtension.base, + &driMediaStreamCounterExtension.base, + &driTexBufferExtension.base, + &dri2FlushExtension.base, + &dri2ImageExtension.base, + &dri2ConfigQueryExtension.base, + &dri2ThrottleExtension.base, + NULL +}; + /** * This is the driver specific part of the createNewScreen entry point. * @@ -634,6 +647,7 @@ dri2_init_screen(__DRIscreen * sPriv) const __DRIconfig **configs; struct dri_screen *screen; struct pipe_screen *pscreen; + const struct drm_conf_ret *throttle_ret = NULL; screen = CALLOC_STRUCT(dri_screen); if (!screen) @@ -643,9 +657,17 @@ dri2_init_screen(__DRIscreen * sPriv) screen->fd = sPriv->fd; sPriv->private = (void *)screen; - sPriv->extensions = dri_screen_extensions; pscreen = driver_descriptor.create_screen(screen->fd); + if (driver_descriptor.configuration) + throttle_ret = driver_descriptor.configuration(DRM_CONF_THROTTLE); + + if (throttle_ret && throttle_ret->val.val_int != -1) { + sPriv->extensions = dri_screen_extensions_throttle; + screen->default_throttle_frames = throttle_ret->val.val_int; + } else + sPriv->extensions = dri_screen_extensions; + /* dri_init_screen_helper checks pscreen for us */ configs = dri_init_screen_helper(screen, pscreen, 32); -- 1.7.4.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] dri-vmwgfx: Hook up a drm_descriptor configuration function
Returns a configuration that makes the dri state-tracker-manager throttle. Also disable kernel-based throttling. Signed-off-by: Thomas Hellstrom --- src/gallium/targets/dri-vmwgfx/target.c | 20 ++-- 1 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/gallium/targets/dri-vmwgfx/target.c b/src/gallium/targets/dri-vmwgfx/target.c index fe3f8fd..442e31b 100644 --- a/src/gallium/targets/dri-vmwgfx/target.c +++ b/src/gallium/targets/dri-vmwgfx/target.c @@ -19,7 +19,7 @@ create_screen(int fd) if (!screen) return NULL; - vmw_winsys_screen_set_throttling(screen, 10); + vmw_winsys_screen_set_throttling(screen, 0); screen = sw_screen_wrap(screen); screen = debug_screen_wrap(screen); @@ -27,4 +27,20 @@ create_screen(int fd) return screen; } -DRM_DRIVER_DESCRIPTOR("vmwgfx", "vmwgfx", create_screen, NULL) +static const struct drm_conf_ret throttle_ret = { + .type = DRM_CONF_INT, + .val.val_int = 2, +}; + +static const struct drm_conf_ret *drm_configuration(enum drm_conf conf) +{ + switch (conf) { + case DRM_CONF_THROTTLE: + return &throttle_ret; + default: + break; + } + return NULL; +} + +DRM_DRIVER_DESCRIPTOR("vmwgfx", "vmwgfx", create_screen, drm_configuration) -- 1.7.4.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] dri2: Implement a throttle dri extension.
Thanks for reviewing, Michel. On 10/11/2011 05:29 PM, Michel Dänzer wrote: On Die, 2011-10-11 at 15:44 +0200, Thomas Hellstrom wrote: The X server has limited throttle support on the server side, but doing this in the client has some benefits: 1) X server throttling is per client. Client side throttling can be done per drawable. 2) It's easier to control the throttling based on what client is run, for example using "driconf". 3) X server throttling requires drm swap complete events. So implement a dri2 throttling extension intended to be used by direct rendering clients. I'm on the fence about whether it's better to add a new extension for this or to add anything missing to the flush extension instead, as the callsites are basically the same. I agree. What made me favor a new extension was that we'd have to add a new function anyway, since we pass more argument. Also the throttling extension will most likely never be called from AIGLX, since that would stall the server. @@ -390,6 +393,15 @@ dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, int width, int height) (*psc->f->flush) (priv->driDrawable); #endif + if (psc->throttle) { + struct glx_context *gc = __glXGetCurrentContext(); + struct dri2_context *dri2Ctx = (struct dri2_context *)gc; + __DRIcontext *ctx = +(dri2Ctx) ? dri2Ctx->driContext : NULL; + + psc->throttle->throttle(ctx, priv->driDrawable, reason); + } + Either way though, these blocks should probably be refactored into a helper function. Sure. I'll take care of that. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] dri2: Implement a throttle dri extension.
On Mit, 2011-10-12 at 11:35 +0200, Thomas Hellstrom wrote: > Thanks for reviewing, Michel. > On 10/11/2011 05:29 PM, Michel Dänzer wrote: > > On Die, 2011-10-11 at 15:44 +0200, Thomas Hellstrom wrote: > > > >> The X server has limited throttle support on the server side, > >> but doing this in the client has some benefits: > >> > >> 1) X server throttling is per client. Client side throttling can be done > >> per drawable. > >> > >> 2) It's easier to control the throttling based on what client is run, > >> for example using "driconf". > >> > >> 3) X server throttling requires drm swap complete events. > >> > >> So implement a dri2 throttling extension intended to be used by direct > >> rendering clients. > >> > > I'm on the fence about whether it's better to add a new extension for > > this or to add anything missing to the flush extension instead, as the > > callsites are basically the same. > > > > I agree. What made me favor a new extension was that we'd have to add a > new function anyway, since > we pass more argument. Also the throttling extension will most likely > never be called from AIGLX, since > that would stall the server. Ah, right. I'm convinced. :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 41715] New: Crash in update_vertex_textures at state_tracker/st_atom_texture.c:285
https://bugs.freedesktop.org/show_bug.cgi?id=41715 Summary: Crash in update_vertex_textures at state_tracker/st_atom_texture.c:285 Product: Mesa Version: git Platform: x86-64 (AMD64) OS/Version: Linux (All) Status: NEW Severity: critical Priority: medium Component: Mesa core AssignedTo: mesa-dev@lists.freedesktop.org ReportedBy: jlp.b...@gmail.com Everytime I try to run an OpenGL app I get a "Program received signal SIGSEGV, Segmentation fault error." Even if I try to run glxgears I get this. I've compiled glxgears and mesa with debug info and this is what I get: [New Thread 0x73a39700 (LWP 2134)] Running synchronized to the vertical refresh. The framerate should be approximately the same as the monitor refresh rate. Program received signal SIGSEGV, Segmentation fault. 0x in ?? () (gdb) bt #0 0x in ?? () #1 0x746d62fd in update_vertex_textures (st=0x72bdb0) at state_tracker/st_atom_texture.c:285 #2 0x746d2c4f in st_validate_state (st=0x72bdb0) at state_tracker/st_atom.c:197 #3 0x746d946a in st_Clear (ctx=0x6c7550, mask=18) at state_tracker/st_cb_clear.c:499 #4 0x00401c9e in draw () at src/xdemos/glxgears.c:253 #5 0x00403332 in draw_gears () at src/xdemos/glxgears.c:315 #6 draw_frame (win=83886082, dpy=0x606010) at src/xdemos/glxgears.c:340 #7 event_loop (win=83886082, dpy=0x606010) at src/xdemos/glxgears.c:696 #8 main (argc=, argv=) at src/xdemos/glxgears.c:776 The hardware is: 01:05.0 VGA compatible controller: ATI Technologies Inc RS482 [Radeon Xpress 200M] Mesa is from git (ae1153c) -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] dri2: Implement a throttle dri extension v2
The X server has limited throttle support on the server side, but doing this in the client has some benefits: 1) X server throttling is per client. Client side throttling can be done per drawable. 2) It's easier to control the throttling based on what client is run, for example using "driconf". 3) X server throttling requires drm swap complete events. So implement a dri2 throttling extension intended to be used by direct rendering clients. Signed-off-by: Thomas Hellstrom --- include/GL/internal/dri_interface.h | 22 ++ src/glx/dri2_glx.c | 55 -- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index 8a9ca19..4f768f0 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -84,6 +84,7 @@ typedef struct __DRIbufferRec __DRIbuffer; typedef struct __DRIdri2ExtensionRec __DRIdri2Extension; typedef struct __DRIdri2LoaderExtensionRec __DRIdri2LoaderExtension; typedef struct __DRI2flushExtensionRec __DRI2flushExtension; +typedef struct __DRI2throttleExtensionRec __DRI2throttleExtension; /*@}*/ @@ -284,6 +285,27 @@ struct __DRI2flushExtensionRec { /** + * Extension that the driver uses to request + * throttle callbacks. + */ + +#define __DRI2_THROTTLE "DRI2_Throttle" +#define __DRI2_THROTTLE_VERSION 1 + +enum __DRI2throttleReason { + __DRI2_THROTTLE_SWAPBUFFER, + __DRI2_THROTTLE_COPYSUBBUFFER, + __DRI2_THROTTLE_FLUSHFRONT +}; + +struct __DRI2throttleExtensionRec { + __DRIextension base; + void (*throttle)(__DRIcontext *ctx, + __DRIdrawable *drawable, + enum __DRI2throttleReason reason); +}; + +/** * XML document describing the configuration options supported by the * driver. */ diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c index 01e3fd6..ef1d0d0 100644 --- a/src/glx/dri2_glx.c +++ b/src/glx/dri2_glx.c @@ -85,6 +85,7 @@ struct dri2_screen { const __DRI2flushExtension *f; const __DRI2configQueryExtension *config; const __DRItexBufferExtension *texBuffer; + const __DRI2throttleExtension *throttle; const __DRIconfig **driver_configs; void *driver; @@ -368,8 +369,32 @@ dri2WaitForSBC(__GLXDRIdrawable *pdraw, int64_t target_sbc, int64_t *ust, #endif /* X_DRI2WaitMSC */ +/** + * dri2Throttle - Request driver throttling + * + * This function uses the DRI2 throttle extension to give the + * driver the opportunity to throttle on flush front, copysubbuffer + * and swapbuffers. + */ static void -dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, int width, int height) +dri2Throttle(struct dri2_screen *psc, +struct dri2_drawable *draw, +enum __DRI2throttleReason reason) +{ + if (psc->throttle) { + struct glx_context *gc = __glXGetCurrentContext(); + struct dri2_context *dri2Ctx = (struct dri2_context *)gc; + __DRIcontext *ctx = +(dri2Ctx) ? dri2Ctx->driContext : NULL; + + psc->throttle->throttle(ctx, draw->driDrawable, reason); + } +} + +static void +__dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, + int width, int height, + enum __DRI2throttleReason reason) { struct dri2_drawable *priv = (struct dri2_drawable *) pdraw; struct dri2_screen *psc = (struct dri2_screen *) pdraw->psc; @@ -389,7 +414,9 @@ dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, int width, int height) if (psc->f) (*psc->f->flush) (priv->driDrawable); #endif - + + dri2Throttle(psc, priv, reason); + region = XFixesCreateRegion(psc->base.dpy, &xrect, 1); DRI2CopyRegion(psc->base.dpy, pdraw->xDrawable, region, DRI2BufferFrontLeft, DRI2BufferBackLeft); @@ -405,6 +432,15 @@ dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, int width, int height) } static void +dri2CopySubBuffer(__GLXDRIdrawable *pdraw, int x, int y, + int width, int height) +{ + __dri2CopySubBuffer(pdraw, x, y, width, height, + __DRI2_THROTTLE_COPYSUBBUFFER); +} + + +static void dri2_copy_drawable(struct dri2_drawable *priv, int dest, int src) { XRectangle xrect; @@ -458,6 +494,7 @@ dri2FlushFrontBuffer(__DRIdrawable *driDrawable, void *loaderPrivate) struct dri2_display *pdp; struct glx_context *gc; struct dri2_drawable *pdraw = loaderPrivate; + struct dri2_screen *psc; if (!pdraw) return; @@ -465,10 +502,14 @@ dri2FlushFrontBuffer(__DRIdrawable *driDrawable, void *loaderPrivate) if (!pdraw->base.psc) return; - priv = __glXInitialize(pdraw->base.psc->dpy); + psc = (struct dri2_screen *) pdraw->base.psc; + + priv = __glXInitialize(psc->base.dpy); pdp = (struct dri2_display *) priv->dri2Display; gc = __glXGetCurrentContext(); + dri2Throttle(psc, pdraw, __DRI2_THROTTLE_FLUSHFRONT); +
Re: [Mesa-dev] [PATCH 1/2] swrast: Fix fastpaths in glRead/WritePixels(GL_DEPTH_STENCIL)
On 10/11/2011 09:57 PM, Eric Anholt wrote: On Mon, 10 Oct 2011 18:10:59 -0700, Chad Versace wrote: For glReadPixels, the user supplied pixels have format GL_UNSIGNED_INT_24_8. But, when the depthstencil buffer's format was MESA_FORMAT_S8_Z24, the fastpath read from the buffer without reordering the depth and stencil bits. To fix this, this patch just skips the fastpath when the format is not MESA_FORMAT_Z24_S8. The problem and fix for glWritePixels is analagous. Fixes the Piglit tests below on i965/gen6 and causes no regressions. general/depthstencil-default_fb-drawpixels-24_8 general/depthstencil-default_fb-readpixels-24_8 EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-drawpixels-24_8 EXT_packed_depth_stencil/fbo-depthstencil-GL_DEPTH24_STENCIL8-readpixels-24_8 Signed-off-by: Chad Versace This code is all ridiculous -- while this fixes the problem, the real issue is that GetRow is returning GL_UNSIGNED_INT_24_8 data whose ordering actually depends on the underlying renderbuffer format. That's not how I understand GetRow is supposed to work. But we all hate GetRow and friends, I think, so I've started on a branch doing to renderbuffers what was done to textures with MapTextureImage. It's in the rbmap branch of my Mesa tree. Non-intel swrast users are broken by it for now. I was going to tackle renderbuffers next but I'm happy to let you do it. :) I took a quick look at your branch it looks great so far. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag
On 10/10/2011 01:53 PM, Christoph Bumiller wrote: This is required for d3d1x's CheckFormatSupport query. It also seems generally useful for state trackers, which could choose alternative rendering paths or formats if blending would come at a significant performance loss. --- src/gallium/docs/source/screen.rst |3 +++ src/gallium/include/pipe/p_defines.h |9 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst index 924858e..4a61835 100644 --- a/src/gallium/docs/source/screen.rst +++ b/src/gallium/docs/source/screen.rst @@ -160,6 +160,9 @@ resources might be created and handled quite differently. * ``PIPE_BIND_DEPTH_STENCIL``: A depth (Z) buffer and/or stencil buffer. Any depth/stencil surface/resource attached to pipe_framebuffer_state::zsbuf must have this flag set. +* ``PIPE_BIND_BLENDABLE``: Used in conjunction with PIPE_BIND_RENDER_TARGET to query + whether a device supports blending for a given format. + If surface creation succeeds with this flag set, the driver must emulate blending. * ``PIPE_BIND_DISPLAY_TARGET``: A surface that can be presented to screen. Arguments to pipe_screen::flush_front_buffer must have this flag set. * ``PIPE_BIND_SAMPLER_VIEW``: A texture that may be sampled from in a fragment diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h index acae4b1..447df35 100644 --- a/src/gallium/include/pipe/p_defines.h +++ b/src/gallium/include/pipe/p_defines.h @@ -295,10 +295,11 @@ enum pipe_transfer_usage { */ #define PIPE_BIND_DEPTH_STENCIL(1<< 0) /* create_surface */ #define PIPE_BIND_RENDER_TARGET(1<< 1) /* create_surface */ -#define PIPE_BIND_SAMPLER_VIEW (1<< 2) /* create_sampler_view */ -#define PIPE_BIND_VERTEX_BUFFER(1<< 3) /* set_vertex_buffers */ -#define PIPE_BIND_INDEX_BUFFER (1<< 4) /* draw_elements */ -#define PIPE_BIND_CONSTANT_BUFFER (1<< 5) /* set_constant_buffer */ +#define PIPE_BIND_BLENDABLE(1<< 2) /* create_surface */ +#define PIPE_BIND_SAMPLER_VIEW (1<< 3) /* create_sampler_view */ +#define PIPE_BIND_VERTEX_BUFFER(1<< 4) /* set_vertex_buffers */ +#define PIPE_BIND_INDEX_BUFFER (1<< 5) /* draw_elements */ +#define PIPE_BIND_CONSTANT_BUFFER (1<< 6) /* set_constant_buffer */ #define PIPE_BIND_DISPLAY_TARGET (1<< 8) /* flush_front_buffer */ #define PIPE_BIND_TRANSFER_WRITE (1<< 9) /* get_transfer */ #define PIPE_BIND_TRANSFER_READ(1<< 10) /* get_transfer */ So do we need to go in and add PIPE_BIND_BLENDABLE to all of our existing surface-create calls in the state tracker, etc? -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag
On 12.10.2011 16:12, Brian Paul wrote: > On 10/10/2011 01:53 PM, Christoph Bumiller wrote: >> This is required for d3d1x's CheckFormatSupport query. >> >> It also seems generally useful for state trackers, which could >> choose alternative rendering paths or formats if blending would >> come at a significant performance loss. >> --- >> src/gallium/docs/source/screen.rst |3 +++ >> src/gallium/include/pipe/p_defines.h |9 + >> 2 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/src/gallium/docs/source/screen.rst >> b/src/gallium/docs/source/screen.rst >> index 924858e..4a61835 100644 >> --- a/src/gallium/docs/source/screen.rst >> +++ b/src/gallium/docs/source/screen.rst >> @@ -160,6 +160,9 @@ resources might be created and handled quite >> differently. >> * ``PIPE_BIND_DEPTH_STENCIL``: A depth (Z) buffer and/or stencil >> buffer. Any >> depth/stencil surface/resource attached to >> pipe_framebuffer_state::zsbuf must >> have this flag set. >> +* ``PIPE_BIND_BLENDABLE``: Used in conjunction with >> PIPE_BIND_RENDER_TARGET to query >> + whether a device supports blending for a given format. >> + If surface creation succeeds with this flag set, the driver must >> emulate blending. >> * ``PIPE_BIND_DISPLAY_TARGET``: A surface that can be presented to >> screen. Arguments to >> pipe_screen::flush_front_buffer must have this flag set. >> * ``PIPE_BIND_SAMPLER_VIEW``: A texture that may be sampled from in >> a fragment >> diff --git a/src/gallium/include/pipe/p_defines.h >> b/src/gallium/include/pipe/p_defines.h >> index acae4b1..447df35 100644 >> --- a/src/gallium/include/pipe/p_defines.h >> +++ b/src/gallium/include/pipe/p_defines.h >> @@ -295,10 +295,11 @@ enum pipe_transfer_usage { >>*/ >> #define PIPE_BIND_DEPTH_STENCIL(1<< 0) /* create_surface */ >> #define PIPE_BIND_RENDER_TARGET(1<< 1) /* create_surface */ >> -#define PIPE_BIND_SAMPLER_VIEW (1<< 2) /* >> create_sampler_view */ >> -#define PIPE_BIND_VERTEX_BUFFER(1<< 3) /* >> set_vertex_buffers */ >> -#define PIPE_BIND_INDEX_BUFFER (1<< 4) /* draw_elements */ >> -#define PIPE_BIND_CONSTANT_BUFFER (1<< 5) /* >> set_constant_buffer */ >> +#define PIPE_BIND_BLENDABLE(1<< 2) /* create_surface */ >> +#define PIPE_BIND_SAMPLER_VIEW (1<< 3) /* >> create_sampler_view */ >> +#define PIPE_BIND_VERTEX_BUFFER(1<< 4) /* >> set_vertex_buffers */ >> +#define PIPE_BIND_INDEX_BUFFER (1<< 5) /* draw_elements */ >> +#define PIPE_BIND_CONSTANT_BUFFER (1<< 6) /* >> set_constant_buffer */ >> #define PIPE_BIND_DISPLAY_TARGET (1<< 8) /* >> flush_front_buffer */ >> #define PIPE_BIND_TRANSFER_WRITE (1<< 9) /* get_transfer */ >> #define PIPE_BIND_TRANSFER_READ(1<< 10) /* get_transfer */ > > So do we need to go in and add PIPE_BIND_BLENDABLE to all of our > existing surface-create calls in the state tracker, etc? > Well, that depends on whether we want to put blending fallbacks at the state tracker level (OpenGL is probably the only API that requires them) or let each driver handle them internally. Right now, drivers for GPUs that don't support blending on some formats (e.g. DX10 level hardware for 16 bits per component or SNORM formats) don't care much about doing fallbacks and silently ignore that blending won't work (speaking at least for nv50 here). You pass the flag on resource creation, not surface creation, but since surface creation can specify a different format, it applies only there. I should change the wording to: "If this flag is set, surface creation may fail if blending is not supported for the specified format. If it is not set, a driver may choose to ignore blending on surfaces with formats that would require emulation." That would leave it to the individual driver to choose whether to bother with fallbacks or not. -Christoph > -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] mesa: check attachment Type field in renderbuffer_exists()
On Mon, 10 Oct 2011 21:05:33 -0600, Brian Paul wrote: > From: Brian Paul > > Instead of the renderbuffer pointer. In the future, attaching a texture > may not mean the renderbuffer pointer gets set too. > Plus, remove some commented-out assertions. I'm curious where you're going with not having a Renderbuffer for textures. I've wished that Mesa core handled making a Renderbuffer for textures, since there was this ugly driver code for cooking up a Renderbuffer even though everyone's should be basically the same. Maybe having the Renderbuffer go away will help, I'm not sure. In the rbmapping in swrast, I'm constantly wanting to know the format of fb->Attachments[WHATEVER] and get a mapping of it. I think that's the most important to have easy access to, whether it's from the Renderbuffer or helper functions on the attachment point (but it's sure easier to be able to pass a gl_renderbuffer around than fb+attachment). pgpFhTkSFsHF3.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] mesa: remove redundant buffer checks in copytexture_error_check()
On Mon, 10 Oct 2011 21:05:34 -0600, Brian Paul wrote: > From: Brian Paul > > There was already a call to _mesa_source_buffer_exists() earlier in > the function. Reviewed-by: Eric Anholt pgpCtpDmdjpXR.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] s/format/baseFormat/ to be more explicit
On Mon, 10 Oct 2011 21:05:36 -0600, Brian Paul wrote: > From: Brian Paul This and 5/6 are also Reviewed-by: Eric Anholt pgpJglcbqFLVY.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: consolidate _mesa_source/dest_buffer_exists()
On Tue, 11 Oct 2011 18:41:33 -0600, Brian Paul wrote: > From: Brian Paul > > v2: add a 'reading' parameter to distinguish between reading and writing > to the renderbuffer (we don't want to check if _ColorReadBuffer is null > when we're about to draw). Eric found this mistake. Reviewed-by: Eric Anholt pgpHtCkuYltpe.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965 Gen6+: De-compact clip plane constants for old VS backend.
On Mon, 10 Oct 2011 15:13:13 -0700, Paul Berry wrote: > In commit 018ea68d8780ab5baeef0b8122b8410e5e55ae6d, when I > de-compacted clip planes on Gen6+, I updated both the old and new VS > back-ends to reflect the change in how clip planes are stored, but I > failed to change the code in gen6_vs_state.c that uploads clip plane > constants when using the old VS back-end. > > As a result, if the set of enabled clip planes wasn't contiguous > starting with 0, then clipping would not occur properly. This patch > corrects gen6_vs_state.c to upload clip plane constants in the new > de-compacted form. > > This only affects the old VS back-end (which is used for > fixed-function and ARB vertex programs, not for GLSL vertex shaders). > > Fixes Piglit test fixed-clip-enables. Reviewed-by: Eric Anholt pgpHo4SDvAwch.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/10] i965: Document the brw_instruction Message Descriptor structures.
On Mon, 10 Oct 2011 16:31:47 -0700, Kenneth Graunke wrote: > Signed-off-by: Kenneth Graunke Reviewed-by: Eric Anholt pgpl8aUbGk0IZ.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/10] i965: Factor out code for setting Message Descriptors.
On Mon, 10 Oct 2011 16:31:49 -0700, Kenneth Graunke wrote: > Every brw_set_???_message function had duplicated code, per-generation, > to set the Message Descriptor and Extended Message Descriptor bits > (SFID, message length, response length, header present, end of thread). > > However, these fields are actually specified as part of the SEND > instruction itself; individual types of messages don't even specify > them (except for header present, but that's in the same bit location). > > Since these are exactly the same regardless of the message type, just > create a function to set them, using the generic message structs. This > not only shortens the code, but hides a lot of the per-generation > complexity (like the SFID being in destreg__conditionalmod) in one spot. > > Signed-off-by: Kenneth Graunke > --- > src/mesa/drivers/dri/i965/brw_eu_emit.c | 195 > --- > 1 files changed, 73 insertions(+), 122 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c > b/src/mesa/drivers/dri/i965/brw_eu_emit.c > index 28bd52f..29aa39b 100644 > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c > @@ -543,59 +544,47 @@ brw_set_dp_write_message(struct brw_compile *p, > { > struct brw_context *brw = p->brw; > struct intel_context *intel = &brw->intel; > - brw_set_src1(p, insn, brw_imm_ud(0)); > + unsigned sfid; > > if (intel->gen >= 7) { >/* Use the Render Cache for RT writes; otherwise use the Data Cache */ > - unsigned sfid = GEN7_SFID_DATAPORT_DATA_CACHE; >if (msg_type == GEN6_DATAPORT_WRITE_MESSAGE_RENDER_TARGET_WRITE) >sfid = GEN6_SFID_DATAPORT_RENDER_CACHE; > + else > + sfid = GEN7_SFID_DATAPORT_DATA_CACHE; > + } else if (intel->gen == 6) { > + /* Use the render cache for all write messages. */ > + sfid = GEN6_SFID_DATAPORT_RENDER_CACHE; > + } else { > + sfid = BRW_SFID_DATAPORT_WRITE; > + } Ah, I hadn't understood that this was happening when I was looking at patch 1/10 and complaining about reads from DATA_CACHE. I'm cool with that now. Both this and that are: Reviewed-by: Eric Anholt which I think makes everything but 2/10. pgpBsJsdLfKOn.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/6] mesa: check attachment Type field in renderbuffer_exists()
On 10/12/2011 10:20 AM, Eric Anholt wrote: On Mon, 10 Oct 2011 21:05:33 -0600, Brian Paul wrote: From: Brian Paul Instead of the renderbuffer pointer. In the future, attaching a texture may not mean the renderbuffer pointer gets set too. Plus, remove some commented-out assertions. I'm curious where you're going with not having a Renderbuffer for textures. At this point, it's more of a change just to reflect the GL spec. The GL spec certainly doesn't say anything about a "fake" renderbuffer being created when there's a texture attachment. So I was changing the checks there to reflect that. I've wished that Mesa core handled making a Renderbuffer for textures, since there was this ugly driver code for cooking up a Renderbuffer even though everyone's should be basically the same. At some point we might have a third type of object which is some kind of union of gl_texture_image and gl_renderbuffer (like gallium's pipe_resource). Or maybe we should just create texture images all the time instead of renderbuffers. But I really haven't begun to consider this in any depth yet. But if you think automatically creating renderbuffer wrappers for textures would be helpful, we could explore that. So far, that's only been an artifact of the way swrast did things. And I'd like to get all that baggage out of core Mesa. Maybe having the Renderbuffer go away will help, I'm not sure. In the rbmapping in swrast, I'm constantly wanting to know the format of fb->Attachments[WHATEVER] and get a mapping of it. I think that's the most important to have easy access to, whether it's from the Renderbuffer or helper functions on the attachment point (but it's sure easier to be able to pass a gl_renderbuffer around than fb+attachment). Understood. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: move gl_texture_image::Data, RowStride, ImageOffsets to swrast
On Mon, 10 Oct 2011 20:27:44 -0600, Brian Paul wrote: > From: Brian Paul > > Only swrast and the drivers that fall back to swrast need these fields now. > This removes the last of the fields related to software rendering from > gl_texture_image. > + /* Allocate the swrast_texture_image::ImageOffsets array now */ > + switch (texobj->Target) { > + case GL_TEXTURE_3D: > + case GL_TEXTURE_2D_ARRAY: > + slices = image->Depth; > + break; > + case GL_TEXTURE_1D_ARRAY: > + slices = image->Height; > + break; > + default: > + slices = 1; > + } > + assert(!intel_image->base.ImageOffsets); > + intel_image->base.ImageOffsets = malloc(slices * sizeof(GLuint)); This (and the corresponding code in the other drivers) should live in swrast with the IsPowerOfTwo and *Scale setup I think. pgp4sI1TMTTMl.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: take into account indices offset for bounds check
On Sat, 24 Sep 2011 13:08:43 +0400, Vadim Girlin wrote: > --- > src/mesa/main/api_validate.c |9 ++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c > index 1fcf5cd..9338f4c 100644 > --- a/src/mesa/main/api_validate.c > +++ b/src/mesa/main/api_validate.c > @@ -257,7 +257,8 @@ _mesa_validate_DrawElements(struct gl_context *ctx, > if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { >/* use indices in the buffer object */ >/* make sure count doesn't go outside buffer bounds */ > - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) > { > + GLbyte * indices_end = (GLbyte*)indices + index_bytes(type, count); > + if (indices_end > (GLbyte*)ctx->Array.ElementArrayBufferObj->Size) { > _mesa_warning(ctx, "glDrawElements index out of buffer bounds"); > return GL_FALSE; >} > @@ -318,7 +319,8 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, > GLenum mode, > if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { >/* use indices in the buffer object */ >/* make sure count doesn't go outside buffer bounds */ > - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) > { > + GLbyte * indices_end = (GLbyte*)indices + index_bytes(type, count); > + if (indices_end > (GLbyte*)ctx->Array.ElementArrayBufferObj->Size) { > _mesa_warning(ctx, "glDrawRangeElements index out of buffer > bounds"); > return GL_FALSE; >} > @@ -457,7 +459,8 @@ _mesa_validate_DrawElementsInstanced(struct gl_context > *ctx, > if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { >/* use indices in the buffer object */ >/* make sure count doesn't go outside buffer bounds */ > - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) > { > + GLbyte * indices_end = (GLbyte*)indices + index_bytes(type, count); > + if (indices_end > (GLbyte*)ctx->Array.ElementArrayBufferObj->Size) { > _mesa_warning(ctx, > "glDrawElementsInstanced index out of buffer bounds"); > return GL_FALSE; This index buffer checking really looks like it should be in a separate helper function. pgpIICzVmAnaT.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag
Am 12.10.2011 16:31, schrieb Christoph Bumiller: >> So do we need to go in and add PIPE_BIND_BLENDABLE to all of our >> existing surface-create calls in the state tracker, etc? >> > > Well, that depends on whether we want to put blending fallbacks at the > state tracker level (OpenGL is probably the only API that requires them) > or let each driver handle them internally. > > Right now, drivers for GPUs that don't support blending on some formats > (e.g. DX10 level hardware for 16 bits per component or SNORM formats) > don't care much about doing fallbacks and silently ignore that blending > won't work (speaking at least for nv50 here). > > You pass the flag on resource creation, not surface creation, but since > surface creation can specify a different format, it applies only there. > > I should change the wording to: > "If this flag is set, surface creation may fail if blending is not > supported for the specified format. If it is not set, a driver may > choose to ignore blending on surfaces with formats that would require > emulation." > > That would leave it to the individual driver to choose whether to bother > with fallbacks or not. How do you emulate blending in some semi-sane way? Looks impossible to me. Maybe if proprietary drivers don't do anything useful in that case (that is when they are asked to render to a surface with blending they don't support) we could just do the same. I don't consider software render fallbacks terribly useful hence my preference would be to just not ask for the PIPE_BLENDABLE flag when we don't know we need blending (which means in the OpenGL state tracker) and just ignore blending in the driver. That's not very correct though but since the whole point seems to be you want to expose formats (in OpenGL) which aren't blendable and you don't expect blending to actually happen how could we do better? Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag
On 12.10.2011 19:17, Roland Scheidegger wrote: > Am 12.10.2011 16:31, schrieb Christoph Bumiller: >>> So do we need to go in and add PIPE_BIND_BLENDABLE to all of our >>> existing surface-create calls in the state tracker, etc? >>> >> Well, that depends on whether we want to put blending fallbacks at the >> state tracker level (OpenGL is probably the only API that requires them) >> or let each driver handle them internally. >> >> Right now, drivers for GPUs that don't support blending on some formats >> (e.g. DX10 level hardware for 16 bits per component or SNORM formats) >> don't care much about doing fallbacks and silently ignore that blending >> won't work (speaking at least for nv50 here). >> >> You pass the flag on resource creation, not surface creation, but since >> surface creation can specify a different format, it applies only there. >> >> I should change the wording to: >> "If this flag is set, surface creation may fail if blending is not >> supported for the specified format. If it is not set, a driver may >> choose to ignore blending on surfaces with formats that would require >> emulation." >> >> That would leave it to the individual driver to choose whether to bother >> with fallbacks or not. > How do you emulate blending in some semi-sane way? Looks impossible to me. The best I can think of, short of software rendering (which the NV blob does), is to render each primitive separately with a texture barrier in between and performing the blend operation at the end of the fragment shader. > Maybe if proprietary drivers don't do anything useful in that case (that > is when they are asked to render to a surface with blending they don't > support) we could just do the same. I don't consider software render > fallbacks terribly useful hence my preference would be to just not ask > for the PIPE_BLENDABLE flag when we don't know we need blending (which > means in the OpenGL state tracker) and just ignore blending in the driver. That's what I'd prefer, not changing the GL state tracker at all. The presence of the BLENDABLE flag would imply strict checking of blending functionality and allow drivers to either refuse to perform it (surface creation to fail, such as with formats that are not supported at all) or have to fall back to software (guarantee that it works), while its absence merely indicates that the state tracker leaves it to the driver to do what seems fit, which is what we have right now. > That's not very correct though but since the whole point seems to be you > want to expose formats (in OpenGL) which aren't blendable and you don't > expect blending to actually happen how could we do better? That, and also, as the comment in the patch says, give other state trackers (especially d3d1x, which has a blending support query) a means to check if blending would work / be slow, which, in my opinion, is quite useful to have. > Roland > ___ > 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
Re: [Mesa-dev] [PATCH 1/3] mesa: move gl_texture_image::Data, RowStride, ImageOffsets to swrast
On 10/12/2011 10:57 AM, Eric Anholt wrote: On Mon, 10 Oct 2011 20:27:44 -0600, Brian Paul wrote: From: Brian Paul Only swrast and the drivers that fall back to swrast need these fields now. This removes the last of the fields related to software rendering from gl_texture_image. + /* Allocate the swrast_texture_image::ImageOffsets array now */ + switch (texobj->Target) { + case GL_TEXTURE_3D: + case GL_TEXTURE_2D_ARRAY: + slices = image->Depth; + break; + case GL_TEXTURE_1D_ARRAY: + slices = image->Height; + break; + default: + slices = 1; + } + assert(!intel_image->base.ImageOffsets); + intel_image->base.ImageOffsets = malloc(slices * sizeof(GLuint)); This (and the corresponding code in the other drivers) should live in swrast with the IsPowerOfTwo and *Scale setup I think. I'd like to do that in a follow-on commit. There's actually an issue with the swrast FetchTexel code that should probably be addressed first. The 2D FetchTexel function is used for both 2D and 1D_ARRAY targets, but ImageOffsets[] is not used there. The 1D array slice/coord is passed as the second texcoord component so the texel address is computed using the RowStride. So while the 1D_ARRAY case in the code above is what we need for glTex[Sub]Image, it's irrelevant and ignored during swrast sampling of a 1D array texture. It's probably always been broken (I doubt i965 user has ever hit that sw fallback path). The simplest fix is to use the FetchTexel3D function when sampling 1D_ARRAYs, pass the slice as the 3rd coordinate so we use ImageOffsets[] and finally, have the swrast driver setup the ImageOffsets[] array as we're doing in the intel driver. Then we could merge some code. I'd like to commit the patch series as-is and then refine things bit by bit. I tested these patches on i965 and swrast and there were no piglit texture/fbo regressions. BTW, under what circumstances will the i965 driver revert to swrast rendering nowadays? The only FALLBACK() instance I see in the driver (just grepping) is for selection/feedback mode. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/10] i965: Use Ivybridge's "Legacy Data Port" for reads/writes.
On 10/11/2011 09:13 PM, Eric Anholt wrote: > On Mon, 10 Oct 2011 16:31:44 -0700, Kenneth Graunke > wrote: >> Using the constant cache for reads isn't going to work for scratch >> reads (variably-indexed arrays or register spills), as these aren't >> constant at all. >> >> Also, in the new VS backend, use the proper message number for OWord >> Dual Block Write messages. It's now 10, instead of 9. Prior to this, >> we were attempting to use the render cache with message 9, which was >> "Memory Barrier." Not quite the desired effect. Hmm, this part of the commit message is crap. Memory Fence is 7. I must've been lost in documentation somewhere. >> +205 piglits. >> >> NOTE: This is a candidate for the 7.11 branch. >> >> Signed-off-by: Kenneth Graunke > >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c >> index 5caebfc..bcd640e 100644 >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c >> @@ -546,6 +546,13 @@ brw_set_dp_write_message(struct brw_compile *p, >> brw_set_src1(p, insn, brw_imm_ud(0)); >> >> if (intel->gen >= 7) { >> + /* Use the Render Cache for RT writes; otherwise use the Data Cache */ >> + unsigned sfid = GEN7_MESSAGE_TARGET_DP_DATA_CACHE; >> + if (msg_type == GEN6_DATAPORT_WRITE_MESSAGE_RENDER_TARGET_WRITE) >> + sfid = GEN6_MESSAGE_TARGET_DP_RENDER_CACHE; >> + >> + insn->header.destreg__conditionalmod = sfid; >> + >>insn->bits3.gen7_dp.binding_table_index = binding_table_index; >>insn->bits3.gen7_dp.msg_control = msg_control; >>insn->bits3.gen7_dp.pixel_scoreboard_clear = pixel_scoreboard_clear; >> @@ -554,9 +561,6 @@ brw_set_dp_write_message(struct brw_compile *p, >>insn->bits3.gen7_dp.response_length = response_length; >>insn->bits3.gen7_dp.msg_length = msg_length; >>insn->bits3.gen7_dp.end_of_thread = end_of_thread; >> - >> - /* We always use the render cache for write messages */ >> - insn->header.destreg__conditionalmod = >> GEN6_MESSAGE_TARGET_DP_RENDER_CACHE; >> } else if (intel->gen == 6) { >>insn->bits3.gen6_dp.binding_table_index = binding_table_index; >>insn->bits3.gen6_dp.msg_control = msg_control; >> @@ -618,7 +622,7 @@ brw_set_dp_read_message(struct brw_compile *p, >>insn->bits3.gen7_dp.response_length = response_length; >>insn->bits3.gen7_dp.msg_length = msg_length; >>insn->bits3.gen7_dp.end_of_thread = 0; >> - insn->header.destreg__conditionalmod = >> GEN6_MESSAGE_TARGET_DP_CONST_CACHE; >> + insn->header.destreg__conditionalmod = >> GEN7_MESSAGE_TARGET_DP_DATA_CACHE; > > So you're writing through the render cache but reading through the data > cache? I'm amazed this works. Seems like you want to be doing what > gen6 does here, and use render cache if asked for it in the target_cache > argument. I think you may have misread... The hunk in brw_set_dp_write_message (first in this mail) uses the Render Cache _only_ for FB writes (the "Render Target Write" message). For everything else, writes go through the Data Cache. For reads (this hunk), I always use the Data Cache. IVB's render cache does not even have a "OWord Dual Block Read" message; I'd have to use the new "Typed Surface Read" operation...which doesn't exist on the Data Cache... When would you want to read through the render cache anyway? I'm thinking of these uses of the data port: - Register spilling (write/read through DC) - Variable-indexed arrays (read/write through DC) - Pull constants (never written; read through DC for now, ideally switch to Constant Cache later) - FB writes (write through Render Cache; never read) I'm not seeing a problem here... >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp >> b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp >> index ccbad25..750b6c3 100644 >> --- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp >> @@ -450,7 +450,9 @@ vec4_visitor::generate_scratch_write(vec4_instruction >> *inst, >> >> uint32_t msg_type; >> >> - if (intel->gen >= 6) >> + if (intel->gen >= 7) >> + msg_type = GEN7_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE; >> + else if (intel->gen == 6) >>msg_type = GEN6_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE; >> else >>msg_type = BRW_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE; > > This hunk (and the hunk of brw_defines.h, and the paragraph of commit > message) looks like a separate commit. Hm. I was trying to avoid intermediate breakage. I was trying to avoid sending a Reserved message type (1001) to the DC DP, but...now that I re-read, I realize I wasn't sending Memory Barrier after all..but sending a reserved message type to the RC DP. Heh. I'll split it back out. Thanks. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freed
Re: [Mesa-dev] [PATCH] gallium: add PIPE_BIND_BLENDABLE flag
Am 12.10.2011 19:35, schrieb Christoph Bumiller: > On 12.10.2011 19:17, Roland Scheidegger wrote: >> Am 12.10.2011 16:31, schrieb Christoph Bumiller: So do we need to go in and add PIPE_BIND_BLENDABLE to all of our existing surface-create calls in the state tracker, etc? >>> Well, that depends on whether we want to put blending fallbacks at the >>> state tracker level (OpenGL is probably the only API that requires them) >>> or let each driver handle them internally. >>> >>> Right now, drivers for GPUs that don't support blending on some formats >>> (e.g. DX10 level hardware for 16 bits per component or SNORM formats) >>> don't care much about doing fallbacks and silently ignore that blending >>> won't work (speaking at least for nv50 here). >>> >>> You pass the flag on resource creation, not surface creation, but since >>> surface creation can specify a different format, it applies only there. >>> >>> I should change the wording to: >>> "If this flag is set, surface creation may fail if blending is not >>> supported for the specified format. If it is not set, a driver may >>> choose to ignore blending on surfaces with formats that would require >>> emulation." >>> >>> That would leave it to the individual driver to choose whether to bother >>> with fallbacks or not. >> How do you emulate blending in some semi-sane way? Looks impossible to me. > > The best I can think of, short of software rendering (which the NV blob > does), is to render each primitive separately with a texture barrier in > between and performing the blend operation at the end of the fragment > shader. That, in contrast to software rendering, sounds potentially useful, but only if you have few large primitives, not really for the general case. > >> Maybe if proprietary drivers don't do anything useful in that case (that >> is when they are asked to render to a surface with blending they don't >> support) we could just do the same. I don't consider software render >> fallbacks terribly useful hence my preference would be to just not ask >> for the PIPE_BLENDABLE flag when we don't know we need blending (which >> means in the OpenGL state tracker) and just ignore blending in the driver. > > That's what I'd prefer, not changing the GL state tracker at all. > > The presence of the BLENDABLE flag would imply strict checking of > blending functionality and allow drivers to either refuse to perform it > (surface creation to fail, such as with formats that are not supported > at all) or have to fall back to software (guarantee that it works), > while its absence merely indicates that the state tracker leaves it to > the driver to do what seems fit, which is what we have right now. > >> That's not very correct though but since the whole point seems to be you >> want to expose formats (in OpenGL) which aren't blendable and you don't >> expect blending to actually happen how could we do better? > > That, and also, as the comment in the patch says, give other state > trackers (especially d3d1x, which has a blending support query) a means > to check if blending would work / be slow, which, in my opinion, is > quite useful to have. Yes, this looks good to me. The OpenGL side will just be a bit sloppy but not much we can do about. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Building with -fno-builtin-memcmp for improved performance
I've changed the scons to always build with -fno-builtin-memcmp. Jose - Original Message - > On Tue, 2011-09-20 at 16:35 +0200, Roland Scheidegger wrote: > > Am 20.09.2011 16:15, schrieb Keith Whitwell: > > > On Tue, 2011-09-20 at 16:02 +0200, Roland Scheidegger wrote: > > >> Am 20.09.2011 12:35, schrieb Keith Whitwell: > > >>> On Tue, 2011-09-20 at 10:59 +0200, Fabio wrote: > > There was a discussion some months ago about using > > -fno-builtin-memcmp for > > improving memcmp performance: > > http://lists.freedesktop.org/archives/mesa-dev/2011-June/009078.html > > > > Since then, was it properly addressed in mesa or the flag is > > still > > recommended? If so, what about adding it in configure.ac? > > >>> > > >>> I've been meaning to follow up on this too. I don't know the > > >>> answer, > > >>> but pinging Roland in case he does. > > >> > > >> I guess it is still recommended. > > >> Ideally this is really something which should be fixed in gcc - > > >> the > > >> compiler has all the knowledge about fixed alignment and size > > >> (if any) > > >> (and more importantly knows if only a binary answer is needed > > >> which > > >> makes this much easier) and doesn't need to do any function > > >> call. > > >> If you enable that flag and some platform just has the same > > >> primitive > > >> repz cmpsb sequence in the system library it will just get even > > >> slower, > > >> though I guess chances of that happening are slim (with the > > >> possible > > >> exception of windows). > > >> I think in most cases it won't make much difference, so nobody > > >> cared to > > >> implement that change. It is most likely still a good idea > > >> unless gcc > > >> addressed that in the meantime... > > > > > > Hmm, it seemed like it made a big difference in the earlier > > > discussion... > > Yes for llvmpipe and one app at least. > > But that struct being compared there is most likely the biggest (by > > far) > > anywhere (at least which is compared in a regular fashion). > > > > > I should take a look at reducing the size of the struct (as > > > mentioned > > > before), but surely there's some way to pull in a better memcmp?? > > > > Well, apart from using -fno-builtin-memcmp we could build our own > > memcmpxx, though the version I did there (returning binary only > > result > > and assuming 32bit alignment/size allowing gcc to optimize it) was > > still > > slower for large sizes than -fno-builtin-memcmp. Of course we could > > optimize it more (e.g. for 64bit aligned/sized things, or using > > hand-coded sse2 versions using 128bit at-a-time comparisons) but > > then it > > gets more complicated, so I wasn't sure it was worth it. > > > > For reference here are the earlier numbers (ipers with llvmpipe): > > original ipers: 12.1 fps > > optimized struct compare: 16.8 fps > > -fno-builtin-memcmp: 18.1 fps > > > > And this was the function I used for getting the numbers: > > > > static INLINE int util_cmp_struct(const void *src1, const void > > *src2, > > unsigned count) > > { > > /* hmm pointer casting is evil */ > > const uint32_t *src1_ptr = (uint32_t *)src1; > > const uint32_t *src2_ptr = (uint32_t *)src2; > > unsigned i; > > assert(count % 4 == 0); > > for (i = 0; i < count/4; i++) { > > if (*src1_ptr != *src2_ptr) { > > return 1; > > } > > src1_ptr++; > > src2_ptr++; > > } > > return 0; > > } > > OK, maybe the first thing to do is fix the compared struct, then > let's > see if there's anything significant left for a better memcmp to > extract. > > I can find some time to do that in the next few days. > > Keith > > ___ > 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
Re: [Mesa-dev] [PATCH] Add an autoconf option for mangling Mesa.
*ping* any review? Dan, I think you're probably the best to review this, if you have a minute. -tom tfo...@sci.utah.edu writes: > From: Tom Fogal > > In addition to setting up the flags correctly, this renames the > generated libraries to ensure they get 'Mangled' in the name. > This is very useful for distros and the like, where mangled Mesa > and non-mangled GL libraries typically need to be installed > side-by-side. > --- > configs/autoconf.in |4 ++-- > configure.ac| 27 --- > 2 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/configs/autoconf.in b/configs/autoconf.in > index 9bbafc9..96fe5da 100644 > --- a/configs/autoconf.in > +++ b/configs/autoconf.in > @@ -64,8 +64,8 @@ FLEX = @FLEX@ > BISON = @BISON@ > > # Library names (base name) > -GL_LIB = GL > -GLU_LIB = GLU > +GL_LIB = @GL_LIB@ > +GLU_LIB = @GLU_LIB@ > GLW_LIB = GLw > OSMESA_LIB = @OSMESA_LIB@ > GLESv1_CM_LIB = GLESv1_CM > diff --git a/configure.ac b/configure.ac > index 49e81ad..df909dd 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -342,6 +342,28 @@ else > fi > > dnl > +dnl Mangled Mesa support > +dnl > +AC_ARG_ENABLE([mangling], > + [AS_HELP_STRING([--enable-mangling], > +[enable mangled symbols and library name @<:@default=disabled@:>@])], > + [enable_mangling="${enableval}"], > + [enable_mangling=no] > +) > +GL_LIB="GL" > +GLU_LIB="GLU" > +OSMESA_LIB="OSMesa" > +if test "x${enable_mangling}" = "xyes" ; then > + DEFINES="${DEFINES} -DUSE_MGL_NAMESPACE" > + GL_LIB="MangledGL" > + GLU_LIB="MangledGLU" > + OSMESA_LIB="MangledOSMesa" > +fi > +AC_SUBST([GL_LIB]) > +AC_SUBST([GLU_LIB]) > +AC_SUBST([OSMESA_LIB]) > + > +dnl > dnl potentially-infringing-but-nobody-knows-for-sure stuff > dnl > AC_ARG_ENABLE([texture-float], > @@ -1280,17 +1302,16 @@ if test "x$osmesa_bits" != x8; then > fi > case "x$osmesa_bits" in > x8) > -OSMESA_LIB=OSMesa > +OSMESA_LIB="${OSMESA_LIB}" > ;; > x16|x32) > -OSMESA_LIB="OSMesa$osmesa_bits" > +OSMESA_LIB="${OSMESA_LIB}$osmesa_bits" > DEFINES="$DEFINES -DCHAN_BITS=$osmesa_bits -DDEFAULT_SOFTWARE_DEPTH_BITS > =31" > ;; > *) > AC_MSG_ERROR([OSMesa bits '$osmesa_bits' is not a valid option]) > ;; > esac > -AC_SUBST([OSMESA_LIB]) > > if test "x$enable_osmesa" = xyes; then > # only link libraries with osmesa if shared > -- > 1.7.3.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Profiling video driver
Hello, I'm using the Gallium R300 driver with the Xorg state tracker on my system. Everything is running very smoothly when I'm using OpenGL ES 2.0 composited mode of kwin. Running for example Notepad++ in wine on the other hand is painfully slow. I've tried to profile a Notepad++ session using oprofile but it seems that the reason why for example scrolling is so slow is not catched by oprofile. At least there is nothing which catches my eyes. I would have expected that "r300_dri.so" or "r300_drv.so" would pop up in the profile log. Is there a special way to profile graphics drivers? Best regards Martin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gl_NormalMatrix issue on Intel driver
Hi Ian, Kenneth, Ian Romanick writes: > On 10/10/2011 03:30 PM, tom fogal wrote: > > One of our programs which relies on shaders heavily is having > > issues, and I have tracked it down to unexpected values in > > gl_NormalMatrix within the fragment shader. > > I think we could make a more general piglit test the reproduce this > sort of failure. Attached is a new patch against piglit master. I took the approach of generating the shader instead of uniforms, because I perceived it to be easier to specify .x, .y, and .z as the gl_NormalMatrix vector element. > > Is this a known issue? Any workarounds available? Anything else I > > could do to help debug? > > Yikes! A *lot* has changed in the fragment shader back-end for i965 > since 7.10.2. Have you at least tried 7.10.3? 7.11? I've compiled Mesa master (983fa4) and enabled it (thanks for the help, Kenneth!) and I can reproduce it there. The attached piglit test succeeds with: the nvidia binary blob, and with both versions of Mesa (7.10.2 and master-983fa4) when LIBGL_ALWAYS_SOFTWARE is set to 1. It fails on both versions of Mesa when software mode is disabled. What else can I do to help? Thanks, -tom From d007b5a25bf6dbcc44beb29c689460d198f9f63b Mon Sep 17 00:00:00 2001 From: Tom Fogal Date: Wed, 12 Oct 2011 15:18:28 -0600 Subject: [PATCH] Add a test program for gl_NormalMatrix. --- tests/all.tests |1 + tests/shaders/CMakeLists.gl.txt |1 + tests/shaders/glsl-fs-normalmatrix.c | 139 ++ 3 files changed, 141 insertions(+), 0 deletions(-) create mode 100644 tests/shaders/glsl-fs-normalmatrix.c diff --git a/tests/all.tests b/tests/all.tests index ee46be2..1a9bb11 100644 --- a/tests/all.tests +++ b/tests/all.tests @@ -386,6 +386,7 @@ add_plain_test(shaders, 'glsl-fs-loop') add_plain_test(shaders, 'glsl-fs-loop-nested') add_plain_test(shaders, 'glsl-fs-mix') add_plain_test(shaders, 'glsl-fs-mix-constant') +add_plain_test(shaders, 'glsl-fs-normalmatrix') add_plain_test(shaders, 'glsl-fs-pointcoord') add_plain_test(shaders, 'glsl-fs-raytrace-bug27060') add_plain_test(shaders, 'glsl-fs-sampler-numbering') diff --git a/tests/shaders/CMakeLists.gl.txt b/tests/shaders/CMakeLists.gl.txt index d0e3005..c21432f 100644 --- a/tests/shaders/CMakeLists.gl.txt +++ b/tests/shaders/CMakeLists.gl.txt @@ -80,6 +80,7 @@ add_executable (glsl-fs-loop glsl-fs-loop.c) add_executable (glsl-fs-loop-nested glsl-fs-loop-nested.c) add_executable (glsl-fs-mix glsl-fs-mix.c) add_executable (glsl-fs-mix-constant glsl-fs-mix-constant.c) +add_executable (glsl-fs-normalmatrix glsl-fs-normalmatrix.c) IF (NOT MSVC) add_executable (glsl-fs-raytrace-bug27060 glsl-fs-raytrace-bug27060.c) ENDIF (NOT MSVC) diff --git a/tests/shaders/glsl-fs-normalmatrix.c b/tests/shaders/glsl-fs-normalmatrix.c new file mode 100644 index 000..f36b0ef --- /dev/null +++ b/tests/shaders/glsl-fs-normalmatrix.c @@ -0,0 +1,139 @@ +/* + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Tom Fogal + * + */ + +/** @file glsl-fs-normalmatrix.c + * + * Tests gl_NormalMatrix for appropriate initial values. + */ + +#include "piglit-util.h" + +int piglit_width = 100, piglit_height = 100; +int piglit_window_mode = GLUT_RGB | GLUT_DOUBLE; + +static const char vert[] = { + "void main() {\n" + " gl_Position = ftransform();\n" + "}\n" +}; +/* Creates a fragment shader which colors everything red if + * gl_NormalMatrix[elem].element + * is between 'low' and 'high', otherwise everything is green. + * The returned string is dynamically allocated and must be free'd by the + * caller. */ +static char* generate_fs(double low, double high, unsigned elem, char element) +{ + char* shader = calloc(1, 1024); + char* tmp = calloc(1, 512); /* for intermediate/appends. */ + + snprintf(shader, 1024, "void main(void) {\n"); + snprintf(tmp, 512, + " if(%f <= gl_Norma
[Mesa-dev] [PATCH v2] mesa: take into account indices offset for bounds check
Signed-off-by: Vadim Girlin --- v2: using helper function src/mesa/main/api_validate.c | 21 ++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 1fcf5cd..432fd08 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -50,6 +50,18 @@ index_bytes(GLenum type, GLsizei count) } } +/** + * Check that the indices don't lie outside buffer object bounds. + */ +static GLboolean +check_index_buffer_bounds(const GLvoid *offset, GLsizei count, GLenum type, + struct gl_buffer_object *elementBuf) +{ + if ((GLbyte*)offset + index_bytes(type, count) > (GLbyte*)elementBuf->Size) + return GL_FALSE; + else + return GL_TRUE; +} /** * Find the max index in the given element/index buffer @@ -257,7 +269,8 @@ _mesa_validate_DrawElements(struct gl_context *ctx, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, +ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawElements index out of buffer bounds"); return GL_FALSE; } @@ -318,7 +331,8 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, +ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawRangeElements index out of buffer bounds"); return GL_FALSE; } @@ -457,7 +471,8 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, +ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawElementsInstanced index out of buffer bounds"); return GL_FALSE; -- 1.7.6.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: move gl_texture_image::Data, RowStride, ImageOffsets to swrast
On Wed, 12 Oct 2011 11:44:10 -0600, Brian Paul wrote: > On 10/12/2011 10:57 AM, Eric Anholt wrote: > > On Mon, 10 Oct 2011 20:27:44 -0600, Brian Paul > > wrote: > >> From: Brian Paul > >> > >> Only swrast and the drivers that fall back to swrast need these fields now. > >> This removes the last of the fields related to software rendering from > >> gl_texture_image. > > > >> + /* Allocate the swrast_texture_image::ImageOffsets array now */ > >> + switch (texobj->Target) { > >> + case GL_TEXTURE_3D: > >> + case GL_TEXTURE_2D_ARRAY: > >> + slices = image->Depth; > >> + break; > >> + case GL_TEXTURE_1D_ARRAY: > >> + slices = image->Height; > >> + break; > >> + default: > >> + slices = 1; > >> + } > >> + assert(!intel_image->base.ImageOffsets); > >> + intel_image->base.ImageOffsets = malloc(slices * sizeof(GLuint)); > > > > This (and the corresponding code in the other drivers) should live in > > swrast with the IsPowerOfTwo and *Scale setup I think. > > I'd like to do that in a follow-on commit. > > There's actually an issue with the swrast FetchTexel code that should > probably be addressed first. > > The 2D FetchTexel function is used for both 2D and 1D_ARRAY targets, > but ImageOffsets[] is not used there. The 1D array slice/coord is > passed as the second texcoord component so the texel address is > computed using the RowStride. So while the 1D_ARRAY case in the code > above is what we need for glTex[Sub]Image, it's irrelevant and ignored > during swrast sampling of a 1D array texture. It's probably always > been broken (I doubt i965 user has ever hit that sw fallback path). > > The simplest fix is to use the FetchTexel3D function when sampling > 1D_ARRAYs, pass the slice as the 3rd coordinate so we use > ImageOffsets[] and finally, have the swrast driver setup the > ImageOffsets[] array as we're doing in the intel driver. Then we > could merge some code. > > I'd like to commit the patch series as-is and then refine things bit > by bit. I tested these patches on i965 and swrast and there were no > piglit texture/fbo regressions. Works for me. > BTW, under what circumstances will the i965 driver revert to swrast > rendering nowadays? The only FALLBACK() instance I see in the driver > (just grepping) is for selection/feedback mode. The FALLBACK() macros is sort of a red herring from i915. The real fallback code is brw_fallback.c, which triggers for: rendermode, no_rast, and texture border. I want to just ditch texture borders like you do in gallium -- successfully rendering them through swrast will be worse than not rendering them. That leaves feedback and select. For SELECT we just need to slightly generalize the meta code that exists for it already, I think. For feedback, it's a bit trickier because that's supposed to happen after clipping. I wonder if giving unclipped results generated by transform_feedback wouldn't be good enough, though? no_rast is just used for testing swrast for me, since the real swrast driver gives bad alpha values and just fails all of piglit. We're not planning on actually supporting general rendering through swrast when we get GL3.0 going, currently -- where we end up rendering in swrast, we should fix that to go through hardware. pgpJrqitdtUEy.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3] mesa: take into account indices offset for bounds check
Signed-off-by: Vadim Girlin --- Sorry, overlooked whitespace issues in v2 src/mesa/main/api_validate.c | 23 --- 1 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index 1fcf5cd..901f50f 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -52,6 +52,20 @@ index_bytes(GLenum type, GLsizei count) /** + * Check that the indices don't lie outside buffer object bounds. + */ +static GLboolean +check_index_buffer_bounds(const GLvoid *offset, GLsizei count, GLenum type, + struct gl_buffer_object *elementBuf) +{ + if ((GLbyte*)offset + index_bytes(type, count) > (GLbyte*)elementBuf->Size) + return GL_FALSE; + else + return GL_TRUE; +} + + +/** * Find the max index in the given element/index buffer */ GLuint @@ -257,7 +271,8 @@ _mesa_validate_DrawElements(struct gl_context *ctx, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, + ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawElements index out of buffer bounds"); return GL_FALSE; } @@ -318,7 +333,8 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, + ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawRangeElements index out of buffer bounds"); return GL_FALSE; } @@ -457,7 +473,8 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx, if (_mesa_is_bufferobj(ctx->Array.ElementArrayBufferObj)) { /* use indices in the buffer object */ /* make sure count doesn't go outside buffer bounds */ - if (index_bytes(type, count) > ctx->Array.ElementArrayBufferObj->Size) { + if (!check_index_buffer_bounds(indices, count, type, + ctx->Array.ElementArrayBufferObj)) { _mesa_warning(ctx, "glDrawElementsInstanced index out of buffer bounds"); return GL_FALSE; -- 1.7.6.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: setup address rounding enable bits
The patch(based on the reading of the emulator) came from while I was trying to fix the oglc pbo texImage.1PBODefaults fail. This case generates a texture with the width and height equal to window's width and height respectively, then try to texture it on the whole window. So, it's exactly one texel for one pixel. And, the min filter and mag filter are GL_LINEAR. It runs with swrast OK, as expected. But it failed with i965 driver. Well, you can't tell the difference from the screen, as the error is quite tiny. From my digging, it seems that there are some tiny error happened while getting tex address. This will break the one texel for one pixel rule in this case. Thus the linear result is taken, with tiny error. This patch would fix several oglc pbo subcase fail on both ILK, SNB and IVB. Signed-off-by: Yuanhan Liu --- src/mesa/drivers/dri/i965/brw_defines.h |7 +++ src/mesa/drivers/dri/i965/brw_wm_sampler_state.c |9 + src/mesa/drivers/dri/i965/gen7_sampler_state.c |9 + 3 files changed, 25 insertions(+), 0 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index a111630..e4647ff 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -196,6 +196,13 @@ #define BRW_MIPFILTER_NEAREST 1 #define BRW_MIPFILTER_LINEAR 3 +#define BRW_ADDRESS_ROUNDING_ENABLE_U_MAG 0x20 +#define BRW_ADDRESS_ROUNDING_ENABLE_U_MIN 0x10 +#define BRW_ADDRESS_ROUNDING_ENABLE_V_MAG 0x08 +#define BRW_ADDRESS_ROUNDING_ENABLE_V_MIN 0x04 +#define BRW_ADDRESS_ROUNDING_ENABLE_R_MAG 0x02 +#define BRW_ADDRESS_ROUNDING_ENABLE_R_MIN 0x01 + #define BRW_POLYGON_FRONT_FACING 0 #define BRW_POLYGON_BACK_FACING 1 diff --git a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c index 6834eba..9fc1c1a 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c @@ -312,6 +312,15 @@ static void brw_update_sampler_state(struct brw_context *brw, intel->batch.bo, brw->wm.sdc_offset[unit], I915_GEM_DOMAIN_SAMPLER, 0); } + + if (sampler->ss0.min_filter != BRW_MAPFILTER_NEAREST) + sampler->ss3.address_round = BRW_ADDRESS_ROUNDING_ENABLE_U_MIN | + BRW_ADDRESS_ROUNDING_ENABLE_V_MIN | + BRW_ADDRESS_ROUNDING_ENABLE_R_MIN; + if (sampler->ss0.mag_filter != BRW_MAPFILTER_NEAREST) + sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG | +BRW_ADDRESS_ROUNDING_ENABLE_V_MAG | +BRW_ADDRESS_ROUNDING_ENABLE_R_MAG; } diff --git a/src/mesa/drivers/dri/i965/gen7_sampler_state.c b/src/mesa/drivers/dri/i965/gen7_sampler_state.c index aee67c8..5fab91c 100644 --- a/src/mesa/drivers/dri/i965/gen7_sampler_state.c +++ b/src/mesa/drivers/dri/i965/gen7_sampler_state.c @@ -167,6 +167,15 @@ gen7_update_sampler_state(struct brw_context *brw, int unit, upload_default_color(brw, gl_sampler, unit); sampler->ss2.default_color_pointer = brw->wm.sdc_offset[unit] >> 5; + + if (sampler->ss0.min_filter != BRW_MAPFILTER_NEAREST) + sampler->ss3.address_round = BRW_ADDRESS_ROUNDING_ENABLE_U_MIN | + BRW_ADDRESS_ROUNDING_ENABLE_V_MIN | + BRW_ADDRESS_ROUNDING_ENABLE_R_MIN; + if (sampler->ss0.mag_filter != BRW_MAPFILTER_NEAREST) + sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG | +BRW_ADDRESS_ROUNDING_ENABLE_V_MAG | +BRW_ADDRESS_ROUNDING_ENABLE_R_MAG; } -- 1.7.4.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Profiling video driver
On Mit, 2011-10-12 at 22:54 +0200, Martin Stolpe wrote: > > I'm using the Gallium R300 driver with the Xorg state tracker on my system. > Everything is running very smoothly when I'm using OpenGL ES 2.0 composited > mode of kwin. Running for example Notepad++ in wine on the other hand is > painfully slow. I've tried to profile a Notepad++ session using oprofile but > it seems that the reason why for example scrolling is so slow is not catched > by oprofile. At least there is nothing which catches my eyes. I would have > expected that "r300_dri.so" or "r300_drv.so" would pop up in the profile log. > Is there a special way to profile graphics drivers? Not really, but in the likely case that the slowness is relaeted to software rendering fallbacks, a lot of cycles may be spent in X server or libpixman code. I prefer sysprof or perf over oprofile, YMMV. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev