From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH 2/2] drm/radeon: add proper support for RADEON_VM_BLOCK_SIZE Date: Thu, 01 May 2014 19:22:05 +0200 Message-ID: <536282BD.6010206@vodafone.de> References: <1398963153-1635-1-git-send-email-deathsimple@vodafone.de> <1398963153-1635-2-git-send-email-deathsimple@vodafone.de> <20140501171751.GA3296@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id 4B1B96EDE6 for ; Thu, 1 May 2014 10:22:20 -0700 (PDT) In-Reply-To: <20140501171751.GA3296@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jerome Glisse Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org > /* 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. No, you're right. That change was unintentional, otherwise I would have = updated the comment above as well. Christian. Am 01.05.2014 19:17, schrieb Jerome Glisse: > On Thu, May 01, 2014 at 06:52:33PM +0200, Christian K=F6nig wrote: >> From: Christian K=F6nig >> >> 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=F6nig > 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_devi= ce *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_d= evice *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/ra= deon.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=E9r=F4me > >> = >> /* 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 *r= dev, >> */ >> int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) >> { >> + const unsigned align =3D 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, struc= t radeon_vm *vm) >> return -ENOMEM; >> } >> = >> - r =3D radeon_bo_create(rdev, pd_size, RADEON_VM_PTB_ALIGN_SIZE, false, >> + r =3D 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_devic= e *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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel