[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Lazar, Lijo <[email protected]> > Sent: Friday, December 5, 2025 6:34 PM > To: Lin, Leo <[email protected]>; Deucher, Alexander > <[email protected]>; Koenig, Christian > <[email protected]>; David Airlie <[email protected]>; Simona > Vetter <[email protected]>; Maarten Lankhorst > <[email protected]>; Maxime Ripard > <[email protected]>; Thomas Zimmermann <[email protected]>; > Jonathan Corbet <[email protected]> > Cc: [email protected]; [email protected]; linux- > [email protected]; [email protected]; Tsao, Anson > <[email protected]>; Mario Limonciello (AMD) (kernel.org) > <[email protected]> > Subject: Re: [PATCH v5 4/5] drm/amdgpu: add UMA allocation interfaces to > sysfs > > > > 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) <[email protected]> > > Signed-off-by: Mario Limonciello (AMD) <[email protected]> > > Reviewed-by: Alex Deucher <[email protected]> > > Signed-off-by: Yo-Jung Leo Lin (AMD) <[email protected]> > > --- > > 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
Hi Lijo, Originally, I got the idea of putting init/fini in amdgpu_acpi_init/fini() from previous discussion[1], but now I see your point that amdgpu_device.c is a more suitable place for handling things that need both ACPI and atom at the same time. If both you and Alex agree with moving this into amdgpu_device.c I'm glad to make that change in the next version. [1] https://lore.kernel.org/all/cadnq5_ps7pfseo84hvpfbevjqrfbxheact0w35xvjng0cjr...@mail.gmail.com/ Best, Leo > > Thanks, > Lijo > > > + int rc; > > + > > + 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; > > +} > >
