Re: [Mesa-dev] [PATCH] intel/compiler: Set flag reg/subreg number properly
Thanks for reviewing the patch. On 4/8/19 10:34 AM, Anuj Phogat wrote: > On Wed, Mar 27, 2019 at 4:05 PM Sagar Ghuge wrote: >> >> If predicate control is set to None, then in that case we can simply set >> flag reg/subreg number to zero. This allows round-tripping through the >> assembler/disassembler >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_eu_emit.c| 7 +++ >> src/intel/compiler/brw_fs_generator.cpp | 1 + >> 2 files changed, 8 insertions(+) >> >> diff --git a/src/intel/compiler/brw_eu_emit.c >> b/src/intel/compiler/brw_eu_emit.c >> index 94e247e1a39..f59543db8df 100644 >> --- a/src/intel/compiler/brw_eu_emit.c >> +++ b/src/intel/compiler/brw_eu_emit.c >> @@ -2267,6 +2267,13 @@ brw_fb_WRITE(struct brw_codegen *p, >> brw_inst_set_sfid(devinfo, insn, target_cache); >> brw_inst_set_compression(devinfo, insn, false); >> >> + if (brw_inst_pred_control(devinfo, insn) == BRW_PREDICATE_NONE) { >> + brw_inst_set_flag_subreg_nr(devinfo, insn, 0); >> + if (devinfo->gen >= 7) { >> + brw_inst_set_flag_reg_nr(devinfo, insn, 0); >> + } >> + } >> + >> if (devinfo->gen >= 6) { >>/* headerless version, just submit color payload */ >>src0 = payload; >> diff --git a/src/intel/compiler/brw_fs_generator.cpp >> b/src/intel/compiler/brw_fs_generator.cpp >> index c24d4eb7cab..242450c605e 100644 >> --- a/src/intel/compiler/brw_fs_generator.cpp >> +++ b/src/intel/compiler/brw_fs_generator.cpp >> @@ -307,6 +307,7 @@ fs_generator::fire_fb_write(fs_inst *inst, >>brw_set_default_mask_control(p, BRW_MASK_DISABLE); >>brw_set_default_predicate_control(p, BRW_PREDICATE_NONE); >>brw_set_default_compression_control(p, BRW_COMPRESSION_NONE); >> + brw_set_default_flag_reg(p, 0, 0); >>brw_MOV(p, offset(retype(payload, BRW_REGISTER_TYPE_UD), 1), >>offset(retype(implied_header, BRW_REGISTER_TYPE_UD), 1)); >>brw_pop_insn_state(p); >> -- >> 2.20.1 >> >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > Change looks correct and harmless. > Reviewed-by: Anuj Phogat > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/disasm: Disassemble JIP offset for while
Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index efca3e2ce7d..440e51faa61 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1661,7 +1661,8 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, format(file, "Pop: %"PRIu64, brw_inst_gen4_pop_count(devinfo, inst)); } else if (devinfo->gen < 6 && (opcode == BRW_OPCODE_IF || opcode == BRW_OPCODE_IFF || - opcode == BRW_OPCODE_HALT)) { + opcode == BRW_OPCODE_HALT || + opcode == BRW_OPCODE_WHILE)) { pad(file, 16); format(file, "Jump: %d", brw_inst_gen4_jump_count(devinfo, inst)); } else if (devinfo->gen < 6 && opcode == BRW_OPCODE_ENDIF) { -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Set flag reg/subreg number properly
If predicate control is set to None, then in that case we can simply set flag reg/subreg number to zero. This allows round-tripping through the assembler/disassembler Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu_emit.c| 7 +++ src/intel/compiler/brw_fs_generator.cpp | 1 + 2 files changed, 8 insertions(+) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 94e247e1a39..f59543db8df 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -2267,6 +2267,13 @@ brw_fb_WRITE(struct brw_codegen *p, brw_inst_set_sfid(devinfo, insn, target_cache); brw_inst_set_compression(devinfo, insn, false); + if (brw_inst_pred_control(devinfo, insn) == BRW_PREDICATE_NONE) { + brw_inst_set_flag_subreg_nr(devinfo, insn, 0); + if (devinfo->gen >= 7) { + brw_inst_set_flag_reg_nr(devinfo, insn, 0); + } + } + if (devinfo->gen >= 6) { /* headerless version, just submit color payload */ src0 = payload; diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp index c24d4eb7cab..242450c605e 100644 --- a/src/intel/compiler/brw_fs_generator.cpp +++ b/src/intel/compiler/brw_fs_generator.cpp @@ -307,6 +307,7 @@ fs_generator::fire_fb_write(fs_inst *inst, brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_predicate_control(p, BRW_PREDICATE_NONE); brw_set_default_compression_control(p, BRW_COMPRESSION_NONE); + brw_set_default_flag_reg(p, 0, 0); brw_MOV(p, offset(retype(payload, BRW_REGISTER_TYPE_UD), 1), offset(retype(implied_header, BRW_REGISTER_TYPE_UD), 1)); brw_pop_insn_state(p); -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Replicate 16 bit immediate value correctly
Thank you for reviewing the patch. On 3/26/19 4:46 PM, Matt Turner wrote: > On Tue, Mar 26, 2019 at 3:35 PM Sagar Ghuge wrote: >> >> For the W or UW (signed or unsigned word) source types, the 16-bit value >> must be replicated in both the low and high words of the 32-bit >> immediate value. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_fs.cpp | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp >> index 0c2439d9daf..f8cb91fcf21 100644 >> --- a/src/intel/compiler/brw_fs.cpp >> +++ b/src/intel/compiler/brw_fs.cpp >> @@ -4069,6 +4069,9 @@ fs_visitor::lower_integer_multiplication() >> mul->src[1].type = BRW_REGISTER_TYPE_UW; >> mul->src[1].stride *= 2; >> >> +if (mul->src[1].file == IMM) >> + mul->src[1].ud = ((mul->src[1].ud & 0x) | >> + mul->src[1].ud << 16); > > Please put braces around the statement, since it's in nested control flow. > I will fix this :) > Reviewed-by: Matt Turner > ___ > 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] intel/compiler: Replicate 16 bit immediate value correctly
For the W or UW (signed or unsigned word) source types, the 16-bit value must be replicated in both the low and high words of the 32-bit immediate value. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_fs.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp index 0c2439d9daf..f8cb91fcf21 100644 --- a/src/intel/compiler/brw_fs.cpp +++ b/src/intel/compiler/brw_fs.cpp @@ -4069,6 +4069,9 @@ fs_visitor::lower_integer_multiplication() mul->src[1].type = BRW_REGISTER_TYPE_UW; mul->src[1].stride *= 2; +if (mul->src[1].file == IMM) + mul->src[1].ud = ((mul->src[1].ud & 0x) | + mul->src[1].ud << 16); } else if (devinfo->gen == 7 && !devinfo->is_haswell && inst->group > 0) { /* Among other things the quarter control bits influence which -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Print quad value in hex format
From: Sagar Ghuge Print quad value same as unsigned quad so that we can distinguish in between quater control disassembled values for e.g 1/2/3[Q] and immediate quad value for e.g 1Q. This allows round-tripping through the assembler/disassembler. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index efca3e2ce7d..04efa965cef 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1309,7 +1309,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type, format(file, "0x%016"PRIx64"UQ", brw_inst_imm_uq(devinfo, inst)); break; case BRW_REGISTER_TYPE_Q: - format(file, "%"PRId64"Q", brw_inst_imm_uq(devinfo, inst)); + format(file, "0x%016"PRIx64"Q", brw_inst_imm_uq(devinfo, inst)); break; case BRW_REGISTER_TYPE_UD: format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Print quad value in hex format
Please ignore this patch. Somehow my emails are getting delayed. This is just a duplicate of the other patch which is on mailing list. On 3/24/19 10:19 AM, Sagar Ghuge wrote: > From: Sagar Ghuge > > Print quad value same as unsigned quad so that we can distinguish in > between quater control disassembled values for e.g 1/2/3[Q] and > immediate quad value for e.g 1Q. This allows round-tripping through the > assembler/disassembler. > > Signed-off-by: Sagar Ghuge > --- > src/intel/compiler/brw_disasm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c > index efca3e2ce7d..04efa965cef 100644 > --- a/src/intel/compiler/brw_disasm.c > +++ b/src/intel/compiler/brw_disasm.c > @@ -1309,7 +1309,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, > enum brw_reg_type type, >format(file, "0x%016"PRIx64"UQ", brw_inst_imm_uq(devinfo, inst)); >break; > case BRW_REGISTER_TYPE_Q: > - format(file, "%"PRId64"Q", brw_inst_imm_uq(devinfo, inst)); > + format(file, "0x%016"PRIx64"Q", brw_inst_imm_uq(devinfo, inst)); >break; > case BRW_REGISTER_TYPE_UD: >format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Print quad value in hex format
Print quad value same as unsigned quad so that we can distinguish in between quater control disassembled values for e.g 1/2/3[Q] and immediate quad value for e.g 1Q. This allows round-tripping through the assembler/disassembler. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index efca3e2ce7d..04efa965cef 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1309,7 +1309,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type, format(file, "0x%016"PRIx64"UQ", brw_inst_imm_uq(devinfo, inst)); break; case BRW_REGISTER_TYPE_Q: - format(file, "%"PRId64"Q", brw_inst_imm_uq(devinfo, inst)); + format(file, "0x%016"PRIx64"Q", brw_inst_imm_uq(devinfo, inst)); break; case BRW_REGISTER_TYPE_UD: format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); -- 2.20.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 9/9] anv: Enabled the VK_EXT_sample_locations extension
Reviewed-by: Sagar Ghuge On 3/12/19 3:34 AM, Eleni Maria Stea wrote: > Enabled the VK_EXT_sample_locations for Intel Gen >= 7. > > v2: Replaced device.info->gen >= 7 with True, as Anv doesn't support > anything below Gen7. (Lionel Landwerlin) > --- > src/intel/vulkan/anv_extensions.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/vulkan/anv_extensions.py > b/src/intel/vulkan/anv_extensions.py > index 9e4e03e46df..5a30c733c5c 100644 > --- a/src/intel/vulkan/anv_extensions.py > +++ b/src/intel/vulkan/anv_extensions.py > @@ -129,7 +129,7 @@ EXTENSIONS = [ > Extension('VK_EXT_inline_uniform_block', 1, True), > Extension('VK_EXT_pci_bus_info', 2, True), > Extension('VK_EXT_post_depth_coverage', 1, > 'device->info.gen >= 9'), > -Extension('VK_EXT_sample_locations', 1, False), > +Extension('VK_EXT_sample_locations', 1, True), > Extension('VK_EXT_sampler_filter_minmax', 1, > 'device->info.gen >= 9'), > Extension('VK_EXT_scalar_block_layout', 1, True), > Extension('VK_EXT_shader_stencil_export', 1, > 'device->info.gen >= 9'), > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/9] anv: Added the VK_EXT_sample_locations extension to the anv_extensions list
Reviewed-by: Sagar Ghuge On 3/12/19 3:34 AM, Eleni Maria Stea wrote: > Added the VK_EXT_sample_locations to the anv_extensions.py list to > generate the related entrypoints. > --- > src/intel/vulkan/anv_extensions.py | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/intel/vulkan/anv_extensions.py > b/src/intel/vulkan/anv_extensions.py > index 6fff293dee4..9e4e03e46df 100644 > --- a/src/intel/vulkan/anv_extensions.py > +++ b/src/intel/vulkan/anv_extensions.py > @@ -129,6 +129,7 @@ EXTENSIONS = [ > Extension('VK_EXT_inline_uniform_block', 1, True), > Extension('VK_EXT_pci_bus_info', 2, True), > Extension('VK_EXT_post_depth_coverage', 1, > 'device->info.gen >= 9'), > +Extension('VK_EXT_sample_locations', 1, False), > Extension('VK_EXT_sampler_filter_minmax', 1, > 'device->info.gen >= 9'), > Extension('VK_EXT_scalar_block_layout', 1, True), > Extension('VK_EXT_shader_stencil_export', 1, > 'device->info.gen >= 9'), > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: Ignore VkRenderPassInputAttachementAspectCreateInfo
Reviewed-by: Sagar Ghuge On 3/12/19 4:21 PM, Jason Ekstrand wrote: > We don't care about the information but there's no sense in throwing a > debug warning about it. It's harmless but annoying to users. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109984 > Cc: Craig Stout > --- > src/intel/vulkan/anv_pass.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c > index 02f2be60e02..5fac5bbb31c 100644 > --- a/src/intel/vulkan/anv_pass.c > +++ b/src/intel/vulkan/anv_pass.c > @@ -332,6 +332,10 @@ VkResult anv_CreateRenderPass( > > vk_foreach_struct(ext, pCreateInfo->pNext) { >switch (ext->sType) { > + case VK_STRUCTURE_TYPE_RENDER_PASS_INPUT_ATTACHMENT_ASPECT_CREATE_INFO: > + /* We don't care about this information */ > + break; > + >case VK_STRUCTURE_TYPE_RENDER_PASS_MULTIVIEW_CREATE_INFO: { > VkRenderPassMultiviewCreateInfo *mv = (void *)ext; > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 8/9] anv: Removed unused header file
Reviewed-by: Sagar Ghuge On 3/12/19 3:34 AM, Eleni Maria Stea wrote: > In src/intel/vulkan/genX_blorp_exec.c we included the file: > common/gen_sample_positions.h but not use it. Removed. > --- > src/intel/vulkan/genX_blorp_exec.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/intel/vulkan/genX_blorp_exec.c > b/src/intel/vulkan/genX_blorp_exec.c > index e9c85d56d5f..0eeefaaa9d6 100644 > --- a/src/intel/vulkan/genX_blorp_exec.c > +++ b/src/intel/vulkan/genX_blorp_exec.c > @@ -31,7 +31,6 @@ > #undef __gen_combine_address > > #include "common/gen_l3_config.h" > -#include "common/gen_sample_positions.h" > #include "blorp/blorp_genX_exec.h" > > static void * > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/9] anv: Set the values for the VkPhysicalDeviceSampleLocationsPropertiesEXT
On Mon, 2019-03-11 at 17:04 +0200, Eleni Maria Stea wrote: > The VkPhysicalDeviceSampleLocationPropertiesEXT struct is filled with > implementation dependent values and according to the table from the > Vulkan Specification section [36.1. Limit Requirements]: > > pname | max | min > pname:sampleLocationSampleCounts |-|ename:VK_SAMPLE_COU > NT_4_BIT > pname:maxSampleLocationGridSize|-|(1, 1) > pname:sampleLocationCoordinateRange|(0.0, 0.9375)|(0.0, 0.9375) > pname:sampleLocationSubPixelBits |-|4 > pname:variableSampleLocations | false |implementation > dependent > > The hardware only supports setting the same sample location for all > the > pixels, so we only support 1x1 grids. > > Also, variableSampleLocations is set to false because we don't > support the > feature. > --- > src/intel/vulkan/anv_device.c | 21 + > src/intel/vulkan/anv_private.h | 3 +++ > 2 files changed, 24 insertions(+) > > diff --git a/src/intel/vulkan/anv_device.c > b/src/intel/vulkan/anv_device.c > index 729cceb3e32..1e183b7f4ad 100644 > --- a/src/intel/vulkan/anv_device.c > +++ b/src/intel/vulkan/anv_device.c > @@ -1401,6 +1401,27 @@ void anv_GetPhysicalDeviceProperties2( > break; >} > > + case > VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLE_LOCATIONS_PROPERTIES_EXT: { > + VkPhysicalDeviceSampleLocationsPropertiesEXT *props = > +(VkPhysicalDeviceSampleLocationsPropertiesEXT *)ext; > + props->sampleLocationSampleCounts = ISL_SAMPLE_COUNT_2_BIT > | > + ISL_SAMPLE_COUNT_4_BIT > | > + ISL_SAMPLE_COUNT_8_BIT; > + if (pdevice->info.gen >= 9) > +props->sampleLocationSampleCounts |= > ISL_SAMPLE_COUNT_16_BIT; Hi Eleni, Thanks for the series. "isl_device_get_sample_counts" method figure out values according to platform so maybe we can make use of it and ignore ISL_SAMPLE_COUNT_1_BIT. So that we don't have to take care of values according to platform here. I am not sure about this, so it might be a good idea to consult with Jason/Lionel once. :) > + > + props->maxSampleLocationGridSize.width = SAMPLE_LOC_GRID_W; > + props->maxSampleLocationGridSize.height = > SAMPLE_LOC_GRID_H; > + > + props->sampleLocationCoordinateRange[0] = 0; > + props->sampleLocationCoordinateRange[1] = 0.9375; > + props->sampleLocationSubPixelBits = 4; > + > + props->variableSampleLocations = false; Just for consistency, doesn't make any difference but can we use VK_FALSE instead of false. with or without the fix, this patch is: Reviewed-by: Sagar Ghuge > + > + break; > + } > + >default: > anv_debug_ignored_stype(ext->sType); > break; > diff --git a/src/intel/vulkan/anv_private.h > b/src/intel/vulkan/anv_private.h > index eed282ff985..5905299e59d 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -195,6 +195,9 @@ struct gen_l3_config; > > #define anv_printflike(a, b) __attribute__((__format__(__printf__, > a, b))) > > +#define SAMPLE_LOC_GRID_W 1 > +#define SAMPLE_LOC_GRID_H 1 > + > static inline uint32_t > align_down_npot_u32(uint32_t v, uint32_t a) > { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/9] anv: Set the values for the VkPhysicalDeviceSampleLocationsPropertiesEXT
On Mon, 2019-03-11 at 17:04 +0200, Eleni Maria Stea wrote: > The VkPhysicalDeviceSampleLocationPropertiesEXT struct is filled with > implementation dependent values and according to the table from the > Vulkan Specification section [36.1. Limit Requirements]: > > pname | max | min > pname:sampleLocationSampleCounts |-|ename:VK_SAMPLE_COU > NT_4_BIT > pname:maxSampleLocationGridSize|-|(1, 1) > pname:sampleLocationCoordinateRange|(0.0, 0.9375)|(0.0, 0.9375) > pname:sampleLocationSubPixelBits |-|4 > pname:variableSampleLocations | false |implementation > dependent > > The hardware only supports setting the same sample location for all > the > pixels, so we only support 1x1 grids. > > Also, variableSampleLocations is set to false because we don't > support the > feature. > --- > src/intel/vulkan/anv_device.c | 21 + > src/intel/vulkan/anv_private.h | 3 +++ > 2 files changed, 24 insertions(+) > > diff --git a/src/intel/vulkan/anv_device.c > b/src/intel/vulkan/anv_device.c > index 729cceb3e32..1e183b7f4ad 100644 > --- a/src/intel/vulkan/anv_device.c > +++ b/src/intel/vulkan/anv_device.c > @@ -1401,6 +1401,27 @@ void anv_GetPhysicalDeviceProperties2( > break; >} > > + case > VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SAMPLE_LOCATIONS_PROPERTIES_EXT: { > + VkPhysicalDeviceSampleLocationsPropertiesEXT *props = > +(VkPhysicalDeviceSampleLocationsPropertiesEXT *)ext; > + props->sampleLocationSampleCounts = ISL_SAMPLE_COUNT_2_BIT > | > + ISL_SAMPLE_COUNT_4_BIT > | > + ISL_SAMPLE_COUNT_8_BIT; > + if (pdevice->info.gen >= 9) > +props->sampleLocationSampleCounts |= > ISL_SAMPLE_COUNT_16_BIT; Hi Eleni, Thanks for the series. "isl_device_get_sample_counts" method figure out values according to platform so maybe we can make use of it and ignore ISL_SAMPLE_COUNT_1_BIT. So that we don't have to take care of values according to platform here. I am not sure about this, so it might be a good idea to consult with Jason/Lionel once. :) > + > + props->maxSampleLocationGridSize.width = SAMPLE_LOC_GRID_W; > + props->maxSampleLocationGridSize.height = > SAMPLE_LOC_GRID_H; > + > + props->sampleLocationCoordinateRange[0] = 0; > + props->sampleLocationCoordinateRange[1] = 0.9375; > + props->sampleLocationSubPixelBits = 4; > + > + props->variableSampleLocations = false; Just for consistency, doesn't make any difference but can we use VK_FALSE instead of false. with or without the fix, this patch is: Reviewed-by: Sagar Ghuge > + > + break; > + } > + >default: > anv_debug_ignored_stype(ext->sType); > break; > diff --git a/src/intel/vulkan/anv_private.h > b/src/intel/vulkan/anv_private.h > index eed282ff985..5905299e59d 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -195,6 +195,9 @@ struct gen_l3_config; > > #define anv_printflike(a, b) __attribute__((__format__(__printf__, > a, b))) > > +#define SAMPLE_LOC_GRID_W 1 > +#define SAMPLE_LOC_GRID_H 1 > + > static inline uint32_t > align_down_npot_u32(uint32_t v, uint32_t a) > { ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] intel/compiler: Always print flag subregister number
While disassembling the predicate always print flag subregister number to keep grammar same across the generation for assembler tool. v2: Club consecutive format calls (Matt Turner) Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 6c79fec0a1..3778d8d996 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1485,9 +1485,9 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, string(file, "("); err |= control(file, "predicate inverse", pred_inv, brw_inst_pred_inv(devinfo, inst), NULL); - format(file, "f%"PRIu64, devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0); - if (brw_inst_flag_subreg_nr(devinfo, inst)) - format(file, ".%"PRIu64, brw_inst_flag_subreg_nr(devinfo, inst)); + format(file, "f%"PRIu64".%"PRIu64, + devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0, + brw_inst_flag_subreg_nr(devinfo, inst)); if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) { err |= control(file, "predicate control align1", pred_ctrl_align1, brw_inst_pred_control(devinfo, inst), NULL); @@ -1522,10 +1522,9 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, opcode != BRW_OPCODE_CSEL && opcode != BRW_OPCODE_IF && opcode != BRW_OPCODE_WHILE))) { - format(file, ".f%"PRIu64, -devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0); - if (brw_inst_flag_subreg_nr(devinfo, inst)) -format(file, ".%"PRIu64, brw_inst_flag_subreg_nr(devinfo, inst)); + format(file, ".f%"PRIu64".%"PRIu64, +devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0, +brw_inst_flag_subreg_nr(devinfo, inst)); } } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Set swizzle to BRW_SWIZZLE_XXXX for scalar region
When RepCtrl is set, the swizzle field is ignored by the hardware. In order to ensure a 1-to-1 correspondence between the human-readable disassembly and the binary instruction encoding always set the swizzle to (all zeros) when it is unused due to RepCtrl Signed-off-by: Sagar Ghuge Reviewed-by: Matt Turner --- src/intel/compiler/brw_eu_emit.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 4630b83b1a..483037345e 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -833,7 +833,15 @@ brw_inst *brw_##OP(struct brw_codegen *p, \ struct brw_reg src0, \ struct brw_reg src1, \ struct brw_reg src2) \ -{ \ +{ \ + if (p->current->access_mode == BRW_ALIGN_16) { \ + if (src0.vstride == BRW_VERTICAL_STRIDE_0)\ + src0.swizzle = BRW_SWIZZLE_; \ + if (src1.vstride == BRW_VERTICAL_STRIDE_0)\ + src1.swizzle = BRW_SWIZZLE_; \ + if (src2.vstride == BRW_VERTICAL_STRIDE_0)\ + src2.swizzle = BRW_SWIZZLE_; \ + }\ return brw_alu3(p, BRW_OPCODE_##OP, dest, src0, src1, src2);\ } @@ -854,6 +862,15 @@ brw_inst *brw_##OP(struct brw_codegen *p, \ assert(src0.type == BRW_REGISTER_TYPE_DF);\ assert(src1.type == BRW_REGISTER_TYPE_DF);\ assert(src2.type == BRW_REGISTER_TYPE_DF);\ + }\ +\ + if (p->current->access_mode == BRW_ALIGN_16) { \ + if (src0.vstride == BRW_VERTICAL_STRIDE_0)\ + src0.swizzle = BRW_SWIZZLE_; \ + if (src1.vstride == BRW_VERTICAL_STRIDE_0)\ + src1.swizzle = BRW_SWIZZLE_; \ + if (src2.vstride == BRW_VERTICAL_STRIDE_0)\ + src2.swizzle = BRW_SWIZZLE_; \ }\ return brw_alu3(p, BRW_OPCODE_##OP, dest, src0, src1, src2); \ } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Always print flag subregister number
While disassembling the predicate always print flag subregister number to keep grammar same across the generation for assembler tool. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 6c79fec0a1..a218e1451c 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1486,8 +1486,7 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, err |= control(file, "predicate inverse", pred_inv, brw_inst_pred_inv(devinfo, inst), NULL); format(file, "f%"PRIu64, devinfo->gen >= 7 ? brw_inst_flag_reg_nr(devinfo, inst) : 0); - if (brw_inst_flag_subreg_nr(devinfo, inst)) - format(file, ".%"PRIu64, brw_inst_flag_subreg_nr(devinfo, inst)); + format(file, ".%"PRIu64, brw_inst_flag_subreg_nr(devinfo, inst)); if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) { err |= control(file, "predicate control align1", pred_ctrl_align1, brw_inst_pred_control(devinfo, inst), NULL); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Disassemble GEN6_SFID_DATAPORT_SAMPLER_CACHE as dp_sampler
Both BRW_SFID_SAMPLER and GEN6_SFID_DATAPORT_SAMPLER_CACHE are getting disassembled as "sampler", which is misleading for assembler tool. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index cfccdea3b8..6c79fec0a1 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -289,7 +289,7 @@ static const char *const gen6_sfid[16] = { [BRW_SFID_MESSAGE_GATEWAY] = "gateway", [BRW_SFID_URB] = "urb", [BRW_SFID_THREAD_SPAWNER] = "thread_spawner", - [GEN6_SFID_DATAPORT_SAMPLER_CACHE] = "sampler", + [GEN6_SFID_DATAPORT_SAMPLER_CACHE] = "dp_sampler", [GEN6_SFID_DATAPORT_RENDER_CACHE] = "render", [GEN6_SFID_DATAPORT_CONSTANT_CACHE] = "const", [GEN7_SFID_DATAPORT_DATA_CACHE] = "data", -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/fs: Prevent emission of IR instructions not aligned to their own execution size.
Tested-by: Sagar Ghuge On 11/8/18 5:26 PM, Francisco Jerez wrote: > This can occur during payload setup of SIMD-split send message > instructions, which can lead to the emission of header setup > instructions with a non-zero channel group and fixed SIMD width. Such > instructions could end up using undefined channel enable signals > except they don't care since they're always marked force_writemask_all. > > Not known to affect correctness of any workload at this point, but it > would be trivial to back-port to stable if something comes up. > > Reported-by: Sagar Ghuge > --- > src/intel/compiler/brw_fs_builder.h | 20 +--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/src/intel/compiler/brw_fs_builder.h > b/src/intel/compiler/brw_fs_builder.h > index 0cafaf50e56..4846820722c 100644 > --- a/src/intel/compiler/brw_fs_builder.h > +++ b/src/intel/compiler/brw_fs_builder.h > @@ -114,11 +114,25 @@ namespace brw { >fs_builder >group(unsigned n, unsigned i) const >{ > - assert(force_writemask_all || > -(n <= dispatch_width() && i < dispatch_width() / n)); > fs_builder bld = *this; > + > + if (n <= dispatch_width() && i < dispatch_width() / n) { > +bld._group += i * n; > + } else { > +/* The requested channel group isn't a subset of the channel > group > + * of this builder, which means that the resulting instructions > + * would use (potentially undefined) channel enable signals not > + * specified by the parent builder. That's only valid if the > + * instruction doesn't have per-channel semantics, in which case > + * we should clear off the default group index in order to > prevent > + * emitting instructions with channel group not aligned to their > + * own execution size. > + */ > +assert(force_writemask_all); > +bld._group = 0; > + } > + > bld._dispatch_width = n; > - bld._group += i * n; > return bld; >} > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Print message descriptor as immediate source
Hi, Thank you for sharing the following patch series. On 10/26/18 6:15 AM, Eero Tamminen wrote: > Hi, > > Toni had a working assembler in early 2017, but he had to stop working > on it because nobody reviewed the patches needed for the disassembler. > > Maybe his patch series for the disassembly output still has something > useful for your assembler: > Yes, it is. > * i965 shader disassembly label support: > https://patchwork.freedesktop.org/series/19946/ > * i965 shader disassembly formatting changes: > https://patchwork.freedesktop.org/series/19945/ > * Always print message descriptor and SFID for SEND instructions: > https://patchwork.freedesktop.org/patch/223750/ > > ? > > - Eero > > On 10/25/18 2:25 AM, Sagar Ghuge wrote: >> While disassembling send(c) instruction print message descriptor as >> immediate source operand along with message descriptor. This allows >> assembler to read immediate source operand and set bits accordingly. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_disasm.c | 9 +++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/src/intel/compiler/brw_disasm.c >> b/src/intel/compiler/brw_disasm.c >> index 6a7e988641..9c6df9e645 100644 >> --- a/src/intel/compiler/brw_disasm.c >> +++ b/src/intel/compiler/brw_disasm.c >> @@ -1606,7 +1606,12 @@ brw_disassemble_inst(FILE *file, const struct >> gen_device_info *devinfo, >> /* show the indirect descriptor source */ >> pad(file, 48); >> err |= src1(file, devinfo, inst); >> - } >> + pad(file, 64); >> + } else >> + pad(file, 48); >> + >> + /* Print message descriptor as immediate source */ >> + fprintf(file, "0x%08"PRIx64, inst->data[1] >> 32); >> newline(file); >> pad(file, 16); >> @@ -1615,7 +1620,7 @@ brw_disassemble_inst(FILE *file, const struct >> gen_device_info *devinfo, >> fprintf(file, " "); >> err |= control(file, "SFID", devinfo->gen >= 6 ? gen6_sfid : >> gen4_sfid, >> sfid, ); >> - >> + string(file, " MsgDesc:"); >> if (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE) { >> format(file, " indirect"); >> > > ___ > 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] intel/compiler: Print message descriptor as immediate source
On 10/25/18 12:55 AM, Samuel Iglesias Gonsálvez wrote: > On Thursday, 25 October 2018 1:25:53 (CEST) Sagar Ghuge wrote: >> While disassembling send(c) instruction print message descriptor as >> immediate source operand along with message descriptor. This allows >> assembler to read immediate source operand and set bits accordingly. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_disasm.c | 9 +++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/src/intel/compiler/brw_disasm.c >> b/src/intel/compiler/brw_disasm.c index 6a7e988641..9c6df9e645 100644 >> --- a/src/intel/compiler/brw_disasm.c >> +++ b/src/intel/compiler/brw_disasm.c >> @@ -1606,7 +1606,12 @@ brw_disassemble_inst(FILE *file, const struct >> gen_device_info *devinfo, /* show the indirect descriptor source */ >> pad(file, 48); >> err |= src1(file, devinfo, inst); >> - } >> + pad(file, 64); >> + } else >> + pad(file, 48); >> + > > IIRC, we have a coding style rules that is: when the if's body has several > statements like here, then the else body should be between braces even when > it > is one line. That thing increases readability. > Thanks for pointing out. > I have not found it written in the coding style guidelines [0], but I have > seen it in a lot of places of the code. > > I can do this change for you before pushing it tomorrow. Sounds good? > Yep, sounds great. Thank you. > With that fixed, > > Reviewed-by: Samuel Iglesias Gonsálvez > > Sam > > [0] https://www.mesa3d.org/codingstyle.html > >> + /* Print message descriptor as immediate source */ >> + fprintf(file, "0x%08"PRIx64, inst->data[1] >> 32); >> >>newline(file); >>pad(file, 16); >> @@ -1615,7 +1620,7 @@ brw_disassemble_inst(FILE *file, const struct >> gen_device_info *devinfo, fprintf(file, ""); >>err |= control(file, "SFID", devinfo->gen >= 6 ? gen6_sfid : >> gen4_sfid, sfid, ); >> - >> + string(file, " MsgDesc:"); >> >>if (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE) { >> format(file, " indirect"); > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Print hex representation along with floating point value
Thank you for reviewing the patch. On 10/25/18 12:40 AM, Samuel Iglesias Gonsálvez wrote: > Reviewed-by: Samuel Iglesias Gonsálvez > > Do you need somebody to push it to the repo? I can do it tomorrow. > Yes, I don't have commit access. I really appreciate it. > Sam > > On Wednesday, 24 October 2018 22:27:27 (CEST) Sagar Ghuge wrote: >> While encoding the immediate floating point values in instruction we use >> values upto precision 9, but while disassembling, we print precision to >> 6 places, which round up the value and gives wrong interpretation for >> encoded immediate constant. >> >> To avoid misinterpretation of encoded immediate values in instruction >> and disassembled output, print hex representation along with floating >> point value which can be used by assembler in future. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_disasm.c | 12 +--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/src/intel/compiler/brw_disasm.c >> b/src/intel/compiler/brw_disasm.c index 322f4544df..6a7e988641 100644 >> --- a/src/intel/compiler/brw_disasm.c >> +++ b/src/intel/compiler/brw_disasm.c >> @@ -1283,7 +1283,9 @@ imm(FILE *file, const struct gen_device_info *devinfo, >> enum brw_reg_type type, format(file, "0x%08xUV", brw_inst_imm_ud(devinfo, >> inst)); >>break; >> case BRW_REGISTER_TYPE_VF: >> - format(file, "[%-gF, %-gF, %-gF, %-gF]VF", >> + format(file, "0x%"PRIx64"VF", brw_inst_bits(inst, 127, 96)); >> + pad(file, 48); >> + format(file, "/* [%-gF, %-gF, %-gF, %-gF]VF */", >> brw_vf_to_float(brw_inst_imm_ud(devinfo, inst)), >> brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 8), >> brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 16), >> @@ -1293,10 +1295,14 @@ imm(FILE *file, const struct gen_device_info >> *devinfo, enum brw_reg_type type, format(file, "0x%08xV", >> brw_inst_imm_ud(devinfo, inst)); >>break; >> case BRW_REGISTER_TYPE_F: >> - format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); >> + format(file, "0x%"PRIx64"F", brw_inst_bits(inst, 127, 96)); >> + pad(file, 48); >> + format(file, " /* %-gF */", brw_inst_imm_f(devinfo, inst)); >>break; >> case BRW_REGISTER_TYPE_DF: >> - format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); >> + format(file, "0x%016"PRIx64"DF", brw_inst_bits(inst, 127, 64)); >> + pad(file, 48); >> + format(file, "/* %-gDF */", brw_inst_imm_df(devinfo, inst)); >>break; >> case BRW_REGISTER_TYPE_HF: >>string(file, "Half Float IMM"); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Print message descriptor as immediate source
While disassembling send(c) instruction print message descriptor as immediate source operand along with message descriptor. This allows assembler to read immediate source operand and set bits accordingly. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 6a7e988641..9c6df9e645 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1606,7 +1606,12 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, /* show the indirect descriptor source */ pad(file, 48); err |= src1(file, devinfo, inst); - } + pad(file, 64); + } else + pad(file, 48); + + /* Print message descriptor as immediate source */ + fprintf(file, "0x%08"PRIx64, inst->data[1] >> 32); newline(file); pad(file, 16); @@ -1615,7 +1620,7 @@ brw_disassemble_inst(FILE *file, const struct gen_device_info *devinfo, fprintf(file, ""); err |= control(file, "SFID", devinfo->gen >= 6 ? gen6_sfid : gen4_sfid, sfid, ); - + string(file, " MsgDesc:"); if (brw_inst_src1_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE) { format(file, " indirect"); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Print hex representation along with floating point value
While encoding the immediate floating point values in instruction we use values upto precision 9, but while disassembling, we print precision to 6 places, which round up the value and gives wrong interpretation for encoded immediate constant. To avoid misinterpretation of encoded immediate values in instruction and disassembled output, print hex representation along with floating point value which can be used by assembler in future. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_disasm.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 322f4544df..6a7e988641 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1283,7 +1283,9 @@ imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type, format(file, "0x%08xUV", brw_inst_imm_ud(devinfo, inst)); break; case BRW_REGISTER_TYPE_VF: - format(file, "[%-gF, %-gF, %-gF, %-gF]VF", + format(file, "0x%"PRIx64"VF", brw_inst_bits(inst, 127, 96)); + pad(file, 48); + format(file, "/* [%-gF, %-gF, %-gF, %-gF]VF */", brw_vf_to_float(brw_inst_imm_ud(devinfo, inst)), brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 8), brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 16), @@ -1293,10 +1295,14 @@ imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type, format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); break; case BRW_REGISTER_TYPE_F: - format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); + format(file, "0x%"PRIx64"F", brw_inst_bits(inst, 127, 96)); + pad(file, 48); + format(file, " /* %-gF */", brw_inst_imm_f(devinfo, inst)); break; case BRW_REGISTER_TYPE_DF: - format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); + format(file, "0x%016"PRIx64"DF", brw_inst_bits(inst, 127, 64)); + pad(file, 48); + format(file, "/* %-gDF */", brw_inst_imm_df(devinfo, inst)); break; case BRW_REGISTER_TYPE_HF: string(file, "Half Float IMM"); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Set swizzle to BRW_SWIZZLE_XXXX for scalar region
For 3 source operand instruction with access mode align16, channel select does not apply when RepCtrl is enabled, So setting it to BRW_SWIZZLE_ seems more appropriate choice. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu_emit.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 4630b83b1a..4550ff9a7c 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -833,7 +833,15 @@ brw_inst *brw_##OP(struct brw_codegen *p, \ struct brw_reg src0, \ struct brw_reg src1, \ struct brw_reg src2) \ -{ \ +{ \ + if (p->current->access_mode == BRW_ALIGN_16) { \ + if (has_scalar_region(src0)) \ + src0.swizzle = BRW_SWIZZLE_; \ + if (has_scalar_region(src1)) \ + src1.swizzle = BRW_SWIZZLE_; \ + if (has_scalar_region(src2)) \ + src2.swizzle = BRW_SWIZZLE_; \ + }\ return brw_alu3(p, BRW_OPCODE_##OP, dest, src0, src1, src2);\ } @@ -854,6 +862,15 @@ brw_inst *brw_##OP(struct brw_codegen *p, \ assert(src0.type == BRW_REGISTER_TYPE_DF);\ assert(src1.type == BRW_REGISTER_TYPE_DF);\ assert(src2.type == BRW_REGISTER_TYPE_DF);\ + }\ +\ + if (p->current->access_mode == BRW_ALIGN_16) { \ + if (has_scalar_region(src0)) \ + src0.swizzle = BRW_SWIZZLE_; \ + if (has_scalar_region(src1)) \ + src1.swizzle = BRW_SWIZZLE_; \ + if (has_scalar_region(src2)) \ + src2.swizzle = BRW_SWIZZLE_; \ }\ return brw_alu3(p, BRW_OPCODE_##OP, dest, src0, src1, src2); \ } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Print floating point values upto precision 8
On 10/22/18 10:34 AM, Matt Turner wrote: > On Fri, Oct 5, 2018 at 11:15 AM Sagar Ghuge wrote: >> >> avoid misinterpretation of encoded immediate values in instruction and >> disassembled output. >> >> Signed-off-by: Sagar Ghuge >> --- >> While encoding the immediate floating point values in instruction we use >> values upto precision 8, but while disassembling, we print precision to >> 6 places, which round up the value and gives wrong interpretation for >> encoded immediate constant. Printing it upto precision 8 will help in >> reassembling it again. > > Let's put that in the commit message. > >> src/intel/compiler/brw_disasm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/intel/compiler/brw_disasm.c >> b/src/intel/compiler/brw_disasm.c >> index 322f4544df..7cbbc080b3 100644 >> --- a/src/intel/compiler/brw_disasm.c >> +++ b/src/intel/compiler/brw_disasm.c >> @@ -1293,7 +1293,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, >> enum brw_reg_type type, >>format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); >>break; >> case BRW_REGISTER_TYPE_F: >> - format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); >> + format(file, "%.8fF", brw_inst_imm_f(devinfo, inst)); > > I'm not sure 8 digits is sufficient to get an exact representation > that the assembler can "round-trip". This page [1] indicates that 9 > digits are necessary for binary->decimal->binary round-trips. > I was also not sure about it, [1] article is nice. > NIR takes another approach: > > vec1 32 ssa_0 = load_const (0x3f80 /* 1.00 */) > > What do you think about printing the binary representation and the > floating-point value? That way humans can easily read one number and > the assembler can easily read the other :) > I think we can just print F and DF to 9 and 17 precision respectively to avoid output alignment issue. > Also, I think the DF case immediately after this should be handled as well. > Yes, I was planning to handle it when I shift to 64 bit datatypes. But I can club both in single patch. > [1] > https://www.exploringbinary.com/number-of-digits-required-for-round-trip-conversions/ > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/eu: Don't apply chansel when repctrl is set
On 10/22/18 10:14 AM, Matt Turner wrote: > On Tue, Oct 16, 2018 at 4:57 PM Sagar Ghuge wrote: >> > > Let's describe a little of why we're doing this and how we found it. > If I recall correctly, we set a NOP (XYZW) swizzle on 3-src > instructions, except we set an swizzle on LRP. Is that correct? > Yes. I will resend patch with full description. >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_eu_emit.c | 36 >> 1 file changed, 27 insertions(+), 9 deletions(-) >> >> diff --git a/src/intel/compiler/brw_eu_emit.c >> b/src/intel/compiler/brw_eu_emit.c >> index 0cbc682ebc..a6b45fcb1a 100644 >> --- a/src/intel/compiler/brw_eu_emit.c >> +++ b/src/intel/compiler/brw_eu_emit.c >> @@ -765,31 +765,49 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, >> struct brw_reg dest, >>brw_inst_set_3src_a16_dst_writemask(devinfo, inst, dest.writemask); >> >>assert(src0.file == BRW_GENERAL_REGISTER_FILE); >> - brw_inst_set_3src_a16_src0_swizzle(devinfo, inst, src0.swizzle); >>brw_inst_set_3src_a16_src0_subreg_nr(devinfo, inst, >> get_3src_subreg_nr(src0)); >>brw_inst_set_3src_src0_reg_nr(devinfo, inst, src0.nr); >>brw_inst_set_3src_src0_abs(devinfo, inst, src0.abs); >>brw_inst_set_3src_src0_negate(devinfo, inst, src0.negate); >> - brw_inst_set_3src_a16_src0_rep_ctrl(devinfo, inst, >> - src0.vstride == >> BRW_VERTICAL_STRIDE_0); >> + >> + /* RepCtrl field in Source or Destination Operand table in BDW Bspec >> + * says: >> + *"ChanSel does not apply when Replicate Control is set." >> + */ >> + if (src0.vstride == BRW_VERTICAL_STRIDE_0) >> + brw_inst_set_3src_a16_src0_rep_ctrl(devinfo, inst, true); >> + else >> + brw_inst_set_3src_a16_src0_swizzle(devinfo, inst, src0.swizzle); > > I see that the brw_vec1_reg() function uses an swizzle. That > indeed determines what swizzle scalar sources (i.e., those with > rep_ctrl set) will have. > > I would rather we always set the swizzle (so as to not complicate the > code here) and instead figure out why the brw_reg's swizzle field is > sometimes XYZW fix that. > > I modified brw_disasm.c locally to always print the swizzle on 3-src > operands and using the piglit tests > > generated_tests/spec/glsl-1.10/execution/built-in-functions/fs-mix-vec2-vec2-vec2.shader_test > (for LRP) > tests/spec/arb_gpu_shader5/execution/built-in-functions/fs-fma.shader_test > (for MAD) > > I cannot confirm that the swizzles are different between MAD and LRP. > What am I missing? > > lrp(8) g4<1>F g2.4<0,1,0>.xF g2.2<0,1,0>.xF > g2.0<0,1,0>.xF { align16 1Q }; > mad(8) g4<1>F g3.0<0,1,0>.xF g2.4<0,1,0>.xF > g2.0<0,1,0>.xF { align16 1Q }; > My bad, swizzles are not different (I will investigate more with different test), but while assembling we set both the rep_ctrl and the swizzle bits without checking region is scalar or not, https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/intel/compiler/brw_eu_emit.c#L768 and while disassembling we check that if the region is scalar g1<0,1,0> then we don't disassemble the swizzle bits. For src0/src1/src2 in 3src operand, https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/intel/compiler/brw_disasm.c#L1103 That's where we have different behavior for assembling and disassembling the instruction. BDW+ Bspec says RepCtrl : "This field is only present in three-source instructions, for each of the three source operands. It controls replication of the starting channel to all channels in the execution size. ChanSel does not apply when Replicate Control is set." ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Change src1 reg type to unsigned doubleword
Thank you for reviewing the patch. On 10/22/18 5:02 AM, Samuel Iglesias Gonsálvez wrote: > > > On 20/10/18 3:25, Sagar Ghuge wrote: >> To have uniform behavior while disassembling send(c) instruction use >> register type of unsigned doubleword for src1 when message descriptor is >> immediate value. Bspec does not specifiy anything for src1 immediate > > s/specifiy/specify > oops, I will fix it and resend the patch. I don't have push access. > With that fixed and assuming no problems appeared on Intel CI, > Yes, I ran through CI and it looks like there are no failures. > Reviewed-by: Samuel Iglesias Gonsálvez > > Sam > >> default type. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_eu_emit.c| 2 +- >> src/intel/compiler/brw_fs_generator.cpp | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/intel/compiler/brw_eu_emit.c >> b/src/intel/compiler/brw_eu_emit.c >> index 0cbc682ebc..4630b83b1a 100644 >> --- a/src/intel/compiler/brw_eu_emit.c >> +++ b/src/intel/compiler/brw_eu_emit.c >> @@ -371,7 +371,7 @@ brw_set_desc_ex(struct brw_codegen *p, brw_inst *inst, >> assert(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND || >>brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDC); >> brw_inst_set_src1_file_type(devinfo, inst, >> - BRW_IMMEDIATE_VALUE, BRW_REGISTER_TYPE_D); >> + BRW_IMMEDIATE_VALUE, BRW_REGISTER_TYPE_UD); >> brw_inst_set_send_desc(devinfo, inst, desc); >> if (devinfo->gen >= 9) >>brw_inst_set_send_ex_desc(devinfo, inst, ex_desc); >> diff --git a/src/intel/compiler/brw_fs_generator.cpp >> b/src/intel/compiler/brw_fs_generator.cpp >> index cb402cd4e7..08dd83dded 100644 >> --- a/src/intel/compiler/brw_fs_generator.cpp >> +++ b/src/intel/compiler/brw_fs_generator.cpp >> @@ -630,7 +630,7 @@ fs_generator::generate_urb_write(fs_inst *inst, struct >> brw_reg payload) >> >> brw_set_dest(p, insn, brw_null_reg()); >> brw_set_src0(p, insn, payload); >> - brw_set_src1(p, insn, brw_imm_d(0)); >> + brw_set_src1(p, insn, brw_imm_ud(0u)); >> >> brw_inst_set_sfid(p->devinfo, insn, BRW_SFID_URB); >> brw_inst_set_urb_opcode(p->devinfo, insn, GEN8_URB_OPCODE_SIMD8_WRITE); >> @@ -659,7 +659,7 @@ fs_generator::generate_cs_terminate(fs_inst *inst, >> struct brw_reg payload) >> >> brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW)); >> brw_set_src0(p, insn, retype(payload, BRW_REGISTER_TYPE_UW)); >> - brw_set_src1(p, insn, brw_imm_d(0)); >> + brw_set_src1(p, insn, brw_imm_ud(0u)); >> >> /* Terminate a compute shader by sending a message to the thread spawner. >> */ >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Change src1 reg type to unsigned doubleword
To have uniform behavior while disassembling send(c) instruction use register type of unsigned doubleword for src1 when message descriptor is immediate value. Bspec does not specifiy anything for src1 immediate default type. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu_emit.c| 2 +- src/intel/compiler/brw_fs_generator.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 0cbc682ebc..4630b83b1a 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -371,7 +371,7 @@ brw_set_desc_ex(struct brw_codegen *p, brw_inst *inst, assert(brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SEND || brw_inst_opcode(devinfo, inst) == BRW_OPCODE_SENDC); brw_inst_set_src1_file_type(devinfo, inst, - BRW_IMMEDIATE_VALUE, BRW_REGISTER_TYPE_D); + BRW_IMMEDIATE_VALUE, BRW_REGISTER_TYPE_UD); brw_inst_set_send_desc(devinfo, inst, desc); if (devinfo->gen >= 9) brw_inst_set_send_ex_desc(devinfo, inst, ex_desc); diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp index cb402cd4e7..08dd83dded 100644 --- a/src/intel/compiler/brw_fs_generator.cpp +++ b/src/intel/compiler/brw_fs_generator.cpp @@ -630,7 +630,7 @@ fs_generator::generate_urb_write(fs_inst *inst, struct brw_reg payload) brw_set_dest(p, insn, brw_null_reg()); brw_set_src0(p, insn, payload); - brw_set_src1(p, insn, brw_imm_d(0)); + brw_set_src1(p, insn, brw_imm_ud(0u)); brw_inst_set_sfid(p->devinfo, insn, BRW_SFID_URB); brw_inst_set_urb_opcode(p->devinfo, insn, GEN8_URB_OPCODE_SIMD8_WRITE); @@ -659,7 +659,7 @@ fs_generator::generate_cs_terminate(fs_inst *inst, struct brw_reg payload) brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW)); brw_set_src0(p, insn, retype(payload, BRW_REGISTER_TYPE_UW)); - brw_set_src1(p, insn, brw_imm_d(0)); + brw_set_src1(p, insn, brw_imm_ud(0u)); /* Terminate a compute shader by sending a message to the thread spawner. */ -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/eu: Don't apply chansel when repctrl is set
Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu_emit.c | 36 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c index 0cbc682ebc..a6b45fcb1a 100644 --- a/src/intel/compiler/brw_eu_emit.c +++ b/src/intel/compiler/brw_eu_emit.c @@ -765,31 +765,49 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct brw_reg dest, brw_inst_set_3src_a16_dst_writemask(devinfo, inst, dest.writemask); assert(src0.file == BRW_GENERAL_REGISTER_FILE); - brw_inst_set_3src_a16_src0_swizzle(devinfo, inst, src0.swizzle); brw_inst_set_3src_a16_src0_subreg_nr(devinfo, inst, get_3src_subreg_nr(src0)); brw_inst_set_3src_src0_reg_nr(devinfo, inst, src0.nr); brw_inst_set_3src_src0_abs(devinfo, inst, src0.abs); brw_inst_set_3src_src0_negate(devinfo, inst, src0.negate); - brw_inst_set_3src_a16_src0_rep_ctrl(devinfo, inst, - src0.vstride == BRW_VERTICAL_STRIDE_0); + + /* RepCtrl field in Source or Destination Operand table in BDW Bspec + * says: + *"ChanSel does not apply when Replicate Control is set." + */ + if (src0.vstride == BRW_VERTICAL_STRIDE_0) + brw_inst_set_3src_a16_src0_rep_ctrl(devinfo, inst, true); + else + brw_inst_set_3src_a16_src0_swizzle(devinfo, inst, src0.swizzle); assert(src1.file == BRW_GENERAL_REGISTER_FILE); - brw_inst_set_3src_a16_src1_swizzle(devinfo, inst, src1.swizzle); brw_inst_set_3src_a16_src1_subreg_nr(devinfo, inst, get_3src_subreg_nr(src1)); brw_inst_set_3src_src1_reg_nr(devinfo, inst, src1.nr); brw_inst_set_3src_src1_abs(devinfo, inst, src1.abs); brw_inst_set_3src_src1_negate(devinfo, inst, src1.negate); - brw_inst_set_3src_a16_src1_rep_ctrl(devinfo, inst, - src1.vstride == BRW_VERTICAL_STRIDE_0); + + /* RepCtrl field in Source or Destination Operand table in BDW Bspec + * says: + *"ChanSel does not apply when Replicate Control is set." + */ + if (src1.vstride == BRW_VERTICAL_STRIDE_0) + brw_inst_set_3src_a16_src1_rep_ctrl(devinfo, inst, true); + else + brw_inst_set_3src_a16_src1_swizzle(devinfo, inst, src1.swizzle); assert(src2.file == BRW_GENERAL_REGISTER_FILE); - brw_inst_set_3src_a16_src2_swizzle(devinfo, inst, src2.swizzle); brw_inst_set_3src_a16_src2_subreg_nr(devinfo, inst, get_3src_subreg_nr(src2)); brw_inst_set_3src_src2_reg_nr(devinfo, inst, src2.nr); brw_inst_set_3src_src2_abs(devinfo, inst, src2.abs); brw_inst_set_3src_src2_negate(devinfo, inst, src2.negate); - brw_inst_set_3src_a16_src2_rep_ctrl(devinfo, inst, - src2.vstride == BRW_VERTICAL_STRIDE_0); + + /* RepCtrl field in Source or Destination Operand table in BDW Bspec + * says: + *"ChanSel does not apply when Replicate Control is set." + */ + if (src2.vstride == BRW_VERTICAL_STRIDE_0) + brw_inst_set_3src_a16_src2_rep_ctrl(devinfo, inst, true); + else + brw_inst_set_3src_a16_src2_swizzle(devinfo, inst, src2.swizzle); if (devinfo->gen >= 7) { /* Set both the source and destination types based on dest.type, -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Print floating point values upto precision 8
avoid misinterpretation of encoded immediate values in instruction and disassembled output. Signed-off-by: Sagar Ghuge --- While encoding the immediate floating point values in instruction we use values upto precision 8, but while disassembling, we print precision to 6 places, which round up the value and gives wrong interpretation for encoded immediate constant. Printing it upto precision 8 will help in reassembling it again. src/intel/compiler/brw_disasm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 322f4544df..7cbbc080b3 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -1293,7 +1293,7 @@ imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type, format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); break; case BRW_REGISTER_TYPE_F: - format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); + format(file, "%.8fF", brw_inst_imm_f(devinfo, inst)); break; case BRW_REGISTER_TYPE_DF: format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 2/4] intel/decoder: Avoid freeing invalid pointer
Ping. On 9/6/18 12:37 PM, Sagar Ghuge wrote: > v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) > > v3: Handle case where genxml is empty (Lionel Landwerlin) > > Signed-off-by: Sagar Ghuge > --- > src/intel/common/gen_decoder.c | 18 +- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c > index d4db8b89cc..38a5fccb2a 100644 > --- a/src/intel/common/gen_decoder.c > +++ b/src/intel/common/gen_decoder.c > @@ -654,25 +654,27 @@ gen_spec_load_from_path(const struct gen_device_info > *devinfo, > ctx.spec = gen_spec_init(); > if (ctx.spec == NULL) { >fprintf(stderr, "Failed to create gen_spec\n"); > - return NULL; > + goto end; > } > > do { >buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); >len = fread(buf, 1, XML_BUFFER_SIZE, input); > - if (len == 0) { > + if (ferror(input)) { > fprintf(stderr, "fread: %m\n"); > - free(ctx.spec); > + gen_spec_destroy(ctx.spec); > ctx.spec = NULL; > goto end; > - } > + } else if (feof(input)) > + goto end; > + >if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { > fprintf(stderr, > "Error parsing XML at line %ld col %ld: %s\n", > XML_GetCurrentLineNumber(ctx.parser), > XML_GetCurrentColumnNumber(ctx.parser), > XML_ErrorString(XML_GetErrorCode(ctx.parser))); > - free(ctx.spec); > + gen_spec_destroy(ctx.spec); > ctx.spec = NULL; > goto end; >} > @@ -684,6 +686,12 @@ gen_spec_load_from_path(const struct gen_device_info > *devinfo, > fclose(input); > free(filename); > > + /* free ctx.spec if genxml is empty */ > + if (ctx.spec && _mesa_hash_table_num_entries(ctx.spec->commands) == 0) { > + gen_spec_destroy(ctx.spec); > + return NULL; > + } > + > return ctx.spec; > } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/4] intel/decoder: Avoid freeing invalid pointer
v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) v3: Handle case where genxml is empty (Lionel Landwerlin) Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index d4db8b89cc..38a5fccb2a 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -654,25 +654,27 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, ctx.spec = gen_spec_init(); if (ctx.spec == NULL) { fprintf(stderr, "Failed to create gen_spec\n"); - return NULL; + goto end; } do { buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); len = fread(buf, 1, XML_BUFFER_SIZE, input); - if (len == 0) { + if (ferror(input)) { fprintf(stderr, "fread: %m\n"); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; - } + } else if (feof(input)) + goto end; + if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { fprintf(stderr, "Error parsing XML at line %ld col %ld: %s\n", XML_GetCurrentLineNumber(ctx.parser), XML_GetCurrentColumnNumber(ctx.parser), XML_ErrorString(XML_GetErrorCode(ctx.parser))); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; } @@ -684,6 +686,12 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, fclose(input); free(filename); + /* free ctx.spec if genxml is empty */ + if (ctx.spec && _mesa_hash_table_num_entries(ctx.spec->commands) == 0) { + gen_spec_destroy(ctx.spec); + return NULL; + } + return ctx.spec; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 2/4] intel/decoder: Avoid freeing invalid pointer
Please ignore this patch. On 09/06/2018 11:33 AM, Sagar Ghuge wrote: > v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) > > v3: Handle case where genxml is empty (Lionel Landwerlin) > > Signed-off-by: Sagar Ghuge > --- > src/intel/common/gen_decoder.c | 17 - > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c > index d4db8b89cc..04ad81486a 100644 > --- a/src/intel/common/gen_decoder.c > +++ b/src/intel/common/gen_decoder.c > @@ -654,25 +654,26 @@ gen_spec_load_from_path(const struct gen_device_info > *devinfo, > ctx.spec = gen_spec_init(); > if (ctx.spec == NULL) { >fprintf(stderr, "Failed to create gen_spec\n"); > - return NULL; > + goto end; > } > > do { >buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); >len = fread(buf, 1, XML_BUFFER_SIZE, input); > - if (len == 0) { > + if (ferror(input)) { > fprintf(stderr, "fread: %m\n"); > - free(ctx.spec); > + gen_spec_destroy(ctx.spec); > ctx.spec = NULL; > goto end; > - } > + } else if (feof(input)) > +goto end; >if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { > fprintf(stderr, > "Error parsing XML at line %ld col %ld: %s\n", > XML_GetCurrentLineNumber(ctx.parser), > XML_GetCurrentColumnNumber(ctx.parser), > XML_ErrorString(XML_GetErrorCode(ctx.parser))); > - free(ctx.spec); > + gen_spec_destroy(ctx.spec); > ctx.spec = NULL; > goto end; >} > @@ -684,6 +685,12 @@ gen_spec_load_from_path(const struct gen_device_info > *devinfo, > fclose(input); > free(filename); > > + /* free ctx.spec if genxml is empty */ > + if (ctx.spec && _mesa_hash_table_num_entries(ctx.spec->commands) == 0) { > + gen_spec_destroy(ctx.spec); > + return NULL; > + } > + > return ctx.spec; > } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/4] intel/decoder: Avoid freeing invalid pointer
v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) v3: Handle case where genxml is empty (Lionel Landwerlin) Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index d4db8b89cc..04ad81486a 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -654,25 +654,26 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, ctx.spec = gen_spec_init(); if (ctx.spec == NULL) { fprintf(stderr, "Failed to create gen_spec\n"); - return NULL; + goto end; } do { buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); len = fread(buf, 1, XML_BUFFER_SIZE, input); - if (len == 0) { + if (ferror(input)) { fprintf(stderr, "fread: %m\n"); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; - } + } else if (feof(input)) +goto end; if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { fprintf(stderr, "Error parsing XML at line %ld col %ld: %s\n", XML_GetCurrentLineNumber(ctx.parser), XML_GetCurrentColumnNumber(ctx.parser), XML_ErrorString(XML_GetErrorCode(ctx.parser))); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; } @@ -684,6 +685,12 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, fclose(input); free(filename); + /* free ctx.spec if genxml is empty */ + if (ctx.spec && _mesa_hash_table_num_entries(ctx.spec->commands) == 0) { + gen_spec_destroy(ctx.spec); + return NULL; + } + return ctx.spec; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/4] intel/decoder: Avoid freeing invalid pointer
On 09/06/2018 02:39 AM, Lionel Landwerlin wrote: > On 06/09/2018 05:12, Sagar Ghuge wrote: >> v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/common/gen_decoder.c | 15 --- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c >> index d4db8b89cc..9d647033cf 100644 >> --- a/src/intel/common/gen_decoder.c >> +++ b/src/intel/common/gen_decoder.c >> @@ -654,7 +654,7 @@ gen_spec_load_from_path(const struct gen_device_info >> *devinfo, >> ctx.spec = gen_spec_init(); >> if (ctx.spec == NULL) { >> fprintf(stderr, "Failed to create gen_spec\n"); >> - return NULL; >> + goto end; >> } >> do { >> @@ -662,17 +662,26 @@ gen_spec_load_from_path(const struct gen_device_info >> *devinfo, >> len = fread(buf, 1, XML_BUFFER_SIZE, input); >> if (len == 0) { >> fprintf(stderr, "fread: %m\n"); >> - free(ctx.spec); >> + gen_spec_destroy(ctx.spec); >> ctx.spec = NULL; >> goto end; >> + } else { >> + if (ferror(input)) { >> + fprintf(stderr, "fread: %m\n"); >> + gen_spec_destroy(ctx.spec); >> + ctx.spec = NULL; >> + goto end; >> + } else if (feof(input)) >> + goto end; > > > Looking at the fread man page, it seems like len == 0 means either error or > end of file. > I was trying to handle the case - if genxml is already empty/has error on first fread attempt, then free ctx.spec. (len == 0) if it's not empty and there is a error for subsequent fread's - free ctx.spec for rest of the behavior just goto end. > So I think the ferror/feof checks should be inside the if (len == 0). > > > Maybe we can even remove the if (len == 0) and put the if (ferror(input)) ... > instead. > if we remove (len == 0) and kept only ferror(input) & feof(input) check will end up allocating memory for ctx.spec if genxml file is empty. Please let me know if I might missed any other cases. > > - > > Lionel > > >> } >> + >> if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { >> fprintf(stderr, >> "Error parsing XML at line %ld col %ld: %s\n", >> XML_GetCurrentLineNumber(ctx.parser), >> XML_GetCurrentColumnNumber(ctx.parser), >> XML_ErrorString(XML_GetErrorCode(ctx.parser))); >> - free(ctx.spec); >> + gen_spec_destroy(ctx.spec); >> ctx.spec = NULL; >> goto end; >> } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 3/4] intel/decoder: construct correct xml filename
construct correct gen xml filename when we try to load hardware xml description from a given path v2: remove temporary variable (Francesco Ansanelli) Signed-off-by: Sagar Ghuge Reviewed-by: Lionel Landwerlin --- src/intel/common/gen_decoder.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index 9d647033cf..1ad7d6b4d3 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -463,14 +463,13 @@ character_data(void *data, const XML_Char *s, int len) } static int -devinfo_to_gen(const struct gen_device_info *devinfo) +devinfo_to_gen(const struct gen_device_info *devinfo, bool x10) { - int value = 10 * devinfo->gen; - - if (devinfo->is_baytrail || devinfo->is_haswell) - value += 5; + if (devinfo->is_baytrail || devinfo->is_haswell) { + return devinfo->gen * 10 + 5; + } - return value; + return x10 ? devinfo->gen * 10 : devinfo->gen; } static uint32_t zlib_inflate(const void *compressed_data, @@ -558,7 +557,7 @@ gen_spec_load(const struct gen_device_info *devinfo) uint8_t *text_data = NULL; uint32_t text_offset = 0, text_length = 0; MAYBE_UNUSED uint32_t total_length; - uint32_t gen_10 = devinfo_to_gen(devinfo); + uint32_t gen_10 = devinfo_to_gen(devinfo, true); for (int i = 0; i < ARRAY_SIZE(genxml_files_table); i++) { if (genxml_files_table[i].gen_10 == gen_10) { @@ -627,7 +626,7 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, FILE *input; len = snprintf(filename, filename_len, "%s/gen%i.xml", - path, devinfo_to_gen(devinfo)); + path, devinfo_to_gen(devinfo, false)); assert(len < filename_len); input = fopen(filename, "r"); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/4] intel/decoder: Avoid freeing invalid pointer
v2: Free ctx.spec if error while reading genxml (Lionel Landwerlin) Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index d4db8b89cc..9d647033cf 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -654,7 +654,7 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, ctx.spec = gen_spec_init(); if (ctx.spec == NULL) { fprintf(stderr, "Failed to create gen_spec\n"); - return NULL; + goto end; } do { @@ -662,17 +662,26 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, len = fread(buf, 1, XML_BUFFER_SIZE, input); if (len == 0) { fprintf(stderr, "fread: %m\n"); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; + } else { + if (ferror(input)) { +fprintf(stderr, "fread: %m\n"); +gen_spec_destroy(ctx.spec); +ctx.spec = NULL; +goto end; + } else if (feof(input)) +goto end; } + if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { fprintf(stderr, "Error parsing XML at line %ld col %ld: %s\n", XML_GetCurrentLineNumber(ctx.parser), XML_GetCurrentColumnNumber(ctx.parser), XML_ErrorString(XML_GetErrorCode(ctx.parser))); - free(ctx.spec); + gen_spec_destroy(ctx.spec); ctx.spec = NULL; goto end; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 1/4] intel/decoder: add gen_spec_init method
Initialize gen_spec instance properly when loading hardware xml description from specifc directory to avoid segmentation fault. v2: correct function definition (Lionel Landwerlin) Signed-off-by: Sagar Ghuge Reviewed-by: Lionel Landwerlin --- src/intel/common/gen_decoder.c | 51 +++--- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index c6c213fcd1..d4db8b89cc 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -526,6 +526,30 @@ static uint32_t _hash_uint32(const void *key) return (uint32_t) (uintptr_t) key; } +static struct gen_spec * +gen_spec_init(void) +{ + struct gen_spec *spec; + spec = rzalloc(NULL, struct gen_spec); + if (spec == NULL) + return NULL; + + spec->commands = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->structs = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->registers_by_name = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->registers_by_offset = + _mesa_hash_table_create(spec, _hash_uint32, _mesa_key_pointer_equal); + spec->enums = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->access_cache = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + + return spec; +} + struct gen_spec * gen_spec_load(const struct gen_device_info *devinfo) { @@ -560,21 +584,11 @@ gen_spec_load(const struct gen_device_info *devinfo) XML_SetElementHandler(ctx.parser, start_element, end_element); XML_SetCharacterDataHandler(ctx.parser, character_data); - ctx.spec = rzalloc(NULL, struct gen_spec); - - ctx.spec->commands = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->structs = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->registers_by_name = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->registers_by_offset = - _mesa_hash_table_create(ctx.spec, _hash_uint32, _mesa_key_pointer_equal); - ctx.spec->enums = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - - ctx.spec->access_cache = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); + ctx.spec = gen_spec_init(); + if (ctx.spec == NULL) { + fprintf(stderr, "Failed to create gen_spec\n"); + return NULL; + } total_length = zlib_inflate(compress_genxmls, sizeof(compress_genxmls), @@ -636,7 +650,12 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, XML_SetElementHandler(ctx.parser, start_element, end_element); XML_SetCharacterDataHandler(ctx.parser, character_data); ctx.loc.filename = filename; - ctx.spec = rzalloc(NULL, struct gen_spec); + + ctx.spec = gen_spec_init(); + if (ctx.spec == NULL) { + fprintf(stderr, "Failed to create gen_spec\n"); + return NULL; + } do { buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer
On 09/05/2018 11:06 AM, Lionel Landwerlin wrote: > On 05/09/2018 19:02, Sagar Ghuge wrote: >> Hi Lionel, >> >> Thanks for reviewing patches and comments. >> >> On 09/05/2018 10:29 AM, Lionel Landwerlin wrote: >>> On 05/09/2018 18:19, Sagar Ghuge wrote: >>>> Signed-off-by: Sagar Ghuge >>>> --- >>>> src/intel/common/gen_decoder.c | 4 >>>> 1 file changed, 4 deletions(-) >>>> >>>> diff --git a/src/intel/common/gen_decoder.c >>>> b/src/intel/common/gen_decoder.c >>>> index dbd060d53c..c44b8f060d 100644 >>>> --- a/src/intel/common/gen_decoder.c >>>> +++ b/src/intel/common/gen_decoder.c >>>> @@ -662,8 +662,6 @@ gen_spec_load_from_path(const struct gen_device_info >>>> *devinfo, >>>> len = fread(buf, 1, XML_BUFFER_SIZE, input); >>>> if (len == 0) { >>>> fprintf(stderr, "fread: %m\n"); >>>> - free(ctx.spec); >>>> - ctx.spec = NULL; >>>> goto end; >>>> } >>>> if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { >>>> @@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info >>>> *devinfo, >>>> XML_GetCurrentLineNumber(ctx.parser), >>>> XML_GetCurrentColumnNumber(ctx.parser), >>>> XML_ErrorString(XML_GetErrorCode(ctx.parser))); >>>> - free(ctx.spec); >>>> - ctx.spec = NULL; >>>> goto end; >>>> } >>>> } while (len > 0); >>> Looks good but we're still missing a ralloc_free(ctx.spec) after the end >>> label. >> In patch 4, I am freeing up gen_batch_decode_ctx instance using >> gen_batch_decode_ctx_finish() which is also responsible for destroying spec. > > > I see. But I assumed that would be only in the case this function succeeded. > > If we didn't manage to parse the genxml, is there much of a point trying to > decode anything? > ohh. I got it. My bad, I was trying to fix wrong problem I guess. fread does not distinguish between end-of-file and error, so according to that I will free up spec. > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer
Hi Lionel, Thanks for reviewing patches and comments. On 09/05/2018 10:29 AM, Lionel Landwerlin wrote: > On 05/09/2018 18:19, Sagar Ghuge wrote: >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/common/gen_decoder.c | 4 >> 1 file changed, 4 deletions(-) >> >> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c >> index dbd060d53c..c44b8f060d 100644 >> --- a/src/intel/common/gen_decoder.c >> +++ b/src/intel/common/gen_decoder.c >> @@ -662,8 +662,6 @@ gen_spec_load_from_path(const struct gen_device_info >> *devinfo, >> len = fread(buf, 1, XML_BUFFER_SIZE, input); >> if (len == 0) { >> fprintf(stderr, "fread: %m\n"); >> - free(ctx.spec); >> - ctx.spec = NULL; >> goto end; >> } >> if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { >> @@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info >> *devinfo, >> XML_GetCurrentLineNumber(ctx.parser), >> XML_GetCurrentColumnNumber(ctx.parser), >> XML_ErrorString(XML_GetErrorCode(ctx.parser))); >> - free(ctx.spec); >> - ctx.spec = NULL; >> goto end; >> } >> } while (len > 0); > > Looks good but we're still missing a ralloc_free(ctx.spec) after the end > label. In patch 4, I am freeing up gen_batch_decode_ctx instance using gen_batch_decode_ctx_finish() which is also responsible for destroying spec. > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] intel/decoder: Avoid freeing invalid pointer
Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 4 1 file changed, 4 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index dbd060d53c..c44b8f060d 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -662,8 +662,6 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, len = fread(buf, 1, XML_BUFFER_SIZE, input); if (len == 0) { fprintf(stderr, "fread: %m\n"); - free(ctx.spec); - ctx.spec = NULL; goto end; } if (XML_ParseBuffer(ctx.parser, len, len == 0) == 0) { @@ -672,8 +670,6 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, XML_GetCurrentLineNumber(ctx.parser), XML_GetCurrentColumnNumber(ctx.parser), XML_ErrorString(XML_GetErrorCode(ctx.parser))); - free(ctx.spec); - ctx.spec = NULL; goto end; } } while (len > 0); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] intel/decoder: construct correct xml filename
construct correct gen xml filename when we try to load hardware xml description from a given path Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index c44b8f060d..e2509cbd87 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -463,12 +463,18 @@ character_data(void *data, const XML_Char *s, int len) } static int -devinfo_to_gen(const struct gen_device_info *devinfo) +devinfo_to_gen(const struct gen_device_info *devinfo, bool x10) { - int value = 10 * devinfo->gen; + int value = devinfo->gen; - if (devinfo->is_baytrail || devinfo->is_haswell) - value += 5; + if (x10) + value = 10 * devinfo->gen; + + if (devinfo->is_baytrail || devinfo->is_haswell) { + if (!x10) +value *= 10; + value += 5; + } return value; } @@ -558,7 +564,7 @@ gen_spec_load(const struct gen_device_info *devinfo) uint8_t *text_data = NULL; uint32_t text_offset = 0, text_length = 0; MAYBE_UNUSED uint32_t total_length; - uint32_t gen_10 = devinfo_to_gen(devinfo); + uint32_t gen_10 = devinfo_to_gen(devinfo, true); for (int i = 0; i < ARRAY_SIZE(genxml_files_table); i++) { if (genxml_files_table[i].gen_10 == gen_10) { @@ -627,7 +633,7 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, FILE *input; len = snprintf(filename, filename_len, "%s/gen%i.xml", - path, devinfo_to_gen(devinfo)); + path, devinfo_to_gen(devinfo, false)); assert(len < filename_len); input = fopen(filename, "r"); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] intel: aubinator: Fix memory leaks
Signed-off-by: Sagar Ghuge --- src/intel/tools/aubinator.c | 25 + 1 file changed, 25 insertions(+) diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c index 55672fa073..ef0f7650b1 100644 --- a/src/intel/tools/aubinator.c +++ b/src/intel/tools/aubinator.c @@ -95,6 +95,18 @@ aubinator_init(void *user_data, int aub_pci_id, const char *app_name) gen_batch_decode_ctx_init(_ctx, , outfile, batch_flags, xml_path, NULL, NULL, NULL); + + /* Check for valid spec instance, if wrong xml_path is passed then spec +* instance is not initialized properly +*/ + if (!batch_ctx.spec) { + fprintf(stderr, "Failed to initialize gen_batch_decode_ctx " + "spec instance\n"); + free(xml_path); + gen_batch_decode_ctx_finish(_ctx); + exit(EXIT_FAILURE); + } + batch_ctx.max_vbo_decoded_lines = max_vbo_lines; char *color = GREEN_HEADER, *reset_color = NORMAL; @@ -178,14 +190,19 @@ aub_file_open(const char *filename) int fd; file = calloc(1, sizeof *file); + if (file == NULL) + return NULL; + fd = open(filename, O_RDONLY); if (fd == -1) { fprintf(stderr, "open %s failed: %s\n", filename, strerror(errno)); + free(file); exit(EXIT_FAILURE); } if (fstat(fd, ) == -1) { fprintf(stderr, "stat failed: %s\n", strerror(errno)); + free(file); exit(EXIT_FAILURE); } @@ -193,6 +210,7 @@ aub_file_open(const char *filename) PROT_READ, MAP_SHARED, fd, 0); if (file->map == MAP_FAILED) { fprintf(stderr, "mmap failed: %s\n", strerror(errno)); + free(file); exit(EXIT_FAILURE); } @@ -333,6 +351,11 @@ int main(int argc, char *argv[]) } file = aub_file_open(input_file); + if (!file) { + fprintf(stderr, "Unable to allocate buffer to open aub file\n"); + free(xml_path); + exit(EXIT_FAILURE); + } struct aub_read aub_read = { .user_data = , @@ -359,9 +382,11 @@ int main(int argc, char *argv[]) fflush(stdout); /* close the stdout which is opened to write the output */ close(1); + free(file); free(xml_path); wait(NULL); + gen_batch_decode_ctx_finish(_ctx); return EXIT_SUCCESS; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/4] aubinator: construct correct genxml
I was just playing around with aubinator and I found out that it's constructing incorrect gen xml filename when xml option is specified. For e.g it's trying to locate Gen90.xml instead of Gen9.xml This patch series fix memory leaks and construct correct gen xml filename with/without xml option. I am still getting familar with tool so please let me know if I misunderstood things while fixing the problem. Sagar Ghuge (4): intel/decoder: add gen_spec_init method intel/decoder: Avoid freeing invalid pointer intel/decoder: construct correct xml filename intel: aubinator: Fix memory leaks src/intel/common/gen_decoder.c | 73 ++ src/intel/tools/aubinator.c| 25 2 files changed, 72 insertions(+), 26 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] intel/decoder: add gen_spec_init method
Initialize gen_spec instance properly when loading hardware xml description from specifc directory to avoid segmentation fault. Signed-off-by: Sagar Ghuge --- src/intel/common/gen_decoder.c | 51 +++--- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c index c6c213fcd1..dbd060d53c 100644 --- a/src/intel/common/gen_decoder.c +++ b/src/intel/common/gen_decoder.c @@ -526,6 +526,30 @@ static uint32_t _hash_uint32(const void *key) return (uint32_t) (uintptr_t) key; } +static struct gen_spec * +gen_spec_init() +{ + struct gen_spec *spec; + spec = rzalloc(NULL, struct gen_spec); + if (spec == NULL) + return NULL; + + spec->commands = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->structs = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->registers_by_name = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->registers_by_offset = + _mesa_hash_table_create(spec, _hash_uint32, _mesa_key_pointer_equal); + spec->enums = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + spec->access_cache = + _mesa_hash_table_create(spec, _mesa_hash_string, _mesa_key_string_equal); + + return spec; +} + struct gen_spec * gen_spec_load(const struct gen_device_info *devinfo) { @@ -560,21 +584,11 @@ gen_spec_load(const struct gen_device_info *devinfo) XML_SetElementHandler(ctx.parser, start_element, end_element); XML_SetCharacterDataHandler(ctx.parser, character_data); - ctx.spec = rzalloc(NULL, struct gen_spec); - - ctx.spec->commands = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->structs = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->registers_by_name = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - ctx.spec->registers_by_offset = - _mesa_hash_table_create(ctx.spec, _hash_uint32, _mesa_key_pointer_equal); - ctx.spec->enums = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); - - ctx.spec->access_cache = - _mesa_hash_table_create(ctx.spec, _mesa_hash_string, _mesa_key_string_equal); + ctx.spec = gen_spec_init(); + if (ctx.spec == NULL) { + fprintf(stderr, "Failed to create gen_spec\n"); + return NULL; + } total_length = zlib_inflate(compress_genxmls, sizeof(compress_genxmls), @@ -636,7 +650,12 @@ gen_spec_load_from_path(const struct gen_device_info *devinfo, XML_SetElementHandler(ctx.parser, start_element, end_element); XML_SetCharacterDataHandler(ctx.parser, character_data); ctx.loc.filename = filename; - ctx.spec = rzalloc(NULL, struct gen_spec); + + ctx.spec = gen_spec_init(); + if (ctx.spec == NULL) { + fprintf(stderr, "Failed to create gen_spec\n"); + return NULL; + } do { buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs/relnotes: Add AMD_depth_clamp_separate for i965
Hi Marek, Thanks for reviewing but I don't have commit rights yet. :( -- Sagar On 08/29/2018 02:34 PM, Marek Olšák wrote: > Looks good. You can push this without an Rb. > > Marek > > On Tue, Aug 28, 2018 at 5:53 PM, Sagar Ghuge wrote: >> Signed-off-by: Sagar Ghuge >> --- >> docs/relnotes/18.3.0.html | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html >> index 71fb41ca86..6e5e3ef93b 100644 >> --- a/docs/relnotes/18.3.0.html >> +++ b/docs/relnotes/18.3.0.html >> @@ -51,6 +51,7 @@ Note: some of the new features are only available with >> certain drivers. >> >> >> >> +GL_AMD_depth_clamp_separate on i965. >> GL_AMD_framebuffer_multisample_advanced on radeonsi. >> GL_AMD_gpu_shader_int64 on i965, nvc0, radeonsi. >> GL_AMD_multi_draw_indirect on all GL 4.x drivers. >> -- >> 2.17.1 >> >> ___ >> 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 v3] intel/tools: new i965_disasm tool
Adds a new i965 instruction disassemble tool v2: 1) fix a few nits (Matt Turner) 2) Remove i965_disasm header (Matt Turner) v3: 1) Redirect output to correct file descriptors (Matt Turner) 2) Refactor code (Matt Turner) 3) Use better formatting style (Matt Turner) Signed-off-by: Sagar Ghuge --- src/intel/Makefile.tools.am | 14 +++ src/intel/tools/i965_disasm.c | 182 ++ src/intel/tools/meson.build | 11 ++ 3 files changed, 207 insertions(+) create mode 100644 src/intel/tools/i965_disasm.c diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am index 30c8d3b3f7..4809962b18 100644 --- a/src/intel/Makefile.tools.am +++ b/src/intel/Makefile.tools.am @@ -22,6 +22,7 @@ noinst_PROGRAMS += \ tools/aubinator \ tools/aubinator_error_decode \ + tools/i965_disasm \ tools/error2aub @@ -66,6 +67,19 @@ tools_aubinator_error_decode_CFLAGS = \ $(AM_CFLAGS) \ $(ZLIB_CFLAGS) +tools_i965_disasm_SOURCES = \ + tools/i965_disasm.c + +tools_i965_disasm_LDADD = \ + common/libintel_common.la \ + compiler/libintel_compiler.la \ + dev/libintel_dev.la \ + $(top_builddir)/src/util/libmesautil.la \ + $(PTHREAD_LIBS) + +tools_i965_disasm_CFLAGS = \ + $(AM_CFLAGS) + tools_error2aub_SOURCES = \ tools/gen_context.h \ diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c new file mode 100644 index 00..73a6760fc1 --- /dev/null +++ b/src/intel/tools/i965_disasm.c @@ -0,0 +1,182 @@ +/* + * Copyright © 2018 Intel Corporation + * + * 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. + */ + +#include +#include +#include +#include + +#include "compiler/brw_eu.h" +#include "dev/gen_device_info.h" + +uint64_t INTEL_DEBUG; + +/* Return size of file in bytes pointed by fp */ +static size_t +i965_disasm_get_file_size(FILE *fp) +{ + size_t size; + + fseek(fp, 0L, SEEK_END); + size = ftell(fp); + fseek(fp, 0L, SEEK_SET); + + return size; +} + +static void * +i965_disasm_read_binary(FILE *fp, size_t *end) +{ + void *assembly; + + *end = i965_disasm_get_file_size(fp); + + assembly = malloc(*end + 1); + if (assembly == NULL) + return NULL; + + fread(assembly, *end, 1, fp); + fclose(fp); + + return assembly; +} + +static struct gen_device_info * +i965_disasm_init(uint16_t pci_id) +{ + struct gen_device_info *devinfo; + + devinfo = malloc(sizeof *devinfo); + if (devinfo == NULL) + return NULL; + + if (!gen_get_device_info(pci_id, devinfo)) { + fprintf(stderr, "can't find device information: pci_id=0x%x\n", + pci_id); + exit(EXIT_FAILURE); + } + + /* initialize compaction table in order to handle compacted instructions */ + brw_init_compaction_tables(devinfo); + + return devinfo; +} + +static void +print_help(const char *progname, FILE *file) +{ + fprintf(file, + "Usage: %s [OPTION]...\n" + "Disassemble i965 instructions from binary file.\n\n" + " --help display this help and exit\n" + " --binary-path=PATH read binary file from binary file PATH\n" + " --gen=platform disassemble instructions for given \n" + " platform (3 letter platform name)\n", + progname); +} + +int main(int argc, char *argv[]) +{ + FILE *fp = NULL; + void *assembly = NULL; + char *binary_path = NULL; + size_t start = 0, end = 0; + uint16_t pci_id = 0; + int c, i; + struct gen_device_info *devinfo; + + bool help = false; + const struct option i965_disasm_opts[] = { + { "help", no_argument, (int *) , true }, + { "binary-path", required_argum
[Mesa-dev] [PATCH] docs/relnotes: Add AMD_depth_clamp_separate for i965
Signed-off-by: Sagar Ghuge --- docs/relnotes/18.3.0.html | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html index 71fb41ca86..6e5e3ef93b 100644 --- a/docs/relnotes/18.3.0.html +++ b/docs/relnotes/18.3.0.html @@ -51,6 +51,7 @@ Note: some of the new features are only available with certain drivers. +GL_AMD_depth_clamp_separate on i965. GL_AMD_framebuffer_multisample_advanced on radeonsi. GL_AMD_gpu_shader_int64 on i965, nvc0, radeonsi. GL_AMD_multi_draw_indirect on all GL 4.x drivers. -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] intel/tools: new i965_disasm tool
Hey Matt, Thanks for reviewing the patch. On 08/27/2018 11:06 AM, Matt Turner wrote: > On Mon, Aug 20, 2018 at 3:13 PM Sagar Ghuge wrote: >> >> Adds a new i965 instruction disassemble tool >> >> v2: 1) fix a few nits (Matt Turner) >> 2) Remove i965_disasm header (Matt Turner) >> >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/Makefile.tools.am | 14 +++ >> src/intel/tools/i965_disasm.c | 199 ++ >> src/intel/tools/meson.build | 11 ++ >> 3 files changed, 224 insertions(+) >> create mode 100644 src/intel/tools/i965_disasm.c >> >> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am >> index 00624084e6..385819abc2 100644 >> --- a/src/intel/Makefile.tools.am >> +++ b/src/intel/Makefile.tools.am >> @@ -22,6 +22,7 @@ >> noinst_PROGRAMS += \ >> tools/aubinator \ >> tools/aubinator_error_decode \ >> + tools/i965_disasm \ >> tools/error2aub >> >> >> @@ -62,6 +63,19 @@ tools_aubinator_error_decode_CFLAGS = \ >> $(AM_CFLAGS) \ >> $(ZLIB_CFLAGS) >> >> +tools_i965_disasm_SOURCES = \ >> + tools/i965_disasm.c >> + >> +tools_i965_disasm_LDADD = \ >> + common/libintel_common.la \ >> + compiler/libintel_compiler.la \ >> + dev/libintel_dev.la \ >> + $(top_builddir)/src/util/libmesautil.la \ >> + $(PTHREAD_LIBS) >> + >> +tools_i965_disasm_CFLAGS = \ >> + $(AM_CFLAGS) >> + >> >> tools_error2aub_SOURCES = \ >> tools/gen_context.h \ >> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c >> new file mode 100644 >> index 00..757d2c7db1 >> --- /dev/null >> +++ b/src/intel/tools/i965_disasm.c >> @@ -0,0 +1,199 @@ >> +/* >> + * Copyright © 2018 Intel Corporation >> + * >> + * 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. >> + */ >> + >> +#include >> +#include >> + >> +#include "compiler/brw_inst.h" >> +#include "compiler/brw_eu.h" >> +#include "dev/gen_device_info.h" >> + >> +uint64_t INTEL_DEBUG; >> +uint16_t pci_id = 0; >> +FILE *outfile; > > I expected to find code that opens a file and sets outfile, but > outfile is only set to stderr. I'd either add a -o option or get rid > of the outfile variable entirely. Either way works, but in both cases > error messages should be printed to stderr instead of outfile since > you wouldn't want errors intermixed with the disassembly. > > So either: > >1) add a -o out option (defaulting outfile to stdout) and change > errors to be printed to stderr; or > >2) remove the outfile variable, output errors to stderr and > disassembly to stdout > > If you choose to keep outfile, I think it can become a local variable in > main(). > >> + >> +struct i965_disasm { >> +struct gen_device_info devinfo; >> +}; > > I think we should simplify some things by removing the i965_disasm type. > >> + >> +/* Return size of file in bytes pointed by fp */ >> +static size_t >> +i965_disasm_get_file_size(FILE *fp) >> +{ >> + size_t size; >> + >> + fseek(fp, 0L, SEEK_END); >> + size = ftell(fp); >> + fseek(fp, 0L, SEEK_SET); >> + >> + retu
[Mesa-dev] [PATCH v3] intel/eu: print bytes instead of 32 bit hex value
INTEL_DEBUG=hex prints 32 bit hex value and due to endianness of CPU byte order is reversed. In order to disassemble binary files, print each byte instead of 32 bit hex value. v2: Print blank spaces in order to vertically align output of compacted instructions hex value with uncompacted instructions hex value. (Matt Turner) v3: Fix line wrap at correct length Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu.c | 47 +++-- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c index 6ef0a6a577..3fb4e40507 100644 --- a/src/intel/compiler/brw_eu.c +++ b/src/intel/compiler/brw_eu.c @@ -364,24 +364,37 @@ brw_disassemble(const struct gen_device_info *devinfo, if (compacted) { brw_compact_inst *compacted = (void *)insn; -if (dump_hex) { - fprintf(out, "0x%08x 0x%08x ", - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); -} - -brw_uncompact_instruction(devinfo, , compacted); -insn = -offset += 8; + if (dump_hex) { +unsigned char * insn_ptr = ((unsigned char *)[0]); +const unsigned int blank_spaces = 24; +for (int i = 0 ; i < 8; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); +} +/* Make compacted instructions hex value output vertically aligned + * with uncompacted instructions hex value + */ +fprintf(out, "%*c", blank_spaces, ' '); + } + + brw_uncompact_instruction(devinfo, , compacted); + insn = + offset += 8; } else { -if (dump_hex) { - fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ", - ((uint32_t *)insn)[3], - ((uint32_t *)insn)[2], - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); -} -offset += 16; + if (dump_hex) { +unsigned char * insn_ptr = ((unsigned char *)[0]); +for (int i = 0 ; i < 16; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); +} + } + offset += 16; } brw_disassemble_inst(out, devinfo, insn, compacted); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 3/7] mesa: Add support for AMD_depth_clamp_separate
Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, as suggested by Marek Olsak. Driver that enables AMD_depth_clamp_separate will only ever look at DepthClampNear and DepthClampFar, as suggested by Ian Romanick. v2: 1) Remove unnecessary parentheses (Marek Olsak) 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE GL_DEPTH_CLAMP only (Marek Olsak) 3) Clamp against near and far plane separately (Marek Olsak) 4) Clip point separately for near and far Z clipping plane (Marek Olsak) v3: Clamp raster position zw to the range [min(n,f), 0] for near plane and [0, max(n,f)] for far plane (Marek Olsak) v4: Use MIN2 and MAX2 instead of CLAMP (Marek Olsak) Signed-off-by: Sagar Ghuge Reviewed-by: Ian Romanick Reviewed-by: Marek Olšák --- src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++-- src/mesa/main/attrib.c| 40 +++--- src/mesa/main/enable.c| 9 ++- src/mesa/main/get.c | 4 ++ src/mesa/main/get_hash_params.py | 2 +- src/mesa/main/mtypes.h| 1 - src/mesa/main/rastpos.c | 55 --- src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- src/mesa/swrast/s_span.c | 2 +- src/mesa/tnl/t_vb_program.c | 6 +- src/mesa/tnl/t_vb_vertex.c| 8 ++- 12 files changed, 111 insertions(+), 33 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index c051848985..dc54cb67af 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.ScreenSpaceViewportYMax = 1; clip.ViewportXYClipTestEnable = true; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); /* _NEW_TRANSFORM */ if (GEN_GEN == 5 || GEN_IS_G4X) { @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.UserClipDistanceCullTestEnableBitmask = brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); #endif /* _NEW_LIGHT */ @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) for (unsigned i = 0; i < viewport_count; i++) { /* _NEW_VIEWPORT | _NEW_TRANSFORM */ const struct gl_viewport_attrib *vp = >ViewportArray[i]; - if (ctx->Transform.DepthClamp) { + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ - if (!ctx->Transform.DepthClamp) { + if (!(ctx->Transform.DepthClampNear && +ctx->Transform.DepthClampFar)) { #if GEN_GEN >= 9 raster.ViewportZFarClipTestEnable = true; raster.ViewportZNearClipTestEnable = true; diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c index cbe93ab6fa..a46fec73fd 100644 --- a/src/mesa/main/attrib.c +++ b/src/mesa/main/attrib.c @@ -72,7 +72,8 @@ struct gl_enable_attrib GLbitfield ClipPlanes; GLboolean ColorMaterial; GLboolean CullFace; - GLboolean DepthClamp; + GLboolean DepthClampNear; + GLboolean DepthClampFar; GLboolean DepthTest; GLboolean Dither; GLboolean Fog; @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask) attr->ClipPlanes = ctx->Transform.ClipPlanesEnabled; attr->ColorMaterial = ctx->Light.ColorMaterialEnabled; attr->CullFace = ctx->Polygon.CullFlag; - attr->DepthClamp = ctx->Transform.DepthClamp; + attr->DepthClampNear = ctx->Transform.DepthClampNear; + attr->DepthClampFar = ctx->Transform.DepthClampFar; attr->DepthTest = ctx->Depth.Test; attr->Dither = ctx->Color.DitherFlag; attr->Fog = ctx->Fog.Enabled; @@ -627,8 +629,18 @@ pop_enable_group(struct gl_context *ctx, const struct gl_enable_attrib *enable) TEST_AND_UPDATE(ctx->Light.ColorMaterialEnabled, enable->ColorMaterial, GL_
Re: [Mesa-dev] [PATCH v3 3/7] mesa: Add support for AMD_depth_clamp_separate
Thank you for reviewing the patch. On 08/23/2018 06:17 PM, Marek Olšák wrote: > On Thu, Aug 23, 2018 at 8:18 PM, Sagar Ghuge wrote: >> Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle >> GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. >> >> Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, >> as suggested by Marek Olsak. >> >> Driver that enables AMD_depth_clamp_separate will only ever look at >> DepthClampNear and DepthClampFar, as suggested by Ian Romanick. >> >> v2: 1) Remove unnecessary parentheses (Marek Olsak) >> 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE >>GL_DEPTH_CLAMP only (Marek Olsak) >> 3) Clamp against near and far plane separately (Marek Olsak) >> 4) Clip point separately for near and far Z clipping plane (Marek >>Olsak) >> >> v3: Clamp raster position zw to the range [min(n,f), 0] for near plane >> and [0, max(n,f)] for far plane (Marek Olsak) >> >> Signed-off-by: Sagar Ghuge >> Reviewed-by: Ian Romanick >> --- >> src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++-- >> src/mesa/main/attrib.c| 40 ++--- >> src/mesa/main/enable.c| 9 ++- >> src/mesa/main/get.c | 4 ++ >> src/mesa/main/get_hash_params.py | 2 +- >> src/mesa/main/mtypes.h| 1 - >> src/mesa/main/rastpos.c | 59 --- >> src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- >> src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- >> src/mesa/swrast/s_span.c | 2 +- >> src/mesa/tnl/t_vb_program.c | 6 +- >> src/mesa/tnl/t_vb_vertex.c| 8 ++- >> 12 files changed, 115 insertions(+), 33 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c >> b/src/mesa/drivers/dri/i965/genX_state_upload.c >> index c051848985..dc54cb67af 100644 >> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c >> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c >> @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) >>clip.ScreenSpaceViewportYMax = 1; >> >>clip.ViewportXYClipTestEnable = true; >> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; >> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && >> + ctx->Transform.DepthClampFar); >> >>/* _NEW_TRANSFORM */ >>if (GEN_GEN == 5 || GEN_IS_G4X) { >> @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) >>clip.UserClipDistanceCullTestEnableBitmask = >> brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; >> >> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; >> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && >> + ctx->Transform.DepthClampFar); >> #endif >> >>/* _NEW_LIGHT */ >> @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) >> for (unsigned i = 0; i < viewport_count; i++) { >>/* _NEW_VIEWPORT | _NEW_TRANSFORM */ >>const struct gl_viewport_attrib *vp = >ViewportArray[i]; >> - if (ctx->Transform.DepthClamp) { >> + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { >> ccv.MinimumDepth = MIN2(vp->Near, vp->Far); >> ccv.MaximumDepth = MAX2(vp->Near, vp->Far); >>} else { >> @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) >>raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; >> >>/* _NEW_TRANSFORM */ >> - if (!ctx->Transform.DepthClamp) { >> + if (!(ctx->Transform.DepthClampNear && >> +ctx->Transform.DepthClampFar)) { >> #if GEN_GEN >= 9 >> raster.ViewportZFarClipTestEnable = true; >> raster.ViewportZNearClipTestEnable = true; >> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c >> index cbe93ab6fa..a46fec73fd 100644 >> --- a/src/mesa/main/attrib.c >> +++ b/src/mesa/main/attrib.c >> @@ -72,7 +72,8 @@ struct gl_enable_attrib >> GLbitfield ClipPlanes; >> GLboolean ColorMaterial; >> GLboolean CullFace; >> - GLboolean DepthClamp; >> + GLboolean DepthClampNear; >> + G
[Mesa-dev] [PATCH v3 3/7] mesa: Add support for AMD_depth_clamp_separate
Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, as suggested by Marek Olsak. Driver that enables AMD_depth_clamp_separate will only ever look at DepthClampNear and DepthClampFar, as suggested by Ian Romanick. v2: 1) Remove unnecessary parentheses (Marek Olsak) 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE GL_DEPTH_CLAMP only (Marek Olsak) 3) Clamp against near and far plane separately (Marek Olsak) 4) Clip point separately for near and far Z clipping plane (Marek Olsak) v3: Clamp raster position zw to the range [min(n,f), 0] for near plane and [0, max(n,f)] for far plane (Marek Olsak) Signed-off-by: Sagar Ghuge Reviewed-by: Ian Romanick --- src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++-- src/mesa/main/attrib.c| 40 ++--- src/mesa/main/enable.c| 9 ++- src/mesa/main/get.c | 4 ++ src/mesa/main/get_hash_params.py | 2 +- src/mesa/main/mtypes.h| 1 - src/mesa/main/rastpos.c | 59 --- src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- src/mesa/swrast/s_span.c | 2 +- src/mesa/tnl/t_vb_program.c | 6 +- src/mesa/tnl/t_vb_vertex.c| 8 ++- 12 files changed, 115 insertions(+), 33 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index c051848985..dc54cb67af 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.ScreenSpaceViewportYMax = 1; clip.ViewportXYClipTestEnable = true; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); /* _NEW_TRANSFORM */ if (GEN_GEN == 5 || GEN_IS_G4X) { @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.UserClipDistanceCullTestEnableBitmask = brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); #endif /* _NEW_LIGHT */ @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) for (unsigned i = 0; i < viewport_count; i++) { /* _NEW_VIEWPORT | _NEW_TRANSFORM */ const struct gl_viewport_attrib *vp = >ViewportArray[i]; - if (ctx->Transform.DepthClamp) { + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ - if (!ctx->Transform.DepthClamp) { + if (!(ctx->Transform.DepthClampNear && +ctx->Transform.DepthClampFar)) { #if GEN_GEN >= 9 raster.ViewportZFarClipTestEnable = true; raster.ViewportZNearClipTestEnable = true; diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c index cbe93ab6fa..a46fec73fd 100644 --- a/src/mesa/main/attrib.c +++ b/src/mesa/main/attrib.c @@ -72,7 +72,8 @@ struct gl_enable_attrib GLbitfield ClipPlanes; GLboolean ColorMaterial; GLboolean CullFace; - GLboolean DepthClamp; + GLboolean DepthClampNear; + GLboolean DepthClampFar; GLboolean DepthTest; GLboolean Dither; GLboolean Fog; @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask) attr->ClipPlanes = ctx->Transform.ClipPlanesEnabled; attr->ColorMaterial = ctx->Light.ColorMaterialEnabled; attr->CullFace = ctx->Polygon.CullFlag; - attr->DepthClamp = ctx->Transform.DepthClamp; + attr->DepthClampNear = ctx->Transform.DepthClampNear; + attr->DepthClampFar = ctx->Transform.DepthClampFar; attr->DepthTest = ctx->Depth.Test; attr->Dither = ctx->Color.DitherFlag; attr->Fog = ctx->Fog.Enabled; @@ -627,8 +629,18 @@ pop_enable_group(struct gl_context *ctx, const struct gl_enable_attrib *enable) TEST_AND_UPDATE(ctx->Light.ColorMaterialEnabled, enable->ColorMaterial, GL_COLOR_MATERIAL); TEST_AND_UPDATE(ctx->Polygon.
Re: [Mesa-dev] [PATCH v2 3/7] mesa: Add support for AMD_depth_clamp_separate
On 08/22/2018 04:31 PM, Marek Olšák wrote: > On Wed, Aug 22, 2018 at 5:52 PM Sagar Ghuge wrote: >> >> Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle >> GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. >> >> Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, >> as suggested by Marek Olsak. >> >> Driver that enables AMD_depth_clamp_separate will only ever look at >> DepthClampNear and DepthClampFar, as suggested by Ian Romanick. >> >> v2: 1) Remove unnecessary parentheses (Marek Olsak) >> 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE >>GL_DEPTH_CLAMP only (Marek Olsak) >> 3) Clamp against near and far plane separately (Marek Olsak) >> 4) Clip point separately for near and far Z clipping plane (Marek >>Olsak) >> >> Signed-off-by: Sagar Ghuge >> --- >> src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++-- >> src/mesa/main/attrib.c| 40 +++--- >> src/mesa/main/enable.c| 9 ++- >> src/mesa/main/get.c | 4 ++ >> src/mesa/main/get_hash_params.py | 2 +- >> src/mesa/main/mtypes.h| 1 - >> src/mesa/main/rastpos.c | 55 --- >> src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- >> src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- >> src/mesa/swrast/s_span.c | 2 +- >> src/mesa/tnl/t_vb_program.c | 6 +- >> src/mesa/tnl/t_vb_vertex.c| 8 ++- >> 12 files changed, 111 insertions(+), 33 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c >> b/src/mesa/drivers/dri/i965/genX_state_upload.c >> index c051848985..dc54cb67af 100644 >> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c >> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c >> @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) >>clip.ScreenSpaceViewportYMax = 1; >> >>clip.ViewportXYClipTestEnable = true; >> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; >> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && >> + ctx->Transform.DepthClampFar); >> >>/* _NEW_TRANSFORM */ >>if (GEN_GEN == 5 || GEN_IS_G4X) { >> @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) >>clip.UserClipDistanceCullTestEnableBitmask = >> brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; >> >> - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; >> + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && >> + ctx->Transform.DepthClampFar); >> #endif >> >>/* _NEW_LIGHT */ >> @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) >> for (unsigned i = 0; i < viewport_count; i++) { >>/* _NEW_VIEWPORT | _NEW_TRANSFORM */ >>const struct gl_viewport_attrib *vp = >ViewportArray[i]; >> - if (ctx->Transform.DepthClamp) { >> + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { >> ccv.MinimumDepth = MIN2(vp->Near, vp->Far); >> ccv.MaximumDepth = MAX2(vp->Near, vp->Far); >>} else { >> @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) >>raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; >> >>/* _NEW_TRANSFORM */ >> - if (!ctx->Transform.DepthClamp) { >> + if (!(ctx->Transform.DepthClampNear && >> +ctx->Transform.DepthClampFar)) { >> #if GEN_GEN >= 9 >> raster.ViewportZFarClipTestEnable = true; >> raster.ViewportZNearClipTestEnable = true; >> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c >> index cbe93ab6fa..a46fec73fd 100644 >> --- a/src/mesa/main/attrib.c >> +++ b/src/mesa/main/attrib.c >> @@ -72,7 +72,8 @@ struct gl_enable_attrib >> GLbitfield ClipPlanes; >> GLboolean ColorMaterial; >> GLboolean CullFace; >> - GLboolean DepthClamp; >> + GLboolean DepthClampNear; >> + GLboolean DepthClampFar; >> GLboolean DepthTest; >> GLboolean Dither; >> GLboolean Fog; >> @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask) >>attr->
[Mesa-dev] [PATCH v2 3/7] mesa: Add support for AMD_depth_clamp_separate
Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, as suggested by Marek Olsak. Driver that enables AMD_depth_clamp_separate will only ever look at DepthClampNear and DepthClampFar, as suggested by Ian Romanick. v2: 1) Remove unnecessary parentheses (Marek Olsak) 2) if AMD_depth_clamp_separate is unsupported, TEST_AND_UPDATE GL_DEPTH_CLAMP only (Marek Olsak) 3) Clamp against near and far plane separately (Marek Olsak) 4) Clip point separately for near and far Z clipping plane (Marek Olsak) Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++-- src/mesa/main/attrib.c| 40 +++--- src/mesa/main/enable.c| 9 ++- src/mesa/main/get.c | 4 ++ src/mesa/main/get_hash_params.py | 2 +- src/mesa/main/mtypes.h| 1 - src/mesa/main/rastpos.c | 55 --- src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- src/mesa/swrast/s_span.c | 2 +- src/mesa/tnl/t_vb_program.c | 6 +- src/mesa/tnl/t_vb_vertex.c| 8 ++- 12 files changed, 111 insertions(+), 33 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index c051848985..dc54cb67af 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.ScreenSpaceViewportYMax = 1; clip.ViewportXYClipTestEnable = true; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); /* _NEW_TRANSFORM */ if (GEN_GEN == 5 || GEN_IS_G4X) { @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.UserClipDistanceCullTestEnableBitmask = brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); #endif /* _NEW_LIGHT */ @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) for (unsigned i = 0; i < viewport_count; i++) { /* _NEW_VIEWPORT | _NEW_TRANSFORM */ const struct gl_viewport_attrib *vp = >ViewportArray[i]; - if (ctx->Transform.DepthClamp) { + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ - if (!ctx->Transform.DepthClamp) { + if (!(ctx->Transform.DepthClampNear && +ctx->Transform.DepthClampFar)) { #if GEN_GEN >= 9 raster.ViewportZFarClipTestEnable = true; raster.ViewportZNearClipTestEnable = true; diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c index cbe93ab6fa..a46fec73fd 100644 --- a/src/mesa/main/attrib.c +++ b/src/mesa/main/attrib.c @@ -72,7 +72,8 @@ struct gl_enable_attrib GLbitfield ClipPlanes; GLboolean ColorMaterial; GLboolean CullFace; - GLboolean DepthClamp; + GLboolean DepthClampNear; + GLboolean DepthClampFar; GLboolean DepthTest; GLboolean Dither; GLboolean Fog; @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask) attr->ClipPlanes = ctx->Transform.ClipPlanesEnabled; attr->ColorMaterial = ctx->Light.ColorMaterialEnabled; attr->CullFace = ctx->Polygon.CullFlag; - attr->DepthClamp = ctx->Transform.DepthClamp; + attr->DepthClampNear = ctx->Transform.DepthClampNear; + attr->DepthClampFar = ctx->Transform.DepthClampFar; attr->DepthTest = ctx->Depth.Test; attr->Dither = ctx->Color.DitherFlag; attr->Fog = ctx->Fog.Enabled; @@ -627,8 +629,18 @@ pop_enable_group(struct gl_context *ctx, const struct gl_enable_attrib *enable) TEST_AND_UPDATE(ctx->Light.ColorMaterialEnabled, enable->ColorMaterial, GL_COLOR_MATERIAL); TEST_AND_UPDATE(ctx->Polygon.CullFlag, enable->CullFace, GL_CULL_FACE); - TEST_AND_UPDATE(ctx->Transform.DepthClamp, enable->DepthClamp, - GL_DEPTH_CLAMP); + + if (!ctx-&g
[Mesa-dev] [PATCH v2 4/7] mesa: add support for GL_AMD_depth_clamp_separate tokens
_mesa_set_enable() and _mesa_IsEnabled() extended to accept new two tokens GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD. v2: Remove unnecessary parentheses (Marek Olsak) Signed-off-by: Sagar Ghuge --- src/mesa/main/enable.c | 36 1 file changed, 36 insertions(+) diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c index 4bde9052bc..042b9d1f0a 100644 --- a/src/mesa/main/enable.c +++ b/src/mesa/main/enable.c @@ -1017,6 +1017,30 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state) ctx->Transform.DepthClampFar = state; break; + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampNear == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; + ctx->Transform.DepthClampNear = state; + break; + + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampFar == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; + ctx->Transform.DepthClampFar = state; + break; + case GL_FRAGMENT_SHADER_ATI: if (ctx->API != API_OPENGL_COMPAT) goto invalid_enum_error; @@ -1689,6 +1713,18 @@ _mesa_IsEnabled( GLenum cap ) return (ctx->Transform.DepthClampNear || ctx->Transform.DepthClampFar); + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return ctx->Transform.DepthClampNear; + + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return ctx->Transform.DepthClampFar; + case GL_FRAGMENT_SHADER_ATI: if (ctx->API != API_OPENGL_COMPAT) goto invalid_enum_error; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 6/7] i965: add functional changes for AMD_depth_clamp_separate
Gen >= 9 have ability to control clamping of depth values separately at near and far plane. z_w is clamped to the range [min(n,f), 0] if clamping at near plane is enabled, [0, max(n,f)] if clamping at far plane is enabled and [min(n,f) max(n,f)] if clamping at both plane is enabled. v2: 1) Use better coding style (Ian Romanick) Signed-off-by: Sagar Ghuge Reviewed-by: Ian Romanick --- src/mesa/drivers/dri/i965/genX_state_upload.c | 20 ++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index dc54cb67af..b49c5839b5 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -2343,6 +2343,12 @@ genX(upload_cc_viewport)(struct brw_context *brw) if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); + } else if (ctx->Transform.DepthClampNear) { + ccv.MinimumDepth = MIN2(vp->Near, vp->Far); + ccv.MaximumDepth = 0.0; + } else if (ctx->Transform.DepthClampFar) { + ccv.MinimumDepth = 0.0; + ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { ccv.MinimumDepth = 0.0; ccv.MaximumDepth = 1.0; @@ -4607,15 +4613,19 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ +#if GEN_GEN < 9 if (!(ctx->Transform.DepthClampNear && -ctx->Transform.DepthClampFar)) { +ctx->Transform.DepthClampFar)) + raster.ViewportZClipTestEnable = true; +#endif + #if GEN_GEN >= 9 - raster.ViewportZFarClipTestEnable = true; + if (!ctx->Transform.DepthClampNear) raster.ViewportZNearClipTestEnable = true; -#else - raster.ViewportZClipTestEnable = true; + + if (!ctx->Transform.DepthClampFar) + raster.ViewportZFarClipTestEnable = true; #endif - } /* BRW_NEW_CONSERVATIVE_RASTERIZATION */ #if GEN_GEN >= 9 -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/7] mesa: Add types for AMD_depth_clamp_separate.
Add some basic types and storage for the AMD_depth_clamp_separate extension. v2: 1) Drop unnecessary definition (Marek Olsak) 2) Expose extension in compatibility profile (Marek Olsak) Signed-off-by: Sagar Ghuge --- src/mesa/main/extensions_table.h | 1 + src/mesa/main/mtypes.h | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index 746e821886..b7ff437032 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -9,6 +9,7 @@ EXT(3DFX_texture_compression_FXT1 , TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) EXT(AMD_conservative_depth , ARB_conservative_depth , GLL, GLC, x , x , 2009) +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate , GLL, GLC, x , x , 2009) EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend , GLL, GLC, x , x , 2009) EXT(AMD_framebuffer_multisample_advanced, AMD_framebuffer_multisample_advanced , GLL, GLC, x , ES2, 2018) EXT(AMD_performance_monitor , AMD_performance_monitor , GLL, GLC, x , ES2, 2007) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 50ef898fc4..3df657cc5d 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1283,6 +1283,8 @@ struct gl_transform_attrib GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */ GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */ GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ + GLboolean DepthClampNear; /**< GL_AMD_depth_clamp_separate */ + GLboolean DepthClampFar;/**< GL_AMD_depth_clamp_separate */ /** GL_ARB_clip_control */ GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ @@ -4256,6 +4258,7 @@ struct gl_extensions GLboolean OES_viewport_array; /* vendor extensions */ GLboolean AMD_framebuffer_multisample_advanced; + GLboolean AMD_depth_clamp_separate; GLboolean AMD_performance_monitor; GLboolean AMD_pinned_memory; GLboolean AMD_seamless_cubemap_per_texture; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/7]] mesa: Add types for AMD_depth_clamp_separate.
Please ignore this patch. Subject contains extra bracket. I will resend the patch. On 08/22/2018 12:54 PM, Sagar Ghuge wrote: > Add some basic types and storage for the AMD_depth_clamp_separate > extension. > > v2: 1) Drop unnecessary definition (Marek Olsak) > 2) Expose extension in compatibility profile (Marek Olsak) > > Signed-off-by: Sagar Ghuge > --- > src/mesa/main/extensions_table.h | 1 + > src/mesa/main/mtypes.h | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/src/mesa/main/extensions_table.h > b/src/mesa/main/extensions_table.h > index 746e821886..b7ff437032 100644 > --- a/src/mesa/main/extensions_table.h > +++ b/src/mesa/main/extensions_table.h > @@ -9,6 +9,7 @@ > EXT(3DFX_texture_compression_FXT1 , TDFX_texture_compression_FXT1 > , GLL, GLC, x , x , 1999) > > EXT(AMD_conservative_depth , ARB_conservative_depth > , GLL, GLC, x , x , 2009) > +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate > , GLL, GLC, x , x , 2009) > EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend > , GLL, GLC, x , x , 2009) > EXT(AMD_framebuffer_multisample_advanced, > AMD_framebuffer_multisample_advanced , GLL, GLC, x , ES2, 2018) > EXT(AMD_performance_monitor , AMD_performance_monitor > , GLL, GLC, x , ES2, 2007) > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h > index 50ef898fc4..3df657cc5d 100644 > --- a/src/mesa/main/mtypes.h > +++ b/src/mesa/main/mtypes.h > @@ -1283,6 +1283,8 @@ struct gl_transform_attrib > GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */ > GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */ > GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ > + GLboolean DepthClampNear; /**< > GL_AMD_depth_clamp_separate */ > + GLboolean DepthClampFar; /**< > GL_AMD_depth_clamp_separate */ > /** GL_ARB_clip_control */ > GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ > GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ > @@ -4256,6 +4258,7 @@ struct gl_extensions > GLboolean OES_viewport_array; > /* vendor extensions */ > GLboolean AMD_framebuffer_multisample_advanced; > + GLboolean AMD_depth_clamp_separate; > GLboolean AMD_performance_monitor; > GLboolean AMD_pinned_memory; > GLboolean AMD_seamless_cubemap_per_texture; > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 2/7]] mesa: Add types for AMD_depth_clamp_separate.
Add some basic types and storage for the AMD_depth_clamp_separate extension. v2: 1) Drop unnecessary definition (Marek Olsak) 2) Expose extension in compatibility profile (Marek Olsak) Signed-off-by: Sagar Ghuge --- src/mesa/main/extensions_table.h | 1 + src/mesa/main/mtypes.h | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index 746e821886..b7ff437032 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -9,6 +9,7 @@ EXT(3DFX_texture_compression_FXT1 , TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) EXT(AMD_conservative_depth , ARB_conservative_depth , GLL, GLC, x , x , 2009) +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate , GLL, GLC, x , x , 2009) EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend , GLL, GLC, x , x , 2009) EXT(AMD_framebuffer_multisample_advanced, AMD_framebuffer_multisample_advanced , GLL, GLC, x , ES2, 2018) EXT(AMD_performance_monitor , AMD_performance_monitor , GLL, GLC, x , ES2, 2007) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 50ef898fc4..3df657cc5d 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1283,6 +1283,8 @@ struct gl_transform_attrib GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */ GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */ GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ + GLboolean DepthClampNear; /**< GL_AMD_depth_clamp_separate */ + GLboolean DepthClampFar;/**< GL_AMD_depth_clamp_separate */ /** GL_ARB_clip_control */ GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ @@ -4256,6 +4258,7 @@ struct gl_extensions GLboolean OES_viewport_array; /* vendor extensions */ GLboolean AMD_framebuffer_multisample_advanced; + GLboolean AMD_depth_clamp_separate; GLboolean AMD_performance_monitor; GLboolean AMD_pinned_memory; GLboolean AMD_seamless_cubemap_per_texture; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/7] mesa: add support for GL_AMD_depth_clamp_separate tokens
On 08/21/2018 09:28 PM, Marek Olšák wrote: > On Tue, Aug 21, 2018 at 8:02 PM Sagar Ghuge wrote: >> >> _mesa_set_enable() and _mesa_IsEnabled() extended to accept new two >> tokens GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/mesa/main/enable.c | 36 >> 1 file changed, 36 insertions(+) >> >> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c >> index 4bde9052bc..66e224676a 100644 >> --- a/src/mesa/main/enable.c >> +++ b/src/mesa/main/enable.c >> @@ -1017,6 +1017,30 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, >> GLboolean state) >> ctx->Transform.DepthClampFar = state; >> break; >> >> + case GL_DEPTH_CLAMP_NEAR_AMD: >> + if (!_mesa_is_desktop_gl(ctx)) >> +goto invalid_enum_error; >> + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); >> + if (ctx->Transform.DepthClampNear == state) >> +return; >> + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : >> + _NEW_TRANSFORM); >> + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; >> + ctx->Transform.DepthClampNear = state; >> + break; >> + >> + case GL_DEPTH_CLAMP_FAR_AMD: >> + if (!_mesa_is_desktop_gl(ctx)) >> +goto invalid_enum_error; >> + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); >> + if (ctx->Transform.DepthClampFar == state) >> +return; >> + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : >> + _NEW_TRANSFORM); >> + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; >> + ctx->Transform.DepthClampFar = state; >> + break; >> + >>case GL_FRAGMENT_SHADER_ATI: >> if (ctx->API != API_OPENGL_COMPAT) >> goto invalid_enum_error; >> @@ -1689,6 +1713,18 @@ _mesa_IsEnabled( GLenum cap ) >> return (ctx->Transform.DepthClampNear || >> ctx->Transform.DepthClampFar); >> >> + case GL_DEPTH_CLAMP_NEAR_AMD: >> + if (!_mesa_is_desktop_gl(ctx)) >> +goto invalid_enum_error; >> + CHECK_EXTENSION(AMD_depth_clamp_separate); >> + return (ctx->Transform.DepthClampNear); >> + >> + case GL_DEPTH_CLAMP_FAR_AMD: >> + if (!_mesa_is_desktop_gl(ctx)) >> +goto invalid_enum_error; >> + CHECK_EXTENSION(AMD_depth_clamp_separate); >> + return (ctx->Transform.DepthClampFar); > > I'm not really digging these unnecessary parentheses. I should not copy/paste statements. Sorry about that. I will fix those and send v2. Thanks for reviewing all the patches. > > Marek > >> + >>case GL_FRAGMENT_SHADER_ATI: >> if (ctx->API != API_OPENGL_COMPAT) >> goto invalid_enum_error; >> -- >> 2.17.1 >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/7] mesa: add EXTRA_EXT for AMD_depth_clamp_separate
Signed-off-by: Sagar Ghuge Reviewed-by: Ian Romanick --- src/mesa/main/get.c | 1 + src/mesa/main/get_hash_params.py | 5 + 2 files changed, 6 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index 6b2e3637e6..92020e29a9 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -468,6 +468,7 @@ EXTRA_EXT(NV_texture_rectangle); EXTRA_EXT(EXT_stencil_two_side); EXTRA_EXT(EXT_depth_bounds_test); EXTRA_EXT(ARB_depth_clamp); +EXTRA_EXT(AMD_depth_clamp_separate); EXTRA_EXT(ATI_fragment_shader); EXTRA_EXT(EXT_provoking_vertex); EXTRA_EXT(ARB_fragment_shader); diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py index ae1fc6f062..2e1f4c007c 100644 --- a/src/mesa/main/get_hash_params.py +++ b/src/mesa/main/get_hash_params.py @@ -989,6 +989,11 @@ descriptor=[ # GL_ARB_indirect_parameters [ "PARAMETER_BUFFER_BINDING_ARB", "LOC_CUSTOM, TYPE_INT, 0, extra_ARB_indirect_parameters" ], + +# GL 4.1 +# GL_AMD_depth_clamp_separate + [ "DEPTH_CLAMP_NEAR_AMD", "CONTEXT_BOOL(Transform.DepthClampNear), extra_AMD_depth_clamp_separate" ], + [ "DEPTH_CLAMP_FAR_AMD", "CONTEXT_BOOL(Transform.DepthClampFar), extra_AMD_depth_clamp_separate" ], ]}, ] -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] i965: add functional changes for AMD_depth_clamp_separate
Gen >= 9 have ability to control clamping of depth values separately at near and far plane. z_w is clamped to the range [min(n,f), 0] if clamping at near plane is enabled, [0, max(n,f)] if clamping at far plane is enabled and [min(n,f) max(n,f)] if clamping at both plane is enabled. Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/genX_state_upload.c | 23 +++ 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index dc54cb67af..4e978bed91 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -2343,7 +2343,13 @@ genX(upload_cc_viewport)(struct brw_context *brw) if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); - } else { + } else if (ctx->Transform.DepthClampNear) { + ccv.MinimumDepth = MIN2(vp->Near, vp->Far); + ccv.MaximumDepth = 0.0; + } else if (ctx->Transform.DepthClampFar) { + ccv.MinimumDepth = 0.0; + ccv.MaximumDepth = MAX2(vp->Near, vp->Far); + }else { ccv.MinimumDepth = 0.0; ccv.MaximumDepth = 1.0; } @@ -4607,16 +4613,23 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ +#if GEN_GEN < 9 if (!(ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar)) { -#if GEN_GEN >= 9 - raster.ViewportZFarClipTestEnable = true; - raster.ViewportZNearClipTestEnable = true; -#else raster.ViewportZClipTestEnable = true; + } #endif + +#if GEN_GEN >= 9 + if (!ctx->Transform.DepthClampNear) { + raster.ViewportZNearClipTestEnable = true; } + if (!ctx->Transform.DepthClampFar) { + raster.ViewportZFarClipTestEnable = true; + } +#endif + /* BRW_NEW_CONSERVATIVE_RASTERIZATION */ #if GEN_GEN >= 9 raster.ConservativeRasterizationEnable = -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/7] Implement AMD_depth_clamp_separate extension
This patch series implements AMD_depth_clamp_separate extension. Previously I sent patches for comments and based on Ian and Marek's suggestions, I did some changes. 1) Get rid of DepthClamp as DepthClampNear + DepthClampFar replaces it (Marek Olsak & Ian Romanick) 2) Make sure that patch series should build at every commit (Ian Romanick) 3) Change the sequence of patches (Ian Romanick) 4) Wrote a piglit test, in order to test the extension behavior. It is also available as a branch on GitLab : https://gitlab.freedesktop.org/sagarghuge/piglit/commits/amd_depth_clamp_separate This series is also available as a branch on GitLab: https://gitlab.freedesktop.org/sagarghuge/mesa/commits/ext_amd_depth_clamp_sep Sagar Ghuge (7): glapi: define AMD_depth_clamp_separate mesa: Add types for AMD_depth_clamp_separate. mesa: Add support for AMD_depth_clamp_separate mesa: add support for GL_AMD_depth_clamp_separate tokens mesa: add EXTRA_EXT for AMD_depth_clamp_separate i965: add functional changes for AMD_depth_clamp_separate i965: enable AMD_depth_clamp_separate include/GL/glcorearb.h| 2 + .../glapi/gen/AMD_depth_clamp_separate.xml| 15 +++ src/mapi/glapi/gen/Makefile.am| 1 + src/mapi/glapi/gen/gl_API.xml | 3 ++ src/mapi/glapi/gen/meson.build| 1 + src/mesa/drivers/dri/i965/genX_state_upload.c | 34 ++ src/mesa/drivers/dri/i965/intel_extensions.c | 1 + src/mesa/main/attrib.c| 21 ++--- src/mesa/main/enable.c| 45 +-- src/mesa/main/extensions_table.h | 1 + src/mesa/main/get.c | 5 +++ src/mesa/main/get_hash_params.py | 7 ++- src/mesa/main/mtypes.h| 4 +- src/mesa/main/rastpos.c | 6 ++- src/mesa/state_tracker/st_atom_rasterizer.c | 3 +- src/mesa/state_tracker/st_cb_drawpixels.c | 3 +- src/mesa/swrast/s_span.c | 2 +- src/mesa/tnl/t_vb_program.c | 6 ++- src/mesa/tnl/t_vb_vertex.c| 8 ++-- 19 files changed, 137 insertions(+), 31 deletions(-) create mode 100644 src/mapi/glapi/gen/AMD_depth_clamp_separate.xml -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/7] mesa: Add types for AMD_depth_clamp_separate.
Add some basic types and storage for the AMD_depth_clamp_separate extension. Signed-off-by: Sagar Ghuge --- include/GL/glcorearb.h | 2 ++ src/mesa/main/extensions_table.h | 1 + src/mesa/main/mtypes.h | 3 +++ 3 files changed, 6 insertions(+) diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h index 3cf945c8b2..7dc8b4e79b 100644 --- a/include/GL/glcorearb.h +++ b/include/GL/glcorearb.h @@ -1558,6 +1558,8 @@ typedef int64_t GLint64; #define GL_MAX_FRAGMENT_INPUT_COMPONENTS 0x9125 #define GL_CONTEXT_PROFILE_MASK 0x9126 #define GL_DEPTH_CLAMP0x864F +#define GL_DEPTH_CLAMP_NEAR_AMD 0x901E +#define GL_DEPTH_CLAMP_FAR_AMD0x901F #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C #define GL_FIRST_VERTEX_CONVENTION0x8E4D #define GL_LAST_VERTEX_CONVENTION 0x8E4E diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index 746e821886..935da4943b 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -9,6 +9,7 @@ EXT(3DFX_texture_compression_FXT1 , TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) EXT(AMD_conservative_depth , ARB_conservative_depth , GLL, GLC, x , x , 2009) +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate , x , GLC, x , x , 2009) EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend , GLL, GLC, x , x , 2009) EXT(AMD_framebuffer_multisample_advanced, AMD_framebuffer_multisample_advanced , GLL, GLC, x , ES2, 2018) EXT(AMD_performance_monitor , AMD_performance_monitor , GLL, GLC, x , ES2, 2007) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 50ef898fc4..3df657cc5d 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1283,6 +1283,8 @@ struct gl_transform_attrib GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */ GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */ GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ + GLboolean DepthClampNear; /**< GL_AMD_depth_clamp_separate */ + GLboolean DepthClampFar;/**< GL_AMD_depth_clamp_separate */ /** GL_ARB_clip_control */ GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ @@ -4256,6 +4258,7 @@ struct gl_extensions GLboolean OES_viewport_array; /* vendor extensions */ GLboolean AMD_framebuffer_multisample_advanced; + GLboolean AMD_depth_clamp_separate; GLboolean AMD_performance_monitor; GLboolean AMD_pinned_memory; GLboolean AMD_seamless_cubemap_per_texture; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] i965: enable AMD_depth_clamp_separate
Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/intel_extensions.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c index f1c3aeff13..f07d3ab601 100644 --- a/src/mesa/drivers/dri/i965/intel_extensions.c +++ b/src/mesa/drivers/dri/i965/intel_extensions.c @@ -302,6 +302,7 @@ intelInitExtensions(struct gl_context *ctx) if (devinfo->gen >= 9) { ctx->Extensions.ANDROID_extension_pack_es31a = true; + ctx->Extensions.AMD_depth_clamp_separate = true; ctx->Extensions.ARB_shader_stencil_export = true; ctx->Extensions.KHR_blend_equation_advanced_coherent = true; ctx->Extensions.KHR_texture_compression_astc_ldr = true; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/7] glapi: define AMD_depth_clamp_separate
Signed-off-by: Sagar Ghuge Reviewed-by: Ian Romanick --- src/mapi/glapi/gen/AMD_depth_clamp_separate.xml | 15 +++ src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml | 3 +++ src/mapi/glapi/gen/meson.build | 1 + 4 files changed, 20 insertions(+) create mode 100644 src/mapi/glapi/gen/AMD_depth_clamp_separate.xml diff --git a/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml new file mode 100644 index 00..b7142f1b7b --- /dev/null +++ b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index 93acabd968..ab369a0d33 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -188,6 +188,7 @@ API_XML = \ ARB_vertex_attrib_64bit.xml \ ARB_vertex_attrib_binding.xml \ ARB_viewport_array.xml \ + AMD_depth_clamp_separate.xml \ AMD_draw_buffers_blend.xml \ AMD_performance_monitor.xml \ ARB_vertex_type_2_10_10_10_rev.xml \ diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index 95680f1d46..b98af4a1cc 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -12939,6 +12939,9 @@ http://www.w3.org/2001/XInclude"/> +http://www.w3.org/2001/XInclude"/> + diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build index b148627f71..c638b1ece6 100644 --- a/src/mapi/glapi/gen/meson.build +++ b/src/mapi/glapi/gen/meson.build @@ -95,6 +95,7 @@ api_xml_files = files( 'ARB_vertex_attrib_64bit.xml', 'ARB_vertex_attrib_binding.xml', 'ARB_viewport_array.xml', + 'AMD_depth_clamp_separate.xml', 'AMD_draw_buffers_blend.xml', 'AMD_performance_monitor.xml', 'ARB_vertex_type_2_10_10_10_rev.xml', -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] mesa: Add support for AMD_depth_clamp_separate
Enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. Remove DepthClamp, because DepthClampNear + DepthClampFar replaces it, as suggested by Marek Olsak. Driver that enables AMD_depth_clamp_separate will only ever look at DepthClampNear and DepthClampFar, as suggested by Ian Romanick. Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/genX_state_upload.c | 11 ++ src/mesa/main/attrib.c| 21 --- src/mesa/main/enable.c| 9 +--- src/mesa/main/get.c | 4 src/mesa/main/get_hash_params.py | 2 +- src/mesa/main/mtypes.h| 1 - src/mesa/main/rastpos.c | 6 -- src/mesa/state_tracker/st_atom_rasterizer.c | 3 ++- src/mesa/state_tracker/st_cb_drawpixels.c | 3 ++- src/mesa/swrast/s_span.c | 2 +- src/mesa/tnl/t_vb_program.c | 6 -- src/mesa/tnl/t_vb_vertex.c| 8 --- 12 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index c051848985..dc54cb67af 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -1399,7 +1399,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.ScreenSpaceViewportYMax = 1; clip.ViewportXYClipTestEnable = true; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); /* _NEW_TRANSFORM */ if (GEN_GEN == 5 || GEN_IS_G4X) { @@ -1493,7 +1494,8 @@ genX(upload_clip_state)(struct brw_context *brw) clip.UserClipDistanceCullTestEnableBitmask = brw_vue_prog_data(brw->vs.base.prog_data)->cull_distance_mask; - clip.ViewportZClipTestEnable = !ctx->Transform.DepthClamp; + clip.ViewportZClipTestEnable = !(ctx->Transform.DepthClampNear && + ctx->Transform.DepthClampFar); #endif /* _NEW_LIGHT */ @@ -2338,7 +2340,7 @@ genX(upload_cc_viewport)(struct brw_context *brw) for (unsigned i = 0; i < viewport_count; i++) { /* _NEW_VIEWPORT | _NEW_TRANSFORM */ const struct gl_viewport_attrib *vp = >ViewportArray[i]; - if (ctx->Transform.DepthClamp) { + if (ctx->Transform.DepthClampNear && ctx->Transform.DepthClampFar) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { @@ -4605,7 +4607,8 @@ genX(upload_raster)(struct brw_context *brw) raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; /* _NEW_TRANSFORM */ - if (!ctx->Transform.DepthClamp) { + if (!(ctx->Transform.DepthClampNear && +ctx->Transform.DepthClampFar)) { #if GEN_GEN >= 9 raster.ViewportZFarClipTestEnable = true; raster.ViewportZNearClipTestEnable = true; diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c index cbe93ab6fa..cce5d8582e 100644 --- a/src/mesa/main/attrib.c +++ b/src/mesa/main/attrib.c @@ -72,7 +72,8 @@ struct gl_enable_attrib GLbitfield ClipPlanes; GLboolean ColorMaterial; GLboolean CullFace; - GLboolean DepthClamp; + GLboolean DepthClampNear; + GLboolean DepthClampFar; GLboolean DepthTest; GLboolean Dither; GLboolean Fog; @@ -336,7 +337,8 @@ _mesa_PushAttrib(GLbitfield mask) attr->ClipPlanes = ctx->Transform.ClipPlanesEnabled; attr->ColorMaterial = ctx->Light.ColorMaterialEnabled; attr->CullFace = ctx->Polygon.CullFlag; - attr->DepthClamp = ctx->Transform.DepthClamp; + attr->DepthClampNear = ctx->Transform.DepthClampNear; + attr->DepthClampFar = ctx->Transform.DepthClampFar; attr->DepthTest = ctx->Depth.Test; attr->Dither = ctx->Color.DitherFlag; attr->Fog = ctx->Fog.Enabled; @@ -627,8 +629,10 @@ pop_enable_group(struct gl_context *ctx, const struct gl_enable_attrib *enable) TEST_AND_UPDATE(ctx->Light.ColorMaterialEnabled, enable->ColorMaterial, GL_COLOR_MATERIAL); TEST_AND_UPDATE(ctx->Polygon.CullFlag, enable->CullFace, GL_CULL_FACE); - TEST_AND_UPDATE(ctx->Transform.DepthClamp, enable->DepthClamp, - GL_DEPTH_CLAMP); + TEST_AND_UPDATE(ctx->Transform.DepthClampNear, enable->DepthClampNear, + GL_DEPTH_CLAMP_NEAR_AMD); + TEST_AND_UPDATE(ctx->Transform.DepthClampFar, enable->DepthClampFar, + GL_DEPTH_CLAMP_FAR_AMD); TEST_AND_UPDATE(ctx->Depth.Test, enable->DepthTest, GL
[Mesa-dev] [PATCH 4/7] mesa: add support for GL_AMD_depth_clamp_separate tokens
_mesa_set_enable() and _mesa_IsEnabled() extended to accept new two tokens GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD. Signed-off-by: Sagar Ghuge --- src/mesa/main/enable.c | 36 1 file changed, 36 insertions(+) diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c index 4bde9052bc..66e224676a 100644 --- a/src/mesa/main/enable.c +++ b/src/mesa/main/enable.c @@ -1017,6 +1017,30 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state) ctx->Transform.DepthClampFar = state; break; + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampNear == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; + ctx->Transform.DepthClampNear = state; + break; + + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampFar == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClamp ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; + ctx->Transform.DepthClampFar = state; + break; + case GL_FRAGMENT_SHADER_ATI: if (ctx->API != API_OPENGL_COMPAT) goto invalid_enum_error; @@ -1689,6 +1713,18 @@ _mesa_IsEnabled( GLenum cap ) return (ctx->Transform.DepthClampNear || ctx->Transform.DepthClampFar); + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return (ctx->Transform.DepthClampNear); + + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return (ctx->Transform.DepthClampFar); + case GL_FRAGMENT_SHADER_ATI: if (ctx->API != API_OPENGL_COMPAT) goto invalid_enum_error; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.
Thank you for volunteering to test my branch. But before I point you to the branch, I will rework patches according to your comment on patch 3. Again thanks a lot for your and Ian's input. - Sagar On 08/20/2018 04:21 PM, Marek Olšák wrote: > I can try to test the extension with the radeonsi driver. Do you have > a Mesa branch with the final patches? > > Marek > > On Mon, Aug 13, 2018 at 5:35 PM Sagar Ghuge wrote: >> >> Hi everyone, >> >> I am kind of stuck on this part actually. I don't have >> latest AMD graphics card to test following behavior which >> Ian and Marek suggested me. >> >> I have written a piglit test : >> https://gitlab.freedesktop.org/sagarghuge/piglit/blob/320b91ffb131b380f1d27d9c05ab141e0cd9e557/tests/spec/amd_depth_clamp_separate/depth_clamp_get_test.c >> >> It would be great if someone can help me or test it in their >> spare time on latest AMD graphics card and provide some input >> on the extension behavior on AMD's closed source driver. >> >> >> On 08/09/2018 01:11 PM, Marek Olšák wrote: >>> On Thu, Aug 2, 2018 at 2:44 PM, Ian Romanick wrote: >>>> On 08/02/2018 11:30 AM, Ian Romanick wrote: >>>>> On 08/01/2018 08:31 PM, Sagar Ghuge wrote: >>>>>> Add some basic types and storage for the >>>>>> AMD_depth_clamp_separate extension. >>>> >>>> I mentioned this on patch 5, but you should word wrap the commit message >>>> to 70 or 72 columns. >>>> >>>> More substantive comments are below... >>>> >>>>>> Signed-off-by: Sagar Ghuge >>>>>> --- >>>>>> include/GL/glcorearb.h | 2 ++ >>>>>> src/mesa/main/extensions_table.h | 1 + >>>>>> src/mesa/main/mtypes.h | 9 + >>>>>> 3 files changed, 12 insertions(+) >>>>>> >>>>>> diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h >>>>>> index a78bbb6e18..d73ca5a8df 100644 >>>>>> --- a/include/GL/glcorearb.h >>>>>> +++ b/include/GL/glcorearb.h >>>>>> @@ -1558,6 +1558,8 @@ typedef int64_t GLint64; >>>>>> #define GL_MAX_FRAGMENT_INPUT_COMPONENTS 0x9125 >>>>>> #define GL_CONTEXT_PROFILE_MASK 0x9126 >>>>>> #define GL_DEPTH_CLAMP0x864F >>>>>> +#define GL_DEPTH_CLAMP_NEAR_AMD 0x901E >>>>>> +#define GL_DEPTH_CLAMP_FAR_AMD0x901F >>>>>> #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C >>>>>> #define GL_FIRST_VERTEX_CONVENTION0x8E4D >>>>>> #define GL_LAST_VERTEX_CONVENTION 0x8E4E >>>>> >>>>> We should just import the updated versions of the Khronos headers. I >>>>> think Marek sent out a patch to do this. Does that work? >>>>> >>>>>> diff --git a/src/mesa/main/extensions_table.h >>>>>> b/src/mesa/main/extensions_table.h >>>>>> index 3f01896cae..8dc668e087 100644 >>>>>> --- a/src/mesa/main/extensions_table.h >>>>>> +++ b/src/mesa/main/extensions_table.h >>>>>> @@ -9,6 +9,7 @@ >>>>>> EXT(3DFX_texture_compression_FXT1 , >>>>>> TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) >>>>>> >>>>>> EXT(AMD_conservative_depth , ARB_conservative_depth >>>>>> , GLL, GLC, x , x , 2009) >>>>>> +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate >>>>>> , x , GLC, x , x , 2009) >>>>>> EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend >>>>>> , GLL, GLC, x , x , 2009) >>>>>> EXT(AMD_performance_monitor , AMD_performance_monitor >>>>>> , GLL, GLC, x , ES2, 2007) >>>>>> EXT(AMD_pinned_memory , AMD_pinned_memory >>>>>> , GLL, GLC, x , x , 2013) >>>>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h >>>>>> index d71872835d..406746a84c 100644 >>>>>> --- a/src/mesa/main/mtypes.h >>>>>> +++ b/src/mesa/main/mtypes.h >>>>>> @@ -1280,6 +1280,8 @@ struct gl_transform_attrib >>>>>> GLboolean RescaleNormals;
[Mesa-dev] [PATCH v2] intel/tools: new i965_disasm tool
Adds a new i965 instruction disassemble tool v2: 1) fix a few nits (Matt Turner) 2) Remove i965_disasm header (Matt Turner) Signed-off-by: Sagar Ghuge --- src/intel/Makefile.tools.am | 14 +++ src/intel/tools/i965_disasm.c | 199 ++ src/intel/tools/meson.build | 11 ++ 3 files changed, 224 insertions(+) create mode 100644 src/intel/tools/i965_disasm.c diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am index 00624084e6..385819abc2 100644 --- a/src/intel/Makefile.tools.am +++ b/src/intel/Makefile.tools.am @@ -22,6 +22,7 @@ noinst_PROGRAMS += \ tools/aubinator \ tools/aubinator_error_decode \ + tools/i965_disasm \ tools/error2aub @@ -62,6 +63,19 @@ tools_aubinator_error_decode_CFLAGS = \ $(AM_CFLAGS) \ $(ZLIB_CFLAGS) +tools_i965_disasm_SOURCES = \ + tools/i965_disasm.c + +tools_i965_disasm_LDADD = \ + common/libintel_common.la \ + compiler/libintel_compiler.la \ + dev/libintel_dev.la \ + $(top_builddir)/src/util/libmesautil.la \ + $(PTHREAD_LIBS) + +tools_i965_disasm_CFLAGS = \ + $(AM_CFLAGS) + tools_error2aub_SOURCES = \ tools/gen_context.h \ diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c new file mode 100644 index 00..757d2c7db1 --- /dev/null +++ b/src/intel/tools/i965_disasm.c @@ -0,0 +1,199 @@ +/* + * Copyright © 2018 Intel Corporation + * + * 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. + */ + +#include +#include + +#include "compiler/brw_inst.h" +#include "compiler/brw_eu.h" +#include "dev/gen_device_info.h" + +uint64_t INTEL_DEBUG; +uint16_t pci_id = 0; +FILE *outfile; + +struct i965_disasm { +struct gen_device_info devinfo; +}; + +/* Return size of file in bytes pointed by fp */ +static size_t +i965_disasm_get_file_size(FILE *fp) +{ + size_t size; + + fseek(fp, 0L, SEEK_END); + size = ftell(fp); + fseek(fp, 0L, SEEK_SET); + + return size; +} + +/* Return number of bytes read */ +static size_t +i965_disasm_read_binary(FILE *fp, void **assembly) +{ + size_t end = i965_disasm_get_file_size(fp); + *assembly = malloc(end + 1); + fread(*assembly, end, 1, fp); + fclose(fp); + + return end; +} + +/* Disassemble i965 instructions from buffer assembly + * start : starting offset within buffer + * end : points to last byte of buffer + */ +static void +i965_disasm_disassemble(struct i965_disasm *disasm, void *assembly, +int start, int end, FILE *out) +{ + brw_disassemble(>devinfo, assembly, start, end, out); +} + +static struct i965_disasm * +i965_disasm_init(void) +{ + struct gen_device_info devinfo; + struct i965_disasm *i965d; + + i965d = malloc(sizeof *i965d); + if (i965d == NULL) + return NULL; + + if(!gen_get_device_info(pci_id, )) { + fprintf(outfile, "can't find device information: pci_id=0x%x\n", + pci_id); + exit(EXIT_FAILURE); + } + + i965d->devinfo = devinfo; + + /* initialize compaction table in order +* to handle compacted instructions +*/ + brw_init_compaction_tables(>devinfo); + + return i965d; +} + +static void +i965_disasm_destroy(struct i965_disasm *disasm) +{ + free(disasm); +} + +static void +print_help(const char *progname, FILE *file) +{ + fprintf(file, + "Usage: %s [OPTION]...\n" + "Disassemble i965 instructions from binary file.\n\n" + " --help display this help and exit\n" + " --binary-path=PATH read binary file from binary file PATH\n" + " --gen=platform disassemble instructions for given \n" + " platform (3 letter platform name)\n", + progname); +} + +int main(i
Re: [Mesa-dev] [PATCH] intel/tools: new i965_disasm tool
Thanks for reviewing the patch. I will make changes and send v2 accordingly. On 08/20/2018 11:34 AM, Matt Turner wrote: > On Thu, Aug 16, 2018 at 1:51 PM Sagar Ghuge wrote: >> >> Adds a new i965 instruction disassemble tool > > This looks very good. A few comments about the structure inline. > >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/Makefile.tools.am | 15 +++ >> src/intel/tools/i965_disasm.c | 202 ++ >> src/intel/tools/i965_disasm.h | 46 >> src/intel/tools/meson.build | 11 ++ >> 4 files changed, 274 insertions(+) >> create mode 100644 src/intel/tools/i965_disasm.c >> create mode 100644 src/intel/tools/i965_disasm.h >> >> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am >> index 00624084e6..36a3a70a28 100644 >> --- a/src/intel/Makefile.tools.am >> +++ b/src/intel/Makefile.tools.am >> @@ -22,6 +22,7 @@ >> noinst_PROGRAMS += \ >> tools/aubinator \ >> tools/aubinator_error_decode \ >> + tools/i965_disasm \ >> tools/error2aub >> >> >> @@ -62,6 +63,20 @@ tools_aubinator_error_decode_CFLAGS = \ >> $(AM_CFLAGS) \ >> $(ZLIB_CFLAGS) >> >> +tools_i965_disasm_SOURCES = \ >> + tools/i965_disasm.c \ >> + tools/i965_disasm.h >> + >> +tools_i965_disasm_LDADD = \ >> + common/libintel_common.la \ >> + compiler/libintel_compiler.la \ >> + dev/libintel_dev.la \ >> + $(top_builddir)/src/util/libmesautil.la \ >> + $(PTHREAD_LIBS) >> + >> +tools_i965_disasm_CFLAGS = \ >> + $(AM_CFLAGS) >> + > > Looks good. > >> tools_error2aub_SOURCES = \ >> tools/gen_context.h \ >> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c >> new file mode 100644 >> index 00..c880559827 >> --- /dev/null >> +++ b/src/intel/tools/i965_disasm.c >> @@ -0,0 +1,202 @@ >> +/* >> + * Copyright © 2018 Intel Corporation >> + * >> + * 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. >> + */ >> + >> +#include >> +#include >> + >> +#include "compiler/brw_inst.h" >> +#include "compiler/brw_eu.h" >> + >> +#include "i965_disasm.h" >> + >> +uint64_t INTEL_DEBUG; >> +uint16_t pci_id = 0; >> +FILE *outfile; >> + >> +struct i965_disasm { >> +struct gen_device_info devinfo; >> +}; >> + >> +/* Return size of file in bytes pointed by fp */ >> +static size_t >> +i965_disasm_get_file_size(FILE *fp) >> +{ >> + size_t size = 0; > > No need for initialization. > >> + >> + fseek(fp, 0L, SEEK_END); >> + size = ftell(fp); >> + fseek(fp, 0L, SEEK_SET); >> + >> + return size; >> +} >> + >> +/* Return number of bytes read */ >> +static size_t >> +i965_disasm_read_binary(FILE *fp, void **assembly) >> +{ >> + size_t end = i965_disasm_get_file_size(fp); >> + *assembly = malloc(end + 1); >> + fread(*assembly, end, 1, fp); >> + fclose(fp); >> + >> + return end; >> +} >> + >> +static void >> +print_help(const char *progname, FILE *file) >> +{ >> + fprintf(file, >> + "Usage: %s [OPTION
Re: [Mesa-dev] [PATCH] intel/eu: print bytes instead of 32 bit hex value
On 08/20/2018 11:06 AM, Matt Turner wrote: > Cool. This looks pretty good to me. A few comments inline. > > On Wed, Aug 15, 2018 at 2:00 PM Sagar Ghuge wrote: >> >> INTEL_DEBUG=hex prints 32 bit hex value >> and due to endianness of CPU byte order is >> reversed. In order to disassemble binary >> files, print each byte instead of 32 bit hex >> value. > > Let's get your editor configured to line wrap at the correct length > (these lines are too short). > > If you use vim, you should be able to automatically line wrap to the > appropriate length by highlighting the lines and then giving the > command 'gq' > >> Signed-off-by: Sagar Ghuge >> --- >> src/intel/compiler/brw_eu.c | 24 >> 1 file changed, 16 insertions(+), 8 deletions(-) >> >> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c >> index 6ef0a6a577..223e561dff 100644 >> --- a/src/intel/compiler/brw_eu.c >> +++ b/src/intel/compiler/brw_eu.c >> @@ -365,9 +365,14 @@ brw_disassemble(const struct gen_device_info *devinfo, >>if (compacted) { >> brw_compact_inst *compacted = (void *)insn; >> if (dump_hex) { >> - fprintf(out, "0x%08x 0x%08x ", >> - ((uint32_t *)insn)[1], >> - ((uint32_t *)insn)[0]); >> + unsigned char * insn_ptr = ((unsigned char *)[0]); >> + for (int i = 0 ; i < 8; i = i + 4) { >> + fprintf(out, "%02x %02x %02x %02x ", >> + insn_ptr[i], >> + insn_ptr[i + 1], >> + insn_ptr[i + 2], >> + insn_ptr[i + 3]); >> + } > > I like printing the spaces between the bytes. That really shows more > clearly that this is a byte array and not subject to any endianness > issues. > > One suggestion: let's print some blank spaces after the compacted > instruction hex so that the disassembled instruction vertically aligns > with uncompacted instructions. Currently we get disassembly that looks > like > Thanks for reviewing the patch. Yes, I made changes and sent v2 according to your suggestions. > 01 0b 1d 20 00 7c 02 00 mov(8) g124<1>Fg2.3<0,1,0>F > 01 00 60 00 e8 3a a0 2f 5c 00 00 00 00 00 00 00 mov(8) > g125<1>Fg2.7<0,1,0>F > > Also, we don't use tabs in i965. When editing old lines that had tabs, > let's take the opportunity to remove them. > > My ~/.vimrc has > > autocmd BufNewFile,BufRead /home/mattst88/projects/mesa/* set > expandtab tabstop=8 softtabstop=3 shiftwidth=3 > autocmd BufNewFile,BufRead > /home/mattst88/projects/mesa/src/glsl/glcpp/* set noexpandtab > tabstop=8 softtabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead > /home/mattst88/projects/mesa/src/glsl/glsl_parser.yy set noexpandtab > tabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead /home/mattst88/projects/piglit/* set > noexpandtab tabstop=8 softtabstop=8 shiftwidth=8 > > to configure it appropriately for my Mesa and piglit directories. > Thanks for sharing vimrc. I think my struggle ends here about getting coding style correct :) > With those couple of small nits fixed, this will earn my review. > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] intel/eu: print bytes instead of 32 bit hex value
INTEL_DEBUG=hex prints 32 bit hex value and due to endianness of CPU byte order is reversed. In order to disassemble binary files, print each byte instead of 32 bit hex value. v2: Print blank spaces in order to vertically align output of compacted instructions hex value with uncompacted instructions hex value. (Matt Turner) Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu.c | 48 - 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c index 6ef0a6a577..ab87ae90e1 100644 --- a/src/intel/compiler/brw_eu.c +++ b/src/intel/compiler/brw_eu.c @@ -364,24 +364,38 @@ brw_disassemble(const struct gen_device_info *devinfo, if (compacted) { brw_compact_inst *compacted = (void *)insn; -if (dump_hex) { - fprintf(out, "0x%08x 0x%08x ", - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); -} - -brw_uncompact_instruction(devinfo, , compacted); -insn = -offset += 8; + if (dump_hex) { +unsigned char * insn_ptr = ((unsigned char *)[0]); +const unsigned int blank_spaces = 24; +for (int i = 0 ; i < 8; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); +} +/* Make compacted instructions hex value output + * vertically aligned with uncompacted instructions + * hex value + */ +fprintf(out, "%*c", blank_spaces, ' '); + } + + brw_uncompact_instruction(devinfo, , compacted); + insn = + offset += 8; } else { -if (dump_hex) { - fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ", - ((uint32_t *)insn)[3], - ((uint32_t *)insn)[2], - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); -} -offset += 16; + if (dump_hex) { +unsigned char * insn_ptr = ((unsigned char *)[0]); +for (int i = 0 ; i < 16; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); +} + } + offset += 16; } brw_disassemble_inst(out, devinfo, insn, compacted); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate
On 08/13/2018 03:52 PM, Ian Romanick wrote: > On 08/09/2018 01:09 PM, Marek Olšák wrote: >> On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge wrote: >>> enable _mesa_PushAttrib() and _mesa_PopAttrib() >>> to handle GL_DEPTH_CLAMP_NEAR_AMD and >>> GL_DEPTH_CLAMP_FAR_AMD tokens. >>> >>> Signed-off-by: Sagar Ghuge >>> --- >>> src/mesa/main/attrib.c | 16 >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c >>> index cbe93ab6fa..d9f165b428 100644 >>> --- a/src/mesa/main/attrib.c >>> +++ b/src/mesa/main/attrib.c >>> @@ -73,6 +73,8 @@ struct gl_enable_attrib >>> GLboolean ColorMaterial; >>> GLboolean CullFace; >>> GLboolean DepthClamp; >>> + GLboolean DepthClampNear; >>> + GLboolean DepthClampFar; >> >> The first patch uses this. Also, DepthClamp can be removed, because >> DepthClampNear+Far replace it, right? > > Based on your comment on patch 4 and my comments on patch 0, maybe we > should: > > - Remove DepthClamp. Add _DepthClamp, DepthClampNear, and DepthClampFar. I might be missing some pieces. But DepthClampNear + far can replaces DepthClamp. so why do we need _DepthClamp ? (Adding _DepthClamp means it will be derived from DepthClampNear+far, correct ? removing DepthClamp here means, need to completely get rid of every reference of DepthClamp in source code? ) > > - If GL_DEPTH_CLAMP is set, set all three. If GL_DEPTH_CLAMP is > cleared, clear all three. > > - If either of GL_DEPTH_CLAMP_FAR_AMD or GL_DEPTH_CLAMP_NEAR_AMD > changes, change _DepthClamp to DepthClampNear || DepthClampFar. > We only need to handle this case - "Querying DEPTH_CLAMP will return TRUE if DEPTH_CLAMP_NEAR_AMD _or_ DEPTH_CLAMP_FAR_AMD is enabled." I think we don't have to keep changing _DepthClamp, because if we do it then it will enable depth clamping for both the planes and will get different behavior. Please correct me if I am wrong or missing anything. > - Drivers that enable AMD_depth_clamp_separate will only ever look at > DepthClampNear and DepthClampFar. > > I think that gets all the cases correct with the minimum fuss. Marek, > what do you think? > >> 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
[Mesa-dev] [PATCH] intel/tools: new i965_disasm tool
Adds a new i965 instruction disassemble tool Signed-off-by: Sagar Ghuge --- src/intel/Makefile.tools.am | 15 +++ src/intel/tools/i965_disasm.c | 202 ++ src/intel/tools/i965_disasm.h | 46 src/intel/tools/meson.build | 11 ++ 4 files changed, 274 insertions(+) create mode 100644 src/intel/tools/i965_disasm.c create mode 100644 src/intel/tools/i965_disasm.h diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am index 00624084e6..36a3a70a28 100644 --- a/src/intel/Makefile.tools.am +++ b/src/intel/Makefile.tools.am @@ -22,6 +22,7 @@ noinst_PROGRAMS += \ tools/aubinator \ tools/aubinator_error_decode \ + tools/i965_disasm \ tools/error2aub @@ -62,6 +63,20 @@ tools_aubinator_error_decode_CFLAGS = \ $(AM_CFLAGS) \ $(ZLIB_CFLAGS) +tools_i965_disasm_SOURCES = \ + tools/i965_disasm.c \ + tools/i965_disasm.h + +tools_i965_disasm_LDADD = \ + common/libintel_common.la \ + compiler/libintel_compiler.la \ + dev/libintel_dev.la \ + $(top_builddir)/src/util/libmesautil.la \ + $(PTHREAD_LIBS) + +tools_i965_disasm_CFLAGS = \ + $(AM_CFLAGS) + tools_error2aub_SOURCES = \ tools/gen_context.h \ diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c new file mode 100644 index 00..c880559827 --- /dev/null +++ b/src/intel/tools/i965_disasm.c @@ -0,0 +1,202 @@ +/* + * Copyright © 2018 Intel Corporation + * + * 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. + */ + +#include +#include + +#include "compiler/brw_inst.h" +#include "compiler/brw_eu.h" + +#include "i965_disasm.h" + +uint64_t INTEL_DEBUG; +uint16_t pci_id = 0; +FILE *outfile; + +struct i965_disasm { +struct gen_device_info devinfo; +}; + +/* Return size of file in bytes pointed by fp */ +static size_t +i965_disasm_get_file_size(FILE *fp) +{ + size_t size = 0; + + fseek(fp, 0L, SEEK_END); + size = ftell(fp); + fseek(fp, 0L, SEEK_SET); + + return size; +} + +/* Return number of bytes read */ +static size_t +i965_disasm_read_binary(FILE *fp, void **assembly) +{ + size_t end = i965_disasm_get_file_size(fp); + *assembly = malloc(end + 1); + fread(*assembly, end, 1, fp); + fclose(fp); + + return end; +} + +static void +print_help(const char *progname, FILE *file) +{ + fprintf(file, + "Usage: %s [OPTION]...\n" + "Disassemble i965 instructions from binary file.\n\n" + " --help display this help and exit\n" + " --binary-path=PATH read binary file from binary file PATH\n" + " --gen=platform disassemble instructions for given \n" + " platform (3 letter platform name)\n", + progname); +} + +int main(int argc, char *argv[]) +{ + FILE *fp = NULL; + void *assembly = NULL; + char *binary_path = NULL; + size_t start = 0, end = 0; + int c, i; + struct i965_disasm *disasm; + + bool help = false; + const struct option i965_disasm_opts[] = { + { "help", no_argument, (int *) , true }, + { "binary-path", required_argument, NULL, 'b' }, + { "gen", required_argument, NULL, 'g'}, + { NULL,0, NULL, 0 } + }; + + outfile = stderr; + i = 0; + while ((c = getopt_long(argc, argv, "", i965_disasm_opts, )) != -1) { + switch (c) { + case 'g': { + const int id = gen_device_name_to_pci_device_id(optarg); + if (id < 0) { +fprintf(outfile, "can't parse gen: '%s', expected ivb, byt, hsw, " + "bdw, chv, skl, kbl or bxt\n&
[Mesa-dev] [PATCH] intel/eu: print bytes instead of 32 bit hex value
INTEL_DEBUG=hex prints 32 bit hex value and due to endianness of CPU byte order is reversed. In order to disassemble binary files, print each byte instead of 32 bit hex value. Signed-off-by: Sagar Ghuge --- src/intel/compiler/brw_eu.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c index 6ef0a6a577..223e561dff 100644 --- a/src/intel/compiler/brw_eu.c +++ b/src/intel/compiler/brw_eu.c @@ -365,9 +365,14 @@ brw_disassemble(const struct gen_device_info *devinfo, if (compacted) { brw_compact_inst *compacted = (void *)insn; if (dump_hex) { - fprintf(out, "0x%08x 0x%08x ", - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); + unsigned char * insn_ptr = ((unsigned char *)[0]); + for (int i = 0 ; i < 8; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); + } } brw_uncompact_instruction(devinfo, , compacted); @@ -375,11 +380,14 @@ brw_disassemble(const struct gen_device_info *devinfo, offset += 8; } else { if (dump_hex) { - fprintf(out, "0x%08x 0x%08x 0x%08x 0x%08x ", - ((uint32_t *)insn)[3], - ((uint32_t *)insn)[2], - ((uint32_t *)insn)[1], - ((uint32_t *)insn)[0]); + unsigned char * insn_ptr = ((unsigned char *)[0]); + for (int i = 0 ; i < 16; i = i + 4) { + fprintf(out, "%02x %02x %02x %02x ", + insn_ptr[i], + insn_ptr[i + 1], + insn_ptr[i + 2], + insn_ptr[i + 3]); + } } offset += 16; } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate
On 08/13/2018 03:43 PM, Ian Romanick wrote: > On 08/09/2018 01:14 PM, Sagar Ghuge wrote: >> >> >> On 08/09/2018 01:09 PM, Marek Olšák wrote: >>> On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge wrote: >>>> enable _mesa_PushAttrib() and _mesa_PopAttrib() >>>> to handle GL_DEPTH_CLAMP_NEAR_AMD and >>>> GL_DEPTH_CLAMP_FAR_AMD tokens. >>>> >>>> Signed-off-by: Sagar Ghuge >>>> --- >>>> src/mesa/main/attrib.c | 16 >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c >>>> index cbe93ab6fa..d9f165b428 100644 >>>> --- a/src/mesa/main/attrib.c >>>> +++ b/src/mesa/main/attrib.c >>>> @@ -73,6 +73,8 @@ struct gl_enable_attrib >>>> GLboolean ColorMaterial; >>>> GLboolean CullFace; >>>> GLboolean DepthClamp; >>>> + GLboolean DepthClampNear; >>>> + GLboolean DepthClampFar; >>> >>> The first patch uses this. Also, DepthClamp can be removed, because >>> DepthClampNear+Far replace it, right? >> >> Yes, that's true. > > Since this is your first significant patch series... we have an > unwritten (maybe actually written somewhere?) rule that a patch series > should build at every commit. If a patch in the middle of a series > doesn't build, doing git-bisect on a (possibly unrelated) later problem > can be very painful. > Okay, I will make sure that when I send new patch series it should build on every commit. >>> 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] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.
Hi everyone, I am kind of stuck on this part actually. I don't have latest AMD graphics card to test following behavior which Ian and Marek suggested me. I have written a piglit test : https://gitlab.freedesktop.org/sagarghuge/piglit/blob/320b91ffb131b380f1d27d9c05ab141e0cd9e557/tests/spec/amd_depth_clamp_separate/depth_clamp_get_test.c It would be great if someone can help me or test it in their spare time on latest AMD graphics card and provide some input on the extension behavior on AMD's closed source driver. On 08/09/2018 01:11 PM, Marek Olšák wrote: > On Thu, Aug 2, 2018 at 2:44 PM, Ian Romanick wrote: >> On 08/02/2018 11:30 AM, Ian Romanick wrote: >>> On 08/01/2018 08:31 PM, Sagar Ghuge wrote: >>>> Add some basic types and storage for the >>>> AMD_depth_clamp_separate extension. >> >> I mentioned this on patch 5, but you should word wrap the commit message >> to 70 or 72 columns. >> >> More substantive comments are below... >> >>>> Signed-off-by: Sagar Ghuge >>>> --- >>>> include/GL/glcorearb.h | 2 ++ >>>> src/mesa/main/extensions_table.h | 1 + >>>> src/mesa/main/mtypes.h | 9 + >>>> 3 files changed, 12 insertions(+) >>>> >>>> diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h >>>> index a78bbb6e18..d73ca5a8df 100644 >>>> --- a/include/GL/glcorearb.h >>>> +++ b/include/GL/glcorearb.h >>>> @@ -1558,6 +1558,8 @@ typedef int64_t GLint64; >>>> #define GL_MAX_FRAGMENT_INPUT_COMPONENTS 0x9125 >>>> #define GL_CONTEXT_PROFILE_MASK 0x9126 >>>> #define GL_DEPTH_CLAMP0x864F >>>> +#define GL_DEPTH_CLAMP_NEAR_AMD 0x901E >>>> +#define GL_DEPTH_CLAMP_FAR_AMD0x901F >>>> #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C >>>> #define GL_FIRST_VERTEX_CONVENTION0x8E4D >>>> #define GL_LAST_VERTEX_CONVENTION 0x8E4E >>> >>> We should just import the updated versions of the Khronos headers. I >>> think Marek sent out a patch to do this. Does that work? >>> >>>> diff --git a/src/mesa/main/extensions_table.h >>>> b/src/mesa/main/extensions_table.h >>>> index 3f01896cae..8dc668e087 100644 >>>> --- a/src/mesa/main/extensions_table.h >>>> +++ b/src/mesa/main/extensions_table.h >>>> @@ -9,6 +9,7 @@ >>>> EXT(3DFX_texture_compression_FXT1 , >>>> TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) >>>> >>>> EXT(AMD_conservative_depth , ARB_conservative_depth >>>>, GLL, GLC, x , x , 2009) >>>> +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate >>>>, x , GLC, x , x , 2009) >>>> EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend >>>>, GLL, GLC, x , x , 2009) >>>> EXT(AMD_performance_monitor , AMD_performance_monitor >>>>, GLL, GLC, x , ES2, 2007) >>>> EXT(AMD_pinned_memory , AMD_pinned_memory >>>>, GLL, GLC, x , x , 2013) >>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h >>>> index d71872835d..406746a84c 100644 >>>> --- a/src/mesa/main/mtypes.h >>>> +++ b/src/mesa/main/mtypes.h >>>> @@ -1280,6 +1280,8 @@ struct gl_transform_attrib >>>> GLboolean RescaleNormals;/**< >>>> GL_EXT_rescale_normal */ >>>> GLboolean RasterPositionUnclipped; /**< >>>> GL_IBM_rasterpos_clip */ >>>> GLboolean DepthClamp;/**< GL_ARB_depth_clamp */ >>>> + GLboolean DepthClampNear;/**< >>>> GL_AMD_depth_clamp_separate */ >>>> + GLboolean DepthClampFar; /**< >>>> GL_AMD_depth_clamp_separate */ >> >> I think we actually need two more flags here: _DepthClampNear and >> _DepthClampFar. The spec is a little unclear, so you may need to test >> on some AMD closed-source drivers. Specifically, the spec says >> >> "In addition to DEPTH_CLAMP_NEAR_AMD and DEPTH_CLAMP_FAR_AMD, the >> token DEPTH_CLAMP may be used to simultaneously enable or disable >> depth clamping at both the near and far planes." >> >> Based on that, I'm not sure what you'r
Re: [Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate
On 08/09/2018 01:09 PM, Marek Olšák wrote: > On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge wrote: >> enable _mesa_PushAttrib() and _mesa_PopAttrib() >> to handle GL_DEPTH_CLAMP_NEAR_AMD and >> GL_DEPTH_CLAMP_FAR_AMD tokens. >> >> Signed-off-by: Sagar Ghuge >> --- >> src/mesa/main/attrib.c | 16 >> 1 file changed, 16 insertions(+) >> >> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c >> index cbe93ab6fa..d9f165b428 100644 >> --- a/src/mesa/main/attrib.c >> +++ b/src/mesa/main/attrib.c >> @@ -73,6 +73,8 @@ struct gl_enable_attrib >> GLboolean ColorMaterial; >> GLboolean CullFace; >> GLboolean DepthClamp; >> + GLboolean DepthClampNear; >> + GLboolean DepthClampFar; > > The first patch uses this. Also, DepthClamp can be removed, because > DepthClampNear+Far replace it, right? Yes, that's true. > > Marek > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] v3d: Fix coverity control flow warning
if statement should check return value of drmSyncobjImportSyncFile function. Fixes CID 1438127 Signed-off-by: Sagar Ghuge --- src/gallium/drivers/v3d/v3d_fence.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/v3d/v3d_fence.c b/src/gallium/drivers/v3d/v3d_fence.c index 0edcb66d91..58d118a288 100644 --- a/src/gallium/drivers/v3d/v3d_fence.c +++ b/src/gallium/drivers/v3d/v3d_fence.c @@ -79,7 +79,7 @@ v3d_fence_finish(struct pipe_screen *pscreen, return false; } -drmSyncobjImportSyncFile(screen->fd, syncobj, f->fd); +ret = drmSyncobjImportSyncFile(screen->fd, syncobj, f->fd); if (ret) { fprintf(stderr, "Failed to import fence to syncobj: %d\n", ret); return false; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] include: update GL & GLES headers
I have few comments below, I might be wrong about those but except that this patch looks good to me. I checked it against Khronos headers. On 08/01/2018 11:34 AM, Marek Olšák wrote: From: Marek Olšák --- include/GL/glcorearb.h | 68 -- include/GL/glext.h | 77 +--- include/GL/glxext.h| 22 -- include/GLES2/gl2.h| 8 +-- include/GLES2/gl2ext.h | 159 +++-- include/GLES3/gl3.h| 8 +-- 6 files changed, 309 insertions(+), 33 deletions(-) diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h index a78bbb6e182..82cbfbafbda 100644 --- a/include/GL/glcorearb.h +++ b/include/GL/glcorearb.h @@ -1,19 +1,19 @@ -#ifndef __glcorearb_h_ -#define __glcorearb_h_ 1 +#ifndef __gl_glcorearb_h_ +#define __gl_glcorearb_h_ 1 #ifdef __cplusplus extern "C" { #endif /* -** Copyright (c) 2013-2017 The Khronos Group Inc. +** Copyright (c) 2013-2018 The Khronos Group Inc. ** ** Permission is hereby granted, free of charge, to any person obtaining a ** copy of this software and/or associated documentation files (the ** "Materials"), to deal in the Materials without restriction, including ** without limitation the rights to use, copy, modify, merge, publish, ** distribute, sublicense, and/or sell copies of the Materials, and to ** permit persons to whom the Materials are furnished to do so, subject to ** the following conditions: ** ** The above copyright notice and this permission notice shall be included @@ -299,21 +299,21 @@ typedef void (APIENTRYP PFNGLGETDOUBLEVPROC) (GLenum pname, GLdouble *data); typedef GLenum (APIENTRYP PFNGLGETERRORPROC) (void); typedef void (APIENTRYP PFNGLGETFLOATVPROC) (GLenum pname, GLfloat *data); typedef void (APIENTRYP PFNGLGETINTEGERVPROC) (GLenum pname, GLint *data); typedef const GLubyte *(APIENTRYP PFNGLGETSTRINGPROC) (GLenum name); typedef void (APIENTRYP PFNGLGETTEXIMAGEPROC) (GLenum target, GLint level, GLenum format, GLenum type, void *pixels); typedef void (APIENTRYP PFNGLGETTEXPARAMETERFVPROC) (GLenum target, GLenum pname, GLfloat *params); typedef void (APIENTRYP PFNGLGETTEXPARAMETERIVPROC) (GLenum target, GLenum pname, GLint *params); typedef void (APIENTRYP PFNGLGETTEXLEVELPARAMETERFVPROC) (GLenum target, GLint level, GLenum pname, GLfloat *params); typedef void (APIENTRYP PFNGLGETTEXLEVELPARAMETERIVPROC) (GLenum target, GLint level, GLenum pname, GLint *params); typedef GLboolean (APIENTRYP PFNGLISENABLEDPROC) (GLenum cap); -typedef void (APIENTRYP PFNGLDEPTHRANGEPROC) (GLdouble near, GLdouble far); +typedef void (APIENTRYP PFNGLDEPTHRANGEPROC) (GLdouble n, GLdouble f); typedef void (APIENTRYP PFNGLVIEWPORTPROC) (GLint x, GLint y, GLsizei width, GLsizei height); #ifdef GL_GLEXT_PROTOTYPES GLAPI void APIENTRY glCullFace (GLenum mode); GLAPI void APIENTRY glFrontFace (GLenum mode); GLAPI void APIENTRY glHint (GLenum target, GLenum mode); GLAPI void APIENTRY glLineWidth (GLfloat width); GLAPI void APIENTRY glPointSize (GLfloat size); GLAPI void APIENTRY glPolygonMode (GLenum face, GLenum mode); GLAPI void APIENTRY glScissor (GLint x, GLint y, GLsizei width, GLsizei height); GLAPI void APIENTRY glTexParameterf (GLenum target, GLenum pname, GLfloat param); @@ -348,21 +348,21 @@ GLAPI void APIENTRY glGetDoublev (GLenum pname, GLdouble *data); GLAPI GLenum APIENTRY glGetError (void); GLAPI void APIENTRY glGetFloatv (GLenum pname, GLfloat *data); GLAPI void APIENTRY glGetIntegerv (GLenum pname, GLint *data); GLAPI const GLubyte *APIENTRY glGetString (GLenum name); GLAPI void APIENTRY glGetTexImage (GLenum target, GLint level, GLenum format, GLenum type, void *pixels); GLAPI void APIENTRY glGetTexParameterfv (GLenum target, GLenum pname, GLfloat *params); GLAPI void APIENTRY glGetTexParameteriv (GLenum target, GLenum pname, GLint *params); GLAPI void APIENTRY glGetTexLevelParameterfv (GLenum target, GLint level, GLenum pname, GLfloat *params); GLAPI void APIENTRY glGetTexLevelParameteriv (GLenum target, GLint level, GLenum pname, GLint *params); GLAPI GLboolean APIENTRY glIsEnabled (GLenum cap); -GLAPI void APIENTRY glDepthRange (GLdouble near, GLdouble far); +GLAPI void APIENTRY glDepthRange (GLdouble n, GLdouble f); GLAPI void APIENTRY glViewport (GLint x, GLint y, GLsizei width, GLsizei height); #endif #endif /* GL_VERSION_1_0 */ #ifndef GL_VERSION_1_1 #define GL_VERSION_1_1 1 typedef float GLclampf; typedef double GLclampd; #define GL_COLOR_LOGIC_OP 0x0BF2 #define GL_POLYGON_OFFSET_UNITS 0x2A00 @@ -3951,20 +3951,34 @@ GLAPI void APIENTRY glMaxShaderCompilerThreadsKHR (GLuint count); #endif /* GL_KHR_texture_compression_astc_hdr */ #ifndef GL_KHR_texture_compression_astc_ldr #define GL_KHR_texture_compression_astc_ldr 1 #endif /* GL_KHR_texture_compression_astc_ldr */ #ifndef
Re: [Mesa-dev] [RFC][PATCH 2/5] glapi: define AMD_depth_clamp_separate
Okay, I will take care of that. Thanks for reviewing my patches Ian. On 08/02/2018 11:33 AM, Ian Romanick wrote: > This patch should go first in the series. With that changed, this patch is > > Reviewed-by: Ian Romanick > > On 08/01/2018 08:31 PM, Sagar Ghuge wrote: >> Signed-off-by: Sagar Ghuge >> --- >> src/mapi/glapi/gen/AMD_depth_clamp_separate.xml | 15 +++ >> src/mapi/glapi/gen/Makefile.am | 1 + >> src/mapi/glapi/gen/gl_API.xml | 3 +++ >> src/mapi/glapi/gen/meson.build | 1 + >> 4 files changed, 20 insertions(+) >> create mode 100644 src/mapi/glapi/gen/AMD_depth_clamp_separate.xml >> >> diff --git a/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml >> b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml >> new file mode 100644 >> index 00..b7142f1b7b >> --- /dev/null >> +++ b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml >> @@ -0,0 +1,15 @@ >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> + >> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am >> index 93acabd968..ab369a0d33 100644 >> --- a/src/mapi/glapi/gen/Makefile.am >> +++ b/src/mapi/glapi/gen/Makefile.am >> @@ -188,6 +188,7 @@ API_XML = \ >> ARB_vertex_attrib_64bit.xml \ >> ARB_vertex_attrib_binding.xml \ >> ARB_viewport_array.xml \ >> +AMD_depth_clamp_separate.xml \ >> AMD_draw_buffers_blend.xml \ >> AMD_performance_monitor.xml \ >> ARB_vertex_type_2_10_10_10_rev.xml \ >> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml >> index 49807e1ea5..b85cae2eb4 100644 >> --- a/src/mapi/glapi/gen/gl_API.xml >> +++ b/src/mapi/glapi/gen/gl_API.xml >> @@ -12939,6 +12939,9 @@ >> > xmlns:xi="http://www.w3.org/2001/XInclude"/> >> >> +> +xmlns:xi="http://www.w3.org/2001/XInclude"/> >> + >> >> >> >> diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build >> index a6a93cc83b..9c3f243aa9 100644 >> --- a/src/mapi/glapi/gen/meson.build >> +++ b/src/mapi/glapi/gen/meson.build >> @@ -95,6 +95,7 @@ api_xml_files = files( >>'ARB_vertex_attrib_64bit.xml', >>'ARB_vertex_attrib_binding.xml', >>'ARB_viewport_array.xml', >> + 'AMD_depth_clamp_separate.xml', >>'AMD_draw_buffers_blend.xml', >>'AMD_performance_monitor.xml', >>'ARB_vertex_type_2_10_10_10_rev.xml', >> > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 5/5] i965: add functional changes for AMD_depth_clamp_separate
Gen >= 9 have ability to control clamping of depth values separately at near and far plane. z_w is clamped to the range [min(n,f), 0] if clamping at near plane is enabled, [0, max(n,f)] if clamping at far plane is enabled and [min(n,f) max(n,f)] if clamping at both plane is enabled. Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/genX_state_upload.c | 21 --- src/mesa/drivers/dri/i965/intel_extensions.c | 1 + 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index ea5ad55be5..75d9bd2b9b 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -2341,6 +2341,12 @@ genX(upload_cc_viewport)(struct brw_context *brw) if (ctx->Transform.DepthClamp) { ccv.MinimumDepth = MIN2(vp->Near, vp->Far); ccv.MaximumDepth = MAX2(vp->Near, vp->Far); + } else if (ctx->Transform.DepthClampNear) { + ccv.MinimumDepth = MIN2(vp->Near, vp->Far); + ccv.MaximumDepth = 0.0; + } else if (ctx->Transform.DepthClampFar) { + ccv.MinimumDepth = 0.0; + ccv.MaximumDepth = MAX2(vp->Near, vp->Far); } else { ccv.MinimumDepth = 0.0; ccv.MaximumDepth = 1.0; @@ -4603,15 +4609,24 @@ genX(upload_raster)(struct brw_context *brw) /* _NEW_SCISSOR */ raster.ScissorRectangleEnable = ctx->Scissor.EnableFlags; +#if GEN_GEN < 9 /* _NEW_TRANSFORM */ if (!ctx->Transform.DepthClamp) { + raster.ViewportZClipTestEnable = true; + } +#endif + #if GEN_GEN >= 9 - raster.ViewportZFarClipTestEnable = true; + if (!ctx->Transform.DepthClampNear) { raster.ViewportZNearClipTestEnable = true; -#else - raster.ViewportZClipTestEnable = true; + } #endif + +#if GEN_GEN >= 9 + if (!ctx->Transform.DepthClampFar) { + raster.ViewportZFarClipTestEnable = true; } +#endif /* BRW_NEW_CONSERVATIVE_RASTERIZATION */ #if GEN_GEN >= 9 diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c index 2e28445ae3..0b7ce7b368 100644 --- a/src/mesa/drivers/dri/i965/intel_extensions.c +++ b/src/mesa/drivers/dri/i965/intel_extensions.c @@ -300,6 +300,7 @@ intelInitExtensions(struct gl_context *ctx) } if (devinfo->gen >= 9) { + ctx->Extensions.AMD_depth_clamp_separate = true; ctx->Extensions.ANDROID_extension_pack_es31a = true; ctx->Extensions.ARB_shader_stencil_export = true; ctx->Extensions.KHR_blend_equation_advanced_coherent = true; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate
enable _mesa_PushAttrib() and _mesa_PopAttrib() to handle GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD tokens. Signed-off-by: Sagar Ghuge --- src/mesa/main/attrib.c | 16 1 file changed, 16 insertions(+) diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c index cbe93ab6fa..d9f165b428 100644 --- a/src/mesa/main/attrib.c +++ b/src/mesa/main/attrib.c @@ -73,6 +73,8 @@ struct gl_enable_attrib GLboolean ColorMaterial; GLboolean CullFace; GLboolean DepthClamp; + GLboolean DepthClampNear; + GLboolean DepthClampFar; GLboolean DepthTest; GLboolean Dither; GLboolean Fog; @@ -337,6 +339,8 @@ _mesa_PushAttrib(GLbitfield mask) attr->ColorMaterial = ctx->Light.ColorMaterialEnabled; attr->CullFace = ctx->Polygon.CullFlag; attr->DepthClamp = ctx->Transform.DepthClamp; + attr->DepthClampNear = ctx->Transform.DepthClampNear; + attr->DepthClampFar = ctx->Transform.DepthClampFar; attr->DepthTest = ctx->Depth.Test; attr->Dither = ctx->Color.DitherFlag; attr->Fog = ctx->Fog.Enabled; @@ -629,6 +633,10 @@ pop_enable_group(struct gl_context *ctx, const struct gl_enable_attrib *enable) TEST_AND_UPDATE(ctx->Polygon.CullFlag, enable->CullFace, GL_CULL_FACE); TEST_AND_UPDATE(ctx->Transform.DepthClamp, enable->DepthClamp, GL_DEPTH_CLAMP); + TEST_AND_UPDATE(ctx->Transform.DepthClampNear, enable->DepthClampNear, + GL_DEPTH_CLAMP_NEAR_AMD); + TEST_AND_UPDATE(ctx->Transform.DepthClampFar, enable->DepthClampFar, + GL_DEPTH_CLAMP_FAR_AMD); TEST_AND_UPDATE(ctx->Depth.Test, enable->DepthTest, GL_DEPTH_TEST); TEST_AND_UPDATE(ctx->Color.DitherFlag, enable->Dither, GL_DITHER); TEST_AND_UPDATE(ctx->Fog.Enabled, enable->Fog, GL_FOG); @@ -1150,6 +1158,8 @@ _mesa_PopAttrib(void) ctx->DriverFlags.NewClipPlaneEnable | ctx->DriverFlags.NewDepth | ctx->DriverFlags.NewDepthClamp | + ctx->DriverFlags.NewDepthClampNear | + ctx->DriverFlags.NewDepthClampFar | ctx->DriverFlags.NewFramebufferSRGB | ctx->DriverFlags.NewLineState | ctx->DriverFlags.NewLogicOp | @@ -1436,6 +1446,12 @@ _mesa_PopAttrib(void) if (xform->DepthClamp != ctx->Transform.DepthClamp) _mesa_set_enable(ctx, GL_DEPTH_CLAMP, ctx->Transform.DepthClamp); + if (xform->DepthClampNear != ctx->Transform.DepthClampNear) + _mesa_set_enable(ctx, GL_DEPTH_CLAMP_NEAR_AMD, + ctx->Transform.DepthClampNear); + if (xform->DepthClampFar != ctx->Transform.DepthClampFar) + _mesa_set_enable(ctx, GL_DEPTH_CLAMP_FAR_AMD, + ctx->Transform.DepthClampFar); if (ctx->Extensions.ARB_clip_control) _mesa_ClipControl(xform->ClipOrigin, xform->ClipDepthMode); } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.
Add some basic types and storage for the AMD_depth_clamp_separate extension. Signed-off-by: Sagar Ghuge --- include/GL/glcorearb.h | 2 ++ src/mesa/main/extensions_table.h | 1 + src/mesa/main/mtypes.h | 9 + 3 files changed, 12 insertions(+) diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h index a78bbb6e18..d73ca5a8df 100644 --- a/include/GL/glcorearb.h +++ b/include/GL/glcorearb.h @@ -1558,6 +1558,8 @@ typedef int64_t GLint64; #define GL_MAX_FRAGMENT_INPUT_COMPONENTS 0x9125 #define GL_CONTEXT_PROFILE_MASK 0x9126 #define GL_DEPTH_CLAMP0x864F +#define GL_DEPTH_CLAMP_NEAR_AMD 0x901E +#define GL_DEPTH_CLAMP_FAR_AMD0x901F #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C #define GL_FIRST_VERTEX_CONVENTION0x8E4D #define GL_LAST_VERTEX_CONVENTION 0x8E4E diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index 3f01896cae..8dc668e087 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -9,6 +9,7 @@ EXT(3DFX_texture_compression_FXT1 , TDFX_texture_compression_FXT1 , GLL, GLC, x , x , 1999) EXT(AMD_conservative_depth , ARB_conservative_depth , GLL, GLC, x , x , 2009) +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate , x , GLC, x , x , 2009) EXT(AMD_draw_buffers_blend , ARB_draw_buffers_blend , GLL, GLC, x , x , 2009) EXT(AMD_performance_monitor , AMD_performance_monitor , GLL, GLC, x , ES2, 2007) EXT(AMD_pinned_memory , AMD_pinned_memory , GLL, GLC, x , x , 2013) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index d71872835d..406746a84c 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1280,6 +1280,8 @@ struct gl_transform_attrib GLboolean RescaleNormals; /**< GL_EXT_rescale_normal */ GLboolean RasterPositionUnclipped; /**< GL_IBM_rasterpos_clip */ GLboolean DepthClamp; /**< GL_ARB_depth_clamp */ + GLboolean DepthClampNear; /**< GL_AMD_depth_clamp_separate */ + GLboolean DepthClampFar;/**< GL_AMD_depth_clamp_separate */ /** GL_ARB_clip_control */ GLenum16 ClipOrigin; /**< GL_LOWER_LEFT or GL_UPPER_LEFT */ GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */ @@ -4235,6 +4237,7 @@ struct gl_extensions GLboolean OES_texture_view; GLboolean OES_viewport_array; /* vendor extensions */ + GLboolean AMD_depth_clamp_separate; GLboolean AMD_performance_monitor; GLboolean AMD_pinned_memory; GLboolean AMD_seamless_cubemap_per_texture; @@ -4577,6 +4580,12 @@ struct gl_driver_flags /** gl_context::Transform::DepthClamp */ uint64_t NewDepthClamp; + /** gl_context::Transform::DepthClampNear */ + uint64_t NewDepthClampNear; + + /** gl_context::Transform::DepthClampFar */ + uint64_t NewDepthClampFar; + /** gl_context::Line */ uint64_t NewLineState; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 4/5] mesa: implement glGet for AMD_depth_clamp_separate
Signed-off-by: Sagar Ghuge --- src/mesa/main/get.c | 1 + src/mesa/main/get_hash_params.py | 5 + 2 files changed, 6 insertions(+) diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index db0079beb5..b310f014b7 100644 --- a/src/mesa/main/get.c +++ b/src/mesa/main/get.c @@ -468,6 +468,7 @@ EXTRA_EXT(NV_texture_rectangle); EXTRA_EXT(EXT_stencil_two_side); EXTRA_EXT(EXT_depth_bounds_test); EXTRA_EXT(ARB_depth_clamp); +EXTRA_EXT(AMD_depth_clamp_separate); EXTRA_EXT(ATI_fragment_shader); EXTRA_EXT(EXT_provoking_vertex); EXTRA_EXT(ARB_fragment_shader); diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py index 618e306e50..edf3c982d0 100644 --- a/src/mesa/main/get_hash_params.py +++ b/src/mesa/main/get_hash_params.py @@ -982,6 +982,11 @@ descriptor=[ # GL_ARB_indirect_parameters [ "PARAMETER_BUFFER_BINDING_ARB", "LOC_CUSTOM, TYPE_INT, 0, extra_ARB_indirect_parameters" ], + +# GL 4.1 +# GL_AMD_depth_clamp_separate + [ "DEPTH_CLAMP_NEAR_AMD", "CONTEXT_BOOL(Transform.DepthClampNear), extra_AMD_depth_clamp_separate" ], + [ "DEPTH_CLAMP_FAR_AMD", "CONTEXT_BOOL(Transform.DepthClampFar), extra_AMD_depth_clamp_separate" ], ]}, ] -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 2/5] glapi: define AMD_depth_clamp_separate
Signed-off-by: Sagar Ghuge --- src/mapi/glapi/gen/AMD_depth_clamp_separate.xml | 15 +++ src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml | 3 +++ src/mapi/glapi/gen/meson.build | 1 + 4 files changed, 20 insertions(+) create mode 100644 src/mapi/glapi/gen/AMD_depth_clamp_separate.xml diff --git a/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml new file mode 100644 index 00..b7142f1b7b --- /dev/null +++ b/src/mapi/glapi/gen/AMD_depth_clamp_separate.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index 93acabd968..ab369a0d33 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -188,6 +188,7 @@ API_XML = \ ARB_vertex_attrib_64bit.xml \ ARB_vertex_attrib_binding.xml \ ARB_viewport_array.xml \ + AMD_depth_clamp_separate.xml \ AMD_draw_buffers_blend.xml \ AMD_performance_monitor.xml \ ARB_vertex_type_2_10_10_10_rev.xml \ diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index 49807e1ea5..b85cae2eb4 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -12939,6 +12939,9 @@ http://www.w3.org/2001/XInclude"/> +http://www.w3.org/2001/XInclude"/> + diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build index a6a93cc83b..9c3f243aa9 100644 --- a/src/mapi/glapi/gen/meson.build +++ b/src/mapi/glapi/gen/meson.build @@ -95,6 +95,7 @@ api_xml_files = files( 'ARB_vertex_attrib_64bit.xml', 'ARB_vertex_attrib_binding.xml', 'ARB_viewport_array.xml', + 'AMD_depth_clamp_separate.xml', 'AMD_draw_buffers_blend.xml', 'AMD_performance_monitor.xml', 'ARB_vertex_type_2_10_10_10_rev.xml', -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC][PATCH 1/5] mesa: add support for GL_AMD_depth_clamp_separate tokens
_mesa_set_enable() and _mesa_IsEnabled() extended to accept new two tokens GL_DEPTH_CLAMP_NEAR_AMD and GL_DEPTH_CLAMP_FAR_AMD. Signed-off-by: Sagar Ghuge --- src/mesa/main/enable.c | 44 +- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c index d1b2f3a962..54729743ab 100644 --- a/src/mesa/main/enable.c +++ b/src/mesa/main/enable.c @@ -1013,6 +1013,32 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state) _NEW_TRANSFORM); ctx->NewDriverState |= ctx->DriverFlags.NewDepthClamp; ctx->Transform.DepthClamp = state; + ctx->Transform.DepthClampNear = state; + ctx->Transform.DepthClampFar = state; + break; + + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampNear == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClampNear ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClampNear; + ctx->Transform.DepthClampNear = state; + break; + + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate, cap); + if (ctx->Transform.DepthClampFar == state) +return; + FLUSH_VERTICES(ctx, ctx->DriverFlags.NewDepthClampFar ? 0 : + _NEW_TRANSFORM); + ctx->NewDriverState |= ctx->DriverFlags.NewDepthClampFar; + ctx->Transform.DepthClampFar = state; break; case GL_FRAGMENT_SHADER_ATI: @@ -1684,7 +1710,23 @@ _mesa_IsEnabled( GLenum cap ) if (!_mesa_is_desktop_gl(ctx)) goto invalid_enum_error; CHECK_EXTENSION(ARB_depth_clamp); - return ctx->Transform.DepthClamp; + return (ctx->Transform.DepthClamp || +ctx->Transform.DepthClampNear || +ctx->Transform.DepthClampFar); + + /* GL_AMD_depth_clamp_separate */ + case GL_DEPTH_CLAMP_NEAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return ctx->Transform.DepthClampNear; + + /* GL_AMD_depth_clamp_separate */ + case GL_DEPTH_CLAMP_FAR_AMD: + if (!_mesa_is_desktop_gl(ctx)) +goto invalid_enum_error; + CHECK_EXTENSION(AMD_depth_clamp_separate); + return ctx->Transform.DepthClampFar; case GL_FRAGMENT_SHADER_ATI: if (ctx->API != API_OPENGL_COMPAT) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Expose ARB_base_instance extension
The extension requires at least OpenGL 3.0 and OpenGL ES 3.0. Fixes two ext_base_instance tests: arb_base_instance-baseinstance-doesnt-affect-gl-instance-id_gles3 arb_base_instance-drawarrays_gles3 Signed-off-by: Sagar Ghuge --- src/mesa/drivers/dri/i965/intel_extensions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c index f837356478..9d119d0b4c 100644 --- a/src/mesa/drivers/dri/i965/intel_extensions.c +++ b/src/mesa/drivers/dri/i965/intel_extensions.c @@ -315,7 +315,7 @@ intelInitExtensions(struct gl_context *ctx) if (devinfo->gen >= 6) ctx->Extensions.INTEL_performance_query = true; - if (ctx->API == API_OPENGL_CORE) + if (ctx->API != API_OPENGL_COMPAT) ctx->Extensions.ARB_base_instance = true; if (ctx->API != API_OPENGL_CORE) ctx->Extensions.ARB_color_buffer_float = true; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev