Re: [Mesa-dev] [PATCH 3/3] i965/tiled_memoyp: Get rid of the direction parameter to get_memcpy

2016-04-08 Thread Jason Ekstrand 
On Thu, Apr 7, 2016 at 4:57 PM, Matt Turner  wrote:

> On Thu, Apr 7, 2016 at 1:56 PM, Jason Ekstrand 
> wrote:
> > Now that we can use the much simpler rgba8_copy function, we don't need
> to
> > hand different functions out based on direction.
> > ---
>
> Typo in the subject, and PATCH 2/3 needs a bugzilla link.
>

Fixed


> The series is
>
> Reviewed-by: Matt Turner 
>
> I noticed that gcc does *not* unroll the loop in the 64-byte case.
> I've got some small optimizations to this code that I'll send after
> this series, so I'll just fix that up at the same time.
>

I just pushed.  I added a Cc to stable for the first two patches.  I
recommend you do as well for whatever you send that fixes the unrolling
problem.
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] i965/tiled_memoyp: Get rid of the direction parameter to get_memcpy

2016-04-08 Thread Chad Versace 
On 04/07/2016 01:56 PM, Jason Ekstrand wrote:
> Now that we can use the much simpler rgba8_copy function, we don't need to
> hand different functions out based on direction.
> ---
>  src/mesa/drivers/dri/i965/intel_pixel_read.c   |  3 +--
>  src/mesa/drivers/dri/i965/intel_tex_image.c|  3 +--
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c |  3 +--
>  src/mesa/drivers/dri/i965/intel_tiled_memcpy.c |  3 +--
>  src/mesa/drivers/dri/i965/intel_tiled_memcpy.h | 15 +--
>  5 files changed, 5 insertions(+), 22 deletions(-)

I read the patches closely, and I like their approach.

For the series,
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 3/3] i965/tiled_memoyp: Get rid of the direction parameter to get_memcpy

2016-04-07 Thread Roland Scheidegger 
Am 07.04.2016 um 22:56 schrieb Jason Ekstrand:
> Now that we can use the much simpler rgba8_copy function, we don't need to
> hand different functions out based on direction.
> ---
>  src/mesa/drivers/dri/i965/intel_pixel_read.c   |  3 +--
>  src/mesa/drivers/dri/i965/intel_tex_image.c|  3 +--
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c |  3 +--
>  src/mesa/drivers/dri/i965/intel_tiled_memcpy.c |  3 +--
>  src/mesa/drivers/dri/i965/intel_tiled_memcpy.h | 15 +--
>  5 files changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c 
> b/src/mesa/drivers/dri/i965/intel_pixel_read.c
> index 31030b1..a486d6e 100644
> --- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
> +++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
> @@ -141,8 +141,7 @@ intel_readpixels_tiled_memcpy(struct gl_context * ctx,
> if (rb->_BaseFormat == GL_RGB)
>return false;
>  
> -   if (!intel_get_memcpy(rb->Format, format, type, _copy, ,
> - INTEL_DOWNLOAD))
> +   if (!intel_get_memcpy(rb->Format, format, type, _copy, ))
>return false;
>  
> if (!irb->mt ||
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 1601edd..bee8be1 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -404,8 +404,7 @@ intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx,
> if (texImage->_BaseFormat == GL_RGB)
>return false;
>  
> -   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ,
> - INTEL_DOWNLOAD))
> +   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ))
>return false;
>  
> /* If this is a nontrivial texture view, let another path handle it 
> instead. */
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c 
> b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> index 4849a41..9561968 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> @@ -119,8 +119,7 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
> if (ctx->_ImageTransferState)
>return false;
>  
> -   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ,
> - INTEL_UPLOAD))
> +   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ))
>return false;
>  
> /* If this is a nontrivial texture view, let another path handle it 
> instead. */
> diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c 
> b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
> index 66c1f9b..0a68751 100644
> --- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
> +++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
> @@ -745,8 +745,7 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
>   * \return true if the format and type combination are valid
>   */
>  bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
> -  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
> -  enum intel_memcpy_direction direction)
> +  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp)
>  {
> if (type == GL_UNSIGNED_INT_8_8_8_8_REV &&
> !(format == GL_RGBA || format == GL_BGRA))
> diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h 
> b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
> index 01543bf..d9148bb 100644
> --- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
> +++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
> @@ -55,20 +55,7 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
>  uint32_t tiling,
>  mem_copy_fn mem_copy);
>  
> -/* Tells intel_get_memcpy() whether the memcpy() is
> - *
> - *  - an upload to the GPU with an aligned destination and a potentially
> - *unaligned source; or
> - *  - a download from the GPU with an aligned source and a potentially
> - *unaligned destination.
> - */
> -enum intel_memcpy_direction {
> -   INTEL_UPLOAD,
> -   INTEL_DOWNLOAD
> -};
> -
>  bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
> -  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
> -  enum intel_memcpy_direction direction);
> +  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp);
>  
>  #endif /* INTEL_TILED_MEMCPY */
> 

Other than hacking on the assembly in there I'm really not all that
familiar with that code but these patches look like a nice cleanup.

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

Re: [Mesa-dev] [PATCH 3/3] i965/tiled_memoyp: Get rid of the direction parameter to get_memcpy

2016-04-07 Thread Matt Turner 
On Thu, Apr 7, 2016 at 1:56 PM, Jason Ekstrand  wrote:
> Now that we can use the much simpler rgba8_copy function, we don't need to
> hand different functions out based on direction.
> ---

Typo in the subject, and PATCH 2/3 needs a bugzilla link.

The series is

Reviewed-by: Matt Turner 

I noticed that gcc does *not* unroll the loop in the 64-byte case.
I've got some small optimizations to this code that I'll send after
this series, so I'll just fix that up at the same time.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/3] i965/tiled_memoyp: Get rid of the direction parameter to get_memcpy

2016-04-07 Thread Jason Ekstrand 
Now that we can use the much simpler rgba8_copy function, we don't need to
hand different functions out based on direction.
---
 src/mesa/drivers/dri/i965/intel_pixel_read.c   |  3 +--
 src/mesa/drivers/dri/i965/intel_tex_image.c|  3 +--
 src/mesa/drivers/dri/i965/intel_tex_subimage.c |  3 +--
 src/mesa/drivers/dri/i965/intel_tiled_memcpy.c |  3 +--
 src/mesa/drivers/dri/i965/intel_tiled_memcpy.h | 15 +--
 5 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c 
b/src/mesa/drivers/dri/i965/intel_pixel_read.c
index 31030b1..a486d6e 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
@@ -141,8 +141,7 @@ intel_readpixels_tiled_memcpy(struct gl_context * ctx,
if (rb->_BaseFormat == GL_RGB)
   return false;
 
-   if (!intel_get_memcpy(rb->Format, format, type, _copy, ,
- INTEL_DOWNLOAD))
+   if (!intel_get_memcpy(rb->Format, format, type, _copy, ))
   return false;
 
if (!irb->mt ||
diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
b/src/mesa/drivers/dri/i965/intel_tex_image.c
index 1601edd..bee8be1 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -404,8 +404,7 @@ intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx,
if (texImage->_BaseFormat == GL_RGB)
   return false;
 
-   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ,
- INTEL_DOWNLOAD))
+   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ))
   return false;
 
/* If this is a nontrivial texture view, let another path handle it 
instead. */
diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c 
b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
index 4849a41..9561968 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
@@ -119,8 +119,7 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
if (ctx->_ImageTransferState)
   return false;
 
-   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ,
- INTEL_UPLOAD))
+   if (!intel_get_memcpy(texImage->TexFormat, format, type, _copy, ))
   return false;
 
/* If this is a nontrivial texture view, let another path handle it 
instead. */
diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c 
b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
index 66c1f9b..0a68751 100644
--- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
+++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
@@ -745,8 +745,7 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
  * \return true if the format and type combination are valid
  */
 bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
-  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
-  enum intel_memcpy_direction direction)
+  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp)
 {
if (type == GL_UNSIGNED_INT_8_8_8_8_REV &&
!(format == GL_RGBA || format == GL_BGRA))
diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h 
b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
index 01543bf..d9148bb 100644
--- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
+++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
@@ -55,20 +55,7 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
 uint32_t tiling,
 mem_copy_fn mem_copy);
 
-/* Tells intel_get_memcpy() whether the memcpy() is
- *
- *  - an upload to the GPU with an aligned destination and a potentially
- *unaligned source; or
- *  - a download from the GPU with an aligned source and a potentially
- *unaligned destination.
- */
-enum intel_memcpy_direction {
-   INTEL_UPLOAD,
-   INTEL_DOWNLOAD
-};
-
 bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
-  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
-  enum intel_memcpy_direction direction);
+  GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp);
 
 #endif /* INTEL_TILED_MEMCPY */
-- 
2.5.0.400.gff86faf

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