Re: [Mesa-dev] [PATCH 1/2] panfrost: Initial stub for Panfrost driver

2019-01-31 Thread Eric Anholt
Alyssa Rosenzweig  writes:

> This patch adds an initial stub for the Gallium driver, containing
> simple screen functions and the majority of the driver headers but no
> actual functionality. It further adds the winsys glue for linking in
> this stub driver via kmsro on Rockchip/Amlogic boards.

> +static int
> +panfrost_get_param(struct pipe_screen *screen, enum pipe_cap param)
> +{
> +switch (param) {
> +case PIPE_CAP_NPOT_TEXTURES:
> +case PIPE_CAP_MIXED_FRAMEBUFFER_SIZES:
> +case PIPE_CAP_MIXED_COLOR_DEPTH_BITS:
> +return 1;
> +
> +case PIPE_CAP_SM3:
> +return 1;
> +
> +case PIPE_CAP_POINT_SPRITE:
> +return 1;
> +
> +case PIPE_CAP_MAX_RENDER_TARGETS:
> +return PIPE_MAX_COLOR_BUFS;
> +
> +case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS:
> +return 1;
> +
> +case PIPE_CAP_OCCLUSION_QUERY:
> +case PIPE_CAP_QUERY_TIME_ELAPSED:
> +case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
> +return 1; /* TODO: Queries */
> +
> +case PIPE_CAP_TEXTURE_MIRROR_CLAMP:
> +return 1;
> +
> +case PIPE_CAP_TEXTURE_SWIZZLE:
> +return 1;
> +
> +case PIPE_CAP_TEXTURE_BORDER_COLOR_QUIRK:
> +return 0;
> +
> +case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
> +case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
> +case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
> +return 13;
> +
> +case PIPE_CAP_BLEND_EQUATION_SEPARATE:
> +return 1;
> +
> +case PIPE_CAP_INDEP_BLEND_ENABLE:
> +return 1;
> +
> +case PIPE_CAP_INDEP_BLEND_FUNC:
> +return 1;
> +
> +case PIPE_CAP_TGSI_FS_COORD_ORIGIN_UPPER_LEFT:
> +case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
> +case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER:
> +case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_INTEGER:
> +return 1;
> +
> +case PIPE_CAP_DEPTH_CLIP_DISABLE:
> +return 1;
> +
> +case PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS:
> +return 0; /* no streamout */
> +
> +case PIPE_CAP_MAX_STREAM_OUTPUT_SEPARATE_COMPONENTS:
> +case PIPE_CAP_MAX_STREAM_OUTPUT_INTERLEAVED_COMPONENTS:
> +return 16 * 4;
> +
> +case PIPE_CAP_MAX_GEOMETRY_OUTPUT_VERTICES:
> +case PIPE_CAP_MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS:
> +return 1024;
> +
> +case PIPE_CAP_MAX_VERTEX_STREAMS:
> +return 1;
> +
> +case PIPE_CAP_PRIMITIVE_RESTART:
> +return 0; /* We don't understand this yet */
> +
> +case PIPE_CAP_SHADER_STENCIL_EXPORT:
> +return 1;
> +
> +case PIPE_CAP_TGSI_INSTANCEID:
> +case PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR:
> +case PIPE_CAP_START_INSTANCE:
> +return 0; /* TODO: Instances */
> +
> +case PIPE_CAP_SEAMLESS_CUBE_MAP:
> +case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
> +return 1;
> +
> +case PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS:
> +return 256; /* for GL3 */
> +
> +case PIPE_CAP_MIN_TEXEL_OFFSET:
> +return -8;
> +
> +case PIPE_CAP_MAX_TEXEL_OFFSET:
> +return 7;
> +
> +case PIPE_CAP_CONDITIONAL_RENDER:
> +return 1;
> +
> +case PIPE_CAP_TEXTURE_BARRIER:
> +return 0;
> +
> +case PIPE_CAP_FRAGMENT_COLOR_CLAMPED:
> +case PIPE_CAP_VERTEX_COLOR_UNCLAMPED: /* draw module */
> +case PIPE_CAP_VERTEX_COLOR_CLAMPED: /* draw module */
> +return 1;
> +
> +case PIPE_CAP_MIXED_COLORBUFFER_FORMATS:
> +return 0;
> +
> +case PIPE_CAP_GLSL_FEATURE_LEVEL:
> +return 330;
> +
> +case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
> +case PIPE_CAP_TGSI_TEX_TXF_LZ:
> +return 0;
> +
> +case PIPE_CAP_COMPUTE:
> +return 0;
> +
> +case PIPE_CAP_USER_VERTEX_BUFFERS: /* XXX XXX */
> +case PIPE_CAP_RESOURCE_FROM_USER_MEMORY:
> +return 0;
> +
> +case PIPE_CAP_STREAM_OUTPUT_PAUSE_RESUME:
> +case PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS:
> +case PIPE_CAP_TGSI_VS_LAYER_VIEWPORT:
> +case PIPE_CAP_DOUBLES:
> +case PIPE_CAP_INT64:
> +case PIPE_CAP_INT64_DIVMOD:
> +return 1;
> +
> +case PIPE_CAP_CONSTANT_BUFFER_OFFSET_ALIGNMENT:
> +return 16;
> +
> +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:
> +case PIPE_CAP_VERTEX_ELEMENT_SRC_OFFSET_4BYTE_ALIGNED_ONLY:
> +case PIPE_CAP_TEXTURE_MULTISAMPLE:
> +return 

Re: [Mesa-dev] [PATCH 4/4] vc4: Fix leak

2019-01-29 Thread Eric Anholt
Ernestas Kulik  writes:

> Reported by Coverity: in the case where there exist hardware and
> non-hardware queries, the code does not jump to err_free_query and leaks
> the query.
>
> CID: 1430194
> Signed-off-by: Ernestas Kulik 

I just found these deep in my inbox, and pushed them.


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


Re: [Mesa-dev] [PATCH] vc4: Declare the last cpu pointer as being modified in NEON asm.

2019-01-29 Thread Eric Anholt
Emil Velikov  writes:

> From: Emil Velikov 
>
> Earlier commit addressed 7 of the 8 instances available.
>
> Cc: Carsten Haitzler (Rasterman) 
> Cc: Eric Anholt 
> Fixes: 300d3ae8b14 ("vc4: Declare the cpu pointers as being modified in NEON 
> asm.")
> Signed-off-by: Emil Velikov 
> ---
> Gents my ARM assembly is limited so a confirmation or review would be
> greatly appreciated.

Thanks, I must have lost a hunk in resolving conflicts.  Rebased to
master, and running it through the CTS now.


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


Re: [Mesa-dev] [PATCH] meson: add toggle for TLS support in GLX

2019-01-29 Thread Eric Anholt
Rich Felker  writes:

> On Tue, Jan 29, 2019 at 08:29:32AM +0100, Patrick Steinhardt wrote:
>> On Mon, Jan 28, 2019 at 02:09:18PM -0800, Dylan Baker wrote:
>> > Quoting Adam Jackson (2019-01-28 09:00:13)
>> > > On Sat, 2019-01-26 at 13:56 +0100, Patrick Steinhardt wrote:
>> > > 
>> > > > Unfortunately, I'm not aware of any easy way to do this. The
>> > > > problem is not due to the compiler, as the program compiles and
>> > > > links just fine with musl libc. Instead, this is a runtime issue
>> > > > that happens when dlopen'ing shared objects with the
>> > > > "initial-exec" TLS model. And as far as I understand, the error
>> > > > only occurs if the PT_TLS segment size of the loaded shared
>> > > > object exceeds the space preallocated for it by the libc.
>> > > 
>> > > You can hit that failure mode with glibc too, it just requires many
>> > > more things trying to use TLS. Possibly we should reconsider whether
>> > > initial-exec is really that important for performance anymore or if we
>> > > can get away with global-dynamic.
>> > > 
>> > > - ajax
>> > 
>> > I seem to remember that this is problematic for other's than just musl, 
>> > and that
>> > our use of initial-exec works because of implementation details in glibc.
>> > 
>> > Like Eric, I really don't want this option added, I worked very hard to 
>> > *NOT*
>> > add this because there is a right answer in ever case, either you have 
>> > working
>> > TLS or you don't. The real frustration for me here is that musl absolutely
>> > refuses to give us any way to identify it and turn off TLS automatically 
>> > (which
>> > is what we really want in this case).
>> 
>> Cc'ing Rich Felker, who is the maintainer of musl.
>
> Thanks. I think this is a good example of why we don't do that --
> turning off TLS is the wrong "fix", and potentially introduces a
> different bug: as best I can tell, the !TLS case has asm that assumes
> pthread_getspecific does not clobber floating point/vector registers
> that are call-clobbered per the ABI. This is likely true in practice,
> but really it's up to the compiler that built musl whether it uses
> them; there's no contract not to.
>
> What we would like from the musl side is for TLS to work correctly,
> including when mesa is dynamically loaded. This avoids problems like
> the above and should perform much better. I've been trying to get
> feedback on how to proceed with this for a while, with no response:
> https://bugs.freedesktop.org/show_bug.cgi?id=35268
>
> Are the mesa folks open to patches like what was discussed there?

Maintaining less code sounds great if the new TLS models actually work
out.  We'd just want to see data indicating that the changes haven't
hurt performance on affected architectures on glibc.


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


Re: [Mesa-dev] Merge requests have bad and ambiguous names

2019-01-28 Thread Eric Anholt
Marek Olšák  writes:

> Hi,
>
> People don't prefix merge request names with the component they are
> changing, so unrelated people have to open merge requests they don't really
> want to look at.
>
> Could you start adding the component prefix like we do for commits?

Not sure if you've noticed, there are labels on the MRs that let you
mark the affected areas better than prefixes do (like "anv: Implement
VK_EXT_buffer_device_address" is actually SPIRV, NIR, intel common, and
anv, not just anv).


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


Re: [Mesa-dev] [PATCH 2/4] pipe-loader: Fallback to kmsro driver when no matching driver name found

2019-01-28 Thread Eric Anholt
Eric Anholt  writes:

> [ Unknown signature status ]
> Rob Herring  writes:
>
>> If we can't find a driver matching by name, then use the kmsro driver.
>> This removes the need for needing a driver descriptor for every possible
>> KMS driver.
>>
>> Signed-off-by: Rob Herring 
>
> I've pushed patch 1 and 2, and the hx8357d rebase.

And a followup for the rest of tinydrm:

https://gitlab.freedesktop.org/mesa/mesa/merge_requests/165


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


Re: [Mesa-dev] [PATCH 2/4] pipe-loader: Fallback to kmsro driver when no matching driver name found

2019-01-28 Thread Eric Anholt
Rob Herring  writes:

> If we can't find a driver matching by name, then use the kmsro driver.
> This removes the need for needing a driver descriptor for every possible
> KMS driver.
>
> Signed-off-by: Rob Herring 

I've pushed patch 1 and 2, and the hx8357d rebase.


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


Re: [Mesa-dev] [PATCH] mesa: GLES2 fix for OES float/half-float textures.

2019-01-28 Thread Eric Anholt
Erik Faye-Lund  writes:

> On Fri, 2019-01-25 at 13:41 -0800, Eric Anholt wrote:
>> Nick Kreeger  writes:
>> 
>> > The OES_texture* extensions for float and half-float are valid when
>> > GLES2 is present w/ the matching
>> > OES_texture_float/OES_texture_half_float extensions. This fix
>> > ensures
>> > that these formats are valid for this configuration.
>> 
>> I don't see OES_texture_float.txt specifying these tokens as
>> additions
>> to the list of valid base internal formats (table 3.8 of the GLES2
>> spec), so I think this change is invalid.  Sized internalformats were
>> introduced with GLES3.
>> 
>
> Hmm, seems you're right. I suppose this should be reverted? Nick, what
> do you think?

I don't see it in the tree, though.


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


Re: [Mesa-dev] [PATCH] mesa: GLES2 fix for OES float/half-float textures.

2019-01-25 Thread Eric Anholt
Nick Kreeger  writes:

> The OES_texture* extensions for float and half-float are valid when
> GLES2 is present w/ the matching
> OES_texture_float/OES_texture_half_float extensions. This fix ensures
> that these formats are valid for this configuration.

I don't see OES_texture_float.txt specifying these tokens as additions
to the list of valid base internal formats (table 3.8 of the GLES2
spec), so I think this change is invalid.  Sized internalformats were
introduced with GLES3.


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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-25 Thread Eric Anholt
Rob Clark  writes:

> I guess as discovered with
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/47 maybe we
> should wait to turn on merging MRs via web until we have at least some
> basic build-test CI wired up.. the downside is slower 'maintainer'
> response (if I am working on some long running branch I tend to
> postpone pushing changes from others until I am in a good state to
> rebase, but at least when I merge things via cmdline I do a build test
> and sanity test on at at least one driver ;-))

We should be days, not weeks, away from having build-test CI wired up.
I actually thought Igalia was supposed to be working on it again
already, but then Eric Engestrom started working on a new variation, so
I'm not sure what's going on.

I'm skeptical that we need to do an MR policy change given that
build-breakage and especially unit-test-breakage has been a recurring
issue in the absence of CI, even before MRs.


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


Re: [Mesa-dev] [PATCH] meson: add toggle for TLS support in GLX

2019-01-25 Thread Eric Anholt
Patrick Steinhardt  writes:

> The musl libc library does not have any support for the
> "initial-exec" TLS model and is unlikely to ever implement it.
> Thus, TLS support in GLX has been turned off in musl-based
> distributions to work around problems when dlopen'ing drivers.
> While this is easily possible using the autoconf build system by
> passing `--disable-glx-tls`, meson does not yet have such an
> option.
>
> Add a new toggle "glx-tls" that defaults to `true` to gain parity
> with autoconf. If disabled, `GLX_USE_TLS` will not be defined and
> thus mesa will be built without TLS support.

Could you compile-test instead of having an option for people to get
wrong?


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


Re: [Mesa-dev] [PATCH] mapi: print function declarations for shared glapi

2019-01-25 Thread Eric Anholt
Emil Velikov  writes:

> From: Emil Velikov 
>
> Earlier commit aimed to remove unneeded function declarations. Namely
> OpenGL entrypoints which are not applicable for OpenGLES*
>
> Although it did not consider the shared glapi which needs all,
> including hidden ones. Resulting in warning/errors like the following
>
> ../build/src/mapi/shared-glapi/glapi_mapi_tmp.h:26014:15:
> error: no previous prototype for ‘shared_dispatch_stub_1414’ 
> [-Werror=missing-prototypes]
>
> This patch addressed that.

Applied, to fix my builds.


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


Re: [Mesa-dev] [RFC 2/4] nir: Add a new ALU nir_op_imad

2019-01-25 Thread Eric Anholt
Eduardo Lima Mitev  writes:

> ir3 compiler has an integer multiply-add instruction (IMAD_S24)
> that is used for different offset calculations in the backend.
> Since we intend to move some of these calculations to NIR, we need
> a new ALU op that can represent it.
> ---
>  src/compiler/nir/nir_opcodes.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
> index d32005846a6..b61845fd514 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -754,6 +754,7 @@ def triop_horiz(name, output_size, src1_size, src2_size, 
> src3_size, const_expr):
> [tuint, tuint, tuint], "", const_expr)
>  
>  triop("ffma", tfloat, "src0 * src1 + src2")
> +triop("imad", tint, "src0 * src1 + src2")

The constant-folding expression should be corrected to what the backend
operation actually does, and you should probably call it imad24 or
something in that case.


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


Re: [Mesa-dev] [RFC 1/4] nir: Add a new intrinsic 'load_image_stride'

2019-01-25 Thread Eric Anholt
Eduardo Lima Mitev  writes:

> This is an internal intrinsic intended to be injected by a
> (freedreno-specific) 'lower_sampler_io' pass that will be introduced
> later in this series; and consumed by ir3_compiler_nir.
>
> The intrinsic will load in SSA values for various constants
> for images (image_dims), namely the format's bytes-per-pixel,
> y-stride and z-stride; for which the backend compiler will emit
> the corresponding uniforms.
>
> const_index[0] is the image index, and const_index[1] is the index
> into image_dims' register file for a given image: 0 for bpp, 1 to
> y-stride and 2 for z-stride.

Can you move this documentation of the meaning of the intrinsic into a
comment in the file?


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


Re: [Mesa-dev] [PATCH 2/4] pipe-loader: Fallback to kmsro driver when no matching driver name found

2019-01-24 Thread Eric Anholt
Eric Anholt  writes:

> [ Unknown signature status ]
> Rob Herring  writes:
>
>> If we can't find a driver matching by name, then use the kmsro driver.
>> This removes the need for needing a driver descriptor for every possible
>> KMS driver.
>>
>> Signed-off-by: Rob Herring 
>
> Oh, very nice.
>
> There's some risk here -- for example, vkms (with prime enabled, which
> it doesn't currently) will fail badly at prime sharing buffers with
> hardware drivers doing WC mappings of buffers.  But I guess we should be
> preventing that at the kernel level when we enable prime, and having
> userspace maybe test creating a shared resource when loading kmsro with
> a particular backend.

That is to say,

Reviewed-by: Eric Anholt 

If you've got display-only KMS and a render node, we should make sure
that Mesa glues those two together.


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


Re: [Mesa-dev] [PATCH 3/4] kmsro: Add etnaviv renderonly support

2019-01-24 Thread Eric Anholt
Rob Herring  writes:

> Enable using etnaviv for KMS renderonly. This still needs KMS driver
> name mapping to kmsro to be used automatically.
>
> Signed-off-by: Rob Herring 

> diff --git a/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c 
> b/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c
> index 4448150cc0c6..e086c0858f05 100644
> --- a/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c
> +++ b/src/gallium/winsys/kmsro/drm/kmsro_drm_winsys.c
> @@ -27,6 +27,7 @@
>  
>  #include "kmsro_drm_public.h"
>  #include "vc4/drm/vc4_drm_public.h"
> +#include "etnaviv/drm/etnaviv_drm_public.h"
>  #include "xf86drm.h"
>  
>  #include "pipe/p_screen.h"
> @@ -34,22 +35,39 @@
>  
>  struct pipe_screen *kmsro_drm_screen_create(int fd)
>  {
> +   struct pipe_screen *screen = NULL;
> struct renderonly ro = {
> +  .kms_fd = fd,
> +  .gpu_fd = -1,
> +   };
> +
> +#if defined(GALLIUM_VC4)
> +   ro.gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER);
> +   if (ro.gpu_fd >= 0) {
>/* Passes the vc4-allocated BO through to the KMS-only DRM device using
> * PRIME buffer sharing.  The VC4 BO must be linear, which the SCANOUT
> * flag on allocation will have ensured.
> */
> -  .create_for_resource = renderonly_create_gpu_import_for_resource,
> -  .kms_fd = fd,
> -  .gpu_fd = drmOpenWithType("vc4", NULL, DRM_NODE_RENDER),
> -   };
> +  ro.create_for_resource = renderonly_create_gpu_import_for_resource,
> +  screen = vc4_drm_screen_create_renderonly();
> +  if (!screen)
> + close(ro.gpu_fd);
> +
> +  return screen;
> +   }
> +#endif
>  
> -   if (ro.gpu_fd < 0)
> -  return NULL;
> +#if defined(GALLIUM_ETNAVIV)
> +   ro.gpu_fd = drmOpenWithType("etnaviv", NULL, DRM_NODE_RENDER);
> +   if (ro.gpu_fd >= 0) {
> +  ro.create_for_resource = 
> renderonly_create_kms_dumb_buffer_for_resource,
> +  screen = etna_drm_screen_create_renderonly();
> +  if (!screen)
> + close(ro.gpu_fd);
>  
> -   struct pipe_screen *screen = vc4_drm_screen_create_renderonly();
> -   if (!screen)
> -  close(ro.gpu_fd);
> +  return screen;
> +   }
> +#endif

Would it make more sense to open the first render node once, then check
if its name matches any of the drivers we support and calling their
setup function?

Either way, this would have my ack.


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


Re: [Mesa-dev] [PATCH 2/4] pipe-loader: Fallback to kmsro driver when no matching driver name found

2019-01-24 Thread Eric Anholt
Rob Herring  writes:

> If we can't find a driver matching by name, then use the kmsro driver.
> This removes the need for needing a driver descriptor for every possible
> KMS driver.
>
> Signed-off-by: Rob Herring 

Oh, very nice.

There's some risk here -- for example, vkms (with prime enabled, which
it doesn't currently) will fail badly at prime sharing buffers with
hardware drivers doing WC mappings of buffers.  But I guess we should be
preventing that at the kernel level when we enable prime, and having
userspace maybe test creating a shared resource when loading kmsro with
a particular backend.


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


Re: [Mesa-dev] [PATCH 1/4] pl111: Rename the pl111 driver to "kmsro".

2019-01-24 Thread Eric Anholt
Rob Herring  writes:

> From: Eric Anholt 
>
> The vc4 driver can do prime sharing to many different KMS-only devices,
> such as the various tinydrm drivers for SPI-attached displays.  Rename the
> driver away from "pl111" to represent what it will actually support:
> various sorts of KMS displays with the renderonly layer used to attach a
> GPU.
>
> Acked-by: Emil Velikov 
> Signed-off-by: Rob Herring 

Looks like you missed the other r-bs?

Reviewed-by: Eric Engestrom 
Reviewed-by: Christian Gmeiner 


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


Re: [Mesa-dev] [PATCH v2 1/2] gallium: Enable ASIMD/NEON on aarch64.

2019-01-24 Thread Eric Anholt
Matt Turner  writes:

> NEON (now called ASIMD) is available on all aarch64 CPUs. Our code was
> missing an aarch64 path, leading to util_cpu_caps.has_neon always being
> false on aarch64.
> ---
> Here's the simpler patch to just always enable NEON on aarch64. It suits
> my purposes, but I can imagine that you may prefer the original patch if
> you ever want to do runtime detection of other features on aarch64.

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH 1/2] gallium: Enable aarch64 NEON CPU detection.

2019-01-23 Thread Eric Anholt
Matt Turner  writes:

> NEON (now called ASIMD) is available on all aarch64 CPUs. It seems that
> our code was missing an aarch64 path, leading to util_cpu_caps.has_neon
> always being false on aarch64. I think that means that the NEON tiling
> code in vc4 would not be enabled on aarch64 (vc4_load_lt_image_neon,
> etc).
> ---
> I have very little clue about aarch64 ABIs, so I don't know if there's
> another case that needs to be handled -- aarch32 maybe? Does
> PIPE_ARCH_AARCH64 just mean ARMv8 and so we should check something else
> for the ABI and choose Elf{32,64} based on that?

Do we actually need to do runtime detection?  It sounds like "standard"
armv8 is guaranteed NEON:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/CEGDJGGC.html

For vc4, the aarch64 build uses neon in the _base version of the tiling
implementation.


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


Re: [Mesa-dev] [PATCH] mesa: add EXT_debug_label support

2019-01-18 Thread Eric Anholt
Timothy Arceri  writes:

> On 11/12/18 5:11 pm, Ian Romanick wrote:
>> On 12/10/18 5:52 PM, Timothy Arceri wrote:
>>> On 11/12/18 11:35 am, Ian Romanick wrote:
 It seems like someone already sent out patches to implement this, and we
 decided to not take it for some reason.  Maybe it was Rob?
>>>
>>> I discovered a thread from the beginning of 2017 titled "feature.txt &
>>> EXT_debug_label extension". But couldn't find any implementation.
>>>
>>> There was a reply from yourself, but it seems incorrect to me:
>>>
>>> "I checked both extensions, and they're not "just" aliases.  The EXT adds
>>> a single function with an enum to select the kind of object.  The KHR
>>> adds a function per kind of object.  It would be easy enough to add, but
>>> it seems more valuable to suggest the developer use the more broadly
>>> supported extension."
>> 
>> That's weird for a couple reasons.  One, that's not even the discussion
>> that I was thinking of.  I'll check in the morning to see if I can find
>> it.  Two, I was clearly full of it... I really don't see how I came that
>> conclusion.  I don't even see any other related extensions that I could
>> have been confusing either thing with.
>
> So do you think we should push this?

I don't see any piglit or CTS tests for this extension.


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


Re: [Mesa-dev] [PATCH] gallium: add pipe_grid_info::last_block

2019-01-16 Thread Eric Anholt
Marek Olšák  writes:

> From: "Jiang, Sonny" 
>
> and add radeonsi support. This will be used by radeonsi internally.
>
> Signed-off-by: Sonny Jiang 

It's still strange to me to be proposing changes to gallium core with no
users of the change.


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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-15 Thread Eric Anholt
Daniel Stone  writes:

> Hi,
>
> On Tue, 15 Jan 2019 at 12:21, Rob Clark  wrote:
>> On Tue, Jan 15, 2019 at 1:02 AM Tapani Pälli  wrote:
>> > On 1/14/19 2:36 PM, Daniel Stone wrote:
>> > > On Fri, 11 Jan 2019 at 17:05, Jason Ekstrand  
>> > > wrote:
>> > > In other projects, we looked for ways to apply the tags and ended up
>> > > concluding that they didn't bring enough value to make it worthwhile.
>> > > I don't know if that holds for Mesa, but it would be better to start
>> > > with an actual problem statement - what value does R-b bring and how?
>> > > - then look at ways to solve that problem, rather than just very
>> > > directly finding a way to insert that literal text string into every
>> > > commit message.
>> >
>> > IMO it brings some 'shared responsibility' for correctness of the patch
>
> Oh, no doubt - we certainly haven't abandoned thorough review! So far
> we haven't seen that compromised by not having a name in the commit
> message.
>
>> > and quickly accessible information on who were looking at the change. So
>> > ideally later when filing bug against commit/series there would be more
>> > people than just the committer that should take a look at the possible
>> > regressions. At least in my experience people filing bugs tend to often
>> > also CC the reviewer.
>
> Yeah, that's really helpful. So maybe a useful flow - assuming we
> eventually switch to GitLab issues - would be the ability to associate
> an issue with a commit, which could then automatically drag in people
> who commented on the MR which landed that commit, as well as (at
> least) the reporter of the issue(s) fixed by that MR. That would need
> some kind of clever - probably at least semi-manual - filtering to
> make sure it wasn't just spamming the world, but it's at least a
> starting point.
>
>> +1 .. and also it is nice to see things like Reported-by/Reviewed-by
>> without having to go search somewhere else (ie. outside of git/tig)
>
> My question would again be what value that brings you. Do you just
> like seeing the name there, or do you go poke the people on IRC, or
> follow up via email, or ... ? Again I personally go look through the
> original review to see what came up during that first, but everyone's
> different, so I'm just trying to understand what you actually do with
> that information, so we can figure out if there's a better way to do
> things for everyone rather than just blindly imitating what came
> before.

I've participated in adding Reported-bys, but I've never seen the use.
It felt like "we could record this information, so we should!" rather
than solving a problem.

I've found little use in ccing reviewers on followups, except for
trivial stuff like compiler warnings.  I propose that the solution for
compiler warnings should be CI that prevents you from merging new
compiler warnings anyway.

Basically, I feel like the pain points in the MR process (amending and
re-pushing before clicking "merge") are pre-existing pain points in our
process, slightly amplified.

>> (ofc it would be pretty awesome incentive to switch to gitlab issues
>> if gitlab could automate adding Reported-by tags for MR's associated
>> with an issue.. but I guess checkbox to add Reviewed-by tag would
>> already make my day)
>
> I saw this the other day, which might be more incentive:
> https://csoriano.pages.gitlab.gnome.org/csoriano-blog/post/2019-01-07-issue-handling-automation/

Automatic needinfo closing?  Sign me up.


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


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2019-01-11 Thread Eric Anholt
Gert Wollny  writes:

> Since Meson will eventually be the only build system deprecate autotools
> now. It can still be used by invoking configure with the flag
>   --enable-autotools
>
> Signed-off-by: Gert Wollny 

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH 05/12] nir: rename global/local to private/function memory

2019-01-11 Thread Eric Anholt
Jason Ekstrand  writes:

> On Fri, Jan 11, 2019 at 11:11 AM Kenneth Graunke 
> wrote:
>
>> On Friday, January 11, 2019 8:33:41 AM PST Jason Ekstrand wrote:
>> > On Fri, Jan 11, 2019 at 10:19 AM Kenneth Graunke wrote:
>> > > Those names (nir_var_func_local, nir_var_thread_local, and
>> > > nir_var_thread_global) make more sense to me than private/function.
>> > >
>> > > Another option is `nir_var_local_temp` and `nir_var_shader_temp`,
>> > > indicating that they're just temporary variables, and not anything
>> > > with special semantics like memory.  shader_temp would pair well with
>> > > the existing shader_in/shader_out, since they have the same scope.
>> > >
>> > > I might also consider adding 'mem' to variables representing memory.
>> > >
>> > > So that would look like...
>> > >
>> > >nir_var_shader_in
>> > >nir_var_shader_out
>> > >nir_var_shader_temp  (formerly local/function)
>> > >nir_var_local_temp   (formerly global/private)
>> > >
>> >
>> > Are those flipped?
>>
>> Gah!  Sorry.  Yes.
>>
>>nir_var_shader_in
>>nir_var_shader_out
>>nir_var_shader_temp  (formerly global/private)
>>nir_var_local_temp   (formerly local/function)
>>nir_var_uniform
>>nir_var_system_value
>>nir_var_mem_ubo  (added mem)
>>nir_var_mem_ssbo (added mem)
>>nir_var_mem_shared   (added mem)
>>nir_var_mem_global   (the new global memory type being introduced)
>>
>
> I can work with that.  I do think I'd mildly prefer function_temp over
> local_temp but I think the adding of _temp is an improvement.

I like the _temp suggestion a lot!  And I think I'm also mildly in favor
of function_temp.

(Also, thanks to taking naming seriously, to everyone involved here.
It's hard.)


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


Re: [Mesa-dev] [PATCH] st/glsl: refactor st_link_nir()

2019-01-04 Thread Eric Anholt
Timothy Arceri  writes:

> The functional change here is moving the nir_lower_io_to_scalar_early()
> calls inside st_nir_link_shaders() and moving the st_nir_opts() call
> after the call to nir_lower_io_arrays_to_elements().
>
> This fixes a bug with the following piglit test due to the current code
> not cleaning up dead code after we lower arrays. This was causing an
> assert in the new duplicate varyings link time opt introduced in
> 70be9afccb23.
>
> tests/spec/glsl-1.10/execution/vsfs-unused-array-member.shader_test
>
> Moving the nir_lower_io_to_scalar_early() calls also allows us to tidy
> up the code a little and merge some loops.

I love the reduction in stage tracking logic.

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH 1/2] vc4: Wire up core pipe_debug_callback

2018-12-20 Thread Eric Anholt
Eric Anholt  writes:

> [ Unknown signature status ]
> Rhys Kidd  writes:
>
>> Signed-off-by: Rhys Kidd 
>
> Does this actually do anything for us, though?  Shouldn't we be hooking
> up our perf_debug() calls to it?

I hooked up perf_debug(), extended the commit messages, and pushed.


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


Re: [Mesa-dev] [PATCH 08/16] freedreno: a2xx: enable early-Z testing

2018-12-19 Thread Eric Anholt
Jonathan marek  writes:

> Hi,
>
> I didn't verify it, but both r600 and a3xx disable earlyZ when alpha 
> test is enabled, so this is almost certainly right.
>
> We don't need to worry about the shader writing Z, it is not part of 
> OpenGL ES 2.0 and not implemented by the driver (although the hardware 
> should allow it).
>
> Why should we need to check if the shader does discards?

On a lot of hardware, early Z also does the depth write at that time.  I
see a3xx disables EZ when discards are present.  (There is a piglit test
for this)


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


Re: [Mesa-dev] [PATCH 01/16] glsl/nir: int constants as float for native_integers=false

2018-12-19 Thread Eric Anholt
Jonathan Marek  writes:

> Note: the backend must take care that uniform index is now a float

This makes me think that lowering ints to float should be done near the
end of the compile (followed by maybe an algebraic and a dce).  As is, I
think nir_lower_io() is going to do bad things to dereferences of i/o
arrays.

That said, it looks like this will be fixing way more than it regresses,
so I would go along with it.


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


Re: [Mesa-dev] [PATCH 08/16] freedreno: a2xx: enable early-Z testing

2018-12-19 Thread Eric Anholt
Jonathan Marek  writes:

> Enable earlyZ when alpha test is disabled.
>
> Signed-off-by: Jonathan Marek 
> ---
>  src/gallium/drivers/freedreno/a2xx/fd2_zsa.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/freedreno/a2xx/fd2_zsa.c 
> b/src/gallium/drivers/freedreno/a2xx/fd2_zsa.c
> index 64b31b677b..d3c19b4450 100644
> --- a/src/gallium/drivers/freedreno/a2xx/fd2_zsa.c
> +++ b/src/gallium/drivers/freedreno/a2xx/fd2_zsa.c
> @@ -49,7 +49,8 @@ fd2_zsa_state_create(struct pipe_context *pctx,
>   A2XX_RB_DEPTHCONTROL_ZFUNC(cso->depth.func); /* maps 1:1 */
>  
>   if (cso->depth.enabled)
> - so->rb_depthcontrol |= A2XX_RB_DEPTHCONTROL_Z_ENABLE;
> + so->rb_depthcontrol |= A2XX_RB_DEPTHCONTROL_Z_ENABLE |
> + COND(!cso->alpha.enabled, 
> A2XX_RB_DEPTHCONTROL_EARLY_Z_ENABLE);

Why when alpha test is disabled?  Should you also be checking if the
shader does discards?  How about if the shader writes Z, is anything
preventing early Z then?


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


Re: [Mesa-dev] PSA: Please send MRs to the mailing list

2018-12-17 Thread Eric Anholt
Jason Ekstrand  writes:

> I don't know if it was actually in the doc that Jordan wrote up but it's
> courteous of you to send a quick e-mail to the mailing list when you create
> a new MR so that people who aren't regularly trolling the list of MRs are
> at least aware that it exists.  Of the 20 MRs that have been posted so far,
> I think I'm the only one doing this.  I'm a big fan of MRs but I also don't
> want us MR fans to anger the list. :-)

The conclusion of the MR discussion was that notifying the list was
optional, I thought.

+
+  If the MR may have interest for most of the Mesa community, you can
+  send an email to the mesa-dev email list including a link to the MR.
+  Don't send the patch to mesa-dev, just the MR link.
+

The MR process is heavier-weight than I hoped for, given that we're
currently requiring rebasing tags into commit messages.  I don't want to
add more overhead to it if we don't have to.


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


Re: [Mesa-dev] last call for autotools

2018-12-17 Thread Eric Anholt
Eero Tamminen  writes:

> Hi,
>
> On 17.12.2018 8.08, Marek Olšák wrote:
> [...]
>> I think one of the serious usability issues is that environment 
>> variables such as CFLAGS, CXXFLAGS, LDFLAGS, and PKG_CONFIG_PATH are not 
>> saved by meson for future reconfigures.
>
> I don't know what Meson is supposed to do, but to me that would be
> a bug in a build tool.
>
> Re-configure is supposed to adapt SW to the changes in the build
> environment, and environment variables are part of that (along with
> command line options and SW installed to to the system).  Build
> configure tool deciding to "remember" some of those things instead
> of checking the new situation, seems like a great opportunity for
> confusion.

A user-triggered reconfigure, sure.  Recapture env vars then.  But "git
pull; ninja -C build" losing track of the configuration state is broken.
We don't have to specify all of your meson -Doption=state configuration
on every build, why should you need to specify your PKG_CONFIG_PATH
configure options on every build?


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


Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

2018-12-13 Thread Eric Anholt
Jason Ekstrand  writes:

> On Thu, Dec 13, 2018 at 11:07 AM Axel Davy  wrote:
>
>> On 13/12/2018 17:57, Mathias Fröhlich wrote:
>> > Hi,
>> > Initially it seemed to me that I am about the only one sticking with
>> mailing lists.
>> > And I personally feel like a too small contributor to really try to
>> influence your
>> > decisions too much. But these recent hand full of mails all tell me that
>> I am not
>> > that alone. I personally did contribute to several projects during the
>> past years.
>> > All that only in part time, thus it had to be *very* efficient for
>> myself. And that is
>> > something that I achieved by a consistent 'interface' to all those
>> projects. Just
>> > my widely used and highly convenient mail client. So, all that worked in
>> a sufficiently
>> > efficient way because I could combine this kind of 'work' even with my
>> private mail
>> > that I could handle in between with that single 'interface'. So going to
>> any web site
>> > there is already a detour and having multiple of them for each such
>> project gives an
>> > even longer detour. Okay today it's mostly mesa that is left as well as
>> a communication
>> > middle end used in vizsim applications. But going away too far away from
>> a mailing list
>> > will be mostly a loss of efficiency for me.
>> > As I said, my two cents, that should not keep you all from doing changes
>> that finally
>> > increase your all efficiency ...
>> >
>> > best
>> >
>> > Mathias
>> >
>> >
>> >
>>
>> Hi,
>>
>>
>> I have to add my voice here as well.
>>
>> Even though I do not feel able to give review for most of the mesa code
>> base,
>> I appreciate to have all patches in the mailing list in my mail client.
>>
>>  From time to time, I give feedback for some set of patches, for example
>> when I see patches related to dri3 or that could impact Nine.
>>
>> It also enables me to get an overview of all the recent works and new
>> features Nine could use.
>>
>> I feel like if most patches go through MR without getting a mail
>> feedback, I would not be able to do those as efficiently.
>>
>
> I find it interesting that multiple people have had this same sentiment.
> For me, the exact opposite is true.  I'm someone who is responsible for
> tracking and reviewing dozens of series at a time often with many patches
> each.  My current review backlog is probably 200 patches or more.  Having
> them in a linear stream in my mail isn't at all what I want because once I
> blow past the first page (50 e-mails) stuff easily gets lost.  The ability
> to look at them at the series level, filter by tags, etc. is a huge
> advantage.  I also really like the fact that GitLab pulls all the comments
> for the series together which makes cross-patch discussions easier to track.

Complete agreement here.  I'm both a regular contributor to Mesa (I've
been in Jason's position before 100s-of-patches review backlog), and an
occasional contributor to other projects (xserver is currently
"occasional" status, unfortunately).  I've found MRs to be better at
tracking outsanding review for me than email, and I even use the email
client written by cworth to try to solve this problem for us.

> You can subscribe to an individual label and it will e-mail you every time
> a MR is posted and tagged with that label.  Now, it will probably take a
> bit before we get label discipline sufficient that you can rely on it but
> the mechanism is there.

I'm curious if we can do something with this to replace discipline with
software:

https://about.gitlab.com/2016/08/19/applying-gitlab-labels-automatically/


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


Re: [Mesa-dev] Let's talk about -DDEBUG

2018-12-13 Thread Eric Anholt
Dylan Baker  writes:

> [ Unknown signature status ]
> In the autotools discussion I've come to realize that we also need to talk 
> about
> the -DDEBUG guard. It seems that there are two different uses, and thus two
> different asks about it:
>
> - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging
> - NIR and Intel (at least) use -DDEBUG to hide really expensive checks that 
> are
>   useful, but necessarily tank performance.
>
> The first group would like -DDEBUG in debugoptimized builds, the second
> obviously doesn't.
>
> Is the right solution to move the first group being !NDEBUG, or would it be
> better to split DEBUG into two different defines such as DEBUG_MESSAGES and
> EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with the
> first for both debug and debugoptimized and the second only in debug builds?

I would like to see NIR validation in debugoptimized builds (which is
the build I use on a regular basis: "please catch all bugs you can at
runtime with asserts, but don't waste CPU time by compiling with -O0");


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


Re: [Mesa-dev] last call for autotools

2018-12-11 Thread Eric Anholt
Emil Velikov  writes:

> On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
>>
>> Meson 0.49.0 has been out for a couple of days now, and I'd like to make the
>> final call for autotools. My patch is so massive that it's a huge pain to 
>> send
>> to the list, the latest versions is here:
>> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
>>
> Can you split this up a bit? I'm mostly conserved about a couple of things:
> - we're loosing multiple testing permutations with Travis - there's no
> meson equivalent
> - needed/missing bits are impossible to spot as-is
>
> Personally, I'd do something like:
>  - move .travis autotools permutations to meson
>  - docs changes
>  - rm Makefile{.*,}.am autogen.sh and configure.ac
>  - rm Makefile.sources
>  - .gitignore

I don't think we should block on anything related to travis at this
point.  I wrote it initially hoping it would let us prevent build
breakages.  Instead, whenever I try to use travis to prevent build
breakage, I find that travis has already been broken for a while.

The CI needs to be integrated with our actual repository host, or it
doesn't work.  We should just delete .travis.yml and push the gitlab
stuff forward instead.


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


Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

2018-12-06 Thread Eric Anholt
Jordan Justen  writes:

> This documents a process for using GitLab Merge Requests as an second
> way to submit code changes for Mesa. Only one of the two methods is
> allowed for each patch series.
>
> We will *not* require all patches to be emailed. Some code changes may
> be reviewed and merged without any discussion on the mesa-dev email
> list.

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] Make Jordan an Owner of the mesa project?

2018-12-03 Thread Eric Anholt
Jason Ekstrand  writes:

> Jordan has requested to be made an Owner of the mesa project.  As much as I
> may be the guy who pushed to get everything set up, I don't want to do this
> sort of thing on my own.  As such, I'm asking for some ACKs.  If I can get
> 5 ACKs (at least 2 non-intel) from other Owners and no NAKs, I'll click the
> button.
>
> Personally, I think the answer here is absurdly obvious.  Jordan is one of
> the most involved people in the community. :-D
>
> As a side-note, does this seem like a reasonable process for adding people
> as Owners?

I find Jordan to be entirely reasonable, and people with the motivation
to work on getting consensus on process changes without necessarily
pushing their own agenda are precious.  +1


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


Re: [Mesa-dev] GitLab merge requests, discussion summary - Re: [PATCH] docs: Document optional GitLab code review process

2018-12-03 Thread Eric Anholt
Jordan Justen  writes:

> Lots of discussion here. :) I hoped to just dip our toes into the
> merge request stream, but the consensus is clearly against my idea. Oh
> well. :)
>
> Here's what I gathered from the discussion:
>
> == Keep email, allow duplicate MR, my idea ==
>
> I don't think anyone else was in favor of this. Several were against
> it.
>
> == NACK merge requests altogether ==
>
> I did not see this, so that seems promising for merge requests. :) I
> guess no one strongly disagrees with trying merge requests.
>
> == Allow submitter (or driver) to choose to email or MR, but not both ==
>
> Several seemed to argue that this could be a good next step. But, in
> all those cases, I think they wanted to use merge requests going
> forward.
>
> It was mentioned that vc4 has moved to github in order to allow github
> pull requests to be used.

FWIW I moved to github because it allows contributors to use a familiar
process (no new logins!  non-bugzilla bug tracking!).  It was a success
except:

- I can't merge their PRs directly, I have to manually merge them to
  master anyway.

- Because issues and wiki are on github, they expect that they should be
  building and testing off of some branch in my repo, instead of Mesa
  master.  This does not go well.

I would love to solve both of those problems by having gitlab issues and
MRs enabled on the main repo.

> My opinion would be to try to move a bit slower and allow the
> submitter/project to choose for a trial period. (Dylan and Daniel seem
> to think this could be really frustrating for devs though.) But, if
> everyone seems to think it's reasonable to try to jump straight to
> using merge requests exclusively, I can type that version up...

I vote for this.  I suspect existing devs will be more interested in
switching once they have some good experiences with the new model
(that's how I was, at least).


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


Re: [Mesa-dev] [PATCH mesa] drop autotools

2018-12-03 Thread Eric Anholt
Eric Anholt  writes:

> [ Unknown signature status ]
> Eric Engestrom  writes:
>
>> Cc: Emil Velikov 
>> Cc: Andres Gomez 
>> Cc: Juan A. Suarez Romero 
>> Cc: Dylan Baker 
>> Signed-off-by: Eric Engestrom 
>> ---
>> This patch depends on the releasing procedure in docs/releasing.html to
>> be updated to not rely on autotools anymore.
>>
>> I think our release managers (cc'ed) should work together and figure out
>> the procedure they want to go by; only once that's done can we delete
>> these 200+ files and 14k+ lines :)
>> ---
>
>> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
>> index 1b839b0a485fec867375..9e8a34fbe1b150e6b9d4 100644
>> --- a/docs/relnotes/19.0.0.html
>> +++ b/docs/relnotes/19.0.0.html
>> @@ -51,7 +51,12 @@ Bug fixes
>>  Changes
>>  
>>  
>> -TBD
>> +  
>> +The autools build system has been removed. Please follow the
>> +instruction on https://mesa3d.org/meson.html;>the meson
>
> "instructions"
>
>> +page if you need help, or reach out on > +href="https://mesa3d.org/lists.html;>the mailing list.
>> +  
>>  
>>  
>>  
>
>> diff --git a/src/util/xmlpool/.gitignore b/src/util/xmlpool/.gitignore
>> index 383df727a17031214dee..db5b642826e990a5523c 100644
>> --- a/src/util/xmlpool/.gitignore
>> +++ b/src/util/xmlpool/.gitignore
>> @@ -1,8 +1 @@
>> -ca
>> -de
>> -es
>> -fr
>> -nl
>> -sv
>> -options.h
>>  xmlpool.pot
>
> Shouldn't xmlpool.pot also go away?
>
> Other than that,
>
> Reviewed-by: Eric Anholt 

Oh, we should probably at least disable the autotools tests in
.travis.yml until they can be converted to meson.


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


Re: [Mesa-dev] [PATCH mesa] drop autotools

2018-12-03 Thread Eric Anholt
Eric Engestrom  writes:

> Cc: Emil Velikov 
> Cc: Andres Gomez 
> Cc: Juan A. Suarez Romero 
> Cc: Dylan Baker 
> Signed-off-by: Eric Engestrom 
> ---
> This patch depends on the releasing procedure in docs/releasing.html to
> be updated to not rely on autotools anymore.
>
> I think our release managers (cc'ed) should work together and figure out
> the procedure they want to go by; only once that's done can we delete
> these 200+ files and 14k+ lines :)
> ---

> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index 1b839b0a485fec867375..9e8a34fbe1b150e6b9d4 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -51,7 +51,12 @@ Bug fixes
>  Changes
>  
>  
> -TBD
> +  
> +The autools build system has been removed. Please follow the
> +instruction on https://mesa3d.org/meson.html;>the meson

"instructions"

> +page if you need help, or reach out on  +href="https://mesa3d.org/lists.html;>the mailing list.
> +  
>  
>  
>  

> diff --git a/src/util/xmlpool/.gitignore b/src/util/xmlpool/.gitignore
> index 383df727a17031214dee..db5b642826e990a5523c 100644
> --- a/src/util/xmlpool/.gitignore
> +++ b/src/util/xmlpool/.gitignore
> @@ -1,8 +1 @@
> -ca
> -de
> -es
> -fr
> -nl
> -sv
> -options.h
>  xmlpool.pot

Shouldn't xmlpool.pot also go away?

Other than that,

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH v3 2/5] gallium: Add new PIPE_CAP_SURFACE_SAMPLE_COUNT

2018-11-30 Thread Eric Anholt
"Kristian H. Kristensen"  writes:

> This new pipe cap and the new nr_samples field in pipe_surface lets a
> state tracker bind a render target with a different sample count than
> the resource. This allows for implementing
> EXT_multisampled_render_to_texture and
> EXT_multisampled_render_to_texture2.
>
> Signed-off-by: Kristian H. Kristensen 

> diff --git a/src/gallium/docs/source/screen.rst 
> b/src/gallium/docs/source/screen.rst
> index 0abd164494c..cf2ce33b87f 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative 
> rasterization.
>0 means no limit.
>  * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value for
>of pipe_vertex_element::src_offset.
> +* ``PIPE_CAP_SURFACEA_SAMPLE_COUNT_TEXTURE``: Whether the driver
> +  supports pipe_surface overrides of resource nr_samples. If set, will
> +  enable EXT_multisampled_render_to_texture.

s/SURFACEA/SURFACE/

>  
>  .. _pipe_capf:
>  
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index e99895d30d8..6d96f1ccb5b 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -832,6 +832,7 @@ enum pipe_cap
> PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS,
> PIPE_CAP_MAX_TEXTURE_UPLOAD_MEMORY_BUDGET,
> PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET,
> +   PIPE_CAP_SURFACE_SAMPLE_COUNT,
>  };
>  
>  /**
> diff --git a/src/gallium/include/pipe/p_state.h 
> b/src/gallium/include/pipe/p_state.h
> index fd670345aad..89cffb15bd8 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -443,6 +443,12 @@ struct pipe_surface
> uint16_t width;   /**< logical width in pixels */
> uint16_t height;  /**< logical height in pixels */
>  
> +   /** Number of samples for the surface.  This can be different from the
> +* resource nr_samples when the resource is bound using
> +* FramebufferTexture2DMultisampleEXT.
> +*/
> +   unsigned nr_samples:8;

Don't you mean:

/**
 * Number of samples for the surface.  This will be 0 if rendering
 * should use the resource's nr_samples, or another value if the resource
 * is bound using FramebufferTexture2DMultisampleEXT
 */

Other than that, 1-3 are:

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

2018-11-30 Thread Eric Anholt
Ian Romanick  writes:

> On 11/29/2018 03:53 PM, Eric Anholt wrote:
>> e<#secure method=pgpmime mode=sign>
>> Erik Faye-Lund  writes:
>> 
>>> On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
>>>> Jordan Justen  writes:
>>>>
>>>>> This adds the "Developer's Certificate of Origin 1.1" from the
>>>>> Linux
>>>>> kernel. It indicates that by using Signed-off-by you are certifying
>>>>> that your patch meets the DCO 1.1 guidelines.
>>>>>
>>>>> It also changes Signed-off-by from being optional to being
>>>>> required.
>>>>>
>>>>> Signed-off-by: Jordan Justen 
>>>>> Cc: Matt Turner 
>>>>
>>>> What problem for our project is solved by signed-off-by?  Do you
>>>> think
>>>> that it has actually reduced the incidence of people submitting code
>>>> they don't have permission to submit in the projects where it's been
>>>> used?  I know the behavior in the kernel is that people submit a
>>>> patch,
>>>> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
>>>> just give up.  I don't think anyone stops and says "Wow, that's a
>>>> good
>>>> question.  Maybe I don't have permission to distribute this after
>>>> all?"
>>>>
>>>> /me sees s-o-b as basically just a linux kernel hazing ritual
>>>
>>> I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
>>> reason for the s-o-b is to have a paper-trail for how a patch landed in
>>> the kernel; who it went through etc.
>> 
>> I get the *idea*, I just believe the idea fails in practice.  The S-O-B
>> idea came from "wouldn't it be nice if we could make everyone think
>> about this issue that is important to us" but what we actually got
>> instead is "your patch gets bounced if you don't have a s-o-b on until
>> you slap one on and resend."
>
> You're thinking like an engineer, but the s-o-b is the spawn of lawyers.
>  Supposedly, having someone say "I had the rights to contribute this
> code," even if they didn't think about what that meant, enables you to
> pass that blame along because that person showed intent.  You may not
> have read and thought about every page of your mortgage, but you can
> still be held accountable for every word.  Having an author tag or a
> committer tag does not show that same intent.  In a legal context, the
> s-o-b shifts the onus of auditing code ownership from the distributor to
> the submitter.  When a distributor, like IBM in the SCO case, get sued,
> they can plausibly claim that all of the code they distributed was
> vouched for.  If someone submits code that they do not have the right to
> submit and provides false testimony, then that's on the submitter.

That's an interesting legal theory.  I can't comment on it, not being a
lawyer, and I haven't seen lawyers comment on it, either.  What I do see
for the origin of the DCO is:

https://lkml.org/lkml/2004/5/23/10

which looks a lot more like what I was describing (an engineer trying to
solve a social problem with engineering) than something that came out of
a lawyers.


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


Re: [Mesa-dev] Lets talk about autotools

2018-11-29 Thread Eric Anholt
Emil Velikov  writes:

> Hi all,
>
> I can see why people may opt to not use or maintain the autotools build.
> Although I would kindly ask that we do not remove it just yet.
>
> In Mesa, we have different parts not used by different teams. As such
> we tend to remove stuff when nobody is around to maintain it anymore.
>
> That said, I'm planning to continue maintaining it and would appreciate
> if we keep it in-tree.
>
> As people may be concerned about bugreports and alike we can trivially
> add a warning (as configure is invoked) to forwards any issues to my
> email. Additionally (or alternatively) we can have an autotools bugzilla
> category with me as the default assignee.
>
> What do you guys think?

Strongly disagree.  We shouldn't be maintaining build systems for fun,
we should be doing the minimum amount of build system work to get our
actual work done quickly and reliably.


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


Re: [Mesa-dev] [PATCH] mesa: Revert INTEL_fragment_shader_ordering support

2018-11-29 Thread Eric Anholt
Matt Turner  writes:

> This extension is not properly tested (testing for
> GL_ARB_fragment_shader_interlock is not sufficient), and since this was
> noted in review on August 28th no tests have been sent.
>
> Revert "i965: Add INTEL_fragment_shader_ordering support."
> Revert "mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering"
>
> This reverts commit 03ecec9ed2099f6e2b62994b33dc948dc731e7b8.
> This reverts commit 119435c8778dd26cb7c8bcde9f04b3982239fe60.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> Emil: I just noticed that this was never reverted from master (and it
> needs to be removed before the 18.3 release)

Acked-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

2018-11-29 Thread Eric Anholt
e<#secure method=pgpmime mode=sign>
Erik Faye-Lund  writes:

> On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
>> Jordan Justen  writes:
>> 
>> > This adds the "Developer's Certificate of Origin 1.1" from the
>> > Linux
>> > kernel. It indicates that by using Signed-off-by you are certifying
>> > that your patch meets the DCO 1.1 guidelines.
>> > 
>> > It also changes Signed-off-by from being optional to being
>> > required.
>> > 
>> > Signed-off-by: Jordan Justen 
>> > Cc: Matt Turner 
>> 
>> What problem for our project is solved by signed-off-by?  Do you
>> think
>> that it has actually reduced the incidence of people submitting code
>> they don't have permission to submit in the projects where it's been
>> used?  I know the behavior in the kernel is that people submit a
>> patch,
>> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
>> just give up.  I don't think anyone stops and says "Wow, that's a
>> good
>> question.  Maybe I don't have permission to distribute this after
>> all?"
>> 
>> /me sees s-o-b as basically just a linux kernel hazing ritual
>
> I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
> reason for the s-o-b is to have a paper-trail for how a patch landed in
> the kernel; who it went through etc.

I get the *idea*, I just believe the idea fails in practice.  The S-O-B
idea came from "wouldn't it be nice if we could make everyone think
about this issue that is important to us" but what we actually got
instead is "your patch gets bounced if you don't have a s-o-b on until
you slap one on and resend."

We've already got 1-2 people to contact if there's a question about
authorship, from the committer and author fields.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-29 Thread Eric Anholt
Eric Engestrom  writes:

> On Wednesday, 2018-11-28 13:36:29 -0800, Eric Anholt wrote:
>> Jordan Justen  writes:
>> 
>> > This documents a mechanism for using GitLab Merge Requests as an
>> > optional, secondary way to get code reviews for patchsets.
>> >
>> > We still require all patches to be emailed.
>> >
>> > Aside from the potential usage for code review comments, it might also
>> > help reviewers to find unmerged patchsets which need review. (Assuming
>> > it doesn't fall into the same fate of patchwork with hundreds of open
>> > MRs.)
>> >
>> > Signed-off-by: Jordan Justen 
>> > Cc: Jason Ekstrand 
>> > ---
>> >  docs/submittingpatches.html | 25 +
>> >  1 file changed, 25 insertions(+)
>> >
>> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
>> > index 5d8ba443191..852f28c198a 100644
>> > --- a/docs/submittingpatches.html
>> > +++ b/docs/submittingpatches.html
>> > @@ -24,6 +24,7 @@
>> >  Testing Patches
>> >  Mailing Patches
>> >  Reviewing Patches
>> > +GitLab Merge Requests
>> >  Nominating a commit for a stable branch
>> >  Criteria for accepting patches to the stable 
>> > branch
>> >  Sending backports for the stable branch
>> > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
>> > committed, as long
>> >  as the issues are resolved first.
>> >  
>> >  
>> > +GitLab Merge Requests
>> > +
>> > +
>> > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
>> > +  Requests (MR) can be used as an optional, secondary method of
>> > +  obtaining code review for patches.
>> > +
>> > +
>> > +
>> > +  All patches should be submitted using email as well
>> 
>> Like others, I disagree and think this will lead to confusion.  Let
>> people send to one or the other.
>> 
>> > +  Consider including a link to the MR in your email based
>> > +cover-letter
>> > +  Address code review from both email and the MR
>> > +  Add appropriate labels to your MR. For example:
>> > +
>> > +  Mesa changes affect all drivers: mesa
>> > +  Hardware vendor specific code: amd, intel, nvidia, etc
>> > +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
>> > radv, vc4, etc
>> > +  Other tag examples: gallium, util
>> > +
>> > +  Never use the merge button on the GitLab page to push patches
>> 
>> Why "never use the merge button"?  We've been using rebase+merge in
>> xserver and it's *awesome* and has greatly increased the review rate.
>
> Because GitLab doesn't (yet) support tracking 'reviewed' status in
> the commit message, which is something I think is important to track for
> various reasons. If those are manually added to the commit messages
> before the MR is merged, then there's no issue there.
>
> We could also have a check that an MR can't be merged until all its
> commits contain the string "Reviewed-by:", which might be a better
> solution?

In the xserver we decided that it was more important to have MRs be
available (so a reviewer can click "merge when the tests are green")
than to have in-git tracking of whether commits were reviewed.  I think
it's been a great tradeoff, but I understand if Mesa doesn't want to
make the same one.


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


Re: [Mesa-dev] [PATCH B 14/14] FIXUP: Fix NIR producers and consumers to use unsized conversions

2018-11-28 Thread Eric Anholt
Jason Ekstrand  writes:

> ---
>  src/amd/common/ac_nir_to_llvm.c   | 12 
>  src/compiler/glsl/glsl_to_nir.cpp |  2 +-
>  src/compiler/nir/nir_builder.h| 12 
>  src/compiler/spirv/vtn_glsl450.c  |  4 ++--
>  .../drivers/freedreno/ir3/ir3_compiler_nir.c  | 11 +++
>  src/gallium/drivers/vc4/vc4_program.c |  8 
>  src/intel/compiler/brw_fs_nir.cpp | 19 ---
>  src/intel/compiler/brw_vec4_nir.cpp   |  9 +
>  src/mesa/program/prog_to_nir.c|  4 ++--
>  9 files changed, 53 insertions(+), 28 deletions(-)

Don't forget src/broadcom/compiler :)

B looks pretty reasonable.  I've tried to muster the concentration to
review this series a few times, and between the python and algebraic
changes my eyes keep glazing over.  Acked-in-principle, I guess?


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


Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

2018-11-28 Thread Eric Anholt
Jordan Justen  writes:

> This adds the "Developer's Certificate of Origin 1.1" from the Linux
> kernel. It indicates that by using Signed-off-by you are certifying
> that your patch meets the DCO 1.1 guidelines.
>
> It also changes Signed-off-by from being optional to being required.
>
> Signed-off-by: Jordan Justen 
> Cc: Matt Turner 

What problem for our project is solved by signed-off-by?  Do you think
that it has actually reduced the incidence of people submitting code
they don't have permission to submit in the projects where it's been
used?  I know the behavior in the kernel is that people submit a patch,
it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
just give up.  I don't think anyone stops and says "Wow, that's a good
question.  Maybe I don't have permission to distribute this after all?"

/me sees s-o-b as basically just a linux kernel hazing ritual


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


Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-28 Thread Eric Anholt
Jordan Justen  writes:

> This documents a mechanism for using GitLab Merge Requests as an
> optional, secondary way to get code reviews for patchsets.
>
> We still require all patches to be emailed.
>
> Aside from the potential usage for code review comments, it might also
> help reviewers to find unmerged patchsets which need review. (Assuming
> it doesn't fall into the same fate of patchwork with hundreds of open
> MRs.)
>
> Signed-off-by: Jordan Justen 
> Cc: Jason Ekstrand 
> ---
>  docs/submittingpatches.html | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 5d8ba443191..852f28c198a 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -24,6 +24,7 @@
>  Testing Patches
>  Mailing Patches
>  Reviewing Patches
> +GitLab Merge Requests
>  Nominating a commit for a stable branch
>  Criteria for accepting patches to the stable 
> branch
>  Sending backports for the stable branch
> @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> committed, as long
>  as the issues are resolved first.
>  
>  
> +GitLab Merge Requests
> +
> +
> +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> +  Requests (MR) can be used as an optional, secondary method of
> +  obtaining code review for patches.
> +
> +
> +
> +  All patches should be submitted using email as well

Like others, I disagree and think this will lead to confusion.  Let
people send to one or the other.

> +  Consider including a link to the MR in your email based
> +cover-letter
> +  Address code review from both email and the MR
> +  Add appropriate labels to your MR. For example:
> +
> +  Mesa changes affect all drivers: mesa
> +  Hardware vendor specific code: amd, intel, nvidia, etc
> +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
> radv, vc4, etc
> +  Other tag examples: gallium, util
> +
> +  Never use the merge button on the GitLab page to push patches

Why "never use the merge button"?  We've been using rebase+merge in
xserver and it's *awesome* and has greatly increased the review rate.

> +  Add Reviewed-by tags to your commits and push using git
> +  Close your MR when your patches get pushed!
> +
>  
>  Nominating a commit for a stable branch

Overall:

I can't wait to have a full MR process.  What if we just *never* merged
code that broke the build or introduced warnings, because the MR process
ensured it?  How much time would we all save?  How much less training
would we need to do on new contributors?


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] gallium: Remove unused variable in u_tests.

2018-11-27 Thread Eric Anholt
Fixes: 0d17b685b1ff ("gallium/u_tests: add a compute shader test that clears an 
image")
---
 src/gallium/auxiliary/util/u_tests.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_tests.c 
b/src/gallium/auxiliary/util/u_tests.c
index 7c4e98402bef..365d4fa8f171 100644
--- a/src/gallium/auxiliary/util/u_tests.c
+++ b/src/gallium/auxiliary/util/u_tests.c
@@ -792,7 +792,6 @@ test_compute_clear_image(struct pipe_context *ctx)
 {
struct cso_context *cso;
struct pipe_resource *cb;
-   struct pipe_sampler_view *view = NULL;
const char *text;
 
cso = cso_create_context(ctx, 0);
-- 
2.20.0.rc1

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


Re: [Mesa-dev] Lets talk about autotools

2018-11-26 Thread Eric Anholt
Ian Romanick  writes:

> On 11/26/2018 03:44 PM, Marek Olšák wrote:
>> I don't run meson. I have a script that runs meson. I want to just run
>> the script and I want to it configure or reconfigure.
>> 
>> I don't want to add this failure path into my script: "meson ... ||
>> meson --reconfigure ..."
>
> I have a similar build script.  In my particular, weird setup I've had
> something like this since forever:
>
> if [ -d $BUILD_DIR ]; then
> meson --reconfigure ...
> else
> meson ...
> fi
>
> I even had to do some junk like that in the autotools days.  I'd love to
> have something better. :(

Agreed with this being an irritating feature of meson.


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] gallium: Fix uninitialized variable warning in compute test.

2018-11-26 Thread Eric Anholt
The compiler doesn't know that ny != 0, so x might be uninitialized for
the printf at the end.
---
 src/gallium/tests/trivial/compute.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/tests/trivial/compute.c 
b/src/gallium/tests/trivial/compute.c
index afe5d3e9f2be..20e5a4f140c9 100644
--- a/src/gallium/tests/trivial/compute.c
+++ b/src/gallium/tests/trivial/compute.c
@@ -240,7 +240,7 @@ static void check_tex(struct context *ctx, int slot,
   util_format_get_nblocksy(tex->format, tex->height0));
 struct pipe_transfer *xfer;
 char *map;
-int x, y, i;
+int x = 0, y, i;
 int err = 0;
 
 if (!check)
-- 
2.20.0.rc1

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


Re: [Mesa-dev] [RFC 1/5] nir: Rename nir_op_fne to nir_op_fneu

2018-11-26 Thread Eric Anholt
Jason Ekstrand  writes:

> This way, it's explicit in the opcode name that it's an unordered
> comparison.
> ---
>  src/amd/common/ac_nir_to_llvm.c   |  2 +-
>  src/compiler/glsl/glsl_to_nir.cpp |  4 +-
>  src/compiler/nir/nir.h|  2 +-
>  src/compiler/nir/nir_builder.h|  2 +-
>  src/compiler/nir/nir_loop_analyze.c   |  4 +-
>  src/compiler/nir/nir_lower_alu_to_scalar.c|  2 +-
>  src/compiler/nir/nir_lower_double_ops.c   |  6 +--
>  src/compiler/nir/nir_opcodes.py   |  2 +-
>  src/compiler/nir/nir_opt_algebraic.py | 46 +--
>  src/compiler/spirv/vtn_alu.c  | 10 ++--
>  src/compiler/spirv/vtn_glsl450.c  |  4 +-
>  src/gallium/auxiliary/nir/tgsi_to_nir.c   |  4 +-
>  .../drivers/freedreno/ir3/ir3_compiler_nir.c  |  2 +-
>  src/gallium/drivers/vc4/vc4_program.c |  4 +-
>  src/intel/compiler/brw_fs_nir.cpp |  4 +-
>  src/intel/compiler/brw_vec4_nir.cpp   |  4 +-
>  16 files changed, 51 insertions(+), 51 deletions(-)

Looks like you missed src/broadcom/compiler/


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


Re: [Mesa-dev] [PATCH mesa 00/13] Make standard function available on non-standard platforms

2018-11-20 Thread Eric Anholt
Eric Engestrom  writes:

> ... instead of making standard platforms use non-standard functions.
>
> This also reduces the likelihood of someone forgetting to use the
> non-standard function, and reduces the fix to a simple #include.

Thank you so much for doing this!

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH RFC] egl: Add a 565 pbuffer-only EGL config under X11.

2018-11-19 Thread Eric Anholt
Adam Jackson  writes:

> On Tue, 2018-10-30 at 16:38 -0700, Eric Anholt wrote:
>> Eric Anholt  writes:
>> 
>> > The CTS requires a 565-no-depth-no-stencil config for ES 3.0, but at depth
>> > 24 of X11 we wouldn't do so.  We can satisfy that bad requirement using a
>> > pbuffer-only visual with whatever other buffers the driver happens to have
>> > given us.
>> 
>> Anyone?  Still concerned about getting Mesa to pass the current
>> conformance suite.
>
> I'm not thrilled with the CTS requiring R5G6B5 tbh. Do we know if
> there's some rationale for that? Maybe something along the lines of,
> "GLES target devices are likely to only support low depths, so even a
> workstation GLES needs to implement low depths because otherwise how do
> you know it works". Not that I'd endorse such reasoning.
>
> To the patch itself:
>
>> +   /* Add a 565 pbuffer-only config.  If X11 is depth 24, we wouldn't have 
>> 565
>> +* avialable, which the CTS demands.
>^^
> Typo.
>
>> +*/
>> +   for (int j = 0; dri2_dpy->driver_configs[j]; j++) {
>> +  const __DRIconfig *config = dri2_dpy->driver_configs[j];
>> +  const EGLint config_attrs[] = {
>> + EGL_NATIVE_VISUAL_ID,0,
>> + EGL_NATIVE_VISUAL_TYPE,  EGL_NONE,
>> + EGL_NONE
>> +  };
>
> The commit message says you need no-depth and no-stencil, but you don't
> enforce that here. Are we sure we don't need to?
>
>> +  EGLint surface_type = EGL_PBUFFER_BIT;
>> +  unsigned int rgba_masks[4] = {
>> + 0x1f << 11,
>> + 0x3f << 5,
>> + 0x1f << 0,
>> + 0,
>> +  };
>> +  if (dri2_add_config(disp, config, config_count + 1, surface_type,
>> +  config_attrs, rgba_masks)) {
>> + config_count++;
>> + break;
>> +  }
>> +   }
>
> This seems like it introduces a subtle dependency on how driver_configs
> is ordered. I'm not sure you'd want your lone 565 pbuffer config to be
> sRGB or multisampled. In fact in the latter case you'd end up with an
> EGLConfig with zero bits set for EGL_SURFACE_TYPE, which is a bug in
> dri2_add_config() I suppose.
>
> That's probably not how driver_configs is sorted in reality, and
> passing CTS is a valid thing to want, so with the above addressed:
>
> Reviewed-by: Adam Jackson 
>
> But if CTS can be fixed, let's do that too.

OK, let's try that route first.

https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/1474


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


Re: [Mesa-dev] [PATCH 2/4] freedreno: a2xx: add partial lower_scalar pass for ir2

2018-11-19 Thread Eric Anholt
Jonathan Marek  writes:

> some instructions can only be scalar on a2xx, lower these only

Could we have the core ALU scalar lowering take an optional bitmask or
something of ALU ops that need scalar lowering?


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


Re: [Mesa-dev] [PATCH mesa] gbm: add missing comma between strings

2018-11-18 Thread Eric Anholt
Eric Engestrom  writes:

> Fixes: d971a4230d54069c996bc "loader: Factor out the common driver
>   opening logic from each loader."
> Cc: Eric Anholt 
> Signed-off-by: Eric Engestrom 

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH 5/5] loader: Factor out the common driver opening logic from each loader.

2018-11-16 Thread Eric Anholt
Eric Engestrom  writes:

> On Thursday, 2018-11-15 15:05:27 -0800, Eric Anholt wrote:
>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
>> index f32c0cd9885f..8f035b7c7d84 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -301,31 +301,21 @@ dri_bind_extensions(struct gbm_dri_device *dri,
>> return ret;
>>  }
>>  
>> +static void
>> +gbm_loader_log(int level, const char *fmt, ...)
>> +{
>> +   if (level > _LOADER_WARNING)
>> +  return;
>> +
>> +   va_list ap;
>> +   va_start(ap, fmt);
>> +   vfprintf(stderr, fmt, ap);
>> +   va_end(ap);
>> +}
>
> Why? This is the exact behaviour of the default_logger() :)
>
> If you still want to add it, I'd do it in a separate commit, since it
> has nothing to do with the rest of this patch.
>
> With gbm_loader_log() split out, this is:
> Reviewed-by: Eric Engestrom 

Thanks.  I wrote this when confused by my dlopen() information not
printing in the gbm case (because the useful debug is at a lower level
and can't be upgraded in severity because we expect some dlopen()s to
fail in the normal case).  I ended up cutting out the level check while
debugging, and never went back and confirmed that it was useful with the
check in at all!


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


Re: [Mesa-dev] [PATCH 2/5] glx: Move DRI extensions pointer loading to driOpenDriver().

2018-11-16 Thread Eric Anholt
Emil Velikov  writes:

> On Thu, 15 Nov 2018 at 23:05, Eric Anholt  wrote:
>
>> --- a/src/glx/dri_glx.c
>> +++ b/src/glx/dri_glx.c
>> @@ -199,15 +199,9 @@ clear_driver_config_cache()
>>  static char *
>>  get_driver_config(const char *driverName)
>>  {
>> -   void *handle = driOpenDriver(driverName);
>> -   const __DRIextension **extensions;
>> -
>> -   if (!handle)
>> -  return NULL;
>> -
>> +   void *handle;
> I think we want to initialise this variable.
>
> Otherwise on failure when driGetDriverExtensions() fails, we'll feed
> garbage into dlclose() further down.
> It's unlikely that will happen, but If nothing else the static
> analysis tools will be happy.

After driGetDriverExtensions() failure driOpenDriver() NULLs out the
handle that gets stored to *out_driver_handle.  I did that so that all
the callers didn't have to worry about this.  (aka asprintf is the worst
interface)


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] egl: Print the actual message to the console from _eglError().

2018-11-15 Thread Eric Anholt
Previously we would print errors on the console like:

   libEGL debug: EGL user error 0x3001 (EGL_NOT_INITIALIZED) in eglInitialize

When we had everything we needed for:

   libEGL debug: EGL user error 0x3001 (EGL_NOT_INITIALIZED) in eglInitialize: 
DRI2: failed to find EGLDevice

(for a gbm error in my case)
---
 src/egl/main/eglcurrent.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/egl/main/eglcurrent.c b/src/egl/main/eglcurrent.c
index 7af3011b7576..479f231fb8f6 100644
--- a/src/egl/main/eglcurrent.c
+++ b/src/egl/main/eglcurrent.c
@@ -310,20 +310,28 @@ _eglDebugReport(EGLenum error, const char *funcName,
 
mtx_unlock(_eglGlobal.Mutex);
 
-   if (callback != NULL) {
-  char *buf = NULL;
+   char *message_buf = NULL;
+   if (message != NULL) {
+  va_start(args, message);
+  if (vasprintf(_buf, message, args) < 0)
+ message_buf = NULL;
+  va_end(args);
+   }
 
-  if (message != NULL) {
- va_start(args, message);
- if (vasprintf(, message, args) < 0)
-buf = NULL;
+   if (callback != NULL) {
+  callback(error, funcName, type, thr->Label, thr->CurrentObjectLabel,
+   message_buf);
+   }
 
- va_end(args);
+   if (type == EGL_DEBUG_MSG_CRITICAL_KHR || type == EGL_DEBUG_MSG_ERROR_KHR) {
+  char *func_message_buf = NULL;
+  /* Note: _eglError() is often called with msg == thr->currentFuncName */
+  if (message_buf && funcName && strcmp(message_buf, funcName) != 0) {
+ if (asprintf(_message_buf, "%s: %s", funcName, message_buf) < 0)
+func_message_buf = NULL;
   }
-  callback(error, funcName, type, thr->Label, thr->CurrentObjectLabel, 
buf);
-  free(buf);
+  _eglInternalError(error, func_message_buf ? func_message_buf : funcName);
+  free(func_message_buf);
}
-
-   if (type == EGL_DEBUG_MSG_CRITICAL_KHR || type == EGL_DEBUG_MSG_ERROR_KHR)
-  _eglInternalError(error, funcName);
+   free(message_buf);
 }
-- 
2.19.1

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


[Mesa-dev] [PATCH 4/5] loader: Stop using a local definition for an in-tree header

2018-11-15 Thread Eric Anholt
I need other types from the header now, and "gl.h is big" is not a good
reason to duplicate definitions.
---
 src/loader/loader.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/loader/loader.c b/src/loader/loader.c
index 461f96aa6a44..041a59212be7 100644
--- a/src/loader/loader.c
+++ b/src/loader/loader.c
@@ -41,6 +41,8 @@
 #ifdef MAJOR_IN_SYSMACROS
 #include 
 #endif
+#include 
+#include 
 #include "loader.h"
 
 #ifdef HAVE_LIBDRM
@@ -492,14 +494,6 @@ loader_set_logger(void (*logger)(int level, const char 
*fmt, ...))
log_ = logger;
 }
 
-/* XXX: Local definition to avoid pulling the heavyweight GL/gl.h and
- * GL/internal/dri_interface.h
- */
-
-#ifndef __DRI_DRIVER_GET_EXTENSIONS
-#define __DRI_DRIVER_GET_EXTENSIONS "__driDriverGetExtensions"
-#endif
-
 char *
 loader_get_extensions_name(const char *driver_name)
 {
-- 
2.19.1

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


[Mesa-dev] [PATCH 3/5] egl: Move loader_set_logger() up to egl_dri2.c.

2018-11-15 Thread Eric Anholt
Everyone needs to call it, and platform_x11 forgot to.
---
 src/egl/drivers/dri2/egl_dri2.c | 2 ++
 src/egl/drivers/dri2/platform_android.c | 2 --
 src/egl/drivers/dri2/platform_drm.c | 2 --
 src/egl/drivers/dri2/platform_surfaceless.c | 2 --
 src/egl/drivers/dri2/platform_wayland.c | 4 
 5 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 3b63aebbf9a2..f05c39126c85 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -900,6 +900,8 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
   return EGL_TRUE;
}
 
+   loader_set_logger(_eglLog);
+
switch (disp->Platform) {
case _EGL_PLATFORM_SURFACELESS:
   ret = dri2_initialize_surfaceless(drv, disp);
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index b3ef55896116..366a9ec14e98 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1536,8 +1536,6 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay 
*disp)
if (disp->Options.ForceSoftware)
   return EGL_FALSE;
 
-   loader_set_logger(_eglLog);
-
dri2_dpy = calloc(1, sizeof(*dri2_dpy));
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index fb346e007332..c1ab1c9b0f6e 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -704,8 +704,6 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
if (disp->Options.ForceSoftware)
   return EGL_FALSE;
 
-   loader_set_logger(_eglLog);
-
dri2_dpy = calloc(1, sizeof *dri2_dpy);
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
diff --git a/src/egl/drivers/dri2/platform_surfaceless.c 
b/src/egl/drivers/dri2/platform_surfaceless.c
index 1edfca246dbd..f98095616115 100644
--- a/src/egl/drivers/dri2/platform_surfaceless.c
+++ b/src/egl/drivers/dri2/platform_surfaceless.c
@@ -350,8 +350,6 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay 
*disp)
const char* err;
bool driver_loaded = false;
 
-   loader_set_logger(_eglLog);
-
dri2_dpy = calloc(1, sizeof *dri2_dpy);
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index dc16a69dfbc2..8122c8112887 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -1323,8 +1323,6 @@ dri2_initialize_wayland_drm(_EGLDriver *drv, _EGLDisplay 
*disp)
_EGLDevice *dev;
struct dri2_egl_display *dri2_dpy;
 
-   loader_set_logger(_eglLog);
-
dri2_dpy = calloc(1, sizeof *dri2_dpy);
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
@@ -1986,8 +1984,6 @@ dri2_initialize_wayland_swrast(_EGLDriver *drv, 
_EGLDisplay *disp)
_EGLDevice *dev;
struct dri2_egl_display *dri2_dpy;
 
-   loader_set_logger(_eglLog);
-
dri2_dpy = calloc(1, sizeof *dri2_dpy);
if (!dri2_dpy)
   return _eglError(EGL_BAD_ALLOC, "eglInitialize");
-- 
2.19.1

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


[Mesa-dev] [PATCH 1/5] glx: Remove an old DEFAULT_DRIVER_DIR default.

2018-11-15 Thread Eric Anholt
You can tell by "Mesa/configs/default" how old this is.  Your build system
really has to provide the DEFAULT_DRIVER_DIR, or other loaders will break.
---
 src/glx/dri_common.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index ab5d6c5bc03d..08923d7efd55 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -77,11 +77,6 @@ dri_message(int level, const char *f, ...)
 #define GL_LIB_NAME "libGL.so.1"
 #endif
 
-#ifndef DEFAULT_DRIVER_DIR
-/* this is normally defined in Mesa/configs/default with 
DRI_DRIVER_SEARCH_PATH */
-#define DEFAULT_DRIVER_DIR "/usr/local/lib/dri"
-#endif
-
 /**
  * Try to \c dlopen the named driver.
  *
-- 
2.19.1

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


[Mesa-dev] [PATCH 5/5] loader: Factor out the common driver opening logic from each loader.

2018-11-15 Thread Eric Anholt
I copied the code from egl_dri2.c, but the functionality was equivalent
between all the loaders other than their particular environment variables.
---
 src/egl/Makefile.am |  3 +-
 src/egl/drivers/dri2/egl_dri2.c | 75 +++---
 src/egl/meson.build |  3 --
 src/gbm/Makefile.am |  1 -
 src/gbm/backends/dri/gbm_dri.c  | 96 +
 src/gbm/meson.build |  1 -
 src/glx/Makefile.am |  1 -
 src/glx/SConscript  |  1 -
 src/glx/dri_common.c| 91 +++
 src/glx/meson.build |  1 -
 src/loader/Makefile.am  |  1 +
 src/loader/loader.c | 90 +++
 src/loader/loader.h |  7 +++
 src/loader/meson.build  |  4 +-
 14 files changed, 142 insertions(+), 233 deletions(-)

diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 24a8e96a8e1e..7269912d96f3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -119,8 +119,7 @@ AM_CFLAGS += \
-I$(top_srcdir)/src/egl/drivers/dri2 \
-I$(top_srcdir)/src/gbm/backends/dri \
-I$(top_builddir)/src/egl/wayland/wayland-drm \
-   -I$(top_srcdir)/src/egl/wayland/wayland-drm \
-   -DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\"
+   -I$(top_srcdir)/src/egl/wayland/wayland-drm
 
 nodist_libEGL_common_la_SOURCES = \
$(dri2_backend_GENERATED_FILES)
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f05c39126c85..f998655d2ae0 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -483,75 +483,14 @@ static const __DRIextension **
 dri2_open_driver(_EGLDisplay *disp)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
-   const __DRIextension **extensions = NULL;
-   char path[PATH_MAX], *search_paths, *next, *end;
-   char *get_extensions_name;
-   const __DRIextension **(*get_extensions)(void);
-
-   search_paths = NULL;
-   if (geteuid() == getuid()) {
-  /* don't allow setuid apps to use LIBGL_DRIVERS_PATH */
-  search_paths = getenv("LIBGL_DRIVERS_PATH");
-   }
-   if (search_paths == NULL)
-  search_paths = DEFAULT_DRIVER_DIR;
-
-   dri2_dpy->driver = NULL;
-   end = search_paths + strlen(search_paths);
-   for (char *p = search_paths; p < end; p = next + 1) {
-  int len;
-  next = strchr(p, ':');
-  if (next == NULL)
- next = end;
-
-  len = next - p;
-#if GLX_USE_TLS
-  snprintf(path, sizeof path,
-   "%.*s/tls/%s_dri.so", len, p, dri2_dpy->driver_name);
-  dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
-#endif
-  if (dri2_dpy->driver == NULL) {
- snprintf(path, sizeof path,
-  "%.*s/%s_dri.so", len, p, dri2_dpy->driver_name);
- dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
- if (dri2_dpy->driver == NULL)
-_eglLog(_EGL_DEBUG, "failed to open %s: %s\n", path, dlerror());
-  }
-  /* not need continue to loop all paths once the driver is found */
-  if (dri2_dpy->driver != NULL)
- break;
-   }
-
-   if (dri2_dpy->driver == NULL) {
-  _eglLog(_EGL_WARNING,
-  "DRI2: failed to open %s (search paths %s)",
-  dri2_dpy->driver_name, search_paths);
-  return NULL;
-   }
-
-   _eglLog(_EGL_DEBUG, "DRI2: dlopen(%s)", path);
-
-   get_extensions_name = loader_get_extensions_name(dri2_dpy->driver_name);
-   if (get_extensions_name) {
-  get_extensions = dlsym(dri2_dpy->driver, get_extensions_name);
-  if (get_extensions) {
- extensions = get_extensions();
-  } else {
- _eglLog(_EGL_DEBUG, "driver does not expose %s(): %s\n",
- get_extensions_name, dlerror());
-  }
-  free(get_extensions_name);
-   }
-
-   if (!extensions)
-  extensions = dlsym(dri2_dpy->driver, __DRI_DRIVER_EXTENSIONS);
-   if (extensions == NULL) {
-  _eglLog(_EGL_WARNING,
-  "DRI2: driver exports no extensions (%s)", dlerror());
-  dlclose(dri2_dpy->driver);
-   }
+   static const char *search_path_vars[] = {
+  "LIBGL_DRIVERS_PATH",
+  NULL,
+   };
 
-   return extensions;
+   return loader_open_driver(dri2_dpy->driver_name,
+ _dpy->driver,
+ search_path_vars);
 }
 
 EGLBoolean
diff --git a/src/egl/meson.build b/src/egl/meson.build
index 8c0ffea8b40a..372842967d37 100644
--- a/src/egl/meson.build
+++ b/src/egl/meson.build
@@ -93,9 +93,6 @@ if with_dri2
 'drivers/dri2/egl_dri2.h',
 'drivers/dri2/egl_dri2_fallbacks.h',
   )
-  c_args_for_egl += [
-'-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
-  ]
 
   if with_platform_x11
 files_egl += files('drivers/dri2/platform_x11.c')
diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
index 5097212cda0a..bb246ecebf52 100644
--- a/src/gbm/Makefile.am
+++ b/src/gbm/Makefile.am
@@ -42,7 +42,6 

[Mesa-dev] [PATCH 2/5] glx: Move DRI extensions pointer loading to driOpenDriver().

2018-11-15 Thread Eric Anholt
The only thing you do with a dri driver handle is get the extensions
pointer, so just fold it in to simplify the callers.
---
 src/glx/dri2_glx.c   |  8 +---
 src/glx/dri3_glx.c   |  8 +---
 src/glx/dri_common.c | 20 +++-
 src/glx/dri_common.h |  6 ++
 src/glx/dri_glx.c| 16 +++-
 src/glx/drisw_glx.c  | 17 +
 6 files changed, 23 insertions(+), 52 deletions(-)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 91afc3375058..d8c5ba25f04e 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -1252,13 +1252,7 @@ dri2CreateScreen(int screen, struct glx_display * priv)
   driverName = loader_driverName;
}
 
-   psc->driver = driOpenDriver(driverName);
-   if (psc->driver == NULL) {
-  ErrorMessageF("driver pointer missing\n");
-  goto handle_error;
-   }
-
-   extensions = driGetDriverExtensions(psc->driver, driverName);
+   extensions = driOpenDriver(driverName, >driver);
if (extensions == NULL)
   goto handle_error;
 
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index ce60b95c71e3..298adc80ef16 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -861,13 +861,7 @@ dri3_create_screen(int screen, struct glx_display * priv)
   goto handle_error;
}
 
-   psc->driver = driOpenDriver(driverName);
-   if (psc->driver == NULL) {
-  ErrorMessageF("driver pointer missing\n");
-  goto handle_error;
-   }
-
-   extensions = driGetDriverExtensions(psc->driver, driverName);
+   extensions = driOpenDriver(driverName, >driver);
if (extensions == NULL)
   goto handle_error;
 
diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
index 08923d7efd55..75a5e5025761 100644
--- a/src/glx/dri_common.c
+++ b/src/glx/dri_common.c
@@ -85,12 +85,14 @@ dri_message(int level, const char *f, ...)
  * order to find the driver.
  *
  * \param driverName - a name like "i965", "radeon", "nouveau", etc.
+ * \param out_driver_handle - Address to return the resulting dlopen() handle.
  *
  * \returns
- * A handle from \c dlopen, or \c NULL if driver file not found.
+ * The __DRIextension entrypoint table for the driver, or \c NULL if driver
+ * file not found.
  */
-_X_HIDDEN void *
-driOpenDriver(const char *driverName)
+_X_HIDDEN const __DRIextension **
+driOpenDriver(const char *driverName, void **out_driver_handle)
 {
void *glhandle, *handle;
const char *libPaths, *p, *next;
@@ -148,10 +150,18 @@ driOpenDriver(const char *driverName)
if (glhandle)
   dlclose(glhandle);
 
-   return handle;
+   const __DRIextension **extensions = driGetDriverExtensions(handle,
+  driverName);
+   if (!extensions) {
+  dlclose(handle);
+  handle = NULL;
+   }
+
+   *out_driver_handle = handle;
+   return extensions;
 }
 
-_X_HIDDEN const __DRIextension **
+static const __DRIextension **
 driGetDriverExtensions(void *handle, const char *driver_name)
 {
const __DRIextension **extensions = NULL;
diff --git a/src/glx/dri_common.h b/src/glx/dri_common.h
index 4d97ff82b4d1..363f15bf9bb2 100644
--- a/src/glx/dri_common.h
+++ b/src/glx/dri_common.h
@@ -69,10 +69,8 @@ extern void dri_message(int level, const char *f, ...) 
PRINTFLIKE(2, 3);
 #define ErrorMessageF(...) dri_message(_LOADER_WARNING, __VA_ARGS__)
 #define CriticalErrorMessageF(...) dri_message(_LOADER_FATAL, __VA_ARGS__)
 
-extern void *driOpenDriver(const char *driverName);
-
-extern const __DRIextension **
-driGetDriverExtensions(void *handle, const char *driver_name);
+extern const __DRIextension **driOpenDriver(const char *driverName,
+void **out_driver_handle);
 
 extern bool
 dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
index 5c4346cec0d8..6e9412d3fb17 100644
--- a/src/glx/dri_glx.c
+++ b/src/glx/dri_glx.c
@@ -199,15 +199,9 @@ clear_driver_config_cache()
 static char *
 get_driver_config(const char *driverName)
 {
-   void *handle = driOpenDriver(driverName);
-   const __DRIextension **extensions;
-
-   if (!handle)
-  return NULL;
-
+   void *handle;
char *config = NULL;
-
-   extensions = driGetDriverExtensions(handle, driverName);
+   const __DRIextension **extensions = driOpenDriver(driverName, );
if (extensions) {
   for (int i = 0; extensions[i]; i++) {
  if (strcmp(extensions[i]->name, __DRI_CONFIG_OPTIONS) != 0)
@@ -918,11 +912,7 @@ driCreateScreen(int screen, struct glx_display *priv)
   goto cleanup;
}
 
-   psc->driver = driOpenDriver(driverName);
-   if (psc->driver == NULL)
-  goto cleanup;
-
-   extensions = dlsym(psc->driver, __DRI_DRIVER_EXTENSIONS);
+   extensions = driOpenDriver(driverName, >driver);
if (extensions == NULL) {
   ErrorMessageF("driver exports no extensions (%s)\n", dlerror());
   goto cleanup;
diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
index 

Re: [Mesa-dev] [PATCH] gbm: Clarify acceptable formats for gbm_bo

2018-11-15 Thread Eric Anholt
Daniel Stone  writes:

> gbm_bo_create() was presumably meant to originally accept gbm_bo_format
> enums, but it's accepted GBM_FORMAT_* tokens since the dawn of time.
> This is good, since gbm_bo_format is rarely used and covers a lot less
> ground than GBM_FORMAT_*.
>
> Change the documentation to refer to both; this involves removing a 'see
> also' for gbm_bo_format, since we can't also use \sa to refer to a
> family of anonymous #defines.
>
> Signed-off-by: Daniel Stone 
> Reported-by: Pekka Paalanen 

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH mesa] gbm: add new entrypoint to symbols check

2018-11-14 Thread Eric Anholt
Eric Engestrom  writes:

> Fixes: 6328536ff28ca26f2ad4e "gbm: Introduce a helper function for
>   printing GBM format names."
> Cc: Eric Anholt 
> Signed-off-by: Eric Engestrom 

r-b.

Oh, how I wish we had merge requests that were gated on CI.


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


Re: [Mesa-dev] [PATCH] mesa/st: swap order of clear() and clear_with_quad()

2018-11-13 Thread Eric Anholt
Rob Clark  writes:

> On Tue, Nov 13, 2018 at 5:25 PM Eric Anholt  wrote:
>>
>> Rob Clark  writes:
>>
>> > If we can't clear all the buffers with pctx->clear() (say, for example,
>> > because of ColorMask), push the buffers we *can* clear with pctx->clear()
>> > first.  Tilers want to see clears coming before draws to enable fast-
>> > paths, and clearing one of the attachments with a quad-draw first
>> > confuses that logic.
>>
>> Oh, nice!
>>
>> Reviewed-by: Eric Anholt 
>>
>> Though it feels pretty silly that the ->clear() caller needs a
>> clear_with_quad implementation when the ->clear() implementation in the
>> driver also needs a clear_with_quad implementation for non-fast-cleared
>> buffers.  :/
>
> hmm, so perhaps one easy option is to change pctx->clear() to return a
> boolean, so driver can return false to ask the state tracker to do a
> clear_with_quad()..  maybe that would be a first step towards allowing
> driver to handle clears w/ colormask and possibly scissor (although
> for the later, plus
> glInvalidateFramebuffer()/glInvalidateSubFramebuffer(), I was thinking
> of pctx->invalidate_surface()/pctx->invalidate_sub_surface()).

I was thinking you'd return the mask of what buffers you couldn't (fast)
clear.


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


Re: [Mesa-dev] [PATCH] mesa/st: swap order of clear() and clear_with_quad()

2018-11-13 Thread Eric Anholt
Rob Clark  writes:

> If we can't clear all the buffers with pctx->clear() (say, for example,
> because of ColorMask), push the buffers we *can* clear with pctx->clear()
> first.  Tilers want to see clears coming before draws to enable fast-
> paths, and clearing one of the attachments with a quad-draw first
> confuses that logic.

Oh, nice!

Reviewed-by: Eric Anholt 

Though it feels pretty silly that the ->clear() caller needs a
clear_with_quad implementation when the ->clear() implementation in the
driver also needs a clear_with_quad implementation for non-fast-cleared
buffers.  :/


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


Re: [Mesa-dev] [PATCH 2/2] kmsro: Extend to include hx8357d.

2018-11-12 Thread Eric Anholt
Emil Velikov  writes:

> On Tue, 30 Oct 2018 at 17:49, Eric Anholt  wrote:
>>
>> Emil Velikov  writes:
>>
>> > Hi Eric,
>> >
>> > On Thu, 25 Oct 2018 at 17:39, Eric Anholt  wrote:
>> >>
>> >> This allows vc4 to initialize on the Adafruit PiTFT 3.5" touchscreen with
>> >> the new tinydrm driver I just submitted.  If this series extending the
>> >> pl111/kmsro driver is accepted, then I'll extend kmsro with the other
>> >> tinydrm drivers as well.
>> >
>> > Have you considered teaching the DRI loader about this? Unlike the
>> > tegra driver there's nothing special going on here.
>> > Create a dumb buffer on the DC, export as dmabuf and let the GPU draw into 
>> > it.
>>
>> The problem with modifying mesa's loaders, as I mentioned in my other
>> reply, is that it means you need to update the xserver's loader and get
>> that deployed everywhere, as well.
>
> Agreed that retroactively fixing X isn't fun. At the same time, with
> the config file suggestion in mind, it's a one off change that will
> make code size and maintenance a lot easier.
> We've already handled that with mega drivers and it turned out just fine.

If I go this route, I get to replicate the code out for xserver,
GLX/DRI3, GLX/DRI2, EGL/DRI3, EGL/dri2, EGL/gbm, and whoops I don't even
know how to test with EGL/android.  It's massive code duplication.

With renderonly, you get to write C code in one place for this sharing.
I agree that having a symlink per KMS driver is ugly, and maybe we could
fix the loaders to fall back to trying kmsro, but I don't think the
loaders should be doing the prime sharing.


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


Re: [Mesa-dev] [RFC PATCH] gallium/nir: Add shader-based blending helpers

2018-11-12 Thread Eric Anholt
Alyssa Rosenzweig  writes:

>> I would love to see this up in src/compiler/nir.
>
> Is there a compelling reason to have it in NIR rather in Gallium, adding
> an extra layer of translation overhead / indirection when coming from
> Gallium blend state? (Mythical future Vulkan support, I guess?)

Exactly.  Also I think we had some reason for using it on i965,
somewhere.


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


Re: [Mesa-dev] [RFC PATCH] gallium/nir: Add shader-based blending helpers

2018-11-12 Thread Eric Anholt
Alyssa Rosenzweig  writes:

> Some mobile GPUs lack fixed-function hardware for blending, instead
> emulating blending via internal shaders. In particular for us, vc4 lacks
> most of the fragment pipeline, implementing blending in the epilogue of
> the fragment shader. Newer Malis supported by Panfrost have limitations
> in their fixed-function pipeline, introducing a dedicated "blend shader"
> stage to implement arbitrarily complex operations.
>
> This helper, originally from vc4, generates NIR programs from a Gallium
> blend state. At the moment, it implements only floating-point ES2 blend
> operations; integer blending and non-ES2 modes like logic ops are
> planned for the future.
>
> Signed-off-by: Alyssa Rosenzweig 

I would love to see this up in src/compiler/nir.  We'd have to define
enums for the blend factors in shader_enums.h, but we've done that for
compare funcs and advanced blend, already.  Also, we should probably
convert vc4 over in the process, so it's not just new unused code.

I'm also hoping that at some point I can figure out how to represent the
TLB loads properly in NIR (with respect to multisampling, in particular)
so that we can move the whole blend pipeline up.


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


Re: [Mesa-dev] [PATCH 2/2] util/ralloc: Make sizeof(linear_header) a multiple of 8

2018-11-12 Thread Eric Anholt
Matt Turner  writes:

> Prior to this patch sizeof(linear_header) was 20 bytes in a
> non-debug build on 32-bit platforms. We do some pointer arithmetic to
> calculate the next available location with
>
>ptr = (linear_size_chunk *)((char *)[1] + latest->offset);
>
> in linear_alloc_child(). The [1] adds 20 bytes, so an allocation
> would only be 4-byte aligned.
>
> On 32-bit SPARC a 'sttw' instruction (which stores a consecutive pair of
> 4-byte registers to memory) requires an 8-byte aligned address. Such an
> instruction is used to store to an 8-byte integer type, like intmax_t
> which is used in glcpp's expression_value_t struct.
>
> As a result of the 4-byte alignment returned by linear_alloc_child() we
> would generate a SIGBUS (unaligned exception) on SPARC.
>
> According to the GNU libc manual malloc() always returns memory that has
> at least an alignment of 8-bytes [1]. I think our allocator should do
> the same.
>
> So, simple fix with two parts:
>
>(1) Increase SUBALLOC_ALIGNMENT to 8 unconditionally.
>(2) Mark linear_header with an aligned attribute, which will cause
>its sizeof to be rounded up to that alignment. (We already do
>this for ralloc_header)
>
> With this done, all Mesa's unit tests now pass on SPARC.
>
> [1] 
> https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html
>
> Fixes: 47e17586924f ("glcpp: use the linear allocator for most objects")
> Bug: https://bugs.gentoo.org/636326
> ---
>  src/util/ralloc.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/util/ralloc.c b/src/util/ralloc.c
> index 745b4cf1226..fc35661996d 100644
> --- a/src/util/ralloc.c
> +++ b/src/util/ralloc.c
> @@ -552,10 +552,18 @@ ralloc_vasprintf_rewrite_tail(char **str, size_t 
> *start, const char *fmt,
>   */
>  
>  #define MIN_LINEAR_BUFSIZE 2048
> -#define SUBALLOC_ALIGNMENT sizeof(uintptr_t)
> +#define SUBALLOC_ALIGNMENT 8
>  #define LMAGIC 0x87b9c7d3
>  
> -struct linear_header {
> +struct
> +#ifdef _MSC_VER
> + __declspec(align(8))
> +#elif defined(__LP64__)
> + __attribute__((aligned(16)))
> +#else
> + __attribute__((aligned(8)))
> +#endif
> +   linear_header {
>  #ifndef NDEBUG
> unsigned magic;   /* for debugging */
>  #endif
> @@ -647,6 +655,8 @@ linear_alloc_child(void *parent, unsigned size)
> ptr = (linear_size_chunk *)((char*)[1] + latest->offset);
> ptr->size = size;
> latest->offset += full_size;
> +
> +   assert((uintptr_t)[1] % SUBALLOC_ALIGNMENT == 0);
> return [1];
>  }

These patches are:

Reviewed-by: Eric Anholt 

However, shouldn't we also bump SUBALLOC_ALIGNMENT to 16 on LP64, too,
if that's what glibc is doing for malloc?


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


Re: [Mesa-dev] [PATCH 4/5] nir: Make nir_lower_clip_vs optionally work with variables.

2018-11-12 Thread Eric Anholt
Kenneth Graunke  writes:

> The way nir_lower_clip_vs() works with store_output intrinsics makes a
> ton of assumptions about the driver_location field.
>
> In i965, I'd rather do this lowering early and work with variables.
> ir3 and vc4 could probably do that as well, but I'm not sure exactly
> what paths would need updating, so for now, handle both methods.
> ---
>  src/compiler/nir/nir.h  |  2 +-
>  src/compiler/nir/nir_lower_clip.c   | 45 -
>  src/gallium/drivers/freedreno/ir3/ir3_nir.c |  2 +-
>  src/gallium/drivers/vc4/vc4_program.c   |  3 +-

If you hit src/broadcom/compiler/vir.c as well, the series is:

Reviewed-by: Eric Anholt 

Switching vc4/v3d stuff over to variables has been going well and
showing wins -- I think we should probably finish that work and then
simplify these paths by moving this lowering before lower_io.

In the past, I thought we would want to lower_io at gallium state
creation time, since it was some compile work we could do at
compile/link instead of draw.


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


Re: [Mesa-dev] [PATCH] st/mesa: make use of nir_link_constant_varyings()

2018-11-09 Thread Eric Anholt
Timothy Arceri  writes:

> Shader-db results radeonsi (VEGA):
>
> Totals from affected shaders:
> SGPRS: 161464 -> 161368 (-0.06 %)
> VGPRS: 86904 -> 86292 (-0.70 %)
> Spilled SGPRs: 296 -> 314 (6.08 %)
> Spilled VGPRs: 0 -> 0 (0.00 %)
> Private memory VGPRs: 0 -> 0 (0.00 %)
> Scratch size: 0 -> 0 (0.00 %) dwords per thread
> Code Size: 3618596 -> 3573852 (-1.24 %) bytes
> LDS: 0 -> 0 (0.00 %) blocks
> Max Waves: 26189 -> 26276 (0.33 %)
> Wait states: 0 -> 0 (0.00 %)

I was thinking I wanted to write this patch.

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH v2] nir: add new linking opt nir_move_out_const_to_consumer()

2018-11-08 Thread Eric Anholt
Timothy Arceri  writes:

> This pass moves constant outputs to the consuming shader stage
> where possible.
>
> V2: limit pass to scalars for now
> ---
>
>  V2 doesn't change any shader-db/vkpipeline-db results as all 32bit
>  varyings that we don't skip are already scalar. V2 just avoids a
>  potential bug with doubles as we don't currently split those in
>  nir_lower_io_to_scalar_early().
>
>  src/compiler/nir/nir.h|   2 +
>  src/compiler/nir/nir_linking_helpers.c| 110 ++
>  src/mesa/state_tracker/st_glsl_to_nir.cpp |   2 +-
>  3 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index a0ae9a4430e..0b8c8578953 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2800,6 +2800,8 @@ bool nir_remove_unused_io_vars(nir_shader *shader, 
> struct exec_list *var_list,
>  void nir_compact_varyings(nir_shader *producer, nir_shader *consumer,
>bool default_to_smooth_interp);
>  void nir_link_xfb_varyings(nir_shader *producer, nir_shader *consumer);
> +bool nir_move_out_const_to_consumer(nir_shader *producer,
> +nir_shader *consumer);
>  
>  typedef enum {
> /* If set, this forces all non-flat fragment shader inputs to be
> diff --git a/src/compiler/nir/nir_linking_helpers.c 
> b/src/compiler/nir/nir_linking_helpers.c
> index de6f2481def..a03c327a12f 100644
> --- a/src/compiler/nir/nir_linking_helpers.c
> +++ b/src/compiler/nir/nir_linking_helpers.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "nir.h"
> +#include "nir_builder.h"
>  #include "util/set.h"
>  #include "util/hash_table.h"
>  
> @@ -556,3 +557,112 @@ nir_link_xfb_varyings(nir_shader *producer, nir_shader 
> *consumer)
>}
> }
>  }
> +
> +static bool
> +try_replace_constant_input(nir_shader *shader,
> +   nir_intrinsic_instr *store_intr)
> +{
> +   nir_variable *out_var =
> +  nir_deref_instr_get_variable(nir_src_as_deref(store_intr->src[0]));
> +
> +   if (out_var->data.mode != nir_var_shader_out)
> +  return false;
> +
> +   /* Skip types that require more complex handling.
> +* TODO: add support for these types.
> +*/
> +   if (glsl_type_is_array(out_var->type) ||
> +   glsl_type_is_dual_slot(out_var->type) ||
> +   glsl_type_is_matrix(out_var->type) ||
> +   glsl_type_is_struct(out_var->type))
> +  return false;
> +
> +   /* Limit this pass to scalars for now to keep things simple. Most varyings
> +* should have been lowered to scalars at this point anyway.
> +*/
> +   if (store_intr->num_components != 1)
> +  return false;
> +
> +   if (out_var->data.location < VARYING_SLOT_VAR0 ||
> +   out_var->data.location - VARYING_SLOT_VAR0 >= MAX_VARYING)
> +  return false;
> +
> +   nir_function_impl *impl = nir_shader_get_entrypoint(shader);
> +
> +   nir_builder b;
> +   nir_builder_init(, impl);
> +
> +   bool progress = false;
> +   nir_foreach_block(block, impl) {
> +  nir_foreach_instr(instr, block) {
> + if (instr->type != nir_instr_type_intrinsic)
> +continue;
> +
> + nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
> + if (intr->intrinsic != nir_intrinsic_load_deref)
> +continue;
> +
> + nir_variable *in_var =
> +nir_deref_instr_get_variable(nir_src_as_deref(intr->src[0]));
> +
> + if (in_var->data.mode != nir_var_shader_in)
> +continue;
> +
> + if (in_var->data.location != out_var->data.location ||
> + in_var->data.location_frac != out_var->data.location_frac)
> +continue;
> +
> + b.cursor = nir_before_instr(instr);
> +
> + nir_load_const_instr *out_const =
> +nir_instr_as_load_const(store_intr->src[1].ssa->parent_instr);
> +
> + /* Add new const to replace the input */
> + nir_ssa_def *nconst = nir_build_imm(, store_intr->num_components,
> + intr->dest.ssa.bit_size,
> + out_const->value);
> +
> + nir_ssa_def_rewrite_uses(>dest.ssa, nir_src_for_ssa(nconst));
> +
> + progress = true;
> +  }
> +   }
> +
> +   return progress;
> +}
> +
> +bool
> +nir_move_out_const_to_consumer(nir_shader *producer, nir_shader *consumer)
> +{
> +   /* TODO: Add support for more shader stage combinations */
> +   if (consumer->info.stage != MESA_SHADER_FRAGMENT ||
> +   (producer->info.stage != MESA_SHADER_VERTEX &&
> +producer->info.stage != MESA_SHADER_TESS_EVAL))
> +  return false;
> +
> +   bool progress = false;
> +
> +   nir_function_impl *impl = nir_shader_get_entrypoint(producer);
> +
> +   /* If we find a store in the last block of the producer we can be sure 
> this
> +* is the only possible value for this output.
> +*/
> +   nir_block *last_block = nir_impl_last_block(impl);
> 

[Mesa-dev] [PATCH 1/3] gbm: Move gbm_format_canonicalize() to the core.

2018-11-08 Thread Eric Anholt
I want it for the format name debugging code.
---
 src/gbm/backends/dri/gbm_dri.c | 16 
 src/gbm/main/gbm.c | 16 
 src/gbm/main/gbmint.h  |  3 +++
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index 35ec3a1c3a2a..f32c0cd9885f 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -594,22 +594,6 @@ static const struct gbm_dri_visual gbm_dri_visuals_table[] 
= {
},
 };
 
-/* The two GBM_BO_FORMAT_[XA]RGB formats alias the GBM_FORMAT_*
- * formats of the same name. We want to accept them whenever someone
- * has a GBM format, but never return them to the user. */
-static int
-gbm_format_canonicalize(uint32_t gbm_format)
-{
-   switch (gbm_format) {
-   case GBM_BO_FORMAT_XRGB:
-  return GBM_FORMAT_XRGB;
-   case GBM_BO_FORMAT_ARGB:
-  return GBM_FORMAT_ARGB;
-   default:
-  return gbm_format;
-   }
-}
-
 static int
 gbm_format_to_dri_format(uint32_t gbm_format)
 {
diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
index 0bf2922bacdd..d301661b48ee 100644
--- a/src/gbm/main/gbm.c
+++ b/src/gbm/main/gbm.c
@@ -695,3 +695,19 @@ gbm_surface_has_free_buffers(struct gbm_surface *surf)
 {
return surf->gbm->surface_has_free_buffers(surf);
 }
+
+/* The two GBM_BO_FORMAT_[XA]RGB formats alias the GBM_FORMAT_*
+ * formats of the same name. We want to accept them whenever someone
+ * has a GBM format, but never return them to the user. */
+uint32_t
+gbm_format_canonicalize(uint32_t gbm_format)
+{
+   switch (gbm_format) {
+   case GBM_BO_FORMAT_XRGB:
+  return GBM_FORMAT_XRGB;
+   case GBM_BO_FORMAT_ARGB:
+  return GBM_FORMAT_ARGB;
+   default:
+  return gbm_format;
+   }
+}
diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
index 9220a4ae87da..192577431e25 100644
--- a/src/gbm/main/gbmint.h
+++ b/src/gbm/main/gbmint.h
@@ -133,4 +133,7 @@ struct gbm_backend {
struct gbm_device *(*create_device)(int fd);
 };
 
+uint32_t
+gbm_format_canonicalize(uint32_t gbm_format);
+
 #endif
-- 
2.19.1

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


[Mesa-dev] [PATCH 3/3] egl: Improve the debugging of gbm format matching in DRI configs.

2018-11-08 Thread Eric Anholt
Previously the debug would be:

libEGL debug: No DRI config supports native format 0x20203852
libEGL debug: No DRI config supports native format 0x38385247

but

libEGL debug: No DRI config supports native format R8
libEGL debug: No DRI config supports native format GR88

is a lot easier to understand.

Reviewed-by: Eric Engestrom 
---
 src/egl/drivers/dri2/platform_drm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index 68ec8322e978..fb346e007332 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -664,8 +664,9 @@ drm_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay 
*disp)
 
for (unsigned i = 0; i < ARRAY_SIZE(format_count); i++) {
   if (!format_count[i]) {
- _eglLog(_EGL_DEBUG, "No DRI config supports native format 0x%x",
- visuals[i].gbm_format);
+ struct gbm_format_name_desc desc;
+ _eglLog(_EGL_DEBUG, "No DRI config supports native format %s",
+ gbm_format_get_name(visuals[i].gbm_format, ));
   }
}
 
-- 
2.19.1

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


[Mesa-dev] [PATCH 2/3] gbm: Introduce a helper function for printing GBM format names.

2018-11-08 Thread Eric Anholt
This requires that the caller make a little (stack) allocation to store
the string.

v2: Use gbm_format_canonicalize (suggested by Daniel)
---
 src/gbm/main/gbm.c | 20 
 src/gbm/main/gbm.h |  6 ++
 2 files changed, 26 insertions(+)

diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
index d301661b48ee..2e2014205cb8 100644
--- a/src/gbm/main/gbm.c
+++ b/src/gbm/main/gbm.c
@@ -711,3 +711,23 @@ gbm_format_canonicalize(uint32_t gbm_format)
   return gbm_format;
}
 }
+
+/**
+ * Returns a string representing the fourcc format name.
+ *
+ * \param desc Caller-provided storage for the format name string.
+ * \return String containing the fourcc of the format.
+ */
+GBM_EXPORT char *
+gbm_format_get_name(uint32_t gbm_format, struct gbm_format_name_desc *desc)
+{
+   gbm_format = gbm_format_canonicalize(gbm_format);
+
+   desc->name[0] = gbm_format;
+   desc->name[1] = gbm_format >> 8;
+   desc->name[2] = gbm_format >> 16;
+   desc->name[3] = gbm_format >> 24;
+   desc->name[4] = 0;
+
+   return desc->name;
+}
diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
index e95f9e34960b..9b5288710a5b 100644
--- a/src/gbm/main/gbm.h
+++ b/src/gbm/main/gbm.h
@@ -190,6 +190,9 @@ enum gbm_bo_format {
 #define GBM_FORMAT_YUV444  __gbm_fourcc_code('Y', 'U', '2', '4') /* 
non-subsampled Cb (1) and Cr (2) planes */
 #define GBM_FORMAT_YVU444  __gbm_fourcc_code('Y', 'V', '2', '4') /* 
non-subsampled Cr (1) and Cb (2) planes */
 
+struct gbm_format_name_desc {
+   char name[5];
+};
 
 /**
  * Flags to indicate the intended use for the buffer - these are passed into
@@ -399,6 +402,9 @@ gbm_surface_has_free_buffers(struct gbm_surface *surface);
 void
 gbm_surface_destroy(struct gbm_surface *surface);
 
+char *
+gbm_format_get_name(uint32_t gbm_format, struct gbm_format_name_desc *desc);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.19.1

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


Re: [Mesa-dev] [PATCH 1/3] nir: add new linking opt nir_move_out_const_to_consumer()

2018-11-07 Thread Eric Anholt
Timothy Arceri  writes:

> On 7/11/18 7:40 pm, Samuel Pitoiset wrote:
>> IIRC, I wrote a pass similar to this one a while ago, but never finished 
>> it.
>> 
>> I don't really like the helper name though, how about 
>> nir_link_immediate_varyings()? or nir_link_constant_varyings()?
>
> Sure. Happy to change the name. I''ll go with 
> nir_link_constant_varyings() unless there are any other objections.

Haven't reviewed the code, but I like that name.


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


Re: [Mesa-dev] [PATCH] radv/nir: avoid packing vertex outputs we can eliminate

2018-11-06 Thread Eric Anholt
Timothy Arceri  writes:

> On Mon, Nov 5, 2018, at 4:44 PM, Timothy Arceri wrote:
>> On 6/11/18 11:30 am, Eric Anholt wrote:
>> > Timothy Arceri  writes:
>> > 
>> >> For now I have only enabled this for RADV we can do it
>> >> also for radeonsi also but we need to add a CAP for it.
>> > 
>> > If we're doing this at link time, why not push the constant value from
>> > the producer into the consumer shader and then cut out the varying
>> > entirely?
>> > 
>> 
>> Maybe we could look to do this between vertex stages but we can do this 
>
> Sorry that should be: "can't do this"   
>
>> for fragment inputs (which is what this patch is for) unless they are 
>> marked as flat (which almost never happens). Or I'm I missing something?

If the vertex shader is providing a constant for all the vertices you're
interpolating between, how is that different from being flat?


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


Re: [Mesa-dev] [PATCH] radv/nir: avoid packing vertex outputs we can eliminate

2018-11-05 Thread Eric Anholt
Timothy Arceri  writes:

> For now I have only enabled this for RADV we can do it
> also for radeonsi also but we need to add a CAP for it.

If we're doing this at link time, why not push the constant value from
the producer into the consumer shader and then cut out the varying
entirely?


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


Re: [Mesa-dev] [PATCH] gallium/util: don't let children of fork & exec inherit our thread affinity

2018-11-05 Thread Eric Anholt
"Haehnle, Nicolai"  writes:

> On 31.10.18 21:41, Marek Olšák wrote:
>> Instead of saying that, you should say "Mesa should be slower with most 
>> apps, so that we don't decrease perf for 2 apps too much", because 
>> that's what you are saying. It seems like you have some limited belief 
>> that only allows you to see the problem on one side (2 apps 
>> significantly slower) and prevents you from seeing the problem on the 
>> other side (most apps slower), so for some twisted reason or because you 
>> just wanna have the feeling of "being right", you would rather see most 
>> apps slower than 2 apps significantly slower. I don't give a damn if I'm 
>> right. I could be wrong all day and learn. And you shouldn't give a damn 
>> if you are right either, because if you do, it shows a fragile ego. I'm 
>> looking for objectively the best solution with the least maintenance. 
>> The patch on the mailing list gives me the least maintenance as far as I 
>> can tell. If you have another solution that needs just as little 
>> maintenance, I'm all ears.
>
> So, on a personal level, I feel like maybe your phrasing was a bit harsh 
> and could be toned down.
>
> On the technical level, the problem is that while yes, only a small 
> number of apps are negatively affected, the negative effect as reported 
> is *absolutely massive*. The positive effect of the affinity hack, on 
> the other hand, is obviously decent and very welcome, but unfortunately 
> nowhere near as big.
>
> The reality of users is that most of them don't complain when they're 
> confronted with sucky software. Or they complain within their circles, 
> slowly contributing to a negative reputation of the software in a way 
> that doesn't reach us. (That's a general pattern which causes most 
> software to suck more than necessary.)
>
> We (and really mostly you of all people) have worked very hard to 
> improve our reputation for producing great drivers. We shouldn't 
> squander that.
>
> So I think on the technical merits, the others on this thread are right. 
> The explicit affinity setting was a good initiative and turned out to be 
> very useful, but unfortunately we really need to go with a whitelisting 
> approach at this point, while making sure that the whitelist option gets 
> good exposure on Phoronix et al.
>
> The real long-term solution would be anyway to figure out why the kernel 
> doesn't do a good job of placing the Gallium driver thread on the right 
> CCX, and then fixing that. To be honest, I've always thought that
> the explicit affinity approach is more of a hack. And the proper 
> long-term approach is quite likely to help improve the performance of 
> Ryzen / Threadripper / EPYC in general, not just for Mesa. But my 
> educated guess is that that's not something any of us can do in a week 
> (though I'd be happy for you to prove me wrong :)), and in any case, it 
> takes a long time for kernel improvements like that to filter through to 
> end users, so going with a whitelist seems the right thing to do either way.

I completely agree.


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 1/2] gbm: Introduce a helper function for printing GBM format names.

2018-11-02 Thread Eric Anholt
This requires that the caller make a little (stack) allocation to store
the string.
---
 src/gbm/main/gbm.c | 18 ++
 src/gbm/main/gbm.h |  6 ++
 2 files changed, 24 insertions(+)

diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
index 0bf2922bacdd..174f62ad797f 100644
--- a/src/gbm/main/gbm.c
+++ b/src/gbm/main/gbm.c
@@ -695,3 +695,21 @@ gbm_surface_has_free_buffers(struct gbm_surface *surf)
 {
return surf->gbm->surface_has_free_buffers(surf);
 }
+
+/**
+ * Returns a string representing the fourcc format name.
+ *
+ * \param desc Caller-provided storage for the format name string.
+ * \return String containing the fourcc of the format.
+ */
+GBM_EXPORT char *
+gbm_format_get_name(uint32_t gbm_format, struct gbm_format_name_desc *desc)
+{
+   desc->name[0] = gbm_format;
+   desc->name[1] = gbm_format >> 8;
+   desc->name[2] = gbm_format >> 16;
+   desc->name[3] = gbm_format >> 24;
+   desc->name[4] = 0;
+
+   return desc->name;
+}
diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
index e95f9e34960b..9b5288710a5b 100644
--- a/src/gbm/main/gbm.h
+++ b/src/gbm/main/gbm.h
@@ -190,6 +190,9 @@ enum gbm_bo_format {
 #define GBM_FORMAT_YUV444  __gbm_fourcc_code('Y', 'U', '2', '4') /* 
non-subsampled Cb (1) and Cr (2) planes */
 #define GBM_FORMAT_YVU444  __gbm_fourcc_code('Y', 'V', '2', '4') /* 
non-subsampled Cr (1) and Cb (2) planes */
 
+struct gbm_format_name_desc {
+   char name[5];
+};
 
 /**
  * Flags to indicate the intended use for the buffer - these are passed into
@@ -399,6 +402,9 @@ gbm_surface_has_free_buffers(struct gbm_surface *surface);
 void
 gbm_surface_destroy(struct gbm_surface *surface);
 
+char *
+gbm_format_get_name(uint32_t gbm_format, struct gbm_format_name_desc *desc);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.19.1

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


[Mesa-dev] [PATCH 2/2] egl: Improve the debugging of gbm format matching in DRI configs.

2018-11-02 Thread Eric Anholt
Previously the debug would be:

libEGL debug: No DRI config supports native format 0x20203852
libEGL debug: No DRI config supports native format 0x38385247

but

libEGL debug: No DRI config supports native format R8
libEGL debug: No DRI config supports native format GR88

is a lot easier to understand.
---
 src/egl/drivers/dri2/platform_drm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index 68ec8322e978..fb346e007332 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -664,8 +664,9 @@ drm_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay 
*disp)
 
for (unsigned i = 0; i < ARRAY_SIZE(format_count); i++) {
   if (!format_count[i]) {
- _eglLog(_EGL_DEBUG, "No DRI config supports native format 0x%x",
- visuals[i].gbm_format);
+ struct gbm_format_name_desc desc;
+ _eglLog(_EGL_DEBUG, "No DRI config supports native format %s",
+ gbm_format_get_name(visuals[i].gbm_format, ));
   }
}
 
-- 
2.19.1

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


Re: [Mesa-dev] [PATCH RFC] egl: Add a 565 pbuffer-only EGL config under X11.

2018-10-30 Thread Eric Anholt
Eric Anholt  writes:

> The CTS requires a 565-no-depth-no-stencil config for ES 3.0, but at depth
> 24 of X11 we wouldn't do so.  We can satisfy that bad requirement using a
> pbuffer-only visual with whatever other buffers the driver happens to have
> given us.

Anyone?  Still concerned about getting Mesa to pass the current
conformance suite.


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


Re: [Mesa-dev] [PATCH 2/2] kmsro: Extend to include hx8357d.

2018-10-30 Thread Eric Anholt
Emil Velikov  writes:

> Hi Eric,
>
> On Thu, 25 Oct 2018 at 17:39, Eric Anholt  wrote:
>>
>> This allows vc4 to initialize on the Adafruit PiTFT 3.5" touchscreen with
>> the new tinydrm driver I just submitted.  If this series extending the
>> pl111/kmsro driver is accepted, then I'll extend kmsro with the other
>> tinydrm drivers as well.
>
> Have you considered teaching the DRI loader about this? Unlike the
> tegra driver there's nothing special going on here.
> Create a dumb buffer on the DC, export as dmabuf and let the GPU draw into it.

The problem with modifying mesa's loaders, as I mentioned in my other
reply, is that it means you need to update the xserver's loader and get
that deployed everywhere, as well.


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


Re: [Mesa-dev] [PATCH 2/2] kmsro: Extend to include hx8357d.

2018-10-26 Thread Eric Anholt
Eric Engestrom  writes:

> On Thursday, 2018-10-25 09:39:10 -0700, Eric Anholt wrote:
>> This allows vc4 to initialize on the Adafruit PiTFT 3.5" touchscreen with
>> the new tinydrm driver I just submitted.  If this series extending the
>> pl111/kmsro driver is accepted, then I'll extend kmsro with the other
>> tinydrm drivers as well.
>> ---
>>  src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c | 5 +
>>  src/gallium/drivers/kmsro/Android.mk| 1 +
>>  src/gallium/drivers/kmsro/Automake.inc  | 1 +
>>  src/gallium/targets/dri/meson.build | 1 +
>>  src/gallium/targets/dri/target.c| 1 +
>>  5 files changed, 9 insertions(+)
>> 
>> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c 
>> b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> index 230bafe5e159..73ddab0cbf02 100644
>> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
>> @@ -111,6 +111,11 @@ static const struct drm_driver_descriptor 
>> driver_descriptors[] = {
>>  .create_screen = pipe_kmsro_create_screen,
>>  .configuration = pipe_default_configuration_query,
>>  },
>> +{
>> +   .driver_name = "hx8357d",
>> +.create_screen = pipe_kmsro_create_screen,
>> +.configuration = pipe_default_configuration_query,
>> +},
>
> Is that really all it takes to add a new userspace tinydrm driver? Impressive!
>
> (nit: `.driver_name` missing one space of indentation)
>
> Code-wise, patch #1 is:
> Reviewed-by: Eric Engestrom 
>
> Not completely sure this is all that's needed for this patch, so only:
> Acked-by: Eric Engestrom 
>
> That said, ... (scroll down)
>
>>  {
>>  .driver_name = "virtio_gpu",
>>  .create_screen = pipe_virgl_create_screen,
>> diff --git a/src/gallium/drivers/kmsro/Android.mk 
>> b/src/gallium/drivers/kmsro/Android.mk
>> index 8a851024dc88..f6a444e8865b 100644
>> --- a/src/gallium/drivers/kmsro/Android.mk
>> +++ b/src/gallium/drivers/kmsro/Android.mk
>> @@ -35,5 +35,6 @@ include $(BUILD_STATIC_LIBRARY)
>>  
>>  ifneq ($(HAVE_GALLIUM_KMSRO),)
>>  GALLIUM_TARGET_DRIVERS += pl111
>> +GALLIUM_TARGET_DRIVERS += hx8357d
>>  $(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_kmsro)
>>  endif
>> diff --git a/src/gallium/drivers/kmsro/Automake.inc 
>> b/src/gallium/drivers/kmsro/Automake.inc
>> index 66d125cb440a..d5961c907653 100644
>> --- a/src/gallium/drivers/kmsro/Automake.inc
>> +++ b/src/gallium/drivers/kmsro/Automake.inc
>> @@ -1,6 +1,7 @@
>>  if HAVE_GALLIUM_KMSRO
>>  
>>  TARGET_DRIVERS += pl111
>> +TARGET_DRIVERS += hx8357d
>>  TARGET_CPPFLAGS += -DGALLIUM_KMSRO
>>  TARGET_LIB_DEPS += \
>>  $(top_builddir)/src/gallium/winsys/kmsro/drm/libkmsrodrm.la \
>> diff --git a/src/gallium/targets/dri/meson.build 
>> b/src/gallium/targets/dri/meson.build
>> index c1cb616b4dad..bc63702498ba 100644
>> --- a/src/gallium/targets/dri/meson.build
>> +++ b/src/gallium/targets/dri/meson.build
>> @@ -63,6 +63,7 @@ libgallium_dri = shared_library(
>>  )
>>  
>>  foreach d : [[with_gallium_kmsro, 'pl111_dri.so'],
>> + [with_gallium_kmsro, 'hx8357d_dri.so'],
>
> ... wouldn't we want the user-facing `gallium` option to have the
> `pl111,hx8357d` granularity?
>
> It would be more work, so as a follow-up patch, and they're just
> hardlinks of the same file anyway so they don't really take any disk
> space or compilation time, so I'm not sure it's worth it, but asking the
> question anyway :)

My thought was that since each of the tinydrm gallium drivers is the
same code and just one more loader entrypoint and drm_driver_descriptor
struct, everyone's better off if we don't list a million names in the
meson/autotools options and you just get them all as a group.

What I actually think I want is the loader to know that if you didn't
find a native driver and you've got a KMS node, it should try seeing if
kmsro_dri.so likes your KMS node.  However, that would require Xorg to
also know about this idea (when can we switch AIGLX over to EGL?).


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


Re: [Mesa-dev] [PATCH v1 1/5] meson: compilation flags for sse

2018-10-26 Thread Eric Anholt
Dylan Baker  writes:

> [ Unknown signature status ]
> CC'ing Matt and Emil.
>
> This will make the code non-portable from an SSE capable system to a non-sse
> capable system. That's a pretty old system at this point (Pentium III and 
> Athlon
> XP), and I don't personaly care, but it's quite possible that distros do care.
>
> What do you guys think?

Yeah, we can't do that.


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


Re: [Mesa-dev] [PATCH mesa] util: remove unnecessary random whitespaces

2018-10-25 Thread Eric Anholt
Ian Romanick  writes:

> On 10/25/2018 05:13 AM, Eric Engestrom wrote:
>> On Thursday, 2018-10-25 17:54:16 +1100, Timothy Arceri wrote:
>>> On 25/10/18 7:42 am, Ian Romanick wrote:
 On 10/23/2018 04:15 AM, Eric Engestrom wrote:
> Suggested-by: Timothy Arceri 
>>>
>>> Um no :P I suggested you fix the formatting in your patch to match the Mesa
>>> style.
>> 
>> Right, sorry, you suggested fixing the formatting, but not the fix
>> I went with, so I should've dropped this tag.
>> 
>>>
> Signed-off-by: Eric Engestrom 
> ---
> Timothy, I opted to remove them all instead of adding even more, as it
> would break again next time something changes (the set_foreach() one was
> already broken before my patch for instance) and result in lots of
> unnecessary churn for seemingly no gain, and I don't like hiding the
> backslash away (it hinders readability).

 NAK... we use this formatting everywhere in Mesa.  The point is to move
 the \ characters out of the way.  When you're trying to read a
 multi-line macro, they are distracting, so it is nice to move them over.
>> 
>> I don't have the same opinion, but respecting mesa style is the point
>> here, so I added those whitespace chars, squashed them in the previous
>> patches, and pushed them.
>> 
>> Sorry for this patch :)
>
> It's always worth a try. :)  Everyone has aspects of the Mesa style that
> they don't like.  Sending out a patch like this is a good way to test
> the waters about changing the style.  It does happen from time to time.
> It's better to do it like this than to try to sneak it in a big patch
> series.

I would love to see this style change.  emacs in my experience does
horrible things to these weird (inconsistently-)aligned \s, so I have to
manually align them and fail.


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] kmsro: Extend to include hx8357d.

2018-10-25 Thread Eric Anholt
This allows vc4 to initialize on the Adafruit PiTFT 3.5" touchscreen with
the new tinydrm driver I just submitted.  If this series extending the
pl111/kmsro driver is accepted, then I'll extend kmsro with the other
tinydrm drivers as well.
---
 src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c | 5 +
 src/gallium/drivers/kmsro/Android.mk| 1 +
 src/gallium/drivers/kmsro/Automake.inc  | 1 +
 src/gallium/targets/dri/meson.build | 1 +
 src/gallium/targets/dri/target.c| 1 +
 5 files changed, 9 insertions(+)

diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c 
b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
index 230bafe5e159..73ddab0cbf02 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
@@ -111,6 +111,11 @@ static const struct drm_driver_descriptor 
driver_descriptors[] = {
 .create_screen = pipe_kmsro_create_screen,
 .configuration = pipe_default_configuration_query,
 },
+{
+   .driver_name = "hx8357d",
+.create_screen = pipe_kmsro_create_screen,
+.configuration = pipe_default_configuration_query,
+},
 {
 .driver_name = "virtio_gpu",
 .create_screen = pipe_virgl_create_screen,
diff --git a/src/gallium/drivers/kmsro/Android.mk 
b/src/gallium/drivers/kmsro/Android.mk
index 8a851024dc88..f6a444e8865b 100644
--- a/src/gallium/drivers/kmsro/Android.mk
+++ b/src/gallium/drivers/kmsro/Android.mk
@@ -35,5 +35,6 @@ include $(BUILD_STATIC_LIBRARY)
 
 ifneq ($(HAVE_GALLIUM_KMSRO),)
 GALLIUM_TARGET_DRIVERS += pl111
+GALLIUM_TARGET_DRIVERS += hx8357d
 $(eval GALLIUM_LIBS += $(LOCAL_MODULE) libmesa_winsys_kmsro)
 endif
diff --git a/src/gallium/drivers/kmsro/Automake.inc 
b/src/gallium/drivers/kmsro/Automake.inc
index 66d125cb440a..d5961c907653 100644
--- a/src/gallium/drivers/kmsro/Automake.inc
+++ b/src/gallium/drivers/kmsro/Automake.inc
@@ -1,6 +1,7 @@
 if HAVE_GALLIUM_KMSRO
 
 TARGET_DRIVERS += pl111
+TARGET_DRIVERS += hx8357d
 TARGET_CPPFLAGS += -DGALLIUM_KMSRO
 TARGET_LIB_DEPS += \
 $(top_builddir)/src/gallium/winsys/kmsro/drm/libkmsrodrm.la \
diff --git a/src/gallium/targets/dri/meson.build 
b/src/gallium/targets/dri/meson.build
index c1cb616b4dad..bc63702498ba 100644
--- a/src/gallium/targets/dri/meson.build
+++ b/src/gallium/targets/dri/meson.build
@@ -63,6 +63,7 @@ libgallium_dri = shared_library(
 )
 
 foreach d : [[with_gallium_kmsro, 'pl111_dri.so'],
+ [with_gallium_kmsro, 'hx8357d_dri.so'],
  [with_gallium_radeonsi, 'radeonsi_dri.so'],
  [with_gallium_nouveau, 'nouveau_dri.so'],
  [with_gallium_freedreno, ['msm_dri.so', 'kgsl_dri.so']],
diff --git a/src/gallium/targets/dri/target.c b/src/gallium/targets/dri/target.c
index 2821cef197ea..6b0ea785c405 100644
--- a/src/gallium/targets/dri/target.c
+++ b/src/gallium/targets/dri/target.c
@@ -78,6 +78,7 @@ DEFINE_LOADER_DRM_ENTRYPOINT(v3d)
 #if defined(GALLIUM_VC4)
 DEFINE_LOADER_DRM_ENTRYPOINT(vc4)
 #if defined(GALLIUM_KMSRO)
+DEFINE_LOADER_DRM_ENTRYPOINT(hx8357d)
 DEFINE_LOADER_DRM_ENTRYPOINT(pl111)
 #endif
 #endif
-- 
2.19.1

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


[Mesa-dev] [PATCH 1/2] pl111: Rename the pl111 driver to "kmsro".

2018-10-25 Thread Eric Anholt
The vc4 driver can do prime sharing to many different KMS-only devices,
such as the various tinydrm drivers for SPI-attached displays.  Rename the
driver away from "pl111" to represent what it will actually support:
various sorts of KMS displays with the renderonly layer used to attach a
GPU.
---
 .travis.yml  |  2 +-
 Android.mk   |  4 ++--
 Makefile.am  |  2 +-
 configure.ac | 16 
 meson.build  |  8 
 meson_options.txt|  2 +-
 src/gallium/Android.mk   |  2 +-
 src/gallium/Makefile.am  |  4 ++--
 .../auxiliary/pipe-loader/pipe_loader_drm.c  |  2 +-
 .../auxiliary/target-helpers/drm_helper.h| 12 ++--
 .../auxiliary/target-helpers/drm_helper_public.h |  2 +-
 src/gallium/drivers/{pl111 => kmsro}/Android.mk  |  6 +++---
 src/gallium/drivers/kmsro/Automake.inc   |  9 +
 src/gallium/drivers/{pl111 => kmsro}/Makefile.am |  4 ++--
 .../drivers/{pl111 => kmsro}/Makefile.sources|  0
 src/gallium/drivers/pl111/Automake.inc   |  9 -
 src/gallium/meson.build  |  6 +++---
 src/gallium/targets/dri/Makefile.am  |  2 +-
 src/gallium/targets/dri/meson.build  |  4 ++--
 src/gallium/targets/dri/target.c |  2 +-
 .../winsys/{pl111 => kmsro}/drm/Android.mk   |  2 +-
 .../winsys/{pl111 => kmsro}/drm/Makefile.am  |  4 ++--
 src/gallium/winsys/kmsro/drm/Makefile.sources|  3 +++
 .../drm/kmsro_drm_public.h}  |  8 
 .../drm/kmsro_drm_winsys.c}  |  6 +++---
 .../winsys/{pl111 => kmsro}/drm/meson.build  | 12 ++--
 src/gallium/winsys/pl111/drm/Makefile.sources|  3 ---
 27 files changed, 68 insertions(+), 68 deletions(-)
 rename src/gallium/drivers/{pl111 => kmsro}/Android.mk (91%)
 create mode 100644 src/gallium/drivers/kmsro/Automake.inc
 rename src/gallium/drivers/{pl111 => kmsro}/Makefile.am (55%)
 rename src/gallium/drivers/{pl111 => kmsro}/Makefile.sources (100%)
 delete mode 100644 src/gallium/drivers/pl111/Automake.inc
 rename src/gallium/winsys/{pl111 => kmsro}/drm/Android.mk (97%)
 rename src/gallium/winsys/{pl111 => kmsro}/drm/Makefile.am (94%)
 create mode 100644 src/gallium/winsys/kmsro/drm/Makefile.sources
 rename src/gallium/winsys/{pl111/drm/pl111_drm_public.h => 
kmsro/drm/kmsro_drm_public.h} (89%)
 rename src/gallium/winsys/{pl111/drm/pl111_drm_winsys.c => 
kmsro/drm/kmsro_drm_winsys.c} (92%)
 rename src/gallium/winsys/{pl111 => kmsro}/drm/meson.build (87%)
 delete mode 100644 src/gallium/winsys/pl111/drm/Makefile.sources

diff --git a/.travis.yml b/.travis.yml
index 78e6d251ae4b..8bcd77143569 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -165,7 +165,7 @@ matrix:
 - DRI_LOADERS="--disable-glx --disable-gbm --disable-egl"
 - DRI_DRIVERS=""
 - GALLIUM_ST="--enable-dri --disable-opencl --disable-xa 
--disable-nine --disable-xvmc --disable-vdpau --disable-va 
--disable-omx-bellagio --disable-gallium-osmesa"
-- 
GALLIUM_DRIVERS="i915,nouveau,pl111,r300,r600,freedreno,svga,swrast,v3d,vc4,virgl,etnaviv,imx"
+- 
GALLIUM_DRIVERS="i915,nouveau,kmsro,r300,r600,freedreno,svga,swrast,v3d,vc4,virgl,etnaviv,imx"
 - VULKAN_DRIVERS=""
 - LIBUNWIND_FLAGS="--enable-libunwind"
   addons:
diff --git a/Android.mk b/Android.mk
index 914854c27d63..1a0bdd1736cf 100644
--- a/Android.mk
+++ b/Android.mk
@@ -24,7 +24,7 @@
 # BOARD_GPU_DRIVERS should be defined.  The valid values are
 #
 #   classic drivers: i915 i965
-#   gallium drivers: swrast freedreno i915g nouveau pl111 r300g r600g radeonsi 
vc4 virgl vmwgfx etnaviv imx
+#   gallium drivers: swrast freedreno i915g nouveau kmsro r300g r600g radeonsi 
vc4 virgl vmwgfx etnaviv imx
 #
 # The main target is libGLES_mesa.  For each classic driver enabled, a DRI
 # module will also be built.  DRI modules will be loaded by libGLES_mesa.
@@ -52,7 +52,7 @@ gallium_drivers := \
freedreno.HAVE_GALLIUM_FREEDRENO \
i915g.HAVE_GALLIUM_I915 \
nouveau.HAVE_GALLIUM_NOUVEAU \
-   pl111.HAVE_GALLIUM_PL111 \
+   kmsro.HAVE_GALLIUM_KMSRO \
r300g.HAVE_GALLIUM_R300 \
r600g.HAVE_GALLIUM_R600 \
radeonsi.HAVE_GALLIUM_RADEONSI \
diff --git a/Makefile.am b/Makefile.am
index 9e27db046e52..62c755aeca7f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -45,7 +45,7 @@ AM_DISTCHECK_CONFIGURE_FLAGS = \
--enable-libunwind \
--with-platforms=x11,wayland,drm,surfaceless \
--with-dri-drivers=i915,i965,nouveau,radeon,r200,swrast \
-   
--with-gallium-drivers=i915,nouveau,r300,pl111,r600,radeonsi,freedreno,svga,swrast,vc4,tegra,virgl,swr,etnaviv,imx
 \
+   

Re: [Mesa-dev] [PATCH mesa] util: use *unsigned* ints for bit operations

2018-10-22 Thread Eric Anholt
Eric Engestrom  writes:

> Fixes errors thrown by GCC's Undefined Behaviour sanitizer (ubsan) every
> time this macro is used.

Reviewed-by: Eric Anholt 


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


Re: [Mesa-dev] [PATCH] nir: Allow using nir_lower_io_to_scalar_early on VS input vars.

2018-10-22 Thread Eric Anholt
Eric Anholt  writes:

> This will be used on V3D to cut down the size of the VS inputs in the VPM
> (memory area for sharing data between shader stages).

Ping on this one.  I'd love to land the series behind it.


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


Re: [Mesa-dev] [PATCH 10/31] nir/prog: Use nir_bany in kill handling

2018-10-22 Thread Eric Anholt
Jason Ekstrand  writes:

> We have a helper that does exactly what the bany_inequal was doing.  It
> emits the same code but is a bit higher level and is designed to operate
> on a bvec4.

Patch 6, 8-10 also r-b.  I couldn't make sense of patch 7's optimization
pass beforehand, so I failed to review it.


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


Re: [Mesa-dev] [PATCH 01/31] nir/validate: Print when the validation failed

2018-10-22 Thread Eric Anholt
Jason Ekstrand  writes:

> diff --git a/src/mesa/drivers/dri/i965/brw_program.c 
> b/src/mesa/drivers/dri/i965/brw_program.c
> index f5ebd3c3b05..78050cda359 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -91,14 +91,14 @@ brw_create_nir(struct brw_context *brw,
>  
>nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
>nir_lower_returns(nir);
> -  nir_validate_shader(nir);
> +  nir_validate_shader(nir, NULL);
>NIR_PASS_V(nir, nir_lower_io_to_temporaries,
>   nir_shader_get_entrypoint(nir), true, false);
> } else {
>nir = prog_to_nir(prog, options);
>NIR_PASS_V(nir, nir_lower_regs_to_ssa); /* turn registers into SSA */
> }
> -   nir_validate_shader(nir);
> +   nir_validate_shader(nir, NULL);

It seems like you ought to have valid where args here.  Other than that,
patch 1-5 are:

Reviewed-by: Eric Anholt 


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] nir: Allow using nir_lower_io_to_scalar_early on VS input vars.

2018-10-15 Thread Eric Anholt
This will be used on V3D to cut down the size of the VS inputs in the VPM
(memory area for sharing data between shader stages).
---
 src/compiler/nir/nir_lower_io_to_scalar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_lower_io_to_scalar.c 
b/src/compiler/nir/nir_lower_io_to_scalar.c
index f0c2a6a95d6d..c64f641a0ae4 100644
--- a/src/compiler/nir/nir_lower_io_to_scalar.c
+++ b/src/compiler/nir/nir_lower_io_to_scalar.c
@@ -320,7 +320,9 @@ nir_lower_io_to_scalar_early(nir_shader *shader, 
nir_variable_mode mask)
if (glsl_type_is_64bit(glsl_without_array(var->type)))
   continue;
 
-   if (var->data.location < VARYING_SLOT_VAR0 &&
+   if (!(shader->info.stage == MESA_SHADER_VERTEX &&
+ mode == nir_var_shader_in) &&
+   var->data.location < VARYING_SLOT_VAR0 &&
var->data.location >= 0)
   continue;
 
-- 
2.18.0

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


Re: [Mesa-dev] [PATCH v2 0/5] Use GitLab CI to build Mesa

2018-09-30 Thread Eric Anholt
Daniel Stone  writes:

> Hi all,
>
> On Fri, 21 Sep 2018 at 20:59, Daniel Stone  wrote:
>> On Wed, 29 Aug 2018 at 11:13, Juan A. Suarez Romero  
>> wrote:
>> > This is a first part, version 2, of a more complete proposal to use GitLab 
>> > CI to
>> > build and test Mesa. This first part just adds the required pieces to build
>> > Mesa, using the different supported tools (meson, autotools, and scons).
>> >
>> > A second part, to be sent in the future, will use the results of the 
>> > former to
>> > run different tests and verify everything works fine.
>> >
>> > An example of the pipeline that will result from this patchset can be seen 
>> > at
>> > https://gitlab.freedesktop.org/jasuarez/mesa/pipelines/3070.
>> >
>> > I hope I can explain here all the rationale behind this proposal. Any 
>> > question
>> > is welcomed.
>>
>> I still have an open NAK since this would DoS our existing fd.o CI
>> runner infrastructure; GitLab itself is fine, but subjecting our
>> elderly build machine with a flaky network on the wrong continent to
>> this would be too much.
>>
>> I'm pretty sure that I'll be able to stand up two sets of runners -
>> one still based in the UK, but on modern and dedicated machines with
>> better network; another on a US-based cloud provider, matching our
>> us-east GitLab master location - before XDC starts, so during XDC we
>> could get those as dedicated runners for a fork of the Mesa project
>> and try to gather some stats and profiles on how they actually
>> perform.
>>
>> Hopefully if it goes well then we should be able to switch them on
>> almost immediately.
>
> Unfortunately we're still externally stalled on this. I'd suggest that
> we give Juan owner access to the Mesa project in GitLab, so he can
> connect the existing Igalia runners to our project. These runners
> already do the execution for the gitlab.com project, so should be more
> than enough to run ours until we can get the external runners.

That sounds great to me.


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] nir: Expose nir_remove_unused_io_vars().

2018-09-28 Thread Eric Anholt
For gallium drivers where you want to do some linking at variant compile
time, you don't have the other producer/consumer shader on hand to modify.
By exposing the inner function, the driver can have the used varyings in
the compiled shader cache key and still do linking.

This is also useful for V3D, where the binning shader wants to only output
position and TF varyings.  We've been removing those after nir_lower_io,
but this will be less driver-specific code and let more of the shader get
DCEed early in NIR.
---
 src/compiler/nir/nir.h |  3 +++
 src/compiler/nir/nir_linking_helpers.c | 32 +++---
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index e0df95c391c9..387efc8595e4 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2755,6 +2755,9 @@ void nir_assign_var_locations(struct exec_list *var_list, 
unsigned *size,
 
 /* Some helpers to do very simple linking */
 bool nir_remove_unused_varyings(nir_shader *producer, nir_shader *consumer);
+bool nir_remove_unused_io_vars(nir_shader *shader, struct exec_list *var_list,
+   uint64_t *used_by_other_stage,
+   uint64_t *used_by_other_stage_patches);
 void nir_compact_varyings(nir_shader *producer, nir_shader *consumer,
   bool default_to_smooth_interp);
 
diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c
index 7446bb826f97..85677b7c176a 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -92,10 +92,26 @@ tcs_add_output_reads(nir_shader *shader, uint64_t *read, 
uint64_t *patches_read)
}
 }
 
-static bool
-remove_unused_io_vars(nir_shader *shader, struct exec_list *var_list,
-  uint64_t *used_by_other_stage,
-  uint64_t *used_by_other_stage_patches)
+/**
+ * Helper for removing unused shader I/O variables, by demoting them to global
+ * variables (which may then by dead code eliminated).
+ *
+ * Example usage is:
+ *
+ * progress = nir_remove_unused_io_vars(producer,
+ *  >outputs,
+ *  read, patches_read) ||
+ *  progress;
+ *
+ * The "used" should be an array of 4 uint64_ts (probably of VARYING_BIT_*)
+ * representing each .location_frac used.  Note that for vector variables,
+ * only the first channel (.location_frac) is examined for deciding if the
+ * variable is used!
+ */
+bool
+nir_remove_unused_io_vars(nir_shader *shader, struct exec_list *var_list,
+  uint64_t *used_by_other_stage,
+  uint64_t *used_by_other_stage_patches)
 {
bool progress = false;
uint64_t *used;
@@ -169,11 +185,11 @@ nir_remove_unused_varyings(nir_shader *producer, 
nir_shader *consumer)
   tcs_add_output_reads(producer, read, patches_read);
 
bool progress = false;
-   progress = remove_unused_io_vars(producer, >outputs, read,
-patches_read);
+   progress = nir_remove_unused_io_vars(producer, >outputs, read,
+patches_read);
 
-   progress = remove_unused_io_vars(consumer, >inputs, written,
-patches_written) || progress;
+   progress = nir_remove_unused_io_vars(consumer, >inputs, written,
+patches_written) || progress;
 
return progress;
 }
-- 
2.18.0

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


[Mesa-dev] [PATCH 1/2] nir: Be sure to fix deref modes after demoting shader i/o vars to global.

2018-09-28 Thread Eric Anholt
Fixes assertion failures when calling nir_remove_unused_varyings() or
nir_remove_unused_io_vars().
---
 src/compiler/nir/nir_linking_helpers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1c2..7446bb826f97 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -126,6 +126,9 @@ remove_unused_io_vars(nir_shader *shader, struct exec_list 
*var_list,
   }
}
 
+   if (progress)
+  nir_fixup_deref_modes(shader);
+
return progress;
 }
 
-- 
2.18.0

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


<    1   2   3   4   5   6   7   8   9   10   >