Re: [PATCH 2/2] drm/radeon: add proper support for RADEON_VM_BLOCK_SIZE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 01, 2014 at 06:52:33PM +0200, Christian König wrote:
> From: Christian König <christian.koenig@xxxxxxx>
> 
> This patch makes it possible to decide how many address
> bits are spend on the page directory vs the page tables.
> 
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>

NAK on the basis you are modifying default behavior and hardware
programming in same patch see below.

> ---
>  drivers/gpu/drm/radeon/cik.c       | 1 +
>  drivers/gpu/drm/radeon/cikd.h      | 1 +
>  drivers/gpu/drm/radeon/ni.c        | 1 +
>  drivers/gpu/drm/radeon/nid.h       | 1 +
>  drivers/gpu/drm/radeon/radeon.h    | 2 +-
>  drivers/gpu/drm/radeon/radeon_vm.c | 4 +++-
>  drivers/gpu/drm/radeon/si.c        | 1 +
>  drivers/gpu/drm/radeon/sid.h       | 1 +
>  8 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 38da9f3..f11a9ab 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -5445,6 +5445,7 @@ static int cik_pcie_gart_enable(struct radeon_device *rdev)
>  	       (u32)(rdev->dummy_page.addr >> 12));
>  	WREG32(VM_CONTEXT1_CNTL2, 4);
>  	WREG32(VM_CONTEXT1_CNTL, ENABLE_CONTEXT | PAGE_TABLE_DEPTH(1) |
> +				PAGE_TABLE_BLOCK_SIZE(RADEON_VM_BLOCK_SIZE - 9) |
>  				RANGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
>  				RANGE_PROTECTION_FAULT_ENABLE_DEFAULT |
>  				DUMMY_PAGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
> diff --git a/drivers/gpu/drm/radeon/cikd.h b/drivers/gpu/drm/radeon/cikd.h
> index dd79263..ae88660 100644
> --- a/drivers/gpu/drm/radeon/cikd.h
> +++ b/drivers/gpu/drm/radeon/cikd.h
> @@ -482,6 +482,7 @@
>  #define		READ_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 16)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT		(1 << 18)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 19)
> +#define		PAGE_TABLE_BLOCK_SIZE(x)			(((x) & 0xF) << 24)
>  #define VM_CONTEXT1_CNTL				0x1414
>  #define VM_CONTEXT0_CNTL2				0x1430
>  #define VM_CONTEXT1_CNTL2				0x1434
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index 5e8db9b..1d3209f 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1268,6 +1268,7 @@ static int cayman_pcie_gart_enable(struct radeon_device *rdev)
>  	       (u32)(rdev->dummy_page.addr >> 12));
>  	WREG32(VM_CONTEXT1_CNTL2, 4);
>  	WREG32(VM_CONTEXT1_CNTL, ENABLE_CONTEXT | PAGE_TABLE_DEPTH(1) |
> +				PAGE_TABLE_BLOCK_SIZE(RADEON_VM_BLOCK_SIZE - 9) |
>  				RANGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
>  				RANGE_PROTECTION_FAULT_ENABLE_DEFAULT |
>  				DUMMY_PAGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
> diff --git a/drivers/gpu/drm/radeon/nid.h b/drivers/gpu/drm/radeon/nid.h
> index d996033..2e12e4d 100644
> --- a/drivers/gpu/drm/radeon/nid.h
> +++ b/drivers/gpu/drm/radeon/nid.h
> @@ -128,6 +128,7 @@
>  #define		READ_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 16)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT		(1 << 18)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 19)
> +#define		PAGE_TABLE_BLOCK_SIZE(x)			(((x) & 0xF) << 24)
>  #define VM_CONTEXT1_CNTL				0x1414
>  #define VM_CONTEXT0_CNTL2				0x1430
>  #define VM_CONTEXT1_CNTL2				0x1434
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index e3d6be3..2d80cf2 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -838,7 +838,7 @@ struct radeon_mec {
>  /* defines number of bits in page table versus page directory,
>   * a page is 4KB so we have 12 bits offset, 9 bits in the page
>   * table and the remaining 19 bits are in the page directory */
> -#define RADEON_VM_BLOCK_SIZE   9
> +#define RADEON_VM_BLOCK_SIZE   12

I would first do a patch that add all the code to set this bit inside the
vm context registers and then a patch that change the default current
value explaining why.

This would allow bisection to either find that the register setting is
guilty or that the default value change is the one introducing problem

But really i am being picky here.

Cheers,
Jérôme

>  
>  /* number of entries in page table */
>  #define RADEON_VM_PTE_COUNT (1 << RADEON_VM_BLOCK_SIZE)
> diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c
> index 6bf656e..3900ecf 100644
> --- a/drivers/gpu/drm/radeon/radeon_vm.c
> +++ b/drivers/gpu/drm/radeon/radeon_vm.c
> @@ -964,6 +964,8 @@ void radeon_vm_bo_invalidate(struct radeon_device *rdev,
>   */
>  int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm)
>  {
> +	const unsigned align = min(RADEON_VM_PTB_ALIGN_SIZE,
> +		RADEON_VM_PTE_COUNT * 8);
>  	unsigned pd_size, pd_entries, pts_size;
>  	int r;
>  
> @@ -985,7 +987,7 @@ int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm)
>  		return -ENOMEM;
>  	}
>  
> -	r = radeon_bo_create(rdev, pd_size, RADEON_VM_PTB_ALIGN_SIZE, false,
> +	r = radeon_bo_create(rdev, pd_size, align, false,
>  			     RADEON_GEM_DOMAIN_VRAM, NULL,
>  			     &vm->page_directory);
>  	if (r)
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index dece3be..8fa6be5 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -4095,6 +4095,7 @@ static int si_pcie_gart_enable(struct radeon_device *rdev)
>  	       (u32)(rdev->dummy_page.addr >> 12));
>  	WREG32(VM_CONTEXT1_CNTL2, 4);
>  	WREG32(VM_CONTEXT1_CNTL, ENABLE_CONTEXT | PAGE_TABLE_DEPTH(1) |
> +				PAGE_TABLE_BLOCK_SIZE(RADEON_VM_BLOCK_SIZE - 9) |
>  				RANGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
>  				RANGE_PROTECTION_FAULT_ENABLE_DEFAULT |
>  				DUMMY_PAGE_PROTECTION_FAULT_ENABLE_INTERRUPT |
> diff --git a/drivers/gpu/drm/radeon/sid.h b/drivers/gpu/drm/radeon/sid.h
> index 683532f..da8f867 100644
> --- a/drivers/gpu/drm/radeon/sid.h
> +++ b/drivers/gpu/drm/radeon/sid.h
> @@ -362,6 +362,7 @@
>  #define		READ_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 16)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_INTERRUPT		(1 << 18)
>  #define		WRITE_PROTECTION_FAULT_ENABLE_DEFAULT		(1 << 19)
> +#define		PAGE_TABLE_BLOCK_SIZE(x)			(((x) & 0xF) << 24)
>  #define VM_CONTEXT1_CNTL				0x1414
>  #define VM_CONTEXT0_CNTL2				0x1430
>  #define VM_CONTEXT1_CNTL2				0x1434
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux