Re: [Mesa-dev] [PATCH 1/7] radv/meta: add srgb conversion to end of resolve shader.

[Dave Airlie]

On 5 May 2017 at 01:46, Bas Nieuwenhuizen  wrote:
> Can't we just use a non-SRGB image view into the destination image?
> That should save some operations.

We could possibly here, but not in the subpass path, which a later
patch enables.

I wasn't sure it was worth the effort.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/7] radv/meta: add srgb conversion to end of resolve shader.

[Bas Nieuwenhuizen]

This series is

Reviewed-by: Bas Nieuwenhuizen 

On Thu, May 4, 2017 at 5:27 AM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> If we are resolving into an srgb dest, we need to convert
> to linear so the store does the conversion back.
>
> This should fix some wierdness seen when we subresolves
> hit the compute path.
>
> Signed-off-by: Dave Airlie 
> ---
>  src/amd/vulkan/radv_meta_resolve_cs.c | 58 
> ---
>  src/amd/vulkan/radv_private.h |  1 +
>  src/amd/vulkan/vk_format.h|  7 +
>  3 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c 
> b/src/amd/vulkan/radv_meta_resolve_cs.c
> index dc4a1bb..99a3a06 100644
> --- a/src/amd/vulkan/radv_meta_resolve_cs.c
> +++ b/src/amd/vulkan/radv_meta_resolve_cs.c
> @@ -32,7 +32,7 @@
>  #include "vk_format.h"
>
>  static nir_shader *
> -build_resolve_compute_shader(struct radv_device *dev, bool is_integer, int 
> samples)
> +build_resolve_compute_shader(struct radv_device *dev, bool is_integer, bool 
> is_srgb, int samples)
>  {
> nir_builder b;
> char name[64];
> @@ -45,7 +45,7 @@ build_resolve_compute_shader(struct radv_device *dev, bool 
> is_integer, int sampl
>  false,
>  false,
>  GLSL_TYPE_FLOAT);
> -   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? 
> "int" : "float");
> +   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? 
> "int" : (is_srgb ? "srgb" : "float"));
> nir_builder_init_simple_shader(, NULL, MESA_SHADER_COMPUTE, NULL);
> b.shader->info->name = ralloc_strdup(b.shader, name);
> b.shader->info->cs.local_size[0] = 16;
> @@ -158,6 +158,44 @@ build_resolve_compute_shader(struct radv_device *dev, 
> bool is_integer, int sampl
> b.cursor = nir_after_cf_node(_if->cf_node);
>
> nir_ssa_def *newv = nir_load_var(, color);
> +
> +   if (is_srgb) {
> +   nir_const_value v;
> +   unsigned i;
> +   v.u32[0] = 0x3b4d2e1c; // 0.00313080009
> +
> +   nir_ssa_def *cmp[3];
> +   for (i = 0; i < 3; i++)
> +   cmp[i] = nir_flt(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +
> +   nir_ssa_def *ltvals[3];
> +   v.f32[0] = 12.92;
> +   for (i = 0; i < 3; i++)
> +   ltvals[i] = nir_fmul(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +
> +   nir_ssa_def *gtvals[3];
> +
> +   for (i = 0; i < 3; i++) {
> +   v.f32[0] = 1.0/2.4;
> +   gtvals[i] = nir_fpow(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +   v.f32[0] = 1.055;
> +   gtvals[i] = nir_fmul(, gtvals[i],
> +nir_build_imm(, 1, 32, v));
> +   v.f32[0] = 0.055;
> +   gtvals[i] = nir_fsub(, gtvals[i],
> +nir_build_imm(, 1, 32, v));
> +   }
> +
> +   nir_ssa_def *comp[4];
> +   for (i = 0; i < 3; i++)
> +   comp[i] = nir_bcsel(, cmp[i], ltvals[i], gtvals[i]);
> +   comp[3] = nir_channels(, newv, 3);
> +   newv = nir_vec(, comp, 4);
> +   }
> +
> nir_ssa_def *coord = nir_iadd(, global_id, _offset->dest.ssa);
> nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, 
> nir_intrinsic_image_store);
> store->src[0] = nir_src_for_ssa(coord);
> @@ -230,12 +268,13 @@ static VkResult
>  create_resolve_pipeline(struct radv_device *device,
> int samples,
> bool is_integer,
> +   bool is_srgb,
> VkPipeline *pipeline)
>  {
> VkResult result;
> struct radv_shader_module cs = { .nir = NULL };
>
> -   cs.nir = build_resolve_compute_shader(device, is_integer, samples);
> +   cs.nir = build_resolve_compute_shader(device, is_integer, is_srgb, 
> samples);
>
> /* compute shader */
>
> @@ -282,12 +321,15 @@ radv_device_init_meta_resolve_compute_state(struct 
> radv_device *device)
> for (uint32_t i = 0; i < MAX_SAMPLES_LOG2; ++i) {
> uint32_t samples = 1 << i;
>
> -   res = create_resolve_pipeline(device, samples, false,
> +   res = create_resolve_pipeline(device, samples, false, false,
>   
> 

Re: [Mesa-dev] [PATCH 1/7] radv/meta: add srgb conversion to end of resolve shader.

[Bas Nieuwenhuizen]

Can't we just use a non-SRGB image view into the destination image?
That should save some operations.

On Thu, May 4, 2017 at 5:27 AM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> If we are resolving into an srgb dest, we need to convert
> to linear so the store does the conversion back.
>
> This should fix some wierdness seen when we subresolves
> hit the compute path.
>
> Signed-off-by: Dave Airlie 
> ---
>  src/amd/vulkan/radv_meta_resolve_cs.c | 58 
> ---
>  src/amd/vulkan/radv_private.h |  1 +
>  src/amd/vulkan/vk_format.h|  7 +
>  3 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c 
> b/src/amd/vulkan/radv_meta_resolve_cs.c
> index dc4a1bb..99a3a06 100644
> --- a/src/amd/vulkan/radv_meta_resolve_cs.c
> +++ b/src/amd/vulkan/radv_meta_resolve_cs.c
> @@ -32,7 +32,7 @@
>  #include "vk_format.h"
>
>  static nir_shader *
> -build_resolve_compute_shader(struct radv_device *dev, bool is_integer, int 
> samples)
> +build_resolve_compute_shader(struct radv_device *dev, bool is_integer, bool 
> is_srgb, int samples)
>  {
> nir_builder b;
> char name[64];
> @@ -45,7 +45,7 @@ build_resolve_compute_shader(struct radv_device *dev, bool 
> is_integer, int sampl
>  false,
>  false,
>  GLSL_TYPE_FLOAT);
> -   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? 
> "int" : "float");
> +   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? 
> "int" : (is_srgb ? "srgb" : "float"));
> nir_builder_init_simple_shader(, NULL, MESA_SHADER_COMPUTE, NULL);
> b.shader->info->name = ralloc_strdup(b.shader, name);
> b.shader->info->cs.local_size[0] = 16;
> @@ -158,6 +158,44 @@ build_resolve_compute_shader(struct radv_device *dev, 
> bool is_integer, int sampl
> b.cursor = nir_after_cf_node(_if->cf_node);
>
> nir_ssa_def *newv = nir_load_var(, color);
> +
> +   if (is_srgb) {
> +   nir_const_value v;
> +   unsigned i;
> +   v.u32[0] = 0x3b4d2e1c; // 0.00313080009
> +
> +   nir_ssa_def *cmp[3];
> +   for (i = 0; i < 3; i++)
> +   cmp[i] = nir_flt(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +
> +   nir_ssa_def *ltvals[3];
> +   v.f32[0] = 12.92;
> +   for (i = 0; i < 3; i++)
> +   ltvals[i] = nir_fmul(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +
> +   nir_ssa_def *gtvals[3];
> +
> +   for (i = 0; i < 3; i++) {
> +   v.f32[0] = 1.0/2.4;
> +   gtvals[i] = nir_fpow(, nir_channel(, newv, i),
> +nir_build_imm(, 1, 32, v));
> +   v.f32[0] = 1.055;
> +   gtvals[i] = nir_fmul(, gtvals[i],
> +nir_build_imm(, 1, 32, v));
> +   v.f32[0] = 0.055;
> +   gtvals[i] = nir_fsub(, gtvals[i],
> +nir_build_imm(, 1, 32, v));
> +   }
> +
> +   nir_ssa_def *comp[4];
> +   for (i = 0; i < 3; i++)
> +   comp[i] = nir_bcsel(, cmp[i], ltvals[i], gtvals[i]);
> +   comp[3] = nir_channels(, newv, 3);
> +   newv = nir_vec(, comp, 4);
> +   }
> +
> nir_ssa_def *coord = nir_iadd(, global_id, _offset->dest.ssa);
> nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, 
> nir_intrinsic_image_store);
> store->src[0] = nir_src_for_ssa(coord);
> @@ -230,12 +268,13 @@ static VkResult
>  create_resolve_pipeline(struct radv_device *device,
> int samples,
> bool is_integer,
> +   bool is_srgb,
> VkPipeline *pipeline)
>  {
> VkResult result;
> struct radv_shader_module cs = { .nir = NULL };
>
> -   cs.nir = build_resolve_compute_shader(device, is_integer, samples);
> +   cs.nir = build_resolve_compute_shader(device, is_integer, is_srgb, 
> samples);
>
> /* compute shader */
>
> @@ -282,12 +321,15 @@ radv_device_init_meta_resolve_compute_state(struct 
> radv_device *device)
> for (uint32_t i = 0; i < MAX_SAMPLES_LOG2; ++i) {
> uint32_t samples = 1 << i;
>
> -   res = create_resolve_pipeline(device, samples, false,
> +   res = create_resolve_pipeline(device, samples, false, false,
>   
> 

[Mesa-dev] [PATCH 1/7] radv/meta: add srgb conversion to end of resolve shader.

[Dave Airlie]

From: Dave Airlie 

If we are resolving into an srgb dest, we need to convert
to linear so the store does the conversion back.

This should fix some wierdness seen when we subresolves
hit the compute path.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_meta_resolve_cs.c | 58 ---
 src/amd/vulkan/radv_private.h |  1 +
 src/amd/vulkan/vk_format.h|  7 +
 3 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c 
b/src/amd/vulkan/radv_meta_resolve_cs.c
index dc4a1bb..99a3a06 100644
--- a/src/amd/vulkan/radv_meta_resolve_cs.c
+++ b/src/amd/vulkan/radv_meta_resolve_cs.c
@@ -32,7 +32,7 @@
 #include "vk_format.h"
 
 static nir_shader *
-build_resolve_compute_shader(struct radv_device *dev, bool is_integer, int 
samples)
+build_resolve_compute_shader(struct radv_device *dev, bool is_integer, bool 
is_srgb, int samples)
 {
nir_builder b;
char name[64];
@@ -45,7 +45,7 @@ build_resolve_compute_shader(struct radv_device *dev, bool 
is_integer, int sampl
 false,
 false,
 GLSL_TYPE_FLOAT);
-   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? "int" 
: "float");
+   snprintf(name, 64, "meta_resolve_cs-%d-%s", samples, is_integer ? "int" 
: (is_srgb ? "srgb" : "float"));
nir_builder_init_simple_shader(, NULL, MESA_SHADER_COMPUTE, NULL);
b.shader->info->name = ralloc_strdup(b.shader, name);
b.shader->info->cs.local_size[0] = 16;
@@ -158,6 +158,44 @@ build_resolve_compute_shader(struct radv_device *dev, bool 
is_integer, int sampl
b.cursor = nir_after_cf_node(_if->cf_node);
 
nir_ssa_def *newv = nir_load_var(, color);
+
+   if (is_srgb) {
+   nir_const_value v;
+   unsigned i;
+   v.u32[0] = 0x3b4d2e1c; // 0.00313080009
+
+   nir_ssa_def *cmp[3];
+   for (i = 0; i < 3; i++)
+   cmp[i] = nir_flt(, nir_channel(, newv, i),
+nir_build_imm(, 1, 32, v));
+
+   nir_ssa_def *ltvals[3];
+   v.f32[0] = 12.92;
+   for (i = 0; i < 3; i++)
+   ltvals[i] = nir_fmul(, nir_channel(, newv, i),
+nir_build_imm(, 1, 32, v));
+
+   nir_ssa_def *gtvals[3];
+
+   for (i = 0; i < 3; i++) {
+   v.f32[0] = 1.0/2.4;
+   gtvals[i] = nir_fpow(, nir_channel(, newv, i),
+nir_build_imm(, 1, 32, v));
+   v.f32[0] = 1.055;
+   gtvals[i] = nir_fmul(, gtvals[i],
+nir_build_imm(, 1, 32, v));
+   v.f32[0] = 0.055;
+   gtvals[i] = nir_fsub(, gtvals[i],
+nir_build_imm(, 1, 32, v));
+   }
+
+   nir_ssa_def *comp[4];
+   for (i = 0; i < 3; i++)
+   comp[i] = nir_bcsel(, cmp[i], ltvals[i], gtvals[i]);
+   comp[3] = nir_channels(, newv, 3);
+   newv = nir_vec(, comp, 4);
+   }
+
nir_ssa_def *coord = nir_iadd(, global_id, _offset->dest.ssa);
nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, 
nir_intrinsic_image_store);
store->src[0] = nir_src_for_ssa(coord);
@@ -230,12 +268,13 @@ static VkResult
 create_resolve_pipeline(struct radv_device *device,
int samples,
bool is_integer,
+   bool is_srgb,
VkPipeline *pipeline)
 {
VkResult result;
struct radv_shader_module cs = { .nir = NULL };
 
-   cs.nir = build_resolve_compute_shader(device, is_integer, samples);
+   cs.nir = build_resolve_compute_shader(device, is_integer, is_srgb, 
samples);
 
/* compute shader */
 
@@ -282,12 +321,15 @@ radv_device_init_meta_resolve_compute_state(struct 
radv_device *device)
for (uint32_t i = 0; i < MAX_SAMPLES_LOG2; ++i) {
uint32_t samples = 1 << i;
 
-   res = create_resolve_pipeline(device, samples, false,
+   res = create_resolve_pipeline(device, samples, false, false,
  
>resolve_compute.rc[i].pipeline);
 
-   res = create_resolve_pipeline(device, samples, true,
+   res = create_resolve_pipeline(device, samples, true, false,
  
>resolve_compute.rc[i].i_pipeline);
 
+   res = create_resolve_pipeline(device, samples, false, true,
+