lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 22 Mar 2019 12:00:12 +0100
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Eric Anholt <eric@...olt.net>, dri-devel@...ts.freedesktop.org
Cc:     linux-kernel@...r.kernel.org,
        Maxime Ripard <maxime.ripard@...tlin.com>
Subject: Re: [PATCH RESEND] drm/vc4: Make sure to emit a tile coordinates
 between two MSAA loads.

Hi,

Le mercredi 06 février 2019 à 15:25 -0800, Eric Anholt a écrit :
> The HW only executes a load once the tile coordinates packet happens,
> and only tracks one at a time, so by emitting our two MSAA loads back
> to back we would end up with an undefined color or Z buffer.

This change deals with things that I'm not very familiar with, but here
is my take on what is happening here:

- When we have to do more than a single load in the same render command
list, we need to send out tile coordinates and a dummy store in between
the two, because of internal architecture requirements of the GPU;
- We're dealing with the color buffer first and then the z-stencil
buffer;
- As a result, we need to issue that dummy store in the block handling
the z-stencil, if there was a previous color load in the rcl;
- We previously only did that for non-MSAA z-stencil buffers (without
the FULL_RES flag);
- The same thing actually needs to be done for the MSAA case too, as
the reason why we need that dummy store also applies to MSAA loads.

If my understanding is correct, then consider this:
Reviewed-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>

Otherwise, I'd be happy to know what I misunderstood in the process!

Cheers,

Paul

> Fixes dEQP-EGL.functional.render.multi_context.gles2.rgb888_window
> 
> Signed-off-by: Eric Anholt <eric@...olt.net>
> Cc: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> Cc: Maxime Ripard <maxime.ripard@...tlin.com>
> ---
>  drivers/gpu/drm/vc4/vc4_render_cl.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c b/drivers/gpu/drm/vc4/vc4_render_cl.c
> index 273984f71ae2..3c918eeaf56e 100644
> --- a/drivers/gpu/drm/vc4/vc4_render_cl.c
> +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c
> @@ -148,6 +148,12 @@ static void emit_tile(struct vc4_exec_info *exec,
>  	}
>  
>  	if (setup->zs_read) {
> +		if (setup->color_read) {
> +			/* Exec previous load. */
> +			vc4_tile_coordinates(setup, x, y);
> +			vc4_store_before_load(setup);
> +		}
> +
>  		if (args->zs_read.flags &
>  		    VC4_SUBMIT_RCL_SURFACE_READ_IS_FULL_RES) {
>  			rcl_u8(setup, VC4_PACKET_LOAD_FULL_RES_TILE_BUFFER);
> @@ -156,12 +162,6 @@ static void emit_tile(struct vc4_exec_info *exec,
>  						    &args->zs_read, x, y) |
>  				VC4_LOADSTORE_FULL_RES_DISABLE_COLOR);
>  		} else {
> -			if (setup->color_read) {
> -				/* Exec previous load. */
> -				vc4_tile_coordinates(setup, x, y);
> -				vc4_store_before_load(setup);
> -			}
> -
>  			rcl_u8(setup, VC4_PACKET_LOAD_TILE_BUFFER_GENERAL);
>  			rcl_u16(setup, args->zs_read.bits);
>  			rcl_u32(setup, setup->zs_read->paddr +
> @@ -291,16 +291,15 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec,
>  		}
>  	}
>  	if (setup->zs_read) {
> +		if (setup->color_read) {
> +			loop_body_size += VC4_PACKET_TILE_COORDINATES_SIZE;
> +			loop_body_size += VC4_PACKET_STORE_TILE_BUFFER_GENERAL_SIZE;
> +		}
> +
>  		if (args->zs_read.flags &
>  		    VC4_SUBMIT_RCL_SURFACE_READ_IS_FULL_RES) {
>  			loop_body_size += VC4_PACKET_LOAD_FULL_RES_TILE_BUFFER_SIZE;
>  		} else {
> -			if (setup->color_read &&
> -			    !(args->color_read.flags &
> -			      VC4_SUBMIT_RCL_SURFACE_READ_IS_FULL_RES)) {
> -				loop_body_size += VC4_PACKET_TILE_COORDINATES_SIZE;
> -				loop_body_size += VC4_PACKET_STORE_TILE_BUFFER_GENERAL_SIZE;
> -			}
>  			loop_body_size += VC4_PACKET_LOAD_TILE_BUFFER_GENERAL_SIZE;
>  		}
>  	}
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Powered by blists - more mailing lists