On Tue, Dec 12, 2017 at 1:11 PM, Russell, Kent <Kent.Russell at amd.com> wrote: > That's alright. I admit that it was a bit self-serving in that I was asked to get something somewhere to get the information out, and it was the simplest solution. I can see if I can come up with a more acceptable option in a future patch set, but for now I think Felix is right in that we can just drop this one for now, it's definitely not worth holding up the rest of the patches over. > > Kent Sure, np. I'll drop this for now. Oded > > -----Original Message----- > From: Kuehling, Felix > Sent: Monday, December 11, 2017 2:52 PM > To: Koenig, Christian; Oded Gabbay > Cc: Russell, Kent; amd-gfx list > Subject: Re: [PATCH 28/37] drm/amdkfd: Add support for displaying VRAM usage > > On 2017-12-11 12:28 PM, Christian König wrote: >> Am 11.12.2017 um 17:40 schrieb Oded Gabbay: >>> On Mon, Dec 11, 2017 at 5:32 PM, Oded Gabbay <oded.gabbay at gmail.com> >>> wrote: >>>> On Sat, Dec 9, 2017 at 6:09 AM, Felix Kuehling >>>> <Felix.Kuehling at amd.com> wrote: >>>>> From: Kent Russell <kent.russell at amd.com> >>>>> >>>>> Add a sysfs file in topology (node/x/memory_banks/X/used_memory) >>>>> that reports the current VRAM usage for that node. Only works for >>>>> GPU nodes at this time. >>>>> >>>> As with patch 22 (perf counters), I would not expect this >>>> information to be included in the topology. It doesn't describe the >>>> properties of the device, but a current state. >>>> Oded >>> For example, in amdgpu, the VRAM usage is reported in the INFO IOCTL >>> (AMDGPU_INFO_VRAM_USAGE). See function amdgpu_info_ioctl() >> >> Yep, completely agree. >> >> That stuff is runtime properties and not static attribute nor >> configuration or setup. >> >> So either debugfs or IOCTL are the two best options as far as I can see. > > Right. I admit, this feature was a bit of a hack to quickly enable the HIP team without having to change a bunch of interfaces (ioctls, Thunk, and Runtime). > > This patch isn't critical for enabling dGPU support. I'll drop it for now and we can reimplement it properly later. > > Regards, > Felix > >> >> Christian. >> >>> >>> Thanks, >>> >>> Oded >>> >>> >>>>> Signed-off-by: Kent Russell <kent.russell at amd.com> >>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com> >>>>> --- >>>>> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 49 >>>>> +++++++++++++++++++++++++++---- >>>>> drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 4 ++- >>>>> 2 files changed, 46 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>>>> index 7f0d41e..7f04038 100644 >>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c >>>>> @@ -186,6 +186,8 @@ struct kfd_topology_device >>>>> *kfd_create_topology_device( >>>>> sysfs_show_gen_prop(buffer, "%s %llu\n", name, >>>>> value) >>>>> #define sysfs_show_32bit_val(buffer, value) \ >>>>> sysfs_show_gen_prop(buffer, "%u\n", value) >>>>> +#define sysfs_show_64bit_val(buffer, value) \ >>>>> + sysfs_show_gen_prop(buffer, "%llu\n", value) >>>>> #define sysfs_show_str_val(buffer, value) \ >>>>> sysfs_show_gen_prop(buffer, "%s\n", value) >>>>> >>>>> @@ -268,11 +270,23 @@ static ssize_t mem_show(struct kobject *kobj, >>>>> struct attribute *attr, >>>>> { >>>>> ssize_t ret; >>>>> struct kfd_mem_properties *mem; >>>>> + uint64_t used_mem; >>>>> >>>>> /* Making sure that the buffer is an empty string */ >>>>> buffer[0] = 0; >>>>> >>>>> - mem = container_of(attr, struct kfd_mem_properties, attr); >>>>> + if (strcmp(attr->name, "used_memory") == 0) { >>>>> + mem = container_of(attr, struct kfd_mem_properties, >>>>> attr_used); >>>>> + if (mem->gpu) { >>>>> + used_mem = >>>>> +mem->gpu->kfd2kgd->get_vram_usage( >>>>> + >>>>> mem->gpu->kgd); >>>>> + return sysfs_show_64bit_val(buffer, >>>>> +used_mem); >>>>> + } >>>>> + /* TODO: Report APU/CPU-allocated memory; For now >>>>> return 0 */ >>>>> + return 0; >>>>> + } >>>>> + >>>>> + mem = container_of(attr, struct kfd_mem_properties, >>>>> attr_props); >>>>> sysfs_show_32bit_prop(buffer, "heap_type", >>>>> mem->heap_type); >>>>> sysfs_show_64bit_prop(buffer, "size_in_bytes", >>>>> mem->size_in_bytes); >>>>> sysfs_show_32bit_prop(buffer, "flags", mem->flags); @@ >>>>> -527,7 +541,12 @@ static void kfd_remove_sysfs_node_entry(struct >>>>> kfd_topology_device *dev) >>>>> if (dev->kobj_mem) { >>>>> list_for_each_entry(mem, &dev->mem_props, list) >>>>> if (mem->kobj) { >>>>> - kfd_remove_sysfs_file(mem->kobj, >>>>> &mem->attr); >>>>> + /* TODO: Remove when CPU/APU >>>>> supported */ >>>>> + if (dev->node_props.cpu_cores_count >>>>> == 0) >>>>> + >>>>> +sysfs_remove_file(mem->kobj, >>>>> + >>>>> &mem->attr_used); >>>>> + kfd_remove_sysfs_file(mem->kobj, >>>>> + &mem->attr_props); >>>>> mem->kobj = NULL; >>>>> } >>>>> kobject_del(dev->kobj_mem); @@ -629,12 +648,23 @@ >>>>> static int kfd_build_sysfs_node_entry(struct kfd_topology_device >>>>> *dev, >>>>> if (ret < 0) >>>>> return ret; >>>>> >>>>> - mem->attr.name = "properties"; >>>>> - mem->attr.mode = KFD_SYSFS_FILE_MODE; >>>>> - sysfs_attr_init(&mem->attr); >>>>> - ret = sysfs_create_file(mem->kobj, &mem->attr); >>>>> + mem->attr_props.name = "properties"; >>>>> + mem->attr_props.mode = KFD_SYSFS_FILE_MODE; >>>>> + sysfs_attr_init(&mem->attr_props); >>>>> + ret = sysfs_create_file(mem->kobj, >>>>> +&mem->attr_props); >>>>> if (ret < 0) >>>>> return ret; >>>>> + >>>>> + /* TODO: Support APU/CPU memory usage */ >>>>> + if (dev->node_props.cpu_cores_count == 0) { >>>>> + mem->attr_used.name = "used_memory"; >>>>> + mem->attr_used.mode = KFD_SYSFS_FILE_MODE; >>>>> + sysfs_attr_init(&mem->attr_used); >>>>> + ret = sysfs_create_file(mem->kobj, >>>>> &mem->attr_used); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + } >>>>> + >>>>> i++; >>>>> } >>>>> >>>>> @@ -1075,15 +1105,22 @@ static struct kfd_topology_device >>>>> *kfd_assign_gpu(struct kfd_dev *gpu) >>>>> { >>>>> struct kfd_topology_device *dev; >>>>> struct kfd_topology_device *out_dev = NULL; >>>>> + struct kfd_mem_properties *mem; >>>>> >>>>> down_write(&topology_lock); >>>>> list_for_each_entry(dev, &topology_device_list, list) >>>>> if (!dev->gpu && (dev->node_props.simd_count > 0)) >>>>> { >>>>> dev->gpu = gpu; >>>>> out_dev = dev; >>>>> + >>>>> + /* Assign mem->gpu */ >>>>> + list_for_each_entry(mem, &dev->mem_props, >>>>> list) >>>>> + mem->gpu = dev->gpu; >>>>> + >>>>> break; >>>>> } >>>>> up_write(&topology_lock); >>>>> + >>>>> return out_dev; >>>>> } >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h >>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h >>>>> index 53fca1f..0f698d8 100644 >>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h >>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h >>>>> @@ -93,7 +93,9 @@ struct kfd_mem_properties { >>>>> uint32_t width; >>>>> uint32_t mem_clk_max; >>>>> struct kobject *kobj; >>>>> - struct attribute attr; >>>>> + struct kfd_dev *gpu; >>>>> + struct attribute attr_props; >>>>> + struct attribute attr_used; >>>>> }; >>>>> >>>>> #define HSA_CACHE_TYPE_DATA 0x00000001 >>>>> -- >>>>> 2.7.4 >>>>> >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx at lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> >