Re: [Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Jason Ekstrand]

I'd personally separate the XML changes from the is changes but I don't 
care that much.  Either way, r-b me.



On October 2, 2017 3:49:16 PM Kenneth Graunke  wrote:


These shouldn't matter for non-cubes, and we always enable them all
for cubes, so we may as well set them all the time.  While we're at it,
let's make the genxml fields consistent.  We pick the boolean-per-face
approach because it's clear which bits correspond to which cube faces.

v2: Don't use "mbo" (requested by Matt and Jason).
---
 src/intel/genxml/gen4.xml |  7 ++-
 src/intel/genxml/gen45.xml|  7 ++-
 src/intel/genxml/gen5.xml |  7 ++-
 src/intel/genxml/gen6.xml |  7 ++-
 src/intel/genxml/gen7.xml |  7 ++-
 src/intel/genxml/gen75.xml|  7 ++-
 src/intel/isl/isl_surface_state.c | 18 ++
 7 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/src/intel/genxml/gen4.xml b/src/intel/genxml/gen4.xml
index 6499346c999..fc24329535d 100644
--- a/src/intel/genxml/gen4.xml
+++ b/src/intel/genxml/gen4.xml
@@ -526,7 +526,12 @@
 
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/genxml/gen45.xml b/src/intel/genxml/gen45.xml
index 0f905754071..c91085831ea 100644
--- a/src/intel/genxml/gen45.xml
+++ b/src/intel/genxml/gen45.xml
@@ -531,7 +531,12 @@
   
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/genxml/gen5.xml b/src/intel/genxml/gen5.xml
index 70f50076abf..93e687a32bd 100644
--- a/src/intel/genxml/gen5.xml
+++ b/src/intel/genxml/gen5.xml
@@ -636,7 +636,12 @@
 
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index 8aa03355055..96f4be784e3 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -574,7 +574,12 @@
   
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index 993d10264fa..cc17018b582 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -638,7 +638,12 @@
   
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 105effa8cef..cb408a2bb8d 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -657,7 +657,12 @@
   
   
 
-
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>
+type="bool"/>

 
 
 
diff --git a/src/intel/isl/isl_surface_state.c 
b/src/intel/isl/isl_surface_state.c

index c6a55ce9707..bfb27fa4a44 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -452,18 +452,12 @@ isl_genX(surf_fill_state_s)(const struct isl_device 
*dev, void *state,

s.RenderCacheReadWriteMode = 0;
 #endif

-   if (info->view->usage & ISL_SURF_USAGE_CUBE_BIT) {
-#if GEN_GEN >= 8
-  s.CubeFaceEnablePositiveZ = 1;
-  s.CubeFaceEnableNegativeZ = 1;
-  s.CubeFaceEnablePositiveY = 1;
-  s.CubeFaceEnableNegativeY = 1;
-  s.CubeFaceEnablePositiveX = 1;
-  s.CubeFaceEnableNegativeX = 1;
-#else
-  s.CubeFaceEnables = 0x3f;
-#endif
-   }
+   s.CubeFaceEnablePositiveZ = 1;
+   s.CubeFaceEnableNegativeZ = 1;
+   s.CubeFaceEnablePositiveY = 1;
+   s.CubeFaceEnableNegativeY = 1;
+   s.CubeFaceEnablePositiveX = 1;
+   s.CubeFaceEnableNegativeX = 1;

 #if GEN_GEN >= 6
s.NumberofMultisamples = ffs(info->surf->samples) - 1;
--
2.14.2

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



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

Re: [Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Matt Turner]

Looks good to me.

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

[Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Kenneth Graunke]

These shouldn't matter for non-cubes, and we always enable them all
for cubes, so we may as well set them all the time.  While we're at it,
let's make the genxml fields consistent.  We pick the boolean-per-face
approach because it's clear which bits correspond to which cube faces.

v2: Don't use "mbo" (requested by Matt and Jason).
---
 src/intel/genxml/gen4.xml |  7 ++-
 src/intel/genxml/gen45.xml|  7 ++-
 src/intel/genxml/gen5.xml |  7 ++-
 src/intel/genxml/gen6.xml |  7 ++-
 src/intel/genxml/gen7.xml |  7 ++-
 src/intel/genxml/gen75.xml|  7 ++-
 src/intel/isl/isl_surface_state.c | 18 ++
 7 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/src/intel/genxml/gen4.xml b/src/intel/genxml/gen4.xml
index 6499346c999..fc24329535d 100644
--- a/src/intel/genxml/gen4.xml
+++ b/src/intel/genxml/gen4.xml
@@ -526,7 +526,12 @@
 
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen45.xml b/src/intel/genxml/gen45.xml
index 0f905754071..c91085831ea 100644
--- a/src/intel/genxml/gen45.xml
+++ b/src/intel/genxml/gen45.xml
@@ -531,7 +531,12 @@
   
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen5.xml b/src/intel/genxml/gen5.xml
index 70f50076abf..93e687a32bd 100644
--- a/src/intel/genxml/gen5.xml
+++ b/src/intel/genxml/gen5.xml
@@ -636,7 +636,12 @@
 
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index 8aa03355055..96f4be784e3 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -574,7 +574,12 @@
   
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index 993d10264fa..cc17018b582 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -638,7 +638,12 @@
   
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 105effa8cef..cb408a2bb8d 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -657,7 +657,12 @@
   
   
 
-
+
+
+
+
+
+
 
 
 
diff --git a/src/intel/isl/isl_surface_state.c 
b/src/intel/isl/isl_surface_state.c
index c6a55ce9707..bfb27fa4a44 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -452,18 +452,12 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, 
void *state,
s.RenderCacheReadWriteMode = 0;
 #endif
 
-   if (info->view->usage & ISL_SURF_USAGE_CUBE_BIT) {
-#if GEN_GEN >= 8
-  s.CubeFaceEnablePositiveZ = 1;
-  s.CubeFaceEnableNegativeZ = 1;
-  s.CubeFaceEnablePositiveY = 1;
-  s.CubeFaceEnableNegativeY = 1;
-  s.CubeFaceEnablePositiveX = 1;
-  s.CubeFaceEnableNegativeX = 1;
-#else
-  s.CubeFaceEnables = 0x3f;
-#endif
-   }
+   s.CubeFaceEnablePositiveZ = 1;
+   s.CubeFaceEnableNegativeZ = 1;
+   s.CubeFaceEnablePositiveY = 1;
+   s.CubeFaceEnableNegativeY = 1;
+   s.CubeFaceEnablePositiveX = 1;
+   s.CubeFaceEnableNegativeX = 1;
 
 #if GEN_GEN >= 6
s.NumberofMultisamples = ffs(info->surf->samples) - 1;
-- 
2.14.2

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

Re: [Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Jason Ekstrand]

On September 30, 2017 12:29:56 AM Matt Turner  wrote:

On Fri, Sep 29, 2017 at 10:54 PM, Kenneth Graunke  
wrote:

These shouldn't matter for non-cubes, and we always enable them all
for cubes, so we may as well set them all the time.  We can just mark
the fields "mbo" (must be one) and genxml will automatically set them
for us, and we never even have to think about them.


I don't like overloading the concept of must-be-zero/one. It really
means that the hardware requires bits to be unset/set.


Agreed.  I'd rather we just set them since they do have meaning.


It looks like the if GEN_GEN check in isl_surface_state.c can go away
if we just make all of the genxml match. Not sure why we made gen8+
individual bits and earlier ones a uint bitfield. I think it's really
all the same.


Mostly because I couldn't decide which of the two I liked better so I never 
unified it.



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

Re: [Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Matt Turner]

On Fri, Sep 29, 2017 at 10:54 PM, Kenneth Graunke  wrote:
> These shouldn't matter for non-cubes, and we always enable them all
> for cubes, so we may as well set them all the time.  We can just mark
> the fields "mbo" (must be one) and genxml will automatically set them
> for us, and we never even have to think about them.

I don't like overloading the concept of must-be-zero/one. It really
means that the hardware requires bits to be unset/set.

It looks like the if GEN_GEN check in isl_surface_state.c can go away
if we just make all of the genxml match. Not sure why we made gen8+
individual bits and earlier ones a uint bitfield. I think it's really
all the same.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] intel: Always set Cube Face Enables for all surfaces.

[Kenneth Graunke]

These shouldn't matter for non-cubes, and we always enable them all
for cubes, so we may as well set them all the time.  We can just mark
the fields "mbo" (must be one) and genxml will automatically set them
for us, and we never even have to think about them.
---
 src/intel/genxml/gen10.xml|  7 +--
 src/intel/genxml/gen4.xml |  2 +-
 src/intel/genxml/gen45.xml|  2 +-
 src/intel/genxml/gen5.xml |  2 +-
 src/intel/genxml/gen6.xml |  2 +-
 src/intel/genxml/gen7.xml |  2 +-
 src/intel/genxml/gen75.xml|  2 +-
 src/intel/genxml/gen8.xml |  7 +--
 src/intel/genxml/gen9.xml |  7 +--
 src/intel/isl/isl_surface_state.c | 13 -
 10 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/src/intel/genxml/gen10.xml b/src/intel/genxml/gen10.xml
index a7ae49ae659..e6f10010b6d 100644
--- a/src/intel/genxml/gen10.xml
+++ b/src/intel/genxml/gen10.xml
@@ -714,12 +714,7 @@
   
   
 
-
-
-
-
-
-
+
 
 
 
diff --git a/src/intel/genxml/gen4.xml b/src/intel/genxml/gen4.xml
index 6499346c999..6fe411d09ef 100644
--- a/src/intel/genxml/gen4.xml
+++ b/src/intel/genxml/gen4.xml
@@ -526,7 +526,7 @@
 
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen45.xml b/src/intel/genxml/gen45.xml
index 0f905754071..db297f4d461 100644
--- a/src/intel/genxml/gen45.xml
+++ b/src/intel/genxml/gen45.xml
@@ -531,7 +531,7 @@
   
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen5.xml b/src/intel/genxml/gen5.xml
index 70f50076abf..d33d3be003c 100644
--- a/src/intel/genxml/gen5.xml
+++ b/src/intel/genxml/gen5.xml
@@ -636,7 +636,7 @@
 
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index 8aa03355055..c6394911438 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -574,7 +574,7 @@
   
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index 993d10264fa..6f21fc33de2 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -638,7 +638,7 @@
   
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 105effa8cef..208018359ce 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -657,7 +657,7 @@
   
   
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 99c4aca8fb4..3d4adee2858 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -680,12 +680,7 @@
   
   
 
-
-
-
-
-
-
+
 
 
 
diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml
index 1422463693d..9f94c027c5a 100644
--- a/src/intel/genxml/gen9.xml
+++ b/src/intel/genxml/gen9.xml
@@ -712,12 +712,7 @@
   
   
 
-
-
-
-
-
-
+
 
 
 
diff --git a/src/intel/isl/isl_surface_state.c 
b/src/intel/isl/isl_surface_state.c
index c6a55ce9707..71a6ed2168b 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -452,19 +452,6 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, 
void *state,
s.RenderCacheReadWriteMode = 0;
 #endif
 
-   if (info->view->usage & ISL_SURF_USAGE_CUBE_BIT) {
-#if GEN_GEN >= 8
-  s.CubeFaceEnablePositiveZ = 1;
-  s.CubeFaceEnableNegativeZ = 1;
-  s.CubeFaceEnablePositiveY = 1;
-  s.CubeFaceEnableNegativeY = 1;
-  s.CubeFaceEnablePositiveX = 1;
-  s.CubeFaceEnableNegativeX = 1;
-#else
-  s.CubeFaceEnables = 0x3f;
-#endif
-   }
-
 #if GEN_GEN >= 6
s.NumberofMultisamples = ffs(info->surf->samples) - 1;
 #if GEN_GEN >= 7
-- 
2.14.1

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