From: "Christian König" <deathsimple@vodafone.de>
To: Jerome Glisse <j.glisse@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/radeon: add proper support for RADEON_VM_BLOCK_SIZE
Date: Thu, 01 May 2014 19:22:05 +0200 [thread overview]
Message-ID: <536282BD.6010206@vodafone.de> (raw)
In-Reply-To: <20140501171751.GA3296@gmail.com>
> /* 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önig wrote:
>> From: Christian König <christian.koenig@amd.com>
>>
>> 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@amd.com>
> 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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2014-05-01 17:22 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-01 16:52 [PATCH 1/2] drm/radeon: add large PTE support for NI, SI and CIK v4 Christian König
2014-05-01 16:52 ` [PATCH 2/2] drm/radeon: add proper support for RADEON_VM_BLOCK_SIZE Christian König
2014-05-01 17:17 ` Jerome Glisse
2014-05-01 17:22 ` Christian König [this message]
2014-05-01 17:29 ` [PATCH 1/2] drm/radeon: add large PTE support for NI, SI and CIK v4 Jay Cornwall
2014-05-02 12:13 ` Christian König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=536282BD.6010206@vodafone.de \
--to=deathsimple@vodafone.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=j.glisse@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox