diff mbox series

[v2,1/2] platform/x86/amd: amd_3d_vcache: Add AMD 3D V-Cache optimizer driver

Message ID 20241021134654.337368-2-Basavaraj.Natikar@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Add support of AMD 3D V-Cache optimizer driver | expand

Commit Message

Basavaraj Natikar Oct. 21, 2024, 1:46 p.m. UTC
AMD X3D processors, also known as AMD 3D V-Cache, feature dual Core
Complex Dies (CCDs) and enlarged L3 cache, enabling dynamic mode
switching between Frequency and Cache modes. To optimize performance,
implement the AMD 3D V-Cache Optimizer, which allows selecting either:

Frequency mode: cores within the faster CCD are prioritized before
those in the slower CCD.

Cache mode: cores within the larger L3 CCD are prioritized before
those in the smaller L3 CCD.

Co-developed-by: Perry Yuan <perry.yuan@amd.com>
Signed-off-by: Perry Yuan <perry.yuan@amd.com>
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 MAINTAINERS                           |   7 ++
 drivers/platform/x86/amd/Kconfig      |  12 ++
 drivers/platform/x86/amd/Makefile     |   2 +
 drivers/platform/x86/amd/x3d_vcache.c | 160 ++++++++++++++++++++++++++
 4 files changed, 181 insertions(+)
 create mode 100644 drivers/platform/x86/amd/x3d_vcache.c

Comments

Armin Wolf Oct. 21, 2024, 2:10 p.m. UTC | #1
Am 21.10.24 um 15:46 schrieb Basavaraj Natikar:

> AMD X3D processors, also known as AMD 3D V-Cache, feature dual Core
> Complex Dies (CCDs) and enlarged L3 cache, enabling dynamic mode
> switching between Frequency and Cache modes. To optimize performance,
> implement the AMD 3D V-Cache Optimizer, which allows selecting either:
>
> Frequency mode: cores within the faster CCD are prioritized before
> those in the slower CCD.
>
> Cache mode: cores within the larger L3 CCD are prioritized before
> those in the smaller L3 CCD.
>
> Co-developed-by: Perry Yuan <perry.yuan@amd.com>
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
>   MAINTAINERS                           |   7 ++
>   drivers/platform/x86/amd/Kconfig      |  12 ++
>   drivers/platform/x86/amd/Makefile     |   2 +
>   drivers/platform/x86/amd/x3d_vcache.c | 160 ++++++++++++++++++++++++++
>   4 files changed, 181 insertions(+)
>   create mode 100644 drivers/platform/x86/amd/x3d_vcache.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9659a5a7fb3..11b829956499 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -965,6 +965,13 @@ Q:	https://patchwork.kernel.org/project/linux-rdma/list/
>   F:	drivers/infiniband/hw/efa/
>   F:	include/uapi/rdma/efa-abi.h
>
> +AMD 3D V-CACHE PERFORMANCE OPTIMIZER DRIVER
> +M:	Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> +R:	Mario Limonciello <mario.limonciello@amd.com>
> +L:	platform-driver-x86@vger.kernel.org
> +S:	Supported
> +F:	drivers/platform/x86/amd/x3d_vcache.c
> +
>   AMD ADDRESS TRANSLATION LIBRARY (ATL)
>   M:	Yazen Ghannam <Yazen.Ghannam@amd.com>
>   L:	linux-edac@vger.kernel.org
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index f88682d36447..d73f691020d0 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -6,6 +6,18 @@
>   source "drivers/platform/x86/amd/pmf/Kconfig"
>   source "drivers/platform/x86/amd/pmc/Kconfig"
>
> +config AMD_3D_VCACHE
> +	tristate "AMD 3D V-Cache Performance Optimizer Driver"
> +	depends on X86_64 && ACPI
> +	help
> +	  The driver provides a sysfs interface, enabling the setting of a bias
> +	  that alters CPU core reordering. This bias prefers cores with higher
> +	  frequencies or larger L3 caches on processors supporting AMD 3D V-Cache
> +	  technology.
> +
> +	  If you choose to compile this driver as a module the module will be
> +	  called amd_3d_vcache.
> +
>   config AMD_HSMP
>   	tristate "AMD HSMP Driver"
>   	depends on AMD_NB && X86_64 && ACPI
> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
> index dcec0a46f8af..16e4cce02242 100644
> --- a/drivers/platform/x86/amd/Makefile
> +++ b/drivers/platform/x86/amd/Makefile
> @@ -4,6 +4,8 @@
>   # AMD x86 Platform-Specific Drivers
>   #
>
> +obj-$(CONFIG_AMD_3D_VCACHE)     += amd_3d_vcache.o
> +amd_3d_vcache-objs              := x3d_vcache.o
>   obj-$(CONFIG_AMD_PMC)		+= pmc/
>   amd_hsmp-y			:= hsmp.o
>   obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
> diff --git a/drivers/platform/x86/amd/x3d_vcache.c b/drivers/platform/x86/amd/x3d_vcache.c
> new file mode 100644
> index 000000000000..ec861e7b5e1e
> --- /dev/null
> +++ b/drivers/platform/x86/amd/x3d_vcache.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD 3D V-Cache Performance Optimizer Driver
> + *
> + * Copyright (c) 2024, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> + *          Perry Yuan <perry.yuan@amd.com>
> + *          Mario Limonciello <mario.limonciello@amd.com>
> + *
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +
> +static char *x3d_mode = "frequency";
> +module_param(x3d_mode, charp, 0444);

Hi,

does userspace really need to know the value of this modparam? If no, then please
change the visibility to 0.

> +MODULE_PARM_DESC(x3d_mode, "Initial 3D-VCache mode; 'frequency' (default) or 'cache'");
> +
> +#define DSM_REVISION_ID			0
> +#define DSM_SET_X3D_MODE		1
> +
> +static guid_t x3d_guid = GUID_INIT(0xdff8e55f, 0xbcfd, 0x46fb, 0xba, 0x0a,
> +				   0xef, 0xd0, 0x45, 0x0f, 0x34, 0xee);
> +
> +enum amd_x3d_mode_type {
> +	MODE_INDEX_FREQ,
> +	MODE_INDEX_CACHE,
> +};
> +
> +static const char * const amd_x3d_mode_strings[] = {
> +	[MODE_INDEX_FREQ] = "frequency",
> +	[MODE_INDEX_CACHE] = "cache",
> +};
> +
> +struct amd_x3d_dev {
> +	struct device *dev;
> +	acpi_handle ahandle;
> +	/* To protect x3d mode setting */
> +	struct mutex lock;
> +	enum amd_x3d_mode_type curr_mode;
> +};
> +
> +static int amd_x3d_mode_switch(struct amd_x3d_dev *data, int new_state)
> +{
> +	union acpi_object *out, argv;
> +
> +	guard(mutex)(&data->lock);
> +	argv.type = ACPI_TYPE_INTEGER;
> +	argv.integer.value = new_state;
> +
> +	out = acpi_evaluate_dsm(data->ahandle, &x3d_guid, DSM_REVISION_ID, DSM_SET_X3D_MODE,
> +				&argv);
> +	if (!out) {
> +		dev_err(data->dev, "failed to evaluate _DSM\n");
> +		return -EINVAL;
> +	}
> +
> +	data->curr_mode = new_state;
> +
> +	kfree(out);
> +
> +	return 0;
> +}
> +
> +static ssize_t amd_x3d_mode_store(struct device *dev, struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	struct amd_x3d_dev *data = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = sysfs_match_string(amd_x3d_mode_strings, buf);
> +	if (ret < 0) {
> +		dev_err(dev, "no matching mode to set %s\n", buf);

Maybe just return ret here without printing anything. Otherwise userspace could spam
the kernel log by continuously writing invalid strings.

> +		return ret;
> +	}
> +
> +	ret = amd_x3d_mode_switch(data, ret);
> +	if (ret < 0)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t amd_x3d_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct amd_x3d_dev *data = dev_get_drvdata(dev);
> +
> +	if (!(data->curr_mode == MODE_INDEX_CACHE || data->curr_mode == MODE_INDEX_FREQ))
> +		return -EINVAL;

Can this error condition even be triggered?

> +
> +	return sysfs_emit(buf, "%s\n", amd_x3d_mode_strings[data->curr_mode]);

Please use data->lock here to protect accesses to data->curr_mode.

> +}
> +static DEVICE_ATTR_RW(amd_x3d_mode);
> +
> +static struct attribute *amd_x3d_attrs[] = {
> +	&dev_attr_amd_x3d_mode.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(amd_x3d);
> +
> +static const struct acpi_device_id amd_x3d_acpi_ids[] = {
> +	{"AMDI0101"},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_x3d_acpi_ids);
> +
> +static int amd_x3d_probe(struct platform_device *pdev)
> +{
> +	struct amd_x3d_dev *data;
> +	acpi_handle handle;
> +	int ret;
> +
> +	handle = ACPI_HANDLE(&pdev->dev);
> +	if (!handle)
> +		return -ENODEV;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->dev = &pdev->dev;
> +	data->ahandle = handle;
> +	platform_set_drvdata(pdev, data);
> +
> +	if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID, BIT(DSM_SET_X3D_MODE)))
> +		return dev_err_probe(&pdev->dev, -ENODEV, "not supported on this platform\n");

Please do this check before allocating anything, because the driver should bail out as soon as possible
when encountering unsupported hardware.

Also maybe being silent and just returning -ENODEV is better as this error message serves no purpose.

> +
> +	ret = match_string(amd_x3d_mode_strings, ARRAY_SIZE(amd_x3d_mode_strings), x3d_mode);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, -EINVAL, "invalid mode %s\n", x3d_mode);
> +
> +	devm_mutex_init(data->dev, &data->lock);
> +
> +	ret = amd_x3d_mode_switch(data, ret);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret, "mode switch failed\n");

amd_x3d_mode_switch() already prints an error when failing, so please just return the result directly:

	return amd_x3d_mode_switch(data, ret);

Other than that the driver look good.

Thanks,
Armin Wolf

> +
> +	return 0;
> +}
> +
> +static struct platform_driver amd_3d_vcache_driver = {
> +	.driver = {
> +		.name = "amd_x3d_vcache",
> +		.dev_groups = amd_x3d_groups,
> +		.acpi_match_table = amd_x3d_acpi_ids,
> +	},
> +	.probe = amd_x3d_probe,
> +};
> +module_platform_driver(amd_3d_vcache_driver);
> +
> +MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
> +MODULE_LICENSE("GPL");
Armin Wolf Oct. 21, 2024, 2:20 p.m. UTC | #2
Am 21.10.24 um 16:10 schrieb Armin Wolf:

> Am 21.10.24 um 15:46 schrieb Basavaraj Natikar:
>
>> AMD X3D processors, also known as AMD 3D V-Cache, feature dual Core
>> Complex Dies (CCDs) and enlarged L3 cache, enabling dynamic mode
>> switching between Frequency and Cache modes. To optimize performance,
>> implement the AMD 3D V-Cache Optimizer, which allows selecting either:
>>
>> Frequency mode: cores within the faster CCD are prioritized before
>> those in the slower CCD.
>>
>> Cache mode: cores within the larger L3 CCD are prioritized before
>> those in the smaller L3 CCD.
>>
>> Co-developed-by: Perry Yuan <perry.yuan@amd.com>
>> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>> ---
>>   MAINTAINERS                           |   7 ++
>>   drivers/platform/x86/amd/Kconfig      |  12 ++
>>   drivers/platform/x86/amd/Makefile     |   2 +
>>   drivers/platform/x86/amd/x3d_vcache.c | 160 ++++++++++++++++++++++++++
>>   4 files changed, 181 insertions(+)
>>   create mode 100644 drivers/platform/x86/amd/x3d_vcache.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e9659a5a7fb3..11b829956499 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -965,6 +965,13 @@ Q:
>> https://patchwork.kernel.org/project/linux-rdma/list/
>>   F:    drivers/infiniband/hw/efa/
>>   F:    include/uapi/rdma/efa-abi.h
>>
>> +AMD 3D V-CACHE PERFORMANCE OPTIMIZER DRIVER
>> +M:    Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>> +R:    Mario Limonciello <mario.limonciello@amd.com>
>> +L:    platform-driver-x86@vger.kernel.org
>> +S:    Supported
>> +F:    drivers/platform/x86/amd/x3d_vcache.c
>> +
>>   AMD ADDRESS TRANSLATION LIBRARY (ATL)
>>   M:    Yazen Ghannam <Yazen.Ghannam@amd.com>
>>   L:    linux-edac@vger.kernel.org
>> diff --git a/drivers/platform/x86/amd/Kconfig
>> b/drivers/platform/x86/amd/Kconfig
>> index f88682d36447..d73f691020d0 100644
>> --- a/drivers/platform/x86/amd/Kconfig
>> +++ b/drivers/platform/x86/amd/Kconfig
>> @@ -6,6 +6,18 @@
>>   source "drivers/platform/x86/amd/pmf/Kconfig"
>>   source "drivers/platform/x86/amd/pmc/Kconfig"
>>
>> +config AMD_3D_VCACHE
>> +    tristate "AMD 3D V-Cache Performance Optimizer Driver"
>> +    depends on X86_64 && ACPI
>> +    help
>> +      The driver provides a sysfs interface, enabling the setting of
>> a bias
>> +      that alters CPU core reordering. This bias prefers cores with
>> higher
>> +      frequencies or larger L3 caches on processors supporting AMD
>> 3D V-Cache
>> +      technology.
>> +
>> +      If you choose to compile this driver as a module the module
>> will be
>> +      called amd_3d_vcache.
>> +
>>   config AMD_HSMP
>>       tristate "AMD HSMP Driver"
>>       depends on AMD_NB && X86_64 && ACPI
>> diff --git a/drivers/platform/x86/amd/Makefile
>> b/drivers/platform/x86/amd/Makefile
>> index dcec0a46f8af..16e4cce02242 100644
>> --- a/drivers/platform/x86/amd/Makefile
>> +++ b/drivers/platform/x86/amd/Makefile
>> @@ -4,6 +4,8 @@
>>   # AMD x86 Platform-Specific Drivers
>>   #
>>
>> +obj-$(CONFIG_AMD_3D_VCACHE)     += amd_3d_vcache.o
>> +amd_3d_vcache-objs              := x3d_vcache.o
>>   obj-$(CONFIG_AMD_PMC)        += pmc/
>>   amd_hsmp-y            := hsmp.o
>>   obj-$(CONFIG_AMD_HSMP)        += amd_hsmp.o
>> diff --git a/drivers/platform/x86/amd/x3d_vcache.c
>> b/drivers/platform/x86/amd/x3d_vcache.c
>> new file mode 100644
>> index 000000000000..ec861e7b5e1e
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/x3d_vcache.c
>> @@ -0,0 +1,160 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * AMD 3D V-Cache Performance Optimizer Driver
>> + *
>> + * Copyright (c) 2024, Advanced Micro Devices, Inc.
>> + * All Rights Reserved.
>> + *
>> + * Authors: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>> + *          Perry Yuan <perry.yuan@amd.com>
>> + *          Mario Limonciello <mario.limonciello@amd.com>
>> + *
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/device.h>
>> +#include <linux/errno.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +
>> +static char *x3d_mode = "frequency";
>> +module_param(x3d_mode, charp, 0444);
>
> Hi,
>
> does userspace really need to know the value of this modparam? If no,
> then please
> change the visibility to 0.
>
>> +MODULE_PARM_DESC(x3d_mode, "Initial 3D-VCache mode; 'frequency'
>> (default) or 'cache'");
>> +
>> +#define DSM_REVISION_ID            0
>> +#define DSM_SET_X3D_MODE        1
>> +
>> +static guid_t x3d_guid = GUID_INIT(0xdff8e55f, 0xbcfd, 0x46fb, 0xba,
>> 0x0a,
>> +                   0xef, 0xd0, 0x45, 0x0f, 0x34, 0xee);
>> +
>> +enum amd_x3d_mode_type {
>> +    MODE_INDEX_FREQ,
>> +    MODE_INDEX_CACHE,
>> +};
>> +
>> +static const char * const amd_x3d_mode_strings[] = {
>> +    [MODE_INDEX_FREQ] = "frequency",
>> +    [MODE_INDEX_CACHE] = "cache",
>> +};
>> +
>> +struct amd_x3d_dev {
>> +    struct device *dev;
>> +    acpi_handle ahandle;
>> +    /* To protect x3d mode setting */
>> +    struct mutex lock;
>> +    enum amd_x3d_mode_type curr_mode;
>> +};
>> +
>> +static int amd_x3d_mode_switch(struct amd_x3d_dev *data, int new_state)
>> +{
>> +    union acpi_object *out, argv;
>> +
>> +    guard(mutex)(&data->lock);
>> +    argv.type = ACPI_TYPE_INTEGER;
>> +    argv.integer.value = new_state;
>> +
>> +    out = acpi_evaluate_dsm(data->ahandle, &x3d_guid,
>> DSM_REVISION_ID, DSM_SET_X3D_MODE,
>> +                &argv);
>> +    if (!out) {
>> +        dev_err(data->dev, "failed to evaluate _DSM\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    data->curr_mode = new_state;
>> +
>> +    kfree(out);
>> +
>> +    return 0;
>> +}
>> +
>> +static ssize_t amd_x3d_mode_store(struct device *dev, struct
>> device_attribute *attr,
>> +                  const char *buf, size_t count)
>> +{
>> +    struct amd_x3d_dev *data = dev_get_drvdata(dev);
>> +    int ret;
>> +
>> +    ret = sysfs_match_string(amd_x3d_mode_strings, buf);
>> +    if (ret < 0) {
>> +        dev_err(dev, "no matching mode to set %s\n", buf);
>
> Maybe just return ret here without printing anything. Otherwise
> userspace could spam
> the kernel log by continuously writing invalid strings.
>
>> +        return ret;
>> +    }
>> +
>> +    ret = amd_x3d_mode_switch(data, ret);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    return count;
>> +}
>> +
>> +static ssize_t amd_x3d_mode_show(struct device *dev, struct
>> device_attribute *attr, char *buf)
>> +{
>> +    struct amd_x3d_dev *data = dev_get_drvdata(dev);
>> +
>> +    if (!(data->curr_mode == MODE_INDEX_CACHE || data->curr_mode ==
>> MODE_INDEX_FREQ))
>> +        return -EINVAL;
>
> Can this error condition even be triggered?
>
>> +
>> +    return sysfs_emit(buf, "%s\n",
>> amd_x3d_mode_strings[data->curr_mode]);
>
> Please use data->lock here to protect accesses to data->curr_mode.
>
>> +}
>> +static DEVICE_ATTR_RW(amd_x3d_mode);
>> +
>> +static struct attribute *amd_x3d_attrs[] = {
>> +    &dev_attr_amd_x3d_mode.attr,
>> +    NULL
>> +};
>> +ATTRIBUTE_GROUPS(amd_x3d);
>> +
>> +static const struct acpi_device_id amd_x3d_acpi_ids[] = {
>> +    {"AMDI0101"},
>> +    { },
>> +};
>> +MODULE_DEVICE_TABLE(acpi, amd_x3d_acpi_ids);
>> +
>> +static int amd_x3d_probe(struct platform_device *pdev)
>> +{
>> +    struct amd_x3d_dev *data;
>> +    acpi_handle handle;
>> +    int ret;
>> +
>> +    handle = ACPI_HANDLE(&pdev->dev);
>> +    if (!handle)
>> +        return -ENODEV;
>> +
>> +    data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> +    if (!data)
>> +        return -ENOMEM;
>> +
>> +    data->dev = &pdev->dev;
>> +    data->ahandle = handle;
>> +    platform_set_drvdata(pdev, data);
>> +
>> +    if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID,
>> BIT(DSM_SET_X3D_MODE)))
>> +        return dev_err_probe(&pdev->dev, -ENODEV, "not supported on
>> this platform\n");
>
> Please do this check before allocating anything, because the driver
> should bail out as soon as possible
> when encountering unsupported hardware.
>
> Also maybe being silent and just returning -ENODEV is better as this
> error message serves no purpose.
>
>> +
>> +    ret = match_string(amd_x3d_mode_strings,
>> ARRAY_SIZE(amd_x3d_mode_strings), x3d_mode);
>> +    if (ret < 0)
>> +        return dev_err_probe(&pdev->dev, -EINVAL, "invalid mode
>> %s\n", x3d_mode);
>> +
>> +    devm_mutex_init(data->dev, &data->lock);
>> +
>> +    ret = amd_x3d_mode_switch(data, ret);
>> +    if (ret < 0)
>> +        return dev_err_probe(&pdev->dev, ret, "mode switch failed\n");
>
> amd_x3d_mode_switch() already prints an error when failing, so please
> just return the result directly:
>
>     return amd_x3d_mode_switch(data, ret);
>
> Other than that the driver look good.
>
> Thanks,
> Armin Wolf

I think i missed something: Maybe the setting should be reapplied after suspend/resume?
You certainly need to do this for S4 and maybe even for S3, depending whether or not this
value is guaranteed to to remain stable during suspend.

Thanks,
Armin Wolf

>
>> +
>> +    return 0;
>> +}
>> +
>> +static struct platform_driver amd_3d_vcache_driver = {
>> +    .driver = {
>> +        .name = "amd_x3d_vcache",
>> +        .dev_groups = amd_x3d_groups,
>> +        .acpi_match_table = amd_x3d_acpi_ids,
>> +    },
>> +    .probe = amd_x3d_probe,
>> +};
>> +module_platform_driver(amd_3d_vcache_driver);
>> +
>> +MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
>> +MODULE_LICENSE("GPL");
>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index e9659a5a7fb3..11b829956499 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -965,6 +965,13 @@  Q:	https://patchwork.kernel.org/project/linux-rdma/list/
 F:	drivers/infiniband/hw/efa/
 F:	include/uapi/rdma/efa-abi.h
 
+AMD 3D V-CACHE PERFORMANCE OPTIMIZER DRIVER
+M:	Basavaraj Natikar <Basavaraj.Natikar@amd.com>
+R:	Mario Limonciello <mario.limonciello@amd.com>
+L:	platform-driver-x86@vger.kernel.org
+S:	Supported
+F:	drivers/platform/x86/amd/x3d_vcache.c
+
 AMD ADDRESS TRANSLATION LIBRARY (ATL)
 M:	Yazen Ghannam <Yazen.Ghannam@amd.com>
 L:	linux-edac@vger.kernel.org
diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index f88682d36447..d73f691020d0 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -6,6 +6,18 @@ 
 source "drivers/platform/x86/amd/pmf/Kconfig"
 source "drivers/platform/x86/amd/pmc/Kconfig"
 
+config AMD_3D_VCACHE
+	tristate "AMD 3D V-Cache Performance Optimizer Driver"
+	depends on X86_64 && ACPI
+	help
+	  The driver provides a sysfs interface, enabling the setting of a bias
+	  that alters CPU core reordering. This bias prefers cores with higher
+	  frequencies or larger L3 caches on processors supporting AMD 3D V-Cache
+	  technology.
+
+	  If you choose to compile this driver as a module the module will be
+	  called amd_3d_vcache.
+
 config AMD_HSMP
 	tristate "AMD HSMP Driver"
 	depends on AMD_NB && X86_64 && ACPI
diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
index dcec0a46f8af..16e4cce02242 100644
--- a/drivers/platform/x86/amd/Makefile
+++ b/drivers/platform/x86/amd/Makefile
@@ -4,6 +4,8 @@ 
 # AMD x86 Platform-Specific Drivers
 #
 
+obj-$(CONFIG_AMD_3D_VCACHE)     += amd_3d_vcache.o
+amd_3d_vcache-objs              := x3d_vcache.o
 obj-$(CONFIG_AMD_PMC)		+= pmc/
 amd_hsmp-y			:= hsmp.o
 obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
diff --git a/drivers/platform/x86/amd/x3d_vcache.c b/drivers/platform/x86/amd/x3d_vcache.c
new file mode 100644
index 000000000000..ec861e7b5e1e
--- /dev/null
+++ b/drivers/platform/x86/amd/x3d_vcache.c
@@ -0,0 +1,160 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD 3D V-Cache Performance Optimizer Driver
+ *
+ * Copyright (c) 2024, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
+ *          Perry Yuan <perry.yuan@amd.com>
+ *          Mario Limonciello <mario.limonciello@amd.com>
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+static char *x3d_mode = "frequency";
+module_param(x3d_mode, charp, 0444);
+MODULE_PARM_DESC(x3d_mode, "Initial 3D-VCache mode; 'frequency' (default) or 'cache'");
+
+#define DSM_REVISION_ID			0
+#define DSM_SET_X3D_MODE		1
+
+static guid_t x3d_guid = GUID_INIT(0xdff8e55f, 0xbcfd, 0x46fb, 0xba, 0x0a,
+				   0xef, 0xd0, 0x45, 0x0f, 0x34, 0xee);
+
+enum amd_x3d_mode_type {
+	MODE_INDEX_FREQ,
+	MODE_INDEX_CACHE,
+};
+
+static const char * const amd_x3d_mode_strings[] = {
+	[MODE_INDEX_FREQ] = "frequency",
+	[MODE_INDEX_CACHE] = "cache",
+};
+
+struct amd_x3d_dev {
+	struct device *dev;
+	acpi_handle ahandle;
+	/* To protect x3d mode setting */
+	struct mutex lock;
+	enum amd_x3d_mode_type curr_mode;
+};
+
+static int amd_x3d_mode_switch(struct amd_x3d_dev *data, int new_state)
+{
+	union acpi_object *out, argv;
+
+	guard(mutex)(&data->lock);
+	argv.type = ACPI_TYPE_INTEGER;
+	argv.integer.value = new_state;
+
+	out = acpi_evaluate_dsm(data->ahandle, &x3d_guid, DSM_REVISION_ID, DSM_SET_X3D_MODE,
+				&argv);
+	if (!out) {
+		dev_err(data->dev, "failed to evaluate _DSM\n");
+		return -EINVAL;
+	}
+
+	data->curr_mode = new_state;
+
+	kfree(out);
+
+	return 0;
+}
+
+static ssize_t amd_x3d_mode_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct amd_x3d_dev *data = dev_get_drvdata(dev);
+	int ret;
+
+	ret = sysfs_match_string(amd_x3d_mode_strings, buf);
+	if (ret < 0) {
+		dev_err(dev, "no matching mode to set %s\n", buf);
+		return ret;
+	}
+
+	ret = amd_x3d_mode_switch(data, ret);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t amd_x3d_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct amd_x3d_dev *data = dev_get_drvdata(dev);
+
+	if (!(data->curr_mode == MODE_INDEX_CACHE || data->curr_mode == MODE_INDEX_FREQ))
+		return -EINVAL;
+
+	return sysfs_emit(buf, "%s\n", amd_x3d_mode_strings[data->curr_mode]);
+}
+static DEVICE_ATTR_RW(amd_x3d_mode);
+
+static struct attribute *amd_x3d_attrs[] = {
+	&dev_attr_amd_x3d_mode.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(amd_x3d);
+
+static const struct acpi_device_id amd_x3d_acpi_ids[] = {
+	{"AMDI0101"},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, amd_x3d_acpi_ids);
+
+static int amd_x3d_probe(struct platform_device *pdev)
+{
+	struct amd_x3d_dev *data;
+	acpi_handle handle;
+	int ret;
+
+	handle = ACPI_HANDLE(&pdev->dev);
+	if (!handle)
+		return -ENODEV;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = &pdev->dev;
+	data->ahandle = handle;
+	platform_set_drvdata(pdev, data);
+
+	if (!acpi_check_dsm(handle, &x3d_guid, DSM_REVISION_ID, BIT(DSM_SET_X3D_MODE)))
+		return dev_err_probe(&pdev->dev, -ENODEV, "not supported on this platform\n");
+
+	ret = match_string(amd_x3d_mode_strings, ARRAY_SIZE(amd_x3d_mode_strings), x3d_mode);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, -EINVAL, "invalid mode %s\n", x3d_mode);
+
+	devm_mutex_init(data->dev, &data->lock);
+
+	ret = amd_x3d_mode_switch(data, ret);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "mode switch failed\n");
+
+	return 0;
+}
+
+static struct platform_driver amd_3d_vcache_driver = {
+	.driver = {
+		.name = "amd_x3d_vcache",
+		.dev_groups = amd_x3d_groups,
+		.acpi_match_table = amd_x3d_acpi_ids,
+	},
+	.probe = amd_x3d_probe,
+};
+module_platform_driver(amd_3d_vcache_driver);
+
+MODULE_DESCRIPTION("AMD 3D V-Cache Performance Optimizer Driver");
+MODULE_LICENSE("GPL");