Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tapani Pälli]

On 11/22/2016 09:15 AM, Xu, Randy wrote:

Hi, Tapani

Can you share your detailed CTS result summary and logs? We will double check 
it.
Btw, we do the CTS test on SKL GT2 and android-ia.


Sure, I'll send you the log file. I'm running on Android-IA as well.



Thanks,
Randy

-Original Message-
From: Palli, Tapani
Sent: Tuesday, November 22, 2016 2:44 PM
To: Xu, Randy ; Tomasz Figa 
Cc: Long, Zhifang ; Liu, Zhiquan ; Emil Velikov 
; Kondapally, Kalyan ; ML mesa-dev 

Subject: Re: [Mesa-dev] EGL/android: pbuffer implementation.



On 11/22/2016 08:39 AM, Xu, Randy wrote:

Hi ,Tapani

With this pbuffer patch, only three EGL dEQP failure remained on
Android platform , they are under investigation now

dEQP-EGL.functional.image.create#gles2_renderbuffer_depth16_depth_buffer fail due to 
"=== with config {glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} 
==="
dEQP-EGL.functional.image.modify#renderbuffer_depth16_renderbuffer_clear_depth fail due 
to "=== with config 
{glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} ==="
dEQP-EGL.functional.image.render_multiple_contexts#gles2_renderbuffer_depth16_depth_buffer
 fail due to "=== with config 
{glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} ==="

The multithread cases can pass on Android.


Not for me


Thanks,
Randy


-Original Message-
From: Palli, Tapani
Sent: Tuesday, November 22, 2016 2:32 PM
To: Tomasz Figa 
Cc: Long, Zhifang ; Liu, Zhiquan
; Xu, Randy ; Emil Velikov
; Kondapally, Kalyan
; ML mesa-dev

Subject: Re: [Mesa-dev] EGL/android: pbuffer implementation.



On 11/22/2016 08:08 AM, Tapani Pälli wrote:



On 11/22/2016 08:02 AM, Tomasz Figa wrote:

On Tue, Nov 22, 2016 at 2:44 PM, Tapani Pälli

wrote:



On 11/21/2016 04:15 PM, Emil Velikov wrote:


On 21 November 2016 at 07:23, Tomasz Figa  wrote:


Hi,

On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan

wrote:

@@ -775,14 +837,6 @@ droid_add_configs_for_visuals(_EGLDriver
*drv, _EGLDisplay *dpy)
for (i = 0; dri2_dpy->driver_configs[i]; i++) {
   const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
   struct dri2_egl_config *dri2_conf;
-  unsigned int double_buffered = 0;
-
-  dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[i],
- __DRI_ATTRIB_DOUBLE_BUFFER, _buffered);
-
-  /* support only double buffered configs */
-  if (!double_buffered)
- continue;



Does it really just work like this? Last time I checked the list
of configs generated after such change it contained
single-buffered-only configs with window surface bit set and
double-buffered-only configs with pbuffer bit set, both of which
are invalid. Moreover, this caused some dEQP tests to fail. How was this patch 
tested?


Afaict the patch has (should have?) gone through the Intel CI,
although I'm not sure if the latter builds/runs Android/ARC.



It did go through CI but it does not run any Android/ARC tests.
AFAIK Zhiquan did run pbuffer related tests and reported them
passing on Android.
Which dEQP tests were regressed?


The tests I'm concerned with are all
dEQP-EGL.functional.query_context.get_current_context.*_pbuffer tests.
In my testing on BSW and BDW, a change identical to the part quoted
above made eglGetConfigs() return double buffered configs for
pbuffers, which made the mentioned tests fail because of invalid
config at surface creation time (or SIGSEGV, before my NULL pointer
fix - 94282b6dd0 egl/android: Check return value of
dri2_get_dri_config()).


OK thanks, we will run these tests today and see if something got broken.


I did run all EGL tests now and all of the above 'get_current_context.*pbuffer' 
tests pass (some formats are skipped though). I don't have specifically BSW and 
BDW right now though, this was run on Joule (BXT).

With my run I got 7 failing EGL tests:

dEQP-EGL.functional.image.modify.tex_rgb5_a1_tex_subimage_rgba8
dEQP-EGL.functional.image.render_multiple_contexts.gles2_renderbuffer_
depth16_depth_buffer dEQP-EGL.functional.multithread.window
dEQP-EGL.functional.multithread.pbuffer_window
dEQP-EGL.functional.multithread.window_context
dEQP-EGL.functional.multithread.pbuffer_window_context
dEQP-EGL.performance.make_current.multi_surface.context_windows_10

(1999 passing, 1616 skipped)

Not sure if multithread ones ever passed. On desktop many of them fail because 
dEQP does not call XInitThreads().



Note that I haven't tested exactly this patch, just a similar
change, identical in terms of the quoted part above. It is also
possible that something changed in other 

[Mesa-dev] [PATCH] mesa/glsl: remove unused uses_builtin_functions field

[Timothy Arceri]

This has been unused since 943b69cddd
---
 src/compiler/glsl/glsl_parser_extras.cpp | 1 -
 src/mesa/main/ff_fragment_shader.cpp | 1 -
 src/mesa/main/mtypes.h   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 88c7822..4f76815 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -1984,7 +1984,6 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct 
gl_shader *shader,
shader->InfoLog = state->info_log;
shader->Version = state->language_version;
shader->IsES = state->es_shader;
-   shader->info.uses_builtin_functions = state->uses_builtin_functions;
 
/* Retain any live IR, but trash the rest. */
reparent_ir(shader->ir, shader->ir);
diff --git a/src/mesa/main/ff_fragment_shader.cpp 
b/src/mesa/main/ff_fragment_shader.cpp
index c2618b7..036c5a3 100644
--- a/src/mesa/main/ff_fragment_shader.cpp
+++ b/src/mesa/main/ff_fragment_shader.cpp
@@ -1256,7 +1256,6 @@ create_new_program(struct gl_context *ctx, struct 
state_key *key)
 
p.shader->CompileStatus = true;
p.shader->Version = state->language_version;
-   p.shader->info.uses_builtin_functions = state->uses_builtin_functions;
p.shader_program->Shaders =
   (gl_shader **)malloc(sizeof(*p.shader_program->Shaders));
p.shader_program->Shaders[0] = p.shader;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 0b38b4f..93a70b4 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2234,7 +2234,6 @@ struct gl_subroutine_function
  */
 struct gl_shader_info
 {
-   bool uses_builtin_functions;
bool uses_gl_fragcoord;
bool redeclares_gl_fragcoord;
bool ARB_fragment_coord_conventions_enable;
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 1/4] radv/meta: cleanup resolve vertex state emission

[Bas Nieuwenhuizen]

For the series:
Reviewed-by: Bas Nieuwenhuizen 

On Tue, Nov 22, 2016 at 7:34 AM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> For the hw resolve there is no need to emit any sort
> of texture coordinates, so drop them all in the meta path.
>
> Signed-off-by: Dave Airlie 
> ---
>  src/amd/vulkan/radv_meta_resolve.c | 49 
> ++
>  1 file changed, 2 insertions(+), 47 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_meta_resolve.c 
> b/src/amd/vulkan/radv_meta_resolve.c
> index 697a264..66f1466 100644
> --- a/src/amd/vulkan/radv_meta_resolve.c
> +++ b/src/amd/vulkan/radv_meta_resolve.c
> @@ -33,7 +33,6 @@
>   */
>  struct vertex_attrs {
> float position[2]; /**< 3DPRIM_RECTLIST */
> -   float tex_position[2];
>  };
>
>  /* passthrough vertex shader */
> @@ -45,8 +44,6 @@ build_nir_vs(void)
> nir_builder b;
> nir_variable *a_position;
> nir_variable *v_position;
> -   nir_variable *a_tex_position;
> -   nir_variable *v_tex_position;
>
> nir_builder_init_simple_shader(, NULL, MESA_SHADER_VERTEX, NULL);
> b.shader->info->name = ralloc_strdup(b.shader, "meta_resolve_vs");
> @@ -59,16 +56,7 @@ build_nir_vs(void)
>  "gl_Position");
> v_position->data.location = VARYING_SLOT_POS;
>
> -   a_tex_position = nir_variable_create(b.shader, nir_var_shader_in, 
> vec4,
> -"a_tex_position");
> -   a_tex_position->data.location = VERT_ATTRIB_GENERIC1;
> -
> -   v_tex_position = nir_variable_create(b.shader, nir_var_shader_out, 
> vec4,
> -"v_tex_position");
> -   v_tex_position->data.location = VARYING_SLOT_VAR0;
> -
> nir_copy_var(, v_position, a_position);
> -   nir_copy_var(, v_tex_position, a_tex_position);
>
> return b.shader;
>  }
> @@ -79,22 +67,16 @@ build_nir_fs(void)
>  {
> const struct glsl_type *vec4 = glsl_vec4_type();
> nir_builder b;
> -   nir_variable *v_tex_position; /* vec4, varying texture coordinate */
> nir_variable *f_color; /* vec4, fragment output color */
>
> nir_builder_init_simple_shader(, NULL, MESA_SHADER_FRAGMENT, NULL);
> b.shader->info->name = ralloc_asprintf(b.shader,
>"meta_resolve_fs");
>
> -   v_tex_position = nir_variable_create(b.shader, nir_var_shader_in, 
> vec4,
> -"v_tex_position");
> -   v_tex_position->data.location = VARYING_SLOT_VAR0;
> -
> f_color = nir_variable_create(b.shader, nir_var_shader_out, vec4,
>   "f_color");
> f_color->data.location = FRAG_RESULT_DATA0;
> -
> -   nir_copy_var(, f_color, v_tex_position);
> +   nir_store_var(, f_color, nir_imm_vec4(, 0.0, 0.0, 0.0, 1.0), 0xf);
>
> return b.shader;
>  }
> @@ -198,7 +180,7 @@ create_pipeline(struct radv_device *device,
>
> .inputRate = VK_VERTEX_INPUT_RATE_VERTEX
>},
>},
> -  
> .vertexAttributeDescriptionCount = 2,
> +  
> .vertexAttributeDescriptionCount = 1,
>
> .pVertexAttributeDescriptions = (VkVertexInputAttributeDescription[]) {
>{
>/* 
> Position */
> @@ -207,13 +189,6 @@ create_pipeline(struct radv_device *device,
>
> .format = VK_FORMAT_R32G32_SFLOAT,
>
> .offset = offsetof(struct vertex_attrs, position),
>},
> -  {
> -  /* 
> Texture Coordinate */
> -  
> .location = 1,
> -  
> .binding = 0,
> -  
> .format = VK_FORMAT_R32G32_SFLOAT,
> -  
> .offset = offsetof(struct vertex_attrs, tex_position),
> -  },
>},
>},
> 

Re: [Mesa-dev] [PATCH] radv: add support for shader stats dump

[Bas Nieuwenhuizen]

Reviewed-by: Bas Nieuwenhuizen 

On Tue, Nov 22, 2016 at 5:44 AM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> I've started working on a shader-db alike for Vulkan,
> it's based on vktrace and it records pipelines, this
> adds support to dump the shader stats exactly like
> radeonsi does, so I can reuse the shader-db scripts it
> uses.
>
> Signed-off-by: Dave Airlie 
> ---
>  src/amd/vulkan/radv_device.c   |  2 +-
>  src/amd/vulkan/radv_pipeline.c | 84 
> ++
>  src/amd/vulkan/radv_private.h  |  3 ++
>  3 files changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 5acaf56..313d7a5 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -682,7 +682,7 @@ VkResult radv_CreateDevice(
> }
> device->allow_fast_clears = env_var_as_boolean("RADV_FAST_CLEARS", 
> false);
> device->allow_dcc = !env_var_as_boolean("RADV_DCC_DISABLE", true);
> -
> +   device->shader_stats_dump = env_var_as_boolean("RADV_SHADER_STATS", 
> false);
> if (device->allow_fast_clears && device->allow_dcc)
> radv_finishme("DCC fast clears have not been tested\n");
>
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index fca0173..5f3ebb3 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -257,6 +257,81 @@ radv_shader_compile_to_nir(struct radv_device *device,
> return nir;
>  }
>
> +static const char *radv_get_shader_name(struct radv_shader_variant *var,
> +   gl_shader_stage stage)
> +{
> +   switch (stage) {
> +   case MESA_SHADER_VERTEX: return "Vertex Shader as VS";
> +   case MESA_SHADER_FRAGMENT: return "Pixel Shader";
> +   case MESA_SHADER_COMPUTE: return "Compute Shader";
> +   default:
> +   return "Unknown shader";
> +   };
> +
> +}
> +static void radv_dump_pipeline_stats(struct radv_device *device, struct 
> radv_pipeline *pipeline)
> +{
> +   unsigned lds_increment = 
> device->instance->physicalDevice.rad_info.chip_class >= CIK ? 512 : 256;
> +   struct radv_shader_variant *var;
> +   struct ac_shader_config *conf;
> +   int i;
> +   FILE *file = stderr;
> +   unsigned max_simd_waves = 10;
> +   unsigned lds_per_wave = 0;
> +
> +   for (i = 0; i < MESA_SHADER_STAGES; i++) {
> +   if (!pipeline->shaders[i])
> +   continue;
> +   var = pipeline->shaders[i];
> +
> +   conf = >config;
> +
> +   if (i == MESA_SHADER_FRAGMENT) {
> +   lds_per_wave = conf->lds_size * lds_increment +
> +   align(var->info.fs.num_interp * 48, 
> lds_increment);
> +   }
> +
> +   if (conf->num_sgprs) {
> +   if 
> (device->instance->physicalDevice.rad_info.chip_class >= VI)
> +   max_simd_waves = MIN2(max_simd_waves, 800 / 
> conf->num_sgprs);
> +   else
> +   max_simd_waves = MIN2(max_simd_waves, 512 / 
> conf->num_sgprs);
> +   }
> +
> +   if (conf->num_vgprs)
> +   max_simd_waves = MIN2(max_simd_waves, 256 / 
> conf->num_vgprs);
> +
> +   /* LDS is 64KB per CU (4 SIMDs), divided into 16KB blocks per 
> SIMD
> +* that PS can use.
> +*/
> +   if (lds_per_wave)
> +   max_simd_waves = MIN2(max_simd_waves, 16384 / 
> lds_per_wave);

For SI we had 32 KiB/CU? Not that I expect it to matter since we don't
really use LDS for PS.

> +
> +   fprintf(file, "\n%s:\n",
> +   radv_get_shader_name(var, i));
> +   if (i == MESA_SHADER_FRAGMENT) {
> +   fprintf(file, "*** SHADER CONFIG ***\n"
> +   "SPI_PS_INPUT_ADDR = 0x%04x\n"
> +   "SPI_PS_INPUT_ENA  = 0x%04x\n",
> +   conf->spi_ps_input_addr, 
> conf->spi_ps_input_ena);
> +   }
> +   fprintf(file, "*** SHADER STATS ***\n"
> +   "SGPRS: %d\n"
> +   "VGPRS: %d\n"
> +   "Spilled SGPRs: %d\n"
> +   "Spilled VGPRs: %d\n"
> +   "Code Size: %d bytes\n"
> +   "LDS: %d blocks\n"
> +   "Scratch: %d bytes per wave\n"
> +   "Max Waves: %d\n"
> +   "\n\n\n",
> +   conf->num_sgprs, conf->num_vgprs,
> +   conf->spilled_sgprs, conf->spilled_vgprs, 
> var->code_size,
> +   conf->lds_size, 

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tomasz Figa]

On Tue, Nov 22, 2016 at 3:15 PM, Liu, Zhiquan  wrote:
> Hi Tomasz,
>
> Thanks for you commends.
>> >
>>
>> Looks like this patch has already landed, but please let me try to
>> confirm some things here anyway. Would you mind keeping me on CC for
>> any future patches for the EGL/Android module? (I believe
>> scripts/get_reviewer.pl should already include me on the list of
>> suggested reviewers, similarly for Rob Herring, who did a great job
>> before helping us with testing on platforms other than i915.)
>>
>> [snip]
> I just know the script, I will run next time.
>>
>> > @@ -353,6 +353,18 @@ droid_destroy_surface(_EGLDriver *drv,
>> _EGLDisplay *disp, _EGLSurface *surf)
>> >dri2_surf->window->common.decRef(_surf->window->common);
>> > }
>> >
>> > +   if (dri2_surf->dri_image_back) {
>> > +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back",
>> > + __func__,
>> __LINE__);
>> > +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>> > +  dri2_surf->dri_image_back = NULL;
>> > +   }
>>
>> Is this a fix for another bug by any chance? Note that we already take
>> care of dri_image_back for window surfaces in
>> droid_window_cancel_buffer(), which calls droid_window_enqueue_buffer(), 
>> which does the real free on the image.
>> It doesn't hurt to have it here as well, though, so treat this just as
>> a random thought of mine.
> IMHO,droid_window_cancel_buffer should not call droid_window_enqueue_buffer() 
> It should call dri2_surf->window->cancelBuffer, this should change in another 
> patch.
> So we add destroyImage here.

Yeah, if we change it as you mention it will make perfect sense.

>>
>> > +
>> > +   if (dri2_surf->dri_image_front) {
>> > +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front",
>> > + __func__,
>> __LINE__);
>> > +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>> > +  dri2_surf->dri_image_front = NULL;
>> > +   }
>> > +
>> > (*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);
>> >
>> > free(dri2_surf);
>>
>> [snip]
>>
>> > @@ -439,25 +451,75 @@ droid_image_get_buffers(__DRIdrawable
>> *driDrawable,
>> >struct __DRIimageList *images)  {
>> > struct dri2_egl_surface *dri2_surf = loaderPrivate;
>> > +   struct dri2_egl_display *dri2_dpy =
>> > +  dri2_egl_display(dri2_surf->base.Resource.Display);
>> >
>> > images->image_mask = 0;
>> > +   images->front = NULL;
>> > +   images->back = NULL;
>>
>> I'm not 100% sure this is the expected behavior of this function. My
>> understanding was that image_mask and error code would guard the other
>> members. It would make sense since typically if a function fails it
>> should keep the passed writable arguments unchanged. Is there a
>> precise description of the semantics used by the image loader extension 
>> written down somewhere?
>>
> Here, I just follow the image_mask operation.

Yeah, but my understanding is that images->image_mask tells the caller
which members of images has been updated. Anyway, since it seems to be
working fine and we don't have the extension semantics documented
precisely, we can just keep it as is.

>> >
>> > if (update_buffers(dri2_surf) < 0)
>> >return 0;
>> >
>> > if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
>> > -  /*
>> > -   * We don't support front buffers and GLES doesn't require them for
>> > -   * window surfaces, but some DRI drivers will request them anyway.
>> > -   * We just ignore such request as other platforms backends do.
>> > +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>> > + /* According current EGL spec,
>> > +  * front buffer rendering for window surface is not supported 
>> > now */
>> > + _eglLog(_EGL_WARNING,
>> > +   "%s:%d front buffer rendering for window surface is
>> > + not
>> supported!\n",
>> > +   __func__, __LINE__);
>> > + return 0;
>>
>> This is a semantic change and according to the old comment it might
>> break some drivers ("We don't support front buffers and GLES doesn't
>> require them for window surfaces, but some DRI drivers will request them 
>> anyway.").
> https://patchwork.freedesktop.org/patch/120682/ do you thinks this patch is 
> good?
> it deal pbuffer in front and back buffer.

The patch from your link will also error out if front buffer is
requested for a window surface. The code before your patch would just
silently ignore such request without setting respective bit in
images->image_mask, but other requested buffers would still be
collected. That patch is also incorrect because it allocates a back
buffer for pbuffer surfaces, but those (in Mesa) are supposed to have
only front buffer.

>>
>> > +  }
>> > +
>> > +  /* The EGL 1.5 spec states that pbuffers are single-buffered. 
>> > Specifically,
>> > +   * the spec states that they have a back buffer but no front 
>> > buffer, in
>> > +   * contrast to pixmaps, which 

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tapani Pälli]

On 11/22/2016 08:39 AM, Xu, Randy wrote:

Hi ,Tapani

With this pbuffer patch, only three EGL dEQP failure remained on Android 
platform , they are under investigation now

dEQP-EGL.functional.image.create#gles2_renderbuffer_depth16_depth_buffer fail due to 
"=== with config {glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} 
==="
dEQP-EGL.functional.image.modify#renderbuffer_depth16_renderbuffer_clear_depth fail due 
to "=== with config 
{glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} ==="
dEQP-EGL.functional.image.render_multiple_contexts#gles2_renderbuffer_depth16_depth_buffer
 fail due to "=== with config 
{glformat=rgbad24s8ms0,rotation=unspecified,surfacetype=window} ==="

The multithread cases can pass on Android.


Not for me


Thanks,
Randy


-Original Message-
From: Palli, Tapani
Sent: Tuesday, November 22, 2016 2:32 PM
To: Tomasz Figa 
Cc: Long, Zhifang ; Liu, Zhiquan ; Xu, Randy 
; Emil Velikov ; Kondapally, Kalyan 
; ML mesa-dev 
Subject: Re: [Mesa-dev] EGL/android: pbuffer implementation.



On 11/22/2016 08:08 AM, Tapani Pälli wrote:



On 11/22/2016 08:02 AM, Tomasz Figa wrote:

On Tue, Nov 22, 2016 at 2:44 PM, Tapani Pälli

wrote:



On 11/21/2016 04:15 PM, Emil Velikov wrote:


On 21 November 2016 at 07:23, Tomasz Figa  wrote:


Hi,

On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan

wrote:

@@ -775,14 +837,6 @@ droid_add_configs_for_visuals(_EGLDriver
*drv, _EGLDisplay *dpy)
for (i = 0; dri2_dpy->driver_configs[i]; i++) {
   const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
   struct dri2_egl_config *dri2_conf;
-  unsigned int double_buffered = 0;
-
-  dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[i],
- __DRI_ATTRIB_DOUBLE_BUFFER, _buffered);
-
-  /* support only double buffered configs */
-  if (!double_buffered)
- continue;



Does it really just work like this? Last time I checked the list
of configs generated after such change it contained
single-buffered-only configs with window surface bit set and
double-buffered-only configs with pbuffer bit set, both of which
are invalid. Moreover, this caused some dEQP tests to fail. How was this patch 
tested?


Afaict the patch has (should have?) gone through the Intel CI,
although I'm not sure if the latter builds/runs Android/ARC.



It did go through CI but it does not run any Android/ARC tests.
AFAIK Zhiquan did run pbuffer related tests and reported them
passing on Android.
Which dEQP tests were regressed?


The tests I'm concerned with are all
dEQP-EGL.functional.query_context.get_current_context.*_pbuffer tests.
In my testing on BSW and BDW, a change identical to the part quoted
above made eglGetConfigs() return double buffered configs for
pbuffers, which made the mentioned tests fail because of invalid
config at surface creation time (or SIGSEGV, before my NULL pointer
fix - 94282b6dd0 egl/android: Check return value of
dri2_get_dri_config()).


OK thanks, we will run these tests today and see if something got broken.


I did run all EGL tests now and all of the above 'get_current_context.*pbuffer' 
tests pass (some formats are skipped though). I don't have specifically BSW and 
BDW right now though, this was run on Joule (BXT).

With my run I got 7 failing EGL tests:

dEQP-EGL.functional.image.modify.tex_rgb5_a1_tex_subimage_rgba8
dEQP-EGL.functional.image.render_multiple_contexts.gles2_renderbuffer_depth16_depth_buffer
dEQP-EGL.functional.multithread.window
dEQP-EGL.functional.multithread.pbuffer_window
dEQP-EGL.functional.multithread.window_context
dEQP-EGL.functional.multithread.pbuffer_window_context
dEQP-EGL.performance.make_current.multi_surface.context_windows_10

(1999 passing, 1616 skipped)

Not sure if multithread ones ever passed. On desktop many of them fail because 
dEQP does not call XInitThreads().



Note that I haven't tested exactly this patch, just a similar change,
identical in terms of the quoted part above. It is also possible that
something changed in other part of the EGL stack, which fixed the
problem. I will retest current Mesa master a bit later.

Best regards,
Tomasz



// Tapani
___
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 3/4] radv/ac: add implementation of load_sample_pos intrinsic.

[Dave Airlie]

From: Dave Airlie 

This fixes a bunch of crashes in CTS tests looking for this.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 17cefea..741a082 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2764,6 +2764,15 @@ static LLVMValueRef load_sample_position(struct 
nir_to_llvm_context *ctx,
return build_gather_values(ctx, result, 2);
 }
 
+static LLVMValueRef load_sample_pos(struct nir_to_llvm_context *ctx)
+{
+   LLVMValueRef values[2];
+
+   values[0] = emit_ffract(ctx, ctx->frag_pos[0]);
+   values[1] = emit_ffract(ctx, ctx->frag_pos[1]);
+   return build_gather_values(ctx, values, 2);
+}
+
 static LLVMValueRef visit_interp(struct nir_to_llvm_context *ctx,
 nir_intrinsic_instr *instr)
 {
@@ -2888,6 +2897,9 @@ static void visit_intrinsic(struct nir_to_llvm_context 
*ctx,
case nir_intrinsic_load_sample_id:
result = ctx->ancillary;
break;
+   case nir_intrinsic_load_sample_pos:
+   result = load_sample_pos(ctx);
+   break;
case nir_intrinsic_load_front_face:
result = ctx->front_face;
break;
-- 
2.5.5

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

[Mesa-dev] [PATCH 1/4] radv/meta: cleanup resolve vertex state emission

[Dave Airlie]

From: Dave Airlie 

For the hw resolve there is no need to emit any sort
of texture coordinates, so drop them all in the meta path.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_meta_resolve.c | 49 ++
 1 file changed, 2 insertions(+), 47 deletions(-)

diff --git a/src/amd/vulkan/radv_meta_resolve.c 
b/src/amd/vulkan/radv_meta_resolve.c
index 697a264..66f1466 100644
--- a/src/amd/vulkan/radv_meta_resolve.c
+++ b/src/amd/vulkan/radv_meta_resolve.c
@@ -33,7 +33,6 @@
  */
 struct vertex_attrs {
float position[2]; /**< 3DPRIM_RECTLIST */
-   float tex_position[2];
 };
 
 /* passthrough vertex shader */
@@ -45,8 +44,6 @@ build_nir_vs(void)
nir_builder b;
nir_variable *a_position;
nir_variable *v_position;
-   nir_variable *a_tex_position;
-   nir_variable *v_tex_position;
 
nir_builder_init_simple_shader(, NULL, MESA_SHADER_VERTEX, NULL);
b.shader->info->name = ralloc_strdup(b.shader, "meta_resolve_vs");
@@ -59,16 +56,7 @@ build_nir_vs(void)
 "gl_Position");
v_position->data.location = VARYING_SLOT_POS;
 
-   a_tex_position = nir_variable_create(b.shader, nir_var_shader_in, vec4,
-"a_tex_position");
-   a_tex_position->data.location = VERT_ATTRIB_GENERIC1;
-
-   v_tex_position = nir_variable_create(b.shader, nir_var_shader_out, vec4,
-"v_tex_position");
-   v_tex_position->data.location = VARYING_SLOT_VAR0;
-
nir_copy_var(, v_position, a_position);
-   nir_copy_var(, v_tex_position, a_tex_position);
 
return b.shader;
 }
@@ -79,22 +67,16 @@ build_nir_fs(void)
 {
const struct glsl_type *vec4 = glsl_vec4_type();
nir_builder b;
-   nir_variable *v_tex_position; /* vec4, varying texture coordinate */
nir_variable *f_color; /* vec4, fragment output color */
 
nir_builder_init_simple_shader(, NULL, MESA_SHADER_FRAGMENT, NULL);
b.shader->info->name = ralloc_asprintf(b.shader,
   "meta_resolve_fs");
 
-   v_tex_position = nir_variable_create(b.shader, nir_var_shader_in, vec4,
-"v_tex_position");
-   v_tex_position->data.location = VARYING_SLOT_VAR0;
-
f_color = nir_variable_create(b.shader, nir_var_shader_out, vec4,
  "f_color");
f_color->data.location = FRAG_RESULT_DATA0;
-
-   nir_copy_var(, f_color, v_tex_position);
+   nir_store_var(, f_color, nir_imm_vec4(, 0.0, 0.0, 0.0, 1.0), 0xf);
 
return b.shader;
 }
@@ -198,7 +180,7 @@ create_pipeline(struct radv_device *device,
   
.inputRate = VK_VERTEX_INPUT_RATE_VERTEX
   },
   },
-  
.vertexAttributeDescriptionCount = 2,
+  
.vertexAttributeDescriptionCount = 1,
   
.pVertexAttributeDescriptions = (VkVertexInputAttributeDescription[]) {
   {
   /* 
Position */
@@ -207,13 +189,6 @@ create_pipeline(struct radv_device *device,
   .format 
= VK_FORMAT_R32G32_SFLOAT,
   .offset 
= offsetof(struct vertex_attrs, position),
   },
-  {
-  /* 
Texture Coordinate */
-  
.location = 1,
-  .binding 
= 0,
-  .format 
= VK_FORMAT_R32G32_SFLOAT,
-  .offset 
= offsetof(struct vertex_attrs, tex_position),
-  },
   },
   },
   .pInputAssemblyState = 
&(VkPipelineInputAssemblyStateCreateInfo) {
@@ -333,7 +308,6 @@ cleanup:
 
 static void
 emit_resolve(struct radv_cmd_buffer *cmd_buffer,
- const VkOffset2D *src_offset,
  const VkOffset2D *dest_offset,
  const VkExtent2D *resolve_extent)
 {
@@ -346,30 

[Mesa-dev] [PATCH 4/4] radv: fix sample id loading

[Dave Airlie]

From: Dave Airlie 

The sample id is packed into bits 8-12, so adjust
things properly.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 741a082..c68cb9c 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -379,6 +379,23 @@ static LLVMValueRef to_float(struct nir_to_llvm_context 
*ctx, LLVMValueRef v)
return v;
 }
 
+static LLVMValueRef unpack_param(struct nir_to_llvm_context *ctx,
+LLVMValueRef param, unsigned rshift,
+unsigned bitwidth)
+{
+   LLVMValueRef value = param;
+   if (rshift)
+   value = LLVMBuildLShr(ctx->builder, value,
+ LLVMConstInt(ctx->i32, rshift, false), 
"");
+
+   if (rshift + bitwidth < 32) {
+   unsigned mask = (1 << bitwidth) - 1;
+   value = LLVMBuildAnd(ctx->builder, value,
+LLVMConstInt(ctx->i32, mask, false), "");
+   }
+   return value;
+}
+
 static LLVMValueRef build_gep0(struct nir_to_llvm_context *ctx,
   LLVMValueRef base_ptr, LLVMValueRef index)
 {
@@ -2895,7 +2912,7 @@ static void visit_intrinsic(struct nir_to_llvm_context 
*ctx,
result = ctx->start_instance;
break;
case nir_intrinsic_load_sample_id:
-   result = ctx->ancillary;
+   result = unpack_param(ctx, ctx->ancillary, 8, 4);
break;
case nir_intrinsic_load_sample_pos:
result = load_sample_pos(ctx);
-- 
2.5.5

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

[Mesa-dev] [PATCH 2/4] radv/ac: cleanup ddxy emission

[Dave Airlie]

From: Dave Airlie 

This cleans up the ddxy emission along the same lines as
radeonsi. It also means we don't use LDS on VI chips we
use the dspermute interface, it also removes some duplicated
code.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 136 +---
 1 file changed, 43 insertions(+), 93 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 1d71795..17cefea 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -130,6 +130,8 @@ struct nir_to_llvm_context {
bool has_ddxy;
unsigned num_clips;
unsigned num_culls;
+
+   bool has_ds_bpermute;
 };
 
 struct ac_tex_info {
@@ -377,14 +379,23 @@ static LLVMValueRef to_float(struct nir_to_llvm_context 
*ctx, LLVMValueRef v)
return v;
 }
 
+static LLVMValueRef build_gep0(struct nir_to_llvm_context *ctx,
+  LLVMValueRef base_ptr, LLVMValueRef index)
+{
+   LLVMValueRef indices[2] = {
+   ctx->i32zero,
+   index,
+   };
+   return LLVMBuildGEP(ctx->builder, base_ptr,
+   indices, 2, "");
+}
+
 static LLVMValueRef build_indexed_load(struct nir_to_llvm_context *ctx,
   LLVMValueRef base_ptr, LLVMValueRef 
index,
   bool uniform)
 {
LLVMValueRef pointer;
-   LLVMValueRef indices[] = {ctx->i32zero, index};
-
-   pointer = LLVMBuildGEP(ctx->builder, base_ptr, indices, 2, "");
+   pointer = build_gep0(ctx, base_ptr, index);
if (uniform)
LLVMSetMetadata(pointer, ctx->uniform_md_kind, ctx->empty_md);
return LLVMBuildLoad(ctx->builder, pointer, "");
@@ -1132,55 +1143,44 @@ static LLVMValueRef get_thread_id(struct 
nir_to_llvm_context *ctx)
 #define TID_MASK_TOP  0xfffd
 #define TID_MASK_LEFT 0xfffe
 static LLVMValueRef emit_ddxy(struct nir_to_llvm_context *ctx,
- nir_alu_instr *instr,
+ nir_op op,
  LLVMValueRef src0)
 {
-   LLVMValueRef indices[2];
-   LLVMValueRef store_ptr, load_ptr0, load_ptr1;
LLVMValueRef tl, trbl, result;
LLVMValueRef tl_tid, trbl_tid;
LLVMValueRef args[2];
+   LLVMValueRef thread_id;
unsigned mask;
int idx;
ctx->has_ddxy = true;
-   if (!ctx->lds)
+
+   if (!ctx->lds && !ctx->has_ds_bpermute)
ctx->lds = LLVMAddGlobalInAddressSpace(ctx->module,
   LLVMArrayType(ctx->i32, 
64),
   "ddxy_lds", 
LOCAL_ADDR_SPACE);
 
-   indices[0] = ctx->i32zero;
-   indices[1] = get_thread_id(ctx);
-   store_ptr = LLVMBuildGEP(ctx->builder, ctx->lds,
-indices, 2, "");
-
-   if (instr->op == nir_op_fddx_fine || instr->op == nir_op_fddx)
+   thread_id = get_thread_id(ctx);
+   if (op == nir_op_fddx_fine || op == nir_op_fddx)
mask = TID_MASK_LEFT;
-   else if (instr->op == nir_op_fddy_fine || instr->op == nir_op_fddy)
+   else if (op == nir_op_fddy_fine || op == nir_op_fddy)
mask = TID_MASK_TOP;
else
mask = TID_MASK_TOP_LEFT;
 
-   tl_tid = LLVMBuildAnd(ctx->builder, indices[1],
+   tl_tid = LLVMBuildAnd(ctx->builder, thread_id,
  LLVMConstInt(ctx->i32, mask, false), "");
-   indices[1] = tl_tid;
-   load_ptr0 = LLVMBuildGEP(ctx->builder, ctx->lds,
-indices, 2, "");
-
/* for DDX we want to next X pixel, DDY next Y pixel. */
-   if (instr->op == nir_op_fddx_fine ||
-   instr->op == nir_op_fddx_coarse ||
-   instr->op == nir_op_fddx)
+   if (op == nir_op_fddx_fine ||
+   op == nir_op_fddx_coarse ||
+   op == nir_op_fddx)
idx = 1;
else
idx = 2;
 
-   trbl_tid = LLVMBuildAdd(ctx->builder, indices[1],
+   trbl_tid = LLVMBuildAdd(ctx->builder, tl_tid,
LLVMConstInt(ctx->i32, idx, false), "");
-   indices[1] = trbl_tid;
-   load_ptr1 = LLVMBuildGEP(ctx->builder, ctx->lds,
-indices, 2, "");
 
-   if (ctx->options->family >= CHIP_TONGA) {
+   if (ctx->has_ds_bpermute) {
args[0] = LLVMBuildMul(ctx->builder, tl_tid,
   LLVMConstInt(ctx->i32, 4, false), "");
args[1] = src0;
@@ -1194,8 +1194,13 @@ static LLVMValueRef emit_ddxy(struct nir_to_llvm_context 
*ctx,
   ctx->i32, args, 2,
   AC_FUNC_ATTR_READNONE);
} else {
-   LLVMBuildStore(ctx->builder, src0, 

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tapani Pälli]

On 11/22/2016 08:08 AM, Tapani Pälli wrote:



On 11/22/2016 08:02 AM, Tomasz Figa wrote:

On Tue, Nov 22, 2016 at 2:44 PM, Tapani Pälli 
wrote:



On 11/21/2016 04:15 PM, Emil Velikov wrote:


On 21 November 2016 at 07:23, Tomasz Figa  wrote:


Hi,

On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan 
wrote:

@@ -775,14 +837,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
_EGLDisplay *dpy)
for (i = 0; dri2_dpy->driver_configs[i]; i++) {
   const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
   struct dri2_egl_config *dri2_conf;
-  unsigned int double_buffered = 0;
-
-  dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[i],
- __DRI_ATTRIB_DOUBLE_BUFFER, _buffered);
-
-  /* support only double buffered configs */
-  if (!double_buffered)
- continue;



Does it really just work like this? Last time I checked the list of
configs generated after such change it contained single-buffered-only
configs with window surface bit set and double-buffered-only configs
with pbuffer bit set, both of which are invalid. Moreover, this caused
some dEQP tests to fail. How was this patch tested?


Afaict the patch has (should have?) gone through the Intel CI,
although I'm not sure if the latter builds/runs Android/ARC.



It did go through CI but it does not run any Android/ARC tests. AFAIK
Zhiquan did run pbuffer related tests and reported them passing on
Android.
Which dEQP tests were regressed?


The tests I'm concerned with are all
dEQP-EGL.functional.query_context.get_current_context.*_pbuffer tests.
In my testing on BSW and BDW, a change identical to the part quoted
above made eglGetConfigs() return double buffered configs for
pbuffers, which made the mentioned tests fail because of invalid
config at surface creation time (or SIGSEGV, before my NULL pointer
fix - 94282b6dd0 egl/android: Check return value of
dri2_get_dri_config()).


OK thanks, we will run these tests today and see if something got broken.


I did run all EGL tests now and all of the above 
'get_current_context.*pbuffer' tests pass (some formats are skipped 
though). I don't have specifically BSW and BDW right now though, this 
was run on Joule (BXT).


With my run I got 7 failing EGL tests:

dEQP-EGL.functional.image.modify.tex_rgb5_a1_tex_subimage_rgba8
dEQP-EGL.functional.image.render_multiple_contexts.gles2_renderbuffer_depth16_depth_buffer
dEQP-EGL.functional.multithread.window
dEQP-EGL.functional.multithread.pbuffer_window
dEQP-EGL.functional.multithread.window_context
dEQP-EGL.functional.multithread.pbuffer_window_context
dEQP-EGL.performance.make_current.multi_surface.context_windows_10

(1999 passing, 1616 skipped)

Not sure if multithread ones ever passed. On desktop many of them fail 
because dEQP does not call XInitThreads().




Note that I haven't tested exactly this patch, just a similar change,
identical in terms of the quoted part above. It is also possible that
something changed in other part of the EGL stack, which fixed the
problem. I will retest current Mesa master a bit later.

Best regards,
Tomasz



// Tapani
___
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] EGL/android: pbuffer implementation.

[Liu, Zhiquan]

Hi Tomasz,

Thanks for you commends.
> >
> 
> Looks like this patch has already landed, but please let me try to 
> confirm some things here anyway. Would you mind keeping me on CC for 
> any future patches for the EGL/Android module? (I believe 
> scripts/get_reviewer.pl should already include me on the list of 
> suggested reviewers, similarly for Rob Herring, who did a great job 
> before helping us with testing on platforms other than i915.)
> 
> [snip]
I just know the script, I will run next time.
> 
> > @@ -353,6 +353,18 @@ droid_destroy_surface(_EGLDriver *drv,
> _EGLDisplay *disp, _EGLSurface *surf)
> >dri2_surf->window->common.decRef(_surf->window->common);
> > }
> >
> > +   if (dri2_surf->dri_image_back) {
> > +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", 
> > + __func__,
> __LINE__);
> > +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
> > +  dri2_surf->dri_image_back = NULL;
> > +   }
> 
> Is this a fix for another bug by any chance? Note that we already take 
> care of dri_image_back for window surfaces in 
> droid_window_cancel_buffer(), which calls droid_window_enqueue_buffer(), 
> which does the real free on the image.
> It doesn't hurt to have it here as well, though, so treat this just as 
> a random thought of mine.
IMHO,droid_window_cancel_buffer should not call droid_window_enqueue_buffer() 
It should call dri2_surf->window->cancelBuffer, this should change in another 
patch.
So we add destroyImage here.
> 
> > +
> > +   if (dri2_surf->dri_image_front) {
> > +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", 
> > + __func__,
> __LINE__);
> > +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
> > +  dri2_surf->dri_image_front = NULL;
> > +   }
> > +
> > (*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);
> >
> > free(dri2_surf);
> 
> [snip]
> 
> > @@ -439,25 +451,75 @@ droid_image_get_buffers(__DRIdrawable
> *driDrawable,
> >struct __DRIimageList *images)  {
> > struct dri2_egl_surface *dri2_surf = loaderPrivate;
> > +   struct dri2_egl_display *dri2_dpy =
> > +  dri2_egl_display(dri2_surf->base.Resource.Display);
> >
> > images->image_mask = 0;
> > +   images->front = NULL;
> > +   images->back = NULL;
> 
> I'm not 100% sure this is the expected behavior of this function. My 
> understanding was that image_mask and error code would guard the other 
> members. It would make sense since typically if a function fails it 
> should keep the passed writable arguments unchanged. Is there a 
> precise description of the semantics used by the image loader extension 
> written down somewhere?
> 
Here, I just follow the image_mask operation.
> >
> > if (update_buffers(dri2_surf) < 0)
> >return 0;
> >
> > if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
> > -  /*
> > -   * We don't support front buffers and GLES doesn't require them for
> > -   * window surfaces, but some DRI drivers will request them anyway.
> > -   * We just ignore such request as other platforms backends do.
> > +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> > + /* According current EGL spec,
> > +  * front buffer rendering for window surface is not supported now 
> > */
> > + _eglLog(_EGL_WARNING,
> > +   "%s:%d front buffer rendering for window surface is 
> > + not
> supported!\n",
> > +   __func__, __LINE__);
> > + return 0;
> 
> This is a semantic change and according to the old comment it might 
> break some drivers ("We don't support front buffers and GLES doesn't 
> require them for window surfaces, but some DRI drivers will request them 
> anyway.").
https://patchwork.freedesktop.org/patch/120682/ do you thinks this patch is 
good?
it deal pbuffer in front and back buffer.
> 
> > +  }
> > +
> > +  /* The EGL 1.5 spec states that pbuffers are single-buffered. 
> > Specifically,
> > +   * the spec states that they have a back buffer but no front buffer, 
> > in
> > +   * contrast to pixmaps, which have a front buffer but no back buffer.
> 
> [snip]
> 
> >
> > if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
> > -  if (get_back_bo(dri2_surf) < 0)
> > +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> > + if (get_back_bo(dri2_surf) < 0)
> > +return 0;
> > +  }
> > +
> > +  if (!dri2_surf->dri_image_back) {
> > + _eglLog(_EGL_WARNING,
> > +   "%s:%d dri2_image back buffer allocation failed !\n",
> > +   __func__, __LINE__);
> 
> The error message here is inconsistent. The case of front buffer 
> requested for window surface clearly says that it's illegal, but this 
> one pretends that there was an actual allocation attempt.
I will send a follow patch fixing this.
> 
> Also (my subjective point of view) the whole code could be much more 
> readable if all the allocation of front buffer could be moved to
> get_front_bo() 

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tapani Pälli]

On 11/22/2016 08:02 AM, Tomasz Figa wrote:

On Tue, Nov 22, 2016 at 2:44 PM, Tapani Pälli  wrote:



On 11/21/2016 04:15 PM, Emil Velikov wrote:


On 21 November 2016 at 07:23, Tomasz Figa  wrote:


Hi,

On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan 
wrote:

@@ -775,14 +837,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
_EGLDisplay *dpy)
for (i = 0; dri2_dpy->driver_configs[i]; i++) {
   const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
   struct dri2_egl_config *dri2_conf;
-  unsigned int double_buffered = 0;
-
-  dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[i],
- __DRI_ATTRIB_DOUBLE_BUFFER, _buffered);
-
-  /* support only double buffered configs */
-  if (!double_buffered)
- continue;



Does it really just work like this? Last time I checked the list of
configs generated after such change it contained single-buffered-only
configs with window surface bit set and double-buffered-only configs
with pbuffer bit set, both of which are invalid. Moreover, this caused
some dEQP tests to fail. How was this patch tested?


Afaict the patch has (should have?) gone through the Intel CI,
although I'm not sure if the latter builds/runs Android/ARC.



It did go through CI but it does not run any Android/ARC tests. AFAIK
Zhiquan did run pbuffer related tests and reported them passing on Android.
Which dEQP tests were regressed?


The tests I'm concerned with are all
dEQP-EGL.functional.query_context.get_current_context.*_pbuffer tests.
In my testing on BSW and BDW, a change identical to the part quoted
above made eglGetConfigs() return double buffered configs for
pbuffers, which made the mentioned tests fail because of invalid
config at surface creation time (or SIGSEGV, before my NULL pointer
fix - 94282b6dd0 egl/android: Check return value of
dri2_get_dri_config()).


OK thanks, we will run these tests today and see if something got broken.


Note that I haven't tested exactly this patch, just a similar change,
identical in terms of the quoted part above. It is also possible that
something changed in other part of the EGL stack, which fixed the
problem. I will retest current Mesa master a bit later.

Best regards,
Tomasz



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

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tomasz Figa]

On Tue, Nov 22, 2016 at 2:44 PM, Tapani Pälli  wrote:
>
>
> On 11/21/2016 04:15 PM, Emil Velikov wrote:
>>
>> On 21 November 2016 at 07:23, Tomasz Figa  wrote:
>>>
>>> Hi,
>>>
>>> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan 
>>> wrote:
 @@ -775,14 +837,6 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
 _EGLDisplay *dpy)
 for (i = 0; dri2_dpy->driver_configs[i]; i++) {
const EGLint surface_type = EGL_WINDOW_BIT | EGL_PBUFFER_BIT;
struct dri2_egl_config *dri2_conf;
 -  unsigned int double_buffered = 0;
 -
 -  dri2_dpy->core->getConfigAttrib(dri2_dpy->driver_configs[i],
 - __DRI_ATTRIB_DOUBLE_BUFFER, _buffered);
 -
 -  /* support only double buffered configs */
 -  if (!double_buffered)
 - continue;
>>>
>>>
>>> Does it really just work like this? Last time I checked the list of
>>> configs generated after such change it contained single-buffered-only
>>> configs with window surface bit set and double-buffered-only configs
>>> with pbuffer bit set, both of which are invalid. Moreover, this caused
>>> some dEQP tests to fail. How was this patch tested?
>>>
>> Afaict the patch has (should have?) gone through the Intel CI,
>> although I'm not sure if the latter builds/runs Android/ARC.
>
>
> It did go through CI but it does not run any Android/ARC tests. AFAIK
> Zhiquan did run pbuffer related tests and reported them passing on Android.
> Which dEQP tests were regressed?

The tests I'm concerned with are all
dEQP-EGL.functional.query_context.get_current_context.*_pbuffer tests.
In my testing on BSW and BDW, a change identical to the part quoted
above made eglGetConfigs() return double buffered configs for
pbuffers, which made the mentioned tests fail because of invalid
config at surface creation time (or SIGSEGV, before my NULL pointer
fix - 94282b6dd0 egl/android: Check return value of
dri2_get_dri_config()).

Note that I haven't tested exactly this patch, just a similar change,
identical in terms of the quoted part above. It is also possible that
something changed in other part of the EGL stack, which fixed the
problem. I will retest current Mesa master a bit later.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tapani Pälli]

On 11/21/2016 04:15 PM, Emil Velikov wrote:

On 21 November 2016 at 07:23, Tomasz Figa  wrote:

Hi,

On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan  wrote:

mesa android path didn't support pbuffer, so add pbuffer support to
fix most deqp and cts pbuffer test cases fail;
add single buffer config to support pbuffer, and create image for
pbuffer when pbuffer type is front surface.
The EGL 1.5 spec states that pbuffers have a back buffer but no front
buffer, single-buffered surfaces with no front buffer confuse Mesa;
so we deviate from the spec, following the precedent of Mesa's
EGL X11 platform.

Test status: android CTS EGL pbuffer test can run without native crash.
test:[DEQP,EGL]all deqp pbuffer case passed.

V3: update commit message and code review changes.

Signed-off-by: Liu Zhiquan 
Signed-off-by: Kalyan Kondapally 
---
 src/egl/drivers/dri2/egl_dri2.h |  3 +-
 src/egl/drivers/dri2/platform_android.c | 98 +
 2 files changed, 78 insertions(+), 23 deletions(-)



Looks like this patch has already landed, but please let me try to
confirm some things here anyway. Would you mind keeping me on CC for
any future patches for the EGL/Android module? (




I believe
scripts/get_reviewer.pl should already include me on the list of
suggested reviewers, similarly for Rob Herring, who did a great job
before helping us with testing on platforms other than i915.)


I'll add you and update the documentation to reference the script.

Rob H let me know if you'd like to be in there as well which files
(platform_egl.c, Android build and/or other) and which email you'd
prefer.


[snip]


@@ -353,6 +353,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLSurface *surf)
   dri2_surf->window->common.decRef(_surf->window->common);
}

+   if (dri2_surf->dri_image_back) {
+  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__, 
__LINE__);
+  dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
+  dri2_surf->dri_image_back = NULL;
+   }


Is this a fix for another bug by any chance? Note that we already take
care of dri_image_back for window surfaces in
droid_window_cancel_buffer(), which calls
droid_window_enqueue_buffer(), which does the real free on the image.
It doesn't hurt to have it here as well, though, so treat this just as
a random thought of mine.


+
+   if (dri2_surf->dri_image_front) {
+  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__, 
__LINE__);
+  dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
+  dri2_surf->dri_image_front = NULL;
+   }
+
(*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);

free(dri2_surf);


[snip]


@@ -439,25 +451,75 @@ droid_image_get_buffers(__DRIdrawable *driDrawable,
   struct __DRIimageList *images)
 {
struct dri2_egl_surface *dri2_surf = loaderPrivate;
+   struct dri2_egl_display *dri2_dpy =
+  dri2_egl_display(dri2_surf->base.Resource.Display);

images->image_mask = 0;
+   images->front = NULL;
+   images->back = NULL;


I'm not 100% sure this is the expected behavior of this function. My
understanding was that image_mask and error code would guard the other
members. It would make sense since typically if a function fails it
should keep the passed writable arguments unchanged. Is there a
precise description of the semantics used by the image loader
extension written down somewhere?



if (update_buffers(dri2_surf) < 0)
   return 0;

if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
-  /*
-   * We don't support front buffers and GLES doesn't require them for
-   * window surfaces, but some DRI drivers will request them anyway.
-   * We just ignore such request as other platforms backends do.
+  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
+ /* According current EGL spec,
+  * front buffer rendering for window surface is not supported now */
+ _eglLog(_EGL_WARNING,
+   "%s:%d front buffer rendering for window surface is not 
supported!\n",
+   __func__, __LINE__);
+ return 0;


This is a semantic change and according to the old comment it might
break some drivers ("We don't support front buffers and GLES doesn't
require them for window surfaces, but some DRI drivers will request
them anyway.").


Fwiw -


+  }
+
+  /* The EGL 1.5 spec states that pbuffers are single-buffered. 
Specifically,
+   * the spec states that they have a back buffer but no front buffer, in
+   * contrast to pixmaps, which have a front buffer but no back buffer.


[snip]



if (buffer_mask & __DRI_IMAGE_BUFFER_BACK) {
-  if (get_back_bo(dri2_surf) < 0)
+  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
+ if (get_back_bo(dri2_surf) < 0)
+return 0;
+  }
+
+  if (!dri2_surf->dri_image_back) {
+ 

Re: [Mesa-dev] [PATCH 1/4] anv/cmd_buffer: Align push constants to 64 bytes

[Jason Ekstrand]

Jordan pointed out that this patch is bogus.  Please disregard.

On Mon, Nov 21, 2016 at 8:26 PM, Jason Ekstrand 
wrote:

> This is required by compute shaders.
>
> Cc: "13.0" 
> Cc: Jordan Justen 
> ---
>  src/intel/vulkan/anv_cmd_buffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_
> buffer.c
> index d882c18..c6122db 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -632,7 +632,7 @@ anv_cmd_buffer_push_constants(struct anv_cmd_buffer
> *cmd_buffer,
> struct anv_state state =
>anv_cmd_buffer_alloc_dynamic_state(cmd_buffer,
>   prog_data->nr_params *
> sizeof(float),
> - 32 /* bottom 5 bits MBZ */);
> + 64 /* bottom 5 bits MBZ */);
>
> /* Walk through the param array and fill the buffer with data */
> uint32_t *u32_map = state.map;
> --
> 2.5.0.400.gff86faf
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: add support for shader stats dump

[Dave Airlie]

From: Dave Airlie 

I've started working on a shader-db alike for Vulkan,
it's based on vktrace and it records pipelines, this
adds support to dump the shader stats exactly like
radeonsi does, so I can reuse the shader-db scripts it
uses.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_device.c   |  2 +-
 src/amd/vulkan/radv_pipeline.c | 84 ++
 src/amd/vulkan/radv_private.h  |  3 ++
 3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 5acaf56..313d7a5 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -682,7 +682,7 @@ VkResult radv_CreateDevice(
}
device->allow_fast_clears = env_var_as_boolean("RADV_FAST_CLEARS", 
false);
device->allow_dcc = !env_var_as_boolean("RADV_DCC_DISABLE", true);
-
+   device->shader_stats_dump = env_var_as_boolean("RADV_SHADER_STATS", 
false);
if (device->allow_fast_clears && device->allow_dcc)
radv_finishme("DCC fast clears have not been tested\n");
 
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index fca0173..5f3ebb3 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -257,6 +257,81 @@ radv_shader_compile_to_nir(struct radv_device *device,
return nir;
 }
 
+static const char *radv_get_shader_name(struct radv_shader_variant *var,
+   gl_shader_stage stage)
+{
+   switch (stage) {
+   case MESA_SHADER_VERTEX: return "Vertex Shader as VS";
+   case MESA_SHADER_FRAGMENT: return "Pixel Shader";
+   case MESA_SHADER_COMPUTE: return "Compute Shader";
+   default:
+   return "Unknown shader";
+   };
+
+}
+static void radv_dump_pipeline_stats(struct radv_device *device, struct 
radv_pipeline *pipeline)
+{
+   unsigned lds_increment = 
device->instance->physicalDevice.rad_info.chip_class >= CIK ? 512 : 256;
+   struct radv_shader_variant *var;
+   struct ac_shader_config *conf;
+   int i;
+   FILE *file = stderr;
+   unsigned max_simd_waves = 10;
+   unsigned lds_per_wave = 0;
+
+   for (i = 0; i < MESA_SHADER_STAGES; i++) {
+   if (!pipeline->shaders[i])
+   continue;
+   var = pipeline->shaders[i];
+
+   conf = >config;
+
+   if (i == MESA_SHADER_FRAGMENT) {
+   lds_per_wave = conf->lds_size * lds_increment +
+   align(var->info.fs.num_interp * 48, 
lds_increment);
+   }
+
+   if (conf->num_sgprs) {
+   if 
(device->instance->physicalDevice.rad_info.chip_class >= VI)
+   max_simd_waves = MIN2(max_simd_waves, 800 / 
conf->num_sgprs);
+   else
+   max_simd_waves = MIN2(max_simd_waves, 512 / 
conf->num_sgprs);
+   }
+
+   if (conf->num_vgprs)
+   max_simd_waves = MIN2(max_simd_waves, 256 / 
conf->num_vgprs);
+
+   /* LDS is 64KB per CU (4 SIMDs), divided into 16KB blocks per 
SIMD
+* that PS can use.
+*/
+   if (lds_per_wave)
+   max_simd_waves = MIN2(max_simd_waves, 16384 / 
lds_per_wave);
+
+   fprintf(file, "\n%s:\n",
+   radv_get_shader_name(var, i));
+   if (i == MESA_SHADER_FRAGMENT) {
+   fprintf(file, "*** SHADER CONFIG ***\n"
+   "SPI_PS_INPUT_ADDR = 0x%04x\n"
+   "SPI_PS_INPUT_ENA  = 0x%04x\n",
+   conf->spi_ps_input_addr, 
conf->spi_ps_input_ena);
+   }
+   fprintf(file, "*** SHADER STATS ***\n"
+   "SGPRS: %d\n"
+   "VGPRS: %d\n"
+   "Spilled SGPRs: %d\n"
+   "Spilled VGPRs: %d\n"
+   "Code Size: %d bytes\n"
+   "LDS: %d blocks\n"
+   "Scratch: %d bytes per wave\n"
+   "Max Waves: %d\n"
+   "\n\n\n",
+   conf->num_sgprs, conf->num_vgprs,
+   conf->spilled_sgprs, conf->spilled_vgprs, 
var->code_size,
+   conf->lds_size, conf->scratch_bytes_per_wave,
+   max_simd_waves);
+   }
+}
+
 void radv_shader_variant_destroy(struct radv_device *device,
  struct radv_shader_variant *variant)
 {
@@ -297,6 +372,7 @@ struct radv_shader_variant 
*radv_shader_variant_create(struct radv_device *devic
  >info, shader, , dump);
LLVMDisposeTargetMachine(tm);
 
+   variant->code_size = binary.code_size;
bool 

[Mesa-dev] [PATCH 1/4] anv/cmd_buffer: Align push constants to 64 bytes

[Jason Ekstrand]

This is required by compute shaders.

Cc: "13.0" 
Cc: Jordan Justen 
---
 src/intel/vulkan/anv_cmd_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c
index d882c18..c6122db 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -632,7 +632,7 @@ anv_cmd_buffer_push_constants(struct anv_cmd_buffer 
*cmd_buffer,
struct anv_state state =
   anv_cmd_buffer_alloc_dynamic_state(cmd_buffer,
  prog_data->nr_params * sizeof(float),
- 32 /* bottom 5 bits MBZ */);
+ 64 /* bottom 5 bits MBZ */);
 
/* Walk through the param array and fill the buffer with data */
uint32_t *u32_map = state.map;
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 4/4] anv/cmd_buffer: Emit a CS stall before setting a CS pipeline

[Jason Ekstrand]

Cc: "13.0" 
Cc: Jordan Justen 
---
 src/intel/vulkan/genX_cmd_buffer.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index eded1c9..6af2a18 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1726,8 +1726,20 @@ genX(cmd_buffer_flush_compute_state)(struct 
anv_cmd_buffer *cmd_buffer)
 
genX(flush_pipeline_select_gpgpu)(cmd_buffer);
 
-   if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)
+   if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE) {
+  /* From the Sky Lake PRM Vol 2a, MEDIA_VFE_STATE:
+   *
+   *"A stalling PIPE_CONTROL is required before MEDIA_VFE_STATE unless
+   *the only bits that are changed are scoreboard related: Scoreboard
+   *Enable, Scoreboard Type, Scoreboard Mask, Scoreboard * Delta. For
+   *these scoreboard related states, a MEDIA_STATE_FLUSH is
+   *sufficient."
+   */
+  cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
+
   anv_batch_emit_batch(_buffer->batch, >batch);
+   }
 
if (cmd_buffer->state.push_constants_dirty & VK_SHADER_STAGE_COMPUTE_BIT) {
   struct anv_state push_state =
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 3/4] anv/cmd_buffer: Re-emit MEDIA_CURBE_LOAD when CS push constants are dirty

[Jason Ekstrand]

This can happen even if the binding table isn't changed.  For instance, you
could have dynamic offsets with your descriptor set.  This fixes the new
stress.lots-of-surface-state.cs.dynamic cricible test.

Cc: "13.0" 
Cc: Jordan Justen 
---
 src/intel/vulkan/genX_cmd_buffer.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index 6645d1b..eded1c9 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1687,19 +1687,10 @@ flush_compute_descriptor_set(struct anv_cmd_buffer 
*cmd_buffer)
   result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, );
   assert(result == VK_SUCCESS);
}
+
result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, );
assert(result == VK_SUCCESS);
 
-
-   struct anv_state push_state = anv_cmd_buffer_cs_push_constants(cmd_buffer);
-
-   if (push_state.alloc_size) {
-  anv_batch_emit(_buffer->batch, GENX(MEDIA_CURBE_LOAD), curbe) {
- curbe.CURBETotalDataLength= push_state.alloc_size;
- curbe.CURBEDataStartAddress   = push_state.offset;
-  }
-   }
-
uint32_t iface_desc_data_dw[GENX(INTERFACE_DESCRIPTOR_DATA_length)];
struct GENX(INTERFACE_DESCRIPTOR_DATA) desc = {
   .BindingTablePointer = surfaces.offset,
@@ -1738,6 +1729,18 @@ genX(cmd_buffer_flush_compute_state)(struct 
anv_cmd_buffer *cmd_buffer)
if (cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)
   anv_batch_emit_batch(_buffer->batch, >batch);
 
+   if (cmd_buffer->state.push_constants_dirty & VK_SHADER_STAGE_COMPUTE_BIT) {
+  struct anv_state push_state =
+ anv_cmd_buffer_cs_push_constants(cmd_buffer);
+
+  if (push_state.alloc_size) {
+ anv_batch_emit(_buffer->batch, GENX(MEDIA_CURBE_LOAD), curbe) {
+curbe.CURBETotalDataLength= push_state.alloc_size;
+curbe.CURBEDataStartAddress   = push_state.offset;
+ }
+  }
+   }
+
if ((cmd_buffer->state.descriptors_dirty & VK_SHADER_STAGE_COMPUTE_BIT) ||
(cmd_buffer->state.compute_dirty & ANV_CMD_DIRTY_PIPELINE)) {
   /* FIXME: figure out descriptors for gen7 */
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 2/4] anv/cmd_buffer: Handle running out of binding tables in compute shaders

[Jason Ekstrand]

If we try to allocate a binding table and fail, we have to get a new
binding table block, re-emit STATE_BASE_ADDRESS, and then try again.  We
already handle this correctly for 3D and blorp but it never got handled for
CS.  This fixes the new stress.lots-of-surface-state.cs.static crucible test.

Cc: "13.0" 
Cc: Jordan Justen 
---
 src/intel/vulkan/genX_cmd_buffer.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index de3253f..6645d1b 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1674,12 +1674,22 @@ flush_compute_descriptor_set(struct anv_cmd_buffer 
*cmd_buffer)
struct anv_state surfaces = { 0, }, samplers = { 0, };
VkResult result;
 
-   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, );
-   if (result != VK_SUCCESS)
-  return result;
result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, );
-   if (result != VK_SUCCESS)
-  return result;
+   if (result != VK_SUCCESS) {
+  result = anv_cmd_buffer_new_binding_table_block(cmd_buffer);
+  assert(result == VK_SUCCESS);
+
+  /* Re-emit state base addresses so we get the new surface state base
+   * address before we start emitting binding tables etc.
+   */
+  genX(cmd_buffer_emit_state_base_address)(cmd_buffer);
+
+  result = emit_binding_table(cmd_buffer, MESA_SHADER_COMPUTE, );
+  assert(result == VK_SUCCESS);
+   }
+   result = emit_samplers(cmd_buffer, MESA_SHADER_COMPUTE, );
+   assert(result == VK_SUCCESS);
+
 
struct anv_state push_state = anv_cmd_buffer_cs_push_constants(cmd_buffer);
 
-- 
2.5.0.400.gff86faf

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

Re: [Mesa-dev] [PATCH 3/4] glsl: add gl_linked_shader::SourceChecksum

[Timothy Arceri]

IMO we should wrap all these with:

#ifdef DEBUG

Including the existing SourceChecksum declaration of SourceChecksum in
gl_shader.

With those changes series is:

Reviewed-by: Timothy Arceri 

On Tue, 2016-11-22 at 02:20 +0100, Marek Olšák wrote:
> From: Marek Olšák 
> 
> for debugging
> ---
>  src/compiler/glsl/linker.cpp | 8 
>  src/mesa/main/mtypes.h   | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/src/compiler/glsl/linker.cpp
> b/src/compiler/glsl/linker.cpp
> index 61a029c..9678a74 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -2290,20 +2290,28 @@ link_intrastage_shaders(void *mem_ctx,
>    array_resize_visitor input_resize_visitor(num_vertices, prog,
>  MESA_SHADER_GEOMETRY
> );
>    foreach_in_list(ir_instruction, ir, linked->ir) {
>   ir->accept(_resize_visitor);
>    }
> }
>  
> if (ctx->Const.VertexID_is_zero_based)
>    lower_vertex_id(linked);
>  
> +   /* Compute the source checksum. */
> +   linked->SourceChecksum = 0;
> +   for (unsigned i = 0; i < num_shaders; i++) {
> +  if (shader_list[i] == NULL)
> + continue;
> +  linked->SourceChecksum ^= shader_list[i]->SourceChecksum;
> +   }
> +
> return linked;
>  }
>  
>  /**
>   * Update the sizes of linked shader uniform arrays to the maximum
>   * array index used.
>   *
>   * From page 81 (page 95 of the PDF) of the OpenGL 2.1 spec:
>   *
>   * If one or more elements of an array are active,
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index e870053..e6ae6d6 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2275,20 +2275,21 @@ struct gl_shader_info
>    bool LocalSizeVariable;
> } Comp;
>  };
>  
>  /**
>   * A linked GLSL shader object.
>   */
>  struct gl_linked_shader
>  {
> gl_shader_stage Stage;
> +   unsigned SourceChecksum;
>  
> struct gl_program *Program;  /**< Post-compile assembly code */
>  
> /**
>  * \name Sampler tracking
>  *
>  * \note Each of these fields is only set post-linking.
>  */
> /*@{*/
> unsigned num_samplers;/**< Number of samplers used by
> this shader. */
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel/aubinator: Use the correct length for MEDIA commands

[Jason Ekstrand]

---
 src/intel/tools/decoder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c
index 6bd02bf..55488eb 100644
--- a/src/intel/tools/decoder.c
+++ b/src/intel/tools/decoder.c
@@ -612,8 +612,8 @@ gen_group_get_length(struct gen_group *group, const 
uint32_t *p)
  return field(h, 0, 7) + 2;
   case 1:
  return 1;
-  case 2:
- return 2;
+  case 2: /* MEDIA */
+ return field(h, 0, 7) + 2;
   case 3:
  return field(h, 0, 7) + 2;
   }
-- 
2.5.0.400.gff86faf

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

Re: [Mesa-dev] [PATCH 1/3] radv: Store UUID in physical device.

[Edward O'Callaghan]

This series is,
Acked-by: Edward O'Callaghan 

On 11/22/2016 12:19 PM, Bas Nieuwenhuizen wrote:
> No sense in repeatedly determining it. Also, it might be dependent
> on the device as shaders get compiled differently for SI/CIK/VI etc.
> 
> Signed-off-by: Bas Nieuwenhuizen 
> ---
>  src/amd/vulkan/radv_device.c | 18 ++
>  src/amd/vulkan/radv_pipeline_cache.c |  8 
>  src/amd/vulkan/radv_private.h|  4 ++--
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 390fde0..b731171 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -44,6 +44,13 @@
>  #include "util/debug.h"
>  struct radv_dispatch_table dtable;
>  
> +static void
> +radv_device_get_cache_uuid(void *uuid)
> +{
> + memset(uuid, 0, VK_UUID_SIZE);
> + snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
> +}
> +
>  static VkResult
>  radv_physical_device_init(struct radv_physical_device *device,
> struct radv_instance *instance,
> @@ -89,6 +96,8 @@ radv_physical_device_init(struct radv_physical_device 
> *device,
>   goto fail;
>   }
>  
> + radv_device_get_cache_uuid(device->uuid);
> +
>   fprintf(stderr, "WARNING: radv is not a conformant vulkan 
> implementation, testing use only.\n");
>   device->name = device->rad_info.name;
>   return VK_SUCCESS;
> @@ -361,13 +370,6 @@ void radv_GetPhysicalDeviceFeatures(
>   };
>  }
>  
> -void
> -radv_device_get_cache_uuid(void *uuid)
> -{
> - memset(uuid, 0, VK_UUID_SIZE);
> - snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
> -}
> -
>  void radv_GetPhysicalDeviceProperties(
>   VkPhysicalDevicephysicalDevice,
>   VkPhysicalDeviceProperties* pProperties)
> @@ -498,7 +500,7 @@ void radv_GetPhysicalDeviceProperties(
>   };
>  
>   strcpy(pProperties->deviceName, pdevice->name);
> - radv_device_get_cache_uuid(pProperties->pipelineCacheUUID);
> + memcpy(pProperties->pipelineCacheUUID, pdevice->uuid, VK_UUID_SIZE);
>  }
>  
>  void radv_GetPhysicalDeviceQueueFamilyProperties(
> diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
> b/src/amd/vulkan/radv_pipeline_cache.c
> index b429355..db824a2 100644
> --- a/src/amd/vulkan/radv_pipeline_cache.c
> +++ b/src/amd/vulkan/radv_pipeline_cache.c
> @@ -308,8 +308,8 @@ radv_pipeline_cache_load(struct radv_pipeline_cache 
> *cache,
>const void *data, size_t size)
>  {
>   struct radv_device *device = cache->device;
> + struct radv_physical_device *pdevice = 
> >instance->physicalDevice;
>   struct cache_header header;
> - uint8_t uuid[VK_UUID_SIZE];
>  
>   if (size < sizeof(header))
>   return;
> @@ -322,8 +322,7 @@ radv_pipeline_cache_load(struct radv_pipeline_cache 
> *cache,
>   return;
>   if (header.device_id != 
> device->instance->physicalDevice.rad_info.pci_id)
>   return;
> - radv_device_get_cache_uuid(uuid);
> - if (memcmp(header.uuid, uuid, VK_UUID_SIZE) != 0)
> + if (memcmp(header.uuid, pdevice->uuid, VK_UUID_SIZE) != 0)
>   return;
>  
>   char *end = (void *) data + size;
> @@ -405,6 +404,7 @@ VkResult radv_GetPipelineCacheData(
>  {
>   RADV_FROM_HANDLE(radv_device, device, _device);
>   RADV_FROM_HANDLE(radv_pipeline_cache, cache, _cache);
> + struct radv_physical_device *pdevice = 
> >instance->physicalDevice;
>   struct cache_header *header;
>   VkResult result = VK_SUCCESS;
>   const size_t size = sizeof(*header) + cache->total_size;
> @@ -422,7 +422,7 @@ VkResult radv_GetPipelineCacheData(
>   header->header_version = VK_PIPELINE_CACHE_HEADER_VERSION_ONE;
>   header->vendor_id = 0x1002;
>   header->device_id = device->instance->physicalDevice.rad_info.pci_id;
> - radv_device_get_cache_uuid(header->uuid);
> + memcpy(header->uuid, pdevice->uuid, VK_UUID_SIZE);
>   p += header->header_size;
>  
>   struct cache_entry *entry;
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index ad5232b..3d17895 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -258,6 +258,8 @@ struct radv_physical_device {
>   uint32_tpci_vendor_id;
>   uint32_tpci_device_id;
>  
> + uint8_t uuid[VK_UUID_SIZE];
> +
>   struct wsi_device   wsi_device;
>  };
>  
> @@ -453,8 +455,6 @@ struct radv_device {
>   float sample_locations_16x[16][2];
>  };
>  
> -void radv_device_get_cache_uuid(void *uuid);
> -
>  struct radv_device_memory {
>   struct radeon_winsys_bo  *bo;
>   uint32_t type_index;
> 



signature.asc
Description: 

[Mesa-dev] [PATCH 4/4] mesa: use special checksums for unset checksums and fixed-func shaders

[Marek Olšák]

From: Marek Olšák 

for debugging
---
 src/mesa/main/ff_fragment_shader.cpp | 1 +
 src/mesa/main/shaderobj.c| 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/mesa/main/ff_fragment_shader.cpp 
b/src/mesa/main/ff_fragment_shader.cpp
index e00eac5..2497108 100644
--- a/src/mesa/main/ff_fragment_shader.cpp
+++ b/src/mesa/main/ff_fragment_shader.cpp
@@ -1195,20 +1195,21 @@ emit_instructions(texenv_fragment_program *p)
  */
 static struct gl_shader_program *
 create_new_program(struct gl_context *ctx, struct state_key *key)
 {
texenv_fragment_program p;
unsigned int unit;
_mesa_glsl_parse_state *state;
 
p.mem_ctx = ralloc_context(NULL);
p.shader = _mesa_new_shader(0, MESA_SHADER_FRAGMENT);
+   p.shader->SourceChecksum = 0xf18ed; /* fixed */
p.shader->ir = new(p.shader) exec_list;
state = new(p.shader) _mesa_glsl_parse_state(ctx, MESA_SHADER_FRAGMENT,
p.shader);
p.shader->symbols = state->symbols;
p.top_instructions = p.shader->ir;
p.instructions = p.shader->ir;
p.state = key;
p.shader_program = _mesa_new_shader_program(0);
 
/* Tell the linker to ignore the fact that we're building a
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 167431d..3d0fbe8 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -102,20 +102,21 @@ _mesa_init_shader(struct gl_shader *shader)
  * Allocate a new gl_shader object, initialize it.
  */
 struct gl_shader *
 _mesa_new_shader(GLuint name, gl_shader_stage stage)
 {
struct gl_shader *shader;
shader = rzalloc(NULL, struct gl_shader);
if (shader) {
   shader->Stage = stage;
   shader->Name = name;
+  shader->SourceChecksum = 0xa110c; /* alloc */
   _mesa_init_shader(shader);
}
return shader;
 }
 
 
 /**
  * Allocate a new gl_linked_shader object.
  * Called via ctx->Driver.NewShader()
  */
-- 
2.7.4

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

[Mesa-dev] [PATCH 3/4] glsl: add gl_linked_shader::SourceChecksum

[Marek Olšák]

From: Marek Olšák 

for debugging
---
 src/compiler/glsl/linker.cpp | 8 
 src/mesa/main/mtypes.h   | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 61a029c..9678a74 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2290,20 +2290,28 @@ link_intrastage_shaders(void *mem_ctx,
   array_resize_visitor input_resize_visitor(num_vertices, prog,
 MESA_SHADER_GEOMETRY);
   foreach_in_list(ir_instruction, ir, linked->ir) {
  ir->accept(_resize_visitor);
   }
}
 
if (ctx->Const.VertexID_is_zero_based)
   lower_vertex_id(linked);
 
+   /* Compute the source checksum. */
+   linked->SourceChecksum = 0;
+   for (unsigned i = 0; i < num_shaders; i++) {
+  if (shader_list[i] == NULL)
+ continue;
+  linked->SourceChecksum ^= shader_list[i]->SourceChecksum;
+   }
+
return linked;
 }
 
 /**
  * Update the sizes of linked shader uniform arrays to the maximum
  * array index used.
  *
  * From page 81 (page 95 of the PDF) of the OpenGL 2.1 spec:
  *
  * If one or more elements of an array are active,
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index e870053..e6ae6d6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2275,20 +2275,21 @@ struct gl_shader_info
   bool LocalSizeVariable;
} Comp;
 };
 
 /**
  * A linked GLSL shader object.
  */
 struct gl_linked_shader
 {
gl_shader_stage Stage;
+   unsigned SourceChecksum;
 
struct gl_program *Program;  /**< Post-compile assembly code */
 
/**
 * \name Sampler tracking
 *
 * \note Each of these fields is only set post-linking.
 */
/*@{*/
unsigned num_samplers;  /**< Number of samplers used by this shader. */
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/4] mesa: use util_hash_crc32 instead of _mesa_str_checksum

[Marek Olšák]

From: Marek Olšák 

---
 src/mesa/main/imports.c   | 22 --
 src/mesa/main/imports.h   |  3 ---
 src/mesa/main/shaderapi.c |  3 ++-
 3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 808b8f6..de4dd0e 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -245,42 +245,20 @@ _mesa_bitcount_64(uint64_t n)
for (bits = 0; n > 0; n = n >> 1) {
   bits += (n & 1);
}
return bits;
 }
 #endif
 
 /*@}*/
 
 
-/**/
-/** \name String */
-/*@{*/
-
-
-/** Compute simple checksum/hash for a string */
-unsigned int
-_mesa_str_checksum(const char *str)
-{
-   /* This could probably be much better */
-   unsigned int sum, i;
-   const char *c;
-   sum = i = 1;
-   for (c = str; *c; c++, i++)
-  sum += *c * (i % 100);
-   return sum + i;
-}
-
-
-/*@}*/
-
-
 /** Needed due to #ifdef's, above. */
 int
 _mesa_vsnprintf(char *str, size_t size, const char *fmt, va_list args)
 {
return vsnprintf( str, size, fmt, args);
 }
 
 /** Wrapper around vsnprintf() */
 int
 _mesa_snprintf( char *str, size_t size, const char *fmt, ... )
diff --git a/src/mesa/main/imports.h b/src/mesa/main/imports.h
index 21bcb10..ef7c378 100644
--- a/src/mesa/main/imports.h
+++ b/src/mesa/main/imports.h
@@ -339,23 +339,20 @@ extern unsigned int
 _mesa_bitcount_64(uint64_t n);
 #endif
 
 
 static inline bool
 _mesa_half_is_negative(GLhalfARB h)
 {
return h & 0x8000;
 }
 
-extern unsigned int
-_mesa_str_checksum(const char *str);
-
 extern int
 _mesa_snprintf( char *str, size_t size, const char *fmt, ... ) PRINTFLIKE(3, 
4);
 
 extern int
 _mesa_vsnprintf(char *str, size_t size, const char *fmt, va_list arg);
 
 
 #if defined(_MSC_VER) && !defined(snprintf)
 #define snprintf _snprintf
 #endif
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index ab15300..9592857 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -52,20 +52,21 @@
 #include "compiler/glsl/glsl_parser_extras.h"
 #include "compiler/glsl/ir.h"
 #include "compiler/glsl/ir_uniform.h"
 #include "compiler/glsl/program.h"
 #include "program/program.h"
 #include "program/prog_print.h"
 #include "program/prog_parameter.h"
 #include "util/ralloc.h"
 #include "util/hash_table.h"
 #include "util/mesa-sha1.h"
+#include "util/crc32.h"
 
 /**
  * Return mask of GLSL_x flags by examining the MESA_GLSL env var.
  */
 GLbitfield
 _mesa_get_shader_flags(void)
 {
GLbitfield flags = 0x0;
const char *env = getenv("MESA_GLSL");
 
@@ -985,21 +986,21 @@ get_shader_source(struct gl_context *ctx, GLuint shader, 
GLsizei maxLength,
  */
 static void
 shader_source(struct gl_shader *sh, const GLchar *source)
 {
assert(sh);
 
/* free old shader source string and install new one */
free((void *)sh->Source);
sh->Source = source;
 #ifdef DEBUG
-   sh->SourceChecksum = _mesa_str_checksum(sh->Source);
+   sh->SourceChecksum = util_hash_crc32(sh->Source, strlen(sh->Source));
 #endif
 }
 
 
 /**
  * Compile a shader.
  */
 void
 _mesa_compile_shader(struct gl_context *ctx, struct gl_shader *sh)
 {
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/4] util: import CRC32 implementation from gallium

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/auxiliary/Makefile.sources|  2 --
 src/gallium/drivers/radeonsi/si_state_shaders.c   |  2 +-
 src/util/Makefile.sources |  2 ++
 src/{gallium/auxiliary/util/u_hash.c => util/crc32.c} |  4 ++--
 src/{gallium/auxiliary/util/u_hash.h => util/crc32.h} | 12 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)
 rename src/{gallium/auxiliary/util/u_hash.c => util/crc32.c} (98%)
 rename src/{gallium/auxiliary/util/u_hash.h => util/crc32.h} (92%)

diff --git a/src/gallium/auxiliary/Makefile.sources 
b/src/gallium/auxiliary/Makefile.sources
index 3fda2eb..5d4fe30 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -243,22 +243,20 @@ C_SOURCES := \
util/u_format_yuv.h \
util/u_format_zs.c \
util/u_format_zs.h \
util/u_framebuffer.c \
util/u_framebuffer.h \
util/u_gen_mipmap.c \
util/u_gen_mipmap.h \
util/u_half.h \
util/u_handle_table.c \
util/u_handle_table.h \
-   util/u_hash.c \
-   util/u_hash.h \
util/u_hash_table.c \
util/u_hash_table.h \
util/u_helpers.c \
util/u_helpers.h \
util/u_index_modify.c \
util/u_index_modify.h \
util/u_inlines.h \
util/u_keymap.c \
util/u_keymap.h \
util/u_linear.c \
diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
b/src/gallium/drivers/radeonsi/si_state_shaders.c
index cd4b339..fd0be30 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -25,21 +25,21 @@
  *  Marek Olšák 
  */
 
 #include "si_pipe.h"
 #include "sid.h"
 #include "radeon/r600_cs.h"
 
 #include "tgsi/tgsi_parse.h"
 #include "tgsi/tgsi_ureg.h"
 #include "util/hash_table.h"
-#include "util/u_hash.h"
+#include "util/crc32.h"
 #include "util/u_memory.h"
 #include "util/u_prim.h"
 
 /* SHADER_CACHE */
 
 /**
  * Return the TGSI binary in a buffer. The first 4 bytes contain its size as
  * integer.
  */
 static void *si_get_tgsi_binary(struct si_shader_selector *sel)
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index b7ca347..d2ae5e7 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -1,14 +1,16 @@
 MESA_UTIL_FILES := \
bitscan.c \
bitscan.h \
bitset.h \
+   crc32.c \
+   crc32.h \
debug.c \
debug.h \
disk_cache.c \
disk_cache.h \
format_r11g11b10f.h \
format_rgb9e5.h \
format_srgb.h \
half_float.c \
half_float.h \
hash_table.c\
diff --git a/src/gallium/auxiliary/util/u_hash.c b/src/util/crc32.c
similarity index 98%
rename from src/gallium/auxiliary/util/u_hash.c
rename to src/util/crc32.c
index b67653e..44d637c 100644
--- a/src/gallium/auxiliary/util/u_hash.c
+++ b/src/util/crc32.c
@@ -20,27 +20,27 @@
  * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
  * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS 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.
  *
  **/
 
 /**
  * @file
- * Hash functions implementation.
+ * CRC32 implementation.
  * 
  * @author Jose Fonseca
  */
 
 
-#include "u_hash.h"
+#include "crc32.h"
 
 
 static const uint32_t 
 util_crc32_table[256] = {
0x, 0x77073096, 0xee0e612c, 0x990951ba, 
0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3, 
0x0edb8832, 0x79dcb8a4, 0xe0d5e91e, 0x97d2d988, 
0x09b64c2b, 0x7eb17cbd, 0xe7b82d07, 0x90bf1d91, 
0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de, 
0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 
diff --git a/src/gallium/auxiliary/util/u_hash.h b/src/util/crc32.h
similarity index 92%
rename from src/gallium/auxiliary/util/u_hash.h
rename to src/util/crc32.h
index 8d92b07..b6a21f4 100644
--- a/src/gallium/auxiliary/util/u_hash.h
+++ b/src/util/crc32.h
@@ -20,36 +20,36 @@
  * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
  * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS 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.
  *
  **/
 
 /**
  * @file
- * Hash functions.
+ * CRC32 function.
  * 
  * @author Jose Fonseca 
  */
 
-#ifndef U_HASH_H_
-#define U_HASH_H_
+#ifndef CRC32_H_
+#define CRC32_H_
 
-
-#include "pipe/p_compiler.h"
+#include 
+#include 
 
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 

 

[Mesa-dev] [PATCH 3/3] radv: Incorporate GPU family into cache UUID.

[Bas Nieuwenhuizen]

Invalidates the cache when someone switches cards.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 04c0bdc..8595973 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -61,9 +61,10 @@ radv_get_function_timestamp(void *ptr, uint32_t* timestamp)
 }
 
 static int
-radv_device_get_cache_uuid(void *uuid)
+radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
 {
uint32_t mesa_timestamp, llvm_timestamp;
+   uint16_t f = family;
memset(uuid, 0, VK_UUID_SIZE);
if (radv_get_function_timestamp(radv_device_get_cache_uuid, 
_timestamp) ||
radv_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo, 
_timestamp))
@@ -71,7 +72,8 @@ radv_device_get_cache_uuid(void *uuid)
 
memcpy(uuid, _timestamp, 4);
memcpy((char*)uuid + 4, _timestamp, 4);
-   snprintf((char*)uuid + 8, VK_UUID_SIZE - 8, "radv");
+   memcpy((char*)uuid + 8, , 2);
+   snprintf((char*)uuid + 10, VK_UUID_SIZE - 10, "radv");
return 0;
 }
 
@@ -120,7 +122,7 @@ radv_physical_device_init(struct radv_physical_device 
*device,
goto fail;
}
 
-   if (radv_device_get_cache_uuid(device->uuid)) {
+   if (radv_device_get_cache_uuid(device->rad_info.family, device->uuid)) {
radv_finish_wsi(device);
device->ws->destroy(device->ws);
goto fail;
-- 
2.10.2

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

[Mesa-dev] [PATCH 2/3] radv: Use library mtime for cache UUID.

[Bas Nieuwenhuizen]

We want to also invalidate the cache when LLVM gets changed. As the
specific LLVM revision is not fixed at build time, we will need to
check at runtime. Computing a checksum for LLVM is going to be very
expensive, so just use the mtime.

Tested on my computer that the returned DSO for the LLVM symbol is
actually the LLVM DSO.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 36 
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index b731171..04c0bdc 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -25,10 +25,12 @@
  * IN THE SOFTWARE.
  */
 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include "radv_private.h"
 #include "util/strtod.h"
 
@@ -40,15 +42,37 @@
 #include "ac_llvm_util.h"
 #include "vk_format.h"
 #include "sid.h"
-#include "radv_timestamp.h"
 #include "util/debug.h"
 struct radv_dispatch_table dtable;
 
-static void
+static int
+radv_get_function_timestamp(void *ptr, uint32_t* timestamp)
+{
+   Dl_info info;
+   struct stat st;
+   if (!dladdr(ptr, ) || !info.dli_fname) {
+   return -1;
+   }
+   if (stat(info.dli_fname, )) {
+   return -1;
+   }
+   *timestamp = st.st_mtim.tv_sec;
+   return 0;
+}
+
+static int
 radv_device_get_cache_uuid(void *uuid)
 {
+   uint32_t mesa_timestamp, llvm_timestamp;
memset(uuid, 0, VK_UUID_SIZE);
-   snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
+   if (radv_get_function_timestamp(radv_device_get_cache_uuid, 
_timestamp) ||
+   radv_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo, 
_timestamp))
+   return -1;
+
+   memcpy(uuid, _timestamp, 4);
+   memcpy((char*)uuid + 4, _timestamp, 4);
+   snprintf((char*)uuid + 8, VK_UUID_SIZE - 8, "radv");
+   return 0;
 }
 
 static VkResult
@@ -96,7 +120,11 @@ radv_physical_device_init(struct radv_physical_device 
*device,
goto fail;
}
 
-   radv_device_get_cache_uuid(device->uuid);
+   if (radv_device_get_cache_uuid(device->uuid)) {
+   radv_finish_wsi(device);
+   device->ws->destroy(device->ws);
+   goto fail;
+   }
 
fprintf(stderr, "WARNING: radv is not a conformant vulkan 
implementation, testing use only.\n");
device->name = device->rad_info.name;
-- 
2.10.2

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

[Mesa-dev] [PATCH 1/3] radv: Store UUID in physical device.

[Bas Nieuwenhuizen]

No sense in repeatedly determining it. Also, it might be dependent
on the device as shaders get compiled differently for SI/CIK/VI etc.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 18 ++
 src/amd/vulkan/radv_pipeline_cache.c |  8 
 src/amd/vulkan/radv_private.h|  4 ++--
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 390fde0..b731171 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -44,6 +44,13 @@
 #include "util/debug.h"
 struct radv_dispatch_table dtable;
 
+static void
+radv_device_get_cache_uuid(void *uuid)
+{
+   memset(uuid, 0, VK_UUID_SIZE);
+   snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
+}
+
 static VkResult
 radv_physical_device_init(struct radv_physical_device *device,
  struct radv_instance *instance,
@@ -89,6 +96,8 @@ radv_physical_device_init(struct radv_physical_device *device,
goto fail;
}
 
+   radv_device_get_cache_uuid(device->uuid);
+
fprintf(stderr, "WARNING: radv is not a conformant vulkan 
implementation, testing use only.\n");
device->name = device->rad_info.name;
return VK_SUCCESS;
@@ -361,13 +370,6 @@ void radv_GetPhysicalDeviceFeatures(
};
 }
 
-void
-radv_device_get_cache_uuid(void *uuid)
-{
-   memset(uuid, 0, VK_UUID_SIZE);
-   snprintf(uuid, VK_UUID_SIZE, "radv-%s", RADV_TIMESTAMP);
-}
-
 void radv_GetPhysicalDeviceProperties(
VkPhysicalDevicephysicalDevice,
VkPhysicalDeviceProperties* pProperties)
@@ -498,7 +500,7 @@ void radv_GetPhysicalDeviceProperties(
};
 
strcpy(pProperties->deviceName, pdevice->name);
-   radv_device_get_cache_uuid(pProperties->pipelineCacheUUID);
+   memcpy(pProperties->pipelineCacheUUID, pdevice->uuid, VK_UUID_SIZE);
 }
 
 void radv_GetPhysicalDeviceQueueFamilyProperties(
diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
b/src/amd/vulkan/radv_pipeline_cache.c
index b429355..db824a2 100644
--- a/src/amd/vulkan/radv_pipeline_cache.c
+++ b/src/amd/vulkan/radv_pipeline_cache.c
@@ -308,8 +308,8 @@ radv_pipeline_cache_load(struct radv_pipeline_cache *cache,
 const void *data, size_t size)
 {
struct radv_device *device = cache->device;
+   struct radv_physical_device *pdevice = 
>instance->physicalDevice;
struct cache_header header;
-   uint8_t uuid[VK_UUID_SIZE];
 
if (size < sizeof(header))
return;
@@ -322,8 +322,7 @@ radv_pipeline_cache_load(struct radv_pipeline_cache *cache,
return;
if (header.device_id != 
device->instance->physicalDevice.rad_info.pci_id)
return;
-   radv_device_get_cache_uuid(uuid);
-   if (memcmp(header.uuid, uuid, VK_UUID_SIZE) != 0)
+   if (memcmp(header.uuid, pdevice->uuid, VK_UUID_SIZE) != 0)
return;
 
char *end = (void *) data + size;
@@ -405,6 +404,7 @@ VkResult radv_GetPipelineCacheData(
 {
RADV_FROM_HANDLE(radv_device, device, _device);
RADV_FROM_HANDLE(radv_pipeline_cache, cache, _cache);
+   struct radv_physical_device *pdevice = 
>instance->physicalDevice;
struct cache_header *header;
VkResult result = VK_SUCCESS;
const size_t size = sizeof(*header) + cache->total_size;
@@ -422,7 +422,7 @@ VkResult radv_GetPipelineCacheData(
header->header_version = VK_PIPELINE_CACHE_HEADER_VERSION_ONE;
header->vendor_id = 0x1002;
header->device_id = device->instance->physicalDevice.rad_info.pci_id;
-   radv_device_get_cache_uuid(header->uuid);
+   memcpy(header->uuid, pdevice->uuid, VK_UUID_SIZE);
p += header->header_size;
 
struct cache_entry *entry;
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index ad5232b..3d17895 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -258,6 +258,8 @@ struct radv_physical_device {
uint32_tpci_vendor_id;
uint32_tpci_device_id;
 
+   uint8_t uuid[VK_UUID_SIZE];
+
struct wsi_device   wsi_device;
 };
 
@@ -453,8 +455,6 @@ struct radv_device {
float sample_locations_16x[16][2];
 };
 
-void radv_device_get_cache_uuid(void *uuid);
-
 struct radv_device_memory {
struct radeon_winsys_bo  *bo;
uint32_t type_index;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH 1/2] radeonsi: add a debug flag that disables optimized shader variants

[Edward O'Callaghan]

This series is,
Reviewed-by: Edward O'Callaghan 

On 11/22/2016 08:41 AM, Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/gallium/drivers/radeon/r600_pipe_common.c   | 1 +
>  src/gallium/drivers/radeon/r600_pipe_common.h   | 1 +
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 5 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c 
> b/src/gallium/drivers/radeon/r600_pipe_common.c
> index 1a7a712..15e8a12 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.c
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
> @@ -664,20 +664,21 @@ static const struct debug_named_value 
> common_debug_options[] = {
>   { "gs", DBG_GS, "Print geometry shaders" },
>   { "ps", DBG_PS, "Print pixel shaders" },
>   { "cs", DBG_CS, "Print compute shaders" },
>   { "tcs", DBG_TCS, "Print tessellation control shaders" },
>   { "tes", DBG_TES, "Print tessellation evaluation shaders" },
>   { "noir", DBG_NO_IR, "Don't print the LLVM IR"},
>   { "notgsi", DBG_NO_TGSI, "Don't print the TGSI"},
>   { "noasm", DBG_NO_ASM, "Don't print disassembled shaders"},
>   { "preoptir", DBG_PREOPT_IR, "Print the LLVM IR before initial 
> optimizations" },
>   { "checkir", DBG_CHECK_IR, "Enable additional sanity checks on shader 
> IR" },
> + { "nooptvariant", DBG_NO_OPT_VARIANT, "Disable compiling optimized 
> shader variants." },
>  
>   { "testdma", DBG_TEST_DMA, "Invoke SDMA tests and exit." },
>  
>   /* features */
>   { "nodma", DBG_NO_ASYNC_DMA, "Disable asynchronous DMA" },
>   { "nohyperz", DBG_NO_HYPERZ, "Disable Hyper-Z" },
>   /* GL uses the word INVALIDATE, gallium uses the word DISCARD */
>   { "noinvalrange", DBG_NO_DISCARD_RANGE, "Disable handling of 
> INVALIDATE_RANGE map flags" },
>   { "no2d", DBG_NO_2D_TILING, "Disable 2D tiling" },
>   { "notiling", DBG_NO_TILING, "Disable tiling" },
> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h 
> b/src/gallium/drivers/radeon/r600_pipe_common.h
> index 941fc37..25d40da 100644
> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
> @@ -72,20 +72,21 @@
>  #define DBG_GS   (1 << 7)
>  #define DBG_PS   (1 << 8)
>  #define DBG_CS   (1 << 9)
>  #define DBG_TCS  (1 << 10)
>  #define DBG_TES  (1 << 11)
>  #define DBG_NO_IR(1 << 12)
>  #define DBG_NO_TGSI  (1 << 13)
>  #define DBG_NO_ASM   (1 << 14)
>  #define DBG_PREOPT_IR(1 << 15)
>  #define DBG_CHECK_IR (1 << 16)
> +#define DBG_NO_OPT_VARIANT   (1 << 17)
>  /* gaps */
>  #define DBG_TEST_DMA (1 << 20)
>  /* Bits 21-31 are reserved for the r600g driver. */
>  /* features */
>  #define DBG_NO_ASYNC_DMA (1llu << 32)
>  #define DBG_NO_HYPERZ(1llu << 33)
>  #define DBG_NO_DISCARD_RANGE (1llu << 34)
>  #define DBG_NO_2D_TILING (1llu << 35)
>  #define DBG_NO_TILING(1llu << 36)
>  #define DBG_SWITCH_ON_EOP(1llu << 37)
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
> b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index cd4b339..0f5a920 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1114,20 +1114,25 @@ static void si_build_shader_variant(void *job, int 
> thread_index)
>  /* Select the hw shader variant depending on the current state. */
>  static int si_shader_select_with_key(struct si_screen *sscreen,
>struct si_shader_ctx_state *state,
>struct si_shader_key *key,
>int thread_index)
>  {
>   static const struct si_shader_key zeroed;
>   struct si_shader_selector *sel = state->cso;
>   struct si_shader *current = state->current;
>   struct si_shader *iter, *shader = NULL;
> +
> + if (unlikely(sscreen->b.chip_class & DBG_NO_OPT_VARIANT)) {
> + memset(>opt, 0, sizeof(key->opt));
> + }
> +
>  again:
>   /* Check if we don't need to change anything.
>* This path is also used for most shaders that don't need multiple
>* variants, it will cost just a computation of the key and this
>* test. */
>   if (likely(current &&
>  memcmp(>key, key, sizeof(*key)) == 0 &&
>  (!current->is_optimized ||
>   util_queue_fence_is_signalled(>optimized_ready
>   return 0;
> 



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] swr: don't claim to allow setting layer/viewport from VS

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 20, 2016, at 12:20 PM, Ilia Mirkin 
> wrote:

This may ultimately be possible to support, but for now it's not hooked
up and the swr core only supports this output from GS.

This normally wouldn't matter, but we lie about supporting GL 3.2, and
also the blitter and st/mesa will make use of this functionality if
claimed.

Signed-off-by: Ilia Mirkin >
---
src/gallium/drivers/swr/swr_screen.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_screen.cpp 
b/src/gallium/drivers/swr/swr_screen.cpp
index 9affa02..bbecee5 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -252,10 +252,10 @@ swr_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
   case PIPE_CAP_USER_CONSTANT_BUFFERS:
   case PIPE_CAP_STREAM_OUTPUT_PAUSE_RESUME:
   case PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS:
-   case PIPE_CAP_TGSI_VS_LAYER_VIEWPORT:
  return 1;
   case PIPE_CAP_CONSTANT_BUFFER_OFFSET_ALIGNMENT:
  return 16;
+   case PIPE_CAP_TGSI_VS_LAYER_VIEWPORT:
   case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
   case PIPE_CAP_VERTEX_BUFFER_OFFSET_4BYTE_ALIGNED_ONLY:
   case PIPE_CAP_VERTEX_BUFFER_STRIDE_4BYTE_ALIGNED_ONLY:
--
2.7.3


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

Re: [Mesa-dev] [PATCH v3] swr: calculate viewport width/height based on the scale

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 20, 2016, at 10:32 AM, Ilia Mirkin 
> wrote:

The former calculations were for min/max y. The width/height don't take
translate into account.

Signed-off-by: Ilia Mirkin >
---

v2 -> v3:
- reduce viewport width when clamping the x/y offsets to 0
- subtract vp->y from height, not vp->x

Let's hope I don't need to write a v4 of this trivial patch.

src/gallium/drivers/swr/swr_state.cpp | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index 520faea..0302439 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1018,9 +1018,9 @@ swr_update_derived(struct pipe_context *pipe,
  SWR_VIEWPORT_MATRICES *vpm = >derived.vpm;

  vp->x = state->translate[0] - state->scale[0];
-  vp->width = state->translate[0] + state->scale[0];
+  vp->width = 2 * state->scale[0];
  vp->y = state->translate[1] - fabs(state->scale[1]);
-  vp->height = state->translate[1] + fabs(state->scale[1]);
+  vp->height = 2 * fabs(state->scale[1]);
  util_viewport_zmin_zmax(state, rasterizer->clip_halfz,
  >minZ, >maxZ);

@@ -1033,10 +1033,16 @@ swr_update_derived(struct pipe_context *pipe,

  /* Now that the matrix is calculated, clip the view coords to screen
   * size.  OpenGL allows for -ve x,y in the viewport. */
-  vp->x = std::max(vp->x, 0.0f);
-  vp->y = std::max(vp->y, 0.0f);
-  vp->width = std::min(vp->width, (float)fb->width);
-  vp->height = std::min(vp->height, (float)fb->height);
+  if (vp->x < 0.0f) {
+ vp->width += vp->x;
+ vp->x = 0.0f;
+  }
+  if (vp->y < 0.0f) {
+ vp->height += vp->y;
+ vp->y = 0.0f;
+  }
+  vp->width = std::min(vp->width, (float)fb->width - vp->x);
+  vp->height = std::min(vp->height, (float)fb->height - vp->y);

  SwrSetViewports(ctx->swrContext, 1, vp, vpm);
   }
--
2.7.3


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

[Mesa-dev] clover: OpenCL image support

[Edward O'Callaghan]

The following series implements what is needed on clover's
side for OpenCL image support. This seems to work with some
basic applications with radeonsi. Although, I have not yet
included the patch to enable by default until the LLVM bugs
this can potentially trigger with more complex kernels are
resolved.

Please kindly review,

Edward O'Callaghan (4):
 [PATCH 1/4] clover: Implement 'CL_MEM_OBJECT_IMAGE2D_ARRAY'
 [PATCH 2/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D'
 [PATCH 3/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D_BUFFER'
 [PATCH 4/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D_ARRAY'
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D_BUFFER'

[Edward O'Callaghan]

Signed-off-by: Edward O'Callaghan 
---
 src/gallium/state_trackers/clover/api/memory.cpp  |  9 -
 src/gallium/state_trackers/clover/core/memory.cpp | 13 +
 src/gallium/state_trackers/clover/core/memory.hpp | 10 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index 41e344e..7aee417 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -174,6 +174,14 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
  desc->image_width,
  desc->image_row_pitch, host_ptr);
 
+   case CL_MEM_OBJECT_IMAGE1D_BUFFER:
+  if (!desc->image_width)
+ throw error(CL_INVALID_IMAGE_SIZE);
+
+  return new image1d_buffer(ctx, flags, format,
+desc->image_width,
+desc->image_row_pitch, host_ptr);
+
case CL_MEM_OBJECT_IMAGE2D:
   if (!desc->image_width || !desc->image_height)
  throw error(CL_INVALID_IMAGE_SIZE);
@@ -223,7 +231,6 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
  desc->image_slice_pitch, host_ptr);
 
case CL_MEM_OBJECT_IMAGE1D_ARRAY:
-   case CL_MEM_OBJECT_IMAGE1D_BUFFER:
   // XXX - Not implemented.
   throw error(CL_IMAGE_FORMAT_NOT_SUPPORTED);
 
diff --git a/src/gallium/state_trackers/clover/core/memory.cpp 
b/src/gallium/state_trackers/clover/core/memory.cpp
index 246bd2d..94b1273 100644
--- a/src/gallium/state_trackers/clover/core/memory.cpp
+++ b/src/gallium/state_trackers/clover/core/memory.cpp
@@ -198,6 +198,19 @@ image1d::type() const {
return CL_MEM_OBJECT_IMAGE1D;
 }
 
+image1d_buffer::image1d_buffer(clover::context , cl_mem_flags flags,
+   const cl_image_format *format,
+   size_t width, size_t row_pitch,
+   void *host_ptr) :
+   image(ctx, flags, format, width, 0, 1,
+ row_pitch, 0, row_pitch, host_ptr) {
+}
+
+cl_mem_object_type
+image1d_buffer::type() const {
+   return CL_MEM_OBJECT_IMAGE1D_BUFFER;
+}
+
 image2d::image2d(clover::context , cl_mem_flags flags,
  const cl_image_format *format, size_t width,
  size_t height, size_t row_pitch,
diff --git a/src/gallium/state_trackers/clover/core/memory.hpp 
b/src/gallium/state_trackers/clover/core/memory.hpp
index ad9052b..1394fe6 100644
--- a/src/gallium/state_trackers/clover/core/memory.hpp
+++ b/src/gallium/state_trackers/clover/core/memory.hpp
@@ -144,6 +144,16 @@ namespace clover {
   virtual cl_mem_object_type type() const;
};
 
+   class image1d_buffer : public image {
+   public:
+  image1d_buffer(clover::context , cl_mem_flags flags,
+ const cl_image_format *format,
+ size_t width, size_t row_pitch,
+ void *host_ptr);
+
+  virtual cl_mem_object_type type() const;
+   };
+
class image2d : public image {
public:
   image2d(clover::context , cl_mem_flags flags,
-- 
2.7.4

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

[Mesa-dev] [PATCH 4/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D_ARRAY'

[Edward O'Callaghan]

Signed-off-by: Edward O'Callaghan 
---
 src/gallium/state_trackers/clover/api/memory.cpp  | 13 +
 src/gallium/state_trackers/clover/core/memory.cpp | 14 ++
 src/gallium/state_trackers/clover/core/memory.hpp | 11 +++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index 7aee417..9b3b70e 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -182,6 +182,15 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
 desc->image_width,
 desc->image_row_pitch, host_ptr);
 
+   case CL_MEM_OBJECT_IMAGE1D_ARRAY:
+  if (!desc->image_width)
+ throw error(CL_INVALID_IMAGE_SIZE);
+
+  return new image1d_array(ctx, flags, format,
+   desc->image_width,
+   desc->image_array_size, desc->image_slice_pitch,
+   host_ptr);
+
case CL_MEM_OBJECT_IMAGE2D:
   if (!desc->image_width || !desc->image_height)
  throw error(CL_INVALID_IMAGE_SIZE);
@@ -230,10 +239,6 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
  desc->image_depth, desc->image_row_pitch,
  desc->image_slice_pitch, host_ptr);
 
-   case CL_MEM_OBJECT_IMAGE1D_ARRAY:
-  // XXX - Not implemented.
-  throw error(CL_IMAGE_FORMAT_NOT_SUPPORTED);
-
default:
   throw error(CL_INVALID_IMAGE_DESCRIPTOR);
}
diff --git a/src/gallium/state_trackers/clover/core/memory.cpp 
b/src/gallium/state_trackers/clover/core/memory.cpp
index 94b1273..517d53f 100644
--- a/src/gallium/state_trackers/clover/core/memory.cpp
+++ b/src/gallium/state_trackers/clover/core/memory.cpp
@@ -211,6 +211,20 @@ image1d_buffer::type() const {
return CL_MEM_OBJECT_IMAGE1D_BUFFER;
 }
 
+image1d_array::image1d_array(clover::context , cl_mem_flags flags,
+ const cl_image_format *format,
+ size_t width,
+ size_t array_size, size_t slice_pitch,
+ void *host_ptr) :
+   image(ctx, flags, format, width, 0, 1,
+ 0, slice_pitch, slice_pitch * array_size, host_ptr) {
+}
+
+cl_mem_object_type
+image1d_array::type() const {
+   return CL_MEM_OBJECT_IMAGE1D_ARRAY;
+}
+
 image2d::image2d(clover::context , cl_mem_flags flags,
  const cl_image_format *format, size_t width,
  size_t height, size_t row_pitch,
diff --git a/src/gallium/state_trackers/clover/core/memory.hpp 
b/src/gallium/state_trackers/clover/core/memory.hpp
index 1394fe6..a54db83 100644
--- a/src/gallium/state_trackers/clover/core/memory.hpp
+++ b/src/gallium/state_trackers/clover/core/memory.hpp
@@ -154,6 +154,17 @@ namespace clover {
   virtual cl_mem_object_type type() const;
};
 
+   class image1d_array : public image {
+   public:
+  image1d_array(clover::context , cl_mem_flags flags,
+const cl_image_format *format,
+size_t width,
+size_t array_size, size_t slice_pitch,
+void *host_ptr);
+
+  virtual cl_mem_object_type type() const;
+   };
+
class image2d : public image {
public:
   image2d(clover::context , cl_mem_flags flags,
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/4] clover: Implement 'CL_MEM_OBJECT_IMAGE1D'

[Edward O'Callaghan]

Signed-off-by: Edward O'Callaghan 
---
 src/gallium/state_trackers/clover/api/memory.cpp  |  9 -
 src/gallium/state_trackers/clover/core/memory.cpp | 13 +
 src/gallium/state_trackers/clover/core/memory.hpp | 10 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index 58f56d1..41e344e 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -166,6 +166,14 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
ret_error(r_errcode, CL_SUCCESS);
 
switch (desc->image_type) {
+   case CL_MEM_OBJECT_IMAGE1D:
+  if (!desc->image_width)
+ throw error(CL_INVALID_IMAGE_SIZE);
+
+  return new image1d(ctx, flags, format,
+ desc->image_width,
+ desc->image_row_pitch, host_ptr);
+
case CL_MEM_OBJECT_IMAGE2D:
   if (!desc->image_width || !desc->image_height)
  throw error(CL_INVALID_IMAGE_SIZE);
@@ -214,7 +222,6 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
  desc->image_depth, desc->image_row_pitch,
  desc->image_slice_pitch, host_ptr);
 
-   case CL_MEM_OBJECT_IMAGE1D:
case CL_MEM_OBJECT_IMAGE1D_ARRAY:
case CL_MEM_OBJECT_IMAGE1D_BUFFER:
   // XXX - Not implemented.
diff --git a/src/gallium/state_trackers/clover/core/memory.cpp 
b/src/gallium/state_trackers/clover/core/memory.cpp
index de1862b..246bd2d 100644
--- a/src/gallium/state_trackers/clover/core/memory.cpp
+++ b/src/gallium/state_trackers/clover/core/memory.cpp
@@ -185,6 +185,19 @@ image::slice_pitch() const {
return _slice_pitch;
 }
 
+image1d::image1d(clover::context , cl_mem_flags flags,
+ const cl_image_format *format,
+ size_t width, size_t row_pitch,
+ void *host_ptr) :
+   image(ctx, flags, format, width, 0, 1,
+ row_pitch, 0, row_pitch, host_ptr) {
+}
+
+cl_mem_object_type
+image1d::type() const {
+   return CL_MEM_OBJECT_IMAGE1D;
+}
+
 image2d::image2d(clover::context , cl_mem_flags flags,
  const cl_image_format *format, size_t width,
  size_t height, size_t row_pitch,
diff --git a/src/gallium/state_trackers/clover/core/memory.hpp 
b/src/gallium/state_trackers/clover/core/memory.hpp
index 1a3e8c9..ad9052b 100644
--- a/src/gallium/state_trackers/clover/core/memory.hpp
+++ b/src/gallium/state_trackers/clover/core/memory.hpp
@@ -134,6 +134,16 @@ namespace clover {
std::unique_ptr> resources;
};
 
+   class image1d : public image {
+   public:
+  image1d(clover::context , cl_mem_flags flags,
+  const cl_image_format *format,
+  size_t width, size_t row_pitch,
+  void *host_ptr);
+
+  virtual cl_mem_object_type type() const;
+   };
+
class image2d : public image {
public:
   image2d(clover::context , cl_mem_flags flags,
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/4] clover: Implement 'CL_MEM_OBJECT_IMAGE2D_ARRAY'

[Edward O'Callaghan]

Signed-off-by: Edward O'Callaghan 
---
 src/gallium/state_trackers/clover/api/memory.cpp  | 17 -
 src/gallium/state_trackers/clover/core/memory.cpp | 14 ++
 src/gallium/state_trackers/clover/core/memory.hpp | 11 +++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/clover/api/memory.cpp 
b/src/gallium/state_trackers/clover/api/memory.cpp
index 9b3cd8b..58f56d1 100644
--- a/src/gallium/state_trackers/clover/api/memory.cpp
+++ b/src/gallium/state_trackers/clover/api/memory.cpp
@@ -181,6 +181,22 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
  desc->image_width, desc->image_height,
  desc->image_row_pitch, host_ptr);
 
+   case CL_MEM_OBJECT_IMAGE2D_ARRAY:
+  if (!desc->image_width || !desc->image_height)
+ throw error(CL_INVALID_IMAGE_SIZE);
+
+  if (all_of([=](const device ) {
+   const size_t max = 1 << dev.max_image_levels_2d();
+   return (desc->image_width > max ||
+   desc->image_height > max);
+}, ctx.devices()))
+ throw error(CL_INVALID_IMAGE_SIZE);
+
+  return new image2d_array(ctx, flags, format,
+   desc->image_width, desc->image_height,
+   desc->image_array_size, desc->image_slice_pitch,
+   host_ptr);
+
case CL_MEM_OBJECT_IMAGE3D:
   if (!desc->image_width || !desc->image_height || !desc->image_depth)
  throw error(CL_INVALID_IMAGE_SIZE);
@@ -201,7 +217,6 @@ clCreateImage(cl_context d_ctx, cl_mem_flags d_flags,
case CL_MEM_OBJECT_IMAGE1D:
case CL_MEM_OBJECT_IMAGE1D_ARRAY:
case CL_MEM_OBJECT_IMAGE1D_BUFFER:
-   case CL_MEM_OBJECT_IMAGE2D_ARRAY:
   // XXX - Not implemented.
   throw error(CL_IMAGE_FORMAT_NOT_SUPPORTED);
 
diff --git a/src/gallium/state_trackers/clover/core/memory.cpp 
b/src/gallium/state_trackers/clover/core/memory.cpp
index b852e68..de1862b 100644
--- a/src/gallium/state_trackers/clover/core/memory.cpp
+++ b/src/gallium/state_trackers/clover/core/memory.cpp
@@ -198,6 +198,20 @@ image2d::type() const {
return CL_MEM_OBJECT_IMAGE2D;
 }
 
+image2d_array::image2d_array(clover::context , cl_mem_flags flags,
+ const cl_image_format *format,
+ size_t width, size_t height,
+ size_t array_size, size_t slice_pitch,
+ void *host_ptr) :
+   image(ctx, flags, format, width, height, 1,
+ 0, slice_pitch, slice_pitch * array_size, host_ptr) {
+}
+
+cl_mem_object_type
+image2d_array::type() const {
+   return CL_MEM_OBJECT_IMAGE2D_ARRAY;
+}
+
 image3d::image3d(clover::context , cl_mem_flags flags,
  const cl_image_format *format,
  size_t width, size_t height, size_t depth,
diff --git a/src/gallium/state_trackers/clover/core/memory.hpp 
b/src/gallium/state_trackers/clover/core/memory.hpp
index bd6da6b..1a3e8c9 100644
--- a/src/gallium/state_trackers/clover/core/memory.hpp
+++ b/src/gallium/state_trackers/clover/core/memory.hpp
@@ -144,6 +144,17 @@ namespace clover {
   virtual cl_mem_object_type type() const;
};
 
+   class image2d_array : public image {
+   public:
+  image2d_array(clover::context , cl_mem_flags flags,
+const cl_image_format *format,
+size_t width, size_t height,
+size_t array_size, size_t slice_pitch,
+void *host_ptr);
+
+  virtual cl_mem_object_type type() const;
+   };
+
class image3d : public image {
public:
   image3d(clover::context , cl_mem_flags flags,
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] swr: call swr_update_derived unconditionally when drawing/clearing

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak 

> On Nov 19, 2016, at 11:09 PM, Ilia Mirkin  wrote:
> 
> Currently a sequence like draw/map/draw/map will cause the second map to
> not wait for the second draw. This is because the first map will clear
> the resource business bit, and the second draw won't reset it since no
> state has changed.
> 
> swr_update_derived does a tiny bit of extra work, including updating the
> SWR_BACKEND_STATE as well as waiting for prending fences. If that's a
> problem, we could call swr_update_resource_status directly from
> draw/clear handlers.
> 
> Fixes clearbuffer-stencil, clearbuffer-depth, clearbuffer-depth-stencil,
> and clearbuffer-display-lists.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> src/gallium/drivers/swr/swr_clear.cpp | 3 +--
> src/gallium/drivers/swr/swr_draw.cpp  | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_clear.cpp 
> b/src/gallium/drivers/swr/swr_clear.cpp
> index 7ac308e..f59179f 100644
> --- a/src/gallium/drivers/swr/swr_clear.cpp
> +++ b/src/gallium/drivers/swr/swr_clear.cpp
> @@ -40,8 +40,7 @@ swr_clear(struct pipe_context *pipe,
>if (!swr_check_render_cond(pipe))
>   return;
> 
> -   if (ctx->dirty)
> -  swr_update_derived(pipe);
> +   swr_update_derived(pipe);
> 
>if (buffers & PIPE_CLEAR_COLOR && fb->nr_cbufs) {
>   for (unsigned i = 0; i < fb->nr_cbufs; ++i)
> diff --git a/src/gallium/drivers/swr/swr_draw.cpp 
> b/src/gallium/drivers/swr/swr_draw.cpp
> index fc39f6f..c4d5e5c 100644
> --- a/src/gallium/drivers/swr/swr_draw.cpp
> +++ b/src/gallium/drivers/swr/swr_draw.cpp
> @@ -90,8 +90,7 @@ swr_draw_vbo(struct pipe_context *pipe, const struct 
> pipe_draw_info *info)
>}
> 
>/* Update derived state, pass draw info to update function */
> -   if (ctx->dirty)
> -  swr_update_derived(pipe, info);
> +   swr_update_derived(pipe, info);
> 
>swr_update_draw_context(ctx);
> 
> -- 
> 2.7.3
> 

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

Re: [Mesa-dev] [PATCH] swr: allocate all scratch space in one go for vertex buffers

[Cherniak, Bruce]

Reviewed-by: Bruce Cherniak  

> On Nov 20, 2016, at 4:25 PM, Ilia Mirkin  wrote:
> 
> Multiple buffers may reference client arrays. When this happens, we
> might reach for scratch space multiple times, which could cause later
> arrays to invalidate the pointers allocated for the earlier ones.
> 
> This fixes copyteximage 2D_ARRAY.
> 
> Signed-off-by: Ilia Mirkin 
> ---
> 
> It also fixes a bunch of random piglits. Also explains why it was always the
> first thing that failed in copyteximage 2D_ARRAY -- the later ones had enough
> scratch space to hold the whole thing.
> 
> src/gallium/drivers/swr/swr_scratch.cpp |  4 ++--
> src/gallium/drivers/swr/swr_state.cpp   | 32 +---
> 2 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_scratch.cpp 
> b/src/gallium/drivers/swr/swr_scratch.cpp
> index 28eb2ac..2515c8b 100644
> --- a/src/gallium/drivers/swr/swr_scratch.cpp
> +++ b/src/gallium/drivers/swr/swr_scratch.cpp
> @@ -35,7 +35,6 @@ swr_copy_to_scratch_space(struct swr_context *ctx,
> {
>void *ptr;
>assert(space);
> -   assert(user_buffer);
>assert(size);
> 
>if (size >= 2048) { /* XXX TODO create KNOB_ for this */
> @@ -82,7 +81,8 @@ swr_copy_to_scratch_space(struct swr_context *ctx,
>}
> 
>/* Copy user_buffer to scratch */
> -   memcpy(ptr, user_buffer, size);
> +   if (user_buffer)
> +  memcpy(ptr, user_buffer, size);
> 
>return ptr;
> }
> diff --git a/src/gallium/drivers/swr/swr_state.cpp 
> b/src/gallium/drivers/swr/swr_state.cpp
> index f5f42ba..dcbe434 100644
> --- a/src/gallium/drivers/swr/swr_state.cpp
> +++ b/src/gallium/drivers/swr/swr_state.cpp
> @@ -1039,14 +1039,39 @@ swr_update_derived(struct pipe_context *pipe,
>/* Set vertex & index buffers */
>/* (using draw info if called by swr_draw_vbo) */
>if (ctx->dirty & SWR_NEW_VERTEX) {
> -  uint32_t size, pitch, max_vertex, partial_inbounds;
> +  uint32_t size, pitch, max_vertex, partial_inbounds, scratch_total;
>   const uint8_t *p_data;
> +  uint8_t *scratch = NULL;
> 
>   /* If being called by swr_draw_vbo, copy draw details */
>   struct pipe_draw_info info = {0};
>   if (p_draw_info)
>  info = *p_draw_info;
> 
> +  /* We must get all the scratch space in one go */
> +  scratch_total = 0;
> +  for (UINT i = 0; i < ctx->num_vertex_buffers; i++) {
> + struct pipe_vertex_buffer *vb = >vertex_buffer[i];
> +
> + if (!vb->user_buffer)
> +continue;
> +
> + if (vb->stride) {
> +size = (info.max_index - info.min_index + 1) * vb->stride;
> + } else {
> +/* pitch = 0, means constant value
> + * set size to 1 vertex */
> +size = ctx->velems->stream_pitch[i];
> + }
> +
> + scratch_total += AlignUp(size, 4);
> +  }
> +
> +  if (scratch_total) {
> + scratch = (uint8_t *)swr_copy_to_scratch_space(
> +   ctx, >scratch->vertex_buffer, NULL, scratch_total);
> +  }
> +
>   /* vertex buffers */
>   SWR_VERTEX_BUFFER_STATE swrVertexBuffers[PIPE_MAX_ATTRIBS];
>   for (UINT i = 0; i < ctx->num_vertex_buffers; i++) {
> @@ -1083,8 +1108,9 @@ swr_update_derived(struct pipe_context *pipe,
> size = AlignUp(size, 4);
> const void *ptr = (const uint8_t *) vb->user_buffer
>+ info.min_index * pitch;
> -ptr = swr_copy_to_scratch_space(
> -   ctx, >scratch->vertex_buffer, ptr, size);
> +memcpy(scratch, ptr, size);
> +ptr = scratch;
> +scratch += size;
> p_data = (const uint8_t *)ptr - info.min_index * pitch;
>  }
> 
> -- 
> 2.7.3
> 

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

Re: [Mesa-dev] [PATCH] swr: report a reasonable max lod bias

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 19, 2016, at 10:11 AM, Ilia Mirkin 
> wrote:

This is the same value that llvmpipe uses. Since swr uses the same
sampler logic, makes sense for this value to also be the same. Most
applications don't care.

Signed-off-by: Ilia Mirkin >
---

I kind of assume this is dependent on my layout patches since LODs weren't
always properly handled before.

src/gallium/drivers/swr/swr_screen.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_screen.cpp 
b/src/gallium/drivers/swr/swr_screen.cpp
index 36afcc3..9affa02 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -406,7 +406,7 @@ swr_get_paramf(struct pipe_screen *screen, enum pipe_capf 
param)
   case PIPE_CAPF_MAX_TEXTURE_ANISOTROPY:
  return 0.0;
   case PIPE_CAPF_MAX_TEXTURE_LOD_BIAS:
-  return 0.0;
+  return 16.0; /* arbitrary */
   case PIPE_CAPF_GUARD_BAND_LEFT:
   case PIPE_CAPF_GUARD_BAND_TOP:
   case PIPE_CAPF_GUARD_BAND_RIGHT:
--
2.7.3


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

Re: [Mesa-dev] [PATCH] docs/submitting patches: mention get_reviewers.pl

[tournier.elie]

Nice,

Reviewed-by: Elie Tournier 



2016-11-21 18:56 GMT+01:00 Emil Velikov :

> From: Emil Velikov 
>
> Mention the script - why/how to use alongside a useful trick to make it
> work interactively (thanks Rob!).
>
> Signed-off-by: Emil Velikov 
> ---
> Based on top of the "git tips" from earlier.
> ---
>  docs/submittingpatches.html | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 7ada508..2f4be5d 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -104,6 +104,19 @@ that should be documented with:
>  Reviewed-by: Joe Hacker jhac...@foo.com
>  Acked-by: Joe Hacker jhac...@foo.com
>  
> +In order for your patch to reach the prospective reviewer
> easier/faster,
> +use the script scripts/get_reviewer.pl to get a list of individuals and
> include
> +them in the CC list.
> +
> +Please use common sense and do not blindly add everyone.
> +
> +
> +$ scripts/get_reviewer.pl --help # to get the the help screen
> +$ scripts/get_reviewer.pl -f src/egl/drivers/dri2/platform_android.c
> +Rob Herring  (reviewer:ANDROID EGL
> SUPPORT,added_lines:188/700=27%,removed_lines:58/283=20%)
> +Tomasz Figa  (reviewer:ANDROID EGL
> SUPPORT,authored:12/41=29%,added_lines:308/700=44%,
> removed_lines:115/283=41%)
> +Emil Velikov  (authored:13/41=32%,removed_
> lines:76/283=27%)
> +
>  
>
>
> @@ -335,6 +348,11 @@ commits before sending.
>  git send-email --subject-prefix="PATCH v4" HEAD~8
>  git send-email -v4 HEAD~8 # shorter version, 'inherited' from git
> format-patch
>  
> + Configure git to use the get_reviewer.pl script interactively. Thus
> you
> +can avoid adding the world to the CC list.
> +
> +git config sendemail.cccmd "./scripts/get_reviewer.pl -i"
> +
>  
>
>
> --
> 2.10.2
>
> ___
> 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 1/2] intel/common: Add an is_kabylake field to gen_device_info

[Matt Turner]

On Tue, Nov 8, 2016 at 1:21 PM, Jason Ekstrand  wrote:
> Most of the 3-D engine Kaby Lake is identical to Sky Lake.  However, there
> are a few small differences that we need to be able to detect.
>
> Signed-off-by: Jason Ekstrand 

I noticed this patch has never landed.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] docs/submitting patches: add git tips

[Timothy Arceri]

On 22 November 2016 3:37:32 am AEDT, Emil Velikov  
wrote:
>From: Timothy Arceri 
>
>v2: [Emil Velikov]
> - Add the shorthand git send-email -vX
> - Move to submittingpatches.html
> - Add to the TOC.
>
>Cc: Timothy Arceri 
>Signed-off-by: Emil Velikov 
>Reviewed-by: Brian Paul  (v1)
>---
>Tim any objections on merging this ? IMHO these are quite useful.

No objections I was just to lazy to make the suggested updates. Thanks for 
doing this :-) 

>---
> docs/submittingpatches.html | 20 
> 1 file changed, 20 insertions(+)
>
>diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
>index d3e86f5..7ada508 100644
>--- a/docs/submittingpatches.html
>+++ b/docs/submittingpatches.html
>@@ -25,6 +25,7 @@
> Reviewing Patches
> Nominating a commit for a stable branch
>Criteria for accepting patches to the stable
>branch
>+Git tips
> 
> 
> Basic guidelines
>@@ -317,6 +318,25 @@ be rejected:
>   regression that is unaacceptable for the stable branch.
> 
> 
>+Git tips
>+
>+
>+Test for build breakage between patches e.g last 8 commits.
>+
>+git rebase -i --exec="make -j4" HEAD~8
>+
>+Sets the default mailing address for your repo.
>+
>+git config --local sendemail.to mesa-dev@lists.freedesktop.org
>+
>+ Add version to subject line of patch series in this case for the
>last 8
>+commits before sending.
>+
>+git send-email --subject-prefix="PATCH v4" HEAD~8
>+git send-email -v4 HEAD~8 # shorter version, 'inherited' from git
>format-patch
>+
>+
>+
> 
> 
> 

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

[Mesa-dev] [PATCH 2/2] st/va: fix gop size for rate control

[boyuan.zhang]

From: Boyuan Zhang 

The gop_size in rate control is the budget window for internal rate
control calculation, and shouldn't always equal to idr period. Define
a coefficient to let budget window contains a number of idr period for
proper rate control calculation. Adjust the number of i/p frame remaining
accordingly.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98005

Signed-off-by: Boyuan Zhang 
---
 src/gallium/state_trackers/va/picture.c| 18 --
 src/gallium/state_trackers/va/va_private.h |  2 ++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index 592cdef..b5b9a83 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -351,7 +351,11 @@ handleVAEncSequenceParameterBufferType(vlVaDriver *drv, 
vlVaContext *context, vl
   if (!context->decoder)
  return VA_STATUS_ERROR_ALLOCATION_FAILED;
}
-   context->desc.h264enc.gop_size = h264->intra_idr_period;
+
+   context->gop_coeff = ((1024 + h264->intra_idr_period - 1) / 
h264->intra_idr_period + 1) / 2 * 2;
+   if (context->gop_coeff > VL_VA_ENC_GOP_COEFF)
+  context->gop_coeff = VL_VA_ENC_GOP_COEFF;
+   context->desc.h264enc.gop_size = h264->intra_idr_period * 
context->gop_coeff;
context->desc.h264enc.rate_ctrl.frame_rate_num = h264->time_scale / 2;
context->desc.h264enc.rate_ctrl.frame_rate_den = 1;
return VA_STATUS_SUCCESS;
@@ -391,10 +395,10 @@ handleVAEncPictureParameterBufferType(vlVaDriver *drv, 
vlVaContext *context, vlV
context->desc.h264enc.not_referenced = false;
context->desc.h264enc.is_idr = (h264->pic_fields.bits.idr_pic_flag == 1);
context->desc.h264enc.pic_order_cnt = h264->CurrPic.TopFieldOrderCnt;
-   if (context->desc.h264enc.is_idr)
-  context->desc.h264enc.i_remain = 1;
-   else
-  context->desc.h264enc.i_remain = 0;
+   if (context->desc.h264enc.gop_cnt == 0)
+  context->desc.h264enc.i_remain = context->gop_coeff;
+   else if (context->desc.h264enc.frame_num == 1)
+  context->desc.h264enc.i_remain--;
 
context->desc.h264enc.p_remain = context->desc.h264enc.gop_size - 
context->desc.h264enc.gop_cnt - context->desc.h264enc.i_remain;
 
@@ -578,6 +582,8 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID context_id)
 
context->decoder->end_frame(context->decoder, context->target, 
>desc.base);
if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
+  int idr_period = context->desc.h264enc.gop_size / context->gop_coeff;
+  int p_remain_in_idr = idr_period - context->desc.h264enc.frame_num;
   surf->frame_num_cnt = context->desc.h264enc.frame_num_cnt;
   surf->force_flushed = false;
   if (context->first_single_submitted) {
@@ -585,7 +591,7 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID context_id)
  context->first_single_submitted = false;
  surf->force_flushed = true;
   }
-  if (context->desc.h264enc.p_remain == 1) {
+  if (p_remain_in_idr == 1) {
  if ((context->desc.h264enc.frame_num_cnt % 2) != 0) {
 context->decoder->flush(context->decoder);
 context->first_single_submitted = true;
diff --git a/src/gallium/state_trackers/va/va_private.h 
b/src/gallium/state_trackers/va/va_private.h
index 9e3ba03..900abbc 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -50,6 +50,7 @@
 #define VL_VA_PSCREEN(ctx) (VL_VA_DRIVER(ctx)->vscreen->pscreen)
 
 #define VL_VA_MAX_IMAGE_FORMATS 9
+#define VL_VA_ENC_GOP_COEFF 16
 
 static inline enum pipe_video_chroma_format
 ChromaToPipe(int format)
@@ -245,6 +246,7 @@ typedef struct {
struct vlVaBuffer *coded_buf;
int target_id;
bool first_single_submitted;
+   int gop_coeff;
 } vlVaContext;
 
 typedef struct {
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] st/va: force to submit two consecutive single jobs

[boyuan.zhang]

From: Boyuan Zhang 

The gop_size in rate control is the budget window for internal rate
control calculation, and shouldn't always equal to idr period. Define
a coefficient to let budget window contains a number of idr period for
proper rate control calculation. Adjust the number of i/p frame remaining
accordingly.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98005

Signed-off-by: Boyuan Zhang 
---
 src/gallium/state_trackers/va/picture.c| 24 +++-
 src/gallium/state_trackers/va/surface.c|  8 ++--
 src/gallium/state_trackers/va/va_private.h |  2 ++
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index a8102a4..592cdef 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -413,7 +413,6 @@ handleVAEncPictureParameterBufferType(vlVaDriver *drv, 
vlVaContext *context, vlV
context->desc.h264enc.quant_i_frames = h264->pic_init_qp;
context->desc.h264enc.quant_b_frames = h264->pic_init_qp;
context->desc.h264enc.quant_p_frames = h264->pic_init_qp;
-   context->desc.h264enc.frame_num_cnt++;
context->desc.h264enc.gop_cnt++;
if (context->desc.h264enc.gop_cnt == context->desc.h264enc.gop_size)
   context->desc.h264enc.gop_cnt = 0;
@@ -569,18 +568,33 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID 
context_id)
if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
   coded_buf = context->coded_buf;
   getEncParamPreset(context);
+  context->desc.h264enc.frame_num_cnt++;
   context->decoder->begin_frame(context->decoder, context->target, 
>desc.base);
   context->decoder->encode_bitstream(context->decoder, context->target,
  coded_buf->derived_surface.resource, 
);
-  surf->frame_num_cnt = context->desc.h264enc.frame_num_cnt;
   surf->feedback = feedback;
   surf->coded_buf = coded_buf;
}
 
context->decoder->end_frame(context->decoder, context->target, 
>desc.base);
-   if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE &&
-   context->desc.h264enc.p_remain == 1)
-  context->decoder->flush(context->decoder);
+   if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
+  surf->frame_num_cnt = context->desc.h264enc.frame_num_cnt;
+  surf->force_flushed = false;
+  if (context->first_single_submitted) {
+ context->decoder->flush(context->decoder);
+ context->first_single_submitted = false;
+ surf->force_flushed = true;
+  }
+  if (context->desc.h264enc.p_remain == 1) {
+ if ((context->desc.h264enc.frame_num_cnt % 2) != 0) {
+context->decoder->flush(context->decoder);
+context->first_single_submitted = true;
+ }
+ else
+context->first_single_submitted = false;
+ surf->force_flushed = true;
+  }
+   }
pipe_mutex_unlock(drv->mutex);
return VA_STATUS_SUCCESS;
 }
diff --git a/src/gallium/state_trackers/va/surface.c 
b/src/gallium/state_trackers/va/surface.c
index f8513d9..85d2b9d 100644
--- a/src/gallium/state_trackers/va/surface.c
+++ b/src/gallium/state_trackers/va/surface.c
@@ -125,12 +125,16 @@ vlVaSyncSurface(VADriverContextP ctx, VASurfaceID 
render_target)
 
if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
   int frame_diff;
-  if (context->desc.h264enc.frame_num_cnt > surf->frame_num_cnt)
+  if (context->desc.h264enc.frame_num_cnt >= surf->frame_num_cnt)
  frame_diff = context->desc.h264enc.frame_num_cnt - 
surf->frame_num_cnt;
   else
  frame_diff = 0x - surf->frame_num_cnt + 1 + 
context->desc.h264enc.frame_num_cnt;
-  if (frame_diff < 2)
+  if (frame_diff == 0 && surf->force_flushed == false) {
  context->decoder->flush(context->decoder);
+ if (context->desc.h264enc.rate_ctrl.rate_ctrl_method !=
+PIPE_H264_ENC_RATE_CONTROL_METHOD_DISABLE)
+context->first_single_submitted = true;
+  }
   context->decoder->get_feedback(context->decoder, surf->feedback, 
&(surf->coded_buf->coded_size));
   surf->feedback = NULL;
}
diff --git a/src/gallium/state_trackers/va/va_private.h 
b/src/gallium/state_trackers/va/va_private.h
index c9a6a41..9e3ba03 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -244,6 +244,7 @@ typedef struct {
struct vl_deint_filter *deint;
struct vlVaBuffer *coded_buf;
int target_id;
+   bool first_single_submitted;
 } vlVaContext;
 
 typedef struct {
@@ -274,6 +275,7 @@ typedef struct {
vlVaBuffer *coded_buf;
void *feedback;
unsigned int frame_num_cnt;
+   bool force_flushed;
 } vlVaSurface;
 
 // Public functions:
-- 
2.7.4

___
mesa-dev mailing list

Re: [Mesa-dev] list.h vs. simple_list.h

[Eric Anholt]

Brian Paul  writes:

> [resending, original msg didn't appear on list]
>
> It seems a bit silly to have two linked list utilities.
>
> git-grep indicates that list.h is used more than simple_list.h so maybe 
> we can switch uses of the later to the former.  In fact, I've already 
> found a few files that include simple_list.h but don't actually need it.

Sounds great to me.  list.h has been serving me pretty well.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] radeonsi: print new opt flags in si_dump_shader_key

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_shader.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
b/src/gallium/drivers/radeonsi/si_shader.c
index abe30e5..6228a68 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -6325,20 +6325,29 @@ static void si_dump_shader_key(unsigned shader, struct 
si_shader_key *key,
fprintf(f, "  part.ps.epilog.last_cbuf = %u\n", 
key->part.ps.epilog.last_cbuf);
fprintf(f, "  part.ps.epilog.alpha_func = %u\n", 
key->part.ps.epilog.alpha_func);
fprintf(f, "  part.ps.epilog.alpha_to_one = %u\n", 
key->part.ps.epilog.alpha_to_one);
fprintf(f, "  part.ps.epilog.poly_line_smoothing = %u\n", 
key->part.ps.epilog.poly_line_smoothing);
fprintf(f, "  part.ps.epilog.clamp_color = %u\n", 
key->part.ps.epilog.clamp_color);
break;
 
default:
assert(0);
}
+
+   if ((shader == PIPE_SHADER_GEOMETRY ||
+shader == PIPE_SHADER_TESS_EVAL ||
+shader == PIPE_SHADER_VERTEX) &&
+   !key->as_es && !key->as_ls) {
+   fprintf(f, "  opt.hw_vs.kill_outputs = 0x%"PRIx64"\n", 
key->opt.hw_vs.kill_outputs);
+   fprintf(f, "  opt.hw_vs.kill_outputs2 = 0x%x\n", 
key->opt.hw_vs.kill_outputs2);
+   fprintf(f, "  opt.hw_vs.clip_disable = %u\n", 
key->opt.hw_vs.clip_disable);
+   }
 }
 
 static void si_init_shader_ctx(struct si_shader_context *ctx,
   struct si_screen *sscreen,
   struct si_shader *shader,
   LLVMTargetMachineRef tm)
 {
struct lp_build_tgsi_context *bld_base;
struct lp_build_tgsi_action tmpl = {};
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] radeonsi: add a debug flag that disables optimized shader variants

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/drivers/radeon/r600_pipe_common.c   | 1 +
 src/gallium/drivers/radeon/r600_pipe_common.h   | 1 +
 src/gallium/drivers/radeonsi/si_state_shaders.c | 5 +
 3 files changed, 7 insertions(+)

diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c 
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 1a7a712..15e8a12 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -664,20 +664,21 @@ static const struct debug_named_value 
common_debug_options[] = {
{ "gs", DBG_GS, "Print geometry shaders" },
{ "ps", DBG_PS, "Print pixel shaders" },
{ "cs", DBG_CS, "Print compute shaders" },
{ "tcs", DBG_TCS, "Print tessellation control shaders" },
{ "tes", DBG_TES, "Print tessellation evaluation shaders" },
{ "noir", DBG_NO_IR, "Don't print the LLVM IR"},
{ "notgsi", DBG_NO_TGSI, "Don't print the TGSI"},
{ "noasm", DBG_NO_ASM, "Don't print disassembled shaders"},
{ "preoptir", DBG_PREOPT_IR, "Print the LLVM IR before initial 
optimizations" },
{ "checkir", DBG_CHECK_IR, "Enable additional sanity checks on shader 
IR" },
+   { "nooptvariant", DBG_NO_OPT_VARIANT, "Disable compiling optimized 
shader variants." },
 
{ "testdma", DBG_TEST_DMA, "Invoke SDMA tests and exit." },
 
/* features */
{ "nodma", DBG_NO_ASYNC_DMA, "Disable asynchronous DMA" },
{ "nohyperz", DBG_NO_HYPERZ, "Disable Hyper-Z" },
/* GL uses the word INVALIDATE, gallium uses the word DISCARD */
{ "noinvalrange", DBG_NO_DISCARD_RANGE, "Disable handling of 
INVALIDATE_RANGE map flags" },
{ "no2d", DBG_NO_2D_TILING, "Disable 2D tiling" },
{ "notiling", DBG_NO_TILING, "Disable tiling" },
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h 
b/src/gallium/drivers/radeon/r600_pipe_common.h
index 941fc37..25d40da 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.h
+++ b/src/gallium/drivers/radeon/r600_pipe_common.h
@@ -72,20 +72,21 @@
 #define DBG_GS (1 << 7)
 #define DBG_PS (1 << 8)
 #define DBG_CS (1 << 9)
 #define DBG_TCS(1 << 10)
 #define DBG_TES(1 << 11)
 #define DBG_NO_IR  (1 << 12)
 #define DBG_NO_TGSI(1 << 13)
 #define DBG_NO_ASM (1 << 14)
 #define DBG_PREOPT_IR  (1 << 15)
 #define DBG_CHECK_IR   (1 << 16)
+#define DBG_NO_OPT_VARIANT (1 << 17)
 /* gaps */
 #define DBG_TEST_DMA   (1 << 20)
 /* Bits 21-31 are reserved for the r600g driver. */
 /* features */
 #define DBG_NO_ASYNC_DMA   (1llu << 32)
 #define DBG_NO_HYPERZ  (1llu << 33)
 #define DBG_NO_DISCARD_RANGE   (1llu << 34)
 #define DBG_NO_2D_TILING   (1llu << 35)
 #define DBG_NO_TILING  (1llu << 36)
 #define DBG_SWITCH_ON_EOP  (1llu << 37)
diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
b/src/gallium/drivers/radeonsi/si_state_shaders.c
index cd4b339..0f5a920 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -1114,20 +1114,25 @@ static void si_build_shader_variant(void *job, int 
thread_index)
 /* Select the hw shader variant depending on the current state. */
 static int si_shader_select_with_key(struct si_screen *sscreen,
 struct si_shader_ctx_state *state,
 struct si_shader_key *key,
 int thread_index)
 {
static const struct si_shader_key zeroed;
struct si_shader_selector *sel = state->cso;
struct si_shader *current = state->current;
struct si_shader *iter, *shader = NULL;
+
+   if (unlikely(sscreen->b.chip_class & DBG_NO_OPT_VARIANT)) {
+   memset(>opt, 0, sizeof(key->opt));
+   }
+
 again:
/* Check if we don't need to change anything.
 * This path is also used for most shaders that don't need multiple
 * variants, it will cost just a computation of the key and this
 * test. */
if (likely(current &&
   memcmp(>key, key, sizeof(*key)) == 0 &&
   (!current->is_optimized ||
util_queue_fence_is_signalled(>optimized_ready
return 0;
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH v2] radeonsi: Add missing error-checking to si_create_compute_state (v2)

[Marek Olšák]

Pushed, thanks.

Marek

On Mon, Nov 21, 2016 at 3:24 PM, Edward O'Callaghan
 wrote:
> Reviewed-by: Edward O'Callaghan 
>
> On 11/22/2016 01:20 AM, Mun Gwan-gyeong wrote:
>> When the uploading of shader fails on si_shader_binary_upload(),
>> it returns -ENOMEM. We should handle si_shader_binary_upload() failure path
>> on si_create_compute_state().
>>
>> CID 1394027
>>
>> v2: Fixes from Edward O'Callaghan's review
>>  a) Update explicitly return value check with "si_shader_binary_upload() < 0"
>>  b) Update commit message.
>>
>> Signed-off-by: Mun Gwan-gyeong 
>> ---
>>  src/gallium/drivers/radeonsi/si_compute.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
>> b/src/gallium/drivers/radeonsi/si_compute.c
>> index f1887bb..0815713 100644
>> --- a/src/gallium/drivers/radeonsi/si_compute.c
>> +++ b/src/gallium/drivers/radeonsi/si_compute.c
>> @@ -165,7 +165,11 @@ static void *si_create_compute_state(
>>   }
>>   si_shader_dump(sctx->screen, >shader, >b.debug,
>>  PIPE_SHADER_COMPUTE, stderr);
>> - si_shader_binary_upload(sctx->screen, >shader);
>> + if (si_shader_binary_upload(sctx->screen, >shader) < 
>> 0) {
>> + fprintf(stderr, "LLVM failed to upload shader\n");
>> + FREE(program);
>> + return NULL;
>> + }
>>   }
>>
>>   return program;
>>
>
>
> ___
> 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 v2 1/6] swr: [rasterizer memory] minify original sizes for block formats

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 17, 2016, at 10:56 PM, Ilia Mirkin 
> wrote:

There's no guarantee that mip width/height will be a multiple of the
compressed block size. Doing a divide by the block size first yields
different results than GL expects, so we do the divide at the end.

Signed-off-by: Ilia Mirkin >
---
.../swr/rasterizer/memory/TilingFunctions.h| 36 +++---
1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h 
b/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
index 0694a99..11ed451 100644
--- a/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
+++ b/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
@@ -274,9 +274,12 @@ INLINE void ComputeLODOffset1D(
else
{
uint32_t curWidth = baseWidth;
-// translate mip width from pixels to blocks for block compressed 
formats
-// @note hAlign is already in blocks for compressed formats so no need 
to convert
-if (info.isBC) curWidth /= info.bcWidth;
+// @note hAlign is already in blocks for compressed formats so 
upconvert
+//   so that we have the desired alignment post-divide.
+if (info.isBC)
+{
+hAlign *= info.bcWidth;
+}

offset = GFX_ALIGN(curWidth, hAlign);
for (uint32_t l = 1; l < lod; ++l)
@@ -285,7 +288,7 @@ INLINE void ComputeLODOffset1D(
offset += curWidth;
}

-if (info.isSubsampled)
+if (info.isSubsampled || info.isBC)
{
offset /= info.bcWidth;
}
@@ -312,14 +315,17 @@ INLINE void ComputeLODOffsetX(
else
{
uint32_t curWidth = baseWidth;
-// convert mip width from pixels to blocks for block compressed formats
-// @note hAlign is already in blocks for compressed formats so no need 
to convert
-if (info.isBC) curWidth /= info.bcWidth;
+// @note hAlign is already in blocks for compressed formats so 
upconvert
+//   so that we have the desired alignment post-divide.
+if (info.isBC)
+{
+hAlign *= info.bcWidth;
+}

curWidth = std::max(curWidth >> 1, 1U);
curWidth = GFX_ALIGN(curWidth, hAlign);

-if (info.isSubsampled)
+if (info.isSubsampled || info.isBC)
{
curWidth /= info.bcWidth;
}
@@ -350,9 +356,12 @@ INLINE void ComputeLODOffsetY(
offset = 0;
uint32_t mipHeight = baseHeight;

-// translate mip height from pixels to blocks for block compressed 
formats
-// @note VAlign is already in blocks for compressed formats so no need 
to convert
-if (info.isBC) mipHeight /= info.bcHeight;
+// @note vAlign is already in blocks for compressed formats so 
upconvert
+//   so that we have the desired alignment post-divide.
+if (info.isBC)
+{
+vAlign *= info.bcHeight;
+}

for (uint32_t l = 1; l <= lod; ++l)
{
@@ -360,6 +369,11 @@ INLINE void ComputeLODOffsetY(
offset += ((l != 2) ? alignedMipHeight : 0);
mipHeight = std::max(mipHeight >> 1, 1U);
}
+
+if (info.isBC)
+{
+offset /= info.bcHeight;
+}
}
}

--
2.7.3


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

Re: [Mesa-dev] [PATCH v2 2/6] swr: [rasterizer memory] minify texture width before alignment

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 17, 2016, at 10:56 PM, Ilia Mirkin 
> wrote:

The minification should happen before alignment, not after. See similar
logic on ComputeLODOffsetY. The current logic requires unnecessarily
large textures when there's an initial NPOT size.

Signed-off-by: Ilia Mirkin >
---
src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h 
b/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
index 11ed451..350e44b 100644
--- a/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
+++ b/src/gallium/drivers/swr/rasterizer/memory/TilingFunctions.h
@@ -284,8 +284,8 @@ INLINE void ComputeLODOffset1D(
offset = GFX_ALIGN(curWidth, hAlign);
for (uint32_t l = 1; l < lod; ++l)
{
-curWidth = GFX_ALIGN(std::max(curWidth >> 1, 1U), 
hAlign);
-offset += curWidth;
+curWidth = std::max(curWidth >> 1, 1U);
+offset += GFX_ALIGN(curWidth, hAlign);
}

if (info.isSubsampled || info.isBC)
--
2.7.3


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

Re: [Mesa-dev] [PATCH 0/7] anv: Add support for fast clears

[Jason Ekstrand]

Do you want to review input attachments too?  This series relies on it.

On Nov 21, 2016 9:55 AM, "Jordan Justen"  wrote:

> Series Reviewed-by: Jordan Justen 
>
> On 2016-11-19 11:40:02, Jason Ekstrand wrote:
> > This little series builds on top of the input attachment series I sent
> out
> > earlier this week and adds support for fast clears in Vulkan.  I've
> tested
> > it on both Sky Lake and Haswell and it has no regressions over the input
> > attachments series.
> >
> > Jason Ekstrand (7):
> >   anv/cmd_buffer: Make setup_attachments take a RenderPassBeginInfo
> >   anv/blorp: Add a vk_to_isl_color helper
> >   anv/blorp: Use regular blorp clears for subpass clears
> >   anv/cmd_buffer: Apply remaining flushes in EndCommandBuffer
> >   anv/blorp: Rework flushing around resolves
> >   anv: Add support for fast clears on gen9
> >   anv: Enable fast clears on gen7-8
> >
> >  src/intel/vulkan/anv_blorp.c   | 172 +-
> ---
> >  src/intel/vulkan/anv_image.c   |   2 +-
> >  src/intel/vulkan/anv_private.h |   3 +
> >  src/intel/vulkan/genX_cmd_buffer.c | 138 -
> >  4 files changed, 254 insertions(+), 61 deletions(-)
> >
> > --
> > 2.5.0.400.gff86faf
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/20] radeonsi: fix culling if clip & cull distances are used at the same time

[Marek Olšák]

On Mon, Nov 21, 2016 at 9:01 AM, Nicolai Hähnle  wrote:
> On 16.11.2016 19:38, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>>
>> Fixed piglits:
>> - arb_cull_distance/clip-cull-3
>> - arb_cull_distance/clip-cull-4
>
>
> Cc: stable for this and the previous patch?

It won't apply without some of the previous patches and it's unlikely
we'll ever see a shader with both Clip and Cull distances.

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

[Mesa-dev] [PATCH] docs/submitting patches: mention get_reviewers.pl

[Emil Velikov]

From: Emil Velikov 

Mention the script - why/how to use alongside a useful trick to make it
work interactively (thanks Rob!).

Signed-off-by: Emil Velikov 
---
Based on top of the "git tips" from earlier.
---
 docs/submittingpatches.html | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 7ada508..2f4be5d 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -104,6 +104,19 @@ that should be documented with:
 Reviewed-by: Joe Hacker jhac...@foo.com
 Acked-by: Joe Hacker jhac...@foo.com
 
+In order for your patch to reach the prospective reviewer easier/faster,
+use the script scripts/get_reviewer.pl to get a list of individuals and include
+them in the CC list.
+
+Please use common sense and do not blindly add everyone.
+
+
+$ scripts/get_reviewer.pl --help # to get the the help screen
+$ scripts/get_reviewer.pl -f src/egl/drivers/dri2/platform_android.c
+Rob Herring  (reviewer:ANDROID EGL 
SUPPORT,added_lines:188/700=27%,removed_lines:58/283=20%)
+Tomasz Figa  (reviewer:ANDROID EGL 
SUPPORT,authored:12/41=29%,added_lines:308/700=44%,removed_lines:115/283=41%)
+Emil Velikov  
(authored:13/41=32%,removed_lines:76/283=27%)
+
 
 
 
@@ -335,6 +348,11 @@ commits before sending.
 git send-email --subject-prefix="PATCH v4" HEAD~8
 git send-email -v4 HEAD~8 # shorter version, 'inherited' from git 
format-patch
 
+ Configure git to use the get_reviewer.pl script interactively. Thus you
+can avoid adding the world to the CC list.
+
+git config sendemail.cccmd "./scripts/get_reviewer.pl -i"
+
 
 
 
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH 00/20] RadeonSI: Optimized shader variants and other improvements

[Marek Olšák]

On Mon, Nov 21, 2016 at 9:32 AM, Nicolai Hähnle  wrote:
> On 16.11.2016 19:38, Marek Olšák wrote:
>>
>> Patches 1-10 are random improvements.
>>
>> Patches 11-14 are cleanups.
>>
>> Patches 15-20 add support for optimized shader variants compiled
>> asynchronously (without stalling rendering).
>>
>> The following optimizations are implemented for the shader variants:
>> - If user clipping is disabled, clip distances and the gl_ClipVertex
>>   code is removed from vertex shaders.
>> - All VS outputs not read by a PS are removed. All VS code that
>>   computes those including vertex attribute loads is removed
>>   automatically by dead-code elimination.
>> - If GL_RASTERIZER_DISCARD is set, all varyings are removed from
>>   vertex shaders.
>> - If doing depth-only rendering, it has the same behavior for
>>   vertex shaders as GL_RASTERIZER_DISCARD.
>>
>> It's all about optimizing vertex shaders and VS output resources.
>>
>> I've just realized I might have to limit the compilation of optimized
>> shader variants to only a limited number of threads, so that apps
>> aren't slowed down by this.
>>
>> I hope this is just the beginning of many things we can do with
>> asynchronous compilation.
>
>
> This is very cool stuff and the patches look good to me.
>
> However, for better testing please add debug flags 'noopt' and 'forceopt'
> (or something like that), where noopt disables all async optimized variants
> and forceopt always waits for the optimized variants, so we can run piglit
> with forceopt enabled for additional testing.

R600_DEBUG=mono works like forceopt - the compilation is done on
demand and no shader is compiled asynchronously.

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

Re: [Mesa-dev] [PATCH 0/7] anv: Add support for fast clears

[Jordan Justen]

Series Reviewed-by: Jordan Justen 

On 2016-11-19 11:40:02, Jason Ekstrand wrote:
> This little series builds on top of the input attachment series I sent out
> earlier this week and adds support for fast clears in Vulkan.  I've tested
> it on both Sky Lake and Haswell and it has no regressions over the input
> attachments series.
> 
> Jason Ekstrand (7):
>   anv/cmd_buffer: Make setup_attachments take a RenderPassBeginInfo
>   anv/blorp: Add a vk_to_isl_color helper
>   anv/blorp: Use regular blorp clears for subpass clears
>   anv/cmd_buffer: Apply remaining flushes in EndCommandBuffer
>   anv/blorp: Rework flushing around resolves
>   anv: Add support for fast clears on gen9
>   anv: Enable fast clears on gen7-8
> 
>  src/intel/vulkan/anv_blorp.c   | 172 
> +
>  src/intel/vulkan/anv_image.c   |   2 +-
>  src/intel/vulkan/anv_private.h |   3 +
>  src/intel/vulkan/genX_cmd_buffer.c | 138 -
>  4 files changed, 254 insertions(+), 61 deletions(-)
> 
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 5/5] swr: color interpolation is also supposed to get perspective division

[Ilia Mirkin]

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/swr/swr_shader.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index 428c9b3..294a568 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -457,7 +457,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key )
 
   // load/compute w
   Value *vw = nullptr, *pAttribs;
-  if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE) {
+  if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE ||
+  interpMode == TGSI_INTERPOLATE_COLOR) {
  pAttribs = pPerspAttribs;
  switch (interpLoc) {
  case TGSI_INTERPOLATE_LOC_CENTER:
@@ -596,7 +597,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key )
Value *interp1 = FMUL(vb, vj);
interp = FADD(interp, interp1);
interp = FADD(interp, vc);
-   if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE)
+   if (interpMode == TGSI_INTERPOLATE_PERSPECTIVE ||
+   interpMode == TGSI_INTERPOLATE_COLOR)
   interp = FMUL(interp, vw);
inputs[attrib][channel] = wrap(interp);
 }
-- 
2.7.3

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

[Mesa-dev] [PATCH 4/5] swr: add sprite coord enable mask to fs key

[Ilia Mirkin]

This fixes gl-coord-replace-doesnt-eliminate-frag-tex-coords

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/swr/swr_shader.cpp | 3 ++-
 src/gallium/drivers/swr/swr_shader.h   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index d29f635..428c9b3 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -131,6 +131,7 @@ swr_generate_fs_key(struct swr_jit_fs_key ,
 
key.nr_cbufs = ctx->framebuffer.nr_cbufs;
key.light_twoside = ctx->rasterizer->light_twoside;
+   key.sprite_coord_enable = ctx->rasterizer->sprite_coord_enable;
memcpy(_output_semantic_name,
   >vs->info.base.output_semantic_name,
   sizeof(key.vs_output_semantic_name));
@@ -515,7 +516,7 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key )
   unsigned linkedAttrib =
  locate_linkage(semantic_name, semantic_idx, >vs->info.base);
   if (semantic_name == TGSI_SEMANTIC_GENERIC &&
-  ctx->rasterizer->sprite_coord_enable & (1 << semantic_idx)) {
+  key.sprite_coord_enable & (1 << semantic_idx)) {
  /* we add an extra attrib to the backendState in swr_update_derived. 
*/
  linkedAttrib = ctx->vs->info.base.num_outputs - 1;
  swr_fs->pointSpriteMask |= (1 << linkedAttrib);
diff --git a/src/gallium/drivers/swr/swr_shader.h 
b/src/gallium/drivers/swr/swr_shader.h
index ccdda44..7e3399c 100644
--- a/src/gallium/drivers/swr/swr_shader.h
+++ b/src/gallium/drivers/swr/swr_shader.h
@@ -51,6 +51,7 @@ struct swr_jit_sampler_key {
 struct swr_jit_fs_key : swr_jit_sampler_key {
unsigned nr_cbufs;
unsigned light_twoside;
+   unsigned sprite_coord_enable;
ubyte vs_output_semantic_name[PIPE_MAX_SHADER_OUTPUTS];
ubyte vs_output_semantic_idx[PIPE_MAX_SHADER_OUTPUTS];
 };
-- 
2.7.3

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

[Mesa-dev] [PATCH 2/5] swr: flatshading makes color outputs flat, it doesn't affect others

[Ilia Mirkin]

We were previously not marking the "regular" flat outputs as flat when
flatshading was enabled.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/swr/swr_state.cpp | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index dcbe434..8541aca 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1490,10 +1490,8 @@ swr_update_derived(struct pipe_context *pipe,
   (ctx->rasterizer->sprite_coord_enable ? 1 : 0);
for (unsigned i = 0; i < backendState.numAttributes; i++)
   backendState.numComponents[i] = 4;
-   backendState.constantInterpolationMask =
-  ctx->rasterizer->flatshade ?
-  ctx->fs->flatConstantMask :
-  ctx->fs->constantMask;
+   backendState.constantInterpolationMask = ctx->fs->constantMask |
+  (ctx->rasterizer->flatshade ? ctx->fs->flatConstantMask : 0);
backendState.pointSpriteTexCoordMask = ctx->fs->pointSpriteMask;
 
SwrSetBackendState(ctx->swrContext, );
-- 
2.7.3

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

[Mesa-dev] [PATCH 3/5] swr: rework vert <-> frag shader linkage logic

[Ilia Mirkin]

Fixes a few things:
 - sprite coords only apply to generic varyings, and are a bitmask
 - back color only applies in 2-sided lighting mode
 - handle some odd situations between only some front/back colors being
   there. This is only semi-legal in GL, but we shouldn't start
   crashing.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/swr/swr_shader.cpp | 93 ++
 1 file changed, 50 insertions(+), 43 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index 2f72239..d29f635 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -372,15 +372,6 @@ locate_linkage(ubyte name, ubyte index, struct 
tgsi_shader_info *info)
   }
}
 
-   if (name == TGSI_SEMANTIC_COLOR) { // BCOLOR fallback
-  for (int i = 0; i < PIPE_MAX_SHADER_OUTPUTS; i++) {
- if ((info->output_semantic_name[i] == TGSI_SEMANTIC_BCOLOR)
- && (info->output_semantic_index[i] == index)) {
-return i - 1; // position is not part of the linkage
- }
-  }
-   }
-
return 0x;
 }
 
@@ -523,54 +514,70 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key )
 
   unsigned linkedAttrib =
  locate_linkage(semantic_name, semantic_idx, >vs->info.base);
-  if (linkedAttrib == 0x) {
- // not found - check for point sprite
- if (ctx->rasterizer->sprite_coord_enable) {
-linkedAttrib = ctx->vs->info.base.num_outputs - 1;
-swr_fs->pointSpriteMask |= (1 << linkedAttrib);
- } else {
-fprintf(stderr,
-"Missing %s[%d]\n",
-tgsi_semantic_names[semantic_name],
-semantic_idx);
-assert(0 && "attribute linkage not found");
+  if (semantic_name == TGSI_SEMANTIC_GENERIC &&
+  ctx->rasterizer->sprite_coord_enable & (1 << semantic_idx)) {
+ /* we add an extra attrib to the backendState in swr_update_derived. 
*/
+ linkedAttrib = ctx->vs->info.base.num_outputs - 1;
+ swr_fs->pointSpriteMask |= (1 << linkedAttrib);
+  } else if (linkedAttrib == 0x) {
+ inputs[attrib][0] = wrap(VIMMED1(0.0f));
+ inputs[attrib][1] = wrap(VIMMED1(0.0f));
+ inputs[attrib][2] = wrap(VIMMED1(0.0f));
+ inputs[attrib][3] = wrap(VIMMED1(1.0f));
+ /* If we're reading in color and 2-sided lighting is enabled, we have
+  * to keep going.
+  */
+ if (semantic_name != TGSI_SEMANTIC_COLOR || !key.light_twoside)
+continue;
+  } else {
+ if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
+swr_fs->constantMask |= 1 << linkedAttrib;
+ } else if (interpMode == TGSI_INTERPOLATE_COLOR) {
+swr_fs->flatConstantMask |= 1 << linkedAttrib;
  }
   }
 
-  if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
- swr_fs->constantMask |= 1 << linkedAttrib;
-  } else if (interpMode == TGSI_INTERPOLATE_COLOR) {
- swr_fs->flatConstantMask |= 1 << linkedAttrib;
-  }
-
-  for (int channel = 0; channel < TGSI_NUM_CHANNELS; channel++) {
- if (mask & (1 << channel)) {
-Value *indexA = C(linkedAttrib * 12 + channel);
-Value *indexB = C(linkedAttrib * 12 + channel + 4);
-Value *indexC = C(linkedAttrib * 12 + channel + 8);
+  unsigned bcolorAttrib = 0x;
+  Value *offset = NULL;
+  if (semantic_name == TGSI_SEMANTIC_COLOR && key.light_twoside) {
+ bcolorAttrib = locate_linkage(
+   TGSI_SEMANTIC_BCOLOR, semantic_idx, >vs->info.base);
+ /* Neither front nor back colors were available. Nothing to load. */
+ if (bcolorAttrib == 0x && linkedAttrib == 0x)
+continue;
+ /* If there is no front color, just always use the back color. */
+ if (linkedAttrib == 0x)
+linkedAttrib = bcolorAttrib;
 
-if ((semantic_name == TGSI_SEMANTIC_COLOR)
-&& ctx->rasterizer->light_twoside) {
-   unsigned bcolorAttrib = locate_linkage(
-  TGSI_SEMANTIC_BCOLOR, semantic_idx, >vs->info.base);
+ if (bcolorAttrib != 0x) {
+if (interpMode == TGSI_INTERPOLATE_CONSTANT) {
+   swr_fs->constantMask |= 1 << bcolorAttrib;
+} else if (interpMode == TGSI_INTERPOLATE_COLOR) {
+   swr_fs->flatConstantMask |= 1 << bcolorAttrib;
+}
 
-   unsigned diff = 12 * (bcolorAttrib - linkedAttrib);
+unsigned diff = 12 * (bcolorAttrib - linkedAttrib);
 
+if (diff) {
Value *back =
   XOR(C(1), LOAD(pPS, {0, SWR_PS_CONTEXT_frontFace}), 
"backFace");
 
-   Value *offset = MUL(back, C(diff));
+   offset = MUL(back, C(diff));
   

[Mesa-dev] [PATCH 1/5] swr: only broadcast color0 value, not all color values

[Ilia Mirkin]

The way that dual-source blending is described for GLES2 is very odd,
and we end up with a shader that both has this property set *and* has a
color1 value to be used as the second source. While changing the state
tracker is an option, it seems more reliable to verify that the
broadcast is only done on color0.

Fixes arb_blend_func_extended-fbo-extended-blend-pattern_gles2

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/swr/swr_shader.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/swr/swr_shader.cpp 
b/src/gallium/drivers/swr/swr_shader.cpp
index e4f9796..2f72239 100644
--- a/src/gallium/drivers/swr/swr_shader.cpp
+++ b/src/gallium/drivers/swr/swr_shader.cpp
@@ -645,7 +645,8 @@ BuilderSWR::CompileFS(struct swr_context *ctx, 
swr_jit_fs_key )
 
 LLVMValueRef out =
LLVMBuildLoad(gallivm->builder, outputs[attrib][channel], "");
-if 
(swr_fs->info.base.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS]) {
+if 
(swr_fs->info.base.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS] &&
+swr_fs->info.base.output_semantic_index[attrib] == 0) {
for (uint32_t rt = 0; rt < key.nr_cbufs; rt++) {
   STORE(unwrap(out),
 pPS,
-- 
2.7.3

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

Re: [Mesa-dev] [PATCH 6/7] anv: Add support for fast clears on gen9

[Jordan Justen]

On 2016-11-19 11:40:08, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/anv_blorp.c   | 102 
> -
>  src/intel/vulkan/anv_private.h |   3 ++
>  src/intel/vulkan/genX_cmd_buffer.c | 100 ++--
>  3 files changed, 176 insertions(+), 29 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 24b98ab..cab1906 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1178,16 +1178,35 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
> *cmd_buffer)
>struct blorp_surf surf;
>get_blorp_surf_for_anv_image(image, VK_IMAGE_ASPECT_COLOR_BIT,
> att_state->aux_usage, );
> +  surf.clear_color = vk_to_isl_color(att_state->clear_value.color);
>  
>const VkRect2D render_area = cmd_buffer->state.render_area;
>  
> -  blorp_clear(, , iview->isl.format, iview->isl.swizzle,
> -  iview->isl.base_level,
> -  iview->isl.base_array_layer, fb->layers,
> -  render_area.offset.x, render_area.offset.y,
> -  render_area.offset.x + render_area.extent.width,
> -  render_area.offset.y + render_area.extent.height,
> -  vk_to_isl_color(att_state->clear_value.color), NULL);
> +  if (att_state->fast_clear) {
> + blorp_fast_clear(, , iview->isl.format,
> +  iview->isl.base_level,
> +  iview->isl.base_array_layer, fb->layers,
> +  render_area.offset.x, render_area.offset.y,
> +  render_area.offset.x + render_area.extent.width,
> +  render_area.offset.y + render_area.extent.height);
> +
> + /* From the Sky Lake PRM Vol. 7, "Render Target Fast Clear":
> +  *
> +  *"After Render target fast clear, pipe-control with color cache
> +  *write-flush must be issued before sending any DRAW commands on
> +  *that render target."
> +  */
> + cmd_buffer->state.pending_pipe_bits |=
> +ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
> +  } else {
> + blorp_clear(, , iview->isl.format, iview->isl.swizzle,
> + iview->isl.base_level,
> + iview->isl.base_array_layer, fb->layers,
> + render_area.offset.x, render_area.offset.y,
> + render_area.offset.x + render_area.extent.width,
> + render_area.offset.y + render_area.extent.height,
> + surf.clear_color, NULL);
> +  }
>  
>att_state->pending_clear_aspects = 0;
> }
> @@ -1298,10 +1317,12 @@ ccs_resolve_attachment(struct anv_cmd_buffer 
> *cmd_buffer,
> struct anv_attachment_state *att_state =
>_buffer->state.attachments[att];
>  
> -   assert(att_state->aux_usage != ISL_AUX_USAGE_CCS_D);
> -   if (att_state->aux_usage != ISL_AUX_USAGE_CCS_E)
> +   if (att_state->aux_usage == ISL_AUX_USAGE_NONE)
>return; /* Nothing to resolve */
>  
> +   assert(att_state->aux_usage == ISL_AUX_USAGE_CCS_E ||
> +  att_state->aux_usage == ISL_AUX_USAGE_CCS_D);
> +
> struct anv_render_pass *pass = cmd_buffer->state.pass;
> struct anv_subpass *subpass = cmd_buffer->state.subpass;
> unsigned subpass_idx = subpass - pass->subpasses;
> @@ -1312,14 +1333,17 @@ ccs_resolve_attachment(struct anv_cmd_buffer 
> *cmd_buffer,
>  * of a particular attachment.  That way we only resolve once but it's
>  * still hot in the cache.
>  */
> +   bool found_draw = false;
> +   enum anv_subpass_usage usage = 0;
> for (uint32_t s = subpass_idx + 1; s < pass->subpass_count; s++) {
> -  enum anv_subpass_usage usage = pass->attachments[att].subpass_usage[s];
> +  usage |= pass->attachments[att].subpass_usage[s];
>  
>if (usage & (ANV_SUBPASS_USAGE_DRAW | ANV_SUBPASS_USAGE_RESOLVE_DST)) {
>   /* We found another subpass that draws to this attachment.  We'll
>* wait to resolve until then.
>*/
> - return;
> + found_draw = true;
> + break;
>}
> }
>  
> @@ -1327,12 +1351,60 @@ ccs_resolve_attachment(struct anv_cmd_buffer 
> *cmd_buffer,
> const struct anv_image *image = iview->image;
> assert(image->aspects == VK_IMAGE_ASPECT_COLOR_BIT);
>  
> -   if (image->aux_usage == ISL_AUX_USAGE_CCS_E)
> +   enum blorp_fast_clear_op resolve_op = BLORP_FAST_CLEAR_OP_NONE;
> +   if (!found_draw) {
> +  /* This is the last subpass that writes to this attachment so we need 
> to
> +   * resolve here.  Ideally, we would like to only resolve if the storeOp
> +   * is set to VK_ATTACHMENT_STORE_OP_STORE.  However, we need to ensure
> +   * that the CCS bits are set to "resolved" because there may be copy or
> +   * blit operations (which may 

[Mesa-dev] [PATCH 6/7] anv: descriptorSet: write sampler border colors on update

[Lionel Landwerlin]

When writing texture or sampler information into descriptor sets, also store
the related information for border color workaround on Gen7.5.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/anv_descriptor_set.c | 60 ++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 7b5efc2..1100ae7 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -640,6 +640,21 @@ VkResult anv_FreeDescriptorSets(
return VK_SUCCESS;
 }
 
+static void
+border_color_mask_from_isl_view (uint32_t *mask,
+ uint32_t *alpha_overwrite,
+ const struct isl_view *view) {
+   const struct isl_format_layout *layout = _format_layouts[view->format];
+
+   mask[0] = layout->channels.r.bits > 0 ? 0x : 0;
+   mask[1] = layout->channels.g.bits > 0 ? 0x : 0;
+   mask[2] = layout->channels.b.bits > 0 ? 0x : 0;
+   mask[3] = 0x;
+
+   *alpha_overwrite = (layout->channels.a.bits == 0 ||
+   view->swizzle.a == ISL_CHANNEL_SELECT_ONE) ? 1 : 0;
+}
+
 void anv_UpdateDescriptorSets(
 VkDevice_device,
 uint32_tdescriptorWriteCount,
@@ -656,10 +671,12 @@ void anv_UpdateDescriptorSets(
  >layout->binding[write->dstBinding];
   struct anv_descriptor *desc =
  >descriptors[bind_layout->descriptor_index];
+  int16_t border_color_index = bind_layout->border_color_array_index;
   desc += write->dstArrayElement;
 
   assert(write->descriptorType == bind_layout->type);
 
+  bool sampler_write = false;
   switch (write->descriptorType) {
   case VK_DESCRIPTOR_TYPE_SAMPLER:
  for (uint32_t j = 0; j < write->descriptorCount; j++) {
@@ -670,6 +687,12 @@ void anv_UpdateDescriptorSets(
.type = VK_DESCRIPTOR_TYPE_SAMPLER,
.sampler = sampler,
 };
+
+set->gen7.border_colors[border_color_index].wrapping =
+   sampler->wrapping;
+memcpy(set->gen7.border_colors[border_color_index].color,
+   sampler->color, sizeof(sampler->color));
+sampler_write = true;
  }
  break;
 
@@ -683,11 +706,23 @@ void anv_UpdateDescriptorSets(
 desc[j].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
 desc[j].image_view = iview;
 
+border_color_mask_from_isl_view(
+   set->gen7.border_colors[border_color_index].color_mask,
+   >gen7.border_colors[border_color_index].alpha_overwrite,
+   >isl);
+
 /* If this descriptor has an immutable sampler, we don't want
  * to stomp on it.
  */
-if (sampler)
+if (sampler) {
desc[j].sampler = sampler;
+
+   set->gen7.border_colors[border_color_index].wrapping =
+  sampler->wrapping;
+   memcpy(set->gen7.border_colors[border_color_index].color,
+  sampler->color, sizeof(sampler->color));
+}
+sampler_write = true;
  }
  break;
 
@@ -763,6 +798,29 @@ void anv_UpdateDescriptorSets(
   default:
  break;
   }
+
+  if (sampler_write && set->gen7.border_colors_state.alloc_size > 0) {
+ anv_state_clflush(set->gen7.border_colors_state);
+ const struct anv_descriptor_set_binding_layout *ubo_bind_layout =
+>layout->binding[set->layout->border_color_index];
+ struct anv_buffer_view *view =
+>buffer_views[ubo_bind_layout->buffer_index];
+
+ view->format =
+
anv_isl_format_for_descriptor_type(VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER);
+ view->bo = >dynamic_state_pool.block_pool->bo;
+ view->offset = set->gen7.border_colors_state.offset;
+ view->range = set->gen7.border_colors_state.alloc_size;
+
+ anv_fill_buffer_surface_state(device, view->surface_state,
+   view->format,
+   view->offset, view->range, 1);
+
+ desc[set->layout->border_color_index] = (struct anv_descriptor) {
+.type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER,
+.buffer_view = view,
+ };
+  }
}
 
for (uint32_t i = 0; i < descriptorCopyCount; i++) {
-- 
2.10.2

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

[Mesa-dev] [PATCH 2/7] anv: sampler: store border color & wrapping modes on Gen7.5

[Lionel Landwerlin]

---
 src/intel/vulkan/anv_private.h |  4 +++-
 src/intel/vulkan/genX_state.c  | 21 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 7d7380a..8231f6b 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -1688,7 +1688,9 @@ void anv_buffer_view_fill_image_param(struct anv_device 
*device,
   struct brw_image_param *param);
 
 struct anv_sampler {
-   uint32_t state[4];
+   uint32_t state[4];
+   uint32_t color[4];
+   uint32_t wrapping;
 };
 
 struct anv_framebuffer {
diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index 4122395..5249706 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -166,6 +166,27 @@ VkResult genX(CreateSampler)(
 
uint32_t border_color_offset = device->border_colors.offset +
   pCreateInfo->borderColor * 64;
+#if GEN_IS_HASWELL
+   if (pCreateInfo->borderColor == VK_BORDER_COLOR_INT_TRANSPARENT_BLACK ||
+   pCreateInfo->borderColor == VK_BORDER_COLOR_INT_OPAQUE_BLACK ||
+   pCreateInfo->borderColor == VK_BORDER_COLOR_INT_OPAQUE_WHITE) {
+  sampler->wrapping =
+ (pCreateInfo->addressModeU == VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER 
?
+  0x1 : 0) |
+ (pCreateInfo->addressModeV == VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER 
?
+  0x2 : 0) |
+ (pCreateInfo->addressModeW == VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER 
?
+  0x4 : 0);
+
+  static const uint32_t border_colors[][4] = {
+ [VK_BORDER_COLOR_INT_TRANSPARENT_BLACK] = { 0, 0, 0, 0 },
+ [VK_BORDER_COLOR_INT_OPAQUE_BLACK]  = { 0, 0, 0, 1 },
+ [VK_BORDER_COLOR_INT_OPAQUE_WHITE]  = { 1, 1, 1, 1 }
+  };
+  memcpy(sampler->color, border_colors[pCreateInfo->borderColor],
+ sizeof(sampler->color));
+   }
+#endif
 
bool enable_min_filter_addr_rounding =
   pCreateInfo->minFilter != VK_FILTER_NEAREST;
-- 
2.10.2

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

[Mesa-dev] [PATCH 5/7] anv: descriptorSet: allocate hidden ubo for border colors

[Lionel Landwerlin]

This additional buffer stores workaround border colors & mask based of the
format's number of channels for integer formats on Gen7.5.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/anv_descriptor_set.c | 16 
 src/intel/vulkan/anv_private.h| 13 +
 2 files changed, 29 insertions(+)

diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 30aa50e..7b5efc2 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -500,6 +500,7 @@ anv_descriptor_set_create(struct anv_device *device,
set->buffer_views =
   (struct anv_buffer_view *) >descriptors[layout->size];
set->buffer_count = layout->buffer_count;
+   memset(>gen7, 0, sizeof(set->gen7));
 
/* By defining the descriptors to be zero now, we can later verify that
 * a descriptor has not been populated with user data.
@@ -542,6 +543,15 @@ anv_descriptor_set_create(struct anv_device *device,
   set->buffer_views[b].surface_state = state;
}
 
+   /* Allocate border color workaround buffer if needed. */
+   if (layout->border_color_index > 0) {
+  set->gen7.border_colors_state =
+ anv_state_pool_alloc(>dynamic_state_pool,
+  sizeof(*set->gen7.border_colors) *
+  layout->border_color_count, 64);
+  set->gen7.border_colors = set->gen7.border_colors_state.map;
+   }
+
*out_set = set;
 
return VK_SUCCESS;
@@ -552,6 +562,12 @@ anv_descriptor_set_destroy(struct anv_device *device,
struct anv_descriptor_pool *pool,
struct anv_descriptor_set *set)
 {
+   /* Free border color workaround buffer if it was allocated. */
+   if (set->gen7.border_colors_state.alloc_size > 0) {
+  anv_state_pool_free(>dynamic_state_pool,
+  set->gen7.border_colors_state);
+   }
+
/* Put the buffer view surface state back on the free list. */
for (uint32_t b = 0; b < set->buffer_count; b++) {
   struct surface_state_free_list_entry *entry =
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 19b08b3..d3bbd97 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -878,8 +878,21 @@ struct anv_descriptor {
};
 };
 
+struct gen7_border_color {
+   uint32_t color[4];
+   uint32_t color_mask[4];
+   uint32_t alpha_overwrite;
+   uint32_t wrapping;
+};
+
 struct anv_descriptor_set {
const struct anv_descriptor_set_layout *layout;
+
+   struct {
+  struct anv_state border_colors_state;
+  struct gen7_border_color *border_colors;
+   } gen7;
+
uint32_t size;
uint32_t buffer_count;
struct anv_buffer_view *buffer_views;
-- 
2.10.2

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

[Mesa-dev] [PATCH 1/7] anv: Fix border colors with normalized formats on gen7/gen75

[Lionel Landwerlin]

This replicates the following commit from our GL driver :

commit c25e5300cba7628b58df93ead14ebc3cc32f338c
Author: Kenneth Graunke 
Date:   Fri Jan 20 03:33:40 2012 -0800

i965: Fix border color on Sandybridge and Ivybridge.

On Haswell, it fixes:

  
dEQP-VK.pipeline.sampler.view_type.*.format.*_unorm.address_modes.all_mode_clamp_to_border_*

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/genX_cmd_buffer.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index fff0ba0..69c1ce9 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -106,6 +106,15 @@ genX(cmd_buffer_emit_state_base_address)(struct 
anv_cmd_buffer *cmd_buffer)
   sba.IndirectObjectBufferSizeModifyEnable  = true;
   sba.InstructionBufferSize = 0xf;
   sba.InstructionBuffersizeModifyEnable = true;
+#  else
+  /* Dynamic state upper bound. Although the documentation says that
+   * programming it to zero will cause it to be ignored, that is a lie. If
+   * this isn't programmed to a real bound, the sampler border color
+   * pointer is rejected, causing border color to mysteriously fail.
+   */
+  sba.DynamicStateAccessUpperBound =
+ (struct anv_address) { NULL, 0xf000 };
+  sba.DynamicStateAccessUpperBoundModifyEnable = true;
 #  endif
}
 
-- 
2.10.2

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

[Mesa-dev] [PATCH 3/7] anv: descriptorSetLayout: allocate additional space for a hidden ubo

[Lionel Landwerlin]

This additional buffer will store workaround border colors for integer
formats on Gen7.5.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/anv_descriptor_set.c | 42 +++
 src/intel/vulkan/anv_private.h|  9 
 2 files changed, 51 insertions(+)

diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 578bf7b..82f2d1e 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -48,12 +48,33 @@ VkResult anv_CreateDescriptorSetLayout(
 
uint32_t max_binding = 0;
uint32_t immutable_sampler_count = 0;
+   uint32_t border_colors_wa = 0;
for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) {
   max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding);
   if (pCreateInfo->pBindings[j].pImmutableSamplers)
  immutable_sampler_count += pCreateInfo->pBindings[j].descriptorCount;
+
+
+  /* We need an additional buffer slot on Ivybridge/Haswell to store
+   * integer border color values. Count how many entries we need in that
+   * buffer. */
+  const VkDescriptorSetLayoutBinding *binding = >pBindings[j];
+  switch (binding->descriptorType) {
+  case VK_DESCRIPTOR_TYPE_SAMPLER:
+  case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
+ border_colors_wa += binding->descriptorCount;
+ break;
+
+  default:
+ continue;
+  }
}
 
+   if (device->info.is_haswell && border_colors_wa > 0)
+  max_binding++;
+   else
+  border_colors_wa = 0;
+
size_t size = sizeof(struct anv_descriptor_set_layout) +
  (max_binding + 1) * sizeof(set_layout->binding[0]) +
  immutable_sampler_count * sizeof(struct anv_sampler *);
@@ -69,6 +90,7 @@ VkResult anv_CreateDescriptorSetLayout(
 
memset(set_layout, 0, sizeof(*set_layout));
set_layout->binding_count = max_binding + 1;
+   set_layout->border_color_index = -1;
 
for (uint32_t b = 0; b <= max_binding; b++) {
   /* Initialize all binding_layout entries to -1 */
@@ -119,6 +141,9 @@ VkResult anv_CreateDescriptorSetLayout(
 set_layout->binding[b].stage[s].sampler_index = sampler_count[s];
 sampler_count[s] += binding->descriptorCount;
  }
+ set_layout->binding[b].border_color_array_index =
+set_layout->border_color_count;
+ set_layout->border_color_count += binding->descriptorCount;
  break;
   default:
  break;
@@ -184,6 +209,23 @@ VkResult anv_CreateDescriptorSetLayout(
   set_layout->shader_stages |= binding->stageFlags;
}
 
+   if (border_colors_wa > 0) {
+  uint32_t index = max_binding;
+  set_layout->border_color_index = index;
+  set_layout->binding[index].buffer_index = buffer_count;
+#ifndef NDEBUG
+  set_layout->binding[index].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER;
+#endif
+  set_layout->binding[index].array_size = 1;
+  set_layout->binding[index].descriptor_index = set_layout->size;
+  for (int s = 0; s < ARRAY_SIZE(set_layout->binding[index].stage); s++) {
+ set_layout->binding[index].stage[s].surface_index = surface_count[s];
+ surface_count[s]++;
+  }
+  set_layout->size++;
+  buffer_count++;
+   }
+
set_layout->buffer_count = buffer_count;
set_layout->dynamic_offset_count = dynamic_offset_count;
 
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 8231f6b..19b08b3 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -821,6 +821,9 @@ struct anv_descriptor_set_binding_layout {
/* Index into the descriptor set buffer views */
int16_t buffer_index;
 
+   /* Index into the border color array */
+   int16_t border_color_array_index;
+
struct {
   /* Index into the binding table for the associated surface */
   int16_t surface_index;
@@ -852,6 +855,12 @@ struct anv_descriptor_set_layout {
/* Number of dynamic offsets used by this descriptor set */
uint16_t dynamic_offset_count;
 
+   /* Number of samplers used by this descriptor set */
+   uint16_t border_color_count;
+
+   /* Index of the border color buffer into the descriptor set buffer views */
+   int16_t border_color_index;
+
/* Bindings in this descriptor set */
struct anv_descriptor_set_binding_layout binding[0];
 };
-- 
2.10.2

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

[Mesa-dev] [PATCH 7/7] anv: add nir pass to insert border color workaround

[Lionel Landwerlin]

This pass wraps sampler instructions with some logic to deal with border
colors with integer formats on Gen7.5.

Fixes most of :

  
dEQP-VK.pipeline.sampler.view_type.*.format.*int.address_modes.all_mode_clamp_to_border_*

Remaining 11 failures :
  HSW:
   
dEQP-VK.pipeline.sampler.view_type.*.format.b4g4r4a4_unorm_pack16.address_modes.all_mode_clamp_to_border_opaque_black
   
dEQP-VK.pipeline.sampler.view_type.2d.format.r32_uint.address_modes.all_mode_clamp_to_border_*
   
dEQP-VK.pipeline.sampler.view_type.2d.format.r32g32b32_uint.address_modes.all_mode_clamp_to_border_*

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/Makefile.sources   |   1 +
 src/intel/vulkan/anv_nir.h  |   3 +
 src/intel/vulkan/anv_nir_gen7_border_color_wa.c | 208 
 src/intel/vulkan/anv_pipeline.c |   3 +
 4 files changed, 215 insertions(+)
 create mode 100644 src/intel/vulkan/anv_nir_gen7_border_color_wa.c

diff --git a/src/intel/vulkan/Makefile.sources 
b/src/intel/vulkan/Makefile.sources
index d81bc03..6361356 100644
--- a/src/intel/vulkan/Makefile.sources
+++ b/src/intel/vulkan/Makefile.sources
@@ -34,6 +34,7 @@ VULKAN_FILES := \
anv_nir.h \
anv_nir_apply_dynamic_offsets.c \
anv_nir_apply_pipeline_layout.c \
+   anv_nir_gen7_border_color_wa.c \
anv_nir_lower_push_constants.c \
anv_pass.c \
anv_pipeline.c \
diff --git a/src/intel/vulkan/anv_nir.h b/src/intel/vulkan/anv_nir.h
index 4fff7f4..649b5ed 100644
--- a/src/intel/vulkan/anv_nir.h
+++ b/src/intel/vulkan/anv_nir.h
@@ -40,6 +40,9 @@ void anv_nir_apply_pipeline_layout(struct anv_pipeline 
*pipeline,
nir_shader *shader,
struct brw_stage_prog_data *prog_data,
struct anv_pipeline_bind_map *map);
+void anv_nir_apply_gen7_border_color(struct anv_pipeline *pipeline,
+ nir_shader *shader,
+ struct brw_stage_prog_data *prog_data);
 
 #ifdef __cplusplus
 }
diff --git a/src/intel/vulkan/anv_nir_gen7_border_color_wa.c 
b/src/intel/vulkan/anv_nir_gen7_border_color_wa.c
new file mode 100644
index 000..d65f70f
--- /dev/null
+++ b/src/intel/vulkan/anv_nir_gen7_border_color_wa.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright © 2016 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.
+ */
+
+/*
+ * Implements a pass replaces integer sampler accesses with some logic to work
+ * around broken border colors with integer formats on Ivybridge and Haswell.
+ */
+
+#include "anv_nir.h"
+#include "nir/nir_builder.h"
+#include "nir/nir_opcodes.h"
+
+static void
+wrap_tex_instr(nir_builder *b,
+   nir_tex_instr *tex_instr,
+   nir_variable *sampler,
+   const struct anv_descriptor_set_layout *set_layout)
+{
+   nir_ssa_def *zeroi = nir_imm_int(b, 0);
+   nir_intrinsic_instr *vk_instr =
+  nir_intrinsic_instr_create(b->shader,
+ nir_intrinsic_vulkan_resource_index);
+   vk_instr->num_components = 1;
+   vk_instr->src[0] = nir_src_for_ssa(zeroi); /* array index */
+   nir_intrinsic_set_desc_set(vk_instr, 0);
+   nir_intrinsic_set_binding(vk_instr, set_layout->border_color_index);
+   nir_ssa_dest_init(_instr->instr, _instr->dest, 1, 32, NULL);
+   nir_builder_instr_insert(b, _instr->instr);
+
+   nir_intrinsic_instr *load_wrapping_instr =
+  nir_intrinsic_instr_create(b->shader, nir_intrinsic_load_ubo);
+   load_wrapping_instr->num_components = 1;
+   load_wrapping_instr->src[0] = nir_src_for_ssa(_instr->dest.ssa);
+   load_wrapping_instr->src[1] = /* ubo offset */
+  nir_src_for_ssa(nir_imm_int(b, offsetof(struct gen7_border_color,
+  wrapping)));
+   

[Mesa-dev] [PATCH 0/7] Anv: Border color fixes for Haswell

[Lionel Landwerlin]

Hi,

This is another attempt at fixing the border colors on Haswell. Patch 1 is
probably a genuine fix, the rest implements a workaround for the integer
formats.

The idea is the following, store the border colors and wrapping information
for all the samplers as well as some additional information about the
texture format last bound to a sampler into a UBO. When a shader tries to
access a sampler with integer format, do some bounds checking and load the
border color from the UBO if needed.

Thanks,

Lionel Landwerlin (7):
  anv: Fix border colors with normalized formats on gen7/gen75
  anv: sampler: store border color & wrapping modes on Gen7.5
  anv: descriptorSetLayout: allocate additional space for a hidden ubo
  anv: descriptorPool: allocate additional space for a hidden ubo
  anv: descriptorSet: allocate hidden ubo for border colors
  anv: descriptorSet: write sampler border colors on update
  anv: add nir pass to insert border color workaround

 src/intel/vulkan/Makefile.sources   |   1 +
 src/intel/vulkan/anv_descriptor_set.c   | 136 +++-
 src/intel/vulkan/anv_nir.h  |   3 +
 src/intel/vulkan/anv_nir_gen7_border_color_wa.c | 208 
 src/intel/vulkan/anv_pipeline.c |   3 +
 src/intel/vulkan/anv_private.h  |  26 ++-
 src/intel/vulkan/genX_cmd_buffer.c  |   9 +
 src/intel/vulkan/genX_state.c   |  21 +++
 8 files changed, 405 insertions(+), 2 deletions(-)
 create mode 100644 src/intel/vulkan/anv_nir_gen7_border_color_wa.c

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

[Mesa-dev] [PATCH 4/7] anv: descriptorPool: allocate additional space for a hidden ubo

[Lionel Landwerlin]

This additional buffer will store workaround border colors for integer
formats on Gen7.5.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/anv_descriptor_set.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 82f2d1e..30aa50e 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -371,6 +371,24 @@ VkResult anv_CreateDescriptorPool(
   }
}
 
+   if (device->info.is_haswell) {
+  uint32_t sampler_count = 0;
+  for (uint32_t i = 0; i < pCreateInfo->poolSizeCount; i++) {
+ switch (pCreateInfo->pPoolSizes[i].type) {
+ case VK_DESCRIPTOR_TYPE_SAMPLER:
+ case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
+sampler_count += pCreateInfo->pPoolSizes[i].descriptorCount;
+break;
+ default:
+break;
+ }
+  }
+  if (sampler_count > 0) {
+ buffer_count++;
+ descriptor_count++;
+  }
+   }
+
const size_t size =
   sizeof(*pool) +
   pCreateInfo->maxSets * sizeof(struct anv_descriptor_set) +
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH] docs/submitting patches: add git tips

[Emil Velikov]

On 21 November 2016 at 17:21, Nicolai Hähnle  wrote:
> On 21.11.2016 17:37, Emil Velikov wrote:
>>
>> From: Timothy Arceri 
>>
>> v2: [Emil Velikov]
>>  - Add the shorthand git send-email -vX
>>  - Move to submittingpatches.html
>>  - Add to the TOC.
>>
>> Cc: Timothy Arceri 
>> Signed-off-by: Emil Velikov 
>> Reviewed-by: Brian Paul  (v1)
>> ---
>> Tim any objections on merging this ? IMHO these are quite useful.
>> ---
>>  docs/submittingpatches.html | 20 
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
>> index d3e86f5..7ada508 100644
>> --- a/docs/submittingpatches.html
>> +++ b/docs/submittingpatches.html
>> @@ -25,6 +25,7 @@
>>  Reviewing Patches
>>  Nominating a commit for a stable branch
>>  Criteria for accepting patches to the stable
>> branch
>> +Git tips
>>  
>>
>>  Basic guidelines
>> @@ -317,6 +318,25 @@ be rejected:
>>regression that is unaacceptable for the stable branch.
>>  
>>
>> +Git tips
>> +
>> +
>> +Test for build breakage between patches e.g last 8 commits.
>> +
>> +git rebase -i --exec="make -j4" HEAD~8
>> +
>> +Sets the default mailing address for your repo.
>> +
>> +git config --local sendemail.to mesa-dev@lists.freedesktop.org
>> +
>> + Add version to subject line of patch series in this case for the
>> last 8
>> +commits before sending.
>> +
>> +git send-email --subject-prefix="PATCH v4" HEAD~8
>> +git send-email -v4 HEAD~8 # shorter version, 'inherited' from git
>> format-patch
>
>
> While at it, why not @~8? I was quite happy when I learned that through pure
> dumb luck recently...
>
Nice, did not know about it. Fwiw the following (even shorter version)
also works but I've avoided going "crazy" ;-)
$git send-email -v4 -8

My suggestion has some downsides (cannot append --annotate) so barring
any objections I'll sneak yours in before pushing ;-)

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

Re: [Mesa-dev] [PATCH v5 00/20] configure.ac and llvm the 5th

[Emil Velikov]

On 19 November 2016 at 01:38, Tobias Droste  wrote:
> Patches 1-11 and 13-16 are only rebased, no change in functionality and have 
> a reviewed by.
>
> Patch 12 is changed based on review of Emil (I used "HAVE_GALLIUM_LLVM" 
> instead of "USE_MESA_LLVM").
> Patch 17 is changed based on review of Emil
> Patch 18 is split in 18, 19 and 20 as these were independent changes
>
> @Emil:
> I resent the whole series because there was some rebasing todo.
> You only need to look at 12, 17-20 the others are unchanged.
And these are also
Reviewed-by: Emil Velikov 

> Sorry it took so long, but real life needed me :)
>
No worries, we all have $other things to take care of.

Barring any objections I'll like to get these landed in a few days.
Meanwhile I'll give it a bit of testing on my end.

Thanks again !
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] swr: [rasterizer core] clear data now comes in as float

[Rowley, Timothy O]

Reviewed-by: Tim Rowley 
>

On Nov 17, 2016, at 6:51 PM, Ilia Mirkin 
> wrote:

The non-fast-clear path was never updated after clear colors were passed
in as floats. Remove the now-harmful conversion from unorm8.

Signed-off-by: Ilia Mirkin >
---
src/gallium/drivers/swr/rasterizer/core/backend.cpp | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/gallium/drivers/swr/rasterizer/core/backend.cpp 
b/src/gallium/drivers/swr/rasterizer/core/backend.cpp
index 37de650..45eff15 100644
--- a/src/gallium/drivers/swr/rasterizer/core/backend.cpp
+++ b/src/gallium/drivers/swr/rasterizer/core/backend.cpp
@@ -285,17 +285,11 @@ void ProcessClearBE(DRAW_CONTEXT *pDC, uint32_t workerId, 
uint32_t macroTile, vo

if (pClear->attachmentMask & SWR_ATTACHMENT_MASK_COLOR)
{
-/// @todo clear data should come in as RGBA32_FLOAT
DWORD clearData[4];
-float clearFloat[4];
-clearFloat[0] = ((uint8_t*)(>clearRTColor))[0] / 255.0f;
-clearFloat[1] = ((uint8_t*)(>clearRTColor))[1] / 255.0f;
-clearFloat[2] = ((uint8_t*)(>clearRTColor))[2] / 255.0f;
-clearFloat[3] = ((uint8_t*)(>clearRTColor))[3] / 255.0f;
-clearData[0] = *(DWORD*)[0];
-clearData[1] = *(DWORD*)[1];
-clearData[2] = *(DWORD*)[2];
-clearData[3] = *(DWORD*)[3];
+clearData[0] = *(DWORD*)&(pClear->clearRTColor[0]);
+clearData[1] = *(DWORD*)&(pClear->clearRTColor[1]);
+clearData[2] = *(DWORD*)&(pClear->clearRTColor[2]);
+clearData[3] = *(DWORD*)&(pClear->clearRTColor[3]);

PFN_CLEAR_TILES pfnClearTiles = 
sClearTilesTable[KNOB_COLOR_HOT_TILE_FORMAT];
SWR_ASSERT(pfnClearTiles != nullptr);
--
2.7.3


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

Re: [Mesa-dev] [PATCH] docs/submitting patches: add git tips

[Nicolai Hähnle]

On 21.11.2016 17:37, Emil Velikov wrote:

From: Timothy Arceri 

v2: [Emil Velikov]
 - Add the shorthand git send-email -vX
 - Move to submittingpatches.html
 - Add to the TOC.

Cc: Timothy Arceri 
Signed-off-by: Emil Velikov 
Reviewed-by: Brian Paul  (v1)
---
Tim any objections on merging this ? IMHO these are quite useful.
---
 docs/submittingpatches.html | 20 
 1 file changed, 20 insertions(+)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index d3e86f5..7ada508 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -25,6 +25,7 @@
 Reviewing Patches
 Nominating a commit for a stable branch
 Criteria for accepting patches to the stable branch
+Git tips
 

 Basic guidelines
@@ -317,6 +318,25 @@ be rejected:
   regression that is unaacceptable for the stable branch.
 

+Git tips
+
+
+Test for build breakage between patches e.g last 8 commits.
+
+git rebase -i --exec="make -j4" HEAD~8
+
+Sets the default mailing address for your repo.
+
+git config --local sendemail.to mesa-dev@lists.freedesktop.org
+
+ Add version to subject line of patch series in this case for the last 8
+commits before sending.
+
+git send-email --subject-prefix="PATCH v4" HEAD~8
+git send-email -v4 HEAD~8 # shorter version, 'inherited' from git 
format-patch


While at it, why not @~8? I was quite happy when I learned that through 
pure dumb luck recently...


Cheers,
Nicolai


+
+
+

 
 


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

[Mesa-dev] [PATCH] docs/submitting patches: add git tips

[Emil Velikov]

From: Timothy Arceri 

v2: [Emil Velikov]
 - Add the shorthand git send-email -vX
 - Move to submittingpatches.html
 - Add to the TOC.

Cc: Timothy Arceri 
Signed-off-by: Emil Velikov 
Reviewed-by: Brian Paul  (v1)
---
Tim any objections on merging this ? IMHO these are quite useful.
---
 docs/submittingpatches.html | 20 
 1 file changed, 20 insertions(+)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index d3e86f5..7ada508 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -25,6 +25,7 @@
 Reviewing Patches
 Nominating a commit for a stable branch
 Criteria for accepting patches to the stable branch
+Git tips
 
 
 Basic guidelines
@@ -317,6 +318,25 @@ be rejected:
   regression that is unaacceptable for the stable branch.
 
 
+Git tips
+
+
+Test for build breakage between patches e.g last 8 commits.
+
+git rebase -i --exec="make -j4" HEAD~8
+
+Sets the default mailing address for your repo.
+
+git config --local sendemail.to mesa-dev@lists.freedesktop.org
+
+ Add version to subject line of patch series in this case for the last 8
+commits before sending.
+
+git send-email --subject-prefix="PATCH v4" HEAD~8
+git send-email -v4 HEAD~8 # shorter version, 'inherited' from git 
format-patch
+
+
+
 
 
 
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH v4 09/10] gallium: swr: Added swr build for windows

[Emil Velikov]

On 18 November 2016 at 19:49, Kyriazis, George
 wrote:
>
>
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
>> Behalf Of Emil Velikov
>> Sent: Friday, November 18, 2016 5:24 AM
>> To: Kyriazis, George 
>> Cc: ML mesa-dev 
>> Subject: Re: [Mesa-dev] [PATCH v4 09/10] gallium: swr: Added swr build for
>> windows
>>
>> On 18 November 2016 at 04:27, George Kyriazis 
>> wrote:
>> > Also, modify gen_knobs.py so that each invocation creates a single
>> > generated file.  This is more similar to how the other generators behave.
>> > ---
>> >  src/gallium/SConscript |   1 +
>> >  src/gallium/drivers/swr/Makefile.am|  15 +-
>> >  src/gallium/drivers/swr/SConscript | 216
>> +
>> >  .../drivers/swr/rasterizer/scripts/gen_knobs.py|  51 ++---
>> >  4 files changed, 257 insertions(+), 26 deletions(-)  create mode
>> > 100644 src/gallium/drivers/swr/SConscript
>> >
>> This is getting tiresome :'-(
>>
>> I've mentioned on multiple occasions to keep separate logical changes into
>> individual patches.
>> In my previous sounds I've _explicitly_ left only the ones (SConscript
>> fixes) that should be squashed here.
>> Please read review comments more carefully ?
>>
> I hope we can go past the confusion.  Hopefully last set of patches will be 
> posted in a bit.
>
>> When sending version X of patch series, adding r-b/ack-b/t-b/other tags
>> (where applicable) and brief version history is highly recommended.
>> If in doubt on the latter - skim through git log + grep v2.
>>
> Appreciate the pointer; I am new to mesa check-in practices, and I didn't 
> know.  I think it's a good idea to add this info in the Mesa Development 
> notes webpage, to make it more obvious (http://www.mesa3d.org/devinfo.html).
>
Having a look and most/all the things mentioned seems to be already there.
From splitting, breakage, to the r-b,t-b, ack-b tags.

I've just split devinfo.html in a goal to make things clearer/stand
out. Information about patch submission is in
docs/submittingpatches.html.
If there's anything unclear (or a hyperlink should be nice) do send a
patch. Pretty please ?

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

Re: [Mesa-dev] [PATCH 05/13] glsl: simplifies the merge of the default in layout qualifier

[Andres Gomez]

On Thu, 2016-11-17 at 16:17 +1100, Timothy Arceri wrote:
> On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
...
> > diff --git a/src/compiler/glsl/ast_type.cpp
> > b/src/compiler/glsl/ast_type.cpp
> > index 803d952..064c63b 100644
> > --- a/src/compiler/glsl/ast_type.cpp
> > +++ b/src/compiler/glsl/ast_type.cpp
...
> > @@ -534,6 +549,45 @@
> > ast_type_qualifier::validate_in_qualifier(YYLTYPE *loc,
> >    _mesa_glsl_error(loc, state, "invalid input layout qualifiers
> > used");
> > }
> >  
> > +   /* The checks below are also performed when merging but we want
> > to spit an
> > +* error against the default global input qualifier as soon as we
> > can, with
> > +* the closest error location in the shader.
> > +*/
> 
> 
> This comment looks like it should be removed? Don't the below changes
> remove the validation from the merge?

I don't think I follow what you mean.

The comment applies in the sense that the validation below is done
against the default in qualifier while the validation done in the merge
is between the 2 layout-qualifiers merging.

That validation of the merge will happen against the default in
qualifier in any case, in the last step of the merging. However, since
we want to fail ASAP and with the proper failing column, we have to
validate explicitly against the default in qualifier every time after a
merge.

In my opinion, the comment stands, although maybe I should rephrase.
Dunno.

> 
> 
> > +
> > +   /* Input layout qualifiers can be specified multiple
> > +* times in separate declarations, as long as they match.
> > +*/
> > +   if (state->in_qualifier->flags.q.prim_type && this-
> > > flags.q.prim_type
> > 
> > +   && state->in_qualifier->prim_type != this->prim_type) {
> > +  r = false;
> > +  _mesa_glsl_error(loc, state,
> > +   "conflicting input primitive %s specified",
> > +   state->stage == MESA_SHADER_GEOMETRY ?
> > +   "type" : "mode");
> > +   }
> > +
> > +   if (state->in_qualifier->flags.q.vertex_spacing
> > +   && this->flags.q.vertex_spacing
> > +   && state->in_qualifier->vertex_spacing != this-
> > > vertex_spacing) {
> > 
> > +  r = false;
> > +  _mesa_glsl_error(loc, state,
> > +   "conflicting vertex spacing specified");
> > +   }
> > +
> > +   if (state->in_qualifier->flags.q.ordering && this-
> > > flags.q.ordering
> > 
> > +   && state->in_qualifier->ordering != this->ordering) {
> > +  r = false;
> > +  _mesa_glsl_error(loc, state,
> > +   "conflicting ordering specified");
> > +   }
> > +
> > +   if (state->in_qualifier->flags.q.point_mode && this-
> > > flags.q.point_mode
> > 
> > +   && state->in_qualifier->point_mode != this->point_mode) {
> > +  r  = false;
> 
>            ^
> There is an extra space here          

I'll correct this.

-- 
Br,

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

Re: [Mesa-dev] [PATCH 04/13] glsl: Split default in layout qualifier merge

[Andres Gomez]

On Thu, 2016-11-17 at 16:56 +1100, Timothy Arceri wrote:
> On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
... 
> > diff --git a/src/compiler/glsl/glsl_parser.yy
> > b/src/compiler/glsl/glsl_parser.yy
> > index 50f7097..9f18c15 100644
> > --- a/src/compiler/glsl/glsl_parser.yy
> > +++ b/src/compiler/glsl/glsl_parser.yy
> > @@ -2913,8 +2913,10 @@ layout_in_defaults:
> >   _mesa_glsl_error(&@1, state, "duplicate layout(...)
> > qualifiers");
> >   YYERROR;
> >    } else {
> > - if (!state->in_qualifier->
> > -merge_in_qualifier(& @1, state, $1, $$, false)) {
> > + if (!$1.validate_in_qualifier(& @1, state)) {
> > +YYERROR;
> > + }
> > + if (!$1.merge_into_in_qualifier(& @1, state, $$, false)) {
> >  YYERROR;
> >   }
> 
> 
> To avoid regressions between patches the order should be merge then
> validate right?

No, the original code does first the validation and then the merge.

This swaps when the merging behavior changes, merging the default in
qualifier as the last step in patch 08/13. Then, we first merge and
afterwards validate on the resulting layout-qualifier.

-- 
Br,

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

Re: [Mesa-dev] [PATCH 07/13] glsl: Add comments for the point mode layout-id-qualifier validation

[Andres Gomez]

On Thu, 2016-11-17 at 16:47 +1100, Timothy Arceri wrote:
> On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
> > The point mode value in an ast_type_qualifier can only be true if the
> > flag is already set since this layout-id-qualifier can only be or not
> > be present in a shader.
> > 
> > Hence, it is useless to check for its value if the flag is already
> > set. However, for coherence and compatibility with future changes we
> > do check its value.
> 
> 
> It's dead code. Please remove the error check and replace it with an
> assert.

I'll send a new version.

-- 
Br,

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

Re: [Mesa-dev] [PATCH 04/13] glsl: Split default in layout qualifier merge

[Andres Gomez]

On Thu, 2016-11-17 at 16:45 +1100, Timothy Arceri wrote:

> I might have asked you this before. Is there a reason why we can't call
> merge_qualifier from merge_into_in_qualifier() like we do
> in merge_into_out_qualifier() to take care of some of these for us?
> Since there is duplicate code.

Answering for the sake of completion.

I suppose you have already realized, but patch 05/13 does precisely
that. I thought it was clearer to make the change in 2 steps instead of
in a single patch.

-- 
Br,

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

Re: [Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

[Daniel Stone]

Hi Jonas,

On 21 November 2016 at 05:56, Jonas Ådahl  wrote:
> On Thu, Nov 10, 2016 at 10:38:51AM +, Daniel Stone wrote:
>> On 10 November 2016 at 06:08, Jonas Ådahl  wrote:
>> > On Mon, Oct 24, 2016 at 08:42:59PM +0100, Daniel Stone wrote:
>> That relies on the compositor releasing buffers in a strictly FIFO
>> manner though, which is ... dangerous.
>
> Is FIFO an issue here? Lets assume it looks like this:
>
> [...]
>
> In other terms, if we don't have a buffer released at the point we
> query, we say its a new buffer, making the application assume it needs
> to redraw everything. When we then actually need the buffer for doing
> things, we see if there is any buffer to reuse. What buffer was actually
> released doesn't matter, since the application will redraw everything
> anyway, and if we actually did reuse a buffer, the previously returned
> buffer age was a lie, but a harmless one, and if we eventually actually
> did allocate a new one, it wouldn't even be a lie.

Sorry, I see what you mean now - thanks for the snippet!

Yes, I think that's certainly doable; we could add a parameter to
get_back_bo controlling its wait behaviour. Specifically, when called
from query_buffer_age we would return a released buffer (if any), do
one roundtrip and check again if we didn't have one yet, and if the
roundtrip didn't return any buffers, then we could return an age of -1
to allow the client to continue on. When called from
get_buffers_with_format, it would spin or allocate a new buffer. I
suppose there is a degenerate case there, where you could end up
always failing the age check but not allocating new buffers, but I'm
far too Monday'ed to sketch out what that would be.

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

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Rob Herring]

On Mon, Nov 21, 2016 at 8:15 AM, Emil Velikov  wrote:
> On 21 November 2016 at 07:23, Tomasz Figa  wrote:
>> Hi,
>>
>> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan  wrote:
>>> mesa android path didn't support pbuffer, so add pbuffer support to
>>> fix most deqp and cts pbuffer test cases fail;
>>> add single buffer config to support pbuffer, and create image for
>>> pbuffer when pbuffer type is front surface.
>>> The EGL 1.5 spec states that pbuffers have a back buffer but no front
>>> buffer, single-buffered surfaces with no front buffer confuse Mesa;
>>> so we deviate from the spec, following the precedent of Mesa's
>>> EGL X11 platform.
>>>
>>> Test status: android CTS EGL pbuffer test can run without native crash.
>>> test:[DEQP,EGL]all deqp pbuffer case passed.
>>>
>>> V3: update commit message and code review changes.
>>>
>>> Signed-off-by: Liu Zhiquan 
>>> Signed-off-by: Kalyan Kondapally 
>>> ---
>>>  src/egl/drivers/dri2/egl_dri2.h |  3 +-
>>>  src/egl/drivers/dri2/platform_android.c | 98 
>>> +
>>>  2 files changed, 78 insertions(+), 23 deletions(-)
>>>
>>
>> Looks like this patch has already landed, but please let me try to
>> confirm some things here anyway. Would you mind keeping me on CC for
>> any future patches for the EGL/Android module? (
>
>
>> I believe
>> scripts/get_reviewer.pl should already include me on the list of
>> suggested reviewers, similarly for Rob Herring, who did a great job
>> before helping us with testing on platforms other than i915.)
>>
> I'll add you and update the documentation to reference the script.
>
> Rob H let me know if you'd like to be in there as well which files
> (platform_egl.c, Android build and/or other) and which email you'd
> prefer.

Yes, please. Use my kernel.org address.

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

[Mesa-dev] [PATCH v2] util/disk_cache: close a previously opened handle in disk_cache_put (v2)

[Mun Gwan-gyeong]

We're missing the close() to the matching open().

CID 1373407

v2: Fixes from Emil Velikov's review
Update the teardown in reverse order of the setup/init.

Signed-off-by: Mun Gwan-gyeong 
---
 src/util/disk_cache.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index 4287878..80dc788 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -612,19 +612,18 @@ disk_cache_put(struct disk_cache *cache,
 
p_atomic_add(cache->size, size);
 
+ done:
+   if (fd_final != -1)
+  close(fd_final);
/* This close finally releases the flock, (now that the final dile
 * has been renamed into place and the size has been added).
 */
-   close(fd);
-   fd = -1;
-
- done:
+   if (fd != -1)
+  close(fd);
if (filename_tmp)
   ralloc_free(filename_tmp);
if (filename)
   ralloc_free(filename);
-   if (fd != -1)
-  close(fd);
 }
 
 void *
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH v5 09/11] swr: Modify gen_knobs.{cpp|h} creation script

[Kyriazis, George]

Thank you for the reviews, Emil.

Tim will check-in the series, since he has check-in privileges.

George

> -Original Message-
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Saturday, November 19, 2016 4:23 PM
> To: Kyriazis, George 
> Cc: ML mesa-dev 
> Subject: Re: [Mesa-dev] [PATCH v5 09/11] swr: Modify gen_knobs.{cpp|h}
> creation script
> 
> Hi George,
> 
> Thanks for sorting things.. There's a pretty trivial nitpick below, but please
> don't bother addressing it.
> Things are pretty good as-is - check with Tim/others and land the series.
> 
> On 18 November 2016 at 20:38, George Kyriazis 
> wrote:
> > Modify gen_knobs.py so that each invocation creates a single generated
> > file.  This is more similar to how the other generators behave.
> >
> > v5: remove Scoscript edits from this commit; moved to commit that
> > first adds SConscript
> >
> > Acked-by: Emil Velikov 
> > ---
> >  src/gallium/drivers/swr/Makefile.am| 15 ++-
> >  .../drivers/swr/rasterizer/scripts/gen_knobs.py| 51 --
> 
> 
> > @@ -235,6 +245,7 @@ libswrAVX2_la_LDFLAGS = \  include
> > $(top_srcdir)/install-gallium-links.mk
> >
> >  EXTRA_DIST = \
> > +   SConscript \
> 
> IIRC I've mentioned it a couple of times - this should be part of the commit
> that introduces the file.
> 
> Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] util/disk_cache: close a previously opened handle in disk_cache_put

[Emil Velikov]

On 20 November 2016 at 13:59, Mun Gwan-gyeong  wrote:
> We're missing the close() to the matching open().
>
> CID 1373407
>
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/util/disk_cache.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index 4287878..5ceb3aa 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -612,19 +612,18 @@ disk_cache_put(struct disk_cache *cache,
>
> p_atomic_add(cache->size, size);
>
> -   /* This close finally releases the flock, (now that the final dile
> -* has been renamed into place and the size has been added).
> -*/
> -   close(fd);
> -   fd = -1;
> -
>   done:
> if (filename_tmp)
>ralloc_free(filename_tmp);
> if (filename)
>ralloc_free(filename);
> +   /* This close finally releases the flock, (now that the final dile
> +* has been renamed into place and the size has been added).
> +*/
> if (fd != -1)
>close(fd);
> +   if (fd_final != -1)
> +  close(fd_final);
Please keep the teardown in reverse order of the setup/init. Namely:

fd_final
fd
filename_tmp
filename

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

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Emil Velikov]

On 21 November 2016 at 14:36, Tomasz Figa  wrote:
> On Mon, Nov 21, 2016 at 11:15 PM, Emil Velikov  
> wrote:
>> On 21 November 2016 at 07:23, Tomasz Figa  wrote:
>>> Hi,
>>>
>>> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan  wrote:
 mesa android path didn't support pbuffer, so add pbuffer support to
 fix most deqp and cts pbuffer test cases fail;
 add single buffer config to support pbuffer, and create image for
 pbuffer when pbuffer type is front surface.
 The EGL 1.5 spec states that pbuffers have a back buffer but no front
 buffer, single-buffered surfaces with no front buffer confuse Mesa;
 so we deviate from the spec, following the precedent of Mesa's
 EGL X11 platform.

 Test status: android CTS EGL pbuffer test can run without native crash.
 test:[DEQP,EGL]all deqp pbuffer case passed.

 V3: update commit message and code review changes.

 Signed-off-by: Liu Zhiquan 
 Signed-off-by: Kalyan Kondapally 
 ---
  src/egl/drivers/dri2/egl_dri2.h |  3 +-
  src/egl/drivers/dri2/platform_android.c | 98 
 +
  2 files changed, 78 insertions(+), 23 deletions(-)

>>>
>>> Looks like this patch has already landed, but please let me try to
>>> confirm some things here anyway. Would you mind keeping me on CC for
>>> any future patches for the EGL/Android module? (
>>
>>
>>> I believe
>>> scripts/get_reviewer.pl should already include me on the list of
>>> suggested reviewers, similarly for Rob Herring, who did a great job
>>> before helping us with testing on platforms other than i915.)
>>>
>> I'll add you and update the documentation to reference the script.
>>
>> Rob H let me know if you'd like to be in there as well which files
>> (platform_egl.c, Android build and/or other) and which email you'd
>> prefer.
>
> Thanks!
>

 if (update_buffers(dri2_surf) < 0)
return 0;

 if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
 -  /*
 -   * We don't support front buffers and GLES doesn't require them for
 -   * window surfaces, but some DRI drivers will request them anyway.
 -   * We just ignore such request as other platforms backends do.
 +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
 + /* According current EGL spec,
 +  * front buffer rendering for window surface is not supported 
 now */
 + _eglLog(_EGL_WARNING,
 +   "%s:%d front buffer rendering for window surface is not 
 supported!\n",
 +   __func__, __LINE__);
 + return 0;
>>>
>>> This is a semantic change and according to the old comment it might
>>> break some drivers ("We don't support front buffers and GLES doesn't
>>> require them for window surfaces, but some DRI drivers will request
>>> them anyway.").
>>>
>> Fwiw -
>
> Hmm, not sure I'm getting your point here correctly (it's quite late
> so my parser might be half asleep already), but comment below is about
> pbuffers only. The comment above apparently says that EGL doesn't
> support front buffer rendering, but my comment here is more about the
> DRI extension. The semantics of the image loader extension seemed to
> allow this kind of "aggresive" probing, when the driver sets bits for
> any kinds of buffers it's interested in and it's not fatal if some of
> them are not available. However this was inferred from looking at
> driver code, as I couldn't really find any precise spec.
>
Brain went on, fingers did not follow. I meant to say
"Fwiw I've fine with temporary reverting the patch, if things are
seriously broken on your end."

On the spec side - don't think we have anything but the implicit one
in dri_interface.h

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

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Tomasz Figa]

On Mon, Nov 21, 2016 at 11:15 PM, Emil Velikov  wrote:
> On 21 November 2016 at 07:23, Tomasz Figa  wrote:
>> Hi,
>>
>> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan  wrote:
>>> mesa android path didn't support pbuffer, so add pbuffer support to
>>> fix most deqp and cts pbuffer test cases fail;
>>> add single buffer config to support pbuffer, and create image for
>>> pbuffer when pbuffer type is front surface.
>>> The EGL 1.5 spec states that pbuffers have a back buffer but no front
>>> buffer, single-buffered surfaces with no front buffer confuse Mesa;
>>> so we deviate from the spec, following the precedent of Mesa's
>>> EGL X11 platform.
>>>
>>> Test status: android CTS EGL pbuffer test can run without native crash.
>>> test:[DEQP,EGL]all deqp pbuffer case passed.
>>>
>>> V3: update commit message and code review changes.
>>>
>>> Signed-off-by: Liu Zhiquan 
>>> Signed-off-by: Kalyan Kondapally 
>>> ---
>>>  src/egl/drivers/dri2/egl_dri2.h |  3 +-
>>>  src/egl/drivers/dri2/platform_android.c | 98 
>>> +
>>>  2 files changed, 78 insertions(+), 23 deletions(-)
>>>
>>
>> Looks like this patch has already landed, but please let me try to
>> confirm some things here anyway. Would you mind keeping me on CC for
>> any future patches for the EGL/Android module? (
>
>
>> I believe
>> scripts/get_reviewer.pl should already include me on the list of
>> suggested reviewers, similarly for Rob Herring, who did a great job
>> before helping us with testing on platforms other than i915.)
>>
> I'll add you and update the documentation to reference the script.
>
> Rob H let me know if you'd like to be in there as well which files
> (platform_egl.c, Android build and/or other) and which email you'd
> prefer.

Thanks!

>>>
>>> if (update_buffers(dri2_surf) < 0)
>>>return 0;
>>>
>>> if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
>>> -  /*
>>> -   * We don't support front buffers and GLES doesn't require them for
>>> -   * window surfaces, but some DRI drivers will request them anyway.
>>> -   * We just ignore such request as other platforms backends do.
>>> +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>>> + /* According current EGL spec,
>>> +  * front buffer rendering for window surface is not supported now 
>>> */
>>> + _eglLog(_EGL_WARNING,
>>> +   "%s:%d front buffer rendering for window surface is not 
>>> supported!\n",
>>> +   __func__, __LINE__);
>>> + return 0;
>>
>> This is a semantic change and according to the old comment it might
>> break some drivers ("We don't support front buffers and GLES doesn't
>> require them for window surfaces, but some DRI drivers will request
>> them anyway.").
>>
> Fwiw -

Hmm, not sure I'm getting your point here correctly (it's quite late
so my parser might be half asleep already), but comment below is about
pbuffers only. The comment above apparently says that EGL doesn't
support front buffer rendering, but my comment here is more about the
DRI extension. The semantics of the image loader extension seemed to
allow this kind of "aggresive" probing, when the driver sets bits for
any kinds of buffers it's interested in and it's not fatal if some of
them are not available. However this was inferred from looking at
driver code, as I couldn't really find any precise spec.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] util/disk_cache: close a previously opened handle in disk_cache_put

[Iago Toral]

Reviewed-by: Iago Toral Quiroga 

On Sun, 2016-11-20 at 22:59 +0900, Mun Gwan-gyeong wrote:
> We're missing the close() to the matching open().
> 
> CID 1373407
> 
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/util/disk_cache.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index 4287878..5ceb3aa 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -612,19 +612,18 @@ disk_cache_put(struct disk_cache *cache,
>  
> p_atomic_add(cache->size, size);
>  
> -   /* This close finally releases the flock, (now that the final
> dile
> -* has been renamed into place and the size has been added).
> -*/
> -   close(fd);
> -   fd = -1;
> -
>   done:
> if (filename_tmp)
>    ralloc_free(filename_tmp);
> if (filename)
>    ralloc_free(filename);
> +   /* This close finally releases the flock, (now that the final
> dile
> +* has been renamed into place and the size has been added).
> +*/
> if (fd != -1)
>    close(fd);
> +   if (fd_final != -1)
> +  close(fd_final);
>  }
>  
>  void *
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] radeonsi: Add missing error-checking to si_create_compute_state (v2)

[Mun Gwan-gyeong]

When the uploading of shader fails on si_shader_binary_upload(),
it returns -ENOMEM. We should handle si_shader_binary_upload() failure path
on si_create_compute_state().

CID 1394027

v2: Fixes from Edward O'Callaghan's review
 a) Update explicitly return value check with "si_shader_binary_upload() < 0"
 b) Update commit message.

Signed-off-by: Mun Gwan-gyeong 
---
 src/gallium/drivers/radeonsi/si_compute.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
b/src/gallium/drivers/radeonsi/si_compute.c
index f1887bb..0815713 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -165,7 +165,11 @@ static void *si_create_compute_state(
}
si_shader_dump(sctx->screen, >shader, >b.debug,
   PIPE_SHADER_COMPUTE, stderr);
-   si_shader_binary_upload(sctx->screen, >shader);
+   if (si_shader_binary_upload(sctx->screen, >shader) < 
0) {
+   fprintf(stderr, "LLVM failed to upload shader\n");
+   FREE(program);
+   return NULL;
+   }
}
 
return program;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH v2] radeonsi: Add missing error-checking to si_create_compute_state (v2)

[Edward O'Callaghan]

Reviewed-by: Edward O'Callaghan 

On 11/22/2016 01:20 AM, Mun Gwan-gyeong wrote:
> When the uploading of shader fails on si_shader_binary_upload(),
> it returns -ENOMEM. We should handle si_shader_binary_upload() failure path
> on si_create_compute_state().
> 
> CID 1394027
> 
> v2: Fixes from Edward O'Callaghan's review
>  a) Update explicitly return value check with "si_shader_binary_upload() < 0"
>  b) Update commit message.
> 
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/gallium/drivers/radeonsi/si_compute.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
> b/src/gallium/drivers/radeonsi/si_compute.c
> index f1887bb..0815713 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.c
> +++ b/src/gallium/drivers/radeonsi/si_compute.c
> @@ -165,7 +165,11 @@ static void *si_create_compute_state(
>   }
>   si_shader_dump(sctx->screen, >shader, >b.debug,
>  PIPE_SHADER_COMPUTE, stderr);
> - si_shader_binary_upload(sctx->screen, >shader);
> + if (si_shader_binary_upload(sctx->screen, >shader) < 
> 0) {
> + fprintf(stderr, "LLVM failed to upload shader\n");
> + FREE(program);
> + return NULL;
> + }
>   }
>  
>   return program;
> 



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] EGL/android: pbuffer implementation.

[Emil Velikov]

On 21 November 2016 at 07:23, Tomasz Figa  wrote:
> Hi,
>
> On Wed, Nov 16, 2016 at 11:11 AM, Liu Zhiquan  wrote:
>> mesa android path didn't support pbuffer, so add pbuffer support to
>> fix most deqp and cts pbuffer test cases fail;
>> add single buffer config to support pbuffer, and create image for
>> pbuffer when pbuffer type is front surface.
>> The EGL 1.5 spec states that pbuffers have a back buffer but no front
>> buffer, single-buffered surfaces with no front buffer confuse Mesa;
>> so we deviate from the spec, following the precedent of Mesa's
>> EGL X11 platform.
>>
>> Test status: android CTS EGL pbuffer test can run without native crash.
>> test:[DEQP,EGL]all deqp pbuffer case passed.
>>
>> V3: update commit message and code review changes.
>>
>> Signed-off-by: Liu Zhiquan 
>> Signed-off-by: Kalyan Kondapally 
>> ---
>>  src/egl/drivers/dri2/egl_dri2.h |  3 +-
>>  src/egl/drivers/dri2/platform_android.c | 98 
>> +
>>  2 files changed, 78 insertions(+), 23 deletions(-)
>>
>
> Looks like this patch has already landed, but please let me try to
> confirm some things here anyway. Would you mind keeping me on CC for
> any future patches for the EGL/Android module? (


> I believe
> scripts/get_reviewer.pl should already include me on the list of
> suggested reviewers, similarly for Rob Herring, who did a great job
> before helping us with testing on platforms other than i915.)
>
I'll add you and update the documentation to reference the script.

Rob H let me know if you'd like to be in there as well which files
(platform_egl.c, Android build and/or other) and which email you'd
prefer.

> [snip]
>
>> @@ -353,6 +353,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay 
>> *disp, _EGLSurface *surf)
>>dri2_surf->window->common.decRef(_surf->window->common);
>> }
>>
>> +   if (dri2_surf->dri_image_back) {
>> +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_back", __func__, 
>> __LINE__);
>> +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_back);
>> +  dri2_surf->dri_image_back = NULL;
>> +   }
>
> Is this a fix for another bug by any chance? Note that we already take
> care of dri_image_back for window surfaces in
> droid_window_cancel_buffer(), which calls
> droid_window_enqueue_buffer(), which does the real free on the image.
> It doesn't hurt to have it here as well, though, so treat this just as
> a random thought of mine.
>
>> +
>> +   if (dri2_surf->dri_image_front) {
>> +  _eglLog(_EGL_DEBUG, "%s : %d : destroy dri_image_front", __func__, 
>> __LINE__);
>> +  dri2_dpy->image->destroyImage(dri2_surf->dri_image_front);
>> +  dri2_surf->dri_image_front = NULL;
>> +   }
>> +
>> (*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);
>>
>> free(dri2_surf);
>
> [snip]
>
>> @@ -439,25 +451,75 @@ droid_image_get_buffers(__DRIdrawable *driDrawable,
>>struct __DRIimageList *images)
>>  {
>> struct dri2_egl_surface *dri2_surf = loaderPrivate;
>> +   struct dri2_egl_display *dri2_dpy =
>> +  dri2_egl_display(dri2_surf->base.Resource.Display);
>>
>> images->image_mask = 0;
>> +   images->front = NULL;
>> +   images->back = NULL;
>
> I'm not 100% sure this is the expected behavior of this function. My
> understanding was that image_mask and error code would guard the other
> members. It would make sense since typically if a function fails it
> should keep the passed writable arguments unchanged. Is there a
> precise description of the semantics used by the image loader
> extension written down somewhere?
>
>>
>> if (update_buffers(dri2_surf) < 0)
>>return 0;
>>
>> if (buffer_mask & __DRI_IMAGE_BUFFER_FRONT) {
>> -  /*
>> -   * We don't support front buffers and GLES doesn't require them for
>> -   * window surfaces, but some DRI drivers will request them anyway.
>> -   * We just ignore such request as other platforms backends do.
>> +  if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>> + /* According current EGL spec,
>> +  * front buffer rendering for window surface is not supported now 
>> */
>> + _eglLog(_EGL_WARNING,
>> +   "%s:%d front buffer rendering for window surface is not 
>> supported!\n",
>> +   __func__, __LINE__);
>> + return 0;
>
> This is a semantic change and according to the old comment it might
> break some drivers ("We don't support front buffers and GLES doesn't
> require them for window surfaces, but some DRI drivers will request
> them anyway.").
>
Fwiw -

>> +  }
>> +
>> +  /* The EGL 1.5 spec states that pbuffers are single-buffered. 
>> Specifically,
>> +   * the spec states that they have a back buffer but no front buffer, 
>> in
>> +   * contrast to pixmaps, which have a front buffer but no back buffer.
>
> [snip]
>
>>
>> if (buffer_mask & 

Re: [Mesa-dev] [PATCH v2 2/2] egl/android: Use gralloc::lock_ycbcr for resolving YUV formats (v2)

[Tomasz Figa]

On Mon, Nov 21, 2016 at 10:39 PM, Emil Velikov  wrote:
> On 21 November 2016 at 06:50, Tomasz Figa  wrote:
>> On Thu, Nov 10, 2016 at 5:48 PM, Tapani Pälli  wrote:
>>>
>>>
>>> On 11/10/2016 09:55 AM, Tomasz Figa wrote:

 There is an interface that can be used to query YUV buffers for their
 internal format. Specifically, if gralloc:lock_ycbcr() is given no SW
 usage flags, it's supposed to return plane offsets instead of pointers.
 Let's use this interface to implement support for YUV formats in Android
 EGL backend.
>>>
>>>
>>> IMO lock_ycbcr() is not very intuitive API (read: is horrible) to query
>>> surface information, would be nice to have something generic along the lines
>>> of 'resolve_format' or 'query_format' but I don't know how feasible it would
>>> be to push for such new interface ..
>>
>> Yeah, I agree that it's not very intuitive, however it does its job
>> pretty well without making Android depend on some DRM-specific stuff
>> (which I personally don't have anything against...). Since I'm only a
>> ChromeOS mortal, I can't really tell what we could do about it,
>> though. In any case, any change would only affect further Android
>> versions, so we still need to support the old API for current ones.
>>
>>>
>>> both of these patches look good to me;
>>> Reviewed-by: Tapani Pälli 
>>>
>>
>> Anyway, I think we can commit this for the time being, so:
>>
> Rob H, Chih-Wei please check that your gbm_gralloc/drm_gralloc have
> the [simple implementation [1] of] gralloc:lock_ycbcr. Without it YUV
> be fun.

Technically we don't have to require it for HAL_PIXEL_FORMAT_YV12, but
to avoid hardcoding implementation-specific stride and offset
alignments I decided to just use lock_ycbcr for it as well.

>
>> Gentle ping. :)
>>
> The RGB888 patch and this series is in master.
>
> Thanks for the reminder!

Thanks! I got a bit more time to work on upstreaming the remaining
bits recently and with these two matters sorted out I can proceed with
remaining ones. Stay tuned. :)

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] egl/x11: store xcb_screen_t *screen instead of int screen

[Emil Velikov]

On 17 November 2016 at 13:59, Eric Engestrom  wrote:
> On Friday, 2016-11-11 16:31:16 +, Emil Velikov wrote:
>> From: Emil Velikov 
>>
>> Just fetch and store it once, rather than doing the
>> xcb_setup_roots_iterator + get_xcb_screen dance five times.
>>
>> Signed-off-by: Emil Velikov 
>> ---
>> These two patches are an example of the duplication we have within each
>> loader (be that egl, gbm or glx)_and how we can fold it up :-)
>>
>>  src/egl/drivers/dri2/egl_dri2.h  |  2 +-
>>  src/egl/drivers/dri2/platform_x11.c  | 51 
>> ++--
>>  src/egl/drivers/dri2/platform_x11_dri3.c | 32 ++--
>>  3 files changed, 19 insertions(+), 66 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/egl_dri2.h 
>> b/src/egl/drivers/dri2/egl_dri2.h
>> index 0020a5b..fa94cbe 100644
>> --- a/src/egl/drivers/dri2/egl_dri2.h
>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>> @@ -199,7 +199,7 @@ struct dri2_egl_display
>>
>>  #ifdef HAVE_X11_PLATFORM
>> xcb_connection_t *conn;
>> -   int  screen;
>> +   xcb_screen_t *screen;
>> int  swap_available;
>>  #ifdef HAVE_DRI3
>> struct loader_dri3_extensions loader_dri3_ext;
>> diff --git a/src/egl/drivers/dri2/platform_x11.c 
>> b/src/egl/drivers/dri2/platform_x11.c
>> index e152868..f4bcf4a 100644
>> --- a/src/egl/drivers/dri2/platform_x11.c
>> +++ b/src/egl/drivers/dri2/platform_x11.c
>> @@ -206,10 +206,8 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay 
>> *disp, EGLint type,
>> struct dri2_egl_surface *dri2_surf;
>> xcb_get_geometry_cookie_t cookie;
>> xcb_get_geometry_reply_t *reply;
>> -   xcb_screen_iterator_t s;
>> xcb_generic_error_t *error;
>> xcb_drawable_t drawable;
>> -   xcb_screen_t *screen;
>> const __DRIconfig *config;
>>
>> STATIC_ASSERT(sizeof(uintptr_t) == sizeof(native_surface));
>> @@ -228,16 +226,9 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay 
>> *disp, EGLint type,
>>
>> dri2_surf->region = XCB_NONE;
>> if (type == EGL_PBUFFER_BIT) {
>> -  s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
>> -  screen = get_xcb_screen(s, dri2_dpy->screen);
>> -  if (!screen) {
>> - _eglError(EGL_BAD_ALLOC, "failed to get xcb screen");
>> - goto cleanup_surf;
>> -  }
>> -
>>dri2_surf->drawable = xcb_generate_id(dri2_dpy->conn);
>>xcb_create_pixmap(dri2_dpy->conn, conf->BufferSize,
>> -   dri2_surf->drawable, screen->root,
>> +   dri2_surf->drawable, dri2_dpy->screen->root,
>>   dri2_surf->base.Width, dri2_surf->base.Height);
>> } else {
>>if (!drawable) {
>> @@ -544,20 +535,10 @@ dri2_x11_do_authenticate(struct dri2_egl_display 
>> *dri2_dpy, uint32_t id)
>>  {
>> xcb_dri2_authenticate_reply_t *authenticate;
>> xcb_dri2_authenticate_cookie_t authenticate_cookie;
>> -   xcb_screen_iterator_t s;
>> -   xcb_screen_t *screen;
>> int ret = 0;
>>
>> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
>> -
>> -   screen = get_xcb_screen(s, dri2_dpy->screen);
>> -   if (!screen) {
>> -  _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
>> -  return -1;
>> -   }
>> -
>> authenticate_cookie =
>> -  xcb_dri2_authenticate_unchecked(dri2_dpy->conn, screen->root, id);
>> +  xcb_dri2_authenticate_unchecked(dri2_dpy->conn, 
>> dri2_dpy->screen->root, id);
>> authenticate =
>>xcb_dri2_authenticate_reply(dri2_dpy->conn, authenticate_cookie, 
>> NULL);
>>
>> @@ -598,8 +579,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
>> xcb_dri2_connect_reply_t *connect;
>> xcb_dri2_connect_cookie_t connect_cookie;
>> xcb_generic_error_t *error;
>> -   xcb_screen_iterator_t s;
>> -   xcb_screen_t *screen;
>> char *driver_name, *loader_driver_name, *device_name;
>> const xcb_query_extension_reply_t *extension;
>>
>> @@ -622,13 +601,7 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
>>  XCB_DRI2_MAJOR_VERSION,
>>  XCB_DRI2_MINOR_VERSION);
>>
>> -   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
>> -   screen = get_xcb_screen(s, dri2_dpy->screen);
>> -   if (!screen) {
>> -  _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
>> -  return EGL_FALSE;
>> -   }
>> -   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, screen->root,
>> +   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, 
>> dri2_dpy->screen->root,
>> XCB_DRI2_DRIVER_TYPE_DRI);
>>
>> xfixes_query =
>> @@ -720,7 +693,6 @@ static EGLBoolean
>>  dri2_x11_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
>>   _EGLDisplay *disp, bool supports_preserved)
>>  {
>> - 

[Mesa-dev] [PATCH v2 3/3] auxiliary/vl/dri: call get_xcb_screen() only once

[Emil Velikov]

From: Emil Velikov 

Signed-off-by: Emil Velikov 
Reviewed-by: Eric Engestrom 
---
 src/gallium/auxiliary/vl/vl_winsys_dri.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index 9ecc216..0b2c939 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -326,6 +326,7 @@ vl_dri2_screen_create(Display *display, int screen)
xcb_dri2_authenticate_cookie_t authenticate_cookie;
xcb_dri2_authenticate_reply_t *authenticate = NULL;
xcb_screen_iterator_t s;
+   xcb_screen_t *xcb_screen;
xcb_generic_error_t *error = NULL;
char *device_name;
int fd, device_name_length;
@@ -357,6 +358,9 @@ vl_dri2_screen_create(Display *display, int screen)
   goto free_query;
 
s = xcb_setup_roots_iterator(xcb_get_setup(scrn->conn));
+   xcb_screen = get_xcb_screen(s, screen);
+   if (!xcb_screen)
+  goto free_query;
 
driverType = XCB_DRI2_DRIVER_TYPE_DRI;
 #ifdef DRI2DriverPrimeShift
@@ -374,7 +378,7 @@ vl_dri2_screen_create(Display *display, int screen)
 #endif
 
connect_cookie = xcb_dri2_connect_unchecked(scrn->conn,
-   get_xcb_screen(s, screen)->root,
+   xcb_screen->root,
driverType);
connect = xcb_dri2_connect_reply(scrn->conn, connect_cookie, NULL);
if (connect == NULL ||
@@ -396,7 +400,7 @@ vl_dri2_screen_create(Display *display, int screen)
   goto close_fd;
 
authenticate_cookie = xcb_dri2_authenticate_unchecked(scrn->conn,
- get_xcb_screen(s, 
screen)->root,
+ xcb_screen->root,
  magic);
authenticate = xcb_dri2_authenticate_reply(scrn->conn, authenticate_cookie, 
NULL);
 
-- 
2.10.2

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

[Mesa-dev] [PATCH v2 2/3] egl/x11: store xcb_screen_t *screen instead of int screen

[Emil Velikov]

From: Emil Velikov 

Just fetch and store it once, rather than doing the
xcb_setup_roots_iterator + get_xcb_screen dance five times.

v2: Call xcb_disconnect() on error (Eric)

Signed-off-by: Emil Velikov 
Reviewed-by: Eric Engestrom  (v1)
---
 src/egl/drivers/dri2/egl_dri2.h  |  2 +-
 src/egl/drivers/dri2/platform_x11.c  | 50 ++--
 src/egl/drivers/dri2/platform_x11_dri3.c | 32 ++--
 3 files changed, 18 insertions(+), 66 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index c039b64..467b7dc 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -199,7 +199,7 @@ struct dri2_egl_display
 
 #ifdef HAVE_X11_PLATFORM
xcb_connection_t *conn;
-   int  screen;
+   xcb_screen_t *screen;
int  swap_available;
 #ifdef HAVE_DRI3
struct loader_dri3_extensions loader_dri3_ext;
diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index 00613d9..df39ca8 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -206,10 +206,8 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay 
*disp, EGLint type,
struct dri2_egl_surface *dri2_surf;
xcb_get_geometry_cookie_t cookie;
xcb_get_geometry_reply_t *reply;
-   xcb_screen_iterator_t s;
xcb_generic_error_t *error;
xcb_drawable_t drawable;
-   xcb_screen_t *screen;
const __DRIconfig *config;
 
STATIC_ASSERT(sizeof(uintptr_t) == sizeof(native_surface));
@@ -228,16 +226,9 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay 
*disp, EGLint type,
 
dri2_surf->region = XCB_NONE;
if (type == EGL_PBUFFER_BIT) {
-  s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
-  screen = get_xcb_screen(s, dri2_dpy->screen);
-  if (!screen) {
- _eglError(EGL_BAD_ALLOC, "failed to get xcb screen");
- goto cleanup_surf;
-  }
-
   dri2_surf->drawable = xcb_generate_id(dri2_dpy->conn);
   xcb_create_pixmap(dri2_dpy->conn, conf->BufferSize,
-   dri2_surf->drawable, screen->root,
+   dri2_surf->drawable, dri2_dpy->screen->root,
dri2_surf->base.Width, dri2_surf->base.Height);
} else {
   if (!drawable) {
@@ -544,20 +535,10 @@ dri2_x11_do_authenticate(struct dri2_egl_display 
*dri2_dpy, uint32_t id)
 {
xcb_dri2_authenticate_reply_t *authenticate;
xcb_dri2_authenticate_cookie_t authenticate_cookie;
-   xcb_screen_iterator_t s;
-   xcb_screen_t *screen;
int ret = 0;
 
-   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
-
-   screen = get_xcb_screen(s, dri2_dpy->screen);
-   if (!screen) {
-  _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
-  return -1;
-   }
-
authenticate_cookie =
-  xcb_dri2_authenticate_unchecked(dri2_dpy->conn, screen->root, id);
+  xcb_dri2_authenticate_unchecked(dri2_dpy->conn, dri2_dpy->screen->root, 
id);
authenticate =
   xcb_dri2_authenticate_reply(dri2_dpy->conn, authenticate_cookie, NULL);
 
@@ -598,8 +579,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
xcb_dri2_connect_reply_t *connect;
xcb_dri2_connect_cookie_t connect_cookie;
xcb_generic_error_t *error;
-   xcb_screen_iterator_t s;
-   xcb_screen_t *screen;
char *driver_name, *loader_driver_name, *device_name;
const xcb_query_extension_reply_t *extension;
 
@@ -622,13 +601,7 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
   XCB_DRI2_MAJOR_VERSION,
   XCB_DRI2_MINOR_VERSION);
 
-   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
-   screen = get_xcb_screen(s, dri2_dpy->screen);
-   if (!screen) {
-  _eglLog(_EGL_WARNING, "DRI2: failed to get xcb screen");
-  return EGL_FALSE;
-   }
-   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, screen->root,
+   connect_cookie = xcb_dri2_connect_unchecked(dri2_dpy->conn, 
dri2_dpy->screen->root,
XCB_DRI2_DRIVER_TYPE_DRI);
 
xfixes_query =
@@ -720,7 +693,6 @@ static EGLBoolean
 dri2_x11_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
  _EGLDisplay *disp, bool supports_preserved)
 {
-   xcb_screen_iterator_t s;
xcb_depth_iterator_t d;
xcb_visualtype_t *visuals;
int i, j, count;
@@ -732,8 +704,7 @@ dri2_x11_add_configs_for_visuals(struct dri2_egl_display 
*dri2_dpy,
   EGL_NONE
};
 
-   s = xcb_setup_roots_iterator(xcb_get_setup(dri2_dpy->conn));
-   d = xcb_screen_allowed_depths_iterator(get_xcb_screen(s, dri2_dpy->screen));
+   d = xcb_screen_allowed_depths_iterator(dri2_dpy->screen);
count = 0;
 
surface_type =
@@ -1183,17 +1154,19 @@ static EGLBoolean
 

[Mesa-dev] [PATCH v2 1/3] egl/x11: factor out dri2_get_xcb_connection()

[Emil Velikov]

From: Emil Velikov 

Identical throughout dri2, dri3 and drisw. Next patch will add more
common code, so rather than duplicating it factor out the function.

Note: this also sets eglError on failure. Something that's quite
inconsistent throughout the codebase.

v2: Call xcb_disconnect() on error (Eric)

Note: use xcb_disconnect() even in the xcb_connection_has_error() case
as per the manual:
... memory will not be freed until xcb_disconnect...

Signed-off-by: Emil Velikov 
Reviewed-by: Eric Engestrom  (v1)
---
 src/egl/drivers/dri2/platform_x11.c | 63 -
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index 92c4ead..00613d9 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1180,13 +1180,10 @@ static const __DRIextension *swrast_loader_extensions[] 
= {
 };
 
 static EGLBoolean
-dri2_initialize_x11_swrast(_EGLDriver *drv, _EGLDisplay *disp)
+dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,
+struct dri2_egl_display *dri2_dpy)
 {
-   struct dri2_egl_display *dri2_dpy;
-
-   dri2_dpy = calloc(1, sizeof *dri2_dpy);
-   if (!dri2_dpy)
-  return _eglError(EGL_BAD_ALLOC, "eglInitialize");
+   const char *msg;
 
disp->DriverData = (void *) dri2_dpy;
if (disp->PlatformDisplay == NULL) {
@@ -1200,10 +1197,30 @@ dri2_initialize_x11_swrast(_EGLDriver *drv, _EGLDisplay 
*disp)
}
 
if (!dri2_dpy->conn || xcb_connection_has_error(dri2_dpy->conn)) {
-  _eglLog(_EGL_WARNING, "DRI2: xcb_connect failed");
-  goto cleanup_dpy;
+  msg = "xcb_connect failed";
+  goto disconnect;
}
 
+   return EGL_TRUE;
+disconnect:
+   if (disp->PlatformDisplay == NULL)
+  xcb_disconnect(dri2_dpy->conn);
+
+   return _eglError(EGL_BAD_ALLOC, msg);
+}
+
+static EGLBoolean
+dri2_initialize_x11_swrast(_EGLDriver *drv, _EGLDisplay *disp)
+{
+   struct dri2_egl_display *dri2_dpy;
+
+   dri2_dpy = calloc(1, sizeof *dri2_dpy);
+   if (!dri2_dpy)
+  return _eglError(EGL_BAD_ALLOC, "eglInitialize");
+
+   if (!dri2_get_xcb_connection(drv, disp, dri2_dpy))
+  goto cleanup_dpy;
+
/*
 * Every hardware driver_name is set using strdup. Doing the same in
 * here will allow is to simply free the memory at dri2_terminate().
@@ -1308,21 +1325,8 @@ dri2_initialize_x11_dri3(_EGLDriver *drv, _EGLDisplay 
*disp)
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
 
-   disp->DriverData = (void *) dri2_dpy;
-   if (disp->PlatformDisplay == NULL) {
-  dri2_dpy->conn = xcb_connect(0, _dpy->screen);
-  dri2_dpy->own_device = true;
-   } else {
-  Display *dpy = disp->PlatformDisplay;
-
-  dri2_dpy->conn = XGetXCBConnection(dpy);
-  dri2_dpy->screen = DefaultScreen(dpy);
-   }
-
-   if (!dri2_dpy->conn || xcb_connection_has_error(dri2_dpy->conn)) {
-  _eglLog(_EGL_WARNING, "DRI3: xcb_connect failed");
+   if (!dri2_get_xcb_connection(drv, disp, dri2_dpy))
   goto cleanup_dpy;
-   }
 
if (!dri3_x11_connect(dri2_dpy))
   goto cleanup_conn;
@@ -1421,21 +1425,8 @@ dri2_initialize_x11_dri2(_EGLDriver *drv, _EGLDisplay 
*disp)
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
 
-   disp->DriverData = (void *) dri2_dpy;
-   if (disp->PlatformDisplay == NULL) {
-  dri2_dpy->conn = xcb_connect(0, _dpy->screen);
-  dri2_dpy->own_device = true;
-   } else {
-  Display *dpy = disp->PlatformDisplay;
-
-  dri2_dpy->conn = XGetXCBConnection(dpy);
-  dri2_dpy->screen = DefaultScreen(dpy);
-   }
-
-   if (!dri2_dpy->conn || xcb_connection_has_error(dri2_dpy->conn)) {
-  _eglLog(_EGL_WARNING, "DRI2: xcb_connect failed");
+   if (!dri2_get_xcb_connection(drv, disp, dri2_dpy))
   goto cleanup_dpy;
-   }
 
if (!dri2_x11_connect(dri2_dpy))
   goto cleanup_conn;
-- 
2.10.2

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

Re: [Mesa-dev] [PATCH v2 2/2] egl/android: Use gralloc::lock_ycbcr for resolving YUV formats (v2)

[Emil Velikov]

On 21 November 2016 at 06:50, Tomasz Figa  wrote:
> On Thu, Nov 10, 2016 at 5:48 PM, Tapani Pälli  wrote:
>>
>>
>> On 11/10/2016 09:55 AM, Tomasz Figa wrote:
>>>
>>> There is an interface that can be used to query YUV buffers for their
>>> internal format. Specifically, if gralloc:lock_ycbcr() is given no SW
>>> usage flags, it's supposed to return plane offsets instead of pointers.
>>> Let's use this interface to implement support for YUV formats in Android
>>> EGL backend.
>>
>>
>> IMO lock_ycbcr() is not very intuitive API (read: is horrible) to query
>> surface information, would be nice to have something generic along the lines
>> of 'resolve_format' or 'query_format' but I don't know how feasible it would
>> be to push for such new interface ..
>
> Yeah, I agree that it's not very intuitive, however it does its job
> pretty well without making Android depend on some DRM-specific stuff
> (which I personally don't have anything against...). Since I'm only a
> ChromeOS mortal, I can't really tell what we could do about it,
> though. In any case, any change would only affect further Android
> versions, so we still need to support the old API for current ones.
>
>>
>> both of these patches look good to me;
>> Reviewed-by: Tapani Pälli 
>>
>
> Anyway, I think we can commit this for the time being, so:
>
Rob H, Chih-Wei please check that your gbm_gralloc/drm_gralloc have
the [simple implementation [1] of] gralloc:lock_ycbcr. Without it YUV
be fun.

> Gentle ping. :)
>
The RGB888 patch and this series is in master.

Thanks for the reminder!
Emil

[1] 
https://android.googlesource.com/platform/external/drm_gralloc/+/a43d9523c050b3ddb40190bb8de7920b7521b6ca
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel: aubinator: Fix resource leak in gen_spec_load_from_path

[Mun, Gwan-gyeong]

Hi, Lionel Landwerlin

Thanks for reviewing. As I do not have the permission of mesa git, I
can not push it.

If you have permission for git push, can you push this patch?

Best regards,

Gwan-gyeong.

2016-11-21 20:20 GMT+09:00 Lionel Landwerlin :
> Thanks!
>
> Reviewed-by: Lionel Landwerlin 
>
>
> On 20/11/16 07:07, Mun Gwan-gyeong wrote:
>>
>> This fixes resource leak in gen_spec_load_from_path XML_ParserCreate
>> failure path
>>
>> CID 1373564
>>
>> Signed-off-by: Mun Gwan-gyeong 
>> ---
>>   src/intel/tools/decoder.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c
>> index 6bd02bf..633251a 100644
>> --- a/src/intel/tools/decoder.c
>> +++ b/src/intel/tools/decoder.c
>> @@ -539,6 +539,7 @@ gen_spec_load_from_path(const struct gen_device_info
>> *devinfo,
>>  XML_SetUserData(ctx.parser, );
>>  if (ctx.parser == NULL) {
>> fprintf(stderr, "failed to create parser\n");
>> +  fclose(input);
>> free(filename);
>> return NULL;
>>  }
>
>
>



-- 
Mun, Gwan-gyeong
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev