[Mesa-dev] [PATCH 24.5/24] i965/cnl: Add a preliminary device for Cannonlake

2017-06-02 Thread Anuj Phogat
From: Ben Widawsky 

v2 (Anuj):
Rebased on master and updated pci ids
Remove redundant initialization of max_wm_threads to 64 * 12.
For gen9+ max_wm_threads are initialized in gen_get_device_info().

v3 (Anuj):
Move the patch to end of series.
Remove unused gt1, gt2, gt3 functions.
Remove l3_banks variable. Variable is now available on master.

Signed-off-by: Anuj Phogat 
Signed-off-by: Ben Widawsky 
Cc: Jason Ekstrand 
---
 include/pci_ids/i965_pci_ids.h | 12 ++
 src/intel/common/gen_device_info.c | 46 ++
 2 files changed, 58 insertions(+)

diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
index 17504f5..b296359 100644
--- a/include/pci_ids/i965_pci_ids.h
+++ b/include/pci_ids/i965_pci_ids.h
@@ -165,3 +165,15 @@ CHIPSET(0x5927, kbl_gt3, "Intel(R) Iris Plus Graphics 650 
(Kaby Lake GT3)")
 CHIPSET(0x593B, kbl_gt4, "Intel(R) Kabylake GT4")
 CHIPSET(0x3184, glk, "Intel(R) HD Graphics (Geminilake)")
 CHIPSET(0x3185, glk_2x6, "Intel(R) HD Graphics (Geminilake 2x6)")
+CHIPSET(0x5A49, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
+CHIPSET(0x5A4A, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
+CHIPSET(0x5A41, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
+CHIPSET(0x5A42, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
+CHIPSET(0x5A44, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
+CHIPSET(0x5A59, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
+CHIPSET(0x5A5A, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
+CHIPSET(0x5A5C, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
+CHIPSET(0x5A50, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
+CHIPSET(0x5A51, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
+CHIPSET(0x5A52, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
+CHIPSET(0x5A54, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
diff --git a/src/intel/common/gen_device_info.c 
b/src/intel/common/gen_device_info.c
index 47aed9d..f62fccb 100644
--- a/src/intel/common/gen_device_info.c
+++ b/src/intel/common/gen_device_info.c
@@ -555,6 +555,52 @@ static const struct gen_device_info 
gen_device_info_glk_2x6 = {
GEN9_LP_FEATURES_2X6
 };
 
+#define GEN10_HW_INFO   \
+   .gen = 10,   \
+   .max_vs_threads = 728,   \
+   .max_gs_threads = 432,   \
+   .max_tcs_threads = 432,  \
+   .max_tes_threads = 624,  \
+   .max_cs_threads = 56,\
+   .urb = { \
+  .size = 256,  \
+  .min_entries = {  \
+ [MESA_SHADER_VERTEX]= 64,  \
+ [MESA_SHADER_TESS_EVAL] = 34,  \
+  },\
+  .max_entries = {  \
+  [MESA_SHADER_VERTEX]   = 3936,\
+  [MESA_SHADER_TESS_CTRL]= 896, \
+  [MESA_SHADER_TESS_EVAL]= 2064,\
+  [MESA_SHADER_GEOMETRY] = 832, \
+  },\
+   }
+
+#define GEN10_FEATURES(_gt, _slices, _l3)   \
+   GEN8_FEATURES,   \
+   GEN10_HW_INFO,   \
+   .gt = _gt, .num_slices = _slices, .l3_banks = _l3
+
+static const struct gen_device_info gen_device_info_cnl_2x8 = {
+   /* GT0.5 */
+   GEN10_FEATURES(1, 1, 2)
+};
+
+static const struct gen_device_info gen_device_info_cnl_3x8 = {
+   /* GT1 */
+   GEN10_FEATURES(1, 1, 3)
+};
+
+static const struct gen_device_info gen_device_info_cnl_4x8 = {
+   /* GT 1.5 */
+   GEN10_FEATURES(1, 2, 6)
+};
+
+static const struct gen_device_info gen_device_info_cnl_5x8 = {
+   /* GT2 */
+   GEN10_FEATURES(2, 2, 6)
+};
+
 bool
 gen_get_device_info(int devid, struct gen_device_info *devinfo)
 {
-- 
2.9.3

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


Re: [Mesa-dev] [PATCH] nvc0: Disable BGRA8 images on Fermi

2017-06-02 Thread Ilia Mirkin
Actually, for unknown reasons, storing to BGRA8 images doesn't work on
Fermi. This breaks PBO downloads.

Reviewed-by: Ilia Mirkin 

On Fri, Jun 2, 2017 at 8:45 PM, Lyude  wrote:
> For unknown reasons, using BGRA8 images on Fermi results in breaking
> reads from PBOs, such that they always return 0x0. Discovered this
> through a glamor bug, and confirmed it does indeed break a good number
> of piglit tests such as spec/arb_pixel_buffer_object/pbo-read-argb
>
> Until we have a proper fix, just disable this functionality on Fermi.
>
> Signed-off-by: Lyude 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index c636926..f6c5c72 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -87,11 +87,20 @@ nvc0_screen_is_format_supported(struct pipe_screen 
> *pscreen,
> bindings &= ~(PIPE_BIND_LINEAR |
>   PIPE_BIND_SHARED);
>
> -   if (bindings & PIPE_BIND_SHADER_IMAGE && sample_count > 1 &&
> -   nouveau_screen(pscreen)->class_3d >= GM107_3D_CLASS) {
> -  /* MS images are currently unsupported on Maxwell because they have to
> -   * be handled explicitly. */
> -  return false;
> +   if (bindings & PIPE_BIND_SHADER_IMAGE) {
> +  if (sample_count > 1 &&
> +  nouveau_screen(pscreen)->class_3d >= GM107_3D_CLASS) {
> + /* MS images are currently unsupported on Maxwell because they have 
> to
> +  * be handled explicitly. */
> + return false;
> +  }
> +
> +  if (format == PIPE_FORMAT_B8G8R8A8_UNORM &&
> +  nouveau_screen(pscreen)->class_3d < NVE4_3D_CLASS) {
> + /* This should work on Fermi, but for currently unknown reasons it
> +  * does not and results in breaking reads from pbos. */
> + return false;
> +  }
> }
>
> return (( nvc0_format_table[format].usage |
> --
> 2.9.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 02/24] i965/cnl: Add a preliminary device for Cannonlake

2017-06-02 Thread Anuj Phogat
On Fri, Jun 2, 2017 at 4:56 PM, Jason Ekstrand  wrote:
> Perhaps this should probably be the last patch in the series as it's the one
> that actually enables CNL support?  Otherwise, you could get a commit with
> PCI ids but a completely busted driver.
>
That's a good point. I'll move the patch to end of this series before
pushing upstream.

> On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:
>>
>> From: Ben Widawsky 
>>
>> v2 (Anuj):
>> Rebased on master and updated pci ids
>> Remove redundant initialization of max_wm_threads to 64 * 12.
>> For gen9+ max_wm_threads are initialized in gen_get_device_info().
>>
>> Signed-off-by: Anuj Phogat 
>> Signed-off-by: Ben Widawsky 
>> ---
>>  include/pci_ids/i965_pci_ids.h | 12 
>>  src/intel/common/gen_device_info.c | 58
>> ++
>>  src/intel/common/gen_device_info.h |  1 +
>>  3 files changed, 71 insertions(+)
>>
>> diff --git a/include/pci_ids/i965_pci_ids.h
>> b/include/pci_ids/i965_pci_ids.h
>> index 17504f5..b296359 100644
>> --- a/include/pci_ids/i965_pci_ids.h
>> +++ b/include/pci_ids/i965_pci_ids.h
>> @@ -165,3 +165,15 @@ CHIPSET(0x5927, kbl_gt3, "Intel(R) Iris Plus Graphics
>> 650 (Kaby Lake GT3)")
>>  CHIPSET(0x593B, kbl_gt4, "Intel(R) Kabylake GT4")
>>  CHIPSET(0x3184, glk, "Intel(R) HD Graphics (Geminilake)")
>>  CHIPSET(0x3185, glk_2x6, "Intel(R) HD Graphics (Geminilake 2x6)")
>> +CHIPSET(0x5A49, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
>> +CHIPSET(0x5A4A, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
>> +CHIPSET(0x5A41, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
>> +CHIPSET(0x5A42, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
>> +CHIPSET(0x5A44, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
>> +CHIPSET(0x5A59, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
>> +CHIPSET(0x5A5A, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
>> +CHIPSET(0x5A5C, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
>> +CHIPSET(0x5A50, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
>> +CHIPSET(0x5A51, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
>> +CHIPSET(0x5A52, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
>> +CHIPSET(0x5A54, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
>> diff --git a/src/intel/common/gen_device_info.c
>> b/src/intel/common/gen_device_info.c
>> index 47aed9d..87edb94 100644
>> --- a/src/intel/common/gen_device_info.c
>> +++ b/src/intel/common/gen_device_info.c
>> @@ -555,6 +555,64 @@ static const struct gen_device_info
>> gen_device_info_glk_2x6 = {
>> GEN9_LP_FEATURES_2X6
>>  };
>>
>> +#define GEN10_HW_INFO   \
>> +   .gen = 10,   \
>> +   .max_vs_threads = 728,   \
>> +   .max_gs_threads = 432,   \
>> +   .max_tcs_threads = 432,  \
>> +   .max_tes_threads = 624,  \
>> +   .max_cs_threads = 56,\
>> +   .urb = { \
>> +  .size = 256,  \
>> +  .min_entries = {  \
>> + [MESA_SHADER_VERTEX]= 64,  \
>> + [MESA_SHADER_TESS_EVAL] = 34,  \
>> +  },\
>> +  .max_entries = {  \
>> +  [MESA_SHADER_VERTEX]   = 3936,\
>> +  [MESA_SHADER_TESS_CTRL]= 896, \
>> +  [MESA_SHADER_TESS_EVAL]= 2064,\
>> +  [MESA_SHADER_GEOMETRY] = 832, \
>> +  },\
>> +   }
>> +
>> +#define GEN10_FEATURES(_gt, _slices, _l3)   \
>> +   GEN8_FEATURES,   \
>> +   GEN10_HW_INFO,   \
>> +   .gt = _gt, .num_slices = _slices, .l3_banks = _l3
>> +
>> +static const struct gen_device_info gen_device_info_cnl_2x8 = {
>> +   /* GT0.5 */
>> +   GEN10_FEATURES(1, 1, 2)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_3x8 = {
>> +   /* GT1 */
>> +   GEN10_FEATURES(1, 1, 3)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_4x8 = {
>> +   /* GT 1.5 */
>> +   GEN10_FEATURES(1, 2, 6)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_5x8 = {
>> +   /* GT2 */
>> +   GEN10_FEATURES(2, 2, 6)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_gt1 = {
>> +   GEN10_FEATURES(1, 1, 3)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_gt2 = {
>> +   GEN10_FEATURES(2, 2, 6)
>> +};
>> +
>> +static const struct gen_device_info gen_device_info_cnl_gt3 = {
>> +   GEN10_FEATURES(3, 4, 12)
>> +};
>> +
I'll also get rid of 

Re: [Mesa-dev] [PATCH V2 00/24] Add Cannonlake support

2017-06-02 Thread Anuj Phogat
On Fri, Jun 2, 2017 at 4:48 PM, Jason Ekstrand  wrote:
> On Mon, May 22, 2017 at 9:32 AM, Anuj Phogat  wrote:
>>
>> On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat 
>> wrote:
>> > This series adds support for Cannonlake.
>> >
>> > Changes from V1 to V2:
>> > - Incorporated the review comments from V1.
>> > - Rebased 8 months old CNL branch on top of master
>> > - Wired up Linux and Android build files for gen10
>> > - Replaced the use of few gen9 functions with gen10 specific functions.
>> > - Squashed few patches, dropped few and created new patches.
>> >
>> Thanks to Jason and Ken who have reviewed few patches in this series.
>> Rest of them are still waiting for the review. I really want to land this
>> series
>> (at least first 15-16 patches) soon. There are some very easy patches any
>> one can review like enabling Mesa to build for gen10 etc. Please take a
>> look at them. Thanks :).
>
>
> Finally got around to looking at these again...
>
> Now that we're switching everything over to genxml, I think it's a good idea
> to be a bit more intentional in the way we write new platform patches.
> There are a number of patches in this series that are much harder to review
> than they need to be because they're written more-or-less in order of code
> development and not in a logical reviewable order.  In particular, there's a
> patch which updates a pile of switch statements to get rid of asserts but it
> just moves them all over to gen9.  Moving stuff to gen10 is in a different
> patch.  The result is that it's very hard, without squashing things
> together, to tell whether or not we missed anything when we switched them
> over to actual gen10 functions.  This isn't really a criticism of Anuj and
> Ben.  They've done a lot of rebasaing on top of a lot of driver architecture
> changes.  I think this will be much easier to do better in the future.
>
> In my view, the ideal platform enabling patch series would look something
> like this:
>
>  1) Add genN.xml
Does it make sense to break it down in to few patches based on manual
changes we make to an auto generated genN.xml ? or send out just one
patch and reviewer can diff it with previous gen and verify the changes ?

>  2) Add the #defines and #includes to genxml and the build system stuff to
> generate the packing headers
>  3) Update stuff in src/intel/common such as URB configuration changes
>  4) Update ISL:
> a) Any needed generic ISL changes such as adding new layaouts.  (Cannon
> lake doesn't add anything, so nothing to do here).  This may be multiple
> patches.
> b) Get ISL surface state emit code building for the new hardware.  This
> includes updating the autotools and Android makefiles, adding function
> prototypes, updating switch statements, etc.  If changes are needed in
> isl_surface_state.c or isl_depth_stencil.c, they should be minimal bug still
> enough that the end result is correct.
> 5) Update BLORP as needed for the new platform.  Sadly, there's no way to
> build-test this without the next step since BLORP doesn't build its own genX
> files.
> 6) Get GL driver genxml state-upload and blorp code building and hooked in.
> Core blorp changes should go in their own patch (above) but this will
> include the build system changes for blorp as well.  This also includes
> updating switch statements.
> 8) Implement workarounds, features, etc.
>
> The important part is that we keep related changes together.  In order to
> review this series, I had to squash patches 6, 9, 14, and 15 together.
> Again, I'm not casting any blame here.  The series makes a lot of historical
> sense.  It's just hard to review as-is.
>
I agree with you that the series could have been structured better for ease
of review. Thanks for the reviews and all the great suggestions above. I'll
keep them in mind for future h/w enabling patches.

> --Jason
>
>>
>> > What's remaining:
>> > - Add missing gen10 bits in Vulkan driver.
>> > - Fix failing piglit, cts tests for GL and Vulkan.
>> >
>> > You can also find this series at:
>> > https://github.com/aphogat/mesa.git
>> > branch: reviews
>> >
>> > Anuj Phogat (18):
>> >   i965/cnl: Define genX(x) and GENX(x) for gen10
>> >   i965/cnl: Include gen10_pack.h
>>
>> >   i965/cnl: Add gen10 specific function declarations
>> >   i965/cnl: Update the script generating genX_bits.h
>> >   i965/cnl: Add isl_gen10 header and source files
>> >   i965/cnl: Wire up Mesa build files for gen10
>> >   i965/cnl: Wire up android Mesa build files for gen10
>> >   i965/cnl: Add pci id for INTEL_DEVID_OVERRIDE
>> >   i965/cnl: Add cnl bits in aubinator
>> >   i965/cnl: Update few assertions
>> >   i965/cnl: Handle gen10 in switch cases across the driver
>> >   i965/cnl: Start using CNL MOCS defines
>> >   i965/cnl: Start using gen10 specific functions
>> >   i965/cnl: Don't resolve single sampled color rb in case of sRGB
>> > formats
>> >   i965/cnl: Make URB {VS, GS, HS, DS} sizes 

[Mesa-dev] [PATCH] nvc0: Disable BGRA8 images on Fermi

2017-06-02 Thread Lyude
For unknown reasons, using BGRA8 images on Fermi results in breaking
reads from PBOs, such that they always return 0x0. Discovered this
through a glamor bug, and confirmed it does indeed break a good number
of piglit tests such as spec/arb_pixel_buffer_object/pbo-read-argb

Until we have a proper fix, just disable this functionality on Fermi.

Signed-off-by: Lyude 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index c636926..f6c5c72 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -87,11 +87,20 @@ nvc0_screen_is_format_supported(struct pipe_screen *pscreen,
bindings &= ~(PIPE_BIND_LINEAR |
  PIPE_BIND_SHARED);
 
-   if (bindings & PIPE_BIND_SHADER_IMAGE && sample_count > 1 &&
-   nouveau_screen(pscreen)->class_3d >= GM107_3D_CLASS) {
-  /* MS images are currently unsupported on Maxwell because they have to
-   * be handled explicitly. */
-  return false;
+   if (bindings & PIPE_BIND_SHADER_IMAGE) {
+  if (sample_count > 1 &&
+  nouveau_screen(pscreen)->class_3d >= GM107_3D_CLASS) {
+ /* MS images are currently unsupported on Maxwell because they have to
+  * be handled explicitly. */
+ return false;
+  }
+
+  if (format == PIPE_FORMAT_B8G8R8A8_UNORM &&
+  nouveau_screen(pscreen)->class_3d < NVE4_3D_CLASS) {
+ /* This should work on Fermi, but for currently unknown reasons it
+  * does not and results in breaking reads from pbos. */
+ return false;
+  }
}
 
return (( nvc0_format_table[format].usage |
-- 
2.9.4

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


Re: [Mesa-dev] [PATCH 15/30] i965/miptree: Add new entrypoints for resolve management

2017-06-02 Thread Jason Ekstrand
On Fri, Jun 2, 2017 at 4:30 PM, Nanley Chery  wrote:

> On Fri, May 26, 2017 at 04:30:19PM -0700, Jason Ekstrand wrote:
> > This commit adds a new unified interface for doing resolves.  The basic
> > format is that, prior to any surface access such as texturing or
> > rendering, you call intel_miptree_prepare_access.  If the surface was
> > written, you call intel_miptree_finish_write.  These two functions take
> > parameters which tell them whether or not auxiliary compression and fast
> > clears are supported on the surface.  Later commits will add wrappers
> > around these two functions for texturing, rendering, etc.
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 156
> +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  80 +
> >  2 files changed, 232 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 6cd32ce..0659e75 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -2028,8 +2028,7 @@ bool
> >  intel_miptree_all_slices_resolve_hiz(struct brw_context *brw,
> >struct intel_mipmap_tree *mt)
> >  {
> > -   return intel_miptree_depth_hiz_resolve(brw, mt,
> > -  0, UINT32_MAX, 0, UINT32_MAX,
> > +   return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0,
> UINT32_MAX,
> >BLORP_HIZ_OP_HIZ_RESOLVE);
> >  }
> >
> > @@ -2037,8 +2036,7 @@ bool
> >  intel_miptree_all_slices_resolve_depth(struct brw_context *brw,
> >  struct intel_mipmap_tree *mt)
> >  {
> > -   return intel_miptree_depth_hiz_resolve(brw, mt,
> > -  0, UINT32_MAX, 0, UINT32_MAX,
> > +   return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0,
> UINT32_MAX,
> >BLORP_HIZ_OP_DEPTH_RESOLVE);
> >  }
> >
> > @@ -2221,6 +2219,156 @@ intel_miptree_all_slices_resolve_color(struct
> brw_context *brw,
> > intel_miptree_resolve_color(brw, mt, 0, UINT32_MAX, 0, UINT32_MAX,
> flags);
> >  }
> >
> > +void
> > +intel_miptree_prepare_access(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt,
> > + uint32_t start_level, uint32_t num_levels,
> > + uint32_t start_layer, uint32_t num_layers,
> > + bool aux_supported, bool
> fast_clear_supported)
> > +{
> > +   if (_mesa_is_format_color_format(mt->format)) {
> > +  if (!mt->mcs_buf)
> > + return;
> > +
> > +  if (mt->num_samples > 1) {
> > + /* Nothing to do for MSAA */
> > +  } else {
> > + /* TODO: This is fairly terrible.  We can do better. */
> > + if (!aux_supported || !fast_clear_supported) {
> > +intel_miptree_resolve_color(brw, mt, start_level,
> num_levels,
> > +start_layer, num_layers, 0);
> > + }
> > +  }
> > +   } else if (mt->format == MESA_FORMAT_S_UINT8) {
> > +  /* Nothing to do for stencil */
> > +   } else {
> > +  if (!mt->hiz_buf)
> > + return;
> > +
> > +  if (aux_supported) {
> > + assert(fast_clear_supported);
> > + intel_miptree_depth_hiz_resolve(brw, mt, start_level,
> num_levels,
> > + start_layer, num_layers,
> > + BLORP_HIZ_OP_HIZ_RESOLVE);
> > +  } else {
> > + assert(!fast_clear_supported);
> > + intel_miptree_depth_hiz_resolve(brw, mt, start_level,
> num_levels,
> > + start_layer, num_layers,
> > + BLORP_HIZ_OP_DEPTH_RESOLVE);
> > +  }
> > +   }
> > +}
> > +
> > +void
> > +intel_miptree_finish_write(struct brw_context *brw,
> > +   struct intel_mipmap_tree *mt, uint32_t level,
> > +   uint32_t start_layer, uint32_t num_layers,
> > +   bool written_with_aux)
> > +{
> > +   assert(start_layer < mt->level[level].depth);
> > +   if (num_layers == INTEL_REMAINING_LAYERS)
> > +  num_layers = mt->level[level].depth - start_layer;
> > +   assert(num_layers <= mt->level[level].depth - start_layer);
> > +
> > +   if (_mesa_is_format_color_format(mt->format)) {
> > +  if (mt->num_samples > 1) {
> > + /* Nothing to do for MSAA */
> > +  } else {
> > + if (written_with_aux) {
> > +intel_miptree_used_for_rendering(brw, mt, level,
> > + start_layer, num_layers);
> > + }
> > +  }
> > +   } else if (mt->format == MESA_FORMAT_S_UINT8) {
> > +  /* Nothing to do for stencil */
> > +   } 

Re: [Mesa-dev] [PATCH 17/24] i965/cnl: Implement new pipe control workaround

2017-06-02 Thread Jason Ekstrand
On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> From: Ben Widawsky 
>
> GEN10 requires flushing all previous pipe controls before issuing a render
> target cache flush. The docs seem to fairly explicitly say this is gen10
> only.
>
> V2 (by Anuj): Use flags & PIPE_CONTROL_RENDER_TARGET_FLUSH check. (Ilia)
>   Use recursive call to brw_emit_pipe_control_flush().
>
> Signed-off-by: Ben Widawsky 
> Signed-off-by: Anuj Phogat 
> ---
>  src/mesa/drivers/dri/i965/brw_pipe_control.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index f4ede2d..ecf2fac 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -128,6 +128,17 @@ brw_emit_pipe_control_flush(struct brw_context *brw,
> uint32_t flags)
>   brw_emit_pipe_control_flush(brw, 0);
>}
>
> +  if (brw->gen == 10 && (flags & PIPE_CONTROL_RENDER_TARGET_FLUSH)) {
> +/* Hardware workaround: CNL
> + *
> + * "Before sending a PIPE_CONTROL command with bit 12 set, SW
> + * must issue another PIPE_CONTROL with Render Target Cache
> + * Flush Enable (bit 12) = 0 and Pipe Control Flush Enable
> + * (bit 7) = 1."
>

A more specific spec citation would be good.  Maybe something like:

>From the "Gen10 Workarounds" page:

In any case, the workaround looks correct although the docs say it only
applies to A0 stepping.  Is this still even needed?


> + */
> + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE);
> +  }
> +
>BEGIN_BATCH(6);
>OUT_BATCH(_3DSTATE_PIPE_CONTROL | (6 - 2));
>OUT_BATCH(flags);
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 02/24] i965/cnl: Add a preliminary device for Cannonlake

2017-06-02 Thread Jason Ekstrand
Perhaps this should probably be the last patch in the series as it's the
one that actually enables CNL support?  Otherwise, you could get a commit
with PCI ids but a completely busted driver.

On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> From: Ben Widawsky 
>
> v2 (Anuj):
> Rebased on master and updated pci ids
> Remove redundant initialization of max_wm_threads to 64 * 12.
> For gen9+ max_wm_threads are initialized in gen_get_device_info().
>
> Signed-off-by: Anuj Phogat 
> Signed-off-by: Ben Widawsky 
> ---
>  include/pci_ids/i965_pci_ids.h | 12 
>  src/intel/common/gen_device_info.c | 58 ++
> 
>  src/intel/common/gen_device_info.h |  1 +
>  3 files changed, 71 insertions(+)
>
> diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_
> ids.h
> index 17504f5..b296359 100644
> --- a/include/pci_ids/i965_pci_ids.h
> +++ b/include/pci_ids/i965_pci_ids.h
> @@ -165,3 +165,15 @@ CHIPSET(0x5927, kbl_gt3, "Intel(R) Iris Plus Graphics
> 650 (Kaby Lake GT3)")
>  CHIPSET(0x593B, kbl_gt4, "Intel(R) Kabylake GT4")
>  CHIPSET(0x3184, glk, "Intel(R) HD Graphics (Geminilake)")
>  CHIPSET(0x3185, glk_2x6, "Intel(R) HD Graphics (Geminilake 2x6)")
> +CHIPSET(0x5A49, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
> +CHIPSET(0x5A4A, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
> +CHIPSET(0x5A41, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
> +CHIPSET(0x5A42, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
> +CHIPSET(0x5A44, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
> +CHIPSET(0x5A59, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
> +CHIPSET(0x5A5A, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
> +CHIPSET(0x5A5C, cnl_4x8, "Intel(R) HD Graphics (Cannonlake 4x8 GT1.5)")
> +CHIPSET(0x5A50, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
> +CHIPSET(0x5A51, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
> +CHIPSET(0x5A52, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
> +CHIPSET(0x5A54, cnl_5x8, "Intel(R) HD Graphics (Cannonlake 5x8 GT2)")
> diff --git a/src/intel/common/gen_device_info.c
> b/src/intel/common/gen_device_info.c
> index 47aed9d..87edb94 100644
> --- a/src/intel/common/gen_device_info.c
> +++ b/src/intel/common/gen_device_info.c
> @@ -555,6 +555,64 @@ static const struct gen_device_info
> gen_device_info_glk_2x6 = {
> GEN9_LP_FEATURES_2X6
>  };
>
> +#define GEN10_HW_INFO   \
> +   .gen = 10,   \
> +   .max_vs_threads = 728,   \
> +   .max_gs_threads = 432,   \
> +   .max_tcs_threads = 432,  \
> +   .max_tes_threads = 624,  \
> +   .max_cs_threads = 56,\
> +   .urb = { \
> +  .size = 256,  \
> +  .min_entries = {  \
> + [MESA_SHADER_VERTEX]= 64,  \
> + [MESA_SHADER_TESS_EVAL] = 34,  \
> +  },\
> +  .max_entries = {  \
> +  [MESA_SHADER_VERTEX]   = 3936,\
> +  [MESA_SHADER_TESS_CTRL]= 896, \
> +  [MESA_SHADER_TESS_EVAL]= 2064,\
> +  [MESA_SHADER_GEOMETRY] = 832, \
> +  },\
> +   }
> +
> +#define GEN10_FEATURES(_gt, _slices, _l3)   \
> +   GEN8_FEATURES,   \
> +   GEN10_HW_INFO,   \
> +   .gt = _gt, .num_slices = _slices, .l3_banks = _l3
> +
> +static const struct gen_device_info gen_device_info_cnl_2x8 = {
> +   /* GT0.5 */
> +   GEN10_FEATURES(1, 1, 2)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_3x8 = {
> +   /* GT1 */
> +   GEN10_FEATURES(1, 1, 3)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_4x8 = {
> +   /* GT 1.5 */
> +   GEN10_FEATURES(1, 2, 6)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_5x8 = {
> +   /* GT2 */
> +   GEN10_FEATURES(2, 2, 6)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_gt1 = {
> +   GEN10_FEATURES(1, 1, 3)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_gt2 = {
> +   GEN10_FEATURES(2, 2, 6)
> +};
> +
> +static const struct gen_device_info gen_device_info_cnl_gt3 = {
> +   GEN10_FEATURES(3, 4, 12)
> +};
> +
>  bool
>  gen_get_device_info(int devid, struct gen_device_info *devinfo)
>  {
> diff --git a/src/intel/common/gen_device_info.h
> b/src/intel/common/gen_device_info.h
> index 80676d0..6207630 100644
> --- a/src/intel/common/gen_device_info.h
> +++ b/src/intel/common/gen_device_info.h
> @@ -96,6 +96,7 

Re: [Mesa-dev] [PATCH 11/24] i965/cnl: Add pci id for INTEL_DEVID_OVERRIDE

2017-06-02 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> Signed-off-by: Anuj Phogat 
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 3717728..65a0b5c 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1800,6 +1800,7 @@ parse_devid_override(const char *devid_override)
>{ "bdw", 0x162e },
>{ "skl", 0x1912 },
>{ "kbl", 0x5912 },
> +  { "cnl", 0x5a52 },
> };
>
> for (unsigned i = 0; i < ARRAY_SIZE(name_map); i++) {
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH V2 14/24] i965/cnl: Handle gen10 in switch cases across the driver

2017-06-02 Thread Anuj Phogat
On Fri, Jun 2, 2017 at 4:45 PM, Jason Ekstrand  wrote:
> I had to squash 6, 9, 14, and 15 together in order to be able to review
> properly.  It would be much easier if we split things by component rather
> than by what mechanical change is being done.  I've got more to say on the
> cover letter.  In any case, 6, 9, 14, and 15 are
>
I had them component wise initially. Don't remember what made me change it.
I'll keep it in mind for future gens.

> Reviewed-by: Jason Ekstrand 
>
> If you squashed them together, I wouldn't mind at all.
>
> --Jason
>
> On Tue, May 16, 2017 at 10:25 AM, Anuj Phogat  wrote:
>>
>> V2: Start using gen10 functions isl_gen10*(), gen10_blorp_exec()
>> gen10_init_atoms() (Jason)
>> Remove Vulkan changes. Do them later in a separate patch.
>>
>> Cc: Jason Ekstrand 
>> Signed-off-by: Anuj Phogat 
>> ---
>>  src/intel/common/gen_l3_config.c | 1 +
>>  src/intel/compiler/brw_eu.c  | 2 ++
>>  src/intel/compiler/brw_eu_compact.c  | 1 +
>>  src/intel/isl/isl.c  | 9 +
>>  src/mesa/drivers/dri/i965/brw_blorp.c| 6 ++
>>  src/mesa/drivers/dri/i965/brw_formatquery.c  | 1 +
>>  src/mesa/drivers/dri/i965/brw_state_upload.c | 4 +++-
>>  src/mesa/drivers/dri/i965/intel_screen.c | 1 +
>>  8 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/intel/common/gen_l3_config.c
>> b/src/intel/common/gen_l3_config.c
>> index 0783217..4fe3503 100644
>> --- a/src/intel/common/gen_l3_config.c
>> +++ b/src/intel/common/gen_l3_config.c
>> @@ -116,6 +116,7 @@ get_l3_configs(const struct gen_device_info *devinfo)
>>return (devinfo->is_cherryview ? chv_l3_configs : bdw_l3_configs);
>>
>> case 9:
>> +   case 10:
>>return chv_l3_configs;
>>
>> default:
>> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
>> index 77400c1..2c0dc27 100644
>> --- a/src/intel/compiler/brw_eu.c
>> +++ b/src/intel/compiler/brw_eu.c
>> @@ -412,6 +412,7 @@ enum gen {
>> GEN75 = (1 << 5),
>> GEN8  = (1 << 6),
>> GEN9  = (1 << 7),
>> +   GEN10  = (1 << 8),
>> GEN_ALL = ~0
>>  };
>>
>> @@ -688,6 +689,7 @@ gen_from_devinfo(const struct gen_device_info
>> *devinfo)
>> case 7: return devinfo->is_haswell ? GEN75 : GEN7;
>> case 8: return GEN8;
>> case 9: return GEN9;
>> +   case 10: return GEN10;
>> default:
>>unreachable("not reached");
>> }
>> diff --git a/src/intel/compiler/brw_eu_compact.c
>> b/src/intel/compiler/brw_eu_compact.c
>> index b2af76d..740a395 100644
>> --- a/src/intel/compiler/brw_eu_compact.c
>> +++ b/src/intel/compiler/brw_eu_compact.c
>> @@ -1362,6 +1362,7 @@ brw_init_compaction_tables(const struct
>> gen_device_info *devinfo)
>> assert(gen8_src_index_table[ARRAY_SIZE(gen8_src_index_table) - 1] !=
>> 0);
>>
>> switch (devinfo->gen) {
>> +   case 10:
>> case 9:
>> case 8:
>>control_index_table = gen8_control_index_table;
>> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
>> index f89f351..0ae72a4 100644
>> --- a/src/intel/isl/isl.c
>> +++ b/src/intel/isl/isl.c
>> @@ -1674,6 +1674,9 @@ isl_surf_fill_state_s(const struct isl_device *dev,
>> void *state,
>> case 9:
>>isl_gen9_surf_fill_state_s(dev, state, info);
>>break;
>> +   case 10:
>> +  isl_gen10_surf_fill_state_s(dev, state, info);
>> +  break;
>> default:
>>assert(!"Cannot fill surface state for this gen");
>> }
>> @@ -1705,6 +1708,9 @@ isl_buffer_fill_state_s(const struct isl_device
>> *dev, void *state,
>> case 9:
>>isl_gen9_buffer_fill_state_s(state, info);
>>break;
>> +   case 10:
>> +  isl_gen10_buffer_fill_state_s(state, info);
>> +  break;
>> default:
>>assert(!"Cannot fill surface state for this gen");
>> }
>> @@ -1772,6 +1778,9 @@ isl_emit_depth_stencil_hiz_s(const struct isl_device
>> *dev, void *batch,
>> case 9:
>>isl_gen9_emit_depth_stencil_hiz_s(dev, batch, info);
>>break;
>> +   case 10:
>> +  isl_gen10_emit_depth_stencil_hiz_s(dev, batch, info);
>> +  break;
>> default:
>>assert(!"Cannot fill surface state for this gen");
>> }
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>> b/src/mesa/drivers/dri/i965/brw_blorp.c
>> index b69cb4f..8d3bcbb 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> @@ -99,6 +99,12 @@ brw_blorp_init(struct brw_context *brw)
>>brw->blorp.mocs.vb = SKL_MOCS_WB;
>>brw->blorp.exec = gen9_blorp_exec;
>>break;
>> +   case 10:
>> +  brw->blorp.mocs.tex = SKL_MOCS_WB;
>> +  brw->blorp.mocs.rb = SKL_MOCS_PTE;
>> +  brw->blorp.mocs.vb = SKL_MOCS_WB;
>> +  brw->blorp.exec = gen10_blorp_exec;
>> +  break;
>> default:
>>unreachable("Invalid gen");
>> }
>> diff --git 

Re: [Mesa-dev] [PATCH 10/24] i965/cnl: Wire up android Mesa build files for gen10

2017-06-02 Thread Jason Ekstrand
Acked-by: Jason Ekstrand 

Maybe one of the android people would like to test it?

On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> Note: This patch is untested.
>
> Signed-off-by: Anuj Phogat 
> ---
>  src/intel/Android.genxml.mk  |  5 +
>  src/intel/Android.isl.mk | 20 
>  src/intel/Android.vulkan.mk  | 21 +
>  src/mesa/drivers/dri/i965/Android.mk | 24 +++-
>  4 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/Android.genxml.mk b/src/intel/Android.genxml.mk
> index 4b0746c..e4d8dd8 100644
> --- a/src/intel/Android.genxml.mk
> +++ b/src/intel/Android.genxml.mk
> @@ -96,6 +96,11 @@ $(intermediates)/genxml/gen9_pack.h: PRIVATE_XML :=
> $(LOCAL_PATH)/genxml/gen9.xm
>  $(intermediates)/genxml/gen9_pack.h: $(LOCAL_PATH)/genxml/gen9.xml
> $(LOCAL_PATH)/genxml/gen_pack_header.py
> $(call header-gen)
>
> +$(intermediates)/genxml/gen10_pack.h: PRIVATE_SCRIPT := $(MESA_PYTHON2)
> $(LOCAL_PATH)/genxml/gen_pack_header.py
> +$(intermediates)/genxml/gen10_pack.h: PRIVATE_XML :=
> $(LOCAL_PATH)/genxml/gen10.xml
> +$(intermediates)/genxml/gen10_pack.h: $(LOCAL_PATH)/genxml/gen10.xml
> $(LOCAL_PATH)/genxml/gen_pack_header.py
> +   $(call header-gen)
> +
>  $(intermediates)/genxml/genX_xml.h: $(addprefix 
> $(MESA_TOP)/src/intel/,$(GENXML_XML_FILES))
> $(MESA_TOP)/src/intel/genxml/gen_zipped_file.py
> @mkdir -p $(dir $@)
> @echo "Gen Header: $(PRIVATE_MODULE) <= $(notdir $(@))"
> diff --git a/src/intel/Android.isl.mk b/src/intel/Android.isl.mk
> index 67e6d2d..516ac3a 100644
> --- a/src/intel/Android.isl.mk
> +++ b/src/intel/Android.isl.mk
> @@ -161,6 +161,25 @@ include $(MESA_COMMON_MK)
>  include $(BUILD_STATIC_LIBRARY)
>
>  # ---
> +# Build libmesa_isl_gen10
> +# ---
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_MODULE := libmesa_isl_gen10
> +
> +LOCAL_SRC_FILES := $(ISL_GEN10_FILES)
> +
> +LOCAL_CFLAGS := -DGEN_VERSIONx10=100
> +
> +LOCAL_C_INCLUDES := $(LIBISL_GENX_COMMON_INCLUDES)
> +
> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_genxml
> +
> +include $(MESA_COMMON_MK)
> +include $(BUILD_STATIC_LIBRARY)
> +
> +# ---
>  # Build libmesa_isl
>  # ---
>
> @@ -187,6 +206,7 @@ LOCAL_WHOLE_STATIC_LIBRARIES := \
> libmesa_isl_gen75 \
> libmesa_isl_gen8 \
> libmesa_isl_gen9 \
> +   libmesa_isl_gen10 \
> libmesa_genxml
>
>  # Autogenerated sources
> diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk
> index 831b658..566db29 100644
> --- a/src/intel/Android.vulkan.mk
> +++ b/src/intel/Android.vulkan.mk
> @@ -158,6 +158,26 @@ include $(MESA_COMMON_MK)
>  include $(BUILD_STATIC_LIBRARY)
>
>  #
> +# libanv for gen10
> +#
> +
> +include $(CLEAR_VARS)
> +LOCAL_MODULE := libmesa_anv_gen10
> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
> +
> +LOCAL_SRC_FILES := $(VULKAN_GEN10_FILES)
> +LOCAL_CFLAGS := -DGEN_VERSIONx10=100
> +
> +LOCAL_C_INCLUDES := $(ANV_INCLUDES)
> +
> +LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
> +
> +LOCAL_SHARED_LIBRARIES := libdrm
> +
> +include $(MESA_COMMON_MK)
> +include $(BUILD_STATIC_LIBRARY)
> +
> +#
>  # libmesa_vulkan_common
>  #
>
> @@ -228,6 +248,7 @@ LOCAL_WHOLE_STATIC_LIBRARIES := \
> libmesa_anv_gen75 \
> libmesa_anv_gen8 \
> libmesa_anv_gen9 \
> +   libmesa_anv_gen10 \
> libmesa_intel_compiler \
> libmesa_anv_entrypoints
>
> diff --git a/src/mesa/drivers/dri/i965/Android.mk
> b/src/mesa/drivers/dri/i965/Android.mk
> index 7c4fada..7ee9ab7 100644
> --- a/src/mesa/drivers/dri/i965/Android.mk
> +++ b/src/mesa/drivers/dri/i965/Android.mk
> @@ -47,7 +47,8 @@ I965_PERGEN_LIBS := \
> libmesa_i965_gen7 \
> libmesa_i965_gen75 \
> libmesa_i965_gen8 \
> -   libmesa_i965_gen9
> +   libmesa_i965_gen9 \
> +   libmesa_i965_gen10
>
>  # ---
>  # Build libmesa_i965_gen4
> @@ -218,6 +219,27 @@ include $(MESA_COMMON_MK)
>  include $(BUILD_STATIC_LIBRARY)
>
>  # ---
> +# Build libmesa_i965_gen10
> +# ---
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_MODULE := libmesa_i965_gen10
> +
> +LOCAL_C_INCLUDES := $(I965_PERGEN_COMMON_INCLUDES)
> +
> +LOCAL_SRC_FILES := $(i965_gen10_FILES)
> +
> +LOCAL_SHARED_LIBRARIES := $(I965_PERGEN_SHARED_LIBRARIES)
> +
> +LOCAL_STATIC_LIBRARIES := $(I965_PERGEN_STATIC_LIBRARIES)
> +
> +LOCAL_CFLAGS := -DGEN_VERSIONx10=100
> +
> +include $(MESA_COMMON_MK)
> +include $(BUILD_STATIC_LIBRARY)
> +
> +# ---
>  # Build i965_dri
>  # ---
>
> --
> 2.9.3
>
> ___
> mesa-dev 

Re: [Mesa-dev] [PATCH 01/24] i965: Make feature macros gen8 based

2017-06-02 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> From: Ben Widawsky 
>
> All the "features" of the hardware are similar starting with GEN8, so
> remove as
> much of the GEN9 uniqueness as possible. This makes implementing future gen
> platforms a bit easier.
>
> Signed-off-by: Ben Widawsky 
> Reviewed-by: Anuj Phogat 
> ---
>  src/intel/common/gen_device_info.c | 13 +
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/src/intel/common/gen_device_info.c
> b/src/intel/common/gen_device_info.c
> index 209b293..47aed9d 100644
> --- a/src/intel/common/gen_device_info.c
> +++ b/src/intel/common/gen_device_info.c
> @@ -378,15 +378,8 @@ static const struct gen_device_info
> gen_device_info_chv = {
> }
>  };
>
> -#define GEN9_FEATURES   \
> +#define GEN9_HW_INFO\
> .gen = 9,\
> -   .has_hiz_and_separate_stencil = true,\
> -   .has_resource_streamer = true,   \
> -   .must_use_separate_stencil = true,   \
> -   .has_llc = true, \
> -   .has_pln = true, \
> -   .supports_simd16_3src = true,\
> -   .has_surface_tile_offset = true, \
> .max_vs_threads = 336,   \
> .max_gs_threads = 336,   \
> .max_tcs_threads = 336,  \
> @@ -454,6 +447,10 @@ static const struct gen_device_info
> gen_device_info_chv = {
>},   \
> }
>
> +#define GEN9_FEATURES   \
> +   GEN8_FEATURES,   \
> +   GEN9_HW_INFO
> +
>  static const struct gen_device_info gen_device_info_skl_gt1 = {
> GEN9_FEATURES, .gt = 1,
> .num_slices = 1,
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH V2 00/24] Add Cannonlake support

2017-06-02 Thread Jason Ekstrand
On Mon, May 22, 2017 at 9:32 AM, Anuj Phogat  wrote:

> On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat 
> wrote:
> > This series adds support for Cannonlake.
> >
> > Changes from V1 to V2:
> > - Incorporated the review comments from V1.
> > - Rebased 8 months old CNL branch on top of master
> > - Wired up Linux and Android build files for gen10
> > - Replaced the use of few gen9 functions with gen10 specific functions.
> > - Squashed few patches, dropped few and created new patches.
> >
> Thanks to Jason and Ken who have reviewed few patches in this series.
> Rest of them are still waiting for the review. I really want to land this
> series
> (at least first 15-16 patches) soon. There are some very easy patches any
> one can review like enabling Mesa to build for gen10 etc. Please take a
> look at them. Thanks :).
>

Finally got around to looking at these again...

Now that we're switching everything over to genxml, I think it's a good
idea to be a bit more intentional in the way we write new platform
patches.  There are a number of patches in this series that are much harder
to review than they need to be because they're written more-or-less in
order of code development and not in a logical reviewable order.  In
particular, there's a patch which updates a pile of switch statements to
get rid of asserts but it just moves them all over to gen9.  Moving stuff
to gen10 is in a different patch.  The result is that it's very hard,
without squashing things together, to tell whether or not we missed
anything when we switched them over to actual gen10 functions.  This isn't
really a criticism of Anuj and Ben.  They've done a lot of rebasaing on top
of a lot of driver architecture changes.  I think this will be much easier
to do better in the future.

In my view, the ideal platform enabling patch series would look something
like this:

 1) Add genN.xml
 2) Add the #defines and #includes to genxml and the build system stuff to
generate the packing headers
 3) Update stuff in src/intel/common such as URB configuration changes
 4) Update ISL:
a) Any needed generic ISL changes such as adding new layaouts.  (Cannon
lake doesn't add anything, so nothing to do here).  This may be multiple
patches.
b) Get ISL surface state emit code building for the new hardware.  This
includes updating the autotools and Android makefiles, adding function
prototypes, updating switch statements, etc.  If changes are needed in
isl_surface_state.c or isl_depth_stencil.c, they should be minimal bug
still enough that the end result is correct.
5) Update BLORP as needed for the new platform.  Sadly, there's no way to
build-test this without the next step since BLORP doesn't build its own
genX files.
6) Get GL driver genxml state-upload and blorp code building and hooked
in.  Core blorp changes should go in their own patch (above) but this will
include the build system changes for blorp as well.  This also includes
updating switch statements.
8) Implement workarounds, features, etc.

The important part is that we keep related changes together.  In order to
review this series, I had to squash patches 6, 9, 14, and 15 together.
Again, I'm not casting any blame here.  The series makes a lot of
historical sense.  It's just hard to review as-is.

--Jason


> > What's remaining:
> > - Add missing gen10 bits in Vulkan driver.
> > - Fix failing piglit, cts tests for GL and Vulkan.
> >
> > You can also find this series at:
> > https://github.com/aphogat/mesa.git
> > branch: reviews
> >
> > Anuj Phogat (18):
> >   i965/cnl: Define genX(x) and GENX(x) for gen10
> >   i965/cnl: Include gen10_pack.h
>
>   i965/cnl: Add gen10 specific function declarations
> >   i965/cnl: Update the script generating genX_bits.h
> >   i965/cnl: Add isl_gen10 header and source files
> >   i965/cnl: Wire up Mesa build files for gen10
> >   i965/cnl: Wire up android Mesa build files for gen10
> >   i965/cnl: Add pci id for INTEL_DEVID_OVERRIDE
> >   i965/cnl: Add cnl bits in aubinator
> >   i965/cnl: Update few assertions
> >   i965/cnl: Handle gen10 in switch cases across the driver
> >   i965/cnl: Start using CNL MOCS defines
> >   i965/cnl: Start using gen10 specific functions
> >   i965/cnl: Don't resolve single sampled color rb in case of sRGB formats
> >   i965/cnl: Make URB {VS, GS, HS, DS} sizes non multiple of 3
> >   i965/cnl: Reformat surface_format_info table to accomodate gen10+
> >   i965/cnl: Enable CCS_E and RT support for few formats
> >   i965: Simplify get_l3_way_size() function
> >
> > Ben Widawsky (5):
> >   i965: Make feature macros gen8 based
> >   i965/cnl: Add a preliminary device for Cannonlake
> >   i965/cnl: Implement new pipe control workaround
> >   i965/cnl: Implement depth count workaround
> >   i965/cnl: Restore lossless compression for sRGB formats
> >
> > Jason Ekstrand (1):
> >   i965/cnl: Add gen10.xml
> >
> >  include/pci_ids/i965_pci_ids.h   |   12 +
> >  

Re: [Mesa-dev] [PATCH V2 14/24] i965/cnl: Handle gen10 in switch cases across the driver

2017-06-02 Thread Jason Ekstrand
I had to squash 6, 9, 14, and 15 together in order to be able to review
properly.  It would be much easier if we split things by component rather
than by what mechanical change is being done.  I've got more to say on the
cover letter.  In any case, 6, 9, 14, and 15 are

Reviewed-by: Jason Ekstrand 

If you squashed them together, I wouldn't mind at all.

--Jason

On Tue, May 16, 2017 at 10:25 AM, Anuj Phogat  wrote:

> V2: Start using gen10 functions isl_gen10*(), gen10_blorp_exec()
> gen10_init_atoms() (Jason)
> Remove Vulkan changes. Do them later in a separate patch.
>
> Cc: Jason Ekstrand 
> Signed-off-by: Anuj Phogat 
> ---
>  src/intel/common/gen_l3_config.c | 1 +
>  src/intel/compiler/brw_eu.c  | 2 ++
>  src/intel/compiler/brw_eu_compact.c  | 1 +
>  src/intel/isl/isl.c  | 9 +
>  src/mesa/drivers/dri/i965/brw_blorp.c| 6 ++
>  src/mesa/drivers/dri/i965/brw_formatquery.c  | 1 +
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 4 +++-
>  src/mesa/drivers/dri/i965/intel_screen.c | 1 +
>  8 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/common/gen_l3_config.c b/src/intel/common/gen_l3_
> config.c
> index 0783217..4fe3503 100644
> --- a/src/intel/common/gen_l3_config.c
> +++ b/src/intel/common/gen_l3_config.c
> @@ -116,6 +116,7 @@ get_l3_configs(const struct gen_device_info *devinfo)
>return (devinfo->is_cherryview ? chv_l3_configs : bdw_l3_configs);
>
> case 9:
> +   case 10:
>return chv_l3_configs;
>
> default:
> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
> index 77400c1..2c0dc27 100644
> --- a/src/intel/compiler/brw_eu.c
> +++ b/src/intel/compiler/brw_eu.c
> @@ -412,6 +412,7 @@ enum gen {
> GEN75 = (1 << 5),
> GEN8  = (1 << 6),
> GEN9  = (1 << 7),
> +   GEN10  = (1 << 8),
> GEN_ALL = ~0
>  };
>
> @@ -688,6 +689,7 @@ gen_from_devinfo(const struct gen_device_info *devinfo)
> case 7: return devinfo->is_haswell ? GEN75 : GEN7;
> case 8: return GEN8;
> case 9: return GEN9;
> +   case 10: return GEN10;
> default:
>unreachable("not reached");
> }
> diff --git a/src/intel/compiler/brw_eu_compact.c
> b/src/intel/compiler/brw_eu_compact.c
> index b2af76d..740a395 100644
> --- a/src/intel/compiler/brw_eu_compact.c
> +++ b/src/intel/compiler/brw_eu_compact.c
> @@ -1362,6 +1362,7 @@ brw_init_compaction_tables(const struct
> gen_device_info *devinfo)
> assert(gen8_src_index_table[ARRAY_SIZE(gen8_src_index_table) - 1] !=
> 0);
>
> switch (devinfo->gen) {
> +   case 10:
> case 9:
> case 8:
>control_index_table = gen8_control_index_table;
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index f89f351..0ae72a4 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -1674,6 +1674,9 @@ isl_surf_fill_state_s(const struct isl_device *dev,
> void *state,
> case 9:
>isl_gen9_surf_fill_state_s(dev, state, info);
>break;
> +   case 10:
> +  isl_gen10_surf_fill_state_s(dev, state, info);
> +  break;
> default:
>assert(!"Cannot fill surface state for this gen");
> }
> @@ -1705,6 +1708,9 @@ isl_buffer_fill_state_s(const struct isl_device
> *dev, void *state,
> case 9:
>isl_gen9_buffer_fill_state_s(state, info);
>break;
> +   case 10:
> +  isl_gen10_buffer_fill_state_s(state, info);
> +  break;
> default:
>assert(!"Cannot fill surface state for this gen");
> }
> @@ -1772,6 +1778,9 @@ isl_emit_depth_stencil_hiz_s(const struct
> isl_device *dev, void *batch,
> case 9:
>isl_gen9_emit_depth_stencil_hiz_s(dev, batch, info);
>break;
> +   case 10:
> +  isl_gen10_emit_depth_stencil_hiz_s(dev, batch, info);
> +  break;
> default:
>assert(!"Cannot fill surface state for this gen");
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index b69cb4f..8d3bcbb 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -99,6 +99,12 @@ brw_blorp_init(struct brw_context *brw)
>brw->blorp.mocs.vb = SKL_MOCS_WB;
>brw->blorp.exec = gen9_blorp_exec;
>break;
> +   case 10:
> +  brw->blorp.mocs.tex = SKL_MOCS_WB;
> +  brw->blorp.mocs.rb = SKL_MOCS_PTE;
> +  brw->blorp.mocs.vb = SKL_MOCS_WB;
> +  brw->blorp.exec = gen10_blorp_exec;
> +  break;
> default:
>unreachable("Invalid gen");
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_formatquery.c
> b/src/mesa/drivers/dri/i965/brw_formatquery.c
> index 96cc6e0..5faf91f 100644
> --- a/src/mesa/drivers/dri/i965/brw_formatquery.c
> +++ b/src/mesa/drivers/dri/i965/brw_formatquery.c
> @@ -37,6 +37,7 @@ brw_query_samples_for_format(struct gl_context *ctx,
> GLenum target,
> (void) 

Re: [Mesa-dev] [PATCH 05/24] i965/cnl: Include gen10_pack.h

2017-06-02 Thread Jason Ekstrand
4-5 are

Reviewed-by: Jason Ekstrand 

On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat  wrote:

> Signed-off-by: Anuj Phogat 
> ---
>  src/intel/genxml/genX_pack.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/intel/genxml/genX_pack.h b/src/intel/genxml/genX_pack.h
> index 2ec2226..187e75c 100644
> --- a/src/intel/genxml/genX_pack.h
> +++ b/src/intel/genxml/genX_pack.h
> @@ -44,6 +44,8 @@
>  #  include "genxml/gen8_pack.h"
>  #elif (GEN_VERSIONx10 == 90)
>  #  include "genxml/gen9_pack.h"
> +#elif (GEN_VERSIONx10 == 100)
> +#  include "genxml/gen10_pack.h"
>  #else
>  #  error "Need to add a pack header include for this gen"
>  #endif
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] mapi: Enable assembly language API acceleration for PPC64LE (V2)

2017-06-02 Thread Ben Crocker
Implement assembly language API acceleration for PPC64LE,
analogous to long-standing implementations for X86 and X86-64.

See also similar implementation in libglvnd.

Tested with Piglit.

Signed-off-by: Ben Crocker 
---
 configure.ac |  12 +++
 src/mapi/Makefile.sources|   2 +
 src/mapi/entry.c |  12 ++-
 src/mapi/entry_ppc64le_tls.h | 152 +++
 src/mapi/entry_ppc64le_tsd.h | 210 +++
 5 files changed, 386 insertions(+), 2 deletions(-)
 create mode 100644 src/mapi/entry_ppc64le_tls.h
 create mode 100644 src/mapi/entry_ppc64le_tsd.h

diff --git a/configure.ac b/configure.ac
index 70885fb..2dd24eb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -728,6 +728,13 @@ if test "x$enable_asm" = xyes; then
 ;;
 esac
 ;;
+powerpc64le)
+case "$host_os" in
+linux*)
+asm_arch=ppc64le
+;;
+esac
+;;
 esac
 
 case "$asm_arch" in
@@ -743,6 +750,10 @@ if test "x$enable_asm" = xyes; then
 DEFINES="$DEFINES -DUSE_SPARC_ASM"
 AC_MSG_RESULT([yes, sparc])
 ;;
+ppc64le)
+DEFINES="$DEFINES -DUSE_PPC64LE_ASM"
+AC_MSG_RESULT([yes, ppc64le])
+;;
 *)
 AC_MSG_RESULT([no, platform not supported])
 ;;
@@ -2535,6 +2546,7 @@ AM_CONDITIONAL(HAVE_COMMON_OSMESA, test "x$enable_osmesa" 
= xyes -o \
 AM_CONDITIONAL(HAVE_X86_ASM, test "x$asm_arch" = xx86 -o "x$asm_arch" = 
xx86_64)
 AM_CONDITIONAL(HAVE_X86_64_ASM, test "x$asm_arch" = xx86_64)
 AM_CONDITIONAL(HAVE_SPARC_ASM, test "x$asm_arch" = xsparc)
+AM_CONDITIONAL(HAVE_PPC64LE_ASM, test "x$asm_arch" = xppc64le)
 
 AC_SUBST([NINE_MAJOR], 1)
 AC_SUBST([NINE_MINOR], 0)
diff --git a/src/mapi/Makefile.sources b/src/mapi/Makefile.sources
index 37d6ef3..5647158 100644
--- a/src/mapi/Makefile.sources
+++ b/src/mapi/Makefile.sources
@@ -26,6 +26,8 @@ MAPI_BRIDGE_FILES = \
entry_x86-64_tls.h \
entry_x86_tls.h \
entry_x86_tsd.h \
+   entry_ppc64le_tls.h \
+   entry_ppc64le_tsd.h \
mapi_tmp.h
 
 MAPI_FILES = \
diff --git a/src/mapi/entry.c b/src/mapi/entry.c
index 27d0db4..35f0948 100644
--- a/src/mapi/entry.c
+++ b/src/mapi/entry.c
@@ -25,8 +25,12 @@
  *Chia-I Wu 
  */
 
+#include 
+#include 
+
 #include "entry.h"
 #include "u_current.h"
+#include "util/u_endian.h"
 
 #define _U_STRINGIFY(x) #x
 #define U_STRINGIFY(x) _U_STRINGIFY(x)
@@ -49,10 +53,14 @@
 #   endif
 #elif defined(USE_X86_64_ASM) && defined(__GNUC__) && defined(GLX_USE_TLS)
 #   include "entry_x86-64_tls.h"
+#elif defined(USE_PPC64LE_ASM) && defined(__GNUC__) && 
defined(PIPE_ARCH_LITTLE_ENDIAN)
+#   ifdef GLX_USE_TLS
+#  include "entry_ppc64le_tls.h"
+#   else
+#  include "entry_ppc64le_tsd.h"
+#   endif
 #else
 
-#include 
-
 static inline const struct mapi_table *
 entry_current_get(void)
 {
diff --git a/src/mapi/entry_ppc64le_tls.h b/src/mapi/entry_ppc64le_tls.h
new file mode 100644
index 000..e09a117
--- /dev/null
+++ b/src/mapi/entry_ppc64le_tls.h
@@ -0,0 +1,152 @@
+/*
+ * Mesa 3-D graphics library
+ *
+ * Copyright (C) 2017 Red Hat
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ *Ben Crocker 
+ */
+
+#ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
+#define HIDDEN __attribute__((visibility("hidden")))
+#else
+#define HIDDEN
+#endif
+
+// NOTE: These must be powers of two:
+#define PPC64LE_ENTRY_SIZE 64
+#define PPC64LE_PAGE_ALIGN 65536
+#if ((PPC64LE_ENTRY_SIZE & (PPC64LE_ENTRY_SIZE - 1)) != 0)
+#error PPC64LE_ENTRY_SIZE must be a power of two!
+#endif
+#if ((PPC64LE_PAGE_ALIGN & (PPC64LE_PAGE_ALIGN - 1)) != 0)
+#error PPC64LE_PAGE_ALIGN must be a power of two!
+#endif
+
+__asm__(".text\n"
+".balign " U_STRINGIFY(PPC64LE_ENTRY_SIZE) "\n"
+"ppc64le_entry_start:");
+
+#define STUB_ASM_ENTRY(func)

Re: [Mesa-dev] [PATCH 00/12] Add Cannonlake support

2017-06-02 Thread Anuj Phogat
I now have a v2 of this series on the list with 24 patches. So, please
don't spend any time reviewing this v1 series. Thanks.

On Fri, Apr 14, 2017 at 5:35 PM, Anuj Phogat  wrote:
> This series adds a preliminary support for Cannonlake. We
> still end up using gen9 paths in many cases. My upcoming
> patches will change it by creating new functions, headers
> for gen10. You can also find this series at:
> https://github.com/aphogat/mesa.git
> branch: reviews
>
> Anuj Phogat (4):
>   i965/cnl: Update the script generating genX_bits.h
>   i965/cnl: URB {VS, GS, HS, DS} sizes cannot be a multiple of 3
>   i965/cnl: Update memory barrier assert
>   i965/cnl: Add CNL MOCS defines
>
> Ben Widawsky (7):
>   i965: Make feature macros gen8 based
>   i965/cnl: Implement new pipe control workaround
>   i965/cnl: Implement depth count workaround
>   i965/cnl: Modify thread count shift for VS
>   i965/cnl: Restore lossless compression for sRGB formats
>   i965/cnl: Add a preliminary device for CNL
>   i965/cnl: Properly handle l3 configuration
>
> Jason Ekstrand (1):
>   i965/cnl: Add gen10.xml
>
>  include/pci_ids/i965_pci_ids.h   |   12 +
>  src/intel/Makefile.sources   |3 +-
>  src/intel/common/gen_device_info.c   |   72 +-
>  src/intel/common/gen_device_info.h   |1 +
>  src/intel/common/gen_l3_config.c |   42 +-
>  src/intel/compiler/brw_compiler.h|2 +-
>  src/intel/compiler/brw_eu.c  |2 +
>  src/intel/compiler/brw_eu_compact.c  |1 +
>  src/intel/genxml/gen10.xml   | 3557 
> ++
>  src/intel/genxml/gen_bits_header.py  |4 +-
>  src/intel/isl/isl.c  |2 +
>  src/intel/vulkan/anv_cmd_buffer.c|1 +
>  src/intel/vulkan/anv_device.c|1 +
>  src/intel/vulkan/anv_entrypoints_gen.py  |1 +
>  src/mesa/drivers/dri/i965/brw_blorp.c|6 +
>  src/mesa/drivers/dri/i965/brw_defines.h  |9 +
>  src/mesa/drivers/dri/i965/brw_draw_upload.c  |1 +
>  src/mesa/drivers/dri/i965/brw_formatquery.c  |1 +
>  src/mesa/drivers/dri/i965/brw_pipe_control.c |   18 +
>  src/mesa/drivers/dri/i965/brw_program.c  |2 +-
>  src/mesa/drivers/dri/i965/brw_queryobj.c |8 +
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |2 +
>  src/mesa/drivers/dri/i965/gen7_urb.c |   12 +
>  src/mesa/drivers/dri/i965/gen8_vs_state.c|6 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c|2 +-
>  src/mesa/drivers/dri/i965/intel_screen.c |1 +
>  26 files changed, 3749 insertions(+), 20 deletions(-)
>  create mode 100644 src/intel/genxml/gen10.xml
>
> --
> 2.9.3
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 15/30] i965/miptree: Add new entrypoints for resolve management

2017-06-02 Thread Nanley Chery
On Fri, May 26, 2017 at 04:30:19PM -0700, Jason Ekstrand wrote:
> This commit adds a new unified interface for doing resolves.  The basic
> format is that, prior to any surface access such as texturing or
> rendering, you call intel_miptree_prepare_access.  If the surface was
> written, you call intel_miptree_finish_write.  These two functions take
> parameters which tell them whether or not auxiliary compression and fast
> clears are supported on the surface.  Later commits will add wrappers
> around these two functions for texturing, rendering, etc.
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 156 
> +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  80 +
>  2 files changed, 232 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 6cd32ce..0659e75 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2028,8 +2028,7 @@ bool
>  intel_miptree_all_slices_resolve_hiz(struct brw_context *brw,
>struct intel_mipmap_tree *mt)
>  {
> -   return intel_miptree_depth_hiz_resolve(brw, mt,
> -  0, UINT32_MAX, 0, UINT32_MAX,
> +   return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0, 
> UINT32_MAX,
>BLORP_HIZ_OP_HIZ_RESOLVE);
>  }
>  
> @@ -2037,8 +2036,7 @@ bool
>  intel_miptree_all_slices_resolve_depth(struct brw_context *brw,
>  struct intel_mipmap_tree *mt)
>  {
> -   return intel_miptree_depth_hiz_resolve(brw, mt,
> -  0, UINT32_MAX, 0, UINT32_MAX,
> +   return intel_miptree_depth_hiz_resolve(brw, mt, 0, UINT32_MAX, 0, 
> UINT32_MAX,
>BLORP_HIZ_OP_DEPTH_RESOLVE);
>  }
>  
> @@ -2221,6 +2219,156 @@ intel_miptree_all_slices_resolve_color(struct 
> brw_context *brw,
> intel_miptree_resolve_color(brw, mt, 0, UINT32_MAX, 0, UINT32_MAX, flags);
>  }
>  
> +void
> +intel_miptree_prepare_access(struct brw_context *brw,
> + struct intel_mipmap_tree *mt,
> + uint32_t start_level, uint32_t num_levels,
> + uint32_t start_layer, uint32_t num_layers,
> + bool aux_supported, bool fast_clear_supported)
> +{
> +   if (_mesa_is_format_color_format(mt->format)) {
> +  if (!mt->mcs_buf)
> + return;
> +
> +  if (mt->num_samples > 1) {
> + /* Nothing to do for MSAA */
> +  } else {
> + /* TODO: This is fairly terrible.  We can do better. */
> + if (!aux_supported || !fast_clear_supported) {
> +intel_miptree_resolve_color(brw, mt, start_level, num_levels,
> +start_layer, num_layers, 0);
> + }
> +  }
> +   } else if (mt->format == MESA_FORMAT_S_UINT8) {
> +  /* Nothing to do for stencil */
> +   } else {
> +  if (!mt->hiz_buf)
> + return;
> +
> +  if (aux_supported) {
> + assert(fast_clear_supported);
> + intel_miptree_depth_hiz_resolve(brw, mt, start_level, num_levels,
> + start_layer, num_layers,
> + BLORP_HIZ_OP_HIZ_RESOLVE);
> +  } else {
> + assert(!fast_clear_supported);
> + intel_miptree_depth_hiz_resolve(brw, mt, start_level, num_levels,
> + start_layer, num_layers,
> + BLORP_HIZ_OP_DEPTH_RESOLVE);
> +  }
> +   }
> +}
> +
> +void
> +intel_miptree_finish_write(struct brw_context *brw,
> +   struct intel_mipmap_tree *mt, uint32_t level,
> +   uint32_t start_layer, uint32_t num_layers,
> +   bool written_with_aux)
> +{
> +   assert(start_layer < mt->level[level].depth);
> +   if (num_layers == INTEL_REMAINING_LAYERS)
> +  num_layers = mt->level[level].depth - start_layer;
> +   assert(num_layers <= mt->level[level].depth - start_layer);
> +
> +   if (_mesa_is_format_color_format(mt->format)) {
> +  if (mt->num_samples > 1) {
> + /* Nothing to do for MSAA */
> +  } else {
> + if (written_with_aux) {
> +intel_miptree_used_for_rendering(brw, mt, level,
> + start_layer, num_layers);
> + }
> +  }
> +   } else if (mt->format == MESA_FORMAT_S_UINT8) {
> +  /* Nothing to do for stencil */
> +   } else {
> +  if (written_with_aux) {
> + for (unsigned a = 0; a < num_layers; a++) {
> +intel_miptree_check_level_layer(mt, level, start_layer);
> +intel_miptree_slice_set_needs_depth_resolve(mt, level,
> +   

Re: [Mesa-dev] [PATCH 06/12] i965/cnl: Modify thread count shift for VS

2017-06-02 Thread Anuj Phogat
I just realized that I already dropped this patch in v2 of the series.
Please review the latest series with 24 patches. Thanks.

On Fri, Jun 2, 2017 at 4:23 PM, Anuj Phogat  wrote:
> Yes, I'll drop the patch.
>
> On Fri, Jun 2, 2017 at 11:38 AM, Jason Ekstrand  wrote:
>> I think this patch can be dropped with Rafiel's genxml work.
>>
>> On Fri, Apr 14, 2017 at 5:35 PM, Anuj Phogat  wrote:
>>>
>>> From: Ben Widawsky 
>>>
>>> Signed-off-by: Ben Widawsky 
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_defines.h   | 1 +
>>>  src/mesa/drivers/dri/i965/gen8_vs_state.c | 6 +-
>>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>>> b/src/mesa/drivers/dri/i965/brw_defines.h
>>> index 08106c0..688ff61 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>>> @@ -607,6 +607,7 @@ enum brw_wrap_mode {
>>>  /* DW5 */
>>>  # define GEN6_VS_MAX_THREADS_SHIFT 25
>>>  # define HSW_VS_MAX_THREADS_SHIFT  23
>>> +# define GEN10_VS_MAX_THREADS_SHIFT 22
>>>  # define GEN6_VS_STATISTICS_ENABLE (1 << 10)
>>>  # define GEN6_VS_CACHE_DISABLE (1 << 1)
>>>  # define GEN6_VS_ENABLE(1 << 0)
>>> diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c
>>> b/src/mesa/drivers/dri/i965/gen8_vs_state.c
>>> index 7b66da4..c4ad9cd 100644
>>> --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c
>>> +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c
>>> @@ -75,7 +75,11 @@ upload_vs_state(struct brw_context *brw)
>>> uint32_t simd8_enable =
>>>vue_prog_data->dispatch_mode == DISPATCH_MODE_SIMD8 ?
>>>GEN8_VS_SIMD8_ENABLE : 0;
>>> -   OUT_BATCH(((devinfo->max_vs_threads - 1) << HSW_VS_MAX_THREADS_SHIFT)
>>> |
>>> +
>>> +   uint32_t threads = (devinfo->max_vs_threads - 1);
>>> +   threads <<= brw->gen >= 10 ? GEN10_VS_MAX_THREADS_SHIFT :
>>> +HSW_VS_MAX_THREADS_SHIFT;
>>> +   OUT_BATCH(threads |
>>>   GEN6_VS_STATISTICS_ENABLE |
>>>   simd8_enable |
>>>   GEN6_VS_ENABLE);
>>> --
>>> 2.9.3
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 06/12] i965/cnl: Modify thread count shift for VS

2017-06-02 Thread Anuj Phogat
Yes, I'll drop the patch.

On Fri, Jun 2, 2017 at 11:38 AM, Jason Ekstrand  wrote:
> I think this patch can be dropped with Rafiel's genxml work.
>
> On Fri, Apr 14, 2017 at 5:35 PM, Anuj Phogat  wrote:
>>
>> From: Ben Widawsky 
>>
>> Signed-off-by: Ben Widawsky 
>> ---
>>  src/mesa/drivers/dri/i965/brw_defines.h   | 1 +
>>  src/mesa/drivers/dri/i965/gen8_vs_state.c | 6 +-
>>  2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>> b/src/mesa/drivers/dri/i965/brw_defines.h
>> index 08106c0..688ff61 100644
>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> @@ -607,6 +607,7 @@ enum brw_wrap_mode {
>>  /* DW5 */
>>  # define GEN6_VS_MAX_THREADS_SHIFT 25
>>  # define HSW_VS_MAX_THREADS_SHIFT  23
>> +# define GEN10_VS_MAX_THREADS_SHIFT 22
>>  # define GEN6_VS_STATISTICS_ENABLE (1 << 10)
>>  # define GEN6_VS_CACHE_DISABLE (1 << 1)
>>  # define GEN6_VS_ENABLE(1 << 0)
>> diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c
>> b/src/mesa/drivers/dri/i965/gen8_vs_state.c
>> index 7b66da4..c4ad9cd 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c
>> @@ -75,7 +75,11 @@ upload_vs_state(struct brw_context *brw)
>> uint32_t simd8_enable =
>>vue_prog_data->dispatch_mode == DISPATCH_MODE_SIMD8 ?
>>GEN8_VS_SIMD8_ENABLE : 0;
>> -   OUT_BATCH(((devinfo->max_vs_threads - 1) << HSW_VS_MAX_THREADS_SHIFT)
>> |
>> +
>> +   uint32_t threads = (devinfo->max_vs_threads - 1);
>> +   threads <<= brw->gen >= 10 ? GEN10_VS_MAX_THREADS_SHIFT :
>> +HSW_VS_MAX_THREADS_SHIFT;
>> +   OUT_BATCH(threads |
>>   GEN6_VS_STATISTICS_ENABLE |
>>   simd8_enable |
>>   GEN6_VS_ENABLE);
>> --
>> 2.9.3
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH V2 15/24] i965/cnl: Start using CNL MOCS defines

2017-06-02 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Fri, Jun 2, 2017 at 4:06 PM, Jason Ekstrand  wrote:

> On Mon, May 22, 2017 at 9:54 AM, Rafael Antognolli <
> rafael.antogno...@intel.com> wrote:
>
>> On Tue, May 16, 2017 at 04:01:30PM -0700, Anuj Phogat wrote:
>> > On Tue, May 16, 2017 at 10:34 AM, Anuj Phogat 
>> wrote:
>> > > CNL MOCS defines are duplicates of SKL MOCS defines.
>> > >
>> > I can actually drop this patch and continue using SKL MOCS defines for
>> gen10+.
>> > I also noticed that vulkan needs separate MOCS defines for each gen. Any
>> > preferences for GL driver?
>>
>
> Let's have separate #defines.  That way we can change it later if we want.
>
>
>> I liked the way the vulkan driver does it, and would like to make the GL
>> driver do the same, at least on the gen specific code. That said, I don't
>> have
>> a preference regarding keeping it or dropping it.
>>
>
> Vulkan may be moving more towards the integers and away from the structs.
> Integers are easier to deal with in a lot of cases.
>
>
>> Also consider it:
>>
>> Reviewed-by: Rafael Antognolli 
>>
>> > > V2: Rebased.
>> > >
>> > > Signed-off-by: Anuj Phogat 
>> > > ---
>> > >  src/mesa/drivers/dri/i965/brw_blorp.c| 6 +++---
>> > >  src/mesa/drivers/dri/i965/brw_state.h| 8 
>> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 ++
>> > >  src/mesa/drivers/dri/i965/genX_state_upload.c| 4 +++-
>> > >  4 files changed, 16 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>> b/src/mesa/drivers/dri/i965/brw_blorp.c
>> > > index 8d3bcbb..bcc72df 100644
>> > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> > > @@ -100,9 +100,9 @@ brw_blorp_init(struct brw_context *brw)
>> > >brw->blorp.exec = gen9_blorp_exec;
>> > >break;
>> > > case 10:
>> > > -  brw->blorp.mocs.tex = SKL_MOCS_WB;
>> > > -  brw->blorp.mocs.rb = SKL_MOCS_PTE;
>> > > -  brw->blorp.mocs.vb = SKL_MOCS_WB;
>> > > +  brw->blorp.mocs.tex = CNL_MOCS_WB;
>> > > +  brw->blorp.mocs.rb = CNL_MOCS_PTE;
>> > > +  brw->blorp.mocs.vb = CNL_MOCS_WB;
>> > >brw->blorp.exec = gen10_blorp_exec;
>> > >break;
>> > > default:
>> > > diff --git a/src/mesa/drivers/dri/i965/brw_state.h
>> b/src/mesa/drivers/dri/i965/brw_state.h
>> > > index 4592e3e..4503946 100644
>> > > --- a/src/mesa/drivers/dri/i965/brw_state.h
>> > > +++ b/src/mesa/drivers/dri/i965/brw_state.h
>> > > @@ -410,6 +410,14 @@ void upload_gs_state_for_tf(struct brw_context
>> *brw);
>> > >  /* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */
>> > >  #define SKL_MOCS_PTE (1 << 1)
>> > >
>> > > +/* Cannonlake: MOCS is now an index into an array of 62 different
>> caching
>> > > + * configurations programmed by the kernel.
>> > > + */
>> > > +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */
>> > > +#define CNL_MOCS_WB  (2 << 1)
>> > > +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */
>> > > +#define CNL_MOCS_PTE (1 << 1)
>> > > +
>> > >  #ifdef __cplusplus
>> > >  }
>> > >  #endif
>> > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> > > index c95fb37..c1003cd 100644
>> > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> > > @@ -64,12 +64,14 @@ uint32_t tex_mocs[] = {
>> > > [7] = GEN7_MOCS_L3,
>> > > [8] = BDW_MOCS_WB,
>> > > [9] = SKL_MOCS_WB,
>> > > +   [10] = CNL_MOCS_WB,
>> > >  };
>> > >
>> > >  uint32_t rb_mocs[] = {
>> > > [7] = GEN7_MOCS_L3,
>> > > [8] = BDW_MOCS_PTE,
>> > > [9] = SKL_MOCS_PTE,
>> > > +   [10] = CNL_MOCS_PTE,
>> > >  };
>> > >
>> > >  static void
>> > > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
>> b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> > > index 6619d4d..5710878 100644
>> > > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
>> > > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> > > @@ -333,7 +333,9 @@ genX(emit_vertex_buffer_state)(struct
>> brw_context *brw,
>> > >  #endif
>> > >  #endif
>> > >
>> > > -#if GEN_GEN == 9
>> > > +#if GEN_GEN == 10
>> > > +  .VertexBufferMOCS = CNL_MOCS_WB,
>> > > +#elif GEN_GEN == 9
>> > >.VertexBufferMOCS = SKL_MOCS_WB,
>> > >  #elif GEN_GEN == 8
>> > >.VertexBufferMOCS = BDW_MOCS_WB,
>> > > --
>> > > 2.9.3
>> > >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

Re: [Mesa-dev] [PATCH V2 15/24] i965/cnl: Start using CNL MOCS defines

2017-06-02 Thread Jason Ekstrand
On Mon, May 22, 2017 at 9:54 AM, Rafael Antognolli <
rafael.antogno...@intel.com> wrote:

> On Tue, May 16, 2017 at 04:01:30PM -0700, Anuj Phogat wrote:
> > On Tue, May 16, 2017 at 10:34 AM, Anuj Phogat 
> wrote:
> > > CNL MOCS defines are duplicates of SKL MOCS defines.
> > >
> > I can actually drop this patch and continue using SKL MOCS defines for
> gen10+.
> > I also noticed that vulkan needs separate MOCS defines for each gen. Any
> > preferences for GL driver?
>

Let's have separate #defines.  That way we can change it later if we want.


> I liked the way the vulkan driver does it, and would like to make the GL
> driver do the same, at least on the gen specific code. That said, I don't
> have
> a preference regarding keeping it or dropping it.
>

Vulkan may be moving more towards the integers and away from the structs.
Integers are easier to deal with in a lot of cases.


> Also consider it:
>
> Reviewed-by: Rafael Antognolli 
>
> > > V2: Rebased.
> > >
> > > Signed-off-by: Anuj Phogat 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_blorp.c| 6 +++---
> > >  src/mesa/drivers/dri/i965/brw_state.h| 8 
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 ++
> > >  src/mesa/drivers/dri/i965/genX_state_upload.c| 4 +++-
> > >  4 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > index 8d3bcbb..bcc72df 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > @@ -100,9 +100,9 @@ brw_blorp_init(struct brw_context *brw)
> > >brw->blorp.exec = gen9_blorp_exec;
> > >break;
> > > case 10:
> > > -  brw->blorp.mocs.tex = SKL_MOCS_WB;
> > > -  brw->blorp.mocs.rb = SKL_MOCS_PTE;
> > > -  brw->blorp.mocs.vb = SKL_MOCS_WB;
> > > +  brw->blorp.mocs.tex = CNL_MOCS_WB;
> > > +  brw->blorp.mocs.rb = CNL_MOCS_PTE;
> > > +  brw->blorp.mocs.vb = CNL_MOCS_WB;
> > >brw->blorp.exec = gen10_blorp_exec;
> > >break;
> > > default:
> > > diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> > > index 4592e3e..4503946 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_state.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_state.h
> > > @@ -410,6 +410,14 @@ void upload_gs_state_for_tf(struct brw_context
> *brw);
> > >  /* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */
> > >  #define SKL_MOCS_PTE (1 << 1)
> > >
> > > +/* Cannonlake: MOCS is now an index into an array of 62 different
> caching
> > > + * configurations programmed by the kernel.
> > > + */
> > > +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */
> > > +#define CNL_MOCS_WB  (2 << 1)
> > > +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */
> > > +#define CNL_MOCS_PTE (1 << 1)
> > > +
> > >  #ifdef __cplusplus
> > >  }
> > >  #endif
> > > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > index c95fb37..c1003cd 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > > @@ -64,12 +64,14 @@ uint32_t tex_mocs[] = {
> > > [7] = GEN7_MOCS_L3,
> > > [8] = BDW_MOCS_WB,
> > > [9] = SKL_MOCS_WB,
> > > +   [10] = CNL_MOCS_WB,
> > >  };
> > >
> > >  uint32_t rb_mocs[] = {
> > > [7] = GEN7_MOCS_L3,
> > > [8] = BDW_MOCS_PTE,
> > > [9] = SKL_MOCS_PTE,
> > > +   [10] = CNL_MOCS_PTE,
> > >  };
> > >
> > >  static void
> > > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > > index 6619d4d..5710878 100644
> > > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > > @@ -333,7 +333,9 @@ genX(emit_vertex_buffer_state)(struct brw_context
> *brw,
> > >  #endif
> > >  #endif
> > >
> > > -#if GEN_GEN == 9
> > > +#if GEN_GEN == 10
> > > +  .VertexBufferMOCS = CNL_MOCS_WB,
> > > +#elif GEN_GEN == 9
> > >.VertexBufferMOCS = SKL_MOCS_WB,
> > >  #elif GEN_GEN == 8
> > >.VertexBufferMOCS = BDW_MOCS_WB,
> > > --
> > > 2.9.3
> > >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 08/30] i965: Inline renderbuffer_att_set_needs_depth_resolve

2017-06-02 Thread Jason Ekstrand
On Fri, Jun 2, 2017 at 2:41 PM, Chad Versace 
wrote:

> On Fri 26 May 2017, Jason Ekstrand wrote:
> > ---
> >  src/mesa/drivers/dri/i965/brw_clear.c | 12 ++--
> >  src/mesa/drivers/dri/i965/brw_draw.c  | 12 +++-
> >  src/mesa/drivers/dri/i965/intel_fbo.c | 15 ---
> >  src/mesa/drivers/dri/i965/intel_fbo.h |  3 ---
> >  4 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> > index adaf250..9f4a7e6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_clear.c
> > +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> > @@ -207,7 +207,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> > }
> >
> > -   if (fb->MaxNumLayers > 0) {
> > +   if (depth_att->Layered) {
> >for (unsigned layer = 0; layer < depth_irb->layer_count; layer++)
> {
> >   intel_hiz_exec(brw, mt, depth_irb->mt_level,
> >  depth_irb->mt_layer + layer,
>
> There's a hidden 'else' branch here. My comment below applies to it too.
>
> > @@ -236,7 +236,15 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > /* Now, the HiZ buffer contains data that needs to be resolved to
> the depth
> >  * buffer.
> >  */
> > -   intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> > +   if (fb->MaxNumLayers > 0) {
> > +  for (unsigned layer = 0; layer < depth_irb->layer_count; layer++)
> {
> > + intel_miptree_slice_set_needs_depth_resolve(mt,
> depth_irb->mt_level,
> > +
>  depth_irb->mt_layer + layer);
> > +  }
> > +   } else {
> > +  intel_miptree_slice_set_needs_depth_resolve(mt,
> depth_irb->mt_level,
> > +  depth_irb->mt_layer);
> > +   }
>
> I believe you could simplify this by eliminating the 'else' branch. As
> long as depth_irb->layer_count == 1 for non-layered renderbuffers (and
> I really hope it is), then the 'for' loop does the right thing.
>

Sure.  I was sort-of trying to avoid functional changes.  That said... I'm
happy to make the change.  Lots of stuff would suddenly get cleaner.  Mind
if that's a follow-on patch to the series?


> >
> > return true;
> >  }
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> > index 23a3c6c..f728731 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -373,7 +373,17 @@ brw_postdraw_set_buffers_need_resolve(struct
> brw_context *brw)
> > if (back_irb)
> >back_irb->need_downsample = true;
> > if (depth_irb && brw_depth_writes_enabled(brw)) {
> > -  intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> > +  if (depth_att->Layered) {
> > + for (unsigned layer = 0; layer < depth_irb->layer_count;
> layer++) {
> > +intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> > +
> depth_irb->mt_level,
> > +
> depth_irb->mt_layer + layer);
> > + }
> > +  } else {
>
> And this branch too.
>
> > + intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> > +
>  depth_irb->mt_level,
> > +
>  depth_irb->mt_layer);
> > +  }
> >brw_render_cache_set_add_bo(brw, depth_irb->mt->bo);
> > }
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 09/30] i965/miptree: Move color resolve on map to intel_miptree_map

2017-06-02 Thread Jason Ekstrand
On Fri, Jun 2, 2017 at 3:03 PM, Chad Versace 
wrote:

> On Fri 26 May 2017, Jason Ekstrand wrote:
> > None of the other methods such as blit work with CCS either so we need
> > to do the resolve for all maps.
>
> Not exactly. No down-resolve is needed for maps with
> GL_MAP_INVALIDATE_RANGE_BIT. But that's a separate problem for
> a separate patch.
>

Correct.  I thought about that but I don't think enough people are
uploading depth buffers to make it worth trying to avoid the resolve.


> Reviewed-by: Chad Versace 


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


[Mesa-dev] [PATCH 2/2] radv: Dirty all descriptors sets when changng the pipeline.

2017-06-02 Thread Bas Nieuwenhuizen
Sets could have been ignored during previous descriptor set flush
due to the shader not using them and therefore no SGPR being assigned.

Signed-off-by: Bas Nieuwenhuizen 
Fixes: ae61ddabe8c "radv: move userdata sgpr ownership to compiler side."
---
 src/amd/vulkan/radv_cmd_buffer.c | 17 -
 src/amd/vulkan/radv_meta.c   |  5 ++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index e0ea65604f4..a7a2aa90460 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2186,6 +2186,13 @@ radv_emit_compute_pipeline(struct radv_cmd_buffer 
*cmd_buffer)
assert(cmd_buffer->cs->cdw <= cdw_max);
 }
 
+static void radv_mark_descriptor_sets_dirty(struct radv_cmd_buffer *cmd_buffer)
+{
+   for (unsigned i = 0; i < MAX_SETS; i++) {
+   if (cmd_buffer->state.descriptors[i])
+   cmd_buffer->state.descriptors_dirty |= (1u << i);
+   }
+}
 
 void radv_CmdBindPipeline(
VkCommandBuffer commandBuffer,
@@ -2195,10 +2202,7 @@ void radv_CmdBindPipeline(
RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
RADV_FROM_HANDLE(radv_pipeline, pipeline, _pipeline);
 
-   for (unsigned i = 0; i < MAX_SETS; i++) {
-   if (cmd_buffer->state.descriptors[i])
-   cmd_buffer->state.descriptors_dirty |= (1u << i);
-   }
+   radv_mark_descriptor_sets_dirty(cmd_buffer);
 
switch (pipelineBindPoint) {
case VK_PIPELINE_BIND_POINT_COMPUTE:
@@ -2207,6 +2211,9 @@ void radv_CmdBindPipeline(
break;
case VK_PIPELINE_BIND_POINT_GRAPHICS:
cmd_buffer->state.pipeline = pipeline;
+   if (!pipeline)
+   break;
+
cmd_buffer->state.vertex_descriptors_dirty = true;
cmd_buffer->state.dirty |= RADV_CMD_DIRTY_PIPELINE;
cmd_buffer->push_constant_stages |= pipeline->active_stages;
@@ -2369,7 +2376,6 @@ void radv_CmdSetStencilReference(
cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_REFERENCE;
 }
 
-
 void radv_CmdExecuteCommands(
VkCommandBuffer commandBuffer,
uint32_tcommandBufferCount,
@@ -2414,6 +2420,7 @@ void radv_CmdExecuteCommands(
primary->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_ALL;
primary->state.last_primitive_reset_en = -1;
primary->state.last_primitive_reset_index = 0;
+   radv_mark_descriptor_sets_dirty(primary);
}
 }
 
diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
index 4f359bd6a9d..263181a57f2 100644
--- a/src/amd/vulkan/radv_meta.c
+++ b/src/amd/vulkan/radv_meta.c
@@ -50,9 +50,9 @@ void
 radv_meta_restore(const struct radv_meta_saved_state *state,
  struct radv_cmd_buffer *cmd_buffer)
 {
-   cmd_buffer->state.pipeline = state->old_pipeline;
+   radv_CmdBindPipeline(radv_cmd_buffer_to_handle(cmd_buffer), 
VK_PIPELINE_BIND_POINT_GRAPHICS,
+radv_pipeline_to_handle(state->old_pipeline));
cmd_buffer->state.descriptors[0] = state->old_descriptor_set0;
-   cmd_buffer->state.descriptors_dirty |= (1u << 0);
if (state->vertex_saved) {
memcpy(cmd_buffer->state.vertex_bindings, 
state->old_vertex_bindings,
   sizeof(state->old_vertex_bindings));
@@ -114,7 +114,6 @@ radv_meta_restore_compute(const struct 
radv_meta_saved_compute_state *state,
 radv_pipeline_to_handle(state->old_pipeline));
 
cmd_buffer->state.descriptors[0] = state->old_descriptor_set0;
-   cmd_buffer->state.descriptors_dirty |= (1u << 0);
 
if (push_constant_size) {
memcpy(cmd_buffer->push_constants, state->push_constants, 
push_constant_size);
-- 
2.13.0

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


[Mesa-dev] [PATCH 1/2] radv: Set both compute and graphics SGPRS on descriptor set flush.

2017-06-02 Thread Bas Nieuwenhuizen
We clear the descriptors_dirty array afterwards, so the SGPRs for
the other pipeline don't get updated on the flush for that other
draw/dispatch, so we have to make sure we do it immediately.

Signed-off-by: Bas Nieuwenhuizen 
Fixes: ae61ddabe8c "radv: move userdata sgpr ownership to compiler side."
---
 src/amd/vulkan/radv_cmd_buffer.c | 100 +++
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 2ed93564b96..e0ea65604f4 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -1243,38 +1243,39 @@ emit_stage_descriptor_set_userdata(struct 
radv_cmd_buffer *cmd_buffer,
 
 static void
 radv_emit_descriptor_set_userdata(struct radv_cmd_buffer *cmd_buffer,
- struct radv_pipeline *pipeline,
  VkShaderStageFlags stages,
  struct radv_descriptor_set *set,
  unsigned idx)
 {
-   if (stages & VK_SHADER_STAGE_FRAGMENT_BIT)
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
-  idx, set->va,
-  MESA_SHADER_FRAGMENT);
+   if (cmd_buffer->state.pipeline) {
+   if (stages & VK_SHADER_STAGE_FRAGMENT_BIT)
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  
MESA_SHADER_FRAGMENT);
 
-   if (stages & VK_SHADER_STAGE_VERTEX_BIT)
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
-  idx, set->va,
-  MESA_SHADER_VERTEX);
+   if (stages & VK_SHADER_STAGE_VERTEX_BIT)
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  MESA_SHADER_VERTEX);
 
-   if ((stages & VK_SHADER_STAGE_GEOMETRY_BIT) && 
radv_pipeline_has_gs(pipeline))
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
-  idx, set->va,
-  MESA_SHADER_GEOMETRY);
+   if ((stages & VK_SHADER_STAGE_GEOMETRY_BIT) && 
radv_pipeline_has_gs(cmd_buffer->state.pipeline))
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  
MESA_SHADER_GEOMETRY);
 
-   if ((stages & VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT) && 
radv_pipeline_has_tess(pipeline))
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
-  idx, set->va,
-  MESA_SHADER_TESS_CTRL);
+   if ((stages & VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT) && 
radv_pipeline_has_tess(cmd_buffer->state.pipeline))
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  
MESA_SHADER_TESS_CTRL);
 
-   if ((stages & VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT) && 
radv_pipeline_has_tess(pipeline))
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
-  idx, set->va,
-  MESA_SHADER_TESS_EVAL);
+   if ((stages & VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT) && 
radv_pipeline_has_tess(cmd_buffer->state.pipeline))
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.pipeline,
+  idx, set->va,
+  
MESA_SHADER_TESS_EVAL);
+   }
 
-   if (stages & VK_SHADER_STAGE_COMPUTE_BIT)
-   emit_stage_descriptor_set_userdata(cmd_buffer, pipeline,
+   if (cmd_buffer->state.compute_pipeline && (stages & 
VK_SHADER_STAGE_COMPUTE_BIT))
+   emit_stage_descriptor_set_userdata(cmd_buffer, 
cmd_buffer->state.compute_pipeline,
   idx, set->va,
   MESA_SHADER_COMPUTE);
 }
@@ -1298,8 +1299,7 @@ radv_flush_push_descriptors(struct radv_cmd_buffer 
*cmd_buffer)
 }
 
 static void
-radv_flush_indirect_descriptor_sets(struct radv_cmd_buffer *cmd_buffer,
- 

Re: [Mesa-dev] [PATCH 10/30] i965/blorp: Take an explicit fast clear op in resolve_color

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_blorp.c | 15 ++-
>  src/mesa/drivers/dri/i965/brw_blorp.h |  3 ++-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 ++-
>  3 files changed, 18 insertions(+), 15 deletions(-)

Reviewed-by: Chad Versace 

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


Re: [Mesa-dev] [PATCH 09/30] i965/miptree: Move color resolve on map to intel_miptree_map

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> None of the other methods such as blit work with CCS either so we need
> to do the resolve for all maps.

Not exactly. No down-resolve is needed for maps with
GL_MAP_INVALIDATE_RANGE_BIT. But that's a separate problem for
a separate patch.

Reviewed-by: Chad Versace 

> This change also makes us only resolve
> the one slice we're mapping and not the entire image.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 08/30] i965: Inline renderbuffer_att_set_needs_depth_resolve

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_clear.c | 12 ++--
>  src/mesa/drivers/dri/i965/brw_draw.c  | 12 +++-
>  src/mesa/drivers/dri/i965/intel_fbo.c | 15 ---
>  src/mesa/drivers/dri/i965/intel_fbo.h |  3 ---
>  4 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c 
> b/src/mesa/drivers/dri/i965/brw_clear.c
> index adaf250..9f4a7e6 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -207,7 +207,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
> brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> }
>  
> -   if (fb->MaxNumLayers > 0) {
> +   if (depth_att->Layered) {
>for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {
>   intel_hiz_exec(brw, mt, depth_irb->mt_level,
>  depth_irb->mt_layer + layer,

There's a hidden 'else' branch here. My comment below applies to it too.

> @@ -236,7 +236,15 @@ brw_fast_clear_depth(struct gl_context *ctx)
> /* Now, the HiZ buffer contains data that needs to be resolved to the 
> depth
>  * buffer.
>  */
> -   intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> +   if (fb->MaxNumLayers > 0) {
> +  for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {
> + intel_miptree_slice_set_needs_depth_resolve(mt, depth_irb->mt_level,
> + depth_irb->mt_layer + 
> layer);
> +  }
> +   } else {
> +  intel_miptree_slice_set_needs_depth_resolve(mt, depth_irb->mt_level,
> +  depth_irb->mt_layer);
> +   }

I believe you could simplify this by eliminating the 'else' branch. As
long as depth_irb->layer_count == 1 for non-layered renderbuffers (and
I really hope it is), then the 'for' loop does the right thing.

>  
> return true;
>  }
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index 23a3c6c..f728731 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -373,7 +373,17 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context 
> *brw)
> if (back_irb)
>back_irb->need_downsample = true;
> if (depth_irb && brw_depth_writes_enabled(brw)) {
> -  intel_renderbuffer_att_set_needs_depth_resolve(depth_att);
> +  if (depth_att->Layered) {
> + for (unsigned layer = 0; layer < depth_irb->layer_count; layer++) {
> +intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> +depth_irb->mt_level,
> +depth_irb->mt_layer 
> + layer);
> + }
> +  } else {

And this branch too.

> + intel_miptree_slice_set_needs_depth_resolve(depth_irb->mt,
> + depth_irb->mt_level,
> + depth_irb->mt_layer);
> +  }
>brw_render_cache_set_add_bo(brw, depth_irb->mt->bo);
> }
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 14/30] intel/isl: Add an enum for describing auxiliary compression state

2017-06-02 Thread Nanley Chery
On Fri, Jun 02, 2017 at 01:05:47PM -0700, Jason Ekstrand wrote:
> On Fri, Jun 2, 2017 at 12:57 PM, Nanley Chery  wrote:
> 
> > On Fri, May 26, 2017 at 04:30:18PM -0700, Jason Ekstrand wrote:
> > > This enum describes all of the states that a auxiliary compressed
> >  ^
> >  an
> > > surface can have.  All of the states as well as normative language for
> > > referring to each of the compression operations is provided in the
> > > truly colossal comment for the new isl_aux_state enum.  There is also
> > > a diagram showing how surfaces move between the different states.
> > > ---
> > >  src/intel/isl/isl.h | 142 ++
> > ++
> > >  1 file changed, 142 insertions(+)
> > >
> > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > > index b9d8fa8..df6d3e3 100644
> > > --- a/src/intel/isl/isl.h
> > > +++ b/src/intel/isl/isl.h
> > > @@ -560,6 +560,148 @@ enum isl_aux_usage {
> > > ISL_AUX_USAGE_CCS_E,
> > >  };
> > >
> > > +/**
> > > + * Enum for keeping track of the state an auxiliary compressed surface.
> > > + *
> > > + * For any given auxiliary surface compression format (HiZ, CCS, or
> > MCS), any
> > > + * given slice (lod + array layer) can be in one of the six states
> > described
> > > + * by this enum.  Draw and resolve operations may cause the slice to
> > change
> > > + * from one state to another.  The six valid states are:
> > > + *
> > > + *1) Clear:  In this state, each block in the auxiliary surface
> > contains a
> > > + *   magic value that indicates that the block is in the clear
> > state.  If
> > > + *   a block is in the clear state, it's values in the primary
> > surface are
> > > + *   ignored and the color of the samples in the block is taken
> > either the
> > > + *   RENDER_SURFACE_STATE packet for color or 3DSTATE_CLEAR_PARAMS
> > for
> > > + *   depth.  Since neither the primary surface nor the auxiliary
> > surface
> > > + *   contains the clear value, the surface can be cleared to a
> > different
> > > + *   color by simply changing the clear color without modifying
> > either
> > > + *   surface.
> > > + *
> > > + *2) Compressed w/ Clear:  In this state, neither the auxiliary
> > surface
> > > + *   nor the primary surface has a complete representation of the
> > data.
> > > + *   Instead, both surfaces must be used together or else rendering
> > > + *   corruption may occur.  Depending on the auxiliary compression
> > format
> > > + *   and the data, any given block in the primary surface may
> > contain all,
> > > + *   some, or none of the data required to reconstruct the actual
> > sample
> > > + *   values.  Blocks may also be in the clear state (see Clear) and
> > have
> > > + *   their value taken from outside the surface.
> > > + *
> > > + *3) Compressed w/o Clear:  This state is identical to the state
> > above
> > > + *   except that no blocks are in the clear state.  In this state,
> > all of
> > > + *   the data required to reconstruct the final sample values is
> > contained
> > > + *   in the auxiliary and primary surface and the clear value is not
> > > + *   considered.
> > > + *
> > > + *4) Resolved:  In this state, the primary surface contains 100% of
> > the
> > > + *   data.  The auxiliary surface is also valid so the surface can
> > be
> > > + *   validly used with or without aux enabled.  The auxiliary
> > surface may,
> > > + *   however, contain non-trivial data and any update to the primary
> > > + *   surface with aux disabled will cause the two to get out of
> > sync.
> > > + *
> > > + *5) Pass-through:  In this state, the primary surface contains
> > 100% of the
> > > + *   data and every block in the auxiliary surface contains a magic
> > value
> > > + *   which indicates that the auxiliary surface should be ignored
> > and the
> > > + *   only the primary surface should be considered.  Updating the
> > primary
> > > + *   surface without aux works fine and can be done repeatedly in
> > this
> > > + *   mode.  Writing to a surface in pass-through mode with aux
> > enabled may
> > > + *   cause the auxiliary buffer to contain non-trivial data and no
> > longer
> > > + *   be in the pass-through state.
> > > + *
> > > + *5) Aux Invalid:  In this state, the primary surface contains 100%
> > of the
> >  ^
> >  6
> > > + *   data and the auxiliary surface is completely bogus.  Any
> > attempt to
> > > + *   use the auxiliary surface is liable to result in rendering
> > > + *   corruption.  The only thing that one can do to re-enable aux
> > once
> > > + *   this state is reached is to use an ambiguate pass to
> > transition into
> > > + *   the pass-through state.
> > > + *
> > > + * Drawing with or without aux 

Re: [Mesa-dev] [PATCH 07/30] i965: Get rid of intel_renderbuffer_resolve_*

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> There is exactly one caller so it's a bit pointless to have all of this
> plumbing.  Just inline it at the one place it's used.
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |  7 +--
>  src/mesa/drivers/dri/i965/intel_fbo.c   | 26 --
>  src/mesa/drivers/dri/i965/intel_fbo.h   | 24 
>  3 files changed, 5 insertions(+), 52 deletions(-)

This patch brought back old memories. You're ripping up the code from my
first major project at Intel. And it feels good to see it cleaned up.

Patch 7 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 06/30] i965/miptree: Refactor intel_miptree_resolve_color

2017-06-02 Thread Chad Versace
On Wed 31 May 2017, Jason Ekstrand wrote:
> On Tue, May 30, 2017 at 12:40 AM, Pohjolainen, Topi <[1]
> topi.pohjolai...@gmail.com> wrote:
> 
> On Fri, May 26, 2017 at 04:30:10PM -0700, Jason Ekstrand wrote:
> > The new version now takes a range of levels as well as a range of
> > layers.  It should also be a tiny bit faster because it only walks the
> > resolve_map list once instead of once per layer.
> > ---
> >  src/mesa/drivers/dri/i965/brw_blorp.c         |  3 +-
> >  src/mesa/drivers/dri/i965/brw_context.c       |  9 +++---
> >  src/mesa/drivers/dri/i965/brw_draw.c          |  2 +-
> >  src/mesa/drivers/dri/i965/intel_blit.c        |  8 ++---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 46


> Original (a dozen lines earlier) checked for num_layers. Maybe:
> 
>          /* Mipmapped fast clear is only supported for gen8+. */
> 
> > +      assert(brw->gen >= 8 || map->level == 0);
> 
> 
> Actually, I think I want "assert(brw->gen >= 8 || (map->level == 0 && map->
> layer == 0))"

Which is what appears in wip/i965-resolve-rework-v3. The patch in that
branch is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 05/30] i965/miptree: Clean up the depth resolve helpers a little

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 70 
> ---
>  1 file changed, 30 insertions(+), 40 deletions(-)

Patch 5 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 04/30] i965/miptree: Store fast clear colors in an isl_color_value

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> This commit, out of necessity, makes a number of changes at once:
> 
>  1) Changes intel_mipmap_tree to store the clear color for both color
> and depth as an isl_color_value.
> 
>  2) Changes the depth/stencil emit code to do the format conversion of
> the depth clear value on Haswell and earlier instead of pulling a
> uint32_t directly from the miptree.
> 
>  3) Changes ISL's depth/stencil emit code to perform the format
> conversion of the depth clear value on Haswell and earlier instead
> of assuming that the depth value in the float is pre-converted.
> 
>  4) Changes blorp to pass the depth value through as a float.
> ---
>  src/intel/blorp/blorp_genX_exec.h|  2 +-
>  src/intel/isl/isl_emit_depth_stencil.c   | 19 
>  src/mesa/drivers/dri/i965/brw_blorp.c| 18 +++-
>  src/mesa/drivers/dri/i965/brw_clear.c| 15 ++-
>  src/mesa/drivers/dri/i965/brw_meta_util.c| 56 
> +++-
>  src/mesa/drivers/dri/i965/brw_meta_util.h|  7 +--
>  src/mesa/drivers/dri/i965/brw_misc_state.c   | 23 +-
>  src/mesa/drivers/dri/i965/brw_state.h|  3 ++
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  2 +-
>  src/mesa/drivers/dri/i965/gen6_depth_state.c |  7 ++-
>  src/mesa/drivers/dri/i965/gen7_misc_state.c  |  7 ++-
>  src/mesa/drivers/dri/i965/gen8_depth_state.c |  2 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 31 -
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 34 ++
>  14 files changed, 90 insertions(+), 136 deletions(-)

Patch 4 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/30] i965: Mark depth surfaces as needing a HiZ resolve after blitting

2017-06-02 Thread Chad Versace
On Fri 02 Jun 2017, Jason Ekstrand wrote:
> On Fri, Jun 2, 2017 at 1:01 PM, Chad Versace <[1]chadvers...@chromium.org>
> wrote:
> 
> On Wed 31 May 2017, Pohjolainen, Topi wrote:
> > On Wed, May 31, 2017 at 10:22:09AM -0700, Jason Ekstrand wrote:
> > > On Tue, May 30, 2017 at 7:29 AM, Pohjolainen, Topi <
> > > [2]topi.pohjolai...@gmail.com> wrote:
> 
> > > > > On Fri, May 26, 2017 at 04:30:05PM -0700, Jason Ekstrand wrote:
> > > > > > Cc: "17.0 17.1" <[3]mesa-sta...@lists.freedesktop.org>
> > > > > > ---
> > > > > >  src/mesa/drivers/dri/i965/intel_blit.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> > > > b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > index 2925fc2..b1e1eaa 100644
> > > > > > --- a/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > @@ -329,6 +329,7 @@ intel_miptree_blit(struct brw_context *brw,
> > > > > >     intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> > > > dst_slice);
> > > > > >     intel_miptree_resolve_color(brw, src_mt, src_level,
> src_slice, 1,
> > > > 0);
> > > > > >     intel_miptree_resolve_color(brw, dst_mt, dst_level,
> dst_slice, 1,
> > > > 0);
> > > > > > +   intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
> > > > dst_slice);
> 
> Something feels wrong. Suppose that, before the blit, the HiZ buffer
> contains significant data. Here, we schedule an op to invalidate the HiZ
> data. Also, suppose the the below call to emit_miptree_blit() rejects
> the blit (maybe because the pitch is too big) and returns false. The
> failed blit will likely emit a GL error or, in some cases, i965 will
> fallback to to doing the blit with the 3D engine. In either case, there
> is a likely bug waiting because the HiZ buffer is scheduled to have its
> HiZ data invalidated before its next use, but the HiZ data is still
> significant.
> 
> 
> I agree that you are technically mostly correct.  However, I don't think it's
> actually going to matter in practice for 3 reasons:
> 
>  1) The GL error case is bogus.  We aren't allowed to start modifying things
> and then throw a GL error unless that error is OUT_OF_MEMORY.  All of the
> error-checking should have happened up-front and it is our job to succeed
> somehow.
> 
>  2) By the time we get to a blit path, we've given up on HiZ.  We never fall
> back from BLIT to render.  If BLIT fails, we fall back to software at which
> point we're going to write without HiZ anyway so setting needs_hiz_resolve is
> fine.
> 
>  3) There is no bug waiting.  Since we do a depth resolve first, the HiZ and
> depth will be consistent and anything that anything that would use it with HiZ
> will see the fact that it needs a HiZ resolve and do one.  You can't get into 
> a
> case where we say that it needs a hiz resolve and then edit HiZ without
> resolving.  The worst that happens if I'm completely wrong about (1) and (2) 
> is
> that you get more HiZ resolves than needed.

You convinced with me point 3. I failed to take into account the depth
resolve a few lines above.

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


Re: [Mesa-dev] [PATCH 01/30] i965: Mark depth surfaces as needing a HiZ resolve after blitting

2017-06-02 Thread Jason Ekstrand
On Fri, Jun 2, 2017 at 1:01 PM, Chad Versace 
wrote:

> On Wed 31 May 2017, Pohjolainen, Topi wrote:
> > On Wed, May 31, 2017 at 10:22:09AM -0700, Jason Ekstrand wrote:
> > > On Tue, May 30, 2017 at 7:29 AM, Pohjolainen, Topi <
> > > topi.pohjolai...@gmail.com> wrote:
>
> > > > > On Fri, May 26, 2017 at 04:30:05PM -0700, Jason Ekstrand wrote:
> > > > > > Cc: "17.0 17.1" 
> > > > > > ---
> > > > > >  src/mesa/drivers/dri/i965/intel_blit.c | 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> > > > b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > index 2925fc2..b1e1eaa 100644
> > > > > > --- a/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > > @@ -329,6 +329,7 @@ intel_miptree_blit(struct brw_context *brw,
> > > > > > intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> > > > dst_slice);
> > > > > > intel_miptree_resolve_color(brw, src_mt, src_level,
> src_slice, 1,
> > > > 0);
> > > > > > intel_miptree_resolve_color(brw, dst_mt, dst_level,
> dst_slice, 1,
> > > > 0);
> > > > > > +   intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
> > > > dst_slice);
>
> Something feels wrong. Suppose that, before the blit, the HiZ buffer
> contains significant data. Here, we schedule an op to invalidate the HiZ
> data. Also, suppose the the below call to emit_miptree_blit() rejects
> the blit (maybe because the pitch is too big) and returns false. The
> failed blit will likely emit a GL error or, in some cases, i965 will
> fallback to to doing the blit with the 3D engine. In either case, there
> is a likely bug waiting because the HiZ buffer is scheduled to have its
> HiZ data invalidated before its next use, but the HiZ data is still
> significant.
>

I agree that you are technically mostly correct.  However, I don't think
it's actually going to matter in practice for 3 reasons:

 1) The GL error case is bogus.  We aren't allowed to start modifying
things and then throw a GL error unless that error is OUT_OF_MEMORY.  All
of the error-checking should have happened up-front and it is our job to
succeed somehow.

 2) By the time we get to a blit path, we've given up on HiZ.  We never
fall back from BLIT to render.  If BLIT fails, we fall back to software at
which point we're going to write without HiZ anyway so setting
needs_hiz_resolve is fine.

 3) There is no bug waiting.  Since we do a depth resolve first, the HiZ
and depth will be consistent and anything that anything that would use it
with HiZ will see the fact that it needs a HiZ resolve and do one.  You
can't get into a case where we say that it needs a hiz resolve and then
edit HiZ without resolving.  The worst that happens if I'm completely wrong
about (1) and (2) is that you get more HiZ resolves than needed.

--Jason


> The intel_miptree_slice_set_needs_hiz_resolve() needs to happen *after*
> the successfull call to emit_miptree_blit().
>
> > > > > >
> > > > > > if (src_flip)
> > > > > >src_y = minify(src_mt->physical_height0, src_level -
> > > > src_mt->first_level) - src_y - height;
> > > > > > @@ -387,6 +388,7 @@ intel_miptree_copy(struct brw_context *brw,
> > > > > > intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> > > > dst_slice);
> > > > > > intel_miptree_resolve_color(brw, src_mt, src_level,
> src_slice, 1,
> > > > 0);
> > > > > > intel_miptree_resolve_color(brw, dst_mt, dst_level,
> dst_slice, 1,
> > > > 0);
> > > > > > +   intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
> > > > dst_slice);
>
> Same problem again.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 03/30] intel/isl: Add a helper for determining if a color is 0/1

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> ---
>  src/intel/isl/isl.c | 19 +++
>  src/intel/isl/isl.h |  4 
>  2 files changed, 23 insertions(+)
> 
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 321850e..4eec2fd 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -267,6 +267,25 @@ isl_tiling_get_info(const struct isl_device *dev,
> return true;
>  }
>  
> +bool
> +isl_color_value_is_zero_one(union isl_color_value value,
> +enum isl_format format)
> +{
> +   if (isl_format_has_int_channel(format)) {
> +  for (unsigned i = 0; i < 4; i++) {

This patch assumes the all four channels of isl_color_value are
initialized, even if the isl_format has fewer than four channels.
This smells like the seed of a hard-to-reproduce bug.

Since it's not possible today to query the number of channels in an
isl_format, please at least add a scary comment on the function about
this.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 14/30] intel/isl: Add an enum for describing auxiliary compression state

2017-06-02 Thread Jason Ekstrand
On Fri, Jun 2, 2017 at 12:57 PM, Nanley Chery  wrote:

> On Fri, May 26, 2017 at 04:30:18PM -0700, Jason Ekstrand wrote:
> > This enum describes all of the states that a auxiliary compressed
>  ^
>  an
> > surface can have.  All of the states as well as normative language for
> > referring to each of the compression operations is provided in the
> > truly colossal comment for the new isl_aux_state enum.  There is also
> > a diagram showing how surfaces move between the different states.
> > ---
> >  src/intel/isl/isl.h | 142 ++
> ++
> >  1 file changed, 142 insertions(+)
> >
> > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > index b9d8fa8..df6d3e3 100644
> > --- a/src/intel/isl/isl.h
> > +++ b/src/intel/isl/isl.h
> > @@ -560,6 +560,148 @@ enum isl_aux_usage {
> > ISL_AUX_USAGE_CCS_E,
> >  };
> >
> > +/**
> > + * Enum for keeping track of the state an auxiliary compressed surface.
> > + *
> > + * For any given auxiliary surface compression format (HiZ, CCS, or
> MCS), any
> > + * given slice (lod + array layer) can be in one of the six states
> described
> > + * by this enum.  Draw and resolve operations may cause the slice to
> change
> > + * from one state to another.  The six valid states are:
> > + *
> > + *1) Clear:  In this state, each block in the auxiliary surface
> contains a
> > + *   magic value that indicates that the block is in the clear
> state.  If
> > + *   a block is in the clear state, it's values in the primary
> surface are
> > + *   ignored and the color of the samples in the block is taken
> either the
> > + *   RENDER_SURFACE_STATE packet for color or 3DSTATE_CLEAR_PARAMS
> for
> > + *   depth.  Since neither the primary surface nor the auxiliary
> surface
> > + *   contains the clear value, the surface can be cleared to a
> different
> > + *   color by simply changing the clear color without modifying
> either
> > + *   surface.
> > + *
> > + *2) Compressed w/ Clear:  In this state, neither the auxiliary
> surface
> > + *   nor the primary surface has a complete representation of the
> data.
> > + *   Instead, both surfaces must be used together or else rendering
> > + *   corruption may occur.  Depending on the auxiliary compression
> format
> > + *   and the data, any given block in the primary surface may
> contain all,
> > + *   some, or none of the data required to reconstruct the actual
> sample
> > + *   values.  Blocks may also be in the clear state (see Clear) and
> have
> > + *   their value taken from outside the surface.
> > + *
> > + *3) Compressed w/o Clear:  This state is identical to the state
> above
> > + *   except that no blocks are in the clear state.  In this state,
> all of
> > + *   the data required to reconstruct the final sample values is
> contained
> > + *   in the auxiliary and primary surface and the clear value is not
> > + *   considered.
> > + *
> > + *4) Resolved:  In this state, the primary surface contains 100% of
> the
> > + *   data.  The auxiliary surface is also valid so the surface can
> be
> > + *   validly used with or without aux enabled.  The auxiliary
> surface may,
> > + *   however, contain non-trivial data and any update to the primary
> > + *   surface with aux disabled will cause the two to get out of
> sync.
> > + *
> > + *5) Pass-through:  In this state, the primary surface contains
> 100% of the
> > + *   data and every block in the auxiliary surface contains a magic
> value
> > + *   which indicates that the auxiliary surface should be ignored
> and the
> > + *   only the primary surface should be considered.  Updating the
> primary
> > + *   surface without aux works fine and can be done repeatedly in
> this
> > + *   mode.  Writing to a surface in pass-through mode with aux
> enabled may
> > + *   cause the auxiliary buffer to contain non-trivial data and no
> longer
> > + *   be in the pass-through state.
> > + *
> > + *5) Aux Invalid:  In this state, the primary surface contains 100%
> of the
>  ^
>  6
> > + *   data and the auxiliary surface is completely bogus.  Any
> attempt to
> > + *   use the auxiliary surface is liable to result in rendering
> > + *   corruption.  The only thing that one can do to re-enable aux
> once
> > + *   this state is reached is to use an ambiguate pass to
> transition into
> > + *   the pass-through state.
> > + *
> > + * Drawing with or without aux enabled may implicitly cause the surface
> to
> > + * transition between these states.  There are also four types of
> "resolve"
> > + * operations which cause an explicit transition:
> > + *
>
> I'm not sure why Fast Clear is described as a "resolve" operation.  To
> me, "resolve" operations are 

Re: [Mesa-dev] [PATCH 02/30] i965/surface_state: Images can't handle CCS at all

2017-06-02 Thread Chad Versace
On Fri 26 May 2017, Jason Ekstrand wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Patch 2 is
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 01/30] i965: Mark depth surfaces as needing a HiZ resolve after blitting

2017-06-02 Thread Chad Versace
On Wed 31 May 2017, Pohjolainen, Topi wrote:
> On Wed, May 31, 2017 at 10:22:09AM -0700, Jason Ekstrand wrote:
> > On Tue, May 30, 2017 at 7:29 AM, Pohjolainen, Topi <
> > topi.pohjolai...@gmail.com> wrote:

> > > > On Fri, May 26, 2017 at 04:30:05PM -0700, Jason Ekstrand wrote:
> > > > > Cc: "17.0 17.1" 
> > > > > ---
> > > > >  src/mesa/drivers/dri/i965/intel_blit.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> > > b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > index 2925fc2..b1e1eaa 100644
> > > > > --- a/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> > > > > @@ -329,6 +329,7 @@ intel_miptree_blit(struct brw_context *brw,
> > > > > intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> > > dst_slice);
> > > > > intel_miptree_resolve_color(brw, src_mt, src_level, src_slice, 1,
> > > 0);
> > > > > intel_miptree_resolve_color(brw, dst_mt, dst_level, dst_slice, 1,
> > > 0);
> > > > > +   intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
> > > dst_slice);

Something feels wrong. Suppose that, before the blit, the HiZ buffer
contains significant data. Here, we schedule an op to invalidate the HiZ
data. Also, suppose the the below call to emit_miptree_blit() rejects
the blit (maybe because the pitch is too big) and returns false. The
failed blit will likely emit a GL error or, in some cases, i965 will
fallback to to doing the blit with the 3D engine. In either case, there
is a likely bug waiting because the HiZ buffer is scheduled to have its
HiZ data invalidated before its next use, but the HiZ data is still
significant.

The intel_miptree_slice_set_needs_hiz_resolve() needs to happen *after*
the successfull call to emit_miptree_blit().

> > > > >
> > > > > if (src_flip)
> > > > >src_y = minify(src_mt->physical_height0, src_level -
> > > src_mt->first_level) - src_y - height;
> > > > > @@ -387,6 +388,7 @@ intel_miptree_copy(struct brw_context *brw,
> > > > > intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> > > dst_slice);
> > > > > intel_miptree_resolve_color(brw, src_mt, src_level, src_slice, 1,
> > > 0);
> > > > > intel_miptree_resolve_color(brw, dst_mt, dst_level, dst_slice, 1,
> > > 0);
> > > > > +   intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level,
> > > dst_slice);

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


Re: [Mesa-dev] [PATCH 14/30] intel/isl: Add an enum for describing auxiliary compression state

2017-06-02 Thread Nanley Chery
On Fri, May 26, 2017 at 04:30:18PM -0700, Jason Ekstrand wrote:
> This enum describes all of the states that a auxiliary compressed
 ^
 an
> surface can have.  All of the states as well as normative language for
> referring to each of the compression operations is provided in the
> truly colossal comment for the new isl_aux_state enum.  There is also
> a diagram showing how surfaces move between the different states.
> ---
>  src/intel/isl/isl.h | 142 
> 
>  1 file changed, 142 insertions(+)
> 
> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> index b9d8fa8..df6d3e3 100644
> --- a/src/intel/isl/isl.h
> +++ b/src/intel/isl/isl.h
> @@ -560,6 +560,148 @@ enum isl_aux_usage {
> ISL_AUX_USAGE_CCS_E,
>  };
>  
> +/**
> + * Enum for keeping track of the state an auxiliary compressed surface.
> + *
> + * For any given auxiliary surface compression format (HiZ, CCS, or MCS), any
> + * given slice (lod + array layer) can be in one of the six states described
> + * by this enum.  Draw and resolve operations may cause the slice to change
> + * from one state to another.  The six valid states are:
> + *
> + *1) Clear:  In this state, each block in the auxiliary surface contains 
> a
> + *   magic value that indicates that the block is in the clear state.  If
> + *   a block is in the clear state, it's values in the primary surface 
> are
> + *   ignored and the color of the samples in the block is taken either 
> the
> + *   RENDER_SURFACE_STATE packet for color or 3DSTATE_CLEAR_PARAMS for
> + *   depth.  Since neither the primary surface nor the auxiliary surface
> + *   contains the clear value, the surface can be cleared to a different
> + *   color by simply changing the clear color without modifying either
> + *   surface.
> + *
> + *2) Compressed w/ Clear:  In this state, neither the auxiliary surface
> + *   nor the primary surface has a complete representation of the data.
> + *   Instead, both surfaces must be used together or else rendering
> + *   corruption may occur.  Depending on the auxiliary compression format
> + *   and the data, any given block in the primary surface may contain 
> all,
> + *   some, or none of the data required to reconstruct the actual sample
> + *   values.  Blocks may also be in the clear state (see Clear) and have
> + *   their value taken from outside the surface.
> + *
> + *3) Compressed w/o Clear:  This state is identical to the state above
> + *   except that no blocks are in the clear state.  In this state, all of
> + *   the data required to reconstruct the final sample values is 
> contained
> + *   in the auxiliary and primary surface and the clear value is not
> + *   considered.
> + *
> + *4) Resolved:  In this state, the primary surface contains 100% of the
> + *   data.  The auxiliary surface is also valid so the surface can be
> + *   validly used with or without aux enabled.  The auxiliary surface 
> may,
> + *   however, contain non-trivial data and any update to the primary
> + *   surface with aux disabled will cause the two to get out of sync.
> + *
> + *5) Pass-through:  In this state, the primary surface contains 100% of 
> the
> + *   data and every block in the auxiliary surface contains a magic value
> + *   which indicates that the auxiliary surface should be ignored and the
> + *   only the primary surface should be considered.  Updating the primary
> + *   surface without aux works fine and can be done repeatedly in this
> + *   mode.  Writing to a surface in pass-through mode with aux enabled 
> may
> + *   cause the auxiliary buffer to contain non-trivial data and no longer
> + *   be in the pass-through state.
> + *
> + *5) Aux Invalid:  In this state, the primary surface contains 100% of 
> the
 ^
 6
> + *   data and the auxiliary surface is completely bogus.  Any attempt to
> + *   use the auxiliary surface is liable to result in rendering
> + *   corruption.  The only thing that one can do to re-enable aux once
> + *   this state is reached is to use an ambiguate pass to transition into
> + *   the pass-through state.
> + *
> + * Drawing with or without aux enabled may implicitly cause the surface to
> + * transition between these states.  There are also four types of "resolve"
> + * operations which cause an explicit transition:
> + *

I'm not sure why Fast Clear is described as a "resolve" operation.  To
me, "resolve" operations are those which make data in one buffer match
the data in another buffer. Perhaps you can clarify this?

> + *1) Fast Clear:  This operation writes the magic "clear" value to the
> + *   auxiliary surface.  This operation will safely transition any slice
> + *   of a surface from 

Re: [Mesa-dev] [PATCH] tgsi/scan: fix scanning fragment shaders with PrimID and Position/Face

2017-06-02 Thread Brian Paul

On 06/01/2017 12:10 PM, Marek Olšák wrote:

From: Marek Olšák 

Not relevant to radeonsi, because Position/Face are system values
with radeonsi, while this codepath is for drivers where Position and
Face are ordinary inputs.
---
  src/gallium/auxiliary/tgsi/tgsi_scan.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c 
b/src/gallium/auxiliary/tgsi/tgsi_scan.c
index 847f4fc..018ca4b 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
@@ -550,27 +550,30 @@ scan_declaration(struct tgsi_shader_info *info,
case TGSI_FILE_INPUT:
   info->input_semantic_name[reg] = (ubyte) semName;
   info->input_semantic_index[reg] = (ubyte) semIndex;
   info->input_interpolate[reg] = (ubyte)fulldecl->Interp.Interpolate;
   info->input_interpolate_loc[reg] = (ubyte)fulldecl->Interp.Location;
   info->input_cylindrical_wrap[reg] = 
(ubyte)fulldecl->Interp.CylindricalWrap;

   /* Vertex shaders can have inputs with holes between them. */
   info->num_inputs = MAX2(info->num_inputs, reg + 1);

- if (semName == TGSI_SEMANTIC_PRIMID)
-info->uses_primid = TRUE;
- else if (procType == PIPE_SHADER_FRAGMENT) {
-if (semName == TGSI_SEMANTIC_POSITION)
-   info->reads_position = TRUE;
-else if (semName == TGSI_SEMANTIC_FACE)
-   info->uses_frontface = TRUE;
+ switch (semName) {
+ case TGSI_SEMANTIC_PRIMID:
+info->uses_primid = true;
+break;
+ case TGSI_SEMANTIC_POSITION:
+info->reads_position = true;
+break;
+ case TGSI_SEMANTIC_FACE:
+info->uses_frontface = true;
+break;
   }
   break;

case TGSI_FILE_SYSTEM_VALUE:
   index = fulldecl->Range.First;

   info->system_value_semantic_name[index] = semName;
   info->num_system_values = MAX2(info->num_system_values, index + 1);

   switch (semName) {



Reviewed-by: Brian Paul 

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


Re: [Mesa-dev] [PATCH 1/3] mesa: only emit _NEW_MULTISAMPLE when coverage parameters change

2017-06-02 Thread Brian Paul

On 06/02/2017 09:52 AM, Samuel Pitoiset wrote:

We usually check that given parameters are different before
updating the state.

Signed-off-by: Samuel Pitoiset 
---
  src/mesa/main/multisample.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
index 5453e38632..f0e7a61180 100644
--- a/src/mesa/main/multisample.c
+++ b/src/mesa/main/multisample.c
@@ -41,11 +41,15 @@ _mesa_SampleCoverage(GLclampf value, GLboolean invert)
  {
 GET_CURRENT_CONTEXT(ctx);

-   FLUSH_VERTICES(ctx, 0);
+   value = CLAMP(value, 0.0f, 1.0f);
+
+   if (ctx->Multisample.SampleCoverageInvert == invert &&
+   ctx->Multisample.SampleCoverageValue == value)
+  return;

-   ctx->Multisample.SampleCoverageValue = CLAMP(value, 0.0f, 1.0f);
+   FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
+   ctx->Multisample.SampleCoverageValue = value;
 ctx->Multisample.SampleCoverageInvert = invert;
-   ctx->NewState |= _NEW_MULTISAMPLE;
  }





In this patch and 3, you could leave the CLAMP() where it was.

-Brian

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


Re: [Mesa-dev] [PATCH 20/24] i965/cnl: Don't resolve single sampled color rb in case of sRGB formats

2017-06-02 Thread Jason Ekstrand
On Tue, May 16, 2017 at 3:41 PM, Anuj Phogat  wrote:

> On Fri, May 12, 2017 at 4:38 PM, Anuj Phogat 
> wrote:
> > As sRGB now supports lossless compression, don't we also need to stop
> > resolving single sampled color render buffers for sRGB formats in Gen 10.
> >
> s/don't we/we. Fixed locally.
>
> > Signed-off-by: Anuj Phogat 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> > index 1247d03..9e19617 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -326,7 +326,7 @@ intel_update_state(struct gl_context * ctx, GLuint
> new_state)
> >  * enabled because otherwise the surface state will be programmed
> with the
> >  * linear equivalent format anyway.
> >  */
> > -   if (brw->gen >= 9 && ctx->Color.sRGBEnabled) {
> > +   if (brw->gen == 9 && ctx->Color.sRGBEnabled) {
>

I think I'd mildly prefer that we make this based on
isl_format_supports_ccs_e at some point but that doesn't have to happen
today.

Reviewed-by: Jason Ekstrand 


> >struct gl_framebuffer *fb = ctx->DrawBuffer;
> >for (int i = 0; i < fb->_NumColorDrawBuffers; i++) {
> >   struct gl_renderbuffer *rb = fb->_ColorDrawBuffers[i];
> > --
> > 2.9.3
> >
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 06/12] i965/cnl: Modify thread count shift for VS

2017-06-02 Thread Jason Ekstrand
I think this patch can be dropped with Rafiel's genxml work.

On Fri, Apr 14, 2017 at 5:35 PM, Anuj Phogat  wrote:

> From: Ben Widawsky 
>
> Signed-off-by: Ben Widawsky 
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h   | 1 +
>  src/mesa/drivers/dri/i965/gen8_vs_state.c | 6 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 08106c0..688ff61 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -607,6 +607,7 @@ enum brw_wrap_mode {
>  /* DW5 */
>  # define GEN6_VS_MAX_THREADS_SHIFT 25
>  # define HSW_VS_MAX_THREADS_SHIFT  23
> +# define GEN10_VS_MAX_THREADS_SHIFT 22
>  # define GEN6_VS_STATISTICS_ENABLE (1 << 10)
>  # define GEN6_VS_CACHE_DISABLE (1 << 1)
>  # define GEN6_VS_ENABLE(1 << 0)
> diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c
> b/src/mesa/drivers/dri/i965/gen8_vs_state.c
> index 7b66da4..c4ad9cd 100644
> --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c
> @@ -75,7 +75,11 @@ upload_vs_state(struct brw_context *brw)
> uint32_t simd8_enable =
>vue_prog_data->dispatch_mode == DISPATCH_MODE_SIMD8 ?
>GEN8_VS_SIMD8_ENABLE : 0;
> -   OUT_BATCH(((devinfo->max_vs_threads - 1) << HSW_VS_MAX_THREADS_SHIFT)
> |
> +
> +   uint32_t threads = (devinfo->max_vs_threads - 1);
> +   threads <<= brw->gen >= 10 ? GEN10_VS_MAX_THREADS_SHIFT :
> +HSW_VS_MAX_THREADS_SHIFT;
> +   OUT_BATCH(threads |
>   GEN6_VS_STATISTICS_ENABLE |
>   simd8_enable |
>   GEN6_VS_ENABLE);
> --
> 2.9.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 101071] compiling glsl fails with undefined reference to `pthread_create'

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101071

--- Comment #13 from Emil Velikov  ---
Created attachment 131682
  --> https://bugs.freedesktop.org/attachment.cgi?id=131682=edit
configure.ac: add -pthread to PTHREAD_LIBS

Right, so util_queue_init() pulls util_queue_init/pthread_create.
Still not sure why things behave on your end -> -lpthread is in the link chain,
so GCC should be smart enough to pick it.

Regardless, the attached patch is a step in the right direction and should
address things on your end.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100430] [radv] graphical glitches on dolphin emulator

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100430

--- Comment #9 from jdr...@gmail.com ---
Created attachment 131681
  --> https://bugs.freedesktop.org/attachment.cgi?id=131681=edit
opengl renderer: water is ok

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100430] [radv] graphical glitches on dolphin emulator

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100430

--- Comment #8 from jdr...@gmail.com ---
Created attachment 131680
  --> https://bugs.freedesktop.org/attachment.cgi?id=131680=edit
vulkan renderer: water is missing

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100430] [radv] graphical glitches on dolphin emulator

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100430

--- Comment #7 from jdr...@gmail.com ---
Created attachment 131679
  --> https://bugs.freedesktop.org/attachment.cgi?id=131679=edit
fifo log file (missing water)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100430] [radv] graphical glitches on dolphin emulator

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100430

--- Comment #6 from jdr...@gmail.com ---
Back to mesa stable 17.1, arch x64, fury r9, I still have some problems with
vulkan. The game tested is "Wii Sport Resort".
I've recorded a fifolog
If I select "Vulkan Renderer" in dolphin and if I launch it with
dolphin-emu -b -e 2017-06-02-MissingWater-01.dff
The water is missing.
If I select "OpenGL Renderer" in dolphin and if I launch it with
dolphin-emu -b -e 2017-06-02-MissingWater-01.dff
The water is ok (see screenshots)
If I select "Software Renderer", the water is ok.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100613] Regression in Mesa 17 on s390x (zSystems)

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100613

--- Comment #40 from Ben Crocker  ---
(In reply to Stefan Dirsch from comment #39)
> (In reply to Ben Crocker from comment #36)
> > So far, at least on this bug (or cluster of bugs), we have seen the
> > same behavior on both S390 and big-endian Power8 (PPC64).
> 
> Power8 is ppc64le, isn't it? Confused ...

Power8 can operate in either little-endian (PPC64LE) or big-endian
(PPC64) mode.  RHEL, Fedora, etc. support both.

I've been focusing on Power8 because:

1) it's what I'm more familiar with;
2) we mostly see the same problems on PPC64(BE) and S390.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 1/2] Android: amdgpu_common is only required when building OpenCL

2017-06-02 Thread Jan Vesely
v2: split off Android changes

Signed-off-by: Jan Vesely 
---
 src/gallium/drivers/r600/Android.mk | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/gallium/drivers/r600/Android.mk 
b/src/gallium/drivers/r600/Android.mk
index 87f433d..18c5bb6 100644
--- a/src/gallium/drivers/r600/Android.mk
+++ b/src/gallium/drivers/r600/Android.mk
@@ -30,11 +30,7 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := $(C_SOURCES) $(CXX_SOURCES)
 
-ifeq ($(MESA_ENABLE_LLVM),true)
-LOCAL_STATIC_LIBRARIES := libmesa_amd_common
-else
 LOCAL_C_INCLUDES += $(MESA_TOP)/src/amd/common
-endif
 
 LOCAL_SHARED_LIBRARIES := libdrm_radeon
 LOCAL_MODULE := libmesa_pipe_r600
@@ -45,7 +41,6 @@ include $(BUILD_STATIC_LIBRARY)
 ifneq ($(HAVE_GALLIUM_R600),)
 $(eval GALLIUM_LIBS += \
$(LOCAL_MODULE) \
-   $(LOCAL_STATIC_LIBRARIES) \
libmesa_winsys_radeon)
 $(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
 endif
-- 
2.9.4

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


[Mesa-dev] [PATCH v2 2/2] configure, r600: amd_common is needed only for OpenCL

2017-06-02 Thread Jan Vesely
v2: split off Android changes
add comments

Signed-off-by: Jan Vesely 
Tested-By: Aaron Watry 
---

Aaron,

the diff is mostly the same as v1 (modulo comments),
hope you don't mind reusing your tested by.

Jan

 configure.ac| 4 +++-
 src/gallium/drivers/r600/Automake.inc   | 3 ++-
 src/gallium/targets/pipe-loader/Makefile.am | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5caf316..f1de421 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2631,7 +2631,9 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test x$HAVE_SWRAST_DRI = 
xyes)
 AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
 AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
 
-AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_R600" = xyes -o \
+# FIXME: r600g still depends and amd_common (ac_binary*) when building OpenCL
+AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a \
+  "x$enable_opencl" = xyes \) -o \
   "x$HAVE_GALLIUM_RADEONSI" = xyes -o \
   "x$HAVE_RADEON_VULKAN" = xyes)
 
diff --git a/src/gallium/drivers/r600/Automake.inc 
b/src/gallium/drivers/r600/Automake.inc
index fa45735..642d527 100644
--- a/src/gallium/drivers/r600/Automake.inc
+++ b/src/gallium/drivers/r600/Automake.inc
@@ -13,7 +13,8 @@ TARGET_RADEON_WINSYS = \
 TARGET_RADEON_COMMON = \
$(top_builddir)/src/gallium/drivers/radeon/libradeon.la
 
-if HAVE_GALLIUM_LLVM
+# TODO: drop this dependency. libamd_common requires libdrm_amdgpu.
+if HAVE_AMD_DRIVERS
 TARGET_RADEON_COMMON += \
$(top_builddir)/src/amd/common/libamd_common.la
 endif
diff --git a/src/gallium/targets/pipe-loader/Makefile.am 
b/src/gallium/targets/pipe-loader/Makefile.am
index 5f629a2..b1ef07e 100644
--- a/src/gallium/targets/pipe-loader/Makefile.am
+++ b/src/gallium/targets/pipe-loader/Makefile.am
@@ -131,7 +131,8 @@ pipe_r600_la_LIBADD = \
$(LIBDRM_LIBS) \
$(RADEON_LIBS)
 
-if HAVE_GALLIUM_LLVM
+# TODO: drop this dependency. libamd_common requires libdrm_amdgpu.
+if HAVE_AMD_DRIVERS
 pipe_r600_la_LIBADD += \
$(top_builddir)/src/amd/common/libamd_common.la
 endif
-- 
2.9.4

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


Re: [Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

2017-06-02 Thread Jan Vesely
On Fri, 2017-06-02 at 17:22 +0100, Emil Velikov wrote:
> On 2 June 2017 at 16:34, Jan Vesely  wrote:
> > On Fri, 2017-06-02 at 12:19 +0100, Emil Velikov wrote:
> > > On 1 June 2017 at 21:28, Jan Vesely  wrote:
> > > > Signed-off-by: Jan Vesely 
> > > > ---
> > > > Hi guys,
> > > > 
> > > > this is the first step towards dropping libamd_common dependency.
> > > > It's based on Emil's patches 3/5 and 4/5.
> > > > Enabling opencl still falls back to the old way of requiring 
> > > > libamd_common.
> > > > I'll try to address that in the next step (no time estimate, feel
> > > > free to beat me to it). I think we can drop part of those functions
> > > > rather than just copying them.
> > > > 
> > > 
> > > AFAICT one still need the rest of my series, correct?
> > 
> > kind of, 1/5 is mostly unrelated. 2,3/5 should be replaced by this one.
> > 4/5 is needed,
> > and 5/5 seems to be not applicable since ac_gpu_info.c
> > still needs the header.
> > 
> 
> Hmm indeed. I seems to have misread your patch.
> Which makes me wonder if you've tested the patch as I mentioned earlier:
>  - (re)move amdgpu.h
>  - apply mesa patches (be that any of my, your and other mix)
>  - build r300 and/or r600, w/o radeonsi
>  - the the above combo a try with and w/o opencl
> 
> I'm leaning that things will fail to build?

This patch only removes dependence on libamd_common unless you enable
OpenCL. I haven't addressed the hacky way of using ac_gpu_info.h
Applying 4/5v2 from your series should work OK on top of my patch.

Jan

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


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

2017-06-02 Thread Emil Velikov
On 2 June 2017 at 16:34, Jan Vesely  wrote:
> On Fri, 2017-06-02 at 12:19 +0100, Emil Velikov wrote:
>> On 1 June 2017 at 21:28, Jan Vesely  wrote:
>> > Signed-off-by: Jan Vesely 
>> > ---
>> > Hi guys,
>> >
>> > this is the first step towards dropping libamd_common dependency.
>> > It's based on Emil's patches 3/5 and 4/5.
>> > Enabling opencl still falls back to the old way of requiring libamd_common.
>> > I'll try to address that in the next step (no time estimate, feel
>> > free to beat me to it). I think we can drop part of those functions
>> > rather than just copying them.
>> >
>>
>> AFAICT one still need the rest of my series, correct?
>
> kind of, 1/5 is mostly unrelated. 2,3/5 should be replaced by this one.
> 4/5 is needed,

> and 5/5 seems to be not applicable since ac_gpu_info.c
> still needs the header.
>
Hmm indeed. I seems to have misread your patch.
Which makes me wonder if you've tested the patch as I mentioned earlier:
 - (re)move amdgpu.h
 - apply mesa patches (be that any of my, your and other mix)
 - build r300 and/or r600, w/o radeonsi
 - the the above combo a try with and w/o opencl

I'm leaning that things will fail to build?

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


[Mesa-dev] [Bug 101071] compiling glsl fails with undefined reference to `pthread_create'

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101071

--- Comment #12 from war...@o2.pl ---
Emil,
Due 762a6333f21fd8606f69db6060027c4522d46678 I can't use git auto bisect and
need to do it manually with this commit reverted.

Here are results:

2e98db6... util/vk: Add helpers for finding an extension struct - good
ed28ae7... radv: Increase api version to 1.0.42 - good
407fa77... radv: Set driver version to mesa version - good
3ece76f... radv/ac: gather4 cube workaround integer - good
7372e3c... radv/ac: workaround regression in llvm 4.0 release - good
e2c4435... util/disk_cache: add thread queue to disk cache - bad

It looks like
https://cgit.freedesktop.org/mesa/mesa/commit/?id=e2c4435b078a1471b044219552873a54b1817bac
is root cause.

If You need - I'm ready to test patches to solve OP issue :-)

br

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/3] mesa: only emit _NEW_MULTISAMPLE when sample mask changes

2017-06-02 Thread Samuel Pitoiset
We usually check that given parameters are different before
updating the state.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/multisample.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
index f0e7a61180..16fe2b7ced 100644
--- a/src/mesa/main/multisample.c
+++ b/src/mesa/main/multisample.c
@@ -119,6 +119,9 @@ _mesa_SampleMaski(GLuint index, GLbitfield mask)
   return;
}
 
+   if (ctx->Multisample.SampleMaskValue == mask)
+  return;
+
FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
ctx->Multisample.SampleMaskValue = mask;
 }
-- 
2.13.0

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


[Mesa-dev] [PATCH 3/3] mesa: only emit _NEW_MULTISAMPLE when min sample shading changes

2017-06-02 Thread Samuel Pitoiset
We usually check that given parameters are different before
updating the state.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/multisample.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
index 16fe2b7ced..07786130d5 100644
--- a/src/mesa/main/multisample.c
+++ b/src/mesa/main/multisample.c
@@ -140,10 +140,13 @@ _mesa_MinSampleShading(GLclampf value)
   return;
}
 
-   FLUSH_VERTICES(ctx, 0);
+   value = CLAMP(value, 0.0f, 1.0f);
+
+   if (ctx->Multisample.MinSampleShadingValue == value)
+  return;
 
-   ctx->Multisample.MinSampleShadingValue = CLAMP(value, 0.0f, 1.0f);
-   ctx->NewState |= _NEW_MULTISAMPLE;
+   FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
+   ctx->Multisample.MinSampleShadingValue = value;
 }
 
 /**
-- 
2.13.0

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


[Mesa-dev] [PATCH 1/3] mesa: only emit _NEW_MULTISAMPLE when coverage parameters change

2017-06-02 Thread Samuel Pitoiset
We usually check that given parameters are different before
updating the state.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/multisample.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
index 5453e38632..f0e7a61180 100644
--- a/src/mesa/main/multisample.c
+++ b/src/mesa/main/multisample.c
@@ -41,11 +41,15 @@ _mesa_SampleCoverage(GLclampf value, GLboolean invert)
 {
GET_CURRENT_CONTEXT(ctx);
 
-   FLUSH_VERTICES(ctx, 0);
+   value = CLAMP(value, 0.0f, 1.0f);
+
+   if (ctx->Multisample.SampleCoverageInvert == invert &&
+   ctx->Multisample.SampleCoverageValue == value)
+  return;
 
-   ctx->Multisample.SampleCoverageValue = CLAMP(value, 0.0f, 1.0f);
+   FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
+   ctx->Multisample.SampleCoverageValue = value;
ctx->Multisample.SampleCoverageInvert = invert;
-   ctx->NewState |= _NEW_MULTISAMPLE;
 }
 
 
-- 
2.13.0

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


Re: [Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

2017-06-02 Thread Jan Vesely
On Thu, 2017-06-01 at 16:22 -0500, Aaron Watry wrote:
> On Thu, Jun 1, 2017 at 3:28 PM, Jan Vesely  wrote:
> 
> > Signed-off-by: Jan Vesely 
> > ---
> > Hi guys,
> > 
> > this is the first step towards dropping libamd_common dependency.
> > It's based on Emil's patches 3/5 and 4/5.
> > Enabling opencl still falls back to the old way of requiring libamd_common.
> > I'll try to address that in the next step (no time estimate, feel free to
> > beat me to it). I think we can drop part of those functions rather than
> > just copying them.
> > 
> 
> I did a build test of r600 on my laptop with an old libdrm before/after
> this patch with/without opencl enabled, and got:
> before, no CL: Build Failure
> after, no CL: Build succeeded
> before, with CL:  Build Failure
> after, with CL: Build Failure
> 
> I then went and updated libdrm/amdgpu, and the with/without CL build for
> r600 works again there.  I then built this on my r600 box and
> double-checked that clinfo and piglit's cl-program-tester ran and could
> execute a simple kernel (tests/cl/program/execute/builtin/geometric/
> length.cl), so I'd say that this is:
> Tested-By: Aaron Watry 

thanks. I also tested building r600g using --disable-llvm which builds
OK (not sure if that should be allowed since r300g config fails without
llvm)

> 
> And yes, I agree that we should remove the amdgpu dependency from
> libamd_common (or at least the parts that r600 depends on), but I'd be ok
> with doing that in a follow-up... That, or we also extend vinson's patch
> [0] and wrap it in a check for if opencl is enabled so that we get a
> configure-time failure if R600 is being built with CL and amdgpu's drm
> headers are missing/old.

I think that's a question for AMD devs. my current understanding is
that amdgpu_common is more like
amdgcn_shared_between_radv_and_radeonsi, yet both r300 and r600 use at
least the gpu_info header. I've found only two functions in ac_binary
that are needed by r600 clover, so I plan to copy them. having
common_amdgpu_common to be used by r300/r600/radeonsi/radv would be
nicer, but I'm not sure how much code would fall into that category.

Jan

> 
> --Aaron
> 
> [0] https://lists.freedesktop.org/archives/mesa-dev/2017-May/157327.html
> 
> 
> > 
> > Emil, I didn't find anything in android build that enabled opencl, so I
> > dropped it entirely.
> > 
> > regards,
> > Jan
> > 
> >  configure.ac| 3 ++-
> >  src/gallium/drivers/r600/Android.mk | 5 -
> >  src/gallium/drivers/r600/Automake.inc   | 2 +-
> >  src/gallium/targets/pipe-loader/Makefile.am | 2 +-
> >  4 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 5caf316..e0996a0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2631,7 +2631,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test
> > x$HAVE_SWRAST_DRI = xyes)
> >  AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
> >  AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
> > 
> > -AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_R600" = xyes -o \
> > +AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a
> > \
> > +  "x$enable_opencl" = xyes \) -o \
> >"x$HAVE_GALLIUM_RADEONSI" = xyes -o
> > \
> >"x$HAVE_RADEON_VULKAN" = xyes)
> > 
> > diff --git a/src/gallium/drivers/r600/Android.mk
> > b/src/gallium/drivers/r600/Android.mk
> > index 87f433d..18c5bb6 100644
> > --- a/src/gallium/drivers/r600/Android.mk
> > +++ b/src/gallium/drivers/r600/Android.mk
> > @@ -30,11 +30,7 @@ include $(CLEAR_VARS)
> > 
> >  LOCAL_SRC_FILES := $(C_SOURCES) $(CXX_SOURCES)
> > 
> > -ifeq ($(MESA_ENABLE_LLVM),true)
> > -LOCAL_STATIC_LIBRARIES := libmesa_amd_common
> > -else
> >  LOCAL_C_INCLUDES += $(MESA_TOP)/src/amd/common
> > -endif
> > 
> >  LOCAL_SHARED_LIBRARIES := libdrm_radeon
> >  LOCAL_MODULE := libmesa_pipe_r600
> > @@ -45,7 +41,6 @@ include $(BUILD_STATIC_LIBRARY)
> >  ifneq ($(HAVE_GALLIUM_R600),)
> >  $(eval GALLIUM_LIBS += \
> > $(LOCAL_MODULE) \
> > -   $(LOCAL_STATIC_LIBRARIES) \
> > libmesa_winsys_radeon)
> >  $(eval GALLIUM_SHARED_LIBS += $(LOCAL_SHARED_LIBRARIES))
> >  endif
> > diff --git a/src/gallium/drivers/r600/Automake.inc
> > b/src/gallium/drivers/r600/Automake.inc
> > index fa45735..c96fe74 100644
> > --- a/src/gallium/drivers/r600/Automake.inc
> > +++ b/src/gallium/drivers/r600/Automake.inc
> > @@ -13,7 +13,7 @@ TARGET_RADEON_WINSYS = \
> >  TARGET_RADEON_COMMON = \
> > $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
> > 
> > -if HAVE_GALLIUM_LLVM
> > +if HAVE_AMD_DRIVERS
> >  TARGET_RADEON_COMMON += \
> > $(top_builddir)/src/amd/common/libamd_common.la
> >  endif
> > diff --git a/src/gallium/targets/pipe-loader/Makefile.am
> > 

Re: [Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

2017-06-02 Thread Jan Vesely
On Fri, 2017-06-02 at 12:19 +0100, Emil Velikov wrote:
> On 1 June 2017 at 21:28, Jan Vesely  wrote:
> > Signed-off-by: Jan Vesely 
> > ---
> > Hi guys,
> > 
> > this is the first step towards dropping libamd_common dependency.
> > It's based on Emil's patches 3/5 and 4/5.
> > Enabling opencl still falls back to the old way of requiring libamd_common.
> > I'll try to address that in the next step (no time estimate, feel
> > free to beat me to it). I think we can drop part of those functions
> > rather than just copying them.
> > 
> 
> AFAICT one still need the rest of my series, correct?

kind of, 1/5 is mostly unrelated. 2,3/5 should be replaced by this one.
4/5 is needed, and 5/5 seems to be not applicable since ac_gpu_info.c
still needs the header.

> 
> > Emil, I didn't find anything in android build that enabled opencl,
> > so I dropped it entirely.
> > 
> 
> Good point. Please flesh that in a separate patch though IIRC
> there might be a bit more related cleanups that need squashing.
> 
> > regards,
> > Jan
> > 
> >  configure.ac| 3 ++-
> >  src/gallium/drivers/r600/Android.mk | 5 -
> >  src/gallium/drivers/r600/Automake.inc   | 2 +-
> >  src/gallium/targets/pipe-loader/Makefile.am | 2 +-
> >  4 files changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 5caf316..e0996a0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2631,7 +2631,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test 
> > x$HAVE_SWRAST_DRI = xyes)
> >  AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
> >  AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
> > 
> > -AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_R600" = xyes -o \
> > +AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a \
> > +  "x$enable_opencl" = xyes \) -o \
> 
> Please add a beefy XXX/TODO/FIXME comment and reference the
> s/HAVE_GALLIUM_LLVM/HAVE_AMD_DRIVERS/ changes.
> 
> -Emil

-- 
Jan Vesely 

signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glcpp: simplify the check for hexadecimal number

2017-06-02 Thread Emil Velikov
On 2 June 2017 at 14:56, Vlad Golovkin  wrote:
> -- Forwarded message --
> From: Vlad Golovkin 
> Date: 2017-06-02 16:55 GMT+03:00
> Subject: Re: [Mesa-dev] [PATCH] glcpp: simplify the check for hexadecimal 
> number
> To: Emil Velikov 
>
>
> 2017-06-02 16:07 GMT+03:00 Emil Velikov :
>> Hi Vlad,
>>
>> Welcome to Mesa.
>>
>> Have you don't any benchmarking on this patch via something like shader-db 
>> [1]?
>> Mentioning the results in the commit message is encouraged.
>
> Hi Emil,
>
> If I understand correctly shader-db supports only i965 and radeonsi,
> or am I missing something?
> I would be glad to be proven wrong, because I only have old r600 gpu.
>
In-tree you have helper scripts for i965, nouveau, radeonsi and
freedreno, but one can run shader-db on any driver.

>>
>> On 2 June 2017 at 13:37, Vlad Golovkin  wrote:
>>> This will make it possible to remove unnecessary strlen and strncmp
>>> calls.
>>> ---
>>>  src/compiler/glsl/glcpp/glcpp-parse.y | 10 ++
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
>>> b/src/compiler/glsl/glcpp/glcpp-parse.y
>>> index fe211a0f0b..8cdd4c9d5b 100644
>>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
>>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
>>> @@ -450,10 +450,12 @@ control_line_error:
>>>
>>>  integer_constant:
>>> INTEGER_STRING {
>>> -   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
>>> -   $$ = strtoll ($1 + 2, NULL, 16);
>>> -   } else if ($1[0] == '0') {
>>> -   $$ = strtoll ($1, NULL, 8);
>>> +   if ($1[0] == '0') {
>>> +   if ($1[1] == 'x' && $1[2] != '\0') {
>>> +   $$ = strtoll ($1 + 2, NULL, 16);
>>> +   } else {
>>> +   $$ = strtoll ($1, NULL, 8);
>>> +   }
>>> } else {
>>> $$ = strtoll ($1, NULL, 10);
>>> }
>> Food for thought:
>>
>> Any idea why we'd want to handle any of these details ourselves?
>> strtoll(..., /*base*/ 0) is smart enough, plus it should be better optimised.
> This is a good approach, it will allow to write just one simple
> function call instead of all these checks.
> For some reason i thought that calling strtoll(str, NULL, 0) will make
> it autodetect the base - from 2 to 36, when in reality it basically
> tries to match bases 8, 10 and 16.
>>
>> Another possibility is to swap the INTEGER_STRING token for
>> {DEC,OCT,HEX}... ones each one calling strtoll() with correct base.
> Advantage of this approach, however, is that in this case one can just
> use custom string to num functions that avoid the overhead of strtoll,
> but then again it is better to benchmark before drawing any
> conclusions.
Indeed. Apart from shader-db, shader intensive games may also help -
Deus Ex perhaps?

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


[Mesa-dev] [Bug 100988] glXGetCurrentDisplay() no longer works for FakeGLX contexts?

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100988

Brian Paul  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED

--- Comment #4 from Brian Paul  ---
Pushed as commit c6ba85a8c0f02b3b7058dae7afb6c49f56567319

-- 
You are receiving this mail because:
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Fwd: [PATCH] glcpp: simplify the check for hexadecimal number

2017-06-02 Thread Vlad Golovkin
-- Forwarded message --
From: Vlad Golovkin 
Date: 2017-06-02 16:55 GMT+03:00
Subject: Re: [Mesa-dev] [PATCH] glcpp: simplify the check for hexadecimal number
To: Emil Velikov 


2017-06-02 16:07 GMT+03:00 Emil Velikov :
> Hi Vlad,
>
> Welcome to Mesa.
>
> Have you don't any benchmarking on this patch via something like shader-db 
> [1]?
> Mentioning the results in the commit message is encouraged.

Hi Emil,

If I understand correctly shader-db supports only i965 and radeonsi,
or am I missing something?
I would be glad to be proven wrong, because I only have old r600 gpu.

>
> On 2 June 2017 at 13:37, Vlad Golovkin  wrote:
>> This will make it possible to remove unnecessary strlen and strncmp
>> calls.
>> ---
>>  src/compiler/glsl/glcpp/glcpp-parse.y | 10 ++
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
>> b/src/compiler/glsl/glcpp/glcpp-parse.y
>> index fe211a0f0b..8cdd4c9d5b 100644
>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
>> @@ -450,10 +450,12 @@ control_line_error:
>>
>>  integer_constant:
>> INTEGER_STRING {
>> -   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
>> -   $$ = strtoll ($1 + 2, NULL, 16);
>> -   } else if ($1[0] == '0') {
>> -   $$ = strtoll ($1, NULL, 8);
>> +   if ($1[0] == '0') {
>> +   if ($1[1] == 'x' && $1[2] != '\0') {
>> +   $$ = strtoll ($1 + 2, NULL, 16);
>> +   } else {
>> +   $$ = strtoll ($1, NULL, 8);
>> +   }
>> } else {
>> $$ = strtoll ($1, NULL, 10);
>> }
> Food for thought:
>
> Any idea why we'd want to handle any of these details ourselves?
> strtoll(..., /*base*/ 0) is smart enough, plus it should be better optimised.
This is a good approach, it will allow to write just one simple
function call instead of all these checks.
For some reason i thought that calling strtoll(str, NULL, 0) will make
it autodetect the base - from 2 to 36, when in reality it basically
tries to match bases 8, 10 and 16.
>
> Another possibility is to swap the INTEGER_STRING token for
> {DEC,OCT,HEX}... ones each one calling strtoll() with correct base.
Advantage of this approach, however, is that in this case one can just
use custom string to num functions that avoid the overhead of strtoll,
but then again it is better to benchmark before drawing any
conclusions.
>
> -Emil
>
> [1] https://cgit.freedesktop.org/mesa/shader-db/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glcpp: simplify the check for hexadecimal number

2017-06-02 Thread Emil Velikov
Hi Vlad,

Welcome to Mesa.

Have you don't any benchmarking on this patch via something like shader-db [1]?
Mentioning the results in the commit message is encouraged.

On 2 June 2017 at 13:37, Vlad Golovkin  wrote:
> This will make it possible to remove unnecessary strlen and strncmp
> calls.
> ---
>  src/compiler/glsl/glcpp/glcpp-parse.y | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
> b/src/compiler/glsl/glcpp/glcpp-parse.y
> index fe211a0f0b..8cdd4c9d5b 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -450,10 +450,12 @@ control_line_error:
>
>  integer_constant:
> INTEGER_STRING {
> -   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
> -   $$ = strtoll ($1 + 2, NULL, 16);
> -   } else if ($1[0] == '0') {
> -   $$ = strtoll ($1, NULL, 8);
> +   if ($1[0] == '0') {
> +   if ($1[1] == 'x' && $1[2] != '\0') {
> +   $$ = strtoll ($1 + 2, NULL, 16);
> +   } else {
> +   $$ = strtoll ($1, NULL, 8);
> +   }
> } else {
> $$ = strtoll ($1, NULL, 10);
> }
Food for thought:

Any idea why we'd want to handle any of these details ourselves?
strtoll(..., /*base*/ 0) is smart enough, plus it should be better optimised.

Another possibility is to swap the INTEGER_STRING token for
{DEC,OCT,HEX}... ones each one calling strtoll() with correct base.

-Emil

[1] https://cgit.freedesktop.org/mesa/shader-db/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/4] radeonsi: fix gl_PrimitiveID in tessellation with instanced draws on SI

2017-06-02 Thread Marek Olšák
On Wed, May 3, 2017 at 3:58 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/gallium/drivers/radeonsi/si_state_draw.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c 
> b/src/gallium/drivers/radeonsi/si_state_draw.c
> index e6a9ee0..3d1d1f8 100644
> --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> @@ -181,20 +181,34 @@ static void si_emit_derived_tess_state(struct 
> si_context *sctx,
>
> /* Not necessary for correctness, but improves performance. The
>  * specific value is taken from the proprietary driver.
>  */
> *num_patches = MIN2(*num_patches, 40);
>
> /* SI bug workaround - limit LS-HS threadgroups to only one wave. */
> if (sctx->b.chip_class == SI) {
> unsigned one_wave = 64 / MAX2(num_tcs_input_cp, 
> num_tcs_output_cp);
> *num_patches = MIN2(*num_patches, one_wave);
> +
> +   if (sctx->screen->b.info.max_se == 1) {
> +   /* The VGT HS block increments the patch ID 
> unconditionally
> +* within a single threadgroup. This results in 
> incorrect
> +* patch IDs when instanced draws are used.
> +*
> +* The intended solution is to restrict threadgroups 
> to
> +* a single instance by setting SWITCH_ON_EOI, which
> +* should cause IA to split instances up. However, 
> this
> +* doesn't work correctly on SI when there is no other
> +* SE to switch to.
> +*/
> +   *num_patches = 1;
> +   }

Hi Nicolai,

This commit massively decreases tessellation performance on SI 1-SE
parts. We need a different solution. Would this work: "Set num_patches
to the greatest divisor of the the number of patches per instance."

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


Re: [Mesa-dev] [PATCH 1/3] mesa: remove call to Driver.Scissor() in _mesa_WindowRectanglesEXT()

2017-06-02 Thread Samuel Pitoiset



On 06/02/2017 02:44 PM, Ilia Mirkin wrote:

On Fri, Jun 2, 2017 at 8:40 AM, Samuel Pitoiset
 wrote:



On 06/02/2017 02:34 PM, Ilia Mirkin wrote:


As I mentioned to you on IRC... my understanding of the Driver.Scissor
callback is that it's a guarantee by the mesa core that whenever
Scissor.* changes, Driver.Scissor() gets called if it's there.

Just because nothing cares on the other side of the abstraction
barrier doesn't implicitly mean it should be removed.



Well, the window rectangle fields could be moved outside of the scissor
struct state actually.


They're part of the "scissor" attrib group (see the spec), which was
why I originally put it there. There doesn't *have* to be a 1:1
mapping, but life is easier when it is. (That's the main reason for
the existence of those structs in the first place.)


Right. Anyway, this patch can be discarded because it's not the most 
important one.




   -ilia


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


Re: [Mesa-dev] [PATCH 1/3] mesa: remove call to Driver.Scissor() in _mesa_WindowRectanglesEXT()

2017-06-02 Thread Ilia Mirkin
On Fri, Jun 2, 2017 at 8:40 AM, Samuel Pitoiset
 wrote:
>
>
> On 06/02/2017 02:34 PM, Ilia Mirkin wrote:
>>
>> As I mentioned to you on IRC... my understanding of the Driver.Scissor
>> callback is that it's a guarantee by the mesa core that whenever
>> Scissor.* changes, Driver.Scissor() gets called if it's there.
>>
>> Just because nothing cares on the other side of the abstraction
>> barrier doesn't implicitly mean it should be removed.
>
>
> Well, the window rectangle fields could be moved outside of the scissor
> struct state actually.

They're part of the "scissor" attrib group (see the spec), which was
why I originally put it there. There doesn't *have* to be a 1:1
mapping, but life is easier when it is. (That's the main reason for
the existence of those structs in the first place.)

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


Re: [Mesa-dev] [PATCH mesa 2/2] egl/display: make platform detection thread-safe

2017-06-02 Thread Emil Velikov
On 1 June 2017 at 15:52, Grazvydas Ignotas  wrote:
> On Thu, Jun 1, 2017 at 4:23 PM, Emil Velikov  wrote:
>> Hi guys,
>>
>> On 1 June 2017 at 12:56, Grazvydas Ignotas  wrote:
>>> On Thu, Jun 1, 2017 at 2:15 PM, Eric Engestrom
>>>  wrote:
 If the detections methods ever become able to return different results
 for different threads, the if chain might make threads go back and forth
 between invalid and valid platforms.
 Solve this by doing the detection in a local var and only overwriting
 the global one at the end, if no other thread has updated it since.

 This means the platform detected in the thread might not be the platform
 returned by the function, but this is a different issue that will need
 to be discussed when this becomes possible.

 Reported-by: Grazvydas Ignotas 
>>>
>>> Not really, see https://bugs.freedesktop.org/show_bug.cgi?id=101252
>>>
 Signed-off-by: Eric Engestrom 
 ---

 This is unnecessary in my opinion, but doesn't hurt :)
>>>
>>> It is necessary, without it things will work most of the time but
>>> there will be rare failures.
>>> Imagine there are 2 threads that both call _eglGetNativePlatform()
>>> simultaneously:
>>> - thread 1 completes the first "if (native_platform ==
>>> _EGL_INVALID_PLATFORM)" check and is preemted to do something else
>>> - thread 2 executes the whole function, does "native_platform =
>>> _EGL_NATIVE_PLATFORM" and just before returning it's preemted
>>> - thread 1 wakes up and calls _eglGetNativePlatformFromEnv() which
>>> returns _EGL_INVALID_PLATFORM because no env vars are set, updates
>>> native_platform and then gets preemted again
>>> - thread 2 wakes up and returns wrong _EGL_INVALID_PLATFORM
>>>
>> Afaict this/similar fix is necessary, yet let me put an alternative
>> idea - add locking via _eglGlobal.Mutex.
>> May be an bit of overkill, but it's what we consistently use
>> throughout the code base.
>
> It looks like that mutex is meant to protect _eglGlobal and not some
> random variables, so I'd prefer the atomic version.
>
>> Reverting the patch (as suggested by Grazvydas) does not fully address
>> the problem, but only makes it less likely to hit.
>
> Yeah I meant reverting instead of taking 1/2 and then applying this
> fix on top. OTOH I'm also ok with this series as-is.
>
Right I should have said "a mutex" as opposed to the _eglGlobal one.
Mostly because we have ~20 instances vs ~3 atomics.
In either way, this is more of a bikeshedding so as long as you guys
are happy, let's go with any solution.

In either way, please add the bugzilla reference.
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] mesa: remove call to Driver.Scissor() in _mesa_WindowRectanglesEXT()

2017-06-02 Thread Samuel Pitoiset



On 06/02/2017 02:34 PM, Ilia Mirkin wrote:

As I mentioned to you on IRC... my understanding of the Driver.Scissor
callback is that it's a guarantee by the mesa core that whenever
Scissor.* changes, Driver.Scissor() gets called if it's there.

Just because nothing cares on the other side of the abstraction
barrier doesn't implicitly mean it should be removed.


Well, the window rectangle fields could be moved outside of the scissor 
struct state actually.




On Fri, Jun 2, 2017 at 8:31 AM, Samuel Pitoiset
 wrote:

This is actually useless because this driver call is only used
by the classic DRI drivers which don't support that extension
and probably won't never support it.

Signed-off-by: Samuel Pitoiset 
---
  src/mesa/main/scissor.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
index 1c766f4c41..5f2f61f926 100644
--- a/src/mesa/main/scissor.c
+++ b/src/mesa/main/scissor.c
@@ -258,9 +258,6 @@ _mesa_WindowRectanglesEXT(GLenum mode, GLsizei count, const 
GLint *box)
sizeof(struct gl_scissor_rect) * count);
 ctx->Scissor.NumWindowRects = count;
 ctx->Scissor.WindowRectMode = mode;
-
-   if (ctx->Driver.Scissor)
-  ctx->Driver.Scissor(ctx);
  }


--
2.13.0

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

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


[Mesa-dev] [PATCH] glcpp: simplify the check for hexadecimal number

2017-06-02 Thread Vlad Golovkin
This will make it possible to remove unnecessary strlen and strncmp
calls.
---
 src/compiler/glsl/glcpp/glcpp-parse.y | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
b/src/compiler/glsl/glcpp/glcpp-parse.y
index fe211a0f0b..8cdd4c9d5b 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -450,10 +450,12 @@ control_line_error:
 
 integer_constant:
INTEGER_STRING {
-   if (strlen ($1) >= 3 && strncmp ($1, "0x", 2) == 0) {
-   $$ = strtoll ($1 + 2, NULL, 16);
-   } else if ($1[0] == '0') {
-   $$ = strtoll ($1, NULL, 8);
+   if ($1[0] == '0') {
+   if ($1[1] == 'x' && $1[2] != '\0') {
+   $$ = strtoll ($1 + 2, NULL, 16);
+   } else {
+   $$ = strtoll ($1, NULL, 8);
+   }
} else {
$$ = strtoll ($1, NULL, 10);
}
-- 
2.11.0

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


Re: [Mesa-dev] [PATCH 1/3] mesa: remove call to Driver.Scissor() in _mesa_WindowRectanglesEXT()

2017-06-02 Thread Ilia Mirkin
As I mentioned to you on IRC... my understanding of the Driver.Scissor
callback is that it's a guarantee by the mesa core that whenever
Scissor.* changes, Driver.Scissor() gets called if it's there.

Just because nothing cares on the other side of the abstraction
barrier doesn't implicitly mean it should be removed.

On Fri, Jun 2, 2017 at 8:31 AM, Samuel Pitoiset
 wrote:
> This is actually useless because this driver call is only used
> by the classic DRI drivers which don't support that extension
> and probably won't never support it.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/mesa/main/scissor.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
> index 1c766f4c41..5f2f61f926 100644
> --- a/src/mesa/main/scissor.c
> +++ b/src/mesa/main/scissor.c
> @@ -258,9 +258,6 @@ _mesa_WindowRectanglesEXT(GLenum mode, GLsizei count, 
> const GLint *box)
>sizeof(struct gl_scissor_rect) * count);
> ctx->Scissor.NumWindowRects = count;
> ctx->Scissor.WindowRectMode = mode;
> -
> -   if (ctx->Driver.Scissor)
> -  ctx->Driver.Scissor(ctx);
>  }
>
>
> --
> 2.13.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/3] mesa: add new gl_driver_flags::NewWindowRectangles

2017-06-02 Thread Samuel Pitoiset
This new driver flag will replace _NEW_SCISSOR which is
emitted when setting new window rectangles but it actually
triggers useless changes in the state tracker (like scissor
and rasterizer).

EXT_window_rectangles is currently only supported by Nouveau.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/mtypes.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 69ab584c03..663ba931d7 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4466,6 +4466,11 @@ struct gl_driver_flags
 * gl_context::IntelConservativeRasterization
 */
uint64_t NewIntelConservativeRasterization;
+
+   /**
+* gl_context::Scissor::WindowRects
+*/
+   uint64_t NewWindowRectangles;
 };
 
 struct gl_uniform_buffer_binding
-- 
2.13.0

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


[Mesa-dev] [PATCH 3/3] mesa: make use of NewWindowRectangles driver flags

2017-06-02 Thread Samuel Pitoiset
Now, st_update_window_rectangles() won't be called when the
scissor is going to be updated.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/scissor.c | 4 +++-
 src/mesa/state_tracker/st_context.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
index 5f2f61f926..5cf02168bd 100644
--- a/src/mesa/main/scissor.c
+++ b/src/mesa/main/scissor.c
@@ -253,7 +253,9 @@ _mesa_WindowRectanglesEXT(GLenum mode, GLsizei count, const 
GLint *box)
   box += 4;
}
 
-   FLUSH_VERTICES(ctx, _NEW_SCISSOR);
+   FLUSH_VERTICES(ctx, 0);
+   ctx->NewDriverState |= ctx->DriverFlags.NewWindowRectangles;
+
memcpy(ctx->Scissor.WindowRects, newval,
   sizeof(struct gl_scissor_rect) * count);
ctx->Scissor.NumWindowRects = count;
diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index 4dcc160b50..abdae31585 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -193,8 +193,7 @@ void st_invalidate_state(struct gl_context * ctx, 
GLbitfield new_state)
 
   if (new_state & _NEW_SCISSOR)
  st->dirty |= ST_NEW_RASTERIZER |
-  ST_NEW_SCISSOR |
-  ST_NEW_WINDOW_RECTANGLES;
+  ST_NEW_SCISSOR;
 
   if (new_state & _NEW_FOG)
  st->dirty |= ST_NEW_FS_STATE;
@@ -516,6 +515,7 @@ static void st_init_driver_flags(struct gl_driver_flags *f)
f->NewAtomicBuffer = ST_NEW_ATOMIC_BUFFER;
f->NewShaderStorageBuffer = ST_NEW_STORAGE_BUFFER;
f->NewImageUnits = ST_NEW_IMAGE_UNITS;
+   f->NewWindowRectangles = ST_NEW_WINDOW_RECTANGLES;
 }
 
 struct st_context *st_create_context(gl_api api, struct pipe_context *pipe,
-- 
2.13.0

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


[Mesa-dev] [PATCH 1/3] mesa: remove call to Driver.Scissor() in _mesa_WindowRectanglesEXT()

2017-06-02 Thread Samuel Pitoiset
This is actually useless because this driver call is only used
by the classic DRI drivers which don't support that extension
and probably won't never support it.

Signed-off-by: Samuel Pitoiset 
---
 src/mesa/main/scissor.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
index 1c766f4c41..5f2f61f926 100644
--- a/src/mesa/main/scissor.c
+++ b/src/mesa/main/scissor.c
@@ -258,9 +258,6 @@ _mesa_WindowRectanglesEXT(GLenum mode, GLsizei count, const 
GLint *box)
   sizeof(struct gl_scissor_rect) * count);
ctx->Scissor.NumWindowRects = count;
ctx->Scissor.WindowRectMode = mode;
-
-   if (ctx->Driver.Scissor)
-  ctx->Driver.Scissor(ctx);
 }
 
 
-- 
2.13.0

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


[Mesa-dev] [Bug 100613] Regression in Mesa 17 on s390x (zSystems)

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100613

--- Comment #39 from Stefan Dirsch  ---
(In reply to Ben Crocker from comment #36)
> So far, at least on this bug (or cluster of bugs), we have seen the
> same behavior on both S390 and big-endian Power8 (PPC64).

Power8 is ppc64le, isn't it? Confused ...

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100613] Regression in Mesa 17 on s390x (zSystems)

2017-06-02 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100613

--- Comment #38 from Stefan Dirsch  ---
(In reply to intermedi...@hotmail.com from comment #35)
> just for information 
> here my story about with Michel Danze reply
> https://bugs.freedesktop.org/show_bug.cgi?id=99859#c19
> 
> But there are many post on many big Endian hardware,
> was only curious if on s390 there was the same issue or not.

Well s390x comes without any gfx hardware. And that's the only BE platform SUSE
supports with our current enterprise product (where we use Mesa 17). We also
support ppc64le, but this is obviously LE - unlike ppc64.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/1] automake: r600 should only depend on libamd_common if opencl is enabled

2017-06-02 Thread Emil Velikov
On 1 June 2017 at 21:28, Jan Vesely  wrote:
> Signed-off-by: Jan Vesely 
> ---
> Hi guys,
>
> this is the first step towards dropping libamd_common dependency.
> It's based on Emil's patches 3/5 and 4/5.
> Enabling opencl still falls back to the old way of requiring libamd_common.
> I'll try to address that in the next step (no time estimate, feel free to 
> beat me to it). I think we can drop part of those functions rather than just 
> copying them.
>
AFAICT one still need the rest of my series, correct?

> Emil, I didn't find anything in android build that enabled opencl, so I 
> dropped it entirely.
>
Good point. Please flesh that in a separate patch though IIRC
there might be a bit more related cleanups that need squashing.

> regards,
> Jan
>
>  configure.ac| 3 ++-
>  src/gallium/drivers/r600/Android.mk | 5 -
>  src/gallium/drivers/r600/Automake.inc   | 2 +-
>  src/gallium/targets/pipe-loader/Makefile.am | 2 +-
>  4 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 5caf316..e0996a0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -2631,7 +2631,8 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test x$HAVE_SWRAST_DRI 
> = xyes)
>  AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
>  AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
>
> -AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_R600" = xyes -o \
> +AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a \
> +  "x$enable_opencl" = xyes \) -o \
Please add a beefy XXX/TODO/FIXME comment and reference the
s/HAVE_GALLIUM_LLVM/HAVE_AMD_DRIVERS/ changes.

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


Re: [Mesa-dev] [PATCH] mapi: Enable assembly language API acceleration for PPC64LE

2017-06-02 Thread Emil Velikov
Hi guys,

Ben please add some commit message. If nothing obvious comes to mind,
do mention how you tested the patch.
My PPC ASM is quite limited, so there's only minor suggestions below.

> --- a/src/mapi/Makefile.sources
> +++ b/src/mapi/Makefile.sources
> @@ -15,6 +15,7 @@
>  #this mode, compile MAPI_BRIDGE_FILES with MAPI_MODE_BRIDGE defined.
>
>  MAPI_UTIL_FILES = \
> +   ../util/u_endian.h \
Even though the header is used in entry.c we don't need this line.


> --- a/src/mapi/entry.c
> +++ b/src/mapi/entry.c
> @@ -27,6 +27,10 @@
>
>  #include "entry.h"
>  #include "u_current.h"
> +#include "../util/u_endian.h"
> +
$mesa_top/src is already in the search paths, so you can drop the "../" here.

> +#include 
> +#include 
>
Nit: please move system headers to the top.


> --- /dev/null
> +++ b/src/mapi/entry_ppc64le_tls.h

> +
> +void
> +entry_patch_public(void)
> +{
> +}
> +
We have some code only in the x86 tls case, haven't looked too closely why.
If anything comes to mind why the code is safe [as-is], please add a
small comment.


> +
> +void
> +entry_patch(mapi_func entry, int slot)
> +{
> +   char *code = (char *) entry;
> +   *((uint64_t *) (code + TEMPLATE_OFFSET_SLOT)) = slot * sizeof(mapi_func);
> +}
> +
> +mapi_func
> +entry_generate(int slot)
> +{
> +   char *code;
To be consistent with the existing code base, please add

   mapi_func entry;

> +
> +   code = u_execmem_alloc(sizeof(code_templ));
> +   if (!code)
> +  return NULL;
> +
> +   memcpy(code, code_templ, sizeof(code_templ));
> +
> +   *((uint64_t *) (code + TEMPLATE_OFFSET_TLS_ADDR)) = 
> ppc64le_dispatch_tls();

> +   *((uint64_t *) (code + TEMPLATE_OFFSET_SLOT)) = slot * sizeof(mapi_func);
> +
> + return (mapi_func) code;
... and replace the above with

   entry = (mapi_func) code;
   entry_patch(entry, slot);

   return entry;

Can you do the same in the TSD file?

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


Re: [Mesa-dev] [PATCH 03/11] radeonsi: decrease the number of compiler threads to num CPUs - 1

2017-06-02 Thread Marek Olšák
On Fri, Jun 2, 2017 at 11:26 AM, Grazvydas Ignotas  wrote:
> With the "core wars" starting (16core/32thread "consumer" CPUs
> announced), maybe it's time to add an upper limit? Waking up a core
> from low power state has it's latency, it's sometimes faster to just
> do several jobs on an active core than to wake another one (which will
> also cause more lock contention, the core will just see the job was
> already taken by someone else and go back to sleep).

The upper limit is currently 4.

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


Re: [Mesa-dev] [PATCH 03/11] radeonsi: decrease the number of compiler threads to num CPUs - 1

2017-06-02 Thread Grazvydas Ignotas
With the "core wars" starting (16core/32thread "consumer" CPUs
announced), maybe it's time to add an upper limit? Waking up a core
from low power state has it's latency, it's sometimes faster to just
do several jobs on an active core than to wake another one (which will
also cause more lock contention, the core will just see the job was
already taken by someone else and go back to sleep).

Gražvydas

On Thu, Jun 1, 2017 at 9:18 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> Reserve one core for other things (like draw calls).
> ---
>  src/gallium/drivers/radeonsi/si_pipe.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index de4e5da..4704304 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -874,22 +874,25 @@ struct pipe_screen *radeonsi_screen_create(struct 
> radeon_winsys *ws)
>
> si_init_screen_state_functions(sscreen);
>
> if (!r600_common_screen_init(>b, ws) ||
> !si_init_gs_info(sscreen) ||
> !si_init_shader_cache(sscreen)) {
> FREE(sscreen);
> return NULL;
> }
>
> -   /* Only enable as many threads as we have target machines and CPUs. */
> +   /* Only enable as many threads as we have target machines, but at most
> +* the number of CPUs - 1 if there is more than one.
> +*/
> num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> +   num_cpus = MAX2(1, num_cpus - 1);
> num_compiler_threads = MIN2(num_cpus, ARRAY_SIZE(sscreen->tm));
>
> if (!util_queue_init(>shader_compiler_queue, "si_shader",
>  32, num_compiler_threads)) {
> si_destroy_shader_cache(sscreen);
> FREE(sscreen);
> return NULL;
> }
>
> si_handle_env_var_force_family(sscreen);
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev