[Mesa-dev] [PATCH 3/5] radv: remove some members of radeon surface.

Timothy Arceri tarceri at itsqueeze.com
Tue May 2 04:18:59 UTC 2017



On 02/05/17 10:22, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> We would be storing this info twice per image, no need to,
> remove it from the surface struct.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>   src/amd/vulkan/radv_cmd_buffer.c                   |  2 +-
>   src/amd/vulkan/radv_image.c                        | 19 +++----
>   src/amd/vulkan/radv_radeon_winsys.h                |  7 +--
>   src/amd/vulkan/winsys/amdgpu/radv_amdgpu_surface.c | 60 ++++++++++++----------
>   4 files changed, 40 insertions(+), 48 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 198c599..d165512 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -966,7 +966,7 @@ static void radv_set_optimal_micro_tile_mode(struct radv_device *device,
>   {
>   	struct radv_image *image = att->attachment->image;
>   	uint32_t tile_mode_index;
> -	if (image->surface.nsamples <= 1)
> +	if (image->info.samples <= 1)
>   		return;
>   
>   	if (image->surface.micro_tile_mode != micro_tile_mode) {
> diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
> index cd278ed..24f3620 100644
> --- a/src/amd/vulkan/radv_image.c
> +++ b/src/amd/vulkan/radv_image.c
> @@ -67,22 +67,16 @@ radv_init_surface(struct radv_device *device,
>   
>   	is_depth = vk_format_has_depth(desc);
>   	is_stencil = vk_format_has_stencil(desc);
> -	surface->npix_x = pCreateInfo->extent.width;
> -	surface->npix_y = pCreateInfo->extent.height;
> -	surface->npix_z = pCreateInfo->extent.depth;
>   
>   	surface->blk_w = vk_format_get_blockwidth(pCreateInfo->format);
>   	surface->blk_h = vk_format_get_blockheight(pCreateInfo->format);
>   	surface->blk_d = 1;
> -	surface->array_size = pCreateInfo->arrayLayers;
> -	surface->last_level = pCreateInfo->mipLevels - 1;
>   
>   	surface->bpe = vk_format_get_blocksize(pCreateInfo->format);
>   	/* align byte per element on dword */
>   	if (surface->bpe == 3) {
>   		surface->bpe = 4;
>   	}
> -	surface->nsamples = pCreateInfo->samples ? pCreateInfo->samples : 1;
>   	surface->flags = RADEON_SURF_SET(array_mode, MODE);
>   
>   	switch (pCreateInfo->imageType){
> @@ -467,14 +461,13 @@ radv_image_get_fmask_info(struct radv_device *device,
>   {
>   	/* FMASK is allocated like an ordinary texture. */
>   	struct radeon_surf fmask = image->surface;
> -
> +	struct radeon_surf_info info = image->info;
>   	memset(out, 0, sizeof(*out));
>   
>   	fmask.bo_alignment = 0;
>   	fmask.bo_size = 0;
> -	fmask.nsamples = 1;
>   	fmask.flags |= RADEON_SURF_FMASK;
> -
> +	info.samples = 1;
>   	/* Force 2D tiling if it wasn't set. This may occur when creating
>   	 * FMASK for MSAA resolve on R6xx. On R6xx, the single-sample
>   	 * destination buffer must have an FMASK too. */
> @@ -495,7 +488,7 @@ radv_image_get_fmask_info(struct radv_device *device,
>   		return;
>   	}
>   
> -	device->ws->surface_init(device->ws, &fmask);
> +	device->ws->surface_init(device->ws, &info, &fmask);
>   	assert(fmask.level[0].mode == RADEON_SURF_MODE_2D);
>   
>   	out->slice_tile_max = (fmask.level[0].nblk_x * fmask.level[0].nblk_y) / 64;
> @@ -553,8 +546,8 @@ radv_image_get_cmask_info(struct radv_device *device,
>   
>   	unsigned base_align = num_pipes * pipe_interleave_bytes;
>   
> -	unsigned width = align(image->surface.npix_x, cl_width*8);
> -	unsigned height = align(image->surface.npix_y, cl_height*8);
> +	unsigned width = align(image->info.width, cl_width*8);
> +	unsigned height = align(image->info.height, cl_height*8);
>   	unsigned slice_elements = (width * height) / (8*8);
>   
>   	/* Each element of CMASK is a nibble. */
> @@ -656,7 +649,7 @@ radv_image_create(VkDevice _device,
>   
>   	radv_init_surface(device, &image->surface, create_info);
>   
> -	device->ws->surface_init(device->ws, &image->surface);
> +	device->ws->surface_init(device->ws, &image->info, &image->surface);

Maybe change this to just take image as a param?

Same goes for radv_amdgpu_winsys_surface_init()

With or without that 2-5 are:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

>   
>   	image->size = image->surface.bo_size;
>   	image->alignment = image->surface.bo_alignment;
> diff --git a/src/amd/vulkan/radv_radeon_winsys.h b/src/amd/vulkan/radv_radeon_winsys.h
> index d934e0a..48d72b9 100644
> --- a/src/amd/vulkan/radv_radeon_winsys.h
> +++ b/src/amd/vulkan/radv_radeon_winsys.h
> @@ -184,16 +184,10 @@ struct radeon_surf_level {
>   /* surface defintions from the winsys */
>   struct radeon_surf {
>   	/* These are inputs to the calculator. */
> -	uint32_t                    npix_x;
> -	uint32_t                    npix_y;
> -	uint32_t                    npix_z;
>   	uint32_t                    blk_w;
>   	uint32_t                    blk_h;
>   	uint32_t                    blk_d;
> -	uint32_t                    array_size;
> -	uint32_t                    last_level;
>   	uint32_t                    bpe;
> -	uint32_t                    nsamples;
>   	uint32_t                    flags;
>   
>   	/* These are return values. Some of them can be set by the caller, but
> @@ -343,6 +337,7 @@ struct radeon_winsys {
>   	void (*cs_dump)(struct radeon_winsys_cs *cs, FILE* file, uint32_t trace_id);
>   
>   	int (*surface_init)(struct radeon_winsys *ws,
> +			    const struct radeon_surf_info *surf_info,
>   			    struct radeon_surf *surf);
>   
>   	int (*surface_best)(struct radeon_winsys *ws,
> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_surface.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_surface.c
> index 511f464..55bbd30 100644
> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_surface.c
> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_surface.c
> @@ -47,7 +47,8 @@
>   #define CIASICIDGFXENGINE_SOUTHERNISLAND 0x0000000A
>   #endif
>   
> -static int radv_amdgpu_surface_sanity(const struct radeon_surf *surf)
> +static int radv_amdgpu_surface_sanity(const struct radeon_surf_info *surf_info,
> +				      const struct radeon_surf *surf)
>   {
>   	unsigned type = RADEON_SURF_GET(surf->flags, TYPE);
>   
> @@ -55,14 +56,14 @@ static int radv_amdgpu_surface_sanity(const struct radeon_surf *surf)
>   		return -EINVAL;
>   
>   	/* all dimension must be at least 1 ! */
> -	if (!surf->npix_x || !surf->npix_y || !surf->npix_z ||
> -	    !surf->array_size)
> +	if (!surf_info->width || !surf_info->height || !surf_info->depth ||
> +	    !surf_info->array_size)
>   		return -EINVAL;
>   
>   	if (!surf->blk_w || !surf->blk_h || !surf->blk_d)
>   		return -EINVAL;
>   
> -	switch (surf->nsamples) {
> +	switch (surf_info->samples) {
>   	case 1:
>   	case 2:
>   	case 4:
> @@ -74,24 +75,24 @@ static int radv_amdgpu_surface_sanity(const struct radeon_surf *surf)
>   
>   	switch (type) {
>   	case RADEON_SURF_TYPE_1D:
> -		if (surf->npix_y > 1)
> +		if (surf_info->height > 1)
>   			return -EINVAL;
>   		/* fall through */
>   	case RADEON_SURF_TYPE_2D:
>   	case RADEON_SURF_TYPE_CUBEMAP:
> -		if (surf->npix_z > 1 || surf->array_size > 1)
> +		if (surf_info->depth > 1 || surf_info->array_size > 1)
>   			return -EINVAL;
>   		break;
>   	case RADEON_SURF_TYPE_3D:
> -		if (surf->array_size > 1)
> +		if (surf_info->array_size > 1)
>   			return -EINVAL;
>   		break;
>   	case RADEON_SURF_TYPE_1D_ARRAY:
> -		if (surf->npix_y > 1)
> +		if (surf_info->height > 1)
>   			return -EINVAL;
>   		/* fall through */
>   	case RADEON_SURF_TYPE_2D_ARRAY:
> -		if (surf->npix_z > 1)
> +		if (surf_info->depth > 1)
>   			return -EINVAL;
>   		break;
>   	default:
> @@ -158,6 +159,7 @@ ADDR_HANDLE radv_amdgpu_addr_create(struct amdgpu_gpu_info *amdinfo, int family,
>   }
>   
>   static int radv_compute_level(ADDR_HANDLE addrlib,
> +			      const struct radeon_surf_info *surf_info,
>                                 struct radeon_surf *surf, bool is_stencil,
>                                 unsigned level, unsigned type, bool compressed,
>                                 ADDR_COMPUTE_SURFACE_INFO_INPUT *AddrSurfInfoIn,
> @@ -169,15 +171,15 @@ static int radv_compute_level(ADDR_HANDLE addrlib,
>   	ADDR_E_RETURNCODE ret;
>   
>   	AddrSurfInfoIn->mipLevel = level;
> -	AddrSurfInfoIn->width = u_minify(surf->npix_x, level);
> -	AddrSurfInfoIn->height = u_minify(surf->npix_y, level);
> +	AddrSurfInfoIn->width = u_minify(surf_info->width, level);
> +	AddrSurfInfoIn->height = u_minify(surf_info->height, level);
>   
>   	if (type == RADEON_SURF_TYPE_3D)
> -		AddrSurfInfoIn->numSlices = u_minify(surf->npix_z, level);
> +		AddrSurfInfoIn->numSlices = u_minify(surf_info->depth, level);
>   	else if (type == RADEON_SURF_TYPE_CUBEMAP)
>   		AddrSurfInfoIn->numSlices = 6;
>   	else
> -		AddrSurfInfoIn->numSlices = surf->array_size;
> +		AddrSurfInfoIn->numSlices = surf_info->array_size;
>   
>   	if (level > 0) {
>   		/* Set the base level pitch. This is needed for calculation
> @@ -202,9 +204,9 @@ static int radv_compute_level(ADDR_HANDLE addrlib,
>   	surf_level->offset = align64(surf->bo_size, AddrSurfInfoOut->baseAlign);
>   	surf_level->slice_size = AddrSurfInfoOut->sliceSize;
>   	surf_level->pitch_bytes = AddrSurfInfoOut->pitch * (is_stencil ? 1 : surf->bpe);
> -	surf_level->npix_x = u_minify(surf->npix_x, level);
> -	surf_level->npix_y = u_minify(surf->npix_y, level);
> -	surf_level->npix_z = u_minify(surf->npix_z, level);
> +	surf_level->npix_x = u_minify(surf_info->width, level);
> +	surf_level->npix_y = u_minify(surf_info->height, level);
> +	surf_level->npix_z = u_minify(surf_info->depth, level);
>   	surf_level->nblk_x = AddrSurfInfoOut->pitch;
>   	surf_level->nblk_y = AddrSurfInfoOut->height;
>   	if (type == RADEON_SURF_TYPE_3D)
> @@ -312,6 +314,7 @@ static unsigned cik_get_macro_tile_index(struct radeon_surf *surf)
>   }
>   
>   static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
> +					   const struct radeon_surf_info *surf_info,
>   					   struct radeon_surf *surf)
>   {
>   	struct radv_amdgpu_winsys *ws = radv_amdgpu_winsys(_ws);
> @@ -324,8 +327,9 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	ADDR_TILEINFO AddrTileInfoIn = {0};
>   	ADDR_TILEINFO AddrTileInfoOut = {0};
>   	int r;
> +	uint32_t last_level = surf_info->levels - 1;
>   
> -	r = radv_amdgpu_surface_sanity(surf);
> +	r = radv_amdgpu_surface_sanity(surf_info, surf);
>   	if (r)
>   		return r;
>   
> @@ -340,7 +344,7 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	compressed = surf->blk_w == 4 && surf->blk_h == 4;
>   
>   	/* MSAA and FMASK require 2D tiling. */
> -	if (surf->nsamples > 1 ||
> +	if (surf_info->samples > 1 ||
>   	    (surf->flags & RADEON_SURF_FMASK))
>   		mode = RADEON_SURF_MODE_2D;
>   
> @@ -381,7 +385,7 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   		AddrDccIn.bpp = AddrSurfInfoIn.bpp = surf->bpe * 8;
>   	}
>   
> -	AddrDccIn.numSamples = AddrSurfInfoIn.numSamples = surf->nsamples;
> +	AddrDccIn.numSamples = AddrSurfInfoIn.numSamples = surf_info->samples;
>   	AddrSurfInfoIn.tileIndex = -1;
>   
>   	/* Set the micro tile type. */
> @@ -396,7 +400,7 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	AddrSurfInfoIn.flags.depth = (surf->flags & RADEON_SURF_ZBUFFER) != 0;
>   	AddrSurfInfoIn.flags.cube = type == RADEON_SURF_TYPE_CUBEMAP;
>   	AddrSurfInfoIn.flags.display = (surf->flags & RADEON_SURF_SCANOUT) != 0;
> -	AddrSurfInfoIn.flags.pow2Pad = surf->last_level > 0;
> +	AddrSurfInfoIn.flags.pow2Pad = last_level > 0;
>   	AddrSurfInfoIn.flags.opt4Space = 1;
>   
>   	/* DCC notes:
> @@ -408,8 +412,8 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	AddrSurfInfoIn.flags.dccCompatible = !(surf->flags & RADEON_SURF_Z_OR_SBUFFER) &&
>   		!(surf->flags & RADEON_SURF_DISABLE_DCC) &&
>   		!compressed && AddrDccIn.numSamples <= 1 &&
> -		((surf->array_size == 1 && surf->npix_z == 1) ||
> -		 surf->last_level == 0);
> +		((surf_info->array_size == 1 && surf_info->depth == 1) ||
> +		 last_level == 0);
>   
>   	AddrSurfInfoIn.flags.noStencil = (surf->flags & RADEON_SURF_SBUFFER) == 0;
>   	AddrSurfInfoIn.flags.compressZ = AddrSurfInfoIn.flags.depth;
> @@ -421,7 +425,7 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	 * TODO: update addrlib to a newer version, remove this, and
>   	 * use flags.matchStencilTileCfg = 1 as an alternative fix.
>   	 */
> -	if (surf->last_level > 0)
> +	if (last_level > 0)
>   		AddrSurfInfoIn.flags.noStencil = 1;
>   
>   	/* Set preferred macrotile parameters. This is usually required
> @@ -482,8 +486,8 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	surf->htile_alignment = 1;
>   
>   	/* Calculate texture layout information. */
> -	for (level = 0; level <= surf->last_level; level++) {
> -		r = radv_compute_level(ws->addrlib, surf, false, level, type, compressed,
> +	for (level = 0; level <= last_level; level++) {
> +		r = radv_compute_level(ws->addrlib, surf_info, surf, false, level, type, compressed,
>   				       &AddrSurfInfoIn, &AddrSurfInfoOut, &AddrDccIn, &AddrDccOut);
>   		if (r)
>   			break;
> @@ -515,8 +519,8 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   		/* This will be ignored if AddrSurfInfoIn.pTileInfo is NULL. */
>   		AddrTileInfoIn.tileSplitBytes = surf->stencil_tile_split;
>   
> -		for (level = 0; level <= surf->last_level; level++) {
> -			r = radv_compute_level(ws->addrlib, surf, true, level, type, compressed,
> +		for (level = 0; level <= last_level; level++) {
> +			r = radv_compute_level(ws->addrlib, surf_info, surf, true, level, type, compressed,
>   					       &AddrSurfInfoIn, &AddrSurfInfoOut, &AddrDccIn, &AddrDccOut);
>   			if (r)
>   				return r;
> @@ -540,7 +544,7 @@ static int radv_amdgpu_winsys_surface_init(struct radeon_winsys *_ws,
>   	 * complicated.
>   	 */
>   #if 0
> -	if (surf->dcc_size && surf->last_level > 0) {
> +	if (surf->dcc_size && last_level > 0) {
>   		surf->dcc_size = align64(surf->bo_size >> 8,
>   					 ws->info.pipe_interleave_bytes *
>   					 ws->info.num_tile_pipes);
> 


More information about the mesa-dev mailing list