On 29.06.2016 16:22, StDenis, Tom wrote:
> Thanks. I use vmalloc first to avoid putting the array on the stack (as
> good practice) then I chose vmalloc because there really isn't a need
> for contiguous memory and in theory it should be just as fast to
> allocate. Wouldn't you typically use kmalloc when you need contiguous
> memory? Though this all boils down to a single page since it's so small
> anyways.
If it's potentially non-contiguous memory, isn't there the overhead of
page table manipulation?
Maybe somebody who's more familiar with the kernel has an opinion...
Cheers,
Nicolai
>
>
> Tom
>
>
>
>
>
> ------------------------------------------------------------------------
> *From:* Nicolai Hähnle <nhaehnle at gmail.com>
> *Sent:* Wednesday, June 29, 2016 10:20
> *To:* Tom St Denis; amd-gfx at lists.freedesktop.org
> *Cc:* StDenis, Tom
> *Subject:* Re: [PATCH 3/3] drm/amd/amdgpu: Add bank selection for MMIO
> debugfs (v3)
> Patch 1 & 3 are
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Patch 2 looks mostly fine to me, too, but I'm not sure about the
> vmalloc/vfree vs. kmalloc/kfree.
>
> Cheers,
> Nicolai
>
> On 29.06.2016 15:56, Tom St Denis wrote:
>> (v2) Added INSTANCE selector
>> (v3) Changed order of bank selectors
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 35 +++++++++++++++++++++++++++---
>> 1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 7f08ac02f9de..4a8f66c5ff43 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2244,20 +2244,43 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
>> struct amdgpu_device *adev = f->f_inode->i_private;
>> ssize_t result = 0;
>> int r;
>> + bool use_bank;
>> + unsigned instance_bank, sh_bank, se_bank;
>>
>> if (size & 0x3 || *pos & 0x3)
>> return -EINVAL;
>>
>> + if (*pos & (1ULL << 62)) {
>> + se_bank = (*pos >> 24) & 0x3FF;
>> + sh_bank = (*pos >> 34) & 0x3FF;
>> + instance_bank = (*pos >> 44) & 0x3FF;
>> + use_bank = 1;
>> + *pos &= 0xFFFFFF;
>> + } else {
>> + use_bank = 0;
>> + }
>> +
>> + if (use_bank) {
>> + if (sh_bank >= adev->gfx.config.max_sh_per_se ||
>> + se_bank >= adev->gfx.config.max_shader_engines)
>> + return -EINVAL;
>> + mutex_lock(&adev->grbm_idx_mutex);
>> + amdgpu_gfx_select_se_sh(adev, se_bank,
>> + sh_bank, instance_bank);
>> + }
>> +
>> while (size) {
>> uint32_t value;
>>
>> if (*pos > adev->rmmio_size)
>> - return result;
>> + goto end;
>>
>> value = RREG32(*pos >> 2);
>> r = put_user(value, (uint32_t *)buf);
>> - if (r)
>> - return r;
>> + if (r) {
>> + result = r;
>> + goto end;
>> + }
>>
>> result += 4;
>> buf += 4;
>> @@ -2265,6 +2288,12 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
>> size -= 4;
>> }
>>
>> +end:
>> + if (use_bank) {
>> + amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
>> + mutex_unlock(&adev->grbm_idx_mutex);
>> + }
>> +
>> return result;
>> }
>>
>>
