Am 13.12.2017 um 08:23 schrieb Oded Gabbay:
> 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.
FWIW, there is precedent for this type of information in sysfs. See
/sys/devices/virtual/drm/ttm/memory_accounting/kernel/used_memory.
Regards,
 Felix
>
> 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
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20171216/017511d7/attachment.sig>
