Re: [Mesa-dev] [PATCH] etnaviv: keep track of buffer valid ranges

[Wladimir]

On Thu, Oct 19, 2017 at 11:52 AM, Christian Gmeiner
 wrote:

>>> +   if ((usage & PIPE_TRANSFER_WRITE) &&
>>> + prsc->target == PIPE_BUFFER &&
>>> + !util_ranges_intersect(>valid_buffer_range,
>>> +box->x, box->x + box->width)) {
>>> + /* We are trying to write to a previously uninitialized range. No 
>>> need
>>> +  * to wait.
>>> +  */
>>
>> This unbalances the cpu_prep/fini in the map/unmap path. This isn't
>> allowed and the kernel will start to reject this in the near future.
>>
>
> Good to know that the kernel will reject this behauvior in the near future.

Seems a good idea nevertheless, and very welcome after rework, I've
seen some cases of excessive flushing coming from here.

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

Re: [Mesa-dev] [PATCH] etnaviv: keep track of buffer valid ranges

[Christian Gmeiner]

Hi Lucas,

2017-10-19 11:25 GMT+02:00 Lucas Stach :
> Am Donnerstag, den 19.10.2017, 07:59 +0200 schrieb Christian Gmeiner:
>> This allows a write to proceed to an uninitialized part of a buffer
>> even when the GPU is using the previously-initialized portions.
>> Same is done for freedreno, nouveau and radeon.
>>
>> > Signed-off-by: Christian Gmeiner 
>> ---
>>  src/gallium/drivers/etnaviv/etnaviv_resource.c |  3 +++
>>  src/gallium/drivers/etnaviv/etnaviv_resource.h |  4 
>>  src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 +++---
>>  3 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c 
>> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
>> index d6cccd2dbb..4eb49bf936 100644
>> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
>> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
>> @@ -268,6 +268,7 @@ etna_resource_alloc(struct pipe_screen *pscreen, 
>> unsigned layout,
>>
>> pipe_reference_init(>base.reference, 1);
>> list_inithead(>list);
>> +   util_range_init(>valid_buffer_range);
>>
>> size = setup_miptree(rsc, paddingX, paddingY, msaa_xscale, msaa_yscale);
>>
>> @@ -458,6 +459,7 @@ etna_resource_destroy(struct pipe_screen *pscreen, 
>> struct pipe_resource *prsc)
>> if (rsc->scanout)
>>renderonly_scanout_destroy(rsc->scanout, etna_screen(pscreen)->ro);
>>
>> +   util_range_destroy(>valid_buffer_range);
>> list_delinit(>list);
>>
>> pipe_resource_reference(>texture, NULL);
>> @@ -494,6 +496,7 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
>>
>> pipe_reference_init(>reference, 1);
>> list_inithead(>list);
>> +   util_range_init(>valid_buffer_range);
>> prsc->screen = pscreen;
>>
>> rsc->bo = etna_screen_bo_from_handle(pscreen, handle, >stride);
>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h 
>> b/src/gallium/drivers/etnaviv/etnaviv_resource.h
>> index 0b135e2373..705c1d1af6 100644
>> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
>> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
>> @@ -31,6 +31,7 @@
>>  #include "etnaviv_tiling.h"
>>  #include "pipe/p_state.h"
>>  #include "util/list.h"
>> +#include "util/u_range.h"
>>
>>  struct pipe_screen;
>>
>> @@ -73,6 +74,9 @@ struct etna_resource {
>>
>> struct etna_resource_level levels[ETNA_NUM_LOD];
>>
>> +   /* buffer range that has been initialized */
>> +   struct util_range valid_buffer_range;
>> +
>> /* When we are rendering to a texture, we need a differently tiled 
>> resource */
>> struct pipe_resource *texture;
>> /*
>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
>> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
>> index 6c1edd4835..33f3b058fa 100644
>> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
>> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
>> @@ -126,6 +126,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
>> pipe_transfer *ptrans)
>> if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))
>>etna_bo_cpu_fini(rsc->bo);
>>
>> +   util_range_add(>valid_buffer_range,
>> +  ptrans->box.x,
>> +  ptrans->box.x + ptrans->box.width);
>> +
>> pipe_resource_reference(>rsc, NULL);
>> pipe_resource_reference(>resource, NULL);
>> slab_free(>transfer_pool, trans);
>> @@ -283,7 +287,14 @@ etna_transfer_map(struct pipe_context *pctx, struct 
>> pipe_resource *prsc,
>>  * Pull resources into the CPU domain. Only skipped for unsynchronized
>>  * transfers without a temporary resource.
>>  */
>> -   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
>> +   if ((usage & PIPE_TRANSFER_WRITE) &&
>> + prsc->target == PIPE_BUFFER &&
>> + !util_ranges_intersect(>valid_buffer_range,
>> +box->x, box->x + box->width)) {
>> + /* We are trying to write to a previously uninitialized range. No 
>> need
>> +  * to wait.
>> +  */
>
> This unbalances the cpu_prep/fini in the map/unmap path. This isn't
> allowed and the kernel will start to reject this in the near future.
>

Good to know that the kernel will reject this behauvior in the near future.

> Also you need to differentiate between transfers with and without a
> temporary resource (trans->rsc), as the valid range of the temporary
> might be different from the base resource, especially if we need to
> read back from the base.
>

Thanks for the review - will rework it.

> Regards,
> Lucas
>
>> +} else if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
>>uint32_t prep_flags = 0;
>>
>>if (usage & PIPE_TRANSFER_READ)
>> @@ -360,10 +371,15 @@ fail_prep:
>>
>>  static void
>>  etna_transfer_flush_region(struct pipe_context *pctx,
>> -   struct pipe_transfer *transfer,
>> +   struct pipe_transfer 

Re: [Mesa-dev] [PATCH] etnaviv: keep track of buffer valid ranges

[Lucas Stach]

Am Donnerstag, den 19.10.2017, 07:59 +0200 schrieb Christian Gmeiner:
> This allows a write to proceed to an uninitialized part of a buffer
> even when the GPU is using the previously-initialized portions.
> Same is done for freedreno, nouveau and radeon.
> 
> > Signed-off-by: Christian Gmeiner 
> ---
>  src/gallium/drivers/etnaviv/etnaviv_resource.c |  3 +++
>  src/gallium/drivers/etnaviv/etnaviv_resource.h |  4 
>  src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 +++---
>  3 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c 
> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index d6cccd2dbb..4eb49bf936 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -268,6 +268,7 @@ etna_resource_alloc(struct pipe_screen *pscreen, unsigned 
> layout,
>  
> pipe_reference_init(>base.reference, 1);
> list_inithead(>list);
> +   util_range_init(>valid_buffer_range);
>  
> size = setup_miptree(rsc, paddingX, paddingY, msaa_xscale, msaa_yscale);
>  
> @@ -458,6 +459,7 @@ etna_resource_destroy(struct pipe_screen *pscreen, struct 
> pipe_resource *prsc)
> if (rsc->scanout)
>    renderonly_scanout_destroy(rsc->scanout, etna_screen(pscreen)->ro);
>  
> +   util_range_destroy(>valid_buffer_range);
> list_delinit(>list);
>  
> pipe_resource_reference(>texture, NULL);
> @@ -494,6 +496,7 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
>  
> pipe_reference_init(>reference, 1);
> list_inithead(>list);
> +   util_range_init(>valid_buffer_range);
> prsc->screen = pscreen;
>  
> rsc->bo = etna_screen_bo_from_handle(pscreen, handle, >stride);
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h 
> b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> index 0b135e2373..705c1d1af6 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> @@ -31,6 +31,7 @@
>  #include "etnaviv_tiling.h"
>  #include "pipe/p_state.h"
>  #include "util/list.h"
> +#include "util/u_range.h"
>  
>  struct pipe_screen;
>  
> @@ -73,6 +74,9 @@ struct etna_resource {
>  
> struct etna_resource_level levels[ETNA_NUM_LOD];
>  
> +   /* buffer range that has been initialized */
> +   struct util_range valid_buffer_range;
> +
> /* When we are rendering to a texture, we need a differently tiled 
> resource */
> struct pipe_resource *texture;
> /*
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index 6c1edd4835..33f3b058fa 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -126,6 +126,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
> pipe_transfer *ptrans)
> if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))
>    etna_bo_cpu_fini(rsc->bo);
>  
> +   util_range_add(>valid_buffer_range,
> +  ptrans->box.x,
> +  ptrans->box.x + ptrans->box.width);
> +
> pipe_resource_reference(>rsc, NULL);
> pipe_resource_reference(>resource, NULL);
> slab_free(>transfer_pool, trans);
> @@ -283,7 +287,14 @@ etna_transfer_map(struct pipe_context *pctx, struct 
> pipe_resource *prsc,
>  * Pull resources into the CPU domain. Only skipped for unsynchronized
>  * transfers without a temporary resource.
>  */
> -   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> +   if ((usage & PIPE_TRANSFER_WRITE) &&
> + prsc->target == PIPE_BUFFER &&
> + !util_ranges_intersect(>valid_buffer_range,
> +box->x, box->x + box->width)) {
> + /* We are trying to write to a previously uninitialized range. No 
> need
> +  * to wait.
> +  */

This unbalances the cpu_prep/fini in the map/unmap path. This isn't
allowed and the kernel will start to reject this in the near future.

Also you need to differentiate between transfers with and without a
temporary resource (trans->rsc), as the valid range of the temporary
might be different from the base resource, especially if we need to
read back from the base.

Regards,
Lucas

> +} else if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
>    uint32_t prep_flags = 0;
>  
>    if (usage & PIPE_TRANSFER_READ)
> @@ -360,10 +371,15 @@ fail_prep:
>  
>  static void
>  etna_transfer_flush_region(struct pipe_context *pctx,
> -   struct pipe_transfer *transfer,
> +   struct pipe_transfer *ptrans,
> const struct pipe_box *box)
>  {
> -   /* NOOP for now */
> +   struct etna_resource *rsc = etna_resource(ptrans->resource);
> +
> +   if (ptrans->resource->target == PIPE_BUFFER)
> +  util_range_add(>valid_buffer_range,
> + 

[Mesa-dev] [PATCH] etnaviv: keep track of buffer valid ranges

[Christian Gmeiner]

This allows a write to proceed to an uninitialized part of a buffer
even when the GPU is using the previously-initialized portions.
Same is done for freedreno, nouveau and radeon.

Signed-off-by: Christian Gmeiner 
---
 src/gallium/drivers/etnaviv/etnaviv_resource.c |  3 +++
 src/gallium/drivers/etnaviv/etnaviv_resource.h |  4 
 src/gallium/drivers/etnaviv/etnaviv_transfer.c | 22 +++---
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c 
b/src/gallium/drivers/etnaviv/etnaviv_resource.c
index d6cccd2dbb..4eb49bf936 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
@@ -268,6 +268,7 @@ etna_resource_alloc(struct pipe_screen *pscreen, unsigned 
layout,
 
pipe_reference_init(>base.reference, 1);
list_inithead(>list);
+   util_range_init(>valid_buffer_range);
 
size = setup_miptree(rsc, paddingX, paddingY, msaa_xscale, msaa_yscale);
 
@@ -458,6 +459,7 @@ etna_resource_destroy(struct pipe_screen *pscreen, struct 
pipe_resource *prsc)
if (rsc->scanout)
   renderonly_scanout_destroy(rsc->scanout, etna_screen(pscreen)->ro);
 
+   util_range_destroy(>valid_buffer_range);
list_delinit(>list);
 
pipe_resource_reference(>texture, NULL);
@@ -494,6 +496,7 @@ etna_resource_from_handle(struct pipe_screen *pscreen,
 
pipe_reference_init(>reference, 1);
list_inithead(>list);
+   util_range_init(>valid_buffer_range);
prsc->screen = pscreen;
 
rsc->bo = etna_screen_bo_from_handle(pscreen, handle, >stride);
diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h 
b/src/gallium/drivers/etnaviv/etnaviv_resource.h
index 0b135e2373..705c1d1af6 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
@@ -31,6 +31,7 @@
 #include "etnaviv_tiling.h"
 #include "pipe/p_state.h"
 #include "util/list.h"
+#include "util/u_range.h"
 
 struct pipe_screen;
 
@@ -73,6 +74,9 @@ struct etna_resource {
 
struct etna_resource_level levels[ETNA_NUM_LOD];
 
+   /* buffer range that has been initialized */
+   struct util_range valid_buffer_range;
+
/* When we are rendering to a texture, we need a differently tiled resource 
*/
struct pipe_resource *texture;
/*
diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c 
b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
index 6c1edd4835..33f3b058fa 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
@@ -126,6 +126,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct 
pipe_transfer *ptrans)
if (!trans->rsc && !(ptrans->usage & PIPE_TRANSFER_UNSYNCHRONIZED))
   etna_bo_cpu_fini(rsc->bo);
 
+   util_range_add(>valid_buffer_range,
+  ptrans->box.x,
+  ptrans->box.x + ptrans->box.width);
+
pipe_resource_reference(>rsc, NULL);
pipe_resource_reference(>resource, NULL);
slab_free(>transfer_pool, trans);
@@ -283,7 +287,14 @@ etna_transfer_map(struct pipe_context *pctx, struct 
pipe_resource *prsc,
 * Pull resources into the CPU domain. Only skipped for unsynchronized
 * transfers without a temporary resource.
 */
-   if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
+   if ((usage & PIPE_TRANSFER_WRITE) &&
+ prsc->target == PIPE_BUFFER &&
+ !util_ranges_intersect(>valid_buffer_range,
+box->x, box->x + box->width)) {
+ /* We are trying to write to a previously uninitialized range. No need
+  * to wait.
+  */
+} else if (trans->rsc || !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
   uint32_t prep_flags = 0;
 
   if (usage & PIPE_TRANSFER_READ)
@@ -360,10 +371,15 @@ fail_prep:
 
 static void
 etna_transfer_flush_region(struct pipe_context *pctx,
-   struct pipe_transfer *transfer,
+   struct pipe_transfer *ptrans,
const struct pipe_box *box)
 {
-   /* NOOP for now */
+   struct etna_resource *rsc = etna_resource(ptrans->resource);
+
+   if (ptrans->resource->target == PIPE_BUFFER)
+  util_range_add(>valid_buffer_range,
+ ptrans->box.x + box->x,
+ ptrans->box.x + box->x + box->width);
 }
 
 void
-- 
2.13.6

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