Nouveau - May 2014 - [Mesa-dev] [PATCH 02/12] nv50: setup scissors on clear_render_target/depth_stencil

On 21/05/14 00:39, Ilia Mirkin wrote:
> From: Christoph Bumiller <e0425955 at student.tuwien.ac.at>
> 
> [imirkin: add logic to also clear the "regular" scissors]
Can you please share why are you've added the "regular" scissors ?
Does it matter if we set the screen scissors before the render target params?

Thanks
Emil
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> index 1f37527..6e68fb8 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> @@ -288,6 +288,14 @@ nv50_clear_render_target(struct pipe_context *pipe,
>  
>     PUSH_REFN(push, bo, mt->base.domain | NOUVEAU_BO_WR);
>  
> +   BEGIN_NV04(push, NV50_3D(SCREEN_SCISSOR_HORIZ), 2);
> +   PUSH_DATA (push, ( width << 16) | dstx);
> +   PUSH_DATA (push, (height << 16) | dsty);
> +   BEGIN_NV04(push, NV50_3D(SCISSOR_HORIZ(0)), 2);
> +   PUSH_DATA (push, 8192 << 16);
> +   PUSH_DATA (push, 8192 << 16);
> +   nv50->scissors_dirty |= 1;
> +
>     BEGIN_NV04(push, NV50_3D(RT_CONTROL), 1);
>     PUSH_DATA (push, 1);
>     BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(0)), 5);
> @@ -325,7 +333,7 @@ nv50_clear_render_target(struct pipe_context *pipe,
>                   (z << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
>     }
>  
> -   nv50->dirty |= NV50_NEW_FRAMEBUFFER;
> +   nv50->dirty |= NV50_NEW_FRAMEBUFFER | NV50_NEW_SCISSOR;
>  }
>  
>  static void
> @@ -364,6 +372,14 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>  
>     PUSH_REFN(push, bo, mt->base.domain | NOUVEAU_BO_WR);
>  
> +   BEGIN_NV04(push, NV50_3D(SCREEN_SCISSOR_HORIZ), 2);
> +   PUSH_DATA (push, ( width << 16) | dstx);
> +   PUSH_DATA (push, (height << 16) | dsty);
> +   BEGIN_NV04(push, NV50_3D(SCISSOR_HORIZ(0)), 2);
> +   PUSH_DATA (push, 8192 << 16);
> +   PUSH_DATA (push, 8192 << 16);
> +   nv50->scissors_dirty |= 1;
> +
>     BEGIN_NV04(push, NV50_3D(ZETA_ADDRESS_HIGH), 5);
>     PUSH_DATAh(push, bo->offset + sf->offset);
>     PUSH_DATA (push, bo->offset + sf->offset);
> @@ -390,7 +406,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>                   (z << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
>     }
>  
> -   nv50->dirty |= NV50_NEW_FRAMEBUFFER;
> +   nv50->dirty |= NV50_NEW_FRAMEBUFFER | NV50_NEW_SCISSOR;
>  }
>  
>  void
>

On Wed, May 21, 2014 at 2:57 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
> On 21/05/14 00:39, Ilia Mirkin wrote:
>> From: Christoph Bumiller <e0425955 at student.tuwien.ac.at>
>>
>> [imirkin: add logic to also clear the "regular" scissors]
> Can you please share why are you've added the "regular"
scissors ?
Because I had to when I was implementing the clear_texture and
clear_buffer hooks (clear_texture didn't end up making it in quite yet
because I suck at writing piglit tests, but clear_buffer is there
now). Basically if scissors are set, they will apply to the clear,
which is not what we want.
> Does it matter if we set the screen scissors before the render target
params?
AFAIK, with the exception of a few "go" methods, you can pretty much
randomize the order in which you set all parameters.
>
> Thanks
> Emil
>
>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 20
++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> index 1f37527..6e68fb8 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
>> @@ -288,6 +288,14 @@ nv50_clear_render_target(struct pipe_context
*pipe,
>>
>>     PUSH_REFN(push, bo, mt->base.domain | NOUVEAU_BO_WR);
>>
>> +   BEGIN_NV04(push, NV50_3D(SCREEN_SCISSOR_HORIZ), 2);
>> +   PUSH_DATA (push, ( width << 16) | dstx);
>> +   PUSH_DATA (push, (height << 16) | dsty);
>> +   BEGIN_NV04(push, NV50_3D(SCISSOR_HORIZ(0)), 2);
>> +   PUSH_DATA (push, 8192 << 16);
>> +   PUSH_DATA (push, 8192 << 16);
>> +   nv50->scissors_dirty |= 1;
>> +
>>     BEGIN_NV04(push, NV50_3D(RT_CONTROL), 1);
>>     PUSH_DATA (push, 1);
>>     BEGIN_NV04(push, NV50_3D(RT_ADDRESS_HIGH(0)), 5);
>> @@ -325,7 +333,7 @@ nv50_clear_render_target(struct pipe_context *pipe,
>>                   (z << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
>>     }
>>
>> -   nv50->dirty |= NV50_NEW_FRAMEBUFFER;
>> +   nv50->dirty |= NV50_NEW_FRAMEBUFFER | NV50_NEW_SCISSOR;
>>  }
>>
>>  static void
>> @@ -364,6 +372,14 @@ nv50_clear_depth_stencil(struct pipe_context
*pipe,
>>
>>     PUSH_REFN(push, bo, mt->base.domain | NOUVEAU_BO_WR);
>>
>> +   BEGIN_NV04(push, NV50_3D(SCREEN_SCISSOR_HORIZ), 2);
>> +   PUSH_DATA (push, ( width << 16) | dstx);
>> +   PUSH_DATA (push, (height << 16) | dsty);
>> +   BEGIN_NV04(push, NV50_3D(SCISSOR_HORIZ(0)), 2);
>> +   PUSH_DATA (push, 8192 << 16);
>> +   PUSH_DATA (push, 8192 << 16);
>> +   nv50->scissors_dirty |= 1;
>> +
>>     BEGIN_NV04(push, NV50_3D(ZETA_ADDRESS_HIGH), 5);
>>     PUSH_DATAh(push, bo->offset + sf->offset);
>>     PUSH_DATA (push, bo->offset + sf->offset);
>> @@ -390,7 +406,7 @@ nv50_clear_depth_stencil(struct pipe_context *pipe,
>>                   (z << NV50_3D_CLEAR_BUFFERS_LAYER__SHIFT));
>>     }
>>
>> -   nv50->dirty |= NV50_NEW_FRAMEBUFFER;
>> +   nv50->dirty |= NV50_NEW_FRAMEBUFFER | NV50_NEW_SCISSOR;
>>  }
>>
>>  void
>>
>

On 21/05/14 19:59, Ilia Mirkin wrote:
> On Wed, May 21, 2014 at 2:57 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
>> On 21/05/14 00:39, Ilia Mirkin wrote:
>>> From: Christoph Bumiller <e0425955 at student.tuwien.ac.at>
>>>
>>> [imirkin: add logic to also clear the "regular" scissors]
>> Can you please share why are you've added the "regular"
scissors ?
> 
> Because I had to when I was implementing the clear_texture and
> clear_buffer hooks (clear_texture didn't end up making it in quite yet
> because I suck at writing piglit tests, but clear_buffer is there
> now). Basically if scissors are set, they will apply to the clear,
> which is not what we want.
> 
>> Does it matter if we set the screen scissors before the render target
params?
> 
> AFAIK, with the exception of a few "go" methods, you can pretty
much
> randomize the order in which you set all parameters.
> 
Great, thank you.