Re: [Mesa-dev] [PATCH 26/34] i965: Pretend that CCS modified images are two planes

2017-03-27 Thread Ben Widawsky 

On 17-03-23 14:02:13, Daniel Stone wrote:

Hi Ben,

On 24 January 2017 at 06:21, Ben Widawsky  wrote:

@@ -1018,9 +1018,18 @@ intel_from_planar(__DRIimage *parent, int plane, void 
*loaderPrivate)
 int width, height, offset, stride, dri_format, index;
 struct intel_image_format *f;
 __DRIimage *image;
-
-if (parent == NULL || parent->planar_format == NULL)
-return NULL;
+bool is_aux = parent->aux_offset && plane == 1;
+
+if (parent == NULL || parent->planar_format == NULL) {
+   if (is_aux) {
+  offset = parent->aux_offset;
+  stride = ALIGN(parent->pitch / 32, 128);
+  height = ALIGN(DIV_ROUND_UP(parent->height, 16), 32);
+  dri_format = parent->dri_format;
+  goto done;


The non-goto version in modifiersv9 now throws a warning for 'width'
being uninitialised. I assume this should just be ALIGN(parent->width
/ 32, 32), or ... ?

Cheers,
Daniel


It should be:
width = ALIGN(DIV_ROUND_UP(parent->width, 32), 128);

I'm reworking as much of this as possible to use ISL. modv12 should have that.

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

Re: [Mesa-dev] [PATCH 26/34] i965: Pretend that CCS modified images are two planes

2017-03-23 Thread Daniel Stone 
Hi Ben,

On 24 January 2017 at 06:21, Ben Widawsky  wrote:
> @@ -1018,9 +1018,18 @@ intel_from_planar(__DRIimage *parent, int plane, void 
> *loaderPrivate)
>  int width, height, offset, stride, dri_format, index;
>  struct intel_image_format *f;
>  __DRIimage *image;
> -
> -if (parent == NULL || parent->planar_format == NULL)
> -return NULL;
> +bool is_aux = parent->aux_offset && plane == 1;
> +
> +if (parent == NULL || parent->planar_format == NULL) {
> +   if (is_aux) {
> +  offset = parent->aux_offset;
> +  stride = ALIGN(parent->pitch / 32, 128);
> +  height = ALIGN(DIV_ROUND_UP(parent->height, 16), 32);
> +  dri_format = parent->dri_format;
> +  goto done;

The non-goto version in modifiersv9 now throws a warning for 'width'
being uninitialised. I assume this should just be ALIGN(parent->width
/ 32, 32), or ... ?

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

Re: [Mesa-dev] [PATCH 26/34] i965: Pretend that CCS modified images are two planes

2017-02-27 Thread Ben Widawsky 

On 17-01-31 13:16:24, Jason Ekstrand wrote:

On Mon, Jan 23, 2017 at 10:21 PM, Ben Widawsky  wrote:


Signed-off-by: Ben Widawsky 
Acked-by: Daniel Stone 
---
 src/mesa/drivers/dri/i965/intel_screen.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
b/src/mesa/drivers/dri/i965/intel_screen.c
index 971013f2dd..85070bb54d 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -809,7 +809,7 @@ intel_query_image(__DRIimage *image, int attrib, int
*value)
case __DRI_IMAGE_ATTRIB_FOURCC:
   return intel_lookup_fourcc(image->dri_format, value);
case __DRI_IMAGE_ATTRIB_NUM_PLANES:
-  *value = 1;
+  *value = image->aux_offset ? 2: 1;
   return true;
case __DRI_IMAGE_ATTRIB_OFFSET:
   *value = image->offset;
@@ -1018,9 +1018,18 @@ intel_from_planar(__DRIimage *parent, int plane,
void *loaderPrivate)
 int width, height, offset, stride, dri_format, index;
 struct intel_image_format *f;
 __DRIimage *image;
-
-if (parent == NULL || parent->planar_format == NULL)
-return NULL;
+bool is_aux = parent->aux_offset && plane == 1;



Why is this outside the if?  It seems to only apply when other plane stuff
is NULL as below.




Yes, you're right. Moved (and const'd).


+
+if (parent == NULL || parent->planar_format == NULL) {
+   if (is_aux) {
+  offset = parent->aux_offset;
+  stride = ALIGN(parent->pitch / 32, 128);
+  height = ALIGN(DIV_ROUND_UP(parent->height, 16), 32);
+  dri_format = parent->dri_format;
+  goto done;



Do we really need the goto?  Why not just put the stuff below in an else?




Yeah, that's fine. I like gotos more than I should, probably.


+   }
+   return NULL;
+}

 f = parent->planar_format;

@@ -1034,6 +1043,7 @@ intel_from_planar(__DRIimage *parent, int plane,
void *loaderPrivate)
 offset = parent->offsets[index];
 stride = parent->strides[index];

+done:
 image = intel_allocate_image(parent->screen, dri_format,
loaderPrivate);
 if (image == NULL)
return NULL;
--
2.11.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

Re: [Mesa-dev] [PATCH 26/34] i965: Pretend that CCS modified images are two planes

2017-01-31 Thread Jason Ekstrand 
On Mon, Jan 23, 2017 at 10:21 PM, Ben Widawsky  wrote:

> Signed-off-by: Ben Widawsky 
> Acked-by: Daniel Stone 
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 971013f2dd..85070bb54d 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -809,7 +809,7 @@ intel_query_image(__DRIimage *image, int attrib, int
> *value)
> case __DRI_IMAGE_ATTRIB_FOURCC:
>return intel_lookup_fourcc(image->dri_format, value);
> case __DRI_IMAGE_ATTRIB_NUM_PLANES:
> -  *value = 1;
> +  *value = image->aux_offset ? 2: 1;
>return true;
> case __DRI_IMAGE_ATTRIB_OFFSET:
>*value = image->offset;
> @@ -1018,9 +1018,18 @@ intel_from_planar(__DRIimage *parent, int plane,
> void *loaderPrivate)
>  int width, height, offset, stride, dri_format, index;
>  struct intel_image_format *f;
>  __DRIimage *image;
> -
> -if (parent == NULL || parent->planar_format == NULL)
> -return NULL;
> +bool is_aux = parent->aux_offset && plane == 1;
>

Why is this outside the if?  It seems to only apply when other plane stuff
is NULL as below.


> +
> +if (parent == NULL || parent->planar_format == NULL) {
> +   if (is_aux) {
> +  offset = parent->aux_offset;
> +  stride = ALIGN(parent->pitch / 32, 128);
> +  height = ALIGN(DIV_ROUND_UP(parent->height, 16), 32);
> +  dri_format = parent->dri_format;
> +  goto done;
>

Do we really need the goto?  Why not just put the stuff below in an else?


> +   }
> +   return NULL;
> +}
>
>  f = parent->planar_format;
>
> @@ -1034,6 +1043,7 @@ intel_from_planar(__DRIimage *parent, int plane,
> void *loaderPrivate)
>  offset = parent->offsets[index];
>  stride = parent->strides[index];
>
> +done:
>  image = intel_allocate_image(parent->screen, dri_format,
> loaderPrivate);
>  if (image == NULL)
> return NULL;
> --
> 2.11.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 26/34] i965: Pretend that CCS modified images are two planes

2017-01-23 Thread Ben Widawsky 
Signed-off-by: Ben Widawsky 
Acked-by: Daniel Stone 
---
 src/mesa/drivers/dri/i965/intel_screen.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index 971013f2dd..85070bb54d 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -809,7 +809,7 @@ intel_query_image(__DRIimage *image, int attrib, int *value)
case __DRI_IMAGE_ATTRIB_FOURCC:
   return intel_lookup_fourcc(image->dri_format, value);
case __DRI_IMAGE_ATTRIB_NUM_PLANES:
-  *value = 1;
+  *value = image->aux_offset ? 2: 1;
   return true;
case __DRI_IMAGE_ATTRIB_OFFSET:
   *value = image->offset;
@@ -1018,9 +1018,18 @@ intel_from_planar(__DRIimage *parent, int plane, void 
*loaderPrivate)
 int width, height, offset, stride, dri_format, index;
 struct intel_image_format *f;
 __DRIimage *image;
-
-if (parent == NULL || parent->planar_format == NULL)
-return NULL;
+bool is_aux = parent->aux_offset && plane == 1;
+
+if (parent == NULL || parent->planar_format == NULL) {
+   if (is_aux) {
+  offset = parent->aux_offset;
+  stride = ALIGN(parent->pitch / 32, 128);
+  height = ALIGN(DIV_ROUND_UP(parent->height, 16), 32);
+  dri_format = parent->dri_format;
+  goto done;
+   }
+   return NULL;
+}
 
 f = parent->planar_format;
 
@@ -1034,6 +1043,7 @@ intel_from_planar(__DRIimage *parent, int plane, void 
*loaderPrivate)
 offset = parent->offsets[index];
 stride = parent->strides[index];
 
+done:
 image = intel_allocate_image(parent->screen, dri_format, loaderPrivate);
 if (image == NULL)
return NULL;
-- 
2.11.0

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