Re: [Mesa-dev] [PATCH] intel/compiler: Set flag reg/subreg number properly

2019-04-08 Thread Sagar Ghuge
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

2019-03-27 Thread Sagar Ghuge
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

2019-03-27 Thread Sagar Ghuge
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

2019-03-26 Thread Sagar Ghuge
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

2019-03-26 Thread Sagar Ghuge
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

2019-03-25 Thread Sagar Ghuge
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

2019-03-24 Thread Sagar Ghuge
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

2019-03-24 Thread 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 v2 9/9] anv: Enabled the VK_EXT_sample_locations extension

2019-03-12 Thread Sagar Ghuge
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

2019-03-12 Thread Sagar Ghuge
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

2019-03-12 Thread Sagar Ghuge
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

2019-03-12 Thread Sagar Ghuge
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

2019-03-11 Thread Sagar Ghuge
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

2019-03-11 Thread Sagar Ghuge

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

2018-12-08 Thread Sagar Ghuge
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

2018-12-08 Thread Sagar Ghuge
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

2018-11-15 Thread Sagar Ghuge
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

2018-11-14 Thread Sagar Ghuge
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.

2018-11-09 Thread Sagar Ghuge

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

2018-10-26 Thread Sagar Ghuge
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

2018-10-25 Thread Sagar Ghuge


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

2018-10-25 Thread Sagar Ghuge
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

2018-10-24 Thread Sagar Ghuge
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

2018-10-24 Thread Sagar Ghuge
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

2018-10-24 Thread Sagar Ghuge
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

2018-10-22 Thread Sagar Ghuge


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

2018-10-22 Thread Sagar Ghuge


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

2018-10-22 Thread Sagar Ghuge
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

2018-10-19 Thread Sagar Ghuge
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

2018-10-16 Thread Sagar Ghuge
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

2018-10-05 Thread Sagar Ghuge
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

2018-09-13 Thread Sagar Ghuge
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

2018-09-06 Thread Sagar Ghuge
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

2018-09-06 Thread Sagar Ghuge
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

2018-09-06 Thread Sagar Ghuge
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

2018-09-06 Thread Sagar Ghuge


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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge

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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-09-05 Thread Sagar Ghuge
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

2018-08-29 Thread Sagar Ghuge
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

2018-08-29 Thread Sagar Ghuge
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

2018-08-28 Thread Sagar Ghuge
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

2018-08-27 Thread Sagar Ghuge
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

2018-08-27 Thread Sagar Ghuge
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

2018-08-23 Thread Sagar Ghuge
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

2018-08-23 Thread Sagar Ghuge
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

2018-08-23 Thread Sagar Ghuge
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

2018-08-22 Thread Sagar Ghuge

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

2018-08-22 Thread Sagar Ghuge
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

2018-08-22 Thread Sagar Ghuge
_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

2018-08-22 Thread Sagar Ghuge
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.

2018-08-22 Thread Sagar Ghuge
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.

2018-08-22 Thread Sagar Ghuge
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.

2018-08-22 Thread Sagar Ghuge
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

2018-08-22 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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.

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
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

2018-08-21 Thread Sagar Ghuge
_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.

2018-08-20 Thread Sagar Ghuge
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

2018-08-20 Thread Sagar Ghuge
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

2018-08-20 Thread Sagar Ghuge
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

2018-08-20 Thread Sagar Ghuge


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

2018-08-20 Thread Sagar Ghuge
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

2018-08-20 Thread Sagar Ghuge


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

2018-08-16 Thread Sagar Ghuge
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

2018-08-15 Thread Sagar Ghuge
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

2018-08-14 Thread Sagar Ghuge


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.

2018-08-13 Thread Sagar Ghuge
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

2018-08-09 Thread Sagar Ghuge


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

2018-08-03 Thread Sagar Ghuge
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

2018-08-02 Thread Sagar Ghuge
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

2018-08-02 Thread Sagar Ghuge
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

2018-08-01 Thread Sagar Ghuge
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

2018-08-01 Thread Sagar Ghuge
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.

2018-08-01 Thread Sagar Ghuge
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

2018-08-01 Thread Sagar Ghuge
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

2018-08-01 Thread Sagar Ghuge
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

2018-08-01 Thread Sagar Ghuge
_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

2018-07-25 Thread Sagar Ghuge
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