Re: [Mesa-dev] [PATCH 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers
I've only had a very quick scan over your patches but I think you need to redo your patches that edit case statements as you are not just changing the value of the intended case statement but also all of the cases above it as they do not break or return they just fall through for example: --- a/src/gallium/drivers/llvmpipe/lp_screen.c +++ b/src/gallium/drivers/llvmpipe/lp_screen.c @@ -220,6 +220,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param) case PIPE_CAP_START_INSTANCE: case PIPE_CAP_TEXTURE_MULTISAMPLE: case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT: + return 64; case PIPE_CAP_CUBE_MAP_ARRAY: return 0; Here you are changing the value of PIPE_CAP_START_INSTANCE, PIPE_CAP_TEXTURE_MULTISAMPLE, PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT and possibly other cases above those from 0 to 64. On Mon, 2013-11-25 at 09:06 +0330, Siavash Eliasi wrote: Hello, this is a series of patches to accomplish *Enable ARB_map_buffer_alignment in all drivers* newbie project suggested by Ian Romanick. *Note* that I don't have write access, please merge them upstream after review process. Best Regards, Siavash Eliasi. Siavash Eliasi (17): Modified allocation routine to use alignment of 64 instead of 16. Modified softpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified allocation routines to use alignment of 64 instead of 16. Modified llvmpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified i915_buffer_create to use memory allocation alignment of 64 instead of 16. Modified i915g to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified svga to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified ilo to return 4096 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified Mesa state tracker to unconditionally enable ARB_map_buffer_alignment. Modified _mesa_init_constants to set ctx-Const.MinMapBufferAlignment to 64. Modified _mesa_buffer_data to use _mesa_align_malloc. Modified radeonBufferData to pass ctx-Const.MinMapBufferAlignment as the alignment value to radeon_bo_open. Modified nouveau_bufferobj_data to pass ctx-Const.MinMapBufferAlignment as the alignment value to nouveau_bo_new. Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc. Modified brw_initialize_context_constants to set ctx-Const.MinMapBufferAlignment to 4096. Modified extensions table to use o(dummy_true) instead of o(ARB_map_buffer_alignment). Deleted gl_extensions::ARB_map_buffer_alignment and all of its use cases. src/gallium/drivers/i915/i915_resource_buffer.c | 2 +- src/gallium/drivers/i915/i915_screen.c | 1 + src/gallium/drivers/ilo/ilo_screen.c| 2 +- src/gallium/drivers/llvmpipe/lp_screen.c| 1 + src/gallium/drivers/llvmpipe/lp_texture.c | 4 ++-- src/gallium/drivers/softpipe/sp_screen.c| 2 +- src/gallium/drivers/softpipe/sp_texture.c | 2 +- src/gallium/drivers/svga/svga_screen.c | 1 + src/mesa/drivers/dri/i915/intel_buffer_objects.c| 4 ++-- src/mesa/drivers/dri/i965/brw_context.c | 2 ++ src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c| 3 ++- src/mesa/drivers/dri/radeon/radeon_buffer_objects.c | 2 +- src/mesa/main/bufferobj.c | 4 ++-- src/mesa/main/context.c | 1 + src/mesa/main/extensions.c | 2 +- src/mesa/main/get.c | 1 - src/mesa/main/get_hash_params.py| 2 +- src/mesa/main/mtypes.h | 1 - src/mesa/state_tracker/st_extensions.c | 4 +--- 19 files changed, 22 insertions(+), 19 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers
Tim's correct -- in this case you probably want to move the PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT case up above the big 'this cap is unsupported' catch-all. -- Chris On Mon, Nov 25, 2013 at 7:07 PM, Timothy Arceri t_arc...@yahoo.com.au wrote: I've only had a very quick scan over your patches but I think you need to redo your patches that edit case statements as you are not just changing the value of the intended case statement but also all of the cases above it as they do not break or return they just fall through for example: --- a/src/gallium/drivers/llvmpipe/lp_screen.c +++ b/src/gallium/drivers/llvmpipe/lp_screen.c @@ -220,6 +220,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param) case PIPE_CAP_START_INSTANCE: case PIPE_CAP_TEXTURE_MULTISAMPLE: case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT: + return 64; case PIPE_CAP_CUBE_MAP_ARRAY: return 0; Here you are changing the value of PIPE_CAP_START_INSTANCE, PIPE_CAP_TEXTURE_MULTISAMPLE, PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT and possibly other cases above those from 0 to 64. On Mon, 2013-11-25 at 09:06 +0330, Siavash Eliasi wrote: Hello, this is a series of patches to accomplish *Enable ARB_map_buffer_alignment in all drivers* newbie project suggested by Ian Romanick. *Note* that I don't have write access, please merge them upstream after review process. Best Regards, Siavash Eliasi. Siavash Eliasi (17): Modified allocation routine to use alignment of 64 instead of 16. Modified softpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified allocation routines to use alignment of 64 instead of 16. Modified llvmpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified i915_buffer_create to use memory allocation alignment of 64 instead of 16. Modified i915g to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified svga to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified ilo to return 4096 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified Mesa state tracker to unconditionally enable ARB_map_buffer_alignment. Modified _mesa_init_constants to set ctx-Const.MinMapBufferAlignment to 64. Modified _mesa_buffer_data to use _mesa_align_malloc. Modified radeonBufferData to pass ctx-Const.MinMapBufferAlignment as the alignment value to radeon_bo_open. Modified nouveau_bufferobj_data to pass ctx-Const.MinMapBufferAlignment as the alignment value to nouveau_bo_new. Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc. Modified brw_initialize_context_constants to set ctx-Const.MinMapBufferAlignment to 4096. Modified extensions table to use o(dummy_true) instead of o(ARB_map_buffer_alignment). Deleted gl_extensions::ARB_map_buffer_alignment and all of its use cases. src/gallium/drivers/i915/i915_resource_buffer.c | 2 +- src/gallium/drivers/i915/i915_screen.c | 1 + src/gallium/drivers/ilo/ilo_screen.c| 2 +- src/gallium/drivers/llvmpipe/lp_screen.c| 1 + src/gallium/drivers/llvmpipe/lp_texture.c | 4 ++-- src/gallium/drivers/softpipe/sp_screen.c| 2 +- src/gallium/drivers/softpipe/sp_texture.c | 2 +- src/gallium/drivers/svga/svga_screen.c | 1 + src/mesa/drivers/dri/i915/intel_buffer_objects.c| 4 ++-- src/mesa/drivers/dri/i965/brw_context.c | 2 ++ src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c| 3 ++- src/mesa/drivers/dri/radeon/radeon_buffer_objects.c | 2 +- src/mesa/main/bufferobj.c | 4 ++-- src/mesa/main/context.c | 1 + src/mesa/main/extensions.c | 2 +- src/mesa/main/get.c | 1 - src/mesa/main/get_hash_params.py| 2 +- src/mesa/main/mtypes.h | 1 - src/mesa/state_tracker/st_extensions.c | 4 +--- 19 files changed, 22 insertions(+), 19 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers
Ah, I see you have a follow-up that fixes this already. Looks good, sorry for the noise. -- Chris On Mon, Nov 25, 2013 at 8:19 PM, Chris Forbes chr...@ijw.co.nz wrote: Tim's correct -- in this case you probably want to move the PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT case up above the big 'this cap is unsupported' catch-all. -- Chris On Mon, Nov 25, 2013 at 7:07 PM, Timothy Arceri t_arc...@yahoo.com.au wrote: I've only had a very quick scan over your patches but I think you need to redo your patches that edit case statements as you are not just changing the value of the intended case statement but also all of the cases above it as they do not break or return they just fall through for example: --- a/src/gallium/drivers/llvmpipe/lp_screen.c +++ b/src/gallium/drivers/llvmpipe/lp_screen.c @@ -220,6 +220,7 @@ llvmpipe_get_param(struct pipe_screen *screen, enum pipe_cap param) case PIPE_CAP_START_INSTANCE: case PIPE_CAP_TEXTURE_MULTISAMPLE: case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT: + return 64; case PIPE_CAP_CUBE_MAP_ARRAY: return 0; Here you are changing the value of PIPE_CAP_START_INSTANCE, PIPE_CAP_TEXTURE_MULTISAMPLE, PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT and possibly other cases above those from 0 to 64. On Mon, 2013-11-25 at 09:06 +0330, Siavash Eliasi wrote: Hello, this is a series of patches to accomplish *Enable ARB_map_buffer_alignment in all drivers* newbie project suggested by Ian Romanick. *Note* that I don't have write access, please merge them upstream after review process. Best Regards, Siavash Eliasi. Siavash Eliasi (17): Modified allocation routine to use alignment of 64 instead of 16. Modified softpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified allocation routines to use alignment of 64 instead of 16. Modified llvmpipe to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified i915_buffer_create to use memory allocation alignment of 64 instead of 16. Modified i915g to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified svga to return 64 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified ilo to return 4096 in case of PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT. Modified Mesa state tracker to unconditionally enable ARB_map_buffer_alignment. Modified _mesa_init_constants to set ctx-Const.MinMapBufferAlignment to 64. Modified _mesa_buffer_data to use _mesa_align_malloc. Modified radeonBufferData to pass ctx-Const.MinMapBufferAlignment as the alignment value to radeon_bo_open. Modified nouveau_bufferobj_data to pass ctx-Const.MinMapBufferAlignment as the alignment value to nouveau_bo_new. Modified i915 intel_bufferobj_data to use _mesa_align_malloc instead of malloc. Modified brw_initialize_context_constants to set ctx-Const.MinMapBufferAlignment to 4096. Modified extensions table to use o(dummy_true) instead of o(ARB_map_buffer_alignment). Deleted gl_extensions::ARB_map_buffer_alignment and all of its use cases. src/gallium/drivers/i915/i915_resource_buffer.c | 2 +- src/gallium/drivers/i915/i915_screen.c | 1 + src/gallium/drivers/ilo/ilo_screen.c| 2 +- src/gallium/drivers/llvmpipe/lp_screen.c| 1 + src/gallium/drivers/llvmpipe/lp_texture.c | 4 ++-- src/gallium/drivers/softpipe/sp_screen.c| 2 +- src/gallium/drivers/softpipe/sp_texture.c | 2 +- src/gallium/drivers/svga/svga_screen.c | 1 + src/mesa/drivers/dri/i915/intel_buffer_objects.c| 4 ++-- src/mesa/drivers/dri/i965/brw_context.c | 2 ++ src/mesa/drivers/dri/nouveau/nouveau_bufferobj.c| 3 ++- src/mesa/drivers/dri/radeon/radeon_buffer_objects.c | 2 +- src/mesa/main/bufferobj.c | 4 ++-- src/mesa/main/context.c | 1 + src/mesa/main/extensions.c | 2 +- src/mesa/main/get.c | 1 - src/mesa/main/get_hash_params.py| 2 +- src/mesa/main/mtypes.h | 1 - src/mesa/state_tracker/st_extensions.c | 4 +--- 19 files changed, 22 insertions(+), 19 deletions(-) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers
On 11/25/2013 10:50 AM, Chris Forbes wrote: Ah, I see you have a follow-up that fixes this already. Looks good, sorry for the noise. -- Chris I'm pretty new here and I'm not used to mailing lists, sorry if patches aren't showing up in correct place. Should I send all of patches again (and possibly squash a few) or is that fine by moderators? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers
It would be good to fold the fixes into the patches they belong in. I'd wait until you collect review comments on the whole series, then incorporate all the review you get and send out a new version at that point. -- Chris On Mon, Nov 25, 2013 at 8:29 PM, Siavash Eliasi siavashser...@gmail.com wrote: On 11/25/2013 10:50 AM, Chris Forbes wrote: Ah, I see you have a follow-up that fixes this already. Looks good, sorry for the noise. -- Chris I'm pretty new here and I'm not used to mailing lists, sorry if patches aren't showing up in correct place. Should I send all of patches again (and possibly squash a few) or is that fine by moderators? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
