Re: [Mesa-dev] [PATCH 22/32] i965: Create correctly sized mcs for an image
On Mon, Jan 02, 2017 at 06:37:13PM -0800, Ben Widawsky wrote: > v2: Leave "image+mod" (Topi) > > Signed-off-by: Ben Widawsky> Acked-by: Daniel Stone > --- > src/mesa/drivers/dri/i965/intel_screen.c | 33 > > 1 file changed, 29 insertions(+), 4 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c > b/src/mesa/drivers/dri/i965/intel_screen.c > index 153542c1d1..805de5b461 100644 > --- a/src/mesa/drivers/dri/i965/intel_screen.c > +++ b/src/mesa/drivers/dri/i965/intel_screen.c > @@ -549,6 +549,7 @@ create_image_with_modifier(struct intel_screen *screen, > uint32_t requested_tiling = 0, tiling = 0; > unsigned long pitch; > unsigned tiled_height = 0; > + unsigned ccs_height = 0; You could add a short note into the commit message that ccs_height is left zero, and that it will be hooked in later. Patch title makes one believe that something gets effective. Either way: Reviewed-by: Topi Pohjolainen > > switch (modifier) { > case I915_FORMAT_MOD_Y_TILED: > @@ -566,10 +567,33 @@ create_image_with_modifier(struct intel_screen *screen, > /* For now, all modifiers require some tiling */ > assert(tiling); > > + /* > +* CCS width is always going to be less than or equal to the image's > width. > +* All we need to do is make sure we add extra rows (height) for the CCS. > +* > +* A pair of CCS bits correspond to 8x4 pixels, and must be cacheline > +* granularity. Each CCS tile is laid out in 8b strips, which corresponds > to > +* 1024x512 pixel region. In memory, it looks like the following: > +* > +* ? > +* ??? ??? > +* ??? ??? > +* ??? ??? > +* ??? Image ??? > +* ??? ??? > +* ??? ??? > +* ???x??? > +* ? > +* ??? ??? | > +* ???ccs ??? unused | > +* ?---??? > +* <--pitch--> > +*/ > cpp = _mesa_get_format_bytes(image->format); > - image->bo = drm_intel_bo_alloc_tiled(screen->bufmgr, "image+mod", > -width, tiled_height, cpp, , > -, 0); > + image->bo = drm_intel_bo_alloc_tiled(screen->bufmgr, > +ccs_height ? "image+ccs" : > "image+mod", > +width, tiled_height + ccs_height, > +cpp, , , 0); > if (image->bo == NULL) >return false; > > @@ -587,7 +611,8 @@ create_image_with_modifier(struct intel_screen *screen, > if (image->planar_format) >assert(image->planar_format->nplanes == 1); > > - image->aux_offset = 0; /* y_tiled_height * pitch; */ > + if (ccs_height) > + image->aux_offset = tiled_height * pitch /* + mt->offset */; > > return true; > } > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 23/32] i965/miptree: Add a return for updating of winsys
On Mon, Jan 02, 2017 at 06:37:14PM -0800, Ben Widawsky wrote: > There is nothing particularly useful to do currently if the update > fails, but there is no point carrying on either. As a result, this has a > behavior change. > > v2: Make the return type a bool (Topi) Reviewed-by: Topi Pohjolainen> > Cc: Topi Pohjolainen > Signed-off-by: Ben Widawsky > Acked-by: Daniel Stone > --- > src/mesa/drivers/dri/i965/brw_context.c | 14 -- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 6 +++--- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 2 +- > 3 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > b/src/mesa/drivers/dri/i965/brw_context.c > index 4ca77c789b..7e350c4e47 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -1652,9 +1652,10 @@ intel_process_dri2_buffer(struct brw_context *brw, >return; > } > > - intel_update_winsys_renderbuffer_miptree(brw, rb, bo, > -drawable->w, drawable->h, > -buffer->pitch); > + if (!intel_update_winsys_renderbuffer_miptree(brw, rb, bo, > + drawable->w, drawable->h, > + buffer->pitch)) > + return; > > if (_mesa_is_front_buffer_drawing(fb) && > (buffer->attachment == __DRI_BUFFER_FRONT_LEFT || > @@ -1710,9 +1711,10 @@ intel_update_image_buffer(struct brw_context *intel, > if (last_mt && last_mt->bo == buffer->bo) >return; > > - intel_update_winsys_renderbuffer_miptree(intel, rb, buffer->bo, > -buffer->width, buffer->height, > -buffer->pitch); > + if (!intel_update_winsys_renderbuffer_miptree(intel, rb, buffer->bo, > + buffer->width, > buffer->height, > + buffer->pitch)) > + return; > > if (_mesa_is_front_buffer_drawing(fb) && > buffer_type == __DRI_IMAGE_BUFFER_FRONT && > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index dce8ce3350..a6cc64365d 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -916,7 +916,7 @@ intel_miptree_create_for_image(struct brw_context *intel, > * that will contain the actual rendering (which is lazily resolved to > * irb->singlesample_mt). > */ > -void > +bool > intel_update_winsys_renderbuffer_miptree(struct brw_context *intel, > struct intel_renderbuffer *irb, > drm_intel_bo *bo, > @@ -982,12 +982,12 @@ intel_update_winsys_renderbuffer_miptree(struct > brw_context *intel, > irb->mt = multisample_mt; >} > } > - return; > + return true; > > fail: > intel_miptree_release(>singlesample_mt); > intel_miptree_release(>mt); > - return; > + return false; > } > > struct intel_mipmap_tree* > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > index cf8f1a7687..9b4c85e509 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > @@ -730,7 +730,7 @@ intel_miptree_create_for_image(struct brw_context *intel, > uint32_t pitch, > uint32_t layout_flags); > > -void > +bool > intel_update_winsys_renderbuffer_miptree(struct brw_context *intel, > struct intel_renderbuffer *irb, > drm_intel_bo *bo, > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 19/32] i965/miptree: Allocate mcs_buf for an image's CCS_E
On Mon, Jan 02, 2017 at 06:37:10PM -0800, Ben Widawsky wrote: > This code will disable actually creating these buffers for the scanout, > but it puts the allocation in place. > > Primarily this patch is split out for review, it can be squashed in > later if preferred. > > assert(mt->offset == 0) in ccs creation (as requested by Topi) > > Cc: Topi Pohjolainen> Signed-off-by: Ben Widawsky > Acked-by: Daniel Stone > --- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 97 > +++ > 1 file changed, 85 insertions(+), 12 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index d79cc61ac4..dce8ce3350 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -58,6 +58,11 @@ intel_miptree_alloc_mcs(struct brw_context *brw, > struct intel_mipmap_tree *mt, > GLuint num_samples); > > +static void > +intel_miptree_init_mcs(struct brw_context *brw, > + struct intel_mipmap_tree *mt, > + int init_value); > + > /** > * Determine which MSAA layout should be used by the MSAA surface being > * created, based on the chip generation and the surface type. > @@ -152,6 +157,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct > brw_context *brw, > if (mt->aux_disable & INTEL_AUX_DISABLE_MCS) >return false; > > + if (mt->is_scanout) > + return false; > + > /* This function applies only to non-multisampled render targets. */ > if (mt->num_samples > 1) >return false; > @@ -744,6 +752,7 @@ intel_miptree_create(struct brw_context *brw, > * resolves. > */ >const bool lossless_compression_disabled = INTEL_DEBUG & DEBUG_NO_RBC; > + assert(!mt->is_scanout); >const bool is_lossless_compressed = > unlikely(!lossless_compression_disabled) && > brw->gen >= 9 && !mt->is_scanout && > @@ -810,6 +819,45 @@ intel_miptree_create_for_bo(struct brw_context *brw, > return mt; > } > > +static bool > +create_ccs_buf_for_image(struct brw_context *intel, > + __DRIimage *image, > + struct intel_mipmap_tree *mt) > +{ > + > + struct isl_surf temp_main_surf; > + struct isl_surf temp_ccs_surf; > + > + /* There isn't anything specifically wrong with there being an offset, in > +* which case, the CCS miptree's offset should be mt->offset + > +* image->aux_offset. However, the code today only will have an offset > when > +* this miptree is pointing to a slice from another miptree, and in that > case > +* we'd need to offset within the AUX CCS buffer properly. It's > questionable > +* whether our code handles that case properly, and since it can never > happen > +* for scanout, just use the assertion to prevent it. > +*/ > + assert(mt->offset == 0); Thanks a lot for adding this, especially the comment! > + > + intel_miptree_get_isl_surf(intel, mt, _main_surf); > + if (!isl_surf_get_ccs_surf(>isl_dev, _main_surf, > _ccs_surf)) > + return false; > + > + mt->mcs_buf = calloc(1, sizeof(*mt->mcs_buf)); > + mt->mcs_buf->bo = image->bo; > + drm_intel_bo_reference(image->bo); > + > + mt->mcs_buf->offset = image->aux_offset; > + mt->mcs_buf->size = temp_ccs_surf.size; > + mt->mcs_buf->pitch = temp_ccs_surf.row_pitch; > + mt->mcs_buf->qpitch = isl_surf_get_array_pitch_sa_rows(_ccs_surf); > + > + intel_miptree_init_mcs(intel, mt, 0); > + mt->aux_disable &= ~INTEL_AUX_DISABLE_CCS; > + mt->msaa_layout = INTEL_MSAA_LAYOUT_CMS; > + > + return true; > +} > + > struct intel_mipmap_tree * > intel_miptree_create_for_image(struct brw_context *intel, > __DRIimage *image, > @@ -820,17 +868,42 @@ intel_miptree_create_for_image(struct brw_context > *intel, > uint32_t pitch, > uint32_t layout_flags) > { > - layout_flags = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) ? > - MIPTREE_LAYOUT_FOR_SCANOUT : MIPTREE_LAYOUT_DISABLE_AUX; > - return intel_miptree_create_for_bo(intel, > - image->bo, > - format, > - offset, > - width, > - height, > - 1, > - pitch, > - layout_flags); > + struct intel_mipmap_tree *mt; > + > + /* Other flags will be ignored, so make sure the caller didn't pass any. > */ > + assert((layout_flags & ~MIPTREE_LAYOUT_FOR_SCANOUT) == 0); > + > + if (!image->aux_offset) > + layout_flags |=
Re: [Mesa-dev] [PATCH 2/2] radv: define and implement VK_EXT_queue_global_priority
So two things I'm missing here are advertising the extension and checking the kernel driver version. To advertize the extension, an entry needs to be added to the device_extensions array. However, since that array is const global, you can't really select on kernel version or such, so you'll probably need to switch to creating an array on physical device creation or such. Another thing is that you'll need to increase the amdgpu driver DRM version in the kernel, and then check against it in radv, to see if we can support the feature. On Wed, Jan 4, 2017 at 3:27 AM, Andres Rodriguezwrote: > Add a new extension VK_EXT_queue_global_priority, which allows the > caller to elevate a queue's priority above all other queues in the > system. > > This extension aims to provide a mechanism for compositors to have > achieve a guaranteed quality of service, even when the hardware may be > loaded by a game or application. > --- > docs/specs/VK_EXT_queue_global_priority.txt | 97 > +++ > include/vulkan/vk_ext_queue_global_priority.h | 72 > src/amd/vulkan/radv_device.c | 31 - > src/amd/vulkan/radv_private.h | 12 > src/amd/vulkan/radv_radeon_winsys.h | 8 ++- > src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 20 +- > 6 files changed, 234 insertions(+), 6 deletions(-) > create mode 100644 docs/specs/VK_EXT_queue_global_priority.txt > create mode 100644 include/vulkan/vk_ext_queue_global_priority.h > > diff --git a/docs/specs/VK_EXT_queue_global_priority.txt > b/docs/specs/VK_EXT_queue_global_priority.txt > new file mode 100644 > index 000..9a6a712 > --- /dev/null > +++ b/docs/specs/VK_EXT_queue_global_priority.txt > @@ -0,0 +1,97 @@ > +Name Strings > + > +VK_EXT_queue_global_priority > + > +Extension Type > + > +Queue Extension > + > +Registered Extension Number > + > +Draft > + > +Status > + > +Draft > + > +Version > + > +0 (Early Draft) > + > +Last Modified Date > + > +See git log. > + > +IP Status > + > +No known IP claims. > + > +Dependencies > + > +This extension is written against the Vulkan 1.0.32 specification [1]. > + > +Contributors > + > +Andres Rodriguez, Valve Software > +Pierre-Loup Griffais, Valve Software > + > +Contact > + > +Andres Rodriguez, Valve Software > + > +Overview > + > +In Vulkan 1.0.32 users can specify device-scope queue priorities. In > + some cases it may be useful to extend this concept to a system-wide > scope. > + > + This extension provides a mechanism for caller's to set their > system-wide > +priority. The default queue priority is VK_QUEUE_GLOBAL_PRIORITY_NORMAL. > + > + TODO: privileges required for priority escalation and note about > system > +resource starvation. > + > +New Object Types > + > +VkQueueGlobalPriorityEXT > + VK_STRUCTURE_TYPE_QUEUE_GLOBAL_PRIORITY_EXT > + > +New Enum Constants > + > + None > + > +New Enums > + > + VkQueueGlobalPriority > + VK_QUEUE_GLOBAL_PRIORITY_HIGH > + VK_QUEUE_GLOBAL_PRIORITY_NORMAL > + > +New Structs > + > + typedef struct VkQueueGlobalPriorityEXT { > + VkStructureType sType; > + void* pNext; > + VkQueueGlobalPriority priority; > + } VkQueueGlobalPriorityEXT; > + > +New Functions > + > +None > + > +Description > + > +TODO > + > +Issues > + > + TODO > + > +References > + > +[1]: https://github.com/KhronosGroup/Vulkan-Docs/tree/v1.0-core-20161025 > + > +Version History > + > +See git log for full history. > + > +- Revision 1, 2017-01-01 (Andres Rodriguez) > +- Initial draft > diff --git a/include/vulkan/vk_ext_queue_global_priority.h > b/include/vulkan/vk_ext_queue_global_priority.h > new file mode 100644 > index 000..01c7329 > --- /dev/null > +++ b/include/vulkan/vk_ext_queue_global_priority.h > @@ -0,0 +1,72 @@ > +/* > + * Copyright 2017 Valve Software > + * > + * 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
Re: [Mesa-dev] [PATCH 0/6] Enable OpenGL 4.0 on Haswell
On Tue, 2017-01-03 at 07:48 -0800, Kenneth Graunke wrote: > On Tuesday, January 3, 2017 2:02:19 PM PST Iago Toral wrote: > > > > On Tue, 2017-01-03 at 11:14 +, Chris Wilson wrote: > > > > > > On Tue, Jan 03, 2017 at 11:42:50AM +0100, Iago Toral Quiroga > > > wrote: > > > > > > > > > > > > Enabling GL 4.0 in gen7 requires a bit of work because some > > > > hardware and kernel > > > > combinations may not support all the features. Specifically, we > > > > need to know > > > > if the kernel can do pipelined register writes. Unfortunately, > > > > this > > > > requires > > > > that we emit batches at screen creation time when we don't have > > > > a > > > > brw_context > > > > available (all our current batch emission infrastructure relies > > > > on > > > > this). > > > > See [1] and [2] for more details. > > > Emitting a batch is trivial: > > > > > > https://lists.freedesktop.org/archives/mesa-dev/2015-August/09107 > > > 7.ht > > > ml > > Oh, thanks, I did not know you had done this before. It seems that > > patch got an Rb from Martin when it was sent in 2015 but it never > > landed, right? was there any other issue holding it back? > That was me - I NAK'd the series as a whole. > > > > > > > > > But we do want to share the bufmgr between screens, and you do > > > want > > > to > > > deprecate the current intel_batchbuffer.c due to the very large > > > overhead > > > it imposes. > > It seems that Curro and Ken were more interested in trying to reuse > > a > > bit more of the intel_batchbuffer.c for this rather than the other > > way > > around, but then again that might have only been because my > > original > > implementation was very focused on porting code from that file... > > your > > patch gets this done without using a batchbuffer object at all and > > I > > think it is quite clean. If Ken and Curro also like it and there > > are no > > other concerns I'll be happy to adapt/rebase your patch instead. > > > > Ken was also saying that he was planning some rework of the batch > > emission code, so I don't know what strategy fits his plans better. > > > > Iago > I'd forgotten that Chris did this. His code is much smaller, and > looks > like a nice solution to this problem. > > Iago, could you update the patch Chris linked instead? Sure! > A couple thoughts on Chris's patch: > > I'd prefer to drop brw->has_pipelined_so in favor of checking the > screen > field directly (we've got enough redundant things in brw_context > already). Makes sense to me. > Also, I'm not sure what to make of the hw_has_pipelined_register > bitfield. If we're only using it for one thing, we could just use a > "can_write_sol_offsets" boolean. But it also seems nice. Would it > make sense to generalize it a bit and have: > > #define KERNEL_ALLOWS_SOL_OFFSET_WRITES(1 << 0) > #define KERNEL_ALLOWS_PREDICATE_WRITES (1 << 1) > #define KERNEL_ALLOWS_CS_GPR_WRITES(1 << 2) > #define KERNEL_ALLOWS_HSW_SCRATCH1_AND_ROW_CHICKEN3_WRITES (1 << 3) > #define KERNEL_ALLOWS_LOAD_REGISTER_REG(1 << 4) > > and consolidate the various brw->screen->cmd_parser_version checking > into one place, so intel_screen.c either tries it or queries it, and > then sets a bit, and the rest of the code just checks that bitfield? > > It might be cleaner that way...thoughts? I suppose this could make sense too. I am fine with giving this a go if there is agreement. I'll start by updating Chris's patch as is, then send a follow-up patch to do this if we decide to go down that path. > By the way, I just pushed a patch from Robert which eliminated the > OACONTROL register checking. So you don't need to worry about that. Great, thanks Ken! Iago > --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] radv: Initial implementation of VK_EXT_queue_global_priority
Hey Guys, One of my first non-trivial contributions to mesa here. Let me know if I've messed up anything, bad coding style or some patch validation script I should've run or any other kind of pitfall a newbie might make. I tried to follow the guidelines over at: http://www.mesa3d.org/devinfo.html Although some of them seen to not apply anymore (radv code was tabs instead of 3 spaces). Thanks for taking the time to review :) Regards, Andres From: mesa-dev [mesa-dev-boun...@lists.freedesktop.org] on behalf of Andres Rodriguez [andre...@gmail.com] Sent: Tuesday, January 03, 2017 9:27 PM To: mesa-dev@lists.freedesktop.org Subject: [Mesa-dev] radv: Initial implementation of VK_EXT_queue_global_priority This patch series implements VK_EXT_queue_global_priority, a vulkan extension that focuses on providing scheduling guarantees. For further information on the use case refer to: https://lists.freedesktop.org/archives/amd-gfx/2016-December/004068.html For some perf data on this initial implementation: https://lists.freedesktop.org/archives/amd-gfx/2016-December/004257.html Test apps available at: https://github.com/lostgoat/Vulkan Related amdgpu and libdrm_amdgpu patches can be found on the amd-gfx mailing list. Additionaly, results for vk-cts -n *semaphore* on an RX480: Test run totals: Passed:1821/4046 (45.0%) Failed:0/4046 (0.0%) Not supported: 2225/4046 (55.0%) Warnings: 0/4046 (0.0%) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] radv: define and implement VK_EXT_queue_global_priority
Add a new extension VK_EXT_queue_global_priority, which allows the caller to elevate a queue's priority above all other queues in the system. This extension aims to provide a mechanism for compositors to have achieve a guaranteed quality of service, even when the hardware may be loaded by a game or application. --- docs/specs/VK_EXT_queue_global_priority.txt | 97 +++ include/vulkan/vk_ext_queue_global_priority.h | 72 src/amd/vulkan/radv_device.c | 31 - src/amd/vulkan/radv_private.h | 12 src/amd/vulkan/radv_radeon_winsys.h | 8 ++- src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 20 +- 6 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 docs/specs/VK_EXT_queue_global_priority.txt create mode 100644 include/vulkan/vk_ext_queue_global_priority.h diff --git a/docs/specs/VK_EXT_queue_global_priority.txt b/docs/specs/VK_EXT_queue_global_priority.txt new file mode 100644 index 000..9a6a712 --- /dev/null +++ b/docs/specs/VK_EXT_queue_global_priority.txt @@ -0,0 +1,97 @@ +Name Strings + +VK_EXT_queue_global_priority + +Extension Type + +Queue Extension + +Registered Extension Number + +Draft + +Status + +Draft + +Version + +0 (Early Draft) + +Last Modified Date + +See git log. + +IP Status + +No known IP claims. + +Dependencies + +This extension is written against the Vulkan 1.0.32 specification [1]. + +Contributors + +Andres Rodriguez, Valve Software+Pierre-Loup Griffais, Valve Software + +Contact + +Andres Rodriguez, Valve Software + +Overview + +In Vulkan 1.0.32 users can specify device-scope queue priorities. In + some cases it may be useful to extend this concept to a system-wide scope. + + This extension provides a mechanism for caller's to set their system-wide +priority. The default queue priority is VK_QUEUE_GLOBAL_PRIORITY_NORMAL. + + TODO: privileges required for priority escalation and note about system +resource starvation. + +New Object Types + +VkQueueGlobalPriorityEXT + VK_STRUCTURE_TYPE_QUEUE_GLOBAL_PRIORITY_EXT + +New Enum Constants + + None + +New Enums + + VkQueueGlobalPriority + VK_QUEUE_GLOBAL_PRIORITY_HIGH + VK_QUEUE_GLOBAL_PRIORITY_NORMAL + +New Structs + + typedef struct VkQueueGlobalPriorityEXT { + VkStructureType sType; + void* pNext; + VkQueueGlobalPriority priority; + } VkQueueGlobalPriorityEXT; + +New Functions + +None + +Description + +TODO + +Issues + + TODO + +References + +[1]: https://github.com/KhronosGroup/Vulkan-Docs/tree/v1.0-core-20161025 + +Version History + +See git log for full history. + +- Revision 1, 2017-01-01 (Andres Rodriguez) +- Initial draft diff --git a/include/vulkan/vk_ext_queue_global_priority.h b/include/vulkan/vk_ext_queue_global_priority.h new file mode 100644 index 000..01c7329 --- /dev/null +++ b/include/vulkan/vk_ext_queue_global_priority.h @@ -0,0 +1,72 @@ +/* + * Copyright 2017 Valve Software + * + * 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. + */ + +#ifndef VK_EXT_QUEUE_GLOBAL_PRIORITY_H_ +#define VK_EXT_QUEUE_GLOBAL_PRIORITY_H_ + +#include "vulkan.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* Draft spec lives at . + * + * TODO: Discuss with everyone who cares. + * TODO: Get a proper VkStructureType + * TODO: Finish spec + */ +#define VK_EXT_queue_global_priority +#define VK_EXT_queue_global_priotiry_SPEC_VERSION 0 /* experimental */ +#define VK_EXT_queue_global_priority_EXTENSION_NAME "VK_EXT_queue_global_priority" + +/** System-wide queue priority + * + * High
[Mesa-dev] [PATCH 1/2] radv: use a winsys context per-queue, instead of per device
Queues are independent execution streams. The vulkan spec provides no ordering guarantees for different queues. By using a single context for all queues, we are forcing all commands into an unecessary FIFO ordering. This change is a preparation step to allow our-of-ordering scheduling of certain work tasks. As a side effect, vkQueueWaitIdle will be marginally faster. Previously due to the shared context, vkQueueWaitIdle was equivalent to vkDeviceWaitIdle. --- src/amd/vulkan/radv_device.c | 35 --- src/amd/vulkan/radv_private.h | 2 +- src/amd/vulkan/radv_wsi.c | 2 +- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index e57a419..d9f9a2b 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -651,7 +651,7 @@ void radv_GetPhysicalDeviceMemoryProperties( }; } -static void +static int radv_queue_init(struct radv_device *device, struct radv_queue *queue, int queue_family_index, int idx) { @@ -659,11 +659,19 @@ radv_queue_init(struct radv_device *device, struct radv_queue *queue, queue->device = device; queue->queue_family_index = queue_family_index; queue->queue_idx = idx; + + queue->hw_ctx = device->ws->ctx_create(device->ws); + if (!queue->hw_ctx) + return VK_ERROR_OUT_OF_HOST_MEMORY; + + return VK_SUCCESS; } static void radv_queue_finish(struct radv_queue *queue) { + if (queue->hw_ctx) + queue->device->ws->ctx_destroy(queue->hw_ctx); } VkResult radv_CreateDevice( @@ -718,23 +726,21 @@ VkResult radv_CreateDevice( goto fail; } - device->queue_count[qfi] = queue_create->queueCount; + memset(device->queues[qfi], 0, queue_create->queueCount * sizeof(struct radv_queue)); - for (unsigned q = 0; q < queue_create->queueCount; q++) - radv_queue_init(device, >queues[qfi][q], qfi, q); - } + device->queue_count[qfi] = queue_create->queueCount; - device->hw_ctx = device->ws->ctx_create(device->ws); - if (!device->hw_ctx) { - result = VK_ERROR_OUT_OF_HOST_MEMORY; - goto fail; + for (unsigned q = 0; q < queue_create->queueCount; q++) { + result = radv_queue_init(device, >queues[qfi][q], qfi, q); + if (result != VK_SUCCESS) + goto fail; + } } result = radv_device_init_meta(device); - if (result != VK_SUCCESS) { - device->ws->ctx_destroy(device->hw_ctx); + if (result != VK_SUCCESS) goto fail; - } + device->allow_fast_clears = env_var_as_boolean("RADV_FAST_CLEARS", false); device->allow_dcc = !env_var_as_boolean("RADV_DCC_DISABLE", false); device->shader_stats_dump = env_var_as_boolean("RADV_SHADER_STATS", false); @@ -780,7 +786,6 @@ void radv_DestroyDevice( { RADV_FROM_HANDLE(radv_device, device, _device); - device->ws->ctx_destroy(device->hw_ctx); for (unsigned i = 0; i < RADV_MAX_QUEUE_FAMILIES; i++) { for (unsigned q = 0; q < device->queue_count[i]; q++) radv_queue_finish(>queues[i][q]); @@ -878,7 +883,7 @@ VkResult radv_QueueSubmit( RADV_FROM_HANDLE(radv_queue, queue, _queue); RADV_FROM_HANDLE(radv_fence, fence, _fence); struct radeon_winsys_fence *base_fence = fence ? fence->fence : NULL; - struct radeon_winsys_ctx *ctx = queue->device->hw_ctx; + struct radeon_winsys_ctx *ctx = queue->hw_ctx; int ret; for (uint32_t i = 0; i < submitCount; i++) { @@ -929,7 +934,7 @@ VkResult radv_QueueWaitIdle( { RADV_FROM_HANDLE(radv_queue, queue, _queue); - queue->device->ws->ctx_wait_idle(queue->device->hw_ctx, + queue->device->ws->ctx_wait_idle(queue->hw_ctx, radv_queue_family_to_ring(queue->queue_family_index), queue->queue_idx); return VK_SUCCESS; diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h index f76d38d..d316f71 100644 --- a/src/amd/vulkan/radv_private.h +++ b/src/amd/vulkan/radv_private.h @@ -456,6 +456,7 @@ enum ring_type radv_queue_family_to_ring(int f); struct radv_queue { VK_LOADER_DATA _loader_data; struct radv_device * device; + struct radeon_winsys_ctx*hw_ctx; int queue_family_index; int queue_idx; }; @@ -467,7 +468,6 @@ struct radv_device { struct radv_instance * instance; struct radeon_winsys *ws; - struct radeon_winsys_ctx *hw_ctx; struct radv_meta_state
[Mesa-dev] radv: Initial implementation of VK_EXT_queue_global_priority
This patch series implements VK_EXT_queue_global_priority, a vulkan extension that focuses on providing scheduling guarantees. For further information on the use case refer to: https://lists.freedesktop.org/archives/amd-gfx/2016-December/004068.html For some perf data on this initial implementation: https://lists.freedesktop.org/archives/amd-gfx/2016-December/004257.html Test apps available at: https://github.com/lostgoat/Vulkan Related amdgpu and libdrm_amdgpu patches can be found on the amd-gfx mailing list. Additionaly, results for vk-cts -n *semaphore* on an RX480: Test run totals: Passed:1821/4046 (45.0%) Failed:0/4046 (0.0%) Not supported: 2225/4046 (55.0%) Warnings: 0/4046 (0.0%) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: fix loop iteration count calculation for floats
Drp... Reviewed-by: Jason EkstrandOn Jan 3, 2017 7:00 PM, "Timothy Arceri" wrote: > Fixes performance regression in SynMark PSPom caused by loops with float > counters not always unrolling. > > For example: > >for (float i = 0.02; i < 0.9; i += 0.11) > ... > --- > src/compiler/nir/nir_loop_analyze.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/compiler/nir/nir_loop_analyze.c > b/src/compiler/nir/nir_loop_analyze.c > index 71cbe3c..a5f464a 100644 > --- a/src/compiler/nir/nir_loop_analyze.c > +++ b/src/compiler/nir/nir_loop_analyze.c > @@ -384,8 +384,8 @@ get_iteration(nir_op cond_op, nir_const_value > *initial, nir_const_value *step, > case nir_op_flt: > case nir_op_feq: > case nir_op_fne: { > - int32_t initial_val = initial->f32[0]; > - int32_t span = limit->f32[0] - initial_val; > + float initial_val = initial->f32[0]; > + float span = limit->f32[0] - initial_val; >iter = span / step->f32[0]; >break; > } > -- > 2.9.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: fix loop iteration count calculation for floats
Fixes performance regression in SynMark PSPom caused by loops with float counters not always unrolling. For example: for (float i = 0.02; i < 0.9; i += 0.11) ... --- src/compiler/nir/nir_loop_analyze.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 71cbe3c..a5f464a 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -384,8 +384,8 @@ get_iteration(nir_op cond_op, nir_const_value *initial, nir_const_value *step, case nir_op_flt: case nir_op_feq: case nir_op_fne: { - int32_t initial_val = initial->f32[0]; - int32_t span = limit->f32[0] - initial_val; + float initial_val = initial->f32[0]; + float span = limit->f32[0] - initial_val; iter = span / step->f32[0]; break; } -- 2.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 97852] Unreal Engine corrupted preview viewport
https://bugs.freedesktop.org/show_bug.cgi?id=97852 --- Comment #9 from Grazvydas Ignotas--- A game "Refunct" also suffers from this when "post process" setting is set to "ultra". Since this limitation is rather artificial, and multiple released programs are affected, could it be demoted to a warning? -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Wed, Jan 4, 2017 at 12:56 AM, Timothy Arceriwrote: > On Wed, 2017-01-04 at 00:44 +0100, Marek Olšák wrote: >> On Wed, Jan 4, 2017 at 12:36 AM, Timothy Arceri >> wrote: >> > On Tue, 2017-01-03 at 23:55 +0100, Marek Olšák wrote: >> > > On Tue, Jan 3, 2017 at 11:14 PM, Timothy Arceri >> > > wrote: >> > > > On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: >> > > > > On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri >> > > > > wrote: >> > > > > > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: >> > > > > > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri >> > > > > > > wrote: >> > > > > > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: >> > > > > > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >> > > > > > > > > wrote: >> > > > > > > > > > We only need to set it when linking was successful >> > > > > > > > > > and >> > > > > > > > > > the >> > > > > > > > > > program >> > > > > > > > > > being linked is currently active. >> > > > > > > > > > >> > > > > > > > > > The programs_in_use mask is just used as a flag for >> > > > > > > > > > now >> > > > > > > > > > but >> > > > > > > > > > in >> > > > > > > > > > a following patch we will use it to update the >> > > > > > > > > > CurrentProgram >> > > > > > > > > > array. >> > > > > > > > > > --- >> > > > > > > > > > src/mesa/main/shaderapi.c | 22 >> > > > > > > > > > +- >> > > > > > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) >> > > > > > > > > > >> > > > > > > > > > diff --git a/src/mesa/main/shaderapi.c >> > > > > > > > > > b/src/mesa/main/shaderapi.c >> > > > > > > > > > index 022afab..6c03dcb 100644 >> > > > > > > > > > --- a/src/mesa/main/shaderapi.c >> > > > > > > > > > +++ b/src/mesa/main/shaderapi.c >> > > > > > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct >> > > > > > > > > > gl_context >> > > > > > > > > > *ctx, >> > > > > > > > > > struct gl_shader_program *shProg) >> > > > > > > > > >return; >> > > > > > > > > > } >> > > > > > > > > > >> > > > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > > > > > + unsigned programs_in_use = 0; >> > > > > > > > > > + if (ctx->_Shader) >> > > > > > > > > > + for (unsigned stage = 0; stage < >> > > > > > > > > > MESA_SHADER_STAGES; >> > > > > > > > > > stage++) { >> > > > > > > > > > + if (ctx->_Shader->CurrentProgram[stage] >> > > > > > > > > > == >> > > > > > > > > > shProg) { >> > > > > > > > > > +programs_in_use |= 1 << stage; >> > > > > > > > > > + } >> > > > > > > > > > + } >> > > > > > > > > > >> > > > > > > > > > _mesa_glsl_link_shader(ctx, shProg); >> > > > > > > > > > >> > > > > > > > > > + /* From section 7.3 (Program Objects) of the >> > > > > > > > > > OpenGL >> > > > > > > > > > 4.5 >> > > > > > > > > > spec: >> > > > > > > > > > +* >> > > > > > > > > > +*"If LinkProgram or ProgramBinary >> > > > > > > > > > successfully >> > > > > > > > > > re- >> > > > > > > > > > links a >> > > > > > > > > > program >> > > > > > > > > > +* object that is active for any shader >> > > > > > > > > > stage, >> > > > > > > > > > then >> > > > > > > > > > the >> > > > > > > > > > newly generated >> > > > > > > > > > +* executable code will be installed as >> > > > > > > > > > part of >> > > > > > > > > > the >> > > > > > > > > > current >> > > > > > > > > > rendering >> > > > > > > > > > +* state for all shader stages where the >> > > > > > > > > > program is >> > > > > > > > > > active. >> > > > > > > > > > +* Additionally, the newly generated >> > > > > > > > > > executable >> > > > > > > > > > code is >> > > > > > > > > > made part of >> > > > > > > > > > +* the state of any program pipeline for >> > > > > > > > > > all >> > > > > > > > > > stages >> > > > > > > > > > where >> > > > > > > > > > the program >> > > > > > > > > > +* is attached." >> > > > > > > > > > +*/ >> > > > > > > > > > + if (shProg->data->LinkStatus && >> > > > > > > > > > programs_in_use) { >> > > > > > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > > > > > + } >> > > > > > > > > >> > > > > > > > > This doesn't seem correct. If the context has >> > > > > > > > > unflushed >> > > > > > > > > vertices, >> > > > > > > > > calling FLUSH_VERTICES after linking will use the new >> > > > > > > > > linked >> > > > > > > > > program >> > > > > > > > > instead of the previous program for which the >> > > > > > > > > vertices >> > > > > > > > > were >> > > > > > > > > submitted. >> > > > > > > > >> > > > > > > > Your probably right but this doesn't make anything >> > > > > > > > worse >> > > > > > > > than >> > > > > > > > it >> > > > > > > > already is right? >> > > > > > > > >> > > > > > > > Before this change we where always calling >> > > > > > > >
[Mesa-dev] [Bug 99244] hud_context.c:874:45: error: ‘W_OK’ undeclared
https://bugs.freedesktop.org/show_bug.cgi?id=99244 Vinson Leechanged: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Vinson Lee --- commit cb6f49a902cae1b4df795c0e611526dca467a042 Author: Marek Olšák Date: Mon Jan 2 23:16:48 2017 +0100 gallium/hud: fix the windows build by disabling file dumping -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Wed, 2017-01-04 at 00:44 +0100, Marek Olšák wrote: > On Wed, Jan 4, 2017 at 12:36 AM, Timothy Arceri >wrote: > > On Tue, 2017-01-03 at 23:55 +0100, Marek Olšák wrote: > > > On Tue, Jan 3, 2017 at 11:14 PM, Timothy Arceri > > > wrote: > > > > On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: > > > > > On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri > > > > > wrote: > > > > > > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: > > > > > > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri > > > > > > > wrote: > > > > > > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: > > > > > > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri > > > > > > > > > wrote: > > > > > > > > > > We only need to set it when linking was successful > > > > > > > > > > and > > > > > > > > > > the > > > > > > > > > > program > > > > > > > > > > being linked is currently active. > > > > > > > > > > > > > > > > > > > > The programs_in_use mask is just used as a flag for > > > > > > > > > > now > > > > > > > > > > but > > > > > > > > > > in > > > > > > > > > > a following patch we will use it to update the > > > > > > > > > > CurrentProgram > > > > > > > > > > array. > > > > > > > > > > --- > > > > > > > > > > src/mesa/main/shaderapi.c | 22 > > > > > > > > > > +- > > > > > > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > diff --git a/src/mesa/main/shaderapi.c > > > > > > > > > > b/src/mesa/main/shaderapi.c > > > > > > > > > > index 022afab..6c03dcb 100644 > > > > > > > > > > --- a/src/mesa/main/shaderapi.c > > > > > > > > > > +++ b/src/mesa/main/shaderapi.c > > > > > > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct > > > > > > > > > > gl_context > > > > > > > > > > *ctx, > > > > > > > > > > struct gl_shader_program *shProg) > > > > > > > > > > return; > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > > > > > + unsigned programs_in_use = 0; > > > > > > > > > > + if (ctx->_Shader) > > > > > > > > > > + for (unsigned stage = 0; stage < > > > > > > > > > > MESA_SHADER_STAGES; > > > > > > > > > > stage++) { > > > > > > > > > > + if (ctx->_Shader->CurrentProgram[stage] > > > > > > > > > > == > > > > > > > > > > shProg) { > > > > > > > > > > +programs_in_use |= 1 << stage; > > > > > > > > > > + } > > > > > > > > > > + } > > > > > > > > > > > > > > > > > > > > _mesa_glsl_link_shader(ctx, shProg); > > > > > > > > > > > > > > > > > > > > + /* From section 7.3 (Program Objects) of the > > > > > > > > > > OpenGL > > > > > > > > > > 4.5 > > > > > > > > > > spec: > > > > > > > > > > +* > > > > > > > > > > +*"If LinkProgram or ProgramBinary > > > > > > > > > > successfully > > > > > > > > > > re- > > > > > > > > > > links a > > > > > > > > > > program > > > > > > > > > > +* object that is active for any shader > > > > > > > > > > stage, > > > > > > > > > > then > > > > > > > > > > the > > > > > > > > > > newly generated > > > > > > > > > > +* executable code will be installed as > > > > > > > > > > part of > > > > > > > > > > the > > > > > > > > > > current > > > > > > > > > > rendering > > > > > > > > > > +* state for all shader stages where the > > > > > > > > > > program is > > > > > > > > > > active. > > > > > > > > > > +* Additionally, the newly generated > > > > > > > > > > executable > > > > > > > > > > code is > > > > > > > > > > made part of > > > > > > > > > > +* the state of any program pipeline for > > > > > > > > > > all > > > > > > > > > > stages > > > > > > > > > > where > > > > > > > > > > the program > > > > > > > > > > +* is attached." > > > > > > > > > > +*/ > > > > > > > > > > + if (shProg->data->LinkStatus && > > > > > > > > > > programs_in_use) { > > > > > > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > > > > > + } > > > > > > > > > > > > > > > > > > This doesn't seem correct. If the context has > > > > > > > > > unflushed > > > > > > > > > vertices, > > > > > > > > > calling FLUSH_VERTICES after linking will use the new > > > > > > > > > linked > > > > > > > > > program > > > > > > > > > instead of the previous program for which the > > > > > > > > > vertices > > > > > > > > > were > > > > > > > > > submitted. > > > > > > > > > > > > > > > > Your probably right but this doesn't make anything > > > > > > > > worse > > > > > > > > than > > > > > > > > it > > > > > > > > already is right? > > > > > > > > > > > > > > > > Before this change we where always calling > > > > > > > > FLUSH_VERTICES(ctx, > > > > > > > > _NEW_PROGRAM); > > > > > > > > > > > > > > The useless flagging is a different issue. > > > > > > > > > > > > > > FLUSH_VERTICES should be called before a
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Wed, Jan 4, 2017 at 12:36 AM, Timothy Arceriwrote: > On Tue, 2017-01-03 at 23:55 +0100, Marek Olšák wrote: >> On Tue, Jan 3, 2017 at 11:14 PM, Timothy Arceri >> wrote: >> > On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: >> > > On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri >> > > wrote: >> > > > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: >> > > > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri >> > > > > wrote: >> > > > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: >> > > > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >> > > > > > > wrote: >> > > > > > > > We only need to set it when linking was successful and >> > > > > > > > the >> > > > > > > > program >> > > > > > > > being linked is currently active. >> > > > > > > > >> > > > > > > > The programs_in_use mask is just used as a flag for now >> > > > > > > > but >> > > > > > > > in >> > > > > > > > a following patch we will use it to update the >> > > > > > > > CurrentProgram >> > > > > > > > array. >> > > > > > > > --- >> > > > > > > > src/mesa/main/shaderapi.c | 22 +- >> > > > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) >> > > > > > > > >> > > > > > > > diff --git a/src/mesa/main/shaderapi.c >> > > > > > > > b/src/mesa/main/shaderapi.c >> > > > > > > > index 022afab..6c03dcb 100644 >> > > > > > > > --- a/src/mesa/main/shaderapi.c >> > > > > > > > +++ b/src/mesa/main/shaderapi.c >> > > > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct >> > > > > > > > gl_context >> > > > > > > > *ctx, >> > > > > > > > struct gl_shader_program *shProg) >> > > > > > > >return; >> > > > > > > > } >> > > > > > > > >> > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > > > + unsigned programs_in_use = 0; >> > > > > > > > + if (ctx->_Shader) >> > > > > > > > + for (unsigned stage = 0; stage < >> > > > > > > > MESA_SHADER_STAGES; >> > > > > > > > stage++) { >> > > > > > > > + if (ctx->_Shader->CurrentProgram[stage] == >> > > > > > > > shProg) { >> > > > > > > > +programs_in_use |= 1 << stage; >> > > > > > > > + } >> > > > > > > > + } >> > > > > > > > >> > > > > > > > _mesa_glsl_link_shader(ctx, shProg); >> > > > > > > > >> > > > > > > > + /* From section 7.3 (Program Objects) of the OpenGL >> > > > > > > > 4.5 >> > > > > > > > spec: >> > > > > > > > +* >> > > > > > > > +*"If LinkProgram or ProgramBinary successfully >> > > > > > > > re- >> > > > > > > > links a >> > > > > > > > program >> > > > > > > > +* object that is active for any shader stage, >> > > > > > > > then >> > > > > > > > the >> > > > > > > > newly generated >> > > > > > > > +* executable code will be installed as part of >> > > > > > > > the >> > > > > > > > current >> > > > > > > > rendering >> > > > > > > > +* state for all shader stages where the >> > > > > > > > program is >> > > > > > > > active. >> > > > > > > > +* Additionally, the newly generated executable >> > > > > > > > code is >> > > > > > > > made part of >> > > > > > > > +* the state of any program pipeline for all >> > > > > > > > stages >> > > > > > > > where >> > > > > > > > the program >> > > > > > > > +* is attached." >> > > > > > > > +*/ >> > > > > > > > + if (shProg->data->LinkStatus && programs_in_use) { >> > > > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > > > + } >> > > > > > > >> > > > > > > This doesn't seem correct. If the context has unflushed >> > > > > > > vertices, >> > > > > > > calling FLUSH_VERTICES after linking will use the new >> > > > > > > linked >> > > > > > > program >> > > > > > > instead of the previous program for which the vertices >> > > > > > > were >> > > > > > > submitted. >> > > > > > >> > > > > > Your probably right but this doesn't make anything worse >> > > > > > than >> > > > > > it >> > > > > > already is right? >> > > > > > >> > > > > > Before this change we where always calling >> > > > > > FLUSH_VERTICES(ctx, >> > > > > > _NEW_PROGRAM); >> > > > > >> > > > > The useless flagging is a different issue. >> > > > > >> > > > > FLUSH_VERTICES should be called before a state change, >> > > > > because it >> > > > > flushes draw calls that the driver hasn't even seen yet. >> > > > > After >> > > > > the >> > > > > driver processes those draw calls, _NEW_PROGRAM is set. >> > > > > That's >> > > > > how it >> > > > > works. >> > > > > >> > > > > If you instead change a state and then call FLUSH_VERTICES, >> > > > > the >> > > > > previous unflushed draw calls will use the new state, which >> > > > > is >> > > > > incorrect. >> > > > >> > > > But the state doesn't change just from linking does it? The new >> > > > programs won't be made current until glUseProgram() is called. >> > > >> > > The GL state changes if the
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, 2017-01-03 at 23:55 +0100, Marek Olšák wrote: > On Tue, Jan 3, 2017 at 11:14 PM, Timothy Arceri >wrote: > > On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: > > > On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri > > > wrote: > > > > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: > > > > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri > > > > > wrote: > > > > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: > > > > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri > > > > > > > wrote: > > > > > > > > We only need to set it when linking was successful and > > > > > > > > the > > > > > > > > program > > > > > > > > being linked is currently active. > > > > > > > > > > > > > > > > The programs_in_use mask is just used as a flag for now > > > > > > > > but > > > > > > > > in > > > > > > > > a following patch we will use it to update the > > > > > > > > CurrentProgram > > > > > > > > array. > > > > > > > > --- > > > > > > > > src/mesa/main/shaderapi.c | 22 +- > > > > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/src/mesa/main/shaderapi.c > > > > > > > > b/src/mesa/main/shaderapi.c > > > > > > > > index 022afab..6c03dcb 100644 > > > > > > > > --- a/src/mesa/main/shaderapi.c > > > > > > > > +++ b/src/mesa/main/shaderapi.c > > > > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct > > > > > > > > gl_context > > > > > > > > *ctx, > > > > > > > > struct gl_shader_program *shProg) > > > > > > > > return; > > > > > > > > } > > > > > > > > > > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > > > + unsigned programs_in_use = 0; > > > > > > > > + if (ctx->_Shader) > > > > > > > > + for (unsigned stage = 0; stage < > > > > > > > > MESA_SHADER_STAGES; > > > > > > > > stage++) { > > > > > > > > + if (ctx->_Shader->CurrentProgram[stage] == > > > > > > > > shProg) { > > > > > > > > +programs_in_use |= 1 << stage; > > > > > > > > + } > > > > > > > > + } > > > > > > > > > > > > > > > > _mesa_glsl_link_shader(ctx, shProg); > > > > > > > > > > > > > > > > + /* From section 7.3 (Program Objects) of the OpenGL > > > > > > > > 4.5 > > > > > > > > spec: > > > > > > > > +* > > > > > > > > +*"If LinkProgram or ProgramBinary successfully > > > > > > > > re- > > > > > > > > links a > > > > > > > > program > > > > > > > > +* object that is active for any shader stage, > > > > > > > > then > > > > > > > > the > > > > > > > > newly generated > > > > > > > > +* executable code will be installed as part of > > > > > > > > the > > > > > > > > current > > > > > > > > rendering > > > > > > > > +* state for all shader stages where the > > > > > > > > program is > > > > > > > > active. > > > > > > > > +* Additionally, the newly generated executable > > > > > > > > code is > > > > > > > > made part of > > > > > > > > +* the state of any program pipeline for all > > > > > > > > stages > > > > > > > > where > > > > > > > > the program > > > > > > > > +* is attached." > > > > > > > > +*/ > > > > > > > > + if (shProg->data->LinkStatus && programs_in_use) { > > > > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > > > + } > > > > > > > > > > > > > > This doesn't seem correct. If the context has unflushed > > > > > > > vertices, > > > > > > > calling FLUSH_VERTICES after linking will use the new > > > > > > > linked > > > > > > > program > > > > > > > instead of the previous program for which the vertices > > > > > > > were > > > > > > > submitted. > > > > > > > > > > > > Your probably right but this doesn't make anything worse > > > > > > than > > > > > > it > > > > > > already is right? > > > > > > > > > > > > Before this change we where always calling > > > > > > FLUSH_VERTICES(ctx, > > > > > > _NEW_PROGRAM); > > > > > > > > > > The useless flagging is a different issue. > > > > > > > > > > FLUSH_VERTICES should be called before a state change, > > > > > because it > > > > > flushes draw calls that the driver hasn't even seen yet. > > > > > After > > > > > the > > > > > driver processes those draw calls, _NEW_PROGRAM is set. > > > > > That's > > > > > how it > > > > > works. > > > > > > > > > > If you instead change a state and then call FLUSH_VERTICES, > > > > > the > > > > > previous unflushed draw calls will use the new state, which > > > > > is > > > > > incorrect. > > > > > > > > But the state doesn't change just from linking does it? The new > > > > programs won't be made current until glUseProgram() is called. > > > > > > The GL state changes if the program is current and re-linked. > > > (not > > > sure if all of the internal Mesa state changes too) > > > > Right but this patch only changes things so that FLUSH_VERTICES is > > not > > called when
Re: [Mesa-dev] [RFC] loader: Automatic PRIME detection
On 02/01/2017 15:32, Thierry Reding wrote: On Tue, Dec 27, 2016 at 01:57:21PM +0100, Axel Davy wrote: Hi Thierry, Could you explain why in this situation default_fd would be a non-renderable device node ? This is preparatory work to allow drivers for split display/render setups to be tied together within Mesa. If you want accelerated rendering on those setups, you currently need to patch applications so that they can deal with two separate devices (patches exist to allow this for Weston and kmscube, and possibly others). In order to avoid having to patch applications, the renderonly (the name is slightly confusing) drivers (Christian Gmeiner sent out patches a few weeks ago) open two devices internally and contain some magic to share buffers between the two devices. So the typical use-case would be that you have two separate DRM devices, one representing the scanout device and another representing the GPU. In most cases the scanout device will be display-only, so it will have a /dev/dri/cardX node, but no corresponding /dev/dri/renderDY node. On the other hand the GPU will have a "dummy" /dev/dri/cardX node that doesn't expose any outputs and a corresponding /dev/dri/renderDY node that is used for rendering. A bare-metal application (kmscube, Weston, ...) will have to find a node to perform a modeset with, so it will have to find a /dev/dri/cardX node which exposes one or more outputs. That will be the scanout device node. But given that there's no render node there's no way to accelerate using that device. Composite drivers will bind to the scanout device node and find a render node for the GPU (these are usually ARM SoCs, so it's fairly easy to find the correct render node) and bind the GPU driver to that node. Then the GPU driver will export buffers used for scanout and have the scanout driver import them. That way the application can transparently use those buffers for DRM/KMS. In my understanding, for X11 DRI3 the Xserver is supposed to give you a renderable device node, and for Wayland, the device path advertised is the one used by the server for compositing. Both X11 and Wayland will try to use the device used by the server for compositing. In both cases they will receive the /dev/dri/cardX device node for the scanout device. Effectively X11 and Wayland will call back into the scanout driver for acceleration. However with a minimal renderonly driver that's not going to work. I had posted a complete wrapper driver a long time ago which I think could be improved to allow even this use-case to work, but I got significant pushback. Anyway, both X11 and Wayland use the loader_get_user_preferred_fd() function that this patch modifies to allow overriding the final file descriptor to use. Currently the only way to override is by providing the DRI_PRIME environment variable or by setting up a dri.conf configuration file. loader_get_user_preferred_fd() returns a file descriptor (same as the default_fd by default, or the one referring to the DRI_PRIME node) and a flag that specifies whether or not the devices are the same. This is used in order to determine if DMA-BUF or FLINK should be used to share buffers. X11 has special code paths to deal with the PRIME use-case involving an extra blit, which is somewhat unfortunate because on many devices that's not going to be necessary. For Wayland the EGL platform code checks for availability of a render node to determine whether or not DMA-BUF should be used. That breaks for this particular case as well because we do have a cardX node without a corresponding renderDY node. I suspect that with a full wrapper driver this could be solved more nicely, including avoiding the extra blit in X11 DRI3. However it's a lot more code to maintain than piggy-backing on top of PRIME support, and a lot more complicated to get right (and more potential for breaking existing use-cases). Thierry From what you say, I understand the following, please correct if wrong: Tthe issue is that you need some special format/placement in order to display on the screen. A patch serie was proposed to handle that with some specific handling long ago, but received bad comments. Because a linear buffer is ok to display, you want to force the "different device is rendering" path to enforce a copy to a linear buffer before presentation. In the proposed scheme, X11 DRI3 open returns an fd of the device used for display (but which cannot render) and Wayland gives the path of the device used for display but which cannot be used for rendering. This seems something that should really be discussed with the devs of #xorg-devel and #wayland. My opinion is that is feels contrary to the spirit of X11 DRI3 to give an unusable device fd, and of Wayland to give the path of the device used for display by the server instead of the device the server uses for rendering. What you seem to want is some sort of constraints format/placement/tiling on the buffers shared with the
[Mesa-dev] [Bug 98604] [VDPAU, DRI3] Fullscreen flash video fails when hardware acceleration is enabled.
https://bugs.freedesktop.org/show_bug.cgi?id=98604 --- Comment #10 from smoki--- (In reply to Chris Rankin from comment #9) > (In reply to smoki from comment #8) > > Does this still work for you with DRI2? Asking becuase DRI2 seems now > > affected also. > > I can't say, because Adobe has now "upgraded" its Flash plugin to v24 and > removed the hardware-accelerated v11 plugin from the Yum repository :-| Yeah it is not have decode accel anymore, but issue is still there... it actually does not need to be VDPAU used just that hardware accel enabled. Asking as i just tried it now and now happens with DRI2 too, basically flash whatever default with our driver crashing browser... one just need to wait enough :D Interesting is that it works fine with AMD proprietary driver, so it is our bug likely... mesa probably. -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, Jan 3, 2017 at 11:14 PM, Timothy Arceriwrote: > On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: >> On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri >> wrote: >> > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: >> > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri >> > > wrote: >> > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: >> > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >> > > > > wrote: >> > > > > > We only need to set it when linking was successful and the >> > > > > > program >> > > > > > being linked is currently active. >> > > > > > >> > > > > > The programs_in_use mask is just used as a flag for now but >> > > > > > in >> > > > > > a following patch we will use it to update the >> > > > > > CurrentProgram >> > > > > > array. >> > > > > > --- >> > > > > > src/mesa/main/shaderapi.c | 22 +- >> > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) >> > > > > > >> > > > > > diff --git a/src/mesa/main/shaderapi.c >> > > > > > b/src/mesa/main/shaderapi.c >> > > > > > index 022afab..6c03dcb 100644 >> > > > > > --- a/src/mesa/main/shaderapi.c >> > > > > > +++ b/src/mesa/main/shaderapi.c >> > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct >> > > > > > gl_context >> > > > > > *ctx, >> > > > > > struct gl_shader_program *shProg) >> > > > > >return; >> > > > > > } >> > > > > > >> > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > + unsigned programs_in_use = 0; >> > > > > > + if (ctx->_Shader) >> > > > > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; >> > > > > > stage++) { >> > > > > > + if (ctx->_Shader->CurrentProgram[stage] == >> > > > > > shProg) { >> > > > > > +programs_in_use |= 1 << stage; >> > > > > > + } >> > > > > > + } >> > > > > > >> > > > > > _mesa_glsl_link_shader(ctx, shProg); >> > > > > > >> > > > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 >> > > > > > spec: >> > > > > > +* >> > > > > > +*"If LinkProgram or ProgramBinary successfully re- >> > > > > > links a >> > > > > > program >> > > > > > +* object that is active for any shader stage, then >> > > > > > the >> > > > > > newly generated >> > > > > > +* executable code will be installed as part of the >> > > > > > current >> > > > > > rendering >> > > > > > +* state for all shader stages where the program is >> > > > > > active. >> > > > > > +* Additionally, the newly generated executable >> > > > > > code is >> > > > > > made part of >> > > > > > +* the state of any program pipeline for all stages >> > > > > > where >> > > > > > the program >> > > > > > +* is attached." >> > > > > > +*/ >> > > > > > + if (shProg->data->LinkStatus && programs_in_use) { >> > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > > > + } >> > > > > >> > > > > This doesn't seem correct. If the context has unflushed >> > > > > vertices, >> > > > > calling FLUSH_VERTICES after linking will use the new linked >> > > > > program >> > > > > instead of the previous program for which the vertices were >> > > > > submitted. >> > > > >> > > > Your probably right but this doesn't make anything worse than >> > > > it >> > > > already is right? >> > > > >> > > > Before this change we where always calling FLUSH_VERTICES(ctx, >> > > > _NEW_PROGRAM); >> > > >> > > The useless flagging is a different issue. >> > > >> > > FLUSH_VERTICES should be called before a state change, because it >> > > flushes draw calls that the driver hasn't even seen yet. After >> > > the >> > > driver processes those draw calls, _NEW_PROGRAM is set. That's >> > > how it >> > > works. >> > > >> > > If you instead change a state and then call FLUSH_VERTICES, the >> > > previous unflushed draw calls will use the new state, which is >> > > incorrect. >> > >> > But the state doesn't change just from linking does it? The new >> > programs won't be made current until glUseProgram() is called. >> >> The GL state changes if the program is current and re-linked. (not >> sure if all of the internal Mesa state changes too) > > Right but this patch only changes things so that FLUSH_VERTICES is not > called when the program is *not* current. That idea is OK. The problem is the patch also moves FLUSH_VERTICES after the _mesa_glsl_link_shader call, which breaks the behavior if the program *is* current. Consider this: glUseProgram(); glBegin(); glVertex(); glEnd(); // the vbo module doesn't create a draw call in glEnd; instead, it waits // for another glBegin call and the driver doesn't see any draw calls yet. glDetachShader(); glAttachShader(); glLinkProgram() { _mesa_glsl_link_shader(); // this replaces the current program FLUSH_VERTICES(); // this finally creates the draw call from Begin/End, // but with the new program } The correct
[Mesa-dev] [Bug 98604] [VDPAU, DRI3] Fullscreen flash video fails when hardware acceleration is enabled.
https://bugs.freedesktop.org/show_bug.cgi?id=98604 --- Comment #9 from Chris Rankin--- (In reply to smoki from comment #8) > Does this still work for you with DRI2? Asking becuase DRI2 seems now > affected also. I can't say, because Adobe has now "upgraded" its Flash plugin to v24 and removed the hardware-accelerated v11 plugin from the Yum repository :-| -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/32] gbm: Introduce modifiers into surface/bo creation
On 17-01-02 18:37:02, Ben Widawsky wrote: The idea behind modifiers like this is that the user of GBM will have some mechanism to query what properties the hardware supports for its BO or surface. This information is directly passed in (and stored) so that the DRI implementation can create an image with the appropriate attributes. A getter() will be added later so that the user GBM will be able to query what modifier should be used. I've opted to store all modifiers passed in during creation and to make the determination happen at actual creation time for no reason other than it seems more flexible. v2: Make sure to check if count is non-zero in addition to testing if calloc fails. (Daniel) v3: Remove "usage" and "flags" from modifier creation. Requested by Kristian. Cc: Kristian HøgsbergCc: Daniel Stone Signed-off-by: Ben Widawsky Reviewed-by: Eric Engestrom Acked-by: Daniel Stone --- [snip] + +GBM_EXPORT struct gbm_surface * +gbm_surface_create_with_modifiers(struct gbm_device *gbm, + uint32_t width, uint32_t height, + uint32_t format, uint32_t flags, + const uint64_t *modifiers, const unsigned int count) +{ Kristian, I missed dropping flags from gbm surface creation here. It's fixed locally. [snip] ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 98604] [VDPAU, DRI3] Fullscreen flash video fails when hardware acceleration is enabled.
https://bugs.freedesktop.org/show_bug.cgi?id=98604 --- Comment #8 from smoki--- (In reply to Chris Rankin from comment #5) > I don't see your point here. AFAIK the Flash plugin knows nothing about > either DRI2 or DRI3, and it works fine with DRI2... Does this still work for you with DRI2? Asking becuase DRI2 seems now affected also. -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/25] Enable Float64 capability support for Intel's Vulkan driver
14-18 and 22 are Reviewed-by: Jason EkstrandI'd like to see the way we're enabling it for SPIR-V reworked to use the new struct Dave added. Overall, looks really good. --Jason On Tue, Jan 3, 2017 at 4:32 AM, Samuel Iglesias Gonsálvez < sigles...@igalia.com> wrote: > On Mon, 2017-01-02 at 10:20 -0800, Jason Ekstrand wrote: > > 1-5 and 7-11 are > > > > Reviewed-by: Jason Ekstrand > > > > I left a few trivial comments on them but nothing substantial. > > > > OK, I will fix them. > > Thanks for the review, > > Sam > > > On Fri, Dec 16, 2016 at 6:48 AM, Juan A. Suarez Romero > > wrote: > > > This patch series is a second iteration of previous one: > > > > > > https://lists.freedesktop.org/archives/mesa-dev/2016-November/13650 > > > 7.html > > > > > > Main changes are the ones suggested by Jason, and also a refactor > > > of the way > > > inputs_read bitmap is used in NIR. > > > > > > If you want to test these patches, you can clone our branch with > > > the following > > > command: > > > > > >$ git clone -b spirv-to-nir-rc2 https://github.com/Igalia/mesa.g > > > ithub > > > > > > Thanks, > > > > > > J.A. > > > > > > > > > > > > Juan A. Suarez Romero (2): > > > anv/pipeline: get map for double input attributes > > > nir/i965: use two slots from inputs_read for dvec3/dvec4 vertex > > > input > > > attributes > > > > > > Samuel Iglesias Gonsálvez (23): > > > spirv: fix typo in spec_constant_decoration_cb() > > > spirv: add definition of double based data types > > > spirv: add support for loading DF constants > > > spirv: add DF support to vtn_const_ssa_value() > > > spirv: add DF support to SpvOp*ConstantComposite > > > spirv: fix SpvOpSpecConstantOp with SpvOpVectorShuffle working > > > with > > > double-based vecs > > > spirv: add double support to SpvOpCompositeExtract > > > spirv: add double support to _vtn_variable_load_store > > > spirv: add double support to _vtn_block_load_store() > > > spirv: Enable double floating points when copying variables in > > > _vtn_variable_copy() > > > spirv: add support for doubles on OpComposite{Insert,Extract} > > > compiler/nir: add glsl_type_is_{float,integer}() > > > nir: add nir_get_nir_type_for_glsl_type() > > > nir: add nir_type_conversion_op() > > > spirv/nir: implement DF conversions > > > spirv/nir: add (un)packDouble2x32() translation > > > spirv: add support for doubles to OpSpecConstant > > > isl: fix VA64 support for double and dvecN vertex attributes > > > nir: Add flag to detect platforms with native float64 support > > > spirv: Add nir_options to vtn_builder > > > spirv: enable SpvCapabilityFloat64 only to supported platforms > > > i965: enable nir_option's native_float64 to supported generations > > > anv: enable shaderFloat64 feature > > > > > > src/amd/vulkan/radv_pipeline.c | 5 +- > > > src/compiler/glsl/glsl_to_nir.cpp| 28 ++ > > > src/compiler/nir/nir.c | 83 > > > > > > src/compiler/nir/nir.h | 26 + > > > src/compiler/nir/nir_gather_info.c | 48 + > > > src/compiler/nir_types.cpp | 15 +++ > > > src/compiler/nir_types.h | 2 + > > > src/compiler/spirv/nir_spirv.h | 5 +- > > > src/compiler/spirv/spirv_to_nir.c| 141 > > > ++- > > > src/compiler/spirv/vtn_alu.c | 29 +++--- > > > src/compiler/spirv/vtn_glsl450.c | 2 + > > > src/compiler/spirv/vtn_private.h | 4 +- > > > src/compiler/spirv/vtn_variables.c | 3 + > > > src/intel/isl/isl_format.c | 4 +- > > > src/intel/isl/isl_format_layout.csv | 1 - > > > src/intel/vulkan/anv_device.c| 2 +- > > > src/intel/vulkan/anv_formats.c | 8 +- > > > src/intel/vulkan/anv_pipeline.c | 6 +- > > > src/intel/vulkan/genX_pipeline.c | 63 +++- > > > src/mesa/drivers/dri/i965/brw_compiler.c | 36 --- > > > src/mesa/drivers/dri/i965/brw_draw_upload.c | 11 ++- > > > src/mesa/drivers/dri/i965/brw_fs.cpp | 13 --- > > > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 3 +- > > > src/mesa/drivers/dri/i965/brw_nir.c | 6 +- > > > src/mesa/drivers/dri/i965/brw_nir.h | 1 - > > > src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +-- > > > 26 files changed, 419 insertions(+), 137 deletions(-) > > > > > > -- > > > 2.9.3 > > > > > > ___ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > >
Re: [Mesa-dev] [PATCH v2 22/25] nir/i965: use two slots from inputs_read for dvec3/dvec4 vertex input attributes
I made a few pretty trivial comments. With those addressed, Reviewed-by: Jason EkstrandOn Dec 16, 2016 8:55 AM, "Juan A. Suarez Romero" wrote: So far, input_reads was a bitmap tracking which vertex input locations were being used. In OpenGL, an attribute bigger than a vec4 (like a dvec3 or dvec4) consumes just one location, any other small attribute. So we mark the proper bit in inputs_read, and also the same bit in double_inputs_read if the attribute is a dvec3/dvec4. But in Vulkan, this is slightly different: a dvec3/dvec4 attribute consumes two locations, not just one. And hence two bits would be marked in inputs_read for the same vertex input attribute. To avoid handling two different situations in NIR, we just choose the latest one: in OpenGL, when creating NIR from GLSL/IR, any dvec3/dvec4 vertex input attribute is marked with two bits in the inputs_read bitmap (and also in the double_inputs_read), and following attributes are adjusted accordingly. As example, if in our GLSL/IR shader we have three attributes: layout(location = 0) vec3 attr0; layout(location = 1) dvec4 attr1; layout(location = 2) dvec3 attr2; then in our NIR shader we put attr0 in location 0, attr1 in locations 1 and 2, and attr2 in location 3. attr2 goes in locations 3 *and* 4, correct? Checking carefully, basically we are using slots rather than locations in NIR. When emitting the vertices, we do a inverse map to know the corresponding location for each slot. v2 (Jason): - use two slots from inputs_read for dvec3/dvec4 NIR from GLSL/IR. --- src/compiler/glsl/glsl_to_nir.cpp| 28 + src/compiler/nir/nir_gather_info.c | 48 ++--- src/intel/vulkan/genX_pipeline.c | 63 +--- src/mesa/drivers/dri/i965/brw_draw_upload.c | 11 +++-- src/mesa/drivers/dri/i965/brw_fs.cpp | 13 -- src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 3 +- src/mesa/drivers/dri/i965/brw_nir.c | 6 +-- src/mesa/drivers/dri/i965/brw_nir.h | 1 - src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +++-- 9 files changed, 106 insertions(+), 78 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 4debc37..0814dad 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -129,6 +129,19 @@ private: } /* end of anonymous namespace */ +static void +nir_remap_attributes(nir_shader *shader) +{ + nir_foreach_variable(var, >inputs) { + var->data.location += _mesa_bitcount_64(shader->info->double_inputs_read & + BITFIELD64_MASK(var->data.location)); + } + + /* Once the remap is done, reset double_inputs_read, so later it will have +* which location/slots are doubles */ + shader->info->double_inputs_read = 0; +} + nir_shader * glsl_to_nir(const struct gl_shader_program *shader_prog, gl_shader_stage stage, @@ -146,6 +159,13 @@ glsl_to_nir(const struct gl_shader_program *shader_prog, nir_lower_constant_initializers(shader, (nir_variable_mode)~0); + /* Remap the locations to slots so those requiring two slots will occupy +* two locations. For instance, if we have in the IR code a dvec3 attr0 in +* location 0 and vec4 attr1 in location 1, in NIR attr0 will use +* locations/slots 0 and 1, and attr1 will use location/slot 2 */ + if (shader->stage == MESA_SHADER_VERTEX) + nir_remap_attributes(shader); + shader->info->name = ralloc_asprintf(shader, "GLSL%d", shader_prog->Name); if (shader_prog->Label) shader->info->label = ralloc_strdup(shader, shader_prog->Label); @@ -315,6 +335,14 @@ nir_visitor::visit(ir_variable *ir) } else { var->data.mode = nir_var_shader_in; } + + /* Mark all the locations that require two slots */ + if (glsl_type_is_dual_slot(glsl_without_array(var->type))) { + for (uint i = 0; i < glsl_count_attribute_slots(var->type, true); i++) { +uint64_t bitfield = BITFIELD64_BIT(var->data.location + i); +shader->info->double_inputs_read |= bitfield; + } + } break; case ir_var_shader_out: diff --git a/src/compiler/nir/nir_gather_info.c b/src/compiler/nir/nir_gather_info.c index 07c9949..35a1ce4 100644 --- a/src/compiler/nir/nir_gather_info.c +++ b/src/compiler/nir/nir_gather_info.c @@ -53,11 +53,6 @@ set_io_mask(nir_shader *shader, nir_variable *var, int offset, int len) else shader->info->inputs_read |= bitfield; - /* double inputs read is only for vertex inputs */ - if (shader->stage == MESA_SHADER_VERTEX && - glsl_type_is_dual_slot(glsl_without_array(var->type))) -shader->info->double_inputs_read |= bitfield; - if (shader->stage == MESA_SHADER_FRAGMENT) { shader->info->fs.uses_sample_qualifier |= var->data.sample; } @@ -83,26 +78,21 @@ static void
[Mesa-dev] [Bug 99253] Radeon R7 370 / R9 270X/370X PCIe Bus Error: severity=Uncorrected (Fatal)
https://bugs.freedesktop.org/show_bug.cgi?id=99253 Alex Deucherchanged: What|Removed |Added QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org Component|Other |Drivers/Gallium/radeonsi --- Comment #2 from Alex Deucher --- This is a GPU hang. Ideally you'd have some way to reproduceably trigger it. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, 2017-01-03 at 22:41 +0100, Marek Olšák wrote: > On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceri >wrote: > > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: > > > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri > > > wrote: > > > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: > > > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri > > > > > wrote: > > > > > > We only need to set it when linking was successful and the > > > > > > program > > > > > > being linked is currently active. > > > > > > > > > > > > The programs_in_use mask is just used as a flag for now but > > > > > > in > > > > > > a following patch we will use it to update the > > > > > > CurrentProgram > > > > > > array. > > > > > > --- > > > > > > src/mesa/main/shaderapi.c | 22 +- > > > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/src/mesa/main/shaderapi.c > > > > > > b/src/mesa/main/shaderapi.c > > > > > > index 022afab..6c03dcb 100644 > > > > > > --- a/src/mesa/main/shaderapi.c > > > > > > +++ b/src/mesa/main/shaderapi.c > > > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct > > > > > > gl_context > > > > > > *ctx, > > > > > > struct gl_shader_program *shProg) > > > > > > return; > > > > > > } > > > > > > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > + unsigned programs_in_use = 0; > > > > > > + if (ctx->_Shader) > > > > > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; > > > > > > stage++) { > > > > > > + if (ctx->_Shader->CurrentProgram[stage] == > > > > > > shProg) { > > > > > > +programs_in_use |= 1 << stage; > > > > > > + } > > > > > > + } > > > > > > > > > > > > _mesa_glsl_link_shader(ctx, shProg); > > > > > > > > > > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 > > > > > > spec: > > > > > > +* > > > > > > +*"If LinkProgram or ProgramBinary successfully re- > > > > > > links a > > > > > > program > > > > > > +* object that is active for any shader stage, then > > > > > > the > > > > > > newly generated > > > > > > +* executable code will be installed as part of the > > > > > > current > > > > > > rendering > > > > > > +* state for all shader stages where the program is > > > > > > active. > > > > > > +* Additionally, the newly generated executable > > > > > > code is > > > > > > made part of > > > > > > +* the state of any program pipeline for all stages > > > > > > where > > > > > > the program > > > > > > +* is attached." > > > > > > +*/ > > > > > > + if (shProg->data->LinkStatus && programs_in_use) { > > > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > > > + } > > > > > > > > > > This doesn't seem correct. If the context has unflushed > > > > > vertices, > > > > > calling FLUSH_VERTICES after linking will use the new linked > > > > > program > > > > > instead of the previous program for which the vertices were > > > > > submitted. > > > > > > > > Your probably right but this doesn't make anything worse than > > > > it > > > > already is right? > > > > > > > > Before this change we where always calling FLUSH_VERTICES(ctx, > > > > _NEW_PROGRAM); > > > > > > The useless flagging is a different issue. > > > > > > FLUSH_VERTICES should be called before a state change, because it > > > flushes draw calls that the driver hasn't even seen yet. After > > > the > > > driver processes those draw calls, _NEW_PROGRAM is set. That's > > > how it > > > works. > > > > > > If you instead change a state and then call FLUSH_VERTICES, the > > > previous unflushed draw calls will use the new state, which is > > > incorrect. > > > > But the state doesn't change just from linking does it? The new > > programs won't be made current until glUseProgram() is called. > > The GL state changes if the program is current and re-linked. (not > sure if all of the internal Mesa state changes too) Right but this patch only changes things so that FLUSH_VERTICES is not called when the program is *not* current. When glUseProgram (or the SSO equivalents) are called we should end up in shaderapi.c:use_program() which does FLUSH_VERTICES(ctx, _NEW_PROGRAM | _NEW_PROGRAM_CONSTANTS); if the program is not already the current program and there is already a current program in use. > > Marek > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] radeonsi: add a workaround for the Witcher 2 black transitions
Hi Marek This series is Tested-by: Edmondo TommasinaThe black transitions in Witcher 2 are fixed for me. Thanks edmondo On Tue, Jan 3, 2017 at 8:17 PM, Marek Olšák wrote: > From: Marek Olšák > > --- > src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c > b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c > index 996a458..efe28d1 100644 > --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c > +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c > @@ -629,20 +629,29 @@ store_value_to_array(struct lp_build_tgsi_context > *bld_base, > default: > continue; > } > value = LLVMBuildExtractElement(builder, array, > lp_build_const_int32(gallivm, i), ""); > LLVMBuildStore(builder, value, temp_ptr); > } > } > } > > +/* If this is 1, preload FS inputs at the beginning of shaders. Otherwise, > + * reload them at each use. > + * > + * This must be 1 for Witcher 2 to render correctly. The cause of the > Witcher 2 > + * issue is still unknown. I only know that M0 is correct throughout the > whole > + * shader. > + */ > +#define SI_PRELOAD_FS_INPUTS 1 > + > LLVMValueRef si_llvm_emit_fetch(struct lp_build_tgsi_context *bld_base, > const struct tgsi_full_src_register *reg, > enum tgsi_opcode_type type, > unsigned swizzle) > { > struct si_shader_context *ctx = si_shader_context(bld_base); > struct lp_build_tgsi_soa_context *bld = lp_soa_context(bld_base); > LLVMBuilderRef builder = bld_base->base.gallivm->builder; > LLVMValueRef result = NULL, ptr, ptr2; > > @@ -681,21 +690,22 @@ LLVMValueRef si_llvm_emit_fetch(struct > lp_build_tgsi_context *bld_base, > > case TGSI_FILE_INPUT: { > unsigned index = reg->Register.Index; > LLVMValueRef input[4]; > > /* I don't think doing this for vertex shaders is beneficial. > * For those, we want to make sure the VMEM loads are executed > * only once. Fragment shaders don't care much, because > * v_interp instructions are much cheaper than VMEM loads. > */ > - if (ctx->soa.bld_base.info->processor == PIPE_SHADER_FRAGMENT) > + if (!SI_PRELOAD_FS_INPUTS && > + ctx->soa.bld_base.info->processor == PIPE_SHADER_FRAGMENT) > ctx->load_input(ctx, index, >input_decls[index], > input); > else > memcpy(input, >inputs[index * 4], sizeof(input)); > > result = input[swizzle]; > > if (tgsi_type_is_64bit(type)) { > ptr = result; > ptr2 = input[swizzle + 1]; > return si_llvm_emit_fetch_64bit(bld_base, type, ptr, > ptr2); > @@ -874,21 +884,22 @@ static void emit_declaration(struct > lp_build_tgsi_context *bld_base, > { > unsigned idx; > for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) > { > if (ctx->load_input && > ctx->input_decls[idx].Declaration.File != > TGSI_FILE_INPUT) { > ctx->input_decls[idx] = *decl; > ctx->input_decls[idx].Range.First = idx; > ctx->input_decls[idx].Range.Last = idx; > ctx->input_decls[idx].Semantic.Index += idx - > decl->Range.First; > > - if (bld_base->info->processor != > PIPE_SHADER_FRAGMENT) > + if (SI_PRELOAD_FS_INPUTS || > + bld_base->info->processor != > PIPE_SHADER_FRAGMENT) > ctx->load_input(ctx, idx, > >input_decls[idx], > >inputs[idx * > 4]); > } > } > } > break; > > case TGSI_FILE_SYSTEM_VALUE: > { > unsigned idx; > -- > 2.7.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/53] i965: stop passing gl_shader_program to brw_assign_common_binding_table_offsets()
On Tue, 2017-01-03 at 17:39 +, Lionel Landwerlin wrote: > Reviewed-by: Lionel Landwerlin> > A tiny suggestion further down. > > On 03/01/17 02:43, Timothy Arceri wrote: > > We now get eventhing we need directly from gl_program so there is > > Everything? :) > > > no need for this. > > --- > > src/mesa/drivers/dri/i965/brw_cs.c | 5 ++--- > > src/mesa/drivers/dri/i965/brw_gs.c | 6 ++ > > src/mesa/drivers/dri/i965/brw_shader.cpp | 5 - > > src/mesa/drivers/dri/i965/brw_shader.h | 1 - > > src/mesa/drivers/dri/i965/brw_tcs.c | 2 +- > > src/mesa/drivers/dri/i965/brw_tes.c | 2 +- > > src/mesa/drivers/dri/i965/brw_vs.c | 2 +- > > src/mesa/drivers/dri/i965/brw_wm.c | 6 ++ > > 8 files changed, 9 insertions(+), 20 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_cs.c > > b/src/mesa/drivers/dri/i965/brw_cs.c > > index f220846..8658406 100644 > > --- a/src/mesa/drivers/dri/i965/brw_cs.c > > +++ b/src/mesa/drivers/dri/i965/brw_cs.c > > @@ -36,7 +36,6 @@ > > > > static void > > assign_cs_binding_table_offsets(const struct gen_device_info > > *devinfo, > > -const struct gl_shader_program > > *shader_prog, > > const struct gl_program *prog, > > struct brw_cs_prog_data > > *prog_data) > > { > > @@ -47,7 +46,7 @@ assign_cs_binding_table_offsets(const struct > > gen_device_info *devinfo, > > next_binding_table_offset++; > > > > brw_assign_common_binding_table_offsets(MESA_SHADER_COMPUTE, > > devinfo, > > - shader_prog, prog, > > _data->base, > > + prog, _data->base, > > next_binding_table_off > > set); > > } > > > > @@ -81,7 +80,7 @@ brw_codegen_cs_prog(struct brw_context *brw, > > prog_data.base.total_shared = cp- > > >program.info.cs.shared_size; > > } > > > > - assign_cs_binding_table_offsets(devinfo, prog, >program, > > _data); > > + assign_cs_binding_table_offsets(devinfo, >program, > > _data); > > > > /* Allocate the references to the uniforms that will end up in > > the > > * prog_data associated with the compiled program, and which > > will be freed > > diff --git a/src/mesa/drivers/dri/i965/brw_gs.c > > b/src/mesa/drivers/dri/i965/brw_gs.c > > index 7886737..5896726 100644 > > --- a/src/mesa/drivers/dri/i965/brw_gs.c > > +++ b/src/mesa/drivers/dri/i965/brw_gs.c > > @@ -74,7 +74,6 @@ brw_gs_debug_recompile(struct brw_context *brw, > > struct gl_program *prog, > > > > static void > > assign_gs_binding_table_offsets(const struct gen_device_info > > *devinfo, > > -const struct gl_shader_program > > *shader_prog, > > const struct gl_program *prog, > > struct brw_gs_prog_data > > *prog_data) > > { > > @@ -84,8 +83,7 @@ assign_gs_binding_table_offsets(const struct > > gen_device_info *devinfo, > > uint32_t reserved = devinfo->gen == 6 ? BRW_MAX_SOL_BINDINGS : > > 0; > > > > brw_assign_common_binding_table_offsets(MESA_SHADER_GEOMETRY, > > devinfo, > > - shader_prog, prog, > > - _data->base.base, > > + prog, _data- > > >base.base, > > reserved); > > } > > > > @@ -104,7 +102,7 @@ brw_codegen_gs_prog(struct brw_context *brw, > > > > memset(_data, 0, sizeof(prog_data)); > > > > - assign_gs_binding_table_offsets(devinfo, prog, >program, > > _data); > > + assign_gs_binding_table_offsets(devinfo, >program, > > _data); > > > > /* Allocate the references to the uniforms that will end up in > > the > > * prog_data associated with the compiled program, and which > > will be freed > > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp > > b/src/mesa/drivers/dri/i965/brw_shader.cpp > > index 267f5fb..1ada15d 100644 > > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp > > @@ -1181,17 +1181,12 @@ backend_shader::calculate_cfg() > > uint32_t > > brw_assign_common_binding_table_offsets(gl_shader_stage stage, > > Looks like you could even drop the stage argument here (in another > patch > possibly). > It doesn't appear to be used and you could get it from gl_program. Right good spotting, I'll fix in this patch since it removes the last use of it. Thanks a heap for the reviews :) > > > const struct > > gen_device_info *devinfo, > > -const struct > > gl_shader_program *shader_prog, > > const struct gl_program > > *prog, > >
[Mesa-dev] [PATCH v2 5/5] radv: Create single RADV_DEBUG env var.
Also changed RADV_SHOW_QUEUES to a no compute queue option. That would make more sense later when the compute queue is established, but the transfer queue still experimental. v2: Don't include the trace flag. Signed-off-by: Bas Nieuwenhuizen--- src/amd/vulkan/radv_device.c | 45 +--- src/amd/vulkan/radv_image.c | 4 ++-- src/amd/vulkan/radv_meta_clear.c | 2 +- src/amd/vulkan/radv_pipeline.c | 18 +++ src/amd/vulkan/radv_pipeline_cache.c | 2 +- src/amd/vulkan/radv_private.h| 18 --- 6 files changed, 53 insertions(+), 36 deletions(-) diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index ef8ca1a375..05dac8 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -218,6 +218,18 @@ static const VkAllocationCallbacks default_alloc = { .pfnFree = default_free_func, }; +static const struct debug_control radv_debug_options[] = { + {"fastclears", RADV_DEBUG_FAST_CLEARS}, + {"nodcc", RADV_DEBUG_NO_DCC}, + {"shaders", RADV_DEBUG_DUMP_SHADERS}, + {"nocache", RADV_DEBUG_NO_CACHE}, + {"shaderstats", RADV_DEBUG_DUMP_SHADER_STATS}, + {"nohiz", RADV_DEBUG_NO_HIZ}, + {"nocompute", RADV_DEBUG_NO_COMPUTE_QUEUE}, + {"unsafemath", RADV_DEBUG_UNSAFE_MATH}, + {NULL, 0} +}; + VkResult radv_CreateInstance( const VkInstanceCreateInfo* pCreateInfo, const VkAllocationCallbacks*pAllocator, @@ -276,6 +288,9 @@ VkResult radv_CreateInstance( VG(VALGRIND_CREATE_MEMPOOL(instance, 0, false)); + instance->debug_flags = parse_debug_string(getenv("RADV_DEBUG"), + radv_debug_options); + *pInstance = radv_instance_to_handle(instance); return VK_SUCCESS; @@ -555,12 +570,11 @@ void radv_GetPhysicalDeviceQueueFamilyProperties( { RADV_FROM_HANDLE(radv_physical_device, pdevice, physicalDevice); int num_queue_families = 1; - bool all_queues = env_var_as_boolean("RADV_SHOW_QUEUES", true); int idx; - if (all_queues && pdevice->rad_info.chip_class >= CIK) { - if (pdevice->rad_info.compute_rings > 0) - num_queue_families++; - } + if (pdevice->rad_info.compute_rings > 0 && + pdevice->rad_info.chip_class >= CIK && + !(pdevice->instance->debug_flags & RADV_DEBUG_NO_COMPUTE_QUEUE)) + num_queue_families++; if (pQueueFamilyProperties == NULL) { *pCount = num_queue_families; @@ -583,12 +597,9 @@ void radv_GetPhysicalDeviceQueueFamilyProperties( idx++; } - if (!all_queues) { - *pCount = idx; - return; - } - - if (pdevice->rad_info.compute_rings > 0 && pdevice->rad_info.chip_class >= CIK) { + if (pdevice->rad_info.compute_rings > 0 && + pdevice->rad_info.chip_class >= CIK && + !(pdevice->instance->debug_flags & RADV_DEBUG_NO_COMPUTE_QUEUE)) { if (*pCount > idx) { pQueueFamilyProperties[idx] = (VkQueueFamilyProperties) { .queueFlags = VK_QUEUE_COMPUTE_BIT | VK_QUEUE_TRANSFER_BIT, @@ -699,7 +710,8 @@ VkResult radv_CreateDevice( device->_loader_data.loaderMagic = ICD_LOADER_MAGIC; device->instance = physical_device->instance; - device->shader_stats_dump = false; + + device->debug_flags = device->instance->debug_flags; device->ws = physical_device->ws; if (pAllocator) @@ -735,12 +747,6 @@ VkResult radv_CreateDevice( device->ws->ctx_destroy(device->hw_ctx); goto fail; } - device->allow_fast_clears = env_var_as_boolean("RADV_FAST_CLEARS", false); - device->allow_dcc = !env_var_as_boolean("RADV_DCC_DISABLE", false); - device->shader_stats_dump = env_var_as_boolean("RADV_SHADER_STATS", false); - - if (device->allow_fast_clears && device->allow_dcc) - radv_finishme("DCC fast clears have not been tested\n"); radv_device_init_msaa(device); @@ -1652,7 +1658,8 @@ radv_initialise_color_surface(struct radv_device *device, if (iview->image->fmask.size) cb->cb_color_info |= S_028C70_COMPRESSION(1); - if (iview->image->cmask.size && device->allow_fast_clears) + if (iview->image->cmask.size && + (device->debug_flags & RADV_DEBUG_FAST_CLEARS)) cb->cb_color_info |= S_028C70_FAST_CLEAR(1); if (iview->image->surface.dcc_size && level_info->dcc_enabled) diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c index 9c0bba2165..2a41c8e323 100644 --- a/src/amd/vulkan/radv_image.c +++ b/src/amd/vulkan/radv_image.c @@ -113,7 +113,7 @@ radv_init_surface(struct radv_device
[Mesa-dev] [PATCH v2 3/5] radv: Dump command buffer on hang.
v2: - Now use the filename specified by RADV_TRACE_FILE env var. - Use the same var to enable tracing. I thought we could as well always set the filename explicitly instead of having some arbitrary defaults, and at that point we don't need a separate feature enable. Signed-off-by: Bas Nieuwenhuizen--- src/amd/vulkan/radv_cmd_buffer.c | 35 src/amd/vulkan/radv_device.c | 82 --- src/amd/vulkan/radv_private.h | 5 ++ src/amd/vulkan/radv_radeon_winsys.h | 2 + src/amd/vulkan/si_cmd_buffer.c| 5 ++ src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 30 ++ 6 files changed, 150 insertions(+), 9 deletions(-) diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c index fdb35a0060..651b1dd452 100644 --- a/src/amd/vulkan/radv_cmd_buffer.c +++ b/src/amd/vulkan/radv_cmd_buffer.c @@ -32,6 +32,8 @@ #include "vk_format.h" #include "radv_meta.h" +#include "ac_debug.h" + static void radv_handle_image_transition(struct radv_cmd_buffer *cmd_buffer, struct radv_image *image, VkImageLayout src_layout, @@ -272,6 +274,32 @@ radv_cmd_buffer_upload_data(struct radv_cmd_buffer *cmd_buffer, return true; } +void radv_cmd_buffer_trace_emit(struct radv_cmd_buffer *cmd_buffer) +{ + struct radv_device *device = cmd_buffer->device; + struct radeon_winsys_cs *cs = cmd_buffer->cs; + uint64_t va; + + if (!device->trace_bo) + return; + + va = device->ws->buffer_get_va(device->trace_bo); + + MAYBE_UNUSED unsigned cdw_max = radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 7); + + ++cmd_buffer->state.trace_id; + device->ws->cs_add_buffer(cs, device->trace_bo, 8); + radeon_emit(cs, PKT3(PKT3_WRITE_DATA, 3, 0)); + radeon_emit(cs, S_370_DST_SEL(V_370_MEM_ASYNC) | + S_370_WR_CONFIRM(1) | + S_370_ENGINE_SEL(V_370_ME)); + radeon_emit(cs, va); + radeon_emit(cs, va >> 32); + radeon_emit(cs, cmd_buffer->state.trace_id); + radeon_emit(cs, PKT3(PKT3_NOP, 0, 0)); + radeon_emit(cs, AC_ENCODE_TRACE_POINT(cmd_buffer->state.trace_id)); +} + static void radv_emit_graphics_blend_state(struct radv_cmd_buffer *cmd_buffer, struct radv_pipeline *pipeline) @@ -1929,6 +1957,8 @@ void radv_CmdDraw( S_0287F0_USE_OPAQUE(0)); assert(cmd_buffer->cs->cdw <= cdw_max); + + radv_cmd_buffer_trace_emit(cmd_buffer); } static void radv_emit_primitive_reset_index(struct radv_cmd_buffer *cmd_buffer) @@ -1984,6 +2014,7 @@ void radv_CmdDrawIndexed( radeon_emit(cmd_buffer->cs, V_0287F0_DI_SRC_SEL_DMA); assert(cmd_buffer->cs->cdw <= cdw_max); + radv_cmd_buffer_trace_emit(cmd_buffer); } static void @@ -2035,6 +2066,7 @@ radv_emit_indirect_draw(struct radv_cmd_buffer *cmd_buffer, radeon_emit(cs, count_va >> 32); radeon_emit(cs, stride); /* stride */ radeon_emit(cs, di_src_sel); + radv_cmd_buffer_trace_emit(cmd_buffer); } static void @@ -2188,6 +2220,7 @@ void radv_CmdDispatch( radeon_emit(cmd_buffer->cs, 1); assert(cmd_buffer->cs->cdw <= cdw_max); + radv_cmd_buffer_trace_emit(cmd_buffer); } void radv_CmdDispatchIndirect( @@ -2239,6 +2272,7 @@ void radv_CmdDispatchIndirect( } assert(cmd_buffer->cs->cdw <= cdw_max); + radv_cmd_buffer_trace_emit(cmd_buffer); } void radv_unaligned_dispatch( @@ -2292,6 +2326,7 @@ void radv_unaligned_dispatch( S_00B800_PARTIAL_TG_EN(1)); assert(cmd_buffer->cs->cdw <= cdw_max); + radv_cmd_buffer_trace_emit(cmd_buffer); } void radv_CmdEndRenderPass( diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index e57a419cfa..ef8ca1a375 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -760,16 +760,34 @@ VkResult radv_CreateDevice( device->ws->cs_finalize(device->empty_cs[family]); } + if (getenv("RADV_TRACE_FILE")) { + device->trace_bo = device->ws->buffer_create(device->ws, 4096, 8, + RADEON_DOMAIN_VRAM, RADEON_FLAG_CPU_ACCESS); + if (!device->trace_bo) + goto fail; + + device->trace_id_ptr = device->ws->buffer_map(device->trace_bo); + if (!device->trace_id_ptr) + goto fail; + } + *pDevice = radv_device_to_handle(device); return VK_SUCCESS; fail: + if (device->trace_bo) + device->ws->buffer_destroy(device->trace_bo); + for (unsigned i = 0; i < RADV_MAX_QUEUE_FAMILIES; i++) { for (unsigned q = 0; q <
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Create a disable CCS flag
On 17-01-03 13:12:43, Chad Versace wrote: On Tue 03 Jan 2017, Ben Widawsky wrote: On 17-01-03 08:21:06, Chad Versace wrote: > On Sun 01 Jan 2017, Ben Widawsky wrote: > > Cc: Topi Pohjolainen> > Cc: Chad Versace > > Signed-off-by: Ben Widawsky > > --- > > src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- > > src/mesa/drivers/dri/i965/brw_draw.c | 3 ++- > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 21 ++--- > > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++--- > > 4 files changed, 16 insertions(+), 20 deletions(-) > > > > @@ -2334,19 +2334,18 @@ intel_miptree_make_shareable(struct brw_context *brw, > > > > if (mt->mcs_buf) { > >intel_miptree_all_slices_resolve_color(brw, mt, 0); > > - mt->no_ccs = true; > > + mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS); > >drm_intel_bo_unreference(mt->mcs_buf->bo); > >free(mt->mcs_buf); > >mt->mcs_buf = NULL; > > } > > > > if (mt->hiz_buf) { > > + mt->aux_disable |= INTEL_AUX_DISABLE_HIZ; > >intel_miptree_all_slices_resolve_depth(brw, mt); > >intel_miptree_hiz_buffer_free(mt->hiz_buf); > >mt->hiz_buf = NULL; > > } > > - > > - mt->aux_disable = INTEL_AUX_DISABLE_ALL; > > } > > All look goods to me except the above hunk. As written, in some > instances the driver will allocate (and use) the aux surface *after* > intel_miptree_make_shareable() unshares the miptree. To fix that, the > mt->aux_disable bits must be set unconditionally, outside the if's. > Getting unconditional disable is exactly what the patches are trying to get rid of (specifically for the CCS case). So I really hope there is some solution other than putting it outside of the if's. I understand that is the eventual goal. And I see you recently posted the RBC patch series. > The problem is that the driver sometimes allocates the aux surfaces > lazily. For evidence, grep for all instances of > intel_miptree_alloc_hiz() and intel_miptree_alloc_non_msrt_mcs() outside > of intel_mipmap_tree.c. If the code calls intel_miptree_make_shareable() > on a miptree for which the aux surface has not yet been allocated yet, > then intel_miptree_make_shareable() fails to set the disable bit. Hmm. I wasn't able to find a single regression with this patch, and everything seemed pretty okay to me with the code. Can you find a specific failing case, because I cannot? I'm not surprised that the test suites found no regressions. The bug depends on the user create the EGLImage before the miptree's first use. So... I wrote a test and a bug :). And it seems that problem is worse than I thought. The HiZ problem has causes crashes for a long time. And your patch imporoves the situation for color buffers, changing a crash to a fail. [Piglit] egl_khr_gl_image: Add test that clears a shared image https://lists.freedesktop.org/archives/piglit/2017-January/021626.html Bug 99265 - i965: Piglit egl_khr_gl_renderbuffer_image-clear-shared-image fails https://bugs.freedesktop.org/show_bug.cgi?id=99265 Thanks Chad. I'll see if I can fix the bug in a way that allows me to keep what I need in place. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/14] implement EGL_EXT_image_dma_buf_import_modifiers v2
On 16-11-24 20:50:37, Varad Gautam wrote: This is the second revision to the EGL_EXT_image_dma_buf_import_modifiers [1] series at [2], addressing the comments received. This diverges from v1 due to some reordered/squashed changes, hence the resend. 1-4 are Reviewed-by: Ben Widawsky5-9,12-13: See below 10: Not really my thing, and tied up with 9. 14: Acked-by: Ben Widawsky I guess we need to decide whose patches we're going to use. Many of the patches overlap with the work I did for GBM support (v2 here: https://patchwork.freedesktop.org/series/16244/). While yours went to the list first, I had been working on mine for quite a while. I really would prefer to keep my patches only because the i965 hardware has proven really finicky over this stuff, and I want to avoid breakage, but I'll be as accommodating as possible. Thanks. Ben [snip] ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99253] Radeon R7 370 / R9 270X/370X PCIe Bus Error: severity=Uncorrected (Fatal)
https://bugs.freedesktop.org/show_bug.cgi?id=99253 Jakob Sinclairchanged: What|Removed |Added Version|unspecified |git --- Comment #1 from Jakob Sinclair --- I have a similiar problem with my R9 280X on Linux 4.8.14 but can only reproduce it if I load a map in CS:GO. Everything works fine until the loading bar almost reaches the end. Then the screen turns black or I'm thrown back to tty and greeted with the message: radeon :6e:00.0: ring 0 stalled for more than 1908msec. I don't have access to my main computer right now so I can't get a trace of it at this moment and I can't recall if the problem started when upgrading the kernel or when I updated mesa. -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/13] egl/main: add support for fourth plane tokens
On 16-11-18 14:19:28, Emil Velikov wrote: On 15 November 2016 at 14:24, Varad Gautamwrote: From: Pekka Paalanen The EGL_EXT_dma_buf_import_modifiers extension adds support for a fourth plane, just like DRM KMS API does. Bump maximum dma_buf plane count to four. Signed-off-by: Pekka Paalanen Signed-off-by: Varad Gautam --- src/egl/drivers/dri2/egl_dri2.c | 2 +- src/egl/main/eglimage.c | 12 src/egl/main/eglimage.h | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index 9a41ad0..58d16e1 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -2038,7 +2038,7 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs) * "If is EGL_LINUX_DMA_BUF_EXT, and the EGL_LINUX_DRM_FOURCC_EXT * attribute indicates a single-plane format, EGL_BAD_ATTRIBUTE is * generated if any of the EGL_DMA_BUF_PLANE1_* or EGL_DMA_BUF_PLANE2_* -* attributes are specified." +* or EGL_DMA_BUF_PLANE3_* attributes are specified." */ for (i = plane_n; i < DMA_BUF_MAX_PLANES; ++i) { if (attrs->DMABufPlaneFds[i].IsPresent || diff --git a/src/egl/main/eglimage.c b/src/egl/main/eglimage.c index 411d1ca..cd170c6 100644 --- a/src/egl/main/eglimage.c +++ b/src/egl/main/eglimage.c @@ -133,6 +133,18 @@ _eglParseImageAttribList(_EGLImageAttribs *attrs, _EGLDisplay *dpy, attrs->DMABufPlanePitches[2].Value = val; attrs->DMABufPlanePitches[2].IsPresent = EGL_TRUE; break; + case EGL_DMA_BUF_PLANE3_FD_EXT: + attrs->DMABufPlaneFds[3].Value = val; + attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE; + break; + case EGL_DMA_BUF_PLANE3_OFFSET_EXT: + attrs->DMABufPlaneOffsets[3].Value = val; + attrs->DMABufPlaneOffsets[3].IsPresent = EGL_TRUE; + break; + case EGL_DMA_BUF_PLANE3_PITCH_EXT: + attrs->DMABufPlanePitches[3].Value = val; + attrs->DMABufPlanePitches[3].IsPresent = EGL_TRUE; + break; These should be within an extension guard. Otherwise we'll parse them (and try to push down) even if we don't support them. Something line the following should do it. Either squashed here or separate patch is fine. src/egl/main/egldisplay.h + EGLBoolean EGL_EXT_dma_buf_import_modifiers; src/egl/main/eglapi.c + _EGL_CHECK_EXTENSION(EGL_EXT_dma_buf_import_modifiers); src/egl/main/eglimage.h - /* EGL_EXT_image_dma_buf_import */ + /* EGL_EXT_image_dma_buf_import and EGL_EXT_dma_buf_import_modifiers */ src/egl/main/eglimage.c + case EGL_DMA_BUF_PLANE3_FD_EXT: + if (!disp->Extensions.EXT_dma_buf_import_modifiers) { + err = EGL_BAD_ATTRIBUTE; + break; + } + attrs->DMABufPlaneFds[3].Value = val; + attrs->DMABufPlaneFds[3].IsPresent = EGL_TRUE; + break; and same for OFFSET and PITCH. IMHO we want to keep the new code relatively bug free, so it's better to address those irrespective of the bugs/extra work mentioned below. Afaict none of the existing attribs honour their respective extension (bool). Some of them are kind of ok like EGL_EXT_image_dma_buf_import were we don't have the API/vfunc so even if we parse the values we cannot push them further down. Either way correct extensions' attrib parsing can be addressed, as independent work at a later point in time. Thanks Emil I never bothered to understand how the attrib_list gets populated, so I'll assume you're correct about this. So with Emil's suggestion, everything up to here is: Reviewed-by: Ben Widawsky ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, Jan 3, 2017 at 10:14 PM, Timothy Arceriwrote: > On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: >> On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri >> wrote: >> > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: >> > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >> > > wrote: >> > > > We only need to set it when linking was successful and the >> > > > program >> > > > being linked is currently active. >> > > > >> > > > The programs_in_use mask is just used as a flag for now but in >> > > > a following patch we will use it to update the CurrentProgram >> > > > array. >> > > > --- >> > > > src/mesa/main/shaderapi.c | 22 +- >> > > > 1 file changed, 21 insertions(+), 1 deletion(-) >> > > > >> > > > diff --git a/src/mesa/main/shaderapi.c >> > > > b/src/mesa/main/shaderapi.c >> > > > index 022afab..6c03dcb 100644 >> > > > --- a/src/mesa/main/shaderapi.c >> > > > +++ b/src/mesa/main/shaderapi.c >> > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context >> > > > *ctx, >> > > > struct gl_shader_program *shProg) >> > > >return; >> > > > } >> > > > >> > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > + unsigned programs_in_use = 0; >> > > > + if (ctx->_Shader) >> > > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; >> > > > stage++) { >> > > > + if (ctx->_Shader->CurrentProgram[stage] == shProg) { >> > > > +programs_in_use |= 1 << stage; >> > > > + } >> > > > + } >> > > > >> > > > _mesa_glsl_link_shader(ctx, shProg); >> > > > >> > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 >> > > > spec: >> > > > +* >> > > > +*"If LinkProgram or ProgramBinary successfully re- >> > > > links a >> > > > program >> > > > +* object that is active for any shader stage, then the >> > > > newly generated >> > > > +* executable code will be installed as part of the >> > > > current >> > > > rendering >> > > > +* state for all shader stages where the program is >> > > > active. >> > > > +* Additionally, the newly generated executable code is >> > > > made part of >> > > > +* the state of any program pipeline for all stages >> > > > where >> > > > the program >> > > > +* is attached." >> > > > +*/ >> > > > + if (shProg->data->LinkStatus && programs_in_use) { >> > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > > > + } >> > > >> > > This doesn't seem correct. If the context has unflushed vertices, >> > > calling FLUSH_VERTICES after linking will use the new linked >> > > program >> > > instead of the previous program for which the vertices were >> > > submitted. >> > >> > Your probably right but this doesn't make anything worse than it >> > already is right? >> > >> > Before this change we where always calling FLUSH_VERTICES(ctx, >> > _NEW_PROGRAM); >> >> The useless flagging is a different issue. >> >> FLUSH_VERTICES should be called before a state change, because it >> flushes draw calls that the driver hasn't even seen yet. After the >> driver processes those draw calls, _NEW_PROGRAM is set. That's how it >> works. >> >> If you instead change a state and then call FLUSH_VERTICES, the >> previous unflushed draw calls will use the new state, which is >> incorrect. > > But the state doesn't change just from linking does it? The new > programs won't be made current until glUseProgram() is called. The GL state changes if the program is current and re-linked. (not sure if all of the internal Mesa state changes too) Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/9] i965: Add support for tex upload using gpu
On Mon, Jan 2, 2017 at 12:45 AM, Pohjolainen, Topiwrote: > On Thu, Dec 29, 2016 at 11:55:37AM -0800, Anuj Phogat wrote: >> On Tue, Dec 20, 2016 at 6:45 AM, Topi Pohjolainen >> wrote: >> > Signed-off-by: Topi Pohjolainen >> > --- >> > src/mesa/drivers/dri/i965/intel_tex.h | 8 + >> > src/mesa/drivers/dri/i965/intel_tex_subimage.c | 194 >> > + >> > 2 files changed, 202 insertions(+) >> > >> > diff --git a/src/mesa/drivers/dri/i965/intel_tex.h >> > b/src/mesa/drivers/dri/i965/intel_tex.h >> > index 376f075..c7d0937 100644 >> > --- a/src/mesa/drivers/dri/i965/intel_tex.h >> > +++ b/src/mesa/drivers/dri/i965/intel_tex.h >> > @@ -65,6 +65,14 @@ intel_texsubimage_tiled_memcpy(struct gl_context *ctx, >> > bool for_glTexImage); >> > >> > bool >> > +intel_texsubimage_gpu_copy(struct brw_context *brw, GLuint dims, >> > + struct gl_texture_image *tex_image, >> > + unsigned x, unsigned y, unsigned z, >> > + unsigned w, unsigned h, unsigned d, >> > + GLenum format, GLenum type, const void *pixels, >> > + const struct gl_pixelstore_attrib *packing); >> > + >> > +bool >> > intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx, >> >struct gl_texture_image *texImage, >> >GLint xoffset, GLint yofset, >> > diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c >> > b/src/mesa/drivers/dri/i965/intel_tex_subimage.c >> > index b7e52bc..f999a93 100644 >> > --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c >> > +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c >> > @@ -24,6 +24,7 @@ >> > */ >> > >> > #include "main/bufferobj.h" >> > +#include "main/glformats.h" >> > #include "main/image.h" >> > #include "main/macros.h" >> > #include "main/mtypes.h" >> > @@ -34,8 +35,10 @@ >> > #include "main/enums.h" >> > #include "drivers/common/meta.h" >> > >> > +#include "brw_blorp.h" >> > #include "brw_context.h" >> > #include "intel_batchbuffer.h" >> > +#include "intel_buffer_objects.h" >> > #include "intel_tex.h" >> > #include "intel_mipmap_tree.h" >> > #include "intel_blit.h" >> > @@ -43,6 +46,197 @@ >> > >> > #define FILE_DEBUG_FLAG DEBUG_TEXTURE >> > >> > +static drm_intel_bo * >> > +intel_texsubimage_get_src_as_bo(struct brw_context *brw, unsigned dims, >> > +struct gl_texture_image *tex_image, >> > +unsigned w, unsigned h, unsigned d, >> > +GLenum format, GLenum type, const void >> > *pixels, >> > +const struct gl_pixelstore_attrib >> > *packing) >> > +{ >> > + /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */ >> > + const uint32_t first_pixel = _mesa_image_offset(dims, packing, w, h, >> > + format, type, 0, 0, 0); >> > + const uint32_t last_pixel = _mesa_image_offset(dims, packing, w, h, >> > + format, type, >> > + d - 1, h - 1, w); >> > + const uint32_t size = last_pixel - first_pixel; >> > + >> > + drm_intel_bo * const bo = >> > + drm_intel_bo_alloc(brw->bufmgr, "tmp_tex_subimage_src", size, 64); >> > + >> > + if (bo == NULL) { >> > + perf_debug("intel_texsubimage: temp bo creation failed: size = >> > %u\n", >> > + size); >> > + return false; >> > + } >> > + >> > + if (drm_intel_bo_subdata(bo, 0, size, pixels + first_pixel)) { >> > + perf_debug("intel_texsubimage: temp bo upload failed\n"); >> > + drm_intel_bo_unreference(bo); >> > + return NULL; >> > + } >> > + >> > + return bo; >> > +} >> > + >> > +static uint32_t >> > +intel_texsubimage_get_src_offset(unsigned dims, unsigned w, unsigned h, >> > + GLenum format, GLenum type, >> > + const void *pixels, >> > + const struct gl_pixelstore_attrib >> > *packing) >> > +{ >> > + /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */ >> > + const uint32_t first_pixel = _mesa_image_offset(dims, packing, w, h, >> > + format, type, 0, 0, 0); >> > + >> > + /* In case of buffer object source 'pixels' represents offset in >> > bytes. */ >> > + return first_pixel + (intptr_t)pixels; >> > +} >> > + >> > +/* Consider all the restrictions and determine the format of the source. >> > */ >> > +static mesa_format >> > +intel_texsubimage_check_upload(struct brw_context *brw, >> > + const struct gl_texture_image *tex_image, >> > +
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, 2017-01-03 at 22:07 +0100, Marek Olšák wrote: > On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceri >wrote: > > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: > > > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri > > > wrote: > > > > We only need to set it when linking was successful and the > > > > program > > > > being linked is currently active. > > > > > > > > The programs_in_use mask is just used as a flag for now but in > > > > a following patch we will use it to update the CurrentProgram > > > > array. > > > > --- > > > > src/mesa/main/shaderapi.c | 22 +- > > > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/mesa/main/shaderapi.c > > > > b/src/mesa/main/shaderapi.c > > > > index 022afab..6c03dcb 100644 > > > > --- a/src/mesa/main/shaderapi.c > > > > +++ b/src/mesa/main/shaderapi.c > > > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context > > > > *ctx, > > > > struct gl_shader_program *shProg) > > > > return; > > > > } > > > > > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > + unsigned programs_in_use = 0; > > > > + if (ctx->_Shader) > > > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; > > > > stage++) { > > > > + if (ctx->_Shader->CurrentProgram[stage] == shProg) { > > > > +programs_in_use |= 1 << stage; > > > > + } > > > > + } > > > > > > > > _mesa_glsl_link_shader(ctx, shProg); > > > > > > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 > > > > spec: > > > > +* > > > > +*"If LinkProgram or ProgramBinary successfully re- > > > > links a > > > > program > > > > +* object that is active for any shader stage, then the > > > > newly generated > > > > +* executable code will be installed as part of the > > > > current > > > > rendering > > > > +* state for all shader stages where the program is > > > > active. > > > > +* Additionally, the newly generated executable code is > > > > made part of > > > > +* the state of any program pipeline for all stages > > > > where > > > > the program > > > > +* is attached." > > > > +*/ > > > > + if (shProg->data->LinkStatus && programs_in_use) { > > > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > > > + } > > > > > > This doesn't seem correct. If the context has unflushed vertices, > > > calling FLUSH_VERTICES after linking will use the new linked > > > program > > > instead of the previous program for which the vertices were > > > submitted. > > > > Your probably right but this doesn't make anything worse than it > > already is right? > > > > Before this change we where always calling FLUSH_VERTICES(ctx, > > _NEW_PROGRAM); > > The useless flagging is a different issue. > > FLUSH_VERTICES should be called before a state change, because it > flushes draw calls that the driver hasn't even seen yet. After the > driver processes those draw calls, _NEW_PROGRAM is set. That's how it > works. > > If you instead change a state and then call FLUSH_VERTICES, the > previous unflushed draw calls will use the new state, which is > incorrect. But the state doesn't change just from linking does it? The new programs won't be made current until glUseProgram() is called. > > Marek > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Create a disable CCS flag
On Tue 03 Jan 2017, Ben Widawsky wrote: > On 17-01-03 08:21:06, Chad Versace wrote: > > On Sun 01 Jan 2017, Ben Widawsky wrote: > > > Cc: Topi Pohjolainen> > > Cc: Chad Versace > > > Signed-off-by: Ben Widawsky > > > --- > > > src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- > > > src/mesa/drivers/dri/i965/brw_draw.c | 3 ++- > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 21 ++--- > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++--- > > > 4 files changed, 16 insertions(+), 20 deletions(-) > > > > > > > @@ -2334,19 +2334,18 @@ intel_miptree_make_shareable(struct brw_context > > > *brw, > > > > > > if (mt->mcs_buf) { > > >intel_miptree_all_slices_resolve_color(brw, mt, 0); > > > - mt->no_ccs = true; > > > + mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS); > > >drm_intel_bo_unreference(mt->mcs_buf->bo); > > >free(mt->mcs_buf); > > >mt->mcs_buf = NULL; > > > } > > > > > > if (mt->hiz_buf) { > > > + mt->aux_disable |= INTEL_AUX_DISABLE_HIZ; > > >intel_miptree_all_slices_resolve_depth(brw, mt); > > >intel_miptree_hiz_buffer_free(mt->hiz_buf); > > >mt->hiz_buf = NULL; > > > } > > > - > > > - mt->aux_disable = INTEL_AUX_DISABLE_ALL; > > > } > > > > All look goods to me except the above hunk. As written, in some > > instances the driver will allocate (and use) the aux surface *after* > > intel_miptree_make_shareable() unshares the miptree. To fix that, the > > mt->aux_disable bits must be set unconditionally, outside the if's. > > > > Getting unconditional disable is exactly what the patches are trying to get > rid > of (specifically for the CCS case). So I really hope there is some solution > other than putting it outside of the if's. I understand that is the eventual goal. And I see you recently posted the RBC patch series. > > The problem is that the driver sometimes allocates the aux surfaces > > lazily. For evidence, grep for all instances of > > intel_miptree_alloc_hiz() and intel_miptree_alloc_non_msrt_mcs() outside > > of intel_mipmap_tree.c. If the code calls intel_miptree_make_shareable() > > on a miptree for which the aux surface has not yet been allocated yet, > > then intel_miptree_make_shareable() fails to set the disable bit. > Hmm. I wasn't able to find a single regression with this patch, and everything > seemed pretty okay to me with the code. Can you find a specific failing case, > because I cannot? I'm not surprised that the test suites found no regressions. The bug depends on the user create the EGLImage before the miptree's first use. So... I wrote a test and a bug :). And it seems that problem is worse than I thought. The HiZ problem has causes crashes for a long time. And your patch imporoves the situation for color buffers, changing a crash to a fail. [Piglit] egl_khr_gl_image: Add test that clears a shared image https://lists.freedesktop.org/archives/piglit/2017-January/021626.html Bug 99265 - i965: Piglit egl_khr_gl_renderbuffer_image-clear-shared-image fails https://bugs.freedesktop.org/show_bug.cgi?id=99265 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/hud: add a path separator between dump directory and filename
Pushed, thanks. Marek On Sun, Jan 1, 2017 at 10:31 PM, Edmondo Tommasinawrote: > It's more user friendly and it avoids to write files in unexpected > places. > --- > src/gallium/auxiliary/hud/hud_context.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/auxiliary/hud/hud_context.c > b/src/gallium/auxiliary/hud/hud_context.c > index 779c116..cccdb26 100644 > --- a/src/gallium/auxiliary/hud/hud_context.c > +++ b/src/gallium/auxiliary/hud/hud_context.c > @@ -872,9 +872,10 @@ hud_graph_set_dump_file(struct hud_graph *gr) > char *dump_file; > > if (hud_dump_dir && access(hud_dump_dir, W_OK) == 0) { > - dump_file = malloc(strlen(hud_dump_dir) + sizeof(gr->name)); > + dump_file = malloc(strlen(hud_dump_dir) + sizeof("/") + > sizeof(gr->name)); >if (dump_file) { > strcpy(dump_file, hud_dump_dir); > + strcat(dump_file, "/"); > strcat(dump_file, gr->name); > gr->fd = fopen(dump_file, "w+"); > free(dump_file); > -- > 2.10.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, Jan 3, 2017 at 9:57 PM, Timothy Arceriwrote: > On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: >> On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >> wrote: >> > We only need to set it when linking was successful and the program >> > being linked is currently active. >> > >> > The programs_in_use mask is just used as a flag for now but in >> > a following patch we will use it to update the CurrentProgram >> > array. >> > --- >> > src/mesa/main/shaderapi.c | 22 +- >> > 1 file changed, 21 insertions(+), 1 deletion(-) >> > >> > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c >> > index 022afab..6c03dcb 100644 >> > --- a/src/mesa/main/shaderapi.c >> > +++ b/src/mesa/main/shaderapi.c >> > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context *ctx, >> > struct gl_shader_program *shProg) >> >return; >> > } >> > >> > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > + unsigned programs_in_use = 0; >> > + if (ctx->_Shader) >> > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; >> > stage++) { >> > + if (ctx->_Shader->CurrentProgram[stage] == shProg) { >> > +programs_in_use |= 1 << stage; >> > + } >> > + } >> > >> > _mesa_glsl_link_shader(ctx, shProg); >> > >> > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 spec: >> > +* >> > +*"If LinkProgram or ProgramBinary successfully re-links a >> > program >> > +* object that is active for any shader stage, then the >> > newly generated >> > +* executable code will be installed as part of the current >> > rendering >> > +* state for all shader stages where the program is active. >> > +* Additionally, the newly generated executable code is >> > made part of >> > +* the state of any program pipeline for all stages where >> > the program >> > +* is attached." >> > +*/ >> > + if (shProg->data->LinkStatus && programs_in_use) { >> > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); >> > + } >> >> This doesn't seem correct. If the context has unflushed vertices, >> calling FLUSH_VERTICES after linking will use the new linked program >> instead of the previous program for which the vertices were >> submitted. > > Your probably right but this doesn't make anything worse than it > already is right? > > Before this change we where always calling FLUSH_VERTICES(ctx, > _NEW_PROGRAM); The useless flagging is a different issue. FLUSH_VERTICES should be called before a state change, because it flushes draw calls that the driver hasn't even seen yet. After the driver processes those draw calls, _NEW_PROGRAM is set. That's how it works. If you instead change a state and then call FLUSH_VERTICES, the previous unflushed draw calls will use the new state, which is incorrect. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/1] r600/sb: Fix loop optimization related hangs on eg
Pushed, thanks. Marek On Sun, Nov 20, 2016 at 2:42 PM, Heiko Przybylwrote: > Make sure unused ops and their references are removed, prior to entering > the GCM (global code motion) pass, to stop GCM from breaking the loop > logic and thus hanging the GPU. > > Turns out, that sb has problems with loops and node optimizations > regarding associative folding: > > - the global code motion (gcm) pass moves ops up a loop level/basic block > until they've fulfilled their total usage count > - if there are ops folded into others, the usage count won't be > fulfilled and thus the op moved way up to the top > - within GCM the op would be visited and their deps would be moved > alongside it, to fulfill the src constaints > - in a loop, an unused op is moved out of the loop and GCM would move > the src value ops up as well > - now here arises the problem: if the loop counter is one of the src > values it would get moved up as well, the loop break condition would > never get hit and the shader turn into an endless loop, resulting in the > GPU hanging and being reset > > A reduced (albeit nonsense) piglit example would be: > > [require] > GLSL >= 1.20 > > [fragment shader] > > uniform int SIZE; > uniform vec4 lights[512]; > > void main() > { > float x = 0; > for(int i = 0; i < SIZE; i++) > x += lights[2*i+1].x; > } > > [test] > uniform int SIZE 1 > draw rect -1 -1 2 2 > > Which gets optimized to: > > = SHADER #12 OPT == PS/BARTS/EVERGREEN > = > = 42 dw = 1 gprs = 2 stack > = > ALU 3 @24 > 1 y: MOVR0.y, 0 > t: MULLO_UINT R0.w, [0x0002 2.8026e-45].x, R0.z > > LOOP_START_DX10 @22 > PUSH @6 > ALU 1 @30 KC0[CB0:0-15] > 2 Mx: PRED_SETGE_INT __.x, R0.z, KC0[0].x > JUMP @14 POP:1 > LOOP_BREAK @20 > POP @14 POP:1 > ALU 2 @32 > 3 x: ADD_INTR0.x, R0.w, [0x0002 2.8026e-45].x > > TEX 1 @36 >VFETCH R0.x___, R0.x, RID:0 MFC:16 UCF:0 > FMT[..] > ALU 1 @40 > 4 y: ADDR0.y, R0.y, R0.x > LOOP_END @4 > EXPORT_DONEPIXEL 0 R0. EOP > = SHADER_END > === > > Notice R0.z being the loop counter/break condition relevant register > and being never incremented at all. Also some of the loop content > has been moved out of it, to fulfill the requirements for the one unused > op. > > With a debug build of mesa this would produce an error like > error at : PRED_SETGE_INT __, __, EM.2,R1.x.2||FP@R0.z, C0.x > : operand value R1.x.2||FP@R0.z was not previously written to its gpr > and the compilation would fail due to this. On a release build it gets > passed to the GPU. > > When using this patch, the loop remains intact: > > = SHADER #12 OPT == PS/BARTS/EVERGREEN > = > = 48 dw = 1 gprs = 2 stack > = > ALU 2 @24 > 1 y: MOVR0.y, 0 > z: MOVR0.z, 0 > LOOP_START_DX10 @22 > PUSH @6 > ALU 1 @28 KC0[CB0:0-15] > 2 Mx: PRED_SETGE_INT __.x, R0.z, KC0[0].x > JUMP @14 POP:1 > LOOP_BREAK @20 > POP @14 POP:1 > ALU 4 @30 > 3 t: MULLO_UINT T0.x, [0x0002 2.8026e-45].x, R0.z > > 4 x: ADD_INTR0.x, T0.x, [0x0002 2.8026e-45].x > > TEX 1 @40 >VFETCH R0.x___, R0.x, RID:0 MFC:16 UCF:0 > FMT[..] > ALU 2 @44 > 5 y: ADDR0.y, R0.y, R0.x > z: ADD_INTR0.z, R0.z, 1 > LOOP_END @4 > EXPORT_DONEPIXEL 0 R0. EOP > = SHADER_END > === > > Piglit: ./piglit summary console -d results/*_gpu_noglx > name: unpatched_gpu_noglx patched_gpu_noglx > --- - > pass: 18016 18021 > fail: 748 743 > crash: 7 7 > skip:1124 1124 > timeout:0 0 > warn: 1313 > incomplete: 0 0 > dmesg-warn: 0 0 > dmesg-fail: 0 0 > changes:0 5 > fixes: 0 5 > regressions:0 0 > total: 19908 19908 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94900 > Tested-by: Heiko Przybyl > Tested-on: Barts PRO HD6850 > Signed-off-by: Heiko Przybyl > --- > src/gallium/drivers/r600/sb/sb_dce_cleanup.cpp | 25 ++- >
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, 2017-01-03 at 20:47 +0100, Marek Olšák wrote: > On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceri >wrote: > > We only need to set it when linking was successful and the program > > being linked is currently active. > > > > The programs_in_use mask is just used as a flag for now but in > > a following patch we will use it to update the CurrentProgram > > array. > > --- > > src/mesa/main/shaderapi.c | 22 +- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > > index 022afab..6c03dcb 100644 > > --- a/src/mesa/main/shaderapi.c > > +++ b/src/mesa/main/shaderapi.c > > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context *ctx, > > struct gl_shader_program *shProg) > > return; > > } > > > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > + unsigned programs_in_use = 0; > > + if (ctx->_Shader) > > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; > > stage++) { > > + if (ctx->_Shader->CurrentProgram[stage] == shProg) { > > +programs_in_use |= 1 << stage; > > + } > > + } > > > > _mesa_glsl_link_shader(ctx, shProg); > > > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 spec: > > +* > > +*"If LinkProgram or ProgramBinary successfully re-links a > > program > > +* object that is active for any shader stage, then the > > newly generated > > +* executable code will be installed as part of the current > > rendering > > +* state for all shader stages where the program is active. > > +* Additionally, the newly generated executable code is > > made part of > > +* the state of any program pipeline for all stages where > > the program > > +* is attached." > > +*/ > > + if (shProg->data->LinkStatus && programs_in_use) { > > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > + } > > This doesn't seem correct. If the context has unflushed vertices, > calling FLUSH_VERTICES after linking will use the new linked program > instead of the previous program for which the vertices were > submitted. Your probably right but this doesn't make anything worse than it already is right? Before this change we where always calling FLUSH_VERTICES(ctx, _NEW_PROGRAM); > > Marek > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeonsi: capitalize VM hex addr when dumping buffer list
Reviewed-by: Marek OlšákMarek On Tue, Jan 3, 2017 at 6:41 PM, Samuel Pitoiset wrote: > Useful when debugging with R600_DEBUG=vm,check_vm to match > addr in both outputs. > > Signed-off-by: Samuel Pitoiset > --- > src/gallium/drivers/radeonsi/si_debug.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/radeonsi/si_debug.c > b/src/gallium/drivers/radeonsi/si_debug.c > index 1090dda4d4..ef92790ad5 100644 > --- a/src/gallium/drivers/radeonsi/si_debug.c > +++ b/src/gallium/drivers/radeonsi/si_debug.c > @@ -633,7 +633,7 @@ static void si_dump_bo_list(struct si_context *sctx, > } > > /* Print the buffer. */ > - fprintf(f, " %10"PRIu64"0x%013"PRIx64" > 0x%013"PRIx64" ", > + fprintf(f, " %10"PRIu64"0x%013"PRIX64" > 0x%013"PRIX64" ", > size / page_size, va / page_size, (va + size) / > page_size); > > /* Print the usage. */ > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 30/53] mesa: don't always set _NEW_PROGRAM when linking
On Tue, Jan 3, 2017 at 3:43 AM, Timothy Arceriwrote: > We only need to set it when linking was successful and the program > being linked is currently active. > > The programs_in_use mask is just used as a flag for now but in > a following patch we will use it to update the CurrentProgram > array. > --- > src/mesa/main/shaderapi.c | 22 +- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c > index 022afab..6c03dcb 100644 > --- a/src/mesa/main/shaderapi.c > +++ b/src/mesa/main/shaderapi.c > @@ -1083,10 +1083,30 @@ _mesa_link_program(struct gl_context *ctx, struct > gl_shader_program *shProg) >return; > } > > - FLUSH_VERTICES(ctx, _NEW_PROGRAM); > + unsigned programs_in_use = 0; > + if (ctx->_Shader) > + for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) { > + if (ctx->_Shader->CurrentProgram[stage] == shProg) { > +programs_in_use |= 1 << stage; > + } > + } > > _mesa_glsl_link_shader(ctx, shProg); > > + /* From section 7.3 (Program Objects) of the OpenGL 4.5 spec: > +* > +*"If LinkProgram or ProgramBinary successfully re-links a program > +* object that is active for any shader stage, then the newly > generated > +* executable code will be installed as part of the current rendering > +* state for all shader stages where the program is active. > +* Additionally, the newly generated executable code is made part of > +* the state of any program pipeline for all stages where the program > +* is attached." > +*/ > + if (shProg->data->LinkStatus && programs_in_use) { > + FLUSH_VERTICES(ctx, _NEW_PROGRAM); > + } This doesn't seem correct. If the context has unflushed vertices, calling FLUSH_VERTICES after linking will use the new linked program instead of the previous program for which the vertices were submitted. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swr: remove unneeded llvm version check
Reviewed-by: George KyriazisGeorge > -Original Message- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > Behalf Of Tim Rowley > Sent: Tuesday, January 3, 2017 10:25 AM > To: Cherniak, Bruce ; mesa- > d...@lists.freedesktop.org > Subject: [Mesa-dev] [PATCH] swr: remove unneeded llvm version check > > Old test caused breakage with llvm-svn (4.0.0svn), and not needed as the > minimum required llvm version for swr is 3.6. > --- > src/gallium/drivers/swr/swr_shader.cpp | 4 > 1 file changed, 4 deletions(-) > > diff --git a/src/gallium/drivers/swr/swr_shader.cpp > b/src/gallium/drivers/swr/swr_shader.cpp > index 294a568..979a28b 100644 > --- a/src/gallium/drivers/swr/swr_shader.cpp > +++ b/src/gallium/drivers/swr/swr_shader.cpp > @@ -344,9 +344,7 @@ BuilderSWR::CompileVS(struct swr_context *ctx, > swr_jit_vs_key ) > debug_printf("vert shader %p\n", pFunc); > assert(pFunc && "Error: VertShader = NULL"); > > -#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR >= 5) > JM()->mIsModuleFinalized = true; > -#endif > > return pFunc; > } > @@ -706,9 +704,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, > swr_jit_fs_key ) > debug_printf("frag shader %p\n", kernel); > assert(kernel && "Error: FragShader = NULL"); > > -#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR >= 5) > JM()->mIsModuleFinalized = true; > -#endif > > return kernel; > } > -- > 2.9.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] dri3: Fix MakeCurrent without a default framebuffer
Reviewed-by: Marek OlšákMarek On Tue, Jan 3, 2017 at 12:41 AM, Fredrik Höglund wrote: > In OpenGL 3.0 and later it is legal to make a context current without > a default framebuffer. > > This has been broken since DRI3 support was introduced. > > Cc: "13.0 12.0" > --- > src/glx/dri3_glx.c | 14 ++ > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c > index 358bd5f..4472a0b 100644 > --- a/src/glx/dri3_glx.c > +++ b/src/glx/dri3_glx.c > @@ -209,18 +209,24 @@ dri3_bind_context(struct glx_context *context, struct > glx_context *old, > struct dri3_context *pcp = (struct dri3_context *) context; > struct dri3_screen *psc = (struct dri3_screen *) pcp->base.psc; > struct dri3_drawable *pdraw, *pread; > + __DRIdrawable *dri_draw = NULL, *dri_read = NULL; > > pdraw = (struct dri3_drawable *) driFetchDrawable(context, draw); > pread = (struct dri3_drawable *) driFetchDrawable(context, read); > > driReleaseDrawables(>base); > > - if (pdraw == NULL || pread == NULL) > + if (pdraw) > + dri_draw = pdraw->loader_drawable.dri_drawable; > + else if (draw != None) >return GLXBadDrawable; > > - if (!(*psc->core->bindContext) (pcp->driContext, > - pdraw->loader_drawable.dri_drawable, > - pread->loader_drawable.dri_drawable)) > + if (pread) > + dri_read = pread->loader_drawable.dri_drawable; > + else if (read != None) > + return GLXBadDrawable; > + > + if (!(*psc->core->bindContext) (pcp->driContext, dri_draw, dri_read)) >return GLXBadContext; > > return Success; > -- > 2.1.4 > > ___ > mesa-stable mailing list > mesa-sta...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-stable ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] radeonsi: add a workaround for the Witcher 2 black transitions
From: Marek Olšák--- src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c index 996a458..efe28d1 100644 --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c @@ -629,20 +629,29 @@ store_value_to_array(struct lp_build_tgsi_context *bld_base, default: continue; } value = LLVMBuildExtractElement(builder, array, lp_build_const_int32(gallivm, i), ""); LLVMBuildStore(builder, value, temp_ptr); } } } +/* If this is 1, preload FS inputs at the beginning of shaders. Otherwise, + * reload them at each use. + * + * This must be 1 for Witcher 2 to render correctly. The cause of the Witcher 2 + * issue is still unknown. I only know that M0 is correct throughout the whole + * shader. + */ +#define SI_PRELOAD_FS_INPUTS 1 + LLVMValueRef si_llvm_emit_fetch(struct lp_build_tgsi_context *bld_base, const struct tgsi_full_src_register *reg, enum tgsi_opcode_type type, unsigned swizzle) { struct si_shader_context *ctx = si_shader_context(bld_base); struct lp_build_tgsi_soa_context *bld = lp_soa_context(bld_base); LLVMBuilderRef builder = bld_base->base.gallivm->builder; LLVMValueRef result = NULL, ptr, ptr2; @@ -681,21 +690,22 @@ LLVMValueRef si_llvm_emit_fetch(struct lp_build_tgsi_context *bld_base, case TGSI_FILE_INPUT: { unsigned index = reg->Register.Index; LLVMValueRef input[4]; /* I don't think doing this for vertex shaders is beneficial. * For those, we want to make sure the VMEM loads are executed * only once. Fragment shaders don't care much, because * v_interp instructions are much cheaper than VMEM loads. */ - if (ctx->soa.bld_base.info->processor == PIPE_SHADER_FRAGMENT) + if (!SI_PRELOAD_FS_INPUTS && + ctx->soa.bld_base.info->processor == PIPE_SHADER_FRAGMENT) ctx->load_input(ctx, index, >input_decls[index], input); else memcpy(input, >inputs[index * 4], sizeof(input)); result = input[swizzle]; if (tgsi_type_is_64bit(type)) { ptr = result; ptr2 = input[swizzle + 1]; return si_llvm_emit_fetch_64bit(bld_base, type, ptr, ptr2); @@ -874,21 +884,22 @@ static void emit_declaration(struct lp_build_tgsi_context *bld_base, { unsigned idx; for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) { if (ctx->load_input && ctx->input_decls[idx].Declaration.File != TGSI_FILE_INPUT) { ctx->input_decls[idx] = *decl; ctx->input_decls[idx].Range.First = idx; ctx->input_decls[idx].Range.Last = idx; ctx->input_decls[idx].Semantic.Index += idx - decl->Range.First; - if (bld_base->info->processor != PIPE_SHADER_FRAGMENT) + if (SI_PRELOAD_FS_INPUTS || + bld_base->info->processor != PIPE_SHADER_FRAGMENT) ctx->load_input(ctx, idx, >input_decls[idx], >inputs[idx * 4]); } } } break; case TGSI_FILE_SYSTEM_VALUE: { unsigned idx; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] radeonsi: set si_shader_context::input_decls for ranged decls correctly
From: Marek OlšákThis has no effect because no code uses those members with ranged decls. --- src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c index 2f38949..996a458 100644 --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c @@ -870,23 +870,26 @@ static void emit_declaration(struct lp_build_tgsi_context *bld_base, } break; } case TGSI_FILE_INPUT: { unsigned idx; for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) { if (ctx->load_input && ctx->input_decls[idx].Declaration.File != TGSI_FILE_INPUT) { ctx->input_decls[idx] = *decl; + ctx->input_decls[idx].Range.First = idx; + ctx->input_decls[idx].Range.Last = idx; + ctx->input_decls[idx].Semantic.Index += idx - decl->Range.First; if (bld_base->info->processor != PIPE_SHADER_FRAGMENT) - ctx->load_input(ctx, idx, decl, + ctx->load_input(ctx, idx, >input_decls[idx], >inputs[idx * 4]); } } } break; case TGSI_FILE_SYSTEM_VALUE: { unsigned idx; for (idx = decl->Range.First; idx <= decl->Range.Last; idx++) { -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] radeonsi: cleanly communicate whether si_shader_dump should check R600_DEBUG
From: Marek Olšák--- src/gallium/drivers/radeonsi/si_compute.c | 2 +- src/gallium/drivers/radeonsi/si_debug.c | 2 +- src/gallium/drivers/radeonsi/si_shader.c| 20 +++- src/gallium/drivers/radeonsi/si_shader.h| 2 +- src/gallium/drivers/radeonsi/si_state_shaders.c | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_compute.c b/src/gallium/drivers/radeonsi/si_compute.c index cb14a35..fe29fb1 100644 --- a/src/gallium/drivers/radeonsi/si_compute.c +++ b/src/gallium/drivers/radeonsi/si_compute.c @@ -163,21 +163,21 @@ static void *si_create_compute_state( radeon_elf_read(code, header->num_bytes, >shader.binary); if (program->use_code_object_v2) { const amd_kernel_code_t *code_object = si_compute_get_code_object(program, 0); code_object_to_config(code_object, >shader.config); } else { si_shader_binary_read_config(>shader.binary, >shader.config, 0); } si_shader_dump(sctx->screen, >shader, >b.debug, - PIPE_SHADER_COMPUTE, stderr); + PIPE_SHADER_COMPUTE, stderr, true); if (si_shader_binary_upload(sctx->screen, >shader) < 0) { fprintf(stderr, "LLVM failed to upload shader\n"); FREE(program); return NULL; } } return program; } diff --git a/src/gallium/drivers/radeonsi/si_debug.c b/src/gallium/drivers/radeonsi/si_debug.c index 1090dda..a1cd9e5 100644 --- a/src/gallium/drivers/radeonsi/si_debug.c +++ b/src/gallium/drivers/radeonsi/si_debug.c @@ -38,21 +38,21 @@ static void si_dump_shader(struct si_screen *sscreen, { struct si_shader *current = state->current; if (!state->cso || !current) return; if (current->shader_log) fwrite(current->shader_log, current->shader_log_size, 1, f); else si_shader_dump(sscreen, state->current, NULL, - state->cso->info.processor, f); + state->cso->info.processor, f, false); } /** * Shader compiles can be overridden with arbitrary ELF objects by setting * the environment variable RADEON_REPLACE_SHADERS=num1:filename1[;num2:filename2] */ bool si_replace_shader(unsigned num, struct radeon_shader_binary *binary) { const char *p = debug_get_option_replace_shaders(); const char *semicolon; diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index 8dec55c..5dfbd66 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -6162,21 +6162,22 @@ static void si_shader_dump_disassembly(const struct radeon_shader_binary *binary binary->code[i + 3], binary->code[i + 2], binary->code[i + 1], binary->code[i]); } } } static void si_shader_dump_stats(struct si_screen *sscreen, struct si_shader *shader, struct pipe_debug_callback *debug, unsigned processor, -FILE *file) +FILE *file, +bool check_debug_option) { struct si_shader_config *conf = >config; unsigned num_inputs = shader->selector ? shader->selector->info.num_inputs : 0; unsigned code_size = si_get_shader_binary_size(shader); unsigned lds_increment = sscreen->b.chip_class >= CIK ? 512 : 256; unsigned lds_per_wave = 0; unsigned max_simd_waves = 10; /* Compute LDS usage for PS. */ switch (processor) { @@ -6213,21 +6214,21 @@ static void si_shader_dump_stats(struct si_screen *sscreen, } if (conf->num_vgprs) max_simd_waves = MIN2(max_simd_waves, 256 / conf->num_vgprs); /* LDS is 64KB per CU (4 SIMDs), which is 16KB per SIMD (usage above * 16KB makes some SIMDs unoccupied). */ if (lds_per_wave) max_simd_waves = MIN2(max_simd_waves, 16384 / lds_per_wave); - if (file != stderr || + if (!check_debug_option || r600_can_dump_shader(>b, processor)) { if (processor == PIPE_SHADER_FRAGMENT) { fprintf(file, "*** SHADER CONFIG ***\n" "SPI_PS_INPUT_ADDR = 0x%04x\n" "SPI_PS_INPUT_ENA = 0x%04x\n", conf->spi_ps_input_addr, conf->spi_ps_input_ena); } fprintf(file,
Re: [Mesa-dev] [PATCH 0/2] Spirv: Set push constant base/range
Ping? On 17/12/16 23:43, Lionel Landwerlin wrote: Hi, Up to now, load_push_constant intrinsics generated by the spirv compiler were hardcoded to have an offset of 0 and a range of 128. This series adds a function compute those offset & range. Cheers, Lionel Lionel Landwerlin (2): spirv: move block_size() definition spirv: compute push constant access offset & range src/compiler/spirv/vtn_variables.c | 181 +++- src/intel/vulkan/anv_nir_lower_push_constants.c | 1 - 2 files changed, 114 insertions(+), 68 deletions(-) -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Make CopyImage check against BaseLevel, not 0, for mipmapping.
On Mon, Jan 2, 2017 at 8:01 PM, Kenneth Graunkewrote: > The intention here is to allow "base complete" instead of "mipmap > complete" as long as we're only copying from the base level (which > is usually 0, but not always). > > Signed-off-by: Kenneth Graunke > --- > src/mesa/main/copyimage.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c > index cf25159e880..d10bf8cc61c 100644 > --- a/src/mesa/main/copyimage.c > +++ b/src/mesa/main/copyimage.c > @@ -151,7 +151,7 @@ prepare_target(struct gl_context *ctx, GLuint name, > GLenum target, > >_mesa_test_texobj_completeness(ctx, texObj); >if (!texObj->_BaseComplete || > - (level != 0 && !texObj->_MipmapComplete)) { > + (level != texObj->BaseLevel && !texObj->_MipmapComplete)) { > _mesa_error(ctx, GL_INVALID_OPERATION, > "glCopyImageSubData(%sName incomplete)", dbg_prefix); > return false; > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev Reviewed-by: Anuj Phogat ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 13/53] i965: pass gl_program directly to brw_compile_tes()
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: This is the only thing we use from gl_shader_program so pass it directly. --- src/mesa/drivers/dri/i965/brw_compiler.h | 2 +- src/mesa/drivers/dri/i965/brw_shader.cpp | 6 ++ src/mesa/drivers/dri/i965/brw_tes.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h b/src/mesa/drivers/dri/i965/brw_compiler.h index 5e70601..db8f39c 100644 --- a/src/mesa/drivers/dri/i965/brw_compiler.h +++ b/src/mesa/drivers/dri/i965/brw_compiler.h @@ -803,7 +803,7 @@ brw_compile_tes(const struct brw_compiler *compiler, void *log_data, const struct brw_tes_prog_key *key, struct brw_tes_prog_data *prog_data, const struct nir_shader *shader, -struct gl_shader_program *shader_prog, +struct gl_program *prog, int shader_time_index, unsigned *final_assembly_size, char **error_str); diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp index 1ada15d..0d2f475 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -1341,14 +1341,12 @@ brw_compile_tes(const struct brw_compiler *compiler, const struct brw_tes_prog_key *key, struct brw_tes_prog_data *prog_data, const nir_shader *src_shader, -struct gl_shader_program *shader_prog, +struct gl_program *prog, int shader_time_index, unsigned *final_assembly_size, char **error_str) { const struct gen_device_info *devinfo = compiler->devinfo; - struct gl_linked_shader *shader = - shader_prog->_LinkedShaders[MESA_SHADER_TESS_EVAL]; const bool is_scalar = compiler->scalar_stage[MESA_SHADER_TESS_EVAL]; nir_shader *nir = nir_shader_clone(mem_ctx, src_shader); @@ -1406,7 +1404,7 @@ brw_compile_tes(const struct brw_compiler *compiler, if (is_scalar) { fs_visitor v(compiler, log_data, mem_ctx, (void *) key, - _data->base.base, shader->Program, nir, 8, + _data->base.base, prog, nir, 8, shader_time_index, _vue_map); if (!v.run_tes()) { if (error_str) diff --git a/src/mesa/drivers/dri/i965/brw_tes.c b/src/mesa/drivers/dri/i965/brw_tes.c index 464dbde..f98f874 100644 --- a/src/mesa/drivers/dri/i965/brw_tes.c +++ b/src/mesa/drivers/dri/i965/brw_tes.c @@ -178,7 +178,7 @@ brw_codegen_tes_prog(struct brw_context *brw, char *error_str; const unsigned *program = brw_compile_tes(compiler, brw, mem_ctx, key, _data, nir, - shader_prog, st_index, _size, _str); + >program, st_index, _size, _str); if (program == NULL) { tep->program.sh.data->LinkStatus = false; ralloc_strcat(>program.sh.data->InfoLog, error_str); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: capitalize VM hex addr when dumping buffer list
Useful when debugging with R600_DEBUG=vm,check_vm to match addr in both outputs. Signed-off-by: Samuel Pitoiset--- src/gallium/drivers/radeonsi/si_debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/radeonsi/si_debug.c b/src/gallium/drivers/radeonsi/si_debug.c index 1090dda4d4..ef92790ad5 100644 --- a/src/gallium/drivers/radeonsi/si_debug.c +++ b/src/gallium/drivers/radeonsi/si_debug.c @@ -633,7 +633,7 @@ static void si_dump_bo_list(struct si_context *sctx, } /* Print the buffer. */ - fprintf(f, " %10"PRIu64"0x%013"PRIx64" 0x%013"PRIx64" ", + fprintf(f, " %10"PRIu64"0x%013"PRIX64" 0x%013"PRIX64" ", size / page_size, va / page_size, (va + size) / page_size); /* Print the usage. */ -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/53] i965: pass gl_program to brw_upload_ubo_surfaces()
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: There is no need to pass gl_linked_shader anymore. --- src/mesa/drivers/dri/i965/brw_context.h | 3 +-- src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 4 ++-- src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 4 ++-- src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 4 ++-- src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 4 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 23 +++ 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index ad65a22..f48f36c 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -1503,8 +1503,7 @@ brw_update_sol_surface(struct brw_context *brw, struct gl_buffer_object *buffer_obj, uint32_t *out_offset, unsigned num_vector_components, unsigned stride_dwords, unsigned offset_dwords); -void brw_upload_ubo_surfaces(struct brw_context *brw, -struct gl_linked_shader *shader, +void brw_upload_ubo_surfaces(struct brw_context *brw, struct gl_program *prog, struct brw_stage_state *stage_state, struct brw_stage_prog_data *prog_data); void brw_upload_abo_surfaces(struct brw_context *brw, diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c index e2ef222..dc06462 100644 --- a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c @@ -75,13 +75,13 @@ brw_upload_gs_ubo_surfaces(struct brw_context *brw) struct gl_shader_program *prog = ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]; - if (!prog) + if (!prog || !prog->_LinkedShaders[MESA_SHADER_GEOMETRY]) return; /* BRW_NEW_GS_PROG_DATA */ struct brw_stage_prog_data *prog_data = brw->gs.base.prog_data; - brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_GEOMETRY], + brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->Program, >gs.base, prog_data); } diff --git a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c index 16f0bd2..083058b 100644 --- a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c @@ -75,13 +75,13 @@ brw_upload_tcs_ubo_surfaces(struct brw_context *brw) struct gl_shader_program *prog = ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_CTRL]; - if (!prog) + if (!prog || !prog->_LinkedShaders[MESA_SHADER_TESS_CTRL]) return; /* BRW_NEW_TCS_PROG_DATA */ struct brw_stage_prog_data *prog_data = brw->tcs.base.prog_data; - brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_CTRL], + brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_CTRL]->Program, >tcs.base, prog_data); } diff --git a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c index f74d869..03b301e 100644 --- a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c @@ -75,13 +75,13 @@ brw_upload_tes_ubo_surfaces(struct brw_context *brw) struct gl_shader_program *prog = ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL]; - if (!prog) + if (!prog || !prog->_LinkedShaders[MESA_SHADER_TESS_EVAL]) return; /* BRW_NEW_TES_PROG_DATA */ struct brw_stage_prog_data *prog_data = brw->tes.base.prog_data; - brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_EVAL], + brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_EVAL]->Program, >tes.base, prog_data); } diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c index 6c349f4..5665735 100644 --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c @@ -143,11 +143,11 @@ brw_upload_vs_ubo_surfaces(struct brw_context *brw) struct gl_shader_program *prog = ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX]; - if (!prog) + if (!prog || !prog->_LinkedShaders[MESA_SHADER_VERTEX]) return; /* BRW_NEW_VS_PROG_DATA */ - brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_VERTEX], + brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_VERTEX]->Program, >vs.base, brw->vs.base.prog_data); } diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c index 220e597..eff19de
Re: [Mesa-dev] [PATCH 12/53] i965: stop passing gl_shader_program to brw_nir_setup_glsl_uniforms()
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: We can now just get the data needed from the gl_shader_program_data pointer in gl_program. --- src/mesa/drivers/dri/i965/brw_cs.c | 4 ++-- src/mesa/drivers/dri/i965/brw_gs.c | 2 +- src/mesa/drivers/dri/i965/brw_nir.h| 1 - src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp | 14 ++ src/mesa/drivers/dri/i965/brw_tcs.c| 3 +-- src/mesa/drivers/dri/i965/brw_tes.c| 3 +-- src/mesa/drivers/dri/i965/brw_vs.c | 2 +- src/mesa/drivers/dri/i965/brw_wm.c | 2 +- 8 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c index 8658406..6882bb5 100644 --- a/src/mesa/drivers/dri/i965/brw_cs.c +++ b/src/mesa/drivers/dri/i965/brw_cs.c @@ -103,8 +103,8 @@ brw_codegen_cs_prog(struct brw_context *brw, prog_data.base.nr_params = param_count; prog_data.base.nr_image_params = cp->program.info.num_images; - brw_nir_setup_glsl_uniforms(cp->program.nir, prog, >program, - _data.base, true); + brw_nir_setup_glsl_uniforms(cp->program.nir, >program,_data.base, + true); if (unlikely(brw->perf_debug)) { start_busy = (brw->batch.last_bo && diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c index 5896726..d6dd926 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.c +++ b/src/mesa/drivers/dri/i965/brw_gs.c @@ -124,7 +124,7 @@ brw_codegen_gs_prog(struct brw_context *brw, prog_data.base.base.nr_params = param_count; prog_data.base.base.nr_image_params = gp->program.info.num_images; - brw_nir_setup_glsl_uniforms(gp->program.nir, prog, >program, + brw_nir_setup_glsl_uniforms(gp->program.nir, >program, _data.base.base, compiler->scalar_stage[MESA_SHADER_GEOMETRY]); diff --git a/src/mesa/drivers/dri/i965/brw_nir.h b/src/mesa/drivers/dri/i965/brw_nir.h index 8cfb6c1..a7189c1 100644 --- a/src/mesa/drivers/dri/i965/brw_nir.h +++ b/src/mesa/drivers/dri/i965/brw_nir.h @@ -135,7 +135,6 @@ enum brw_reg_type brw_type_for_nir_type(nir_alu_type type); enum glsl_base_type brw_glsl_base_type_for_nir_type(nir_alu_type type); void brw_nir_setup_glsl_uniforms(nir_shader *shader, - struct gl_shader_program *shader_prog, const struct gl_program *prog, struct brw_stage_prog_data *stage_prog_data, bool is_scalar); diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp index 15a608c..57682e8 100644 --- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp @@ -68,7 +68,7 @@ brw_nir_setup_glsl_builtin_uniform(nir_variable *var, static void brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var, - struct gl_shader_program *shader_prog, + const struct gl_program *prog, struct brw_stage_prog_data *stage_prog_data, bool is_scalar) { @@ -81,9 +81,9 @@ brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var, * with our name, or the prefix of a component that starts with our name. */ unsigned uniform_index = var->data.driver_location / 4; - for (unsigned u = 0; u < shader_prog->data->NumUniformStorage; u++) { + for (unsigned u = 0; u < prog->sh.data->NumUniformStorage; u++) { struct gl_uniform_storage *storage = - _prog->data->UniformStorage[u]; + >sh.data->UniformStorage[u]; if (storage->builtin) continue; @@ -130,9 +130,7 @@ brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var, } void -brw_nir_setup_glsl_uniforms(nir_shader *shader, -struct gl_shader_program *shader_prog, -const struct gl_program *prog, +brw_nir_setup_glsl_uniforms(nir_shader *shader, const struct gl_program *prog, struct brw_stage_prog_data *stage_prog_data, bool is_scalar) { @@ -146,8 +144,8 @@ brw_nir_setup_glsl_uniforms(nir_shader *shader, brw_nir_setup_glsl_builtin_uniform(var, prog, stage_prog_data, is_scalar); } else { - brw_nir_setup_glsl_uniform(shader->stage, var, shader_prog, -stage_prog_data, is_scalar); + brw_nir_setup_glsl_uniform(shader->stage, var, prog, stage_prog_data, +is_scalar); } } } diff --git
Re: [Mesa-dev] [PATCH 10/53] i965: stop passing gl_shader_program to brw_assign_common_binding_table_offsets()
Reviewed-by: Lionel LandwerlinA tiny suggestion further down. On 03/01/17 02:43, Timothy Arceri wrote: We now get eventhing we need directly from gl_program so there is Everything? :) no need for this. --- src/mesa/drivers/dri/i965/brw_cs.c | 5 ++--- src/mesa/drivers/dri/i965/brw_gs.c | 6 ++ src/mesa/drivers/dri/i965/brw_shader.cpp | 5 - src/mesa/drivers/dri/i965/brw_shader.h | 1 - src/mesa/drivers/dri/i965/brw_tcs.c | 2 +- src/mesa/drivers/dri/i965/brw_tes.c | 2 +- src/mesa/drivers/dri/i965/brw_vs.c | 2 +- src/mesa/drivers/dri/i965/brw_wm.c | 6 ++ 8 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c index f220846..8658406 100644 --- a/src/mesa/drivers/dri/i965/brw_cs.c +++ b/src/mesa/drivers/dri/i965/brw_cs.c @@ -36,7 +36,6 @@ static void assign_cs_binding_table_offsets(const struct gen_device_info *devinfo, -const struct gl_shader_program *shader_prog, const struct gl_program *prog, struct brw_cs_prog_data *prog_data) { @@ -47,7 +46,7 @@ assign_cs_binding_table_offsets(const struct gen_device_info *devinfo, next_binding_table_offset++; brw_assign_common_binding_table_offsets(MESA_SHADER_COMPUTE, devinfo, - shader_prog, prog, _data->base, + prog, _data->base, next_binding_table_offset); } @@ -81,7 +80,7 @@ brw_codegen_cs_prog(struct brw_context *brw, prog_data.base.total_shared = cp->program.info.cs.shared_size; } - assign_cs_binding_table_offsets(devinfo, prog, >program, _data); + assign_cs_binding_table_offsets(devinfo, >program, _data); /* Allocate the references to the uniforms that will end up in the * prog_data associated with the compiled program, and which will be freed diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c index 7886737..5896726 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.c +++ b/src/mesa/drivers/dri/i965/brw_gs.c @@ -74,7 +74,6 @@ brw_gs_debug_recompile(struct brw_context *brw, struct gl_program *prog, static void assign_gs_binding_table_offsets(const struct gen_device_info *devinfo, -const struct gl_shader_program *shader_prog, const struct gl_program *prog, struct brw_gs_prog_data *prog_data) { @@ -84,8 +83,7 @@ assign_gs_binding_table_offsets(const struct gen_device_info *devinfo, uint32_t reserved = devinfo->gen == 6 ? BRW_MAX_SOL_BINDINGS : 0; brw_assign_common_binding_table_offsets(MESA_SHADER_GEOMETRY, devinfo, - shader_prog, prog, - _data->base.base, + prog, _data->base.base, reserved); } @@ -104,7 +102,7 @@ brw_codegen_gs_prog(struct brw_context *brw, memset(_data, 0, sizeof(prog_data)); - assign_gs_binding_table_offsets(devinfo, prog, >program, _data); + assign_gs_binding_table_offsets(devinfo, >program, _data); /* Allocate the references to the uniforms that will end up in the * prog_data associated with the compiled program, and which will be freed diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp index 267f5fb..1ada15d 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -1181,17 +1181,12 @@ backend_shader::calculate_cfg() uint32_t brw_assign_common_binding_table_offsets(gl_shader_stage stage, Looks like you could even drop the stage argument here (in another patch possibly). It doesn't appear to be used and you could get it from gl_program. const struct gen_device_info *devinfo, -const struct gl_shader_program *shader_prog, const struct gl_program *prog, struct brw_stage_prog_data *stage_prog_data, uint32_t next_binding_table_offset) { - const struct gl_linked_shader *shader = NULL; int num_textures = util_last_bit(prog->SamplersUsed); - if (shader_prog) - shader = shader_prog->_LinkedShaders[stage]; - stage_prog_data->binding_table.texture_start = next_binding_table_offset; next_binding_table_offset += num_textures; diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h index e8b34d5..bd03204 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.h
Re: [Mesa-dev] [PATCH 09/53] st/mesa/glsl/i965: move ShaderStorageBlocks to gl_program
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: Having it here rather than in gl_linked_shader allows us to simplify the code. Also it is error prone to depend on the gl_linked_shader for programs in current use because a failed linking attempt will free infomation about the current program. In i965 we could be trying to recompile a shader variant but may have lost some required fields. --- src/compiler/glsl/link_uniforms.cpp | 3 ++- src/compiler/glsl/linker.cpp | 9 + src/compiler/glsl/lower_ubo_reference.cpp| 2 +- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/main/mtypes.h | 3 +-- src/mesa/state_tracker/st_atom_storagebuf.c | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index dba4677..90eac5c 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -903,7 +903,8 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader, unsigned num_blocks = var->data.mode == ir_var_uniform ? shader->Program->info.num_ubos : shader->Program->info.num_ssbos; struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ? - shader->Program->sh.UniformBlocks : shader->ShaderStorageBlocks; + shader->Program->sh.UniformBlocks : + shader->Program->sh.ShaderStorageBlocks; if (var->is_interface_instance()) { const ir_array_refcount_entry *const entry = v.get_variable_entry(var); diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index f8cdffd..b8b473b 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1159,7 +1159,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, struct gl_uniform_block **sh_blks; if (validate_ssbo) { sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ssbos; - sh_blks = sh->ShaderStorageBlocks; + sh_blks = sh->Program->sh.ShaderStorageBlocks; } else { sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ubos; sh_blks = sh->Program->sh.UniformBlocks; @@ -1194,7 +1194,8 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, struct gl_linked_shader *sh = prog->_LinkedShaders[i]; struct gl_uniform_block **sh_blks = validate_ssbo ? - sh->ShaderStorageBlocks : sh->Program->sh.UniformBlocks; + sh->Program->sh.ShaderStorageBlocks : + sh->Program->sh.UniformBlocks; blks[j].stageref |= sh_blks[stage_index]->stageref; sh_blks[stage_index] = [j]; @@ -2281,11 +2282,11 @@ link_intrastage_shaders(void *mem_ctx, linked->Program->info.num_ubos = num_ubo_blocks; /* Copy ssbo blocks to linked shader list */ - linked->ShaderStorageBlocks = + linked->Program->sh.ShaderStorageBlocks = ralloc_array(linked, gl_uniform_block *, num_ssbo_blocks); ralloc_steal(linked, ssbo_blocks); for (unsigned i = 0; i < num_ssbo_blocks; i++) { - linked->ShaderStorageBlocks[i] = _blocks[i]; + linked->Program->sh.ShaderStorageBlocks[i] = _blocks[i]; } linked->Program->info.num_ssbos = num_ssbo_blocks; diff --git a/src/compiler/glsl/lower_ubo_reference.cpp b/src/compiler/glsl/lower_ubo_reference.cpp index 02a97bd..bfaddac 100644 --- a/src/compiler/glsl/lower_ubo_reference.cpp +++ b/src/compiler/glsl/lower_ubo_reference.cpp @@ -290,7 +290,7 @@ lower_ubo_reference_visitor::setup_for_load_or_store(void *mem_ctx, struct gl_uniform_block **blocks; if (this->buffer_access_type != ubo_load_access) { num_blocks = shader->Program->info.num_ssbos; - blocks = shader->ShaderStorageBlocks; + blocks = shader->Program->sh.ShaderStorageBlocks; } else { num_blocks = shader->Program->info.num_ubos; blocks = shader->Program->sh.UniformBlocks; diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c index 28cb2ca..220e597 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -1410,7 +1410,7 @@ brw_upload_ubo_surfaces(struct brw_context *brw, for (int i = 0; i < shader->Program->info.num_ssbos; i++) { struct gl_shader_storage_buffer_binding *binding = - >ShaderStorageBufferBindings[shader->ShaderStorageBlocks[i]->Binding]; + >ShaderStorageBufferBindings[shader->Program->sh.ShaderStorageBlocks[i]->Binding]; if (binding->BufferObject == ctx->Shared->NullBufferObj) { brw->vtbl.emit_null_surface_state(brw, 1, 1, 1, _surf_offsets[i]); diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index
Re: [Mesa-dev] [PATCH 08/53] st/mesa/glsl/i965: set num_ssbos directly in shader_info
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: Here we also remove the duplicate field in gl_linked_shader and always get the value from shader_info instead. --- src/compiler/glsl/glsl_to_nir.cpp| 1 - src/compiler/glsl/link_uniforms.cpp | 2 +- src/compiler/glsl/linker.cpp | 14 +++--- src/compiler/glsl/lower_ubo_reference.cpp| 2 +- src/mesa/drivers/dri/i965/brw_shader.cpp | 11 +++ src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 4 ++-- src/mesa/main/mtypes.h | 1 - src/mesa/state_tracker/st_atom_storagebuf.c | 10 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +- 9 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 2620b41..cf3511e 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -150,7 +150,6 @@ glsl_to_nir(const struct gl_shader_program *shader_prog, if (shader_prog->Label) shader->info->label = ralloc_strdup(shader, shader_prog->Label); shader->info->num_textures = util_last_bit(sh->Program->SamplersUsed); - shader->info->num_ssbos = sh->NumShaderStorageBlocks; shader->info->clip_distance_array_size = sh->Program->ClipDistanceArraySize; shader->info->cull_distance_array_size = sh->Program->CullDistanceArraySize; shader->info->has_transform_feedback_varyings = diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index f614b9a..dba4677 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -901,7 +901,7 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader, var->data.mode == ir_var_shader_storage); unsigned num_blocks = var->data.mode == ir_var_uniform ? - shader->Program->info.num_ubos : shader->NumShaderStorageBlocks; + shader->Program->info.num_ubos : shader->Program->info.num_ssbos; struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ? shader->Program->sh.UniformBlocks : shader->ShaderStorageBlocks; diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 1c06034..f8cdffd 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1137,7 +1137,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, if (prog->_LinkedShaders[i]) { if (validate_ssbo) { max_num_buffer_blocks += - prog->_LinkedShaders[i]->NumShaderStorageBlocks; + prog->_LinkedShaders[i]->Program->info.num_ssbos; } else { max_num_buffer_blocks += prog->_LinkedShaders[i]->Program->info.num_ubos; @@ -1158,7 +1158,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, unsigned sh_num_blocks; struct gl_uniform_block **sh_blks; if (validate_ssbo) { - sh_num_blocks = prog->_LinkedShaders[i]->NumShaderStorageBlocks; + sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ssbos; sh_blks = sh->ShaderStorageBlocks; } else { sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ubos; @@ -2287,7 +2287,7 @@ link_intrastage_shaders(void *mem_ctx, for (unsigned i = 0; i < num_ssbo_blocks; i++) { linked->ShaderStorageBlocks[i] = _blocks[i]; } - linked->NumShaderStorageBlocks = num_ssbo_blocks; + linked->Program->info.num_ssbos = num_ssbo_blocks; /* At this point linked should contain all of the linked IR, so * validate it to make sure nothing went wrong. @@ -3099,7 +3099,7 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) } } - total_shader_storage_blocks += sh->NumShaderStorageBlocks; + total_shader_storage_blocks += sh->Program->info.num_ssbos; total_uniform_blocks += sh->Program->info.num_ubos; const unsigned max_uniform_blocks = @@ -3112,10 +3112,10 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) const unsigned max_shader_storage_blocks = ctx->Const.Program[i].MaxShaderStorageBlocks; - if (max_shader_storage_blocks < sh->NumShaderStorageBlocks) { + if (max_shader_storage_blocks < sh->Program->info.num_ssbos) { linker_error(prog, "Too many %s shader storage blocks (%d/%d)\n", _mesa_shader_stage_to_string(i), - sh->NumShaderStorageBlocks, max_shader_storage_blocks); + sh->Program->info.num_ssbos, max_shader_storage_blocks); } } @@ -3224,7 +3224,7 @@ check_image_resources(struct gl_context *ctx, struct gl_shader_program *prog)
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Create a disable CCS flag
On 17-01-03 08:21:06, Chad Versace wrote: On Sun 01 Jan 2017, Ben Widawsky wrote: Cc: Topi PohjolainenCc: Chad Versace Signed-off-by: Ben Widawsky --- src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- src/mesa/drivers/dri/i965/brw_draw.c | 3 ++- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 21 ++--- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++--- 4 files changed, 16 insertions(+), 20 deletions(-) @@ -2334,19 +2334,18 @@ intel_miptree_make_shareable(struct brw_context *brw, if (mt->mcs_buf) { intel_miptree_all_slices_resolve_color(brw, mt, 0); - mt->no_ccs = true; + mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS); drm_intel_bo_unreference(mt->mcs_buf->bo); free(mt->mcs_buf); mt->mcs_buf = NULL; } if (mt->hiz_buf) { + mt->aux_disable |= INTEL_AUX_DISABLE_HIZ; intel_miptree_all_slices_resolve_depth(brw, mt); intel_miptree_hiz_buffer_free(mt->hiz_buf); mt->hiz_buf = NULL; } - - mt->aux_disable = INTEL_AUX_DISABLE_ALL; } All look goods to me except the above hunk. As written, in some instances the driver will allocate (and use) the aux surface *after* intel_miptree_make_shareable() unshares the miptree. To fix that, the mt->aux_disable bits must be set unconditionally, outside the if's. Getting unconditional disable is exactly what the patches are trying to get rid of (specifically for the CCS case). So I really hope there is some solution other than putting it outside of the if's. The problem is that the driver sometimes allocates the aux surfaces lazily. For evidence, grep for all instances of intel_miptree_alloc_hiz() and intel_miptree_alloc_non_msrt_mcs() outside of intel_mipmap_tree.c. If the code calls intel_miptree_make_shareable() on a miptree for which the aux surface has not yet been allocated yet, then intel_miptree_make_shareable() fails to set the disable bit. Hmm. I wasn't able to find a single regression with this patch, and everything seemed pretty okay to me with the code. Can you find a specific failing case, because I cannot? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/disasm: remove printing hstride and width in align16 DF source regions
Reviewed-by: Matt Turner___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] vec4: use DIM instruction when loading DF immediates in HSW
On Tue, Jan 3, 2017 at 7:27 AM, Samuel Iglesias Gonsálvezwrote: > Signed-off-by: Samuel Iglesias Gonsálvez > --- > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > index 065e317..98e023a 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > @@ -1208,6 +1208,15 @@ vec4_visitor::setup_imm_df(double v) > if (devinfo->gen >= 8) >return brw_imm_df(v); > > + /* gen7.5 does not support DF immediates straighforward but the DIM > +* instruction allows to set the 64-bit immediate value. > +*/ > + if (devinfo->is_haswell) { > + dst_reg dst = retype(dst_reg(VGRF, alloc.allocate(2)), > BRW_REGISTER_TYPE_DF); Does this need to be alloc.allocate(2)? Since we're just loading 64-bits worth of data, shouldn't one register be fine? If that is indeed okay, then with that change: Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/53] st/mesa/glsl/i965: set num_ubos directly in shader_info
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: This also removes the duplicate field in gl_linked_shader, and gets num_ubos from shader_info instead. --- src/compiler/glsl/glsl_to_nir.cpp| 1 - src/compiler/glsl/link_uniforms.cpp | 4 ++-- src/compiler/glsl/linker.cpp | 14 +++--- src/compiler/glsl/lower_ubo_reference.cpp| 2 +- src/mesa/drivers/dri/i965/brw_shader.cpp | 4 ++-- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 4 ++-- src/mesa/main/mtypes.h | 1 - src/mesa/state_tracker/st_atom_constbuf.c| 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 ++ 9 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 4e0a33a9..2620b41 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -150,7 +150,6 @@ glsl_to_nir(const struct gl_shader_program *shader_prog, if (shader_prog->Label) shader->info->label = ralloc_strdup(shader, shader_prog->Label); shader->info->num_textures = util_last_bit(sh->Program->SamplersUsed); - shader->info->num_ubos = sh->NumUniformBlocks; shader->info->num_ssbos = sh->NumShaderStorageBlocks; shader->info->clip_distance_array_size = sh->Program->ClipDistanceArraySize; shader->info->cull_distance_array_size = sh->Program->CullDistanceArraySize; diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index cd00837..fd8331b 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -901,7 +901,7 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader, var->data.mode == ir_var_shader_storage); unsigned num_blocks = var->data.mode == ir_var_uniform ? - shader->NumUniformBlocks : shader->NumShaderStorageBlocks; + shader->Program->info.num_ubos : shader->NumShaderStorageBlocks; struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ? shader->UniformBlocks : shader->ShaderStorageBlocks; @@ -1330,7 +1330,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog, sh->num_uniform_components = uniform_size.num_shader_uniform_components; sh->num_combined_uniform_components = sh->num_uniform_components; - for (unsigned i = 0; i < sh->NumUniformBlocks; i++) { + for (unsigned i = 0; i < sh->Program->info.num_ubos; i++) { sh->num_combined_uniform_components += sh->UniformBlocks[i]->UniformBufferSize / 4; } diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index f4f918a..e5bd235 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1140,7 +1140,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, prog->_LinkedShaders[i]->NumShaderStorageBlocks; } else { max_num_buffer_blocks += - prog->_LinkedShaders[i]->NumUniformBlocks; + prog->_LinkedShaders[i]->Program->info.num_ubos; } } } @@ -1161,7 +1161,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, sh_num_blocks = prog->_LinkedShaders[i]->NumShaderStorageBlocks; sh_blks = sh->ShaderStorageBlocks; } else { - sh_num_blocks = prog->_LinkedShaders[i]->NumUniformBlocks; + sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ubos; sh_blks = sh->UniformBlocks; } @@ -2278,7 +2278,7 @@ link_intrastage_shaders(void *mem_ctx, for (unsigned i = 0; i < num_ubo_blocks; i++) { linked->UniformBlocks[i] = _blocks[i]; } - linked->NumUniformBlocks = num_ubo_blocks; + linked->Program->info.num_ubos = num_ubo_blocks; /* Copy ssbo blocks to linked shader list */ linked->ShaderStorageBlocks = @@ -3100,14 +3100,14 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog) } total_shader_storage_blocks += sh->NumShaderStorageBlocks; - total_uniform_blocks += sh->NumUniformBlocks; + total_uniform_blocks += sh->Program->info.num_ubos; const unsigned max_uniform_blocks = ctx->Const.Program[i].MaxUniformBlocks; - if (max_uniform_blocks < sh->NumUniformBlocks) { + if (max_uniform_blocks < sh->Program->info.num_ubos) { linker_error(prog, "Too many %s uniform blocks (%d/%d)\n", - _mesa_shader_stage_to_string(i), sh->NumUniformBlocks, - max_uniform_blocks); + _mesa_shader_stage_to_string(i), + sh->Program->info.num_ubos, max_uniform_blocks); } const unsigned max_shader_storage_blocks = diff --git
Re: [Mesa-dev] [PATCH 07/53] st/mesa/glsl/i965: move per stage UniformBlocks to gl_program
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: This will help allow us to store pointers to gl_program structs in the CurrentProgram array resulting in a bunch of code simplifications. --- src/compiler/glsl/link_uniforms.cpp | 4 ++-- src/compiler/glsl/linker.cpp | 8 src/compiler/glsl/lower_ubo_reference.cpp| 2 +- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +- src/mesa/main/mtypes.h | 4 ++-- src/mesa/state_tracker/st_atom_constbuf.c| 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 18 -- 7 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index fd8331b..f614b9a 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -903,7 +903,7 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader, unsigned num_blocks = var->data.mode == ir_var_uniform ? shader->Program->info.num_ubos : shader->NumShaderStorageBlocks; struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ? - shader->UniformBlocks : shader->ShaderStorageBlocks; + shader->Program->sh.UniformBlocks : shader->ShaderStorageBlocks; if (var->is_interface_instance()) { const ir_array_refcount_entry *const entry = v.get_variable_entry(var); @@ -1332,7 +1332,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog, for (unsigned i = 0; i < sh->Program->info.num_ubos; i++) { sh->num_combined_uniform_components += -sh->UniformBlocks[i]->UniformBufferSize / 4; +sh->Program->sh.UniformBlocks[i]->UniformBufferSize / 4; } } diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index e5bd235..1c06034 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1162,7 +1162,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, sh_blks = sh->ShaderStorageBlocks; } else { sh_num_blocks = prog->_LinkedShaders[i]->Program->info.num_ubos; - sh_blks = sh->UniformBlocks; + sh_blks = sh->Program->sh.UniformBlocks; } for (unsigned int j = 0; j < sh_num_blocks; j++) { @@ -1194,7 +1194,7 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog, struct gl_linked_shader *sh = prog->_LinkedShaders[i]; struct gl_uniform_block **sh_blks = validate_ssbo ? - sh->ShaderStorageBlocks : sh->UniformBlocks; + sh->ShaderStorageBlocks : sh->Program->sh.UniformBlocks; blks[j].stageref |= sh_blks[stage_index]->stageref; sh_blks[stage_index] = [j]; @@ -2272,11 +2272,11 @@ link_intrastage_shaders(void *mem_ctx, } /* Copy ubo blocks to linked shader list */ - linked->UniformBlocks = + linked->Program->sh.UniformBlocks = ralloc_array(linked, gl_uniform_block *, num_ubo_blocks); ralloc_steal(linked, ubo_blocks); for (unsigned i = 0; i < num_ubo_blocks; i++) { - linked->UniformBlocks[i] = _blocks[i]; + linked->Program->sh.UniformBlocks[i] = _blocks[i]; } linked->Program->info.num_ubos = num_ubo_blocks; diff --git a/src/compiler/glsl/lower_ubo_reference.cpp b/src/compiler/glsl/lower_ubo_reference.cpp index 985ecea..5beda45 100644 --- a/src/compiler/glsl/lower_ubo_reference.cpp +++ b/src/compiler/glsl/lower_ubo_reference.cpp @@ -293,7 +293,7 @@ lower_ubo_reference_visitor::setup_for_load_or_store(void *mem_ctx, blocks = shader->ShaderStorageBlocks; } else { num_blocks = shader->Program->info.num_ubos; - blocks = shader->UniformBlocks; + blocks = shader->Program->sh.UniformBlocks; } this->uniform_block = NULL; for (unsigned i = 0; i < num_blocks; i++) { diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c index e3a6033..2140312 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c @@ -1385,7 +1385,7 @@ brw_upload_ubo_surfaces(struct brw_context *brw, for (int i = 0; i < shader->Program->info.num_ubos; i++) { struct gl_uniform_buffer_binding *binding = - >UniformBufferBindings[shader->UniformBlocks[i]->Binding]; + >UniformBufferBindings[shader->Program->sh.UniformBlocks[i]->Binding]; if (binding->BufferObject == ctx->Shared->NullBufferObj) { brw->vtbl.emit_null_surface_state(brw, 1, 1, 1, _surf_offsets[i]); diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 2352bb2..6ddd695 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1999,6 +1999,8 @@ struct gl_program
Re: [Mesa-dev] [PATCH v2 19/25] isl: fix VA64 support for double and dvecN vertex attributes
On Dec 16, 2016 8:55 AM, "Juan A. Suarez Romero"wrote: From: Samuel Iglesias Gonsálvez We use *64*_PASSTHRU formats to upload vertex attributes of 64 bits to avoid conversions. From the BDW PRM, Volume 2d, page 586 (VERTEX_ELEMENT_STATE): "When SourceElementFormat is set to one of the *64*_PASSTHRU formats, 64-bit components are stored in the URB without any conversion. In this case, vertex elements must be written as 128 or 256 bits, with VFCOMP_STORE_0 being used to pad the output as required. E.g., if R64_PASSTHRU is used to copy a 64-bit Red component into the URB, Component 1 must be specified as VFCOMP_STORE_0 (with Components 2,3 set to VFCOMP_NOSTORE) in order to output a 128-bit vertex element, or Components 1-3 must be specified as VFCOMP_STORE_0 in order to output a 256-bit vertex element. Likewise, use of R64G64B64_PASSTHRU requires Component 3 to be specified as VFCOMP_STORE_0 in order to output a 256-bit vertex element." v2 (Jason): - Don't delete unused formats. Signed-off-by: Samuel Iglesias Gonsálvez --- src/intel/isl/isl_format.c | 4 ++-- src/intel/isl/isl_format_layout.csv | 1 - src/intel/vulkan/anv_formats.c | 8 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c index 98806f4..92b630a 100644 --- a/src/intel/isl/isl_format.c +++ b/src/intel/isl/isl_format.c @@ -97,7 +97,7 @@ static const struct surface_format_info format_info[] = { SF( x, x, x, x, x, x, Y, x, x,x, R32G32B32A32_SSCALED) SF( x, x, x, x, x, x, Y, x, x,x, R32G32B32A32_USCALED) SF( x, x, x, x, x, x, 75, x, x,x, R32G32B32A32_SFIXED) - SF( x, x, x, x, x, x, x, x, x,x, R64G64_PASSTHRU) + SF( x, x, x, x, x, x, 80, x, x,x, R64G64_PASSTHRU) SF( Y, 50, x, x, x, x, Y, Y, x,x, R32G32B32_FLOAT) SF( Y, x, x, x, x, x, Y, Y, x,x, R32G32B32_SINT) SF( Y, x, x, x, x, x, Y, Y, x,x, R32G32B32_UINT) @@ -131,7 +131,7 @@ static const struct surface_format_info format_info[] = { SF( x, x, x, x, x, x, Y, x, x,x, R32G32_SSCALED) SF( x, x, x, x, x, x, Y, x, x,x, R32G32_USCALED) SF( x, x, x, x, x, x, 75, x, x,x, R32G32_SFIXED) - SF( x, x, x, x, x, x, x, x, x,x, R64_PASSTHRU) + SF( x, x, x, x, x, x, 80, x, x,x, R64_PASSTHRU) SF( Y, Y, x, Y, Y, Y, Y, x, 60, 90, B8G8R8A8_UNORM) SF( Y, Y, x, x, Y, Y, x, x, x,x, B8G8R8A8_UNORM_SRGB) /* smpl filt shad CK RT AB VB SO color ccs_e */ diff --git a/src/intel/isl/isl_format_layout.csv b/src/intel/isl/isl_format_ layout.csv index f0f31c7..a177eef 100644 --- a/src/intel/isl/isl_format_layout.csv +++ b/src/intel/isl/isl_format_layout.csv @@ -96,7 +96,6 @@ X32_TYPELESS_G8X24_UINT , 64, 1, 1, 1, x32, ui8, x24, , , L32A32_FLOAT, 64, 1, 1, 1, , , , sf32, sf32, ,, linear, R32G32_UNORM, 64, 1, 1, 1, un32, un32, , , , ,, linear, R32G32_SNORM, 64, 1, 1, 1, sn32, sn32, , , , ,, linear, -R64_FLOAT , 64, 1, 1, 1, sf64, , , , , ,, linear, Why are we deleting this? R16G16B16X16_UNORM , 64, 1, 1, 1, un16, un16, un16, x16, , ,, linear, R16G16B16X16_FLOAT , 64, 1, 1, 1, sf16, sf16, sf16, x16, , ,, linear, A32X32_FLOAT, 64, 1, 1, 1, , , , sf32, x32, ,, alpha, diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c index 9ef998c..39810d4 100644 --- a/src/intel/vulkan/anv_formats.c +++ b/src/intel/vulkan/anv_formats.c @@ -156,16 +156,16 @@ static const struct anv_format anv_formats[] = { fmt(VK_FORMAT_R32G32B32A32_SFLOAT, ISL_FORMAT_R32G32B32A32_FLOAT), fmt(VK_FORMAT_R64_UINT,ISL_FORMAT_R64_PASSTHRU), fmt(VK_FORMAT_R64_SINT,ISL_FORMAT_R64_PASSTHRU), - fmt(VK_FORMAT_R64_SFLOAT, ISL_FORMAT_R64_FLOAT), + fmt(VK_FORMAT_R64_SFLOAT, ISL_FORMAT_R64_PASSTHRU), fmt(VK_FORMAT_R64G64_UINT, ISL_FORMAT_R64G64_PASSTHRU), fmt(VK_FORMAT_R64G64_SINT, ISL_FORMAT_R64G64_PASSTHRU), - fmt(VK_FORMAT_R64G64_SFLOAT, ISL_FORMAT_R64G64_FLOAT), + fmt(VK_FORMAT_R64G64_SFLOAT, ISL_FORMAT_R64G64_PASSTHRU), fmt(VK_FORMAT_R64G64B64_UINT, ISL_FORMAT_R64G64B64_PASSTHRU), fmt(VK_FORMAT_R64G64B64_SINT, ISL_FORMAT_R64G64B64_PASSTHRU), - fmt(VK_FORMAT_R64G64B64_SFLOAT,ISL_FORMAT_R64G64B64_FLOAT), + fmt(VK_FORMAT_R64G64B64_SFLOAT,ISL_FORMAT_R64G64B64_PASSTHRU), fmt(VK_FORMAT_R64G64B64A64_UINT, ISL_FORMAT_R64G64B64A64_
Re: [Mesa-dev] [PATCH v2 23/25] spirv: enable SpvCapabilityFloat64 only to supported platforms
Dave recently added a spirv-specific structure for this sort of feature enabling. I think it would be better to use that rather than nir_options. On Dec 16, 2016 8:56 AM, "Juan A. Suarez Romero"wrote: > From: Samuel Iglesias Gonsálvez > > Signed-off-by: Samuel Iglesias Gonsálvez > --- > src/compiler/spirv/spirv_to_nir.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/compiler/spirv/spirv_to_nir.c > b/src/compiler/spirv/spirv_to_nir.c > index 11f6248..f4bf3b4 100644 > --- a/src/compiler/spirv/spirv_to_nir.c > +++ b/src/compiler/spirv/spirv_to_nir.c > @@ -2525,6 +2525,13 @@ vtn_handle_preamble_instruction(struct vtn_builder > *b, SpvOp opcode, >case SpvCapabilityInputAttachment: > break; > > + case SpvCapabilityFloat64: > + if (!b->nir_options->native_float64) { > +vtn_warn("Unsupported SPIR-V capability: %s", > + spirv_capability_to_string(cap)); > + } > + break; > + >case SpvCapabilityGeometryStreams: >case SpvCapabilityTessellation: >case SpvCapabilityTessellationPointSize: > @@ -2532,7 +2539,6 @@ vtn_handle_preamble_instruction(struct vtn_builder > *b, SpvOp opcode, >case SpvCapabilityVector16: >case SpvCapabilityFloat16Buffer: >case SpvCapabilityFloat16: > - case SpvCapabilityFloat64: >case SpvCapabilityInt64: >case SpvCapabilityInt64Atomics: >case SpvCapabilityAtomicStorage: > -- > 2.9.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Patch for freedreno features
On Jan 3, 2017 9:54 AM, "Romain Failliot"wrote: 2017-01-03 10:47 GMT-05:00 Rob Clark : > Hmm, well I guess in future, git-format-patch and attach that to bz.. > but no worries this time, I went ahead and pushed it w/ --author as > Jan suggested Thanks! I'm sorry, I misunderstood, I thought you preferred the patch to be in the mailing list (but I did use git format-patch, so... 1 out of 2 :D ) Next time I'll do as you said! (and actually I do prefer bz over emails) If you plan on making substantial contributions in the future, getting git-send-email working is a good idea. The Mesa project as a whole uses the mailing list for patch submission and review. Sometimes someone will pull a patch from a bug but it's definitely not the recommended method. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 17/25] spirv: add support for doubles to OpSpecConstant
On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romerowrote: > From: Samuel Iglesias Gonsálvez > > Signed-off-by: Samuel Iglesias Gonsálvez > --- > src/amd/vulkan/radv_pipeline.c| 5 +++- > src/compiler/spirv/nir_spirv.h| 5 +++- > src/compiler/spirv/spirv_to_nir.c | 51 ++ > + > src/intel/vulkan/anv_pipeline.c | 5 +++- > 4 files changed, 58 insertions(+), 8 deletions(-) > > diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_ > pipeline.c > index 23ed2d2..6d8299e 100644 > --- a/src/amd/vulkan/radv_pipeline.c > +++ b/src/amd/vulkan/radv_pipeline.c > @@ -188,7 +188,10 @@ radv_shader_compile_to_nir(struct radv_device > *device, > assert(data + entry.size <= > spec_info->pData + spec_info->dataSize); > > spec_entries[i].id = > spec_info->pMapEntries[i].constantID; > - spec_entries[i].data = *(const uint32_t > *)data; > +if (spec_info->dataSize == 8) > + spec_entries[i].data64 = *(const > uint64_t *)data; > +else > + spec_entries[i].data32 = *(const > uint32_t *)data; > radv is silly and uses tabs. :( You used spaces... > } > } > > diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_ > spirv.h > index 500f2cb..33a2781 100644 > --- a/src/compiler/spirv/nir_spirv.h > +++ b/src/compiler/spirv/nir_spirv.h > @@ -38,7 +38,10 @@ extern "C" { > > struct nir_spirv_specialization { > uint32_t id; > - uint32_t data; > + union { > + uint32_t data32; > + uint64_t data64; > + }; > }; > > nir_function *spirv_to_nir(const uint32_t *words, size_t word_count, > diff --git a/src/compiler/spirv/spirv_to_nir.c > b/src/compiler/spirv/spirv_to_nir.c > index 2a60b53..380fbae 100644 > --- a/src/compiler/spirv/spirv_to_nir.c > +++ b/src/compiler/spirv/spirv_to_nir.c > @@ -31,6 +31,14 @@ > #include "nir/nir_constant_expressions.h" > #include "spirv_info.h" > > +struct spec_constant_value { > + bool is_double; > + union { > + uint32_t data32; > + uint64_t data64; > + }; > +}; > + > void > _vtn_warn(const char *file, int line, const char *msg, ...) > { > @@ -942,11 +950,14 @@ spec_constant_decoration_cb(struct vtn_builder *b, > struct vtn_value *v, > if (dec->decoration != SpvDecorationSpecId) >return; > > - uint32_t *const_value = data; > + struct spec_constant_value *const_value = data; > > for (unsigned i = 0; i < b->num_specializations; i++) { >if (b->specializations[i].id == dec->literals[0]) { > - *const_value = b->specializations[i].data; > + if (const_value->is_double) > +const_value->data64 = b->specializations[i].data64; > + else > +const_value->data32 = b->specializations[i].data32; > return; >} > } > @@ -956,8 +967,22 @@ static uint32_t > get_specialization(struct vtn_builder *b, struct vtn_value *val, > uint32_t const_value) > { > - vtn_foreach_decoration(b, val, spec_constant_decoration_cb, > _value); > - return const_value; > + struct spec_constant_value data; > + data.is_double = false; > + data.data32 = const_value; > + vtn_foreach_decoration(b, val, spec_constant_decoration_cb, ); > + return data.data32; > +} > + > +static uint64_t > +get_specialization64(struct vtn_builder *b, struct vtn_value *val, > + uint64_t const_value) > +{ > + struct spec_constant_value data; > + data.is_double = true; > + data.data64 = const_value; > + vtn_foreach_decoration(b, val, spec_constant_decoration_cb, ); > + return data.data64; > } > > static void > @@ -1016,10 +1041,26 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp > opcode, >} >break; > } > - case SpvOpSpecConstant: > + case SpvOpSpecConstant: { >assert(glsl_type_is_scalar(val->const_type)); >val->constant->values[0].u32[0] = get_specialization(b, val, w[3]); > + int bit_size = glsl_get_bit_size(val->const_type); > + if (bit_size == 64) { > + union { > +uint64_t u64; > +struct { > + uint32_t u1; > + uint32_t u2; > +}; > + } di; > + di.u1 = w[3]; > + di.u2 = w[4]; > You could also just do *(const uint64_t *)[3]. Might be easier. > + val->constant->values[0].u64[0] = get_specialization64(b, val, > di.u64); > + } else { > + val->constant->values[0].u32[0] = get_specialization(b, val, > w[3]); > + } >break; > + } > case SpvOpSpecConstantComposite: > case SpvOpConstantComposite: { >unsigned elem_count = count - 3; > diff --git a/src/intel/vulkan/anv_pipeline.c
Re: [Mesa-dev] [PATCH 05/53] st/mesa/glsl/i965: move ImageUnits and ImageAccess fields to gl_program
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: Having it here rather than in gl_linked_shader allows us to simplify the code. Also it is error prone to depend on the gl_linked_shader for programs in current use because a failed linking attempt will free infomation about the current program. In i965 we could be trying to recompile a shader variant but may have lost some required fields. We drop the memset on ImageUnits because gl_program is already created using rzalloc(). --- src/compiler/glsl/link_uniform_initializers.cpp | 5 +-- src/compiler/glsl/link_uniforms.cpp | 3 +- src/mesa/drivers/dri/i965/brw_context.c | 3 +- src/mesa/drivers/dri/i965/brw_context.h | 1 - src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 9 ++--- src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 9 ++--- src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 9 ++--- src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 9 ++--- src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 24 +- src/mesa/main/mtypes.h| 40 +++ src/mesa/main/uniform_query.cpp | 2 +- src/mesa/state_tracker/st_atom_image.c| 3 +- 12 files changed, 50 insertions(+), 67 deletions(-) diff --git a/src/compiler/glsl/link_uniform_initializers.cpp b/src/compiler/glsl/link_uniform_initializers.cpp index 93340ba..6f05a3a 100644 --- a/src/compiler/glsl/link_uniform_initializers.cpp +++ b/src/compiler/glsl/link_uniform_initializers.cpp @@ -141,9 +141,10 @@ set_opaque_binding(void *mem_ctx, gl_shader_program *prog, storage->opaque[sh].active) { for (unsigned i = 0; i < elements; i++) { const unsigned index = storage->opaque[sh].index + i; - if (index >= ARRAY_SIZE(shader->ImageUnits)) + if (index >= ARRAY_SIZE(shader->Program->sh.ImageUnits)) break; - shader->ImageUnits[index] = storage->storage[i].i; + shader->Program->sh.ImageUnits[index] = + storage->storage[i].i; } } } diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp index 256afa3..cd00837 100644 --- a/src/compiler/glsl/link_uniforms.cpp +++ b/src/compiler/glsl/link_uniforms.cpp @@ -613,7 +613,7 @@ private: this->next_image += MAX2(1, uniform->array_elements); for (unsigned i = first; i < MIN2(next_image, MAX_IMAGE_UNIFORMS); i++) -prog->_LinkedShaders[shader_type]->ImageAccess[i] = access; +prog->_LinkedShaders[shader_type]->Program->sh.ImageAccess[i] = access; } } @@ -1308,7 +1308,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog, * types cannot have initializers." */ memset(sh->SamplerUnits, 0, sizeof(sh->SamplerUnits)); - memset(sh->ImageUnits, 0, sizeof(sh->ImageUnits)); link_update_uniform_buffer_variables(sh, i); diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 4ca77c7..e53aefd 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -280,7 +280,8 @@ intel_update_state(struct gl_context * ctx, GLuint new_state) if (unlikely(shader && shader->Program->info.num_images)) { for (unsigned j = 0; j < shader->Program->info.num_images; j++) { -struct gl_image_unit *u = >ImageUnits[shader->ImageUnits[j]]; +struct gl_image_unit *u = + >ImageUnits[shader->Program->sh.ImageUnits[j]]; tex_obj = intel_texture_object(u->TexObj); if (tex_obj && tex_obj->mt) { diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index a281713..ad65a22 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -1512,7 +1512,6 @@ void brw_upload_abo_surfaces(struct brw_context *brw, struct brw_stage_state *stage_state, struct brw_stage_prog_data *prog_data); void brw_upload_image_surfaces(struct brw_context *brw, - struct gl_linked_shader *shader, const struct gl_program *prog, struct brw_stage_state *stage_state, struct brw_stage_prog_data *prog_data); diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c index cf56acf..e2ef222 100644 --- a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c @@ -122,16 +122,13 @@ const struct brw_tracked_state brw_gs_abo_surfaces = { static void
[Mesa-dev] [PATCH] swr: remove unneeded llvm version check
Old test caused breakage with llvm-svn (4.0.0svn), and not needed as the minimum required llvm version for swr is 3.6. --- src/gallium/drivers/swr/swr_shader.cpp | 4 1 file changed, 4 deletions(-) diff --git a/src/gallium/drivers/swr/swr_shader.cpp b/src/gallium/drivers/swr/swr_shader.cpp index 294a568..979a28b 100644 --- a/src/gallium/drivers/swr/swr_shader.cpp +++ b/src/gallium/drivers/swr/swr_shader.cpp @@ -344,9 +344,7 @@ BuilderSWR::CompileVS(struct swr_context *ctx, swr_jit_vs_key ) debug_printf("vert shader %p\n", pFunc); assert(pFunc && "Error: VertShader = NULL"); -#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR >= 5) JM()->mIsModuleFinalized = true; -#endif return pFunc; } @@ -706,9 +704,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, swr_jit_fs_key ) debug_printf("frag shader %p\n", kernel); assert(kernel && "Error: FragShader = NULL"); -#if (LLVM_VERSION_MAJOR == 3) && (LLVM_VERSION_MINOR >= 5) JM()->mIsModuleFinalized = true; -#endif return kernel; } -- 2.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] i965: Replace bool aux disable with enum
On Sun 01 Jan 2017, Ben Widawsky wrote: > As CCS buffers are passed to KMS, it becomes useful to be able to > determine exactly what type of aux buffers are disabled. This was > previously not entirely needed (though the code was a little more > confusing), however it becomes very desirable after a recent patch from > Chad: > > commit 1c8be049bea786c2c054a770025976beba5b8636 > Author: Chad Versace> Date: Fri Dec 9 16:18:11 2016 -0800 > > i965/mt: Disable aux surfaces after making miptree shareable > > The next patch will handle CCS and get rid of no_ccs. > > Signed-off-by: Ben Widawsky > --- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 24 > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +- > 2 files changed, 21 insertions(+), 13 deletions(-) Patch 1 is Reviewed-by: Chad Versace ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Create a disable CCS flag
On Sun 01 Jan 2017, Ben Widawsky wrote: > Cc: Topi Pohjolainen> Cc: Chad Versace > Signed-off-by: Ben Widawsky > --- > src/mesa/drivers/dri/i965/brw_blorp.c | 2 +- > src/mesa/drivers/dri/i965/brw_draw.c | 3 ++- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 21 ++--- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +++--- > 4 files changed, 16 insertions(+), 20 deletions(-) > @@ -2334,19 +2334,18 @@ intel_miptree_make_shareable(struct brw_context *brw, > > if (mt->mcs_buf) { >intel_miptree_all_slices_resolve_color(brw, mt, 0); > - mt->no_ccs = true; > + mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS); >drm_intel_bo_unreference(mt->mcs_buf->bo); >free(mt->mcs_buf); >mt->mcs_buf = NULL; > } > > if (mt->hiz_buf) { > + mt->aux_disable |= INTEL_AUX_DISABLE_HIZ; >intel_miptree_all_slices_resolve_depth(brw, mt); >intel_miptree_hiz_buffer_free(mt->hiz_buf); >mt->hiz_buf = NULL; > } > - > - mt->aux_disable = INTEL_AUX_DISABLE_ALL; > } All look goods to me except the above hunk. As written, in some instances the driver will allocate (and use) the aux surface *after* intel_miptree_make_shareable() unshares the miptree. To fix that, the mt->aux_disable bits must be set unconditionally, outside the if's. The problem is that the driver sometimes allocates the aux surfaces lazily. For evidence, grep for all instances of intel_miptree_alloc_hiz() and intel_miptree_alloc_non_msrt_mcs() outside of intel_mipmap_tree.c. If the code calls intel_miptree_make_shareable() on a miptree for which the aux surface has not yet been allocated yet, then intel_miptree_make_shareable() fails to set the disable bit. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Patch for freedreno features
2017-01-03 10:47 GMT-05:00 Rob Clark: > Hmm, well I guess in future, git-format-patch and attach that to bz.. > but no worries this time, I went ahead and pushed it w/ --author as > Jan suggested Thanks! I'm sorry, I misunderstood, I thought you preferred the patch to be in the mailing list (but I did use git format-patch, so... 1 out of 2 :D ) Next time I'll do as you said! (and actually I do prefer bz over emails) -- Romain "Creak" Failliot ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] Enable OpenGL 4.0 on Haswell
On Tuesday, January 3, 2017 2:02:19 PM PST Iago Toral wrote: > On Tue, 2017-01-03 at 11:14 +, Chris Wilson wrote: > > On Tue, Jan 03, 2017 at 11:42:50AM +0100, Iago Toral Quiroga wrote: > > > > > > Enabling GL 4.0 in gen7 requires a bit of work because some > > > hardware and kernel > > > combinations may not support all the features. Specifically, we > > > need to know > > > if the kernel can do pipelined register writes. Unfortunately, this > > > requires > > > that we emit batches at screen creation time when we don't have a > > > brw_context > > > available (all our current batch emission infrastructure relies on > > > this). > > > See [1] and [2] for more details. > > Emitting a batch is trivial: > > > > https://lists.freedesktop.org/archives/mesa-dev/2015-August/091077.ht > > ml > > Oh, thanks, I did not know you had done this before. It seems that > patch got an Rb from Martin when it was sent in 2015 but it never > landed, right? was there any other issue holding it back? That was me - I NAK'd the series as a whole. > > But we do want to share the bufmgr between screens, and you do want > > to > > deprecate the current intel_batchbuffer.c due to the very large > > overhead > > it imposes. > > It seems that Curro and Ken were more interested in trying to reuse a > bit more of the intel_batchbuffer.c for this rather than the other way > around, but then again that might have only been because my original > implementation was very focused on porting code from that file... your > patch gets this done without using a batchbuffer object at all and I > think it is quite clean. If Ken and Curro also like it and there are no > other concerns I'll be happy to adapt/rebase your patch instead. > > Ken was also saying that he was planning some rework of the batch > emission code, so I don't know what strategy fits his plans better. > > Iago I'd forgotten that Chris did this. His code is much smaller, and looks like a nice solution to this problem. Iago, could you update the patch Chris linked instead? A couple thoughts on Chris's patch: I'd prefer to drop brw->has_pipelined_so in favor of checking the screen field directly (we've got enough redundant things in brw_context already). Also, I'm not sure what to make of the hw_has_pipelined_register bitfield. If we're only using it for one thing, we could just use a "can_write_sol_offsets" boolean. But it also seems nice. Would it make sense to generalize it a bit and have: #define KERNEL_ALLOWS_SOL_OFFSET_WRITES(1 << 0) #define KERNEL_ALLOWS_PREDICATE_WRITES (1 << 1) #define KERNEL_ALLOWS_CS_GPR_WRITES(1 << 2) #define KERNEL_ALLOWS_HSW_SCRATCH1_AND_ROW_CHICKEN3_WRITES (1 << 3) #define KERNEL_ALLOWS_LOAD_REGISTER_REG(1 << 4) and consolidate the various brw->screen->cmd_parser_version checking into one place, so intel_screen.c either tries it or queries it, and then sets a bit, and the rest of the code just checks that bitfield? It might be cleaner that way...thoughts? By the way, I just pushed a patch from Robert which eliminated the OACONTROL register checking. So you don't need to worry about that. --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Patch for freedreno features
On Mon, Jan 2, 2017 at 1:18 PM, Romain Failliotwrote: > 2017-01-01 16:15 GMT-05:00 Rob Clark : >> well, I was trying to not loose authorship/etc from the patch >> (although tbh I've lost track of who the original author was).. >> http://www.mesa3d.org/devinfo.html#submitting has some suggestions. > > I went there, I don't think it's a git problem. I think it's the > default setup of sendmail that in't good (maybe it's not configured, > or it only allows internal emails, idk). > And I'm not that good at changing sendmail settings. Hmm, well I guess in future, git-format-patch and attach that to bz.. but no worries this time, I went ahead and pushed it w/ --author as Jan suggested BR, -R ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 14/25] nir: add nir_type_conversion_op()
On Tue, Jan 3, 2017 at 4:31 AM, Samuel Iglesias Gonsálvez < sigles...@igalia.com> wrote: > On Mon, 2017-01-02 at 10:34 -0800, Jason Ekstrand wrote: > > I've left some inline comments but this patch has gotten me thinking > > about the larger question of how we want to do conversion operations > > in NIR. Ian already has patches for int64 but float16, int8, and > > int16 are all coming. If we keep coming up with one-letter prefixes > > (long, half, short, char?) and doing a2b for every opcode. We could, > > but that would be very confusing. It would be good to cut down on > > the combinatorics a bit. > > > > Here's my straw-man proposal: For each conversion, we leave the > > destination as variable bit width and keep the source fixed. Then we > > have u2f32, u2f64, u2f16, etc. and we have one opcode regardless of > > destination bit width. This doesn't completely break the > > combinatorial explosion but it does let us cut it by a factor of 3 or > > 4. > > > > Another thing that Connor and I discussed was to let conversion > > operations be the one exception to the "source and destination bit- > > widths must match for variable bit-width operations" and add i2i and > > f2f opcodes. That would drop the number of opcodes even further but > > I'm not sure how bad the special-casing would end up being. > > > > Thoughts? > > > > > The first option doesn't really address the problem and feels more like > a work-around, but I guess we could go with that if other people prefer > that approach. Personally, I think i would try the second option. I > can't tell right know the implications this would have for the special- > casing in the validation of variable bit-width opcodes, etc but I guess > we can just try to write that patch and see how it looks like in the > end, we can always resort to u2f32 and cia if we don't like it. > > In any case, regardless the solution we agree on, any of these changes > would require time to get them right as they touch several places in > NIR, so considering that the feature freeze for the next Mesa release > is around the corner (January 13th [0]) I think we also need to discuss > whether we want to tackle this before or after we land Vulkan Fp64. > > We are leaning towards landing Vulkan Fp64 before we do these changes > so we can have that in the next release and also because we need the > vertex input attributes changes introduced with this series for our > Haswell VA64 implementation (which we were hoping to send this week for > review too, although maybe we are trying to squeeze too many things for > the next release :-D) > > What do you think? > I 100% agree. This was mostly a discussion for the future direction. > Sam > > [0] https://lists.freedesktop.org/archives/mesa-dev/2016-November/13701 > 5.html > > > > --Jason Ekstrand > > > > On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romero> ia.com> wrote: > > > From: Samuel Iglesias Gonsálvez > > > > > > This function returns the nir_op corresponding to the conversion > > > between > > > the given nir_alu_type arguments. > > > > > > This function lacks support for integer-based types with bit_size > > > != 32 > > > and for float16 conversion ops. > > > > > > Signed-off-by: Samuel Iglesias Gonsálvez > > > --- > > > src/compiler/nir/nir.c | 83 > > > ++ > > > src/compiler/nir/nir.h | 2 ++ > > > 2 files changed, 85 insertions(+) > > > > > > diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c > > > index 2d882f7..3e00452 100644 > > > --- a/src/compiler/nir/nir.c > > > +++ b/src/compiler/nir/nir.c > > > @@ -1953,3 +1953,86 @@ > > > nir_system_value_from_intrinsic(nir_intrinsic_op intrin) > > >unreachable("intrinsic doesn't produce a system value"); > > > } > > > } > > > + > > > +nir_op > > > +nir_type_conversion_op(nir_alu_type src, nir_alu_type dst) > > > +{ > > > + nir_alu_type src_base_type = (nir_alu_type) > > > nir_alu_type_get_base_type(src); > > > + nir_alu_type dst_base_type = (nir_alu_type) > > > nir_alu_type_get_base_type(dst); > > > + unsigned src_bitsize = nir_alu_type_get_type_size(src); > > > + unsigned dst_bitsize = nir_alu_type_get_type_size(dst); > > > + > > > + if (src_base_type == dst_base_type) { > > > + if (src_bitsize == dst_bitsize) > > > + return (src_base_type == nir_type_float) ? nir_op_fmov : > > > nir_op_imov; > > > + > > > + switch (src_base_type) { > > > + case nir_type_bool: > > > + case nir_type_uint: > > > + case nir_type_int: > > > + return nir_op_imov; > > > > These three cases can't happen right now. We should just assert that > > it's float. > > > > > + case nir_type_float: > > > + /* TODO: implement support for float16 */ > > > + assert(src_bitsize == 64 || dst_bitsize == 64); > > > + return (src_bitsize == 64) ? nir_op_d2f : nir_op_f2d; > > > + default: > > > +
Re: [Mesa-dev] [PATCH 0/6] Enable OpenGL 4.0 on Haswell
On Tuesday, January 3, 2017 11:42:50 AM PST Iago Toral Quiroga wrote: > Enabling GL 4.0 in gen7 requires a bit of work because some hardware and > kernel > combinations may not support all the features. Specifically, we need to know > if the kernel can do pipelined register writes. Unfortunately, this requires > that we emit batches at screen creation time when we don't have a brw_context > available (all our current batch emission infrastructure relies on this). > See [1] and [2] for more details. > > This little series implements the minimum infrastructure necessary so we can > emit a batch to check for register writes at screen creation time. > > The first 4 patches refactor some of the interl_batchbuffer code so they don't > require a brw_context parameter. This way we can reuse the code directly at > screen creation time. Patches 1-4 are: Reviewed-by: Kenneth GraunkeRegardless of what we decide to do, I think they're a nice cleanup, and we may as well land them. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/53] i965: get InfoLog and LinkStatus via the pointer in gl_program
Thanks, I didn't pull -r :) Reviewed-by: Lionel LandwerlinOn 03/01/17 11:44, Timothy Arceri wrote: On Tue, 2017-01-03 at 11:39 +, Lionel Landwerlin wrote: Did you left out accesses to prog-> and shader_prog-> on purpose in : brw_tcs.c brw_tes.c brw_vs.c brw_wm.c ? This patch should really have been squashed with this one I pushed today [1]. I only just noticed after I pushed it that a had a separate one for cs. [1] https://cgit.freedesktop.org/mesa/mesa/commit/?id=b880281f0bb3f4cd6 5d38ae13a0db2dba6d7a5ed On 03/01/17 02:43, Timothy Arceri wrote: --- src/mesa/drivers/dri/i965/brw_cs.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c index 522ddb9..f220846 100644 --- a/src/mesa/drivers/dri/i965/brw_cs.c +++ b/src/mesa/drivers/dri/i965/brw_cs.c @@ -69,10 +69,10 @@ brw_codegen_cs_prog(struct brw_context *brw, memset(_data, 0, sizeof(prog_data)); if (cp->program.info.cs.shared_size > 64 * 1024) { - prog->data->LinkStatus = false; + cp->program.sh.data->LinkStatus = false; const char *error_str = "Compute shader used more than 64KB of shared variables"; - ralloc_strcat(>data->InfoLog, error_str); + ralloc_strcat(>program.sh.data->InfoLog, error_str); _mesa_problem(NULL, "Failed to link compute shader: %s\n", error_str); ralloc_free(mem_ctx); @@ -122,8 +122,8 @@ brw_codegen_cs_prog(struct brw_context *brw, _data, cp->program.nir, st_index, _size, _str); if (program == NULL) { - prog->data->LinkStatus = false; - ralloc_strcat(>data->InfoLog, error_str); + cp->program.sh.data->LinkStatus = false; + ralloc_strcat(>program.sh.data->InfoLog, error_str); _mesa_problem(NULL, "Failed to compile compute shader: %s\n", error_str); ralloc_free(mem_ctx); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99253] Radeon R7 370 / R9 270X/370X PCIe Bus Error: severity=Uncorrected (Fatal)
https://bugs.freedesktop.org/show_bug.cgi?id=99253 jordanchanged: What|Removed |Added CC||jordanca...@gmail.com -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99253] Radeon R7 370 / R9 270X/370X PCIe Bus Error: severity=Uncorrected (Fatal)
https://bugs.freedesktop.org/show_bug.cgi?id=99253 Bug ID: 99253 Summary: Radeon R7 370 / R9 270X/370X PCIe Bus Error: severity=Uncorrected (Fatal) Product: Mesa Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: critical Priority: medium Component: Other Assignee: mesa-dev@lists.freedesktop.org Reporter: jordanca...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org I'd be more than happy to provide more information this is my first bug post. The screen goes black. This happens at random times while using the system but can be reproduced faster playing videos. To catch the error I just loaded up netflix for about 5 minutes. user@user:$ lshw -c video *-display description: VGA compatible controller product: Curacao XT / Trinidad XT [Radeon R7 370 / R9 270X/370X] vendor: Advanced Micro Devices, Inc. [AMD/ATI] physical id: 0 bus info: pci@:6e:00.0 version: 00 width: 64 bits clock: 33MHz capabilities: pm pciexpress msi vga_controller cap_list rom configuration: driver=radeon latency=0 resources: irq:38 memory:c000-cfff memory:fa10-fa13 ioport:e000(size=256) memory:c-d user@user:$ modinfo /lib/modules/4.8.0-32-generic/kernel/drivers/gpu/drm/radeon/radeon.ko filename: /lib/modules/4.8.0-32-generic/kernel/drivers/gpu/drm/radeon/radeon.ko license:GPL and additional rights description:ATI Radeon author: Gareth Hughes, Keith Whitwell, others. firmware: radeon/R520_cp.bin firmware: radeon/RS600_cp.bin firmware: radeon/RS690_cp.bin firmware: radeon/R420_cp.bin firmware: radeon/R300_cp.bin firmware: radeon/R200_cp.bin firmware: radeon/R100_cp.bin firmware: radeon/SUMO2_me.bin firmware: radeon/SUMO2_pfp.bin firmware: radeon/SUMO_me.bin firmware: radeon/SUMO_pfp.bin firmware: radeon/SUMO_rlc.bin firmware: radeon/PALM_me.bin firmware: radeon/PALM_pfp.bin firmware: radeon/CYPRESS_smc.bin firmware: radeon/CYPRESS_rlc.bin firmware: radeon/CYPRESS_me.bin firmware: radeon/CYPRESS_pfp.bin firmware: radeon/JUNIPER_smc.bin firmware: radeon/JUNIPER_rlc.bin firmware: radeon/JUNIPER_me.bin firmware: radeon/JUNIPER_pfp.bin firmware: radeon/REDWOOD_smc.bin firmware: radeon/REDWOOD_rlc.bin firmware: radeon/REDWOOD_me.bin firmware: radeon/REDWOOD_pfp.bin firmware: radeon/CEDAR_smc.bin firmware: radeon/CEDAR_rlc.bin firmware: radeon/CEDAR_me.bin firmware: radeon/CEDAR_pfp.bin firmware: radeon/R700_rlc.bin firmware: radeon/R600_rlc.bin firmware: radeon/RV710_smc.bin firmware: radeon/RV710_me.bin firmware: radeon/RV710_pfp.bin firmware: radeon/RV740_smc.bin firmware: radeon/RV730_smc.bin firmware: radeon/RV730_me.bin firmware: radeon/RV730_pfp.bin firmware: radeon/RV770_smc.bin firmware: radeon/RV770_me.bin firmware: radeon/RV770_pfp.bin firmware: radeon/RS780_me.bin firmware: radeon/RS780_pfp.bin firmware: radeon/RV670_me.bin firmware: radeon/RV670_pfp.bin firmware: radeon/RV635_me.bin firmware: radeon/RV635_pfp.bin firmware: radeon/RV620_me.bin firmware: radeon/RV620_pfp.bin firmware: radeon/RV630_me.bin firmware: radeon/RV630_pfp.bin firmware: radeon/RV610_me.bin firmware: radeon/RV610_pfp.bin firmware: radeon/R600_me.bin firmware: radeon/R600_pfp.bin firmware: radeon/ARUBA_rlc.bin firmware: radeon/ARUBA_me.bin firmware: radeon/ARUBA_pfp.bin firmware: radeon/CAYMAN_smc.bin firmware: radeon/CAYMAN_rlc.bin firmware: radeon/CAYMAN_mc.bin firmware: radeon/CAYMAN_me.bin firmware: radeon/CAYMAN_pfp.bin firmware: radeon/CAICOS_smc.bin firmware: radeon/CAICOS_mc.bin firmware: radeon/CAICOS_me.bin firmware: radeon/CAICOS_pfp.bin firmware: radeon/TURKS_smc.bin firmware: radeon/TURKS_mc.bin firmware: radeon/TURKS_me.bin firmware: radeon/TURKS_pfp.bin firmware: radeon/BTC_rlc.bin firmware: radeon/BARTS_smc.bin firmware: radeon/BARTS_mc.bin firmware: radeon/BARTS_me.bin firmware: radeon/BARTS_pfp.bin firmware: radeon/hainan_k_smc.bin firmware: radeon/hainan_smc.bin firmware: radeon/hainan_rlc.bin firmware: radeon/hainan_mc.bin firmware: radeon/hainan_ce.bin firmware: radeon/hainan_me.bin firmware: radeon/hainan_pfp.bin firmware: radeon/HAINAN_smc.bin firmware: radeon/HAINAN_rlc.bin firmware: radeon/HAINAN_mc2.bin firmware: radeon/HAINAN_mc.bin firmware: radeon/HAINAN_ce.bin
Re: [Mesa-dev] [PATCH 0/6] Enable OpenGL 4.0 on Haswell
On Tue, 2017-01-03 at 11:14 +, Chris Wilson wrote: > On Tue, Jan 03, 2017 at 11:42:50AM +0100, Iago Toral Quiroga wrote: > > > > Enabling GL 4.0 in gen7 requires a bit of work because some > > hardware and kernel > > combinations may not support all the features. Specifically, we > > need to know > > if the kernel can do pipelined register writes. Unfortunately, this > > requires > > that we emit batches at screen creation time when we don't have a > > brw_context > > available (all our current batch emission infrastructure relies on > > this). > > See [1] and [2] for more details. > Emitting a batch is trivial: > > https://lists.freedesktop.org/archives/mesa-dev/2015-August/091077.ht > ml Oh, thanks, I did not know you had done this before. It seems that patch got an Rb from Martin when it was sent in 2015 but it never landed, right? was there any other issue holding it back? > But we do want to share the bufmgr between screens, and you do want > to > deprecate the current intel_batchbuffer.c due to the very large > overhead > it imposes. It seems that Curro and Ken were more interested in trying to reuse a bit more of the intel_batchbuffer.c for this rather than the other way around, but then again that might have only been because my original implementation was very focused on porting code from that file... your patch gets this done without using a batchbuffer object at all and I think it is quite clean. If Ken and Curro also like it and there are no other concerns I'll be happy to adapt/rebase your patch instead. Ken was also saying that he was planning some rework of the batch emission code, so I don't know what strategy fits his plans better. Iago > -Chris > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 06/25] spirv: fix SpvOpSpecConstantOp with SpvOpVectorShuffle working with double-based vecs
On Mon, 2017-01-02 at 10:10 -0800, Jason Ekstrand wrote: > > > On Fri, Dec 16, 2016 at 6:48 AM, Juan A. Suarez Romeroia.com> wrote: > > From: Samuel Iglesias Gonsálvez > > > > We need to pick two 32-bit values per component to perform the > > right shuffle operation. > > > > Signed-off-by: Samuel Iglesias Gonsálvez > > --- > > src/compiler/spirv/spirv_to_nir.c | 25 + > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/src/compiler/spirv/spirv_to_nir.c > > b/src/compiler/spirv/spirv_to_nir.c > > index 5303a94..5126dc9 100644 > > --- a/src/compiler/spirv/spirv_to_nir.c > > +++ b/src/compiler/spirv/spirv_to_nir.c > > @@ -1073,18 +1073,35 @@ vtn_handle_constant(struct vtn_builder *b, > > SpvOp opcode, > > unsigned len0 = glsl_get_vector_elements(v0->const_type); > > unsigned len1 = glsl_get_vector_elements(v1->const_type); > > > > - uint32_t u[8]; > > + if (glsl_get_bit_size(v0->const_type) == 64) > > + len0 *= 2; > > + if (glsl_get_bit_size(v1->const_type) == 64) > > + len1 *= 2; > > + > > + /* Allocate space for two dvec4s */ > > + uint32_t u[16]; > > + assert(len0 + len1 < 16); > > for (unsigned i = 0; i < len0; i++) > > u[i] = v0->constant->values[0].u32[i]; > > for (unsigned i = 0; i < len1; i++) > > u[len0 + i] = v1->constant->values[0].u32[i]; > > > > - for (unsigned i = 0; i < count - 6; i++) { > > + unsigned bit_size = glsl_get_bit_size(val->const_type); > > It wouldn't hurt to assert that all of the values have the same bit > size. In fact, they're all required to have the same base type and > bit width. The only thing allowed to vary between the three is the > number of components. > OK, then I will add that assert. > In light of that, does it make sense to do as 16 uint32_t's or would > it work better just to assert that all the bit sizes are the same and > switch on bit size once? > I am going to simplify this code by taking into account the bit size. My idea is to have two arrays: one with 8 uint32_t's and another with 8 uint64_t's... and fill-and-use one or the other depending on the bit_size. Sam > > + for (unsigned i = 0, j = 0; i < count - 6; i++, j++) { > > uint32_t comp = w[i + 6]; > > + /* In case of doubles, we need to pick two 32-bit > > values, > > + * then we duplicate the component to pick the right > > values. > > + */ > > + if (bit_size == 64) > > + comp *= 2; > > if (comp == (uint32_t)-1) { > > - val->constant->values[0].u32[i] = 0xdeadbeef; > > + val->constant->values[0].u32[j] = 0xdeadbeef; > > + if (bit_size == 64) > > + val->constant->values[0].u32[++j] = 0xdeadbeef; > > } else { > > - val->constant->values[0].u32[i] = u[comp]; > > + val->constant->values[0].u32[j] = u[comp]; > > + if (bit_size == 64) > > + val->constant->values[0].u32[++j] = u[comp + 1]; > > } > > } > > break; > > -- > > 2.9.3 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 00/25] Enable Float64 capability support for Intel's Vulkan driver
On Mon, 2017-01-02 at 10:20 -0800, Jason Ekstrand wrote: > 1-5 and 7-11 are > > Reviewed-by: Jason Ekstrand> > I left a few trivial comments on them but nothing substantial. > OK, I will fix them. Thanks for the review, Sam > On Fri, Dec 16, 2016 at 6:48 AM, Juan A. Suarez Romero > wrote: > > This patch series is a second iteration of previous one: > > > > https://lists.freedesktop.org/archives/mesa-dev/2016-November/13650 > > 7.html > > > > Main changes are the ones suggested by Jason, and also a refactor > > of the way > > inputs_read bitmap is used in NIR. > > > > If you want to test these patches, you can clone our branch with > > the following > > command: > > > > $ git clone -b spirv-to-nir-rc2 https://github.com/Igalia/mesa.g > > ithub > > > > Thanks, > > > > J.A. > > > > > > > > Juan A. Suarez Romero (2): > > anv/pipeline: get map for double input attributes > > nir/i965: use two slots from inputs_read for dvec3/dvec4 vertex > > input > > attributes > > > > Samuel Iglesias Gonsálvez (23): > > spirv: fix typo in spec_constant_decoration_cb() > > spirv: add definition of double based data types > > spirv: add support for loading DF constants > > spirv: add DF support to vtn_const_ssa_value() > > spirv: add DF support to SpvOp*ConstantComposite > > spirv: fix SpvOpSpecConstantOp with SpvOpVectorShuffle working > > with > > double-based vecs > > spirv: add double support to SpvOpCompositeExtract > > spirv: add double support to _vtn_variable_load_store > > spirv: add double support to _vtn_block_load_store() > > spirv: Enable double floating points when copying variables in > > _vtn_variable_copy() > > spirv: add support for doubles on OpComposite{Insert,Extract} > > compiler/nir: add glsl_type_is_{float,integer}() > > nir: add nir_get_nir_type_for_glsl_type() > > nir: add nir_type_conversion_op() > > spirv/nir: implement DF conversions > > spirv/nir: add (un)packDouble2x32() translation > > spirv: add support for doubles to OpSpecConstant > > isl: fix VA64 support for double and dvecN vertex attributes > > nir: Add flag to detect platforms with native float64 support > > spirv: Add nir_options to vtn_builder > > spirv: enable SpvCapabilityFloat64 only to supported platforms > > i965: enable nir_option's native_float64 to supported generations > > anv: enable shaderFloat64 feature > > > > src/amd/vulkan/radv_pipeline.c | 5 +- > > src/compiler/glsl/glsl_to_nir.cpp | 28 ++ > > src/compiler/nir/nir.c | 83 > > > > src/compiler/nir/nir.h | 26 + > > src/compiler/nir/nir_gather_info.c | 48 + > > src/compiler/nir_types.cpp | 15 +++ > > src/compiler/nir_types.h | 2 + > > src/compiler/spirv/nir_spirv.h | 5 +- > > src/compiler/spirv/spirv_to_nir.c | 141 > > ++- > > src/compiler/spirv/vtn_alu.c | 29 +++--- > > src/compiler/spirv/vtn_glsl450.c | 2 + > > src/compiler/spirv/vtn_private.h | 4 +- > > src/compiler/spirv/vtn_variables.c | 3 + > > src/intel/isl/isl_format.c | 4 +- > > src/intel/isl/isl_format_layout.csv | 1 - > > src/intel/vulkan/anv_device.c | 2 +- > > src/intel/vulkan/anv_formats.c | 8 +- > > src/intel/vulkan/anv_pipeline.c | 6 +- > > src/intel/vulkan/genX_pipeline.c | 63 +++- > > src/mesa/drivers/dri/i965/brw_compiler.c | 36 --- > > src/mesa/drivers/dri/i965/brw_draw_upload.c | 11 ++- > > src/mesa/drivers/dri/i965/brw_fs.cpp | 13 --- > > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 3 +- > > src/mesa/drivers/dri/i965/brw_nir.c | 6 +- > > src/mesa/drivers/dri/i965/brw_nir.h | 1 - > > src/mesa/drivers/dri/i965/brw_vec4.cpp | 11 +-- > > 26 files changed, 419 insertions(+), 137 deletions(-) > > > > -- > > 2.9.3 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 13/25] nir: add nir_get_nir_type_for_glsl_type()
On Mon, 2017-01-02 at 10:16 -0800, Jason Ekstrand wrote: > On Fri, Dec 16, 2016 at 6:48 AM, Juan A. Suarez Romeroia.com> wrote: > > From: Samuel Iglesias Gonsálvez > > > > Signed-off-by: Samuel Iglesias Gonsálvez > > --- > > src/compiler/nir/nir.h | 19 +++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h > > index 544d4ba..9310dab 100644 > > --- a/src/compiler/nir/nir.h > > +++ b/src/compiler/nir/nir.h > > @@ -670,6 +670,25 @@ nir_alu_type_get_base_type(nir_alu_type type) > > return type & NIR_ALU_TYPE_BASE_TYPE_MASK; > > } > > > > +static inline nir_alu_type > > +nir_get_nir_type_for_glsl_type(const struct glsl_type *type) > > +{ > > + unsigned bit_size = glsl_get_bit_size(type); > > + if (glsl_type_is_boolean(type)) > > + return (nir_alu_type)(nir_type_bool | bit_size); > > + > > + if (glsl_type_is_integer(type)) { > > + if (glsl_get_base_type(type) == GLSL_TYPE_UINT) > > + return (nir_alu_type)(nir_type_uint | bit_size); > > + else > > + return (nir_alu_type)(nir_type_int | bit_size); > > + } > > + > > + if (glsl_type_is_float(type)) > > + return (nir_alu_type)(nir_type_float | bit_size); > > Wouldn't this be easier to implement as > > switch (glsl_get_base_type(type)) { > case GLSL_TYPE_BOOL: > return nir_type_bool32; > case GLSL_TYPE_UINT: > return nir_type_uint32; > /* etc. */ > } > > Then you could drop the is_integer and is_float helpers. > Right. My idea was to do it more generic by taking into account bit_size. Hence, int64 and float16 support would not need to change this function. In any case, this can be done in a future patch. I will do your approach. Thanks, Sam > > + unreachable("unknown type"); > > +} > > + > > typedef enum { > > NIR_OP_IS_COMMUTATIVE = (1 << 0), > > NIR_OP_IS_ASSOCIATIVE = (1 << 1), > > -- > > 2.9.3 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 14/25] nir: add nir_type_conversion_op()
On Mon, 2017-01-02 at 10:34 -0800, Jason Ekstrand wrote: > I've left some inline comments but this patch has gotten me thinking > about the larger question of how we want to do conversion operations > in NIR. Ian already has patches for int64 but float16, int8, and > int16 are all coming. If we keep coming up with one-letter prefixes > (long, half, short, char?) and doing a2b for every opcode. We could, > but that would be very confusing. It would be good to cut down on > the combinatorics a bit. > > Here's my straw-man proposal: For each conversion, we leave the > destination as variable bit width and keep the source fixed. Then we > have u2f32, u2f64, u2f16, etc. and we have one opcode regardless of > destination bit width. This doesn't completely break the > combinatorial explosion but it does let us cut it by a factor of 3 or > 4. > > Another thing that Connor and I discussed was to let conversion > operations be the one exception to the "source and destination bit- > widths must match for variable bit-width operations" and add i2i and > f2f opcodes. That would drop the number of opcodes even further but > I'm not sure how bad the special-casing would end up being. > > Thoughts? > The first option doesn't really address the problem and feels more like a work-around, but I guess we could go with that if other people prefer that approach. Personally, I think i would try the second option. I can't tell right know the implications this would have for the special- casing in the validation of variable bit-width opcodes, etc but I guess we can just try to write that patch and see how it looks like in the end, we can always resort to u2f32 and cia if we don't like it. In any case, regardless the solution we agree on, any of these changes would require time to get them right as they touch several places in NIR, so considering that the feature freeze for the next Mesa release is around the corner (January 13th [0]) I think we also need to discuss whether we want to tackle this before or after we land Vulkan Fp64. We are leaning towards landing Vulkan Fp64 before we do these changes so we can have that in the next release and also because we need the vertex input attributes changes introduced with this series for our Haswell VA64 implementation (which we were hoping to send this week for review too, although maybe we are trying to squeeze too many things for the next release :-D) What do you think? Sam [0] https://lists.freedesktop.org/archives/mesa-dev/2016-November/13701 5.html > --Jason Ekstrand > > On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romeroia.com> wrote: > > From: Samuel Iglesias Gonsálvez > > > > This function returns the nir_op corresponding to the conversion > > between > > the given nir_alu_type arguments. > > > > This function lacks support for integer-based types with bit_size > > != 32 > > and for float16 conversion ops. > > > > Signed-off-by: Samuel Iglesias Gonsálvez > > --- > > src/compiler/nir/nir.c | 83 > > ++ > > src/compiler/nir/nir.h | 2 ++ > > 2 files changed, 85 insertions(+) > > > > diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c > > index 2d882f7..3e00452 100644 > > --- a/src/compiler/nir/nir.c > > +++ b/src/compiler/nir/nir.c > > @@ -1953,3 +1953,86 @@ > > nir_system_value_from_intrinsic(nir_intrinsic_op intrin) > > unreachable("intrinsic doesn't produce a system value"); > > } > > } > > + > > +nir_op > > +nir_type_conversion_op(nir_alu_type src, nir_alu_type dst) > > +{ > > + nir_alu_type src_base_type = (nir_alu_type) > > nir_alu_type_get_base_type(src); > > + nir_alu_type dst_base_type = (nir_alu_type) > > nir_alu_type_get_base_type(dst); > > + unsigned src_bitsize = nir_alu_type_get_type_size(src); > > + unsigned dst_bitsize = nir_alu_type_get_type_size(dst); > > + > > + if (src_base_type == dst_base_type) { > > + if (src_bitsize == dst_bitsize) > > + return (src_base_type == nir_type_float) ? nir_op_fmov : > > nir_op_imov; > > + > > + switch (src_base_type) { > > + case nir_type_bool: > > + case nir_type_uint: > > + case nir_type_int: > > + return nir_op_imov; > > These three cases can't happen right now. We should just assert that > it's float. > > > + case nir_type_float: > > + /* TODO: implement support for float16 */ > > + assert(src_bitsize == 64 || dst_bitsize == 64); > > + return (src_bitsize == 64) ? nir_op_d2f : nir_op_f2d; > > + default: > > + assert(!"Invalid conversion"); > > + }; > > + } > > + > > + /* Different base type but same bit_size */ > > + if (src_bitsize == dst_bitsize) { > > + /* TODO: This does not include specific conversions between > > + * signed or unsigned integer types of bit size different of > > 32 yet. > > + */ > > + assert(src_bitsize == 32); >
[Mesa-dev] [PATCH 1/2] vec4: use DIM instruction when loading DF immediates in HSW
Signed-off-by: Samuel Iglesias Gonsálvez--- src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 9 + 1 file changed, 9 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp index 065e317..98e023a 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp @@ -1208,6 +1208,15 @@ vec4_visitor::setup_imm_df(double v) if (devinfo->gen >= 8) return brw_imm_df(v); + /* gen7.5 does not support DF immediates straighforward but the DIM +* instruction allows to set the 64-bit immediate value. +*/ + if (devinfo->is_haswell) { + dst_reg dst = retype(dst_reg(VGRF, alloc.allocate(2)), BRW_REGISTER_TYPE_DF); + emit(DIM(dst, brw_imm_df(v)))->force_writemask_all = true; + return swizzle(src_reg(retype(dst, BRW_REGISTER_TYPE_DF)), BRW_SWIZZLE_); + } + /* gen7 does not support DF immediates */ union { double d; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965/disasm: remove printing hstride and width in align16 DF source regions
Signed-off-by: Samuel Iglesias Gonsálvez--- src/mesa/drivers/dri/i965/brw_disasm.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c b/src/mesa/drivers/dri/i965/brw_disasm.c index 6de52b3..167067a 100644 --- a/src/mesa/drivers/dri/i965/brw_disasm.c +++ b/src/mesa/drivers/dri/i965/brw_disasm.c @@ -942,10 +942,7 @@ src_da16(FILE *file, format(file, ".%d", 16 / reg_type_size[_reg_type]); string(file, "<"); err |= control(file, "vert stride", vert_stride, _vert_stride, NULL); - if (reg_type_size[_reg_type] == 8) - string(file, ",2,1>"); - else - string(file, ",4,1>"); + string(file, ">"); err |= src_swizzle(file, BRW_SWIZZLE4(swz_x, swz_y, swz_z, swz_w)); err |= control(file, "src da16 reg type", reg_encoding, _reg_type, NULL); return err; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] error handling
Am 03.01.2017 um 11:47 schrieb Nayan Deshmukh: Hi Christian, Can you please push the patches for me. Done. Christian. Regards, Nayan Nayan Deshmukh (3): vl/compositor: implement error handling st/vdpau: error handling st/va: error handling src/gallium/auxiliary/vl/vl_compositor.c | 13 -- src/gallium/auxiliary/vl/vl_compositor.h | 2 +- src/gallium/state_trackers/va/context.c | 18 ++--- src/gallium/state_trackers/vdpau/device.c | 8 +- src/gallium/state_trackers/vdpau/mixer.c | 43 +++ src/gallium/state_trackers/vdpau/output.c | 14 +++--- 6 files changed, 77 insertions(+), 21 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/53] i965: get InfoLog and LinkStatus via the pointer in gl_program
On Tue, 2017-01-03 at 11:39 +, Lionel Landwerlin wrote: > Did you left out accesses to prog-> and shader_prog-> on purpose in : > brw_tcs.c > brw_tes.c > brw_vs.c > brw_wm.c ? This patch should really have been squashed with this one I pushed today [1]. I only just noticed after I pushed it that a had a separate one for cs. [1] https://cgit.freedesktop.org/mesa/mesa/commit/?id=b880281f0bb3f4cd6 5d38ae13a0db2dba6d7a5ed > > > On 03/01/17 02:43, Timothy Arceri wrote: > > --- > > src/mesa/drivers/dri/i965/brw_cs.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_cs.c > > b/src/mesa/drivers/dri/i965/brw_cs.c > > index 522ddb9..f220846 100644 > > --- a/src/mesa/drivers/dri/i965/brw_cs.c > > +++ b/src/mesa/drivers/dri/i965/brw_cs.c > > @@ -69,10 +69,10 @@ brw_codegen_cs_prog(struct brw_context *brw, > > memset(_data, 0, sizeof(prog_data)); > > > > if (cp->program.info.cs.shared_size > 64 * 1024) { > > - prog->data->LinkStatus = false; > > + cp->program.sh.data->LinkStatus = false; > > const char *error_str = > > "Compute shader used more than 64KB of shared > > variables"; > > - ralloc_strcat(>data->InfoLog, error_str); > > + ralloc_strcat(>program.sh.data->InfoLog, error_str); > > _mesa_problem(NULL, "Failed to link compute shader: %s\n", > > error_str); > > > > ralloc_free(mem_ctx); > > @@ -122,8 +122,8 @@ brw_codegen_cs_prog(struct brw_context *brw, > > _data, cp->program.nir, > > st_index, > > _size, _str); > > if (program == NULL) { > > - prog->data->LinkStatus = false; > > - ralloc_strcat(>data->InfoLog, error_str); > > + cp->program.sh.data->LinkStatus = false; > > + ralloc_strcat(>program.sh.data->InfoLog, error_str); > > _mesa_problem(NULL, "Failed to compile compute shader: > > %s\n", error_str); > > > > ralloc_free(mem_ctx); > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 04/53] i965: get InfoLog and LinkStatus via the pointer in gl_program
Did you left out accesses to prog-> and shader_prog-> on purpose in : brw_tcs.c brw_tes.c brw_vs.c brw_wm.c ? On 03/01/17 02:43, Timothy Arceri wrote: --- src/mesa/drivers/dri/i965/brw_cs.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c index 522ddb9..f220846 100644 --- a/src/mesa/drivers/dri/i965/brw_cs.c +++ b/src/mesa/drivers/dri/i965/brw_cs.c @@ -69,10 +69,10 @@ brw_codegen_cs_prog(struct brw_context *brw, memset(_data, 0, sizeof(prog_data)); if (cp->program.info.cs.shared_size > 64 * 1024) { - prog->data->LinkStatus = false; + cp->program.sh.data->LinkStatus = false; const char *error_str = "Compute shader used more than 64KB of shared variables"; - ralloc_strcat(>data->InfoLog, error_str); + ralloc_strcat(>program.sh.data->InfoLog, error_str); _mesa_problem(NULL, "Failed to link compute shader: %s\n", error_str); ralloc_free(mem_ctx); @@ -122,8 +122,8 @@ brw_codegen_cs_prog(struct brw_context *brw, _data, cp->program.nir, st_index, _size, _str); if (program == NULL) { - prog->data->LinkStatus = false; - ralloc_strcat(>data->InfoLog, error_str); + cp->program.sh.data->LinkStatus = false; + ralloc_strcat(>program.sh.data->InfoLog, error_str); _mesa_problem(NULL, "Failed to compile compute shader: %s\n", error_str); ralloc_free(mem_ctx); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/53] i965: stop depending on gl_shader_program for brw_compute_vue_map() params
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: This removes another dependency on gl_shader_program from the codegen functions, this will help allow us to use gl_program for the CurrentProgram array rather than gl_shader_program. --- src/mesa/drivers/dri/i965/brw_gs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c index 76ceb5b..7886737 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.c +++ b/src/mesa/drivers/dri/i965/brw_gs.c @@ -134,7 +134,7 @@ brw_codegen_gs_prog(struct brw_context *brw, brw_compute_vue_map(devinfo, _data.base.vue_map, outputs_written, - prog->SeparateShader); + gp->program.info.separate_shader); int st_index = -1; if (INTEL_DEBUG & DEBUG_SHADER_TIME) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/53] i965: get shared_size from shader_info rather than gl_shader_program
Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: --- src/mesa/drivers/dri/i965/brw_cs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c index e4d82cf..522ddb9 100644 --- a/src/mesa/drivers/dri/i965/brw_cs.c +++ b/src/mesa/drivers/dri/i965/brw_cs.c @@ -68,7 +68,7 @@ brw_codegen_cs_prog(struct brw_context *brw, memset(_data, 0, sizeof(prog_data)); - if (prog->Comp.SharedSize > 64 * 1024) { + if (cp->program.info.cs.shared_size > 64 * 1024) { prog->data->LinkStatus = false; const char *error_str = "Compute shader used more than 64KB of shared variables"; @@ -78,7 +78,7 @@ brw_codegen_cs_prog(struct brw_context *brw, ralloc_free(mem_ctx); return false; } else { - prog_data.base.total_shared = prog->Comp.SharedSize; + prog_data.base.total_shared = cp->program.info.cs.shared_size; } assign_cs_binding_table_offsets(devinfo, prog, >program, _data); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/53] i965: pass gl_program to the brw_*_debug_recompile() functions
Looks fine to me : Reviewed-by: Lionel LandwerlinOn 03/01/17 02:43, Timothy Arceri wrote: Rather then passing gl_shader_program. The only field use was Name which is the same as the Id field in gl_program. For wm and vs we also make the functions static and move them before the codegen functions. This change reduces the codegen functions dependency on gl_shader_program. --- src/mesa/drivers/dri/i965/brw_gs.c | 8 +-- src/mesa/drivers/dri/i965/brw_tcs.c | 7 +- src/mesa/drivers/dri/i965/brw_tes.c | 7 +- src/mesa/drivers/dri/i965/brw_vs.c | 109 --- src/mesa/drivers/dri/i965/brw_vs.h | 4 -- src/mesa/drivers/dri/i965/brw_wm.c | 125 ++-- src/mesa/drivers/dri/i965/brw_wm.h | 3 - 7 files changed, 125 insertions(+), 138 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c index 47b4e5c..76ceb5b 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.c +++ b/src/mesa/drivers/dri/i965/brw_gs.c @@ -37,16 +37,14 @@ #include "compiler/glsl/ir_uniform.h" static void -brw_gs_debug_recompile(struct brw_context *brw, - struct gl_shader_program *shader_prog, +brw_gs_debug_recompile(struct brw_context *brw, struct gl_program *prog, const struct brw_gs_prog_key *key) { struct brw_cache_item *c = NULL; const struct brw_gs_prog_key *old_key = NULL; bool found = false; - perf_debug("Recompiling geometry shader for program %d\n", - shader_prog->Name); + perf_debug("Recompiling geometry shader for program %d\n", prog->Id); for (unsigned int i = 0; i < brw->cache.size; i++) { for (c = brw->cache.items[i]; c; c = c->next) { @@ -164,7 +162,7 @@ brw_codegen_gs_prog(struct brw_context *brw, if (unlikely(brw->perf_debug)) { if (gp->compiled_once) { - brw_gs_debug_recompile(brw, prog, key); + brw_gs_debug_recompile(brw, >program, key); } if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) { perf_debug("GS compile took %.03f ms and stalled the GPU\n", diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c b/src/mesa/drivers/dri/i965/brw_tcs.c index f890ccf..3f8798a 100644 --- a/src/mesa/drivers/dri/i965/brw_tcs.c +++ b/src/mesa/drivers/dri/i965/brw_tcs.c @@ -116,8 +116,7 @@ create_passthrough_tcs(void *mem_ctx, const struct brw_compiler *compiler, } static void -brw_tcs_debug_recompile(struct brw_context *brw, - struct gl_shader_program *shader_prog, +brw_tcs_debug_recompile(struct brw_context *brw, struct gl_program *prog, const struct brw_tcs_prog_key *key) { struct brw_cache_item *c = NULL; @@ -125,7 +124,7 @@ brw_tcs_debug_recompile(struct brw_context *brw, bool found = false; perf_debug("Recompiling tessellation control shader for program %d\n", - shader_prog->Name); + prog->Id); for (unsigned int i = 0; i < brw->cache.size; i++) { for (c = brw->cache.items[i]; c; c = c->next) { @@ -280,7 +279,7 @@ brw_codegen_tcs_prog(struct brw_context *brw, if (unlikely(brw->perf_debug)) { if (tcp) { if (tcp->compiled_once) { -brw_tcs_debug_recompile(brw, shader_prog, key); +brw_tcs_debug_recompile(brw, >program, key); } tcp->compiled_once = true; } diff --git a/src/mesa/drivers/dri/i965/brw_tes.c b/src/mesa/drivers/dri/i965/brw_tes.c index 2031366..9adde20 100644 --- a/src/mesa/drivers/dri/i965/brw_tes.c +++ b/src/mesa/drivers/dri/i965/brw_tes.c @@ -35,8 +35,7 @@ #include "program/prog_parameter.h" static void -brw_tes_debug_recompile(struct brw_context *brw, - struct gl_shader_program *shader_prog, +brw_tes_debug_recompile(struct brw_context *brw, struct gl_program *prog, const struct brw_tes_prog_key *key) { struct brw_cache_item *c = NULL; @@ -44,7 +43,7 @@ brw_tes_debug_recompile(struct brw_context *brw, bool found = false; perf_debug("Recompiling tessellation evaluation shader for program %d\n", - shader_prog->Name); + prog->Id); for (unsigned int i = 0; i < brw->cache.size; i++) { for (c = brw->cache.items[i]; c; c = c->next) { @@ -194,7 +193,7 @@ brw_codegen_tes_prog(struct brw_context *brw, if (unlikely(brw->perf_debug)) { if (tep->compiled_once) { - brw_tes_debug_recompile(brw, shader_prog, key); + brw_tes_debug_recompile(brw, >program, key); } if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) { perf_debug("TES compile took %.03f ms and stalled the GPU\n", diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index ede6097..2720f82 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++
Re: [Mesa-dev] [PATCH 0/6] Enable OpenGL 4.0 on Haswell
On Tue, Jan 03, 2017 at 11:42:50AM +0100, Iago Toral Quiroga wrote: > Enabling GL 4.0 in gen7 requires a bit of work because some hardware and > kernel > combinations may not support all the features. Specifically, we need to know > if the kernel can do pipelined register writes. Unfortunately, this requires > that we emit batches at screen creation time when we don't have a brw_context > available (all our current batch emission infrastructure relies on this). > See [1] and [2] for more details. Emitting a batch is trivial: https://lists.freedesktop.org/archives/mesa-dev/2015-August/091077.html But we do want to share the bufmgr between screens, and you do want to deprecate the current intel_batchbuffer.c due to the very large overhead it imposes. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 99237] Impossible to create transparent X11/EGL windows while respecting EGL_NATIVE_VISUAL_ID
https://bugs.freedesktop.org/show_bug.cgi?id=99237 Daniel Stonechanged: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #2 from Daniel Stone --- *** This bug has been marked as a duplicate of bug 67676 *** -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] st/vdpau: error handling
handle the cases when vl_compositor_set_csc_matrix(), vl_compositor_init_state() and vl_compositor_init() fail Signed-off-by: Nayan DeshmukhReviewed-by: Christian König --- src/gallium/state_trackers/vdpau/device.c | 8 +- src/gallium/state_trackers/vdpau/mixer.c | 43 +++ src/gallium/state_trackers/vdpau/output.c | 14 +++--- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/gallium/state_trackers/vdpau/device.c b/src/gallium/state_trackers/vdpau/device.c index 81b7582..8bae064 100644 --- a/src/gallium/state_trackers/vdpau/device.c +++ b/src/gallium/state_trackers/vdpau/device.c @@ -128,13 +128,19 @@ vdp_imp_device_create_x11(Display *display, int screen, VdpDevice *device, goto no_handle; } - vl_compositor_init(>compositor, dev->context); + if (!vl_compositor_init(>compositor, dev->context)) { + ret = VDP_STATUS_ERROR; + goto no_compositor; + } + pipe_mutex_init(dev->mutex); *get_proc_address = return VDP_STATUS_OK; +no_compositor: + vlRemoveDataHTAB(*device); no_handle: pipe_sampler_view_reference(>dummy_sv, NULL); no_resource: diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c index aca43c1..1014174 100644 --- a/src/gallium/state_trackers/vdpau/mixer.c +++ b/src/gallium/state_trackers/vdpau/mixer.c @@ -65,11 +65,18 @@ vlVdpVideoMixerCreate(VdpDevice device, pipe_mutex_lock(dev->mutex); - vl_compositor_init_state(>cstate, dev->context); + if (!vl_compositor_init_state(>cstate, dev->context)) { + ret = VDP_STATUS_ERROR; + goto no_compositor_state; + } vl_csc_get_matrix(VL_CSC_COLOR_STANDARD_BT_601, NULL, true, >csc); - if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) - vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, 1.0f, 0.0f); + if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) { + if (!vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, 1.0f, 0.0f)) { + ret = VDP_STATUS_ERROR; + goto err_csc_matrix; + } + } *mixer = vlAddDataHTAB(vmixer); if (*mixer == 0) { @@ -163,7 +170,9 @@ no_params: vlRemoveDataHTAB(*mixer); no_handle: +err_csc_matrix: vl_compositor_cleanup_state(>cstate); +no_compositor_state: pipe_mutex_unlock(dev->mutex); DeviceReference(>device, NULL); FREE(vmixer); @@ -690,8 +699,11 @@ vlVdpVideoMixerSetFeatureEnables(VdpVideoMixer mixer, case VDP_VIDEO_MIXER_FEATURE_LUMA_KEY: vmixer->luma_key.enabled = feature_enables[i]; if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) -vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, - vmixer->luma_key.luma_min, vmixer->luma_key.luma_max); +if (!vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, +vmixer->luma_key.luma_min, vmixer->luma_key.luma_max)) { + pipe_mutex_unlock(vmixer->device->mutex); + return VDP_STATUS_ERROR; +} break; case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: @@ -810,8 +822,11 @@ vlVdpVideoMixerSetAttributeValues(VdpVideoMixer mixer, else memcpy(vmixer->csc, vdp_csc, sizeof(vl_csc_matrix)); if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) -vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, - vmixer->luma_key.luma_min, vmixer->luma_key.luma_max); +if (!vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, + vmixer->luma_key.luma_min, vmixer->luma_key.luma_max)) { + ret = VDP_STATUS_ERROR; + goto fail; +} break; case VDP_VIDEO_MIXER_ATTRIBUTE_NOISE_REDUCTION_LEVEL: @@ -834,8 +849,11 @@ vlVdpVideoMixerSetAttributeValues(VdpVideoMixer mixer, } vmixer->luma_key.luma_min = val; if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) -vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, - vmixer->luma_key.luma_min, vmixer->luma_key.luma_max); +if (!vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, +vmixer->luma_key.luma_min, vmixer->luma_key.luma_max)) { + ret = VDP_STATUS_ERROR; + goto fail; +} break; case VDP_VIDEO_MIXER_ATTRIBUTE_LUMA_KEY_MAX_LUMA: @@ -846,8 +864,11 @@ vlVdpVideoMixerSetAttributeValues(VdpVideoMixer mixer, } vmixer->luma_key.luma_max = val; if (!debug_get_bool_option("G3DVL_NO_CSC", FALSE)) -vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, -
[Mesa-dev] [PATCH 1/3] vl/compositor: implement error handling
pipe_buffer_map and pipe_buffer_create may return NULL Signed-off-by: Nayan DeshmukhReviewed-by: Christian König --- src/gallium/auxiliary/vl/vl_compositor.c | 13 +++-- src/gallium/auxiliary/vl/vl_compositor.h | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/gallium/auxiliary/vl/vl_compositor.c b/src/gallium/auxiliary/vl/vl_compositor.c index 03a0a64..297c3ab 100644 --- a/src/gallium/auxiliary/vl/vl_compositor.c +++ b/src/gallium/auxiliary/vl/vl_compositor.c @@ -919,7 +919,7 @@ vl_compositor_cleanup(struct vl_compositor *c) cleanup_pipe_state(c); } -void +bool vl_compositor_set_csc_matrix(struct vl_compositor_state *s, vl_csc_matrix const *matrix, float luma_min, float luma_max) @@ -932,6 +932,9 @@ vl_compositor_set_csc_matrix(struct vl_compositor_state *s, PIPE_TRANSFER_WRITE | PIPE_TRANSFER_DISCARD_RANGE, _transfer); + if (!ptr) + return false; + memcpy(ptr, matrix, sizeof(vl_csc_matrix)); ptr += sizeof(vl_csc_matrix)/sizeof(float); @@ -939,6 +942,8 @@ vl_compositor_set_csc_matrix(struct vl_compositor_state *s, ptr[1] = luma_max; pipe_buffer_unmap(s->pipe, buf_transfer); + + return true; } void @@ -1246,10 +1251,14 @@ vl_compositor_init_state(struct vl_compositor_state *s, struct pipe_context *pip sizeof(csc_matrix) + 2*sizeof(float) ); + if (!s->csc_matrix) + return false; + vl_compositor_clear_layers(s); vl_csc_get_matrix(VL_CSC_COLOR_STANDARD_IDENTITY, NULL, true, _matrix); - vl_compositor_set_csc_matrix(s, (const vl_csc_matrix *)_matrix, 1.0f, 0.0f); + if (!vl_compositor_set_csc_matrix(s, (const vl_csc_matrix *)_matrix, 1.0f, 0.0f)) + return false; return true; } diff --git a/src/gallium/auxiliary/vl/vl_compositor.h b/src/gallium/auxiliary/vl/vl_compositor.h index ceab5e0..5460619 100644 --- a/src/gallium/auxiliary/vl/vl_compositor.h +++ b/src/gallium/auxiliary/vl/vl_compositor.h @@ -142,7 +142,7 @@ vl_compositor_init_state(struct vl_compositor_state *state, struct pipe_context /** * set yuv -> rgba conversion matrix */ -void +bool vl_compositor_set_csc_matrix(struct vl_compositor_state *settings, const vl_csc_matrix *matrix, float luma_min, float luma_max); -- 2.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] st/va: error handling
handle the cases when vl_compositor_set_csc_matrix(), vl_compositor_init_state() and vl_compositor_init() fail Signed-off-by: Nayan DeshmukhReviewed-by: Christian König --- src/gallium/state_trackers/va/context.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/gallium/state_trackers/va/context.c b/src/gallium/state_trackers/va/context.c index 65ba7db..884b794 100644 --- a/src/gallium/state_trackers/va/context.c +++ b/src/gallium/state_trackers/va/context.c @@ -155,11 +155,14 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx) if (!drv->htab) goto error_htab; - vl_compositor_init(>compositor, drv->pipe); - vl_compositor_init_state(>cstate, drv->pipe); + if (!vl_compositor_init(>compositor, drv->pipe)) + goto error_compositor; + if (!vl_compositor_init_state(>cstate, drv->pipe)) + goto error_compositor_state; vl_csc_get_matrix(VL_CSC_COLOR_STANDARD_BT_601, NULL, true, >csc); - vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, 1.0f, 0.0f); + if (!vl_compositor_set_csc_matrix(>cstate, (const vl_csc_matrix *)>csc, 1.0f, 0.0f)) + goto error_csc_matrix; pipe_mutex_init(drv->mutex); ctx->pDriverData = (void *)drv; @@ -177,6 +180,15 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx) return VA_STATUS_SUCCESS; +error_csc_matrix: + vl_compositor_cleanup_state(>cstate); + +error_compositor_state: + vl_compositor_cleanup(>cstate); + +error_compositor: + handle_table_destroy(drv->htab); + error_htab: drv->pipe->destroy(drv->pipe); -- 2.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] error handling
Hi Christian, Can you please push the patches for me. Regards, Nayan Nayan Deshmukh (3): vl/compositor: implement error handling st/vdpau: error handling st/va: error handling src/gallium/auxiliary/vl/vl_compositor.c | 13 -- src/gallium/auxiliary/vl/vl_compositor.h | 2 +- src/gallium/state_trackers/va/context.c | 18 ++--- src/gallium/state_trackers/vdpau/device.c | 8 +- src/gallium/state_trackers/vdpau/mixer.c | 43 +++ src/gallium/state_trackers/vdpau/output.c | 14 +++--- 6 files changed, 77 insertions(+), 21 deletions(-) -- 2.9.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/6] i965: make intel_batchbuffer_emit_dword() take a batchbuffer as argument
--- src/mesa/drivers/dri/i965/intel_batchbuffer.c | 6 +++--- src/mesa/drivers/dri/i965/intel_batchbuffer.h | 18 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c index f45ef03..75e2fb3 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c @@ -119,7 +119,7 @@ intel_batchbuffer_require_space(struct brw_context *brw, GLuint sz, #ifdef DEBUG assert(sz < BATCH_SZ - BATCH_RESERVED); #endif - if (intel_batchbuffer_space(brw) < sz) + if (intel_batchbuffer_space(>batch) < sz) intel_batchbuffer_flush(brw); enum brw_gpu_ring prev_ring = brw->batch.ring; @@ -408,10 +408,10 @@ _intel_batchbuffer_flush(struct brw_context *brw, brw_finish_batch(brw); /* Mark the end of the buffer. */ - intel_batchbuffer_emit_dword(brw, MI_BATCH_BUFFER_END); + intel_batchbuffer_emit_dword(>batch, MI_BATCH_BUFFER_END); if (USED_BATCH(brw->batch) & 1) { /* Round batchbuffer usage to 2 DWORDs. */ - intel_batchbuffer_emit_dword(brw, MI_NOOP); + intel_batchbuffer_emit_dword(>batch, MI_NOOP); } intel_upload_finish(brw); diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h b/src/mesa/drivers/dri/i965/intel_batchbuffer.h index c4573ec..aca8fa1 100644 --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h @@ -94,27 +94,27 @@ static inline uint32_t float_as_int(float f) * work... */ static inline unsigned -intel_batchbuffer_space(struct brw_context *brw) +intel_batchbuffer_space(struct intel_batchbuffer *batch) { - return (brw->batch.state_batch_offset - brw->batch.reserved_space) - - USED_BATCH(brw->batch) * 4; + return (batch->state_batch_offset - batch->reserved_space) + - USED_BATCH(*batch) * 4; } static inline void -intel_batchbuffer_emit_dword(struct brw_context *brw, GLuint dword) +intel_batchbuffer_emit_dword(struct intel_batchbuffer *batch, GLuint dword) { #ifdef DEBUG - assert(intel_batchbuffer_space(brw) >= 4); + assert(intel_batchbuffer_space(batch) >= 4); #endif - *brw->batch.map_next++ = dword; - assert(brw->batch.ring != UNKNOWN_RING); + *batch->map_next++ = dword; + assert(batch->ring != UNKNOWN_RING); } static inline void -intel_batchbuffer_emit_float(struct brw_context *brw, float f) +intel_batchbuffer_emit_float(struct intel_batchbuffer *batch, float f) { - intel_batchbuffer_emit_dword(brw, float_as_int(f)); + intel_batchbuffer_emit_dword(batch, float_as_int(f)); } static inline void -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev