[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <17ef8c7e-f3ab-475f-9379-1e8f631a3ac3@amd.com>
Date: Fri, 5 Dec 2025 16:15:54 +0530
From: "Lazar, Lijo" <lijo.lazar@....com>
To: "Yo-Jung Leo Lin (AMD)" <Leo.Lin@....com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Jonathan Corbet <corbet@....net>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
"Tsao, Anson" <anson.tsao@....com>,
"Mario Limonciello (AMD) (kernel.org)" <superm1@...nel.org>
Subject: Re: [PATCH v5 4/5] drm/amdgpu: add UMA allocation interfaces to sysfs
On 12/5/2025 4:04 PM, Lazar, Lijo wrote:
>
>
> On 12/5/2025 12:20 PM, Yo-Jung Leo Lin (AMD) wrote:
>> Add a uma/ directory containing two sysfs files as interfaces to
>> inspect or change UMA carveout size. These files are:
>>
>> - uma/carveout_options: a read-only file listing all the available
>> UMA allocation options and their index.
>>
>> - uma/carveout: a file that is both readable and writable. On read,
>> it shows the index of the current setting. Writing a valid index
>> into this file allows users to change the UMA carveout size to that
>> option on the next boot.
>>
>> Co-developed-by: Mario Limonciello (AMD) <superm1@...nel.org>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@...nel.org>
>> Reviewed-by: Alex Deucher <alexander.deucher@....com>
>> Signed-off-by: Yo-Jung Leo Lin (AMD) <Leo.Lin@....com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 7 ++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 138 +++++++++++++++++++
>> ++++++++++
>> 3 files changed, 149 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/
>> amd/amdgpu/amdgpu.h
>> index 6bf626a51dfc..177376ff5811 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1846,4 +1846,8 @@ void amdgpu_device_set_uid(struct amdgpu_uid
>> *uid_info,
>> uint64_t uid);
>> uint64_t amdgpu_device_get_uid(struct amdgpu_uid *uid_info,
>> enum amdgpu_uid_type type, uint8_t inst);
>> +
>> +int amdgpu_acpi_uma_option_init(struct amdgpu_device *adev);
>> +void amdgpu_acpi_uma_option_fini(struct amdgpu_device *adev);
>> +
>> #endif
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/
>> drm/amd/amdgpu/amdgpu_acpi.c
>> index bdafcde51107..b2779fc2f712 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
>> @@ -1288,6 +1288,12 @@ static int amdgpu_acpi_event(struct
>> notifier_block *nb,
>> int amdgpu_acpi_init(struct amdgpu_device *adev)
>> {
>> struct amdgpu_atif *atif = &amdgpu_acpi_priv.atif;
>> + int rc;
>> +
>> + rc = amdgpu_acpi_uma_option_init(adev);
>> +
>> + if (rc)
>> + drm_dbg(adev_to_drm(adev), "Not creating uma carveout
>> interfaces: %d", rc);
>> if (atif->notifications.brightness_change) {
>> if (adev->dc_enabled) {
>> @@ -1340,6 +1346,7 @@ void amdgpu_acpi_get_backlight_caps(struct
>> amdgpu_dm_backlight_caps *caps)
>> void amdgpu_acpi_fini(struct amdgpu_device *adev)
>> {
>> unregister_acpi_notifier(&adev->acpi_nb);
>> + amdgpu_acpi_uma_option_fini(adev);
>> }
>> /**
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/
>> drm/amd/amdgpu/amdgpu_device.c
>> index a7594ae44b20..979298d9c213 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -36,6 +36,7 @@
>> #include <linux/pci.h>
>> #include <linux/pci-p2pdma.h>
>> #include <linux/apple-gmux.h>
>> +#include <linux/nospec.h>
>> #include <drm/drm_atomic_helper.h>
>> #include <drm/drm_client_event.h>
>> @@ -7835,3 +7836,140 @@ u64 amdgpu_device_get_uid(struct amdgpu_uid
>> *uid_info,
>> return uid_info->uid[type][inst];
>> }
>> +
>> +static ssize_t carveout_options_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct drm_device *ddev = dev_get_drvdata(dev);
>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>> + uint32_t memory_carved;
>> + ssize_t size = 0;
>> +
>> + if (!uma_info || !uma_info->num_entries)
>> + return -ENODEV;
>> +
>> + for (int i = 0; i < uma_info->num_entries; i++) {
>> + memory_carved = uma_info->entries[i].memory_carved_mb;
>> + if (memory_carved >= SZ_1G/SZ_1M) {
>> + size += sysfs_emit_at(buf, size, "%d: %s (%u GB)\n",
>> + i,
>> + uma_info->entries[i].name,
>> + memory_carved >> 10);
>> + } else {
>> + size += sysfs_emit_at(buf, size, "%d: %s (%u MB)\n",
>> + i,
>> + uma_info->entries[i].name,
>> + memory_carved);
>> + }
>> + }
>> +
>> + return size;
>> +}
>> +static DEVICE_ATTR_RO(carveout_options);
>> +
>> +static ssize_t carveout_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct drm_device *ddev = dev_get_drvdata(dev);
>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>> +
>> + return sysfs_emit(buf, "%u\n", adev->uma_info.uma_option_index);
>> +}
>> +
>> +static ssize_t carveout_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct drm_device *ddev = dev_get_drvdata(dev);
>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>> + struct amdgpu_uma_carveout_option *opt;
>> + unsigned long val;
>> + uint8_t flags;
>> + int r;
>> +
>> + r = kstrtoul(buf, 10, &val);
>> + if (r)
>> + return r;
>> +
>> + if (val >= uma_info->num_entries)
>> + return -EINVAL;
>> +
>> + val = array_index_nospec(val, uma_info->num_entries);
>> + opt = &uma_info->entries[val];
>> +
>> + if (!(opt->flags & AMDGPU_UMA_FLAG_AUTO) &&
>> + !(opt->flags & AMDGPU_UMA_FLAG_CUSTOM)) {
>> + drm_err_once(ddev, "Option %lu not supported due to lack of
>> Custom/Auto flag", val);
>> + return -EINVAL;
>> + }
>> +
>> + flags = opt->flags;
>> + flags &= ~((flags & AMDGPU_UMA_FLAG_AUTO) >> 1);
>> +
>> + guard(mutex)(&uma_info->update_lock);
>> +
>> + r = amdgpu_acpi_set_uma_allocation_size(adev, val, flags);
>> + if (r)
>> + return r;
>> +
>> + uma_info->uma_option_index = val;
>> +
>> + return count;
>> +}
>> +static DEVICE_ATTR_RW(carveout);
>> +
>> +static struct attribute *amdgpu_uma_attrs[] = {
>> + &dev_attr_carveout.attr,
>> + &dev_attr_carveout_options.attr,
>> + NULL
>> +};
>> +
>> +const struct attribute_group amdgpu_uma_attr_group = {
>> + .name = "uma",
>> + .attrs = amdgpu_uma_attrs
>> +};
>> +
>> +int amdgpu_acpi_uma_option_init(struct amdgpu_device *adev)
>> +{
>
> To clarify on the comment about moving this to amdgpu_device -
>
> UMA option is a device specific option which could be set through acpi.
> Options are retrieved through atom tables.
>
> So the function names remain amdgpu_device_uma_option_init/fini() and
> init/fini are called within amdgpu_device.c
>
Couple of additional comments below -
> Thanks,
> Lijo
>
>> + int rc;
>> +
It's better to have an early exit from here itself if it's not APU.
Otherwise, the dbg prints will show up even on dGPUs.
There also needs to be a check for availability of BIOS (adev->bios) and
(adev->is_atom_fw). For example in Instinct APUs, driver doesn't read
bios image (adev->bios is NULL).
Thanks,
Lijo
>> + if (!amdgpu_acpi_is_set_uma_allocation_size_supported())
>> + return -ENODEV;
>> +
>> + rc = amdgpu_atomfirmware_get_uma_carveout_info(adev, &adev-
>> >uma_info);
>> + if (rc) {
>> + drm_dbg(adev_to_drm(adev),
>> + "Failed to parse UMA carveout info from VBIOS: %d\n", rc);
>> + goto out_info;
>> + }
>> +
>> + mutex_init(&adev->uma_info.update_lock);
>> +
>> + rc = devm_device_add_group(adev->dev, &amdgpu_uma_attr_group);
>> + if (rc) {
>> + drm_dbg(adev_to_drm(adev), "Failed to add UMA carveout sysfs
>> interfaces %d\n", rc);
>> + goto out_attr;
>> + }
>> +
>> + return 0;
>> +
>> +out_attr:
>> + mutex_destroy(&adev->uma_info.update_lock);
>> +out_info:
>> + return rc;
>> +}
>> +
>> +void amdgpu_acpi_uma_option_fini(struct amdgpu_device *adev)
>> +{
>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>> +
>> + if (!amdgpu_acpi_is_set_uma_allocation_size_supported())
>> + return;
>> +
>> + mutex_destroy(&uma_info->update_lock);
>> + uma_info->num_entries = 0;
>> +}
>>
>
Powered by blists - more mailing lists