* [syzbot] [wpan?] [input?] [usb?] memory leak in hwsim_add_one (2)
From: syzbot @ 2023-09-22 1:55 UTC (permalink / raw)
To: alex.aring, davem, edumazet, kuba, linux-input, linux-kernel,
linux-usb, linux-wpan, miquel.raynal, netdev, pabeni, stefan,
syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: e789286468a9 Merge tag 'x86-urgent-2023-09-17' of git://gi..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16db487fa80000
kernel config: https://syzkaller.appspot.com/x/.config?x=943a94479fa8e863
dashboard link: https://syzkaller.appspot.com/bug?extid=d2aa0f55c4ae66a9b75d
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15cc8372680000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/60bec5b60566/disk-e7892864.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/509a449f2ff0/vmlinux-e7892864.xz
kernel image: https://storage.googleapis.com/syzbot-assets/36581da19789/bzImage-e7892864.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+d2aa0f55c4ae66a9b75d@syzkaller.appspotmail.com
BUG: memory leak
unreferenced object 0xffff8881042a8940 (size 64):
comm "swapper/0", pid 1, jiffies 4294937901 (age 1085.750s)
hex dump (first 32 bytes):
00 0d 00 00 00 00 00 00 ff ff ff ff 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff81573dc5>] kmalloc_trace+0x25/0x90 mm/slab_common.c:1114
[<ffffffff831b5a6a>] kmalloc include/linux/slab.h:599 [inline]
[<ffffffff831b5a6a>] kzalloc include/linux/slab.h:720 [inline]
[<ffffffff831b5a6a>] hwsim_add_one+0x14a/0x650 drivers/net/ieee802154/mac802154_hwsim.c:949
[<ffffffff831b5f93>] hwsim_probe+0x23/0xe0 drivers/net/ieee802154/mac802154_hwsim.c:1022
[<ffffffff82c14f93>] platform_probe+0x83/0x110 drivers/base/platform.c:1404
[<ffffffff82c10fc6>] call_driver_probe drivers/base/dd.c:579 [inline]
[<ffffffff82c10fc6>] really_probe+0x126/0x440 drivers/base/dd.c:658
[<ffffffff82c113a3>] __driver_probe_device+0xc3/0x190 drivers/base/dd.c:800
[<ffffffff82c1149a>] driver_probe_device+0x2a/0x120 drivers/base/dd.c:830
[<ffffffff82c117f7>] __driver_attach drivers/base/dd.c:1216 [inline]
[<ffffffff82c117f7>] __driver_attach+0x107/0x1f0 drivers/base/dd.c:1156
[<ffffffff82c0e2f3>] bus_for_each_dev+0xb3/0x110 drivers/base/bus.c:368
[<ffffffff82c0fdf6>] bus_add_driver+0x126/0x2a0 drivers/base/bus.c:673
[<ffffffff82c12ee5>] driver_register+0x85/0x180 drivers/base/driver.c:246
[<ffffffff8756e3a6>] hwsim_init_module+0xc6/0x110 drivers/net/ieee802154/mac802154_hwsim.c:1073
[<ffffffff81001cb6>] do_one_initcall+0x76/0x430 init/main.c:1232
[<ffffffff874d76ea>] do_initcall_level init/main.c:1294 [inline]
[<ffffffff874d76ea>] do_initcalls init/main.c:1310 [inline]
[<ffffffff874d76ea>] do_basic_setup init/main.c:1329 [inline]
[<ffffffff874d76ea>] kernel_init_freeable+0x25a/0x460 init/main.c:1547
[<ffffffff84b3628b>] kernel_init+0x1b/0x290 init/main.c:1437
[<ffffffff81149e35>] ret_from_fork+0x45/0x50 arch/x86/kernel/process.c:147
BUG: memory leak
unreferenced object 0xffff8881042a8780 (size 64):
comm "swapper/0", pid 1, jiffies 4294937902 (age 1085.740s)
hex dump (first 32 bytes):
00 0d 00 00 00 00 00 00 ff ff ff ff 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff81573dc5>] kmalloc_trace+0x25/0x90 mm/slab_common.c:1114
[<ffffffff831b5a6a>] kmalloc include/linux/slab.h:599 [inline]
[<ffffffff831b5a6a>] kzalloc include/linux/slab.h:720 [inline]
[<ffffffff831b5a6a>] hwsim_add_one+0x14a/0x650 drivers/net/ieee802154/mac802154_hwsim.c:949
[<ffffffff831b5fb6>] hwsim_probe+0x46/0xe0 drivers/net/ieee802154/mac802154_hwsim.c:1022
[<ffffffff82c14f93>] platform_probe+0x83/0x110 drivers/base/platform.c:1404
[<ffffffff82c10fc6>] call_driver_probe drivers/base/dd.c:579 [inline]
[<ffffffff82c10fc6>] really_probe+0x126/0x440 drivers/base/dd.c:658
[<ffffffff82c113a3>] __driver_probe_device+0xc3/0x190 drivers/base/dd.c:800
[<ffffffff82c1149a>] driver_probe_device+0x2a/0x120 drivers/base/dd.c:830
[<ffffffff82c117f7>] __driver_attach drivers/base/dd.c:1216 [inline]
[<ffffffff82c117f7>] __driver_attach+0x107/0x1f0 drivers/base/dd.c:1156
[<ffffffff82c0e2f3>] bus_for_each_dev+0xb3/0x110 drivers/base/bus.c:368
[<ffffffff82c0fdf6>] bus_add_driver+0x126/0x2a0 drivers/base/bus.c:673
[<ffffffff82c12ee5>] driver_register+0x85/0x180 drivers/base/driver.c:246
[<ffffffff8756e3a6>] hwsim_init_module+0xc6/0x110 drivers/net/ieee802154/mac802154_hwsim.c:1073
[<ffffffff81001cb6>] do_one_initcall+0x76/0x430 init/main.c:1232
[<ffffffff874d76ea>] do_initcall_level init/main.c:1294 [inline]
[<ffffffff874d76ea>] do_initcalls init/main.c:1310 [inline]
[<ffffffff874d76ea>] do_basic_setup init/main.c:1329 [inline]
[<ffffffff874d76ea>] kernel_init_freeable+0x25a/0x460 init/main.c:1547
[<ffffffff84b3628b>] kernel_init+0x1b/0x290 init/main.c:1437
[<ffffffff81149e35>] ret_from_fork+0x45/0x50 arch/x86/kernel/process.c:147
BUG: memory leak
unreferenced object 0xffff8881007cc000 (size 768):
comm "udevd", pid 4480, jiffies 4295045154 (age 13.270s)
hex dump (first 32 bytes):
01 00 00 00 03 00 00 00 08 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff83e8ff95>] alloc_inode_sb include/linux/fs.h:2909 [inline]
[<ffffffff83e8ff95>] sock_alloc_inode+0x25/0x90 net/socket.c:308
[<ffffffff816c39d3>] alloc_inode+0x23/0x100 fs/inode.c:259
[<ffffffff816c4bf6>] new_inode_pseudo+0x16/0x50 fs/inode.c:1004
[<ffffffff83e8f2ab>] sock_alloc+0x1b/0x90 net/socket.c:634
[<ffffffff83e8f8cd>] __sock_create+0xbd/0x2e0 net/socket.c:1516
[<ffffffff83e92d58>] sock_create net/socket.c:1603 [inline]
[<ffffffff83e92d58>] __sys_socket_create net/socket.c:1640 [inline]
[<ffffffff83e92d58>] __sys_socket+0xb8/0x1a0 net/socket.c:1691
[<ffffffff83e92e5b>] __do_sys_socket net/socket.c:1705 [inline]
[<ffffffff83e92e5b>] __se_sys_socket net/socket.c:1703 [inline]
[<ffffffff83e92e5b>] __x64_sys_socket+0x1b/0x20 net/socket.c:1703
[<ffffffff84b30fc8>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<ffffffff84b30fc8>] do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
[<ffffffff84c0008b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
BUG: memory leak
unreferenced object 0xffff88810c7019a0 (size 32):
comm "udevd", pid 4480, jiffies 4295045154 (age 13.270s)
hex dump (first 32 bytes):
b8 c1 7c 00 81 88 ff ff 70 3d 34 82 ff ff ff ff ..|.....p=4.....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff823457a2>] kmem_cache_zalloc include/linux/slab.h:710 [inline]
[<ffffffff823457a2>] lsm_inode_alloc security/security.c:633 [inline]
[<ffffffff823457a2>] security_inode_alloc+0x32/0xd0 security/security.c:1494
[<ffffffff816c01ad>] inode_init_always+0x1ed/0x230 fs/inode.c:230
[<ffffffff816c39f0>] alloc_inode+0x40/0x100 fs/inode.c:266
[<ffffffff816c4bf6>] new_inode_pseudo+0x16/0x50 fs/inode.c:1004
[<ffffffff83e8f2ab>] sock_alloc+0x1b/0x90 net/socket.c:634
[<ffffffff83e8f8cd>] __sock_create+0xbd/0x2e0 net/socket.c:1516
[<ffffffff83e92d58>] sock_create net/socket.c:1603 [inline]
[<ffffffff83e92d58>] __sys_socket_create net/socket.c:1640 [inline]
[<ffffffff83e92d58>] __sys_socket+0xb8/0x1a0 net/socket.c:1691
[<ffffffff83e92e5b>] __do_sys_socket net/socket.c:1705 [inline]
[<ffffffff83e92e5b>] __se_sys_socket net/socket.c:1703 [inline]
[<ffffffff83e92e5b>] __x64_sys_socket+0x1b/0x20 net/socket.c:1703
[<ffffffff84b30fc8>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<ffffffff84b30fc8>] do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
[<ffffffff84c0008b>] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH v7] HID: mcp2200: added driver for GPIOs of MCP2200
From: Rahul Rameshbabu @ 2023-09-22 4:32 UTC (permalink / raw)
To: Johannes Roith
Cc: jikos, andi.shyti, benjamin.tissoires, christophe.jaillet,
linux-input, linux-kernel, rdunlap
In-Reply-To: <20230921164928.170383-1-johannes@gnu-linux.rocks>
On Thu, 21 Sep, 2023 18:49:28 +0200 "Johannes Roith" <johannes@gnu-linux.rocks> wrote:
> Added a gpiochip compatible driver to control the 8 GPIOs of
> the MCP2200 by using the HID interface.
>
> Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
> GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
> default).
>
> The driver was tested while also using the UART of the chip. Setting
> and reading the GPIOs has no effect on the UART communication. However,
> a reset is triggered after the CONFIGURE command. If the GPIO Direction
> is constantly changed, this will affect the communication at low baud
> rates. This is a hardware problem of the MCP2200 and is not caused by
> the driver.
>
> Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
> ---
Reviewed-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: yang tylor @ 2023-09-22 7:56 UTC (permalink / raw)
To: Conor Dooley
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, hbarnor, jingyliang@chromium.org
In-Reply-To: <20230919-cc4646dbfb953bd34e05658c@fedora>
On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > Hi Conor,
> >
> > > > Additional optional arguments:
> > > > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime
> > > > conditions.
> >
> > > Runtime conditions? Aren't thєse properties of the panel & therefore
> > > fixed? If they were runtime conditions, then setting them statically in
> > > your DT is not going to work, right?
> >
> > Because each platform's display driver ready time is different. TP part
> > need to avoid this timing by measuring the waveform of LCD reset pin
> > low period and TP probe timing. For example, if LCD rst pin low from
> > timestamp 100 to 800, TP driver probe at 600. TP probe will fail. Then
> > user should set ic-det-delay-ms bigger than 200, to avoid LCD rst low
> > timing. As you can see, the timing needs to be measured at runtime to
> > decide how long it should be. Then, if the condition is not changed, the
> > value could keep the same.
>
> That sounds to me like something you would test once for a given
> platform and then the values are static. If you are actually changing it
> at *runtime*, how is doing it through DT suitable? Does your firmware do
> the tests & then set the values in DT dynamically?
>
Yes, you are right. I'll change the description.
> >
> > > It looks like you deleted all of the properties from the previous
> > > submission of these changes. I don't really understand that, it kinda
> > > feels just like appeasement, as you must have needed those properties
> > > to do the firmware loading etc. How are you filling the gap those
> > > properties have left, when you still only have a single compatible
> > > string in thㄟs binding? Is there a way to do runtime detection of which
> > > chip you're dealing with that you are now using?
> >
> > After reviewing, I found the properties could go to IC driver settings :
> > "himax,heatmap_16bits" because it depends on IC's ability;
>
> How do you detect the IC's abilities?
>
The driver code has a part of IC detect process, and each IC has its own
driver code to define its abilities. This part moves to that position.
> > Some
> > could remove and use default values: "himax,fw_size",
> > "himax,boot_time_fw_upgrade". "himax,fw_size" has a default value in
> > IC settings, and likely won't change in this IC.
>
> Okay.
>
> > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > should be removed. "himax,fw_in_flash", I use the kernel config for
> > user to select.
>
> That seems like a bad idea, we want to be able to build one kernel that
> works for all hardware at the same time.
>
I see, so I should take that back?
I'll explain more about it.
> > "himax,pid" could be remove and use default firmware name
> > "himax_i2chid.bin" to load. It was added because users may desire to
> > choose a special name like "himax_i2chid_{pid}.bin" instead of the default
> > one.
> > It also could be replaced with newly added "himax",id-gpios" which is still
> > experimental.
>
> Also, pleae don't top post, but instead reply in-line with my comments,
> as I have done here.
>
Ok.
> > Btw, I encounter an error of patch [2/2], which says:
> > BOUNCE linux-input@vger.kernel.org: Message too long (>100000 chars)
> > and the patch didn't appear at patchwork.kernel.org. What should I do to
> > deal with this problem?
>
> No idea. Maybe try to split it into multiple patches?
> The other option is to also cc patches@lists.linux.dev as that has some
> higher capacities, but that's not going to be a silver bullet.
Thanks for the reply. I'll try multiple commits to reduce the size.
Thanks,
Tylor
^ permalink raw reply
* [RESEND PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-09-22 8:37 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230922083801.3056724-1-quic_fenglinw@quicinc.com>
Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
1 file changed, 73 insertions(+), 49 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..d6b468324c77 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+#define SSBL_VIB_DRV_REG 0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT 3
+
+#define SPMI_VIB_DRV_REG 0x41
+#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT 0
+
+#define SPMI_VIB_EN_REG 0x46
+#define SPMI_VIB_EN_BIT BIT(7)
+
#define VIB_MAX_LEVEL_mV (3100)
#define VIB_MIN_LEVEL_mV (1200)
#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
#define MAX_FF_SPEED 0xff
-struct pm8xxx_regs {
- unsigned int enable_addr;
- unsigned int enable_mask;
+enum vib_hw_type {
+ SSBI_VIB,
+ SPMI_VIB,
+};
- unsigned int drv_addr;
- unsigned int drv_mask;
- unsigned int drv_shift;
- unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+ enum vib_hw_type hw_type;
+ unsigned int enable_addr;
+ unsigned int drv_addr;
};
-static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
- .drv_shift = 3,
- .drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+ .hw_type = SSBI_VIB,
+ .drv_addr = SSBL_VIB_DRV_REG,
};
-static struct pm8xxx_regs pm8916_regs = {
- .enable_addr = 0xc046,
- .enable_mask = BIT(7),
- .drv_addr = 0xc041,
- .drv_mask = 0x1F,
- .drv_shift = 0,
- .drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+ .hw_type = SPMI_VIB,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_DRV_REG,
};
/**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
* @vib_input_dev: input device supporting force feedback
* @work: work structure to set the vibration parameters
* @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
struct input_dev *vib_input_dev;
struct work_struct work;
struct regmap *regmap;
- const struct pm8xxx_regs *regs;
+ const struct pm8xxx_vib_data *data;
+ unsigned int reg_base;
int speed;
int level;
bool active;
@@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- const struct pm8xxx_regs *regs = vib->regs;
+ u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
+ u32 shift = SPMI_VIB_DRV_SHIFT;
+
+ if (vib->data->hw_type == SSBI_VIB) {
+ mask = SSBI_VIB_DRV_LEVEL_MASK;
+ shift = SSBI_VIB_DRV_SHIFT;
+ }
if (on)
- val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+ val |= (vib->level << shift) & mask;
else
- val &= ~regs->drv_mask;
+ val &= ~mask;
- rc = regmap_write(vib->regmap, regs->drv_addr, val);
+ rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, val);
if (rc < 0)
return rc;
vib->reg_vib_drv = val;
- if (regs->enable_mask)
- rc = regmap_update_bits(vib->regmap, regs->enable_addr,
- regs->enable_mask, on ? ~0 : 0);
+ if (vib->data->hw_type == SSBI_VIB)
+ return 0;
- return rc;
+ mask = SPMI_VIB_EN_BIT;
+ val = on ? SPMI_VIB_EN_BIT : 0;
+ return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
}
/**
@@ -102,13 +119,6 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
static void pm8xxx_work_handler(struct work_struct *work)
{
struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
- const struct pm8xxx_regs *regs = vib->regs;
- int rc;
- unsigned int val;
-
- rc = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (rc < 0)
- return;
/*
* pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
{
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
+ const struct pm8xxx_vib_data *data;
int error;
- unsigned int val;
- const struct pm8xxx_regs *regs;
+ unsigned int val, reg_base;
vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
if (!vib)
@@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
- regs = of_device_get_match_data(&pdev->dev);
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data)
+ return -EINVAL;
- /* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (error < 0)
- return error;
+ if (data->hw_type != SSBI_VIB) {
+ error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
+ if (error < 0) {
+ dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+ return error;
+ }
+
+ vib->reg_base += reg_base;
+ }
- val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
if (error < 0)
return error;
- vib->regs = regs;
+ /* operate in manual mode */
+ if (data->hw_type == SSBI_VIB) {
+ val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+ error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+ if (error < 0)
+ return error;
+ }
+
+ vib->data = data;
vib->reg_vib_drv = val;
input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
static const struct of_device_id pm8xxx_vib_id_table[] = {
- { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
+ { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* [RESEND PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-09-22 8:38 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input, devicetree
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar,
Krzysztof Kozlowski
In-Reply-To: <20230922083801.3056724-1-quic_fenglinw@quicinc.com>
Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
.../bindings/input/qcom,pm8xxx-vib.yaml | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index c8832cd0d7da..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,18 @@ maintainers:
properties:
compatible:
- enum:
- - qcom,pm8058-vib
- - qcom,pm8916-vib
- - qcom,pm8921-vib
+ oneOf:
+ - enum:
+ - qcom,pm8058-vib
+ - qcom,pm8916-vib
+ - qcom,pm8921-vib
+ - qcom,pmi632-vib
+ - items:
+ - enum:
+ - qcom,pm7250b-vib
+ - qcom,pm7325b-vib
+ - qcom,pm7550ba-vib
+ - const: qcom,pmi632-vib
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [RESEND PATCH v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu @ 2023-09-22 8:38 UTC (permalink / raw)
To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
agross, andersson, dmitry.baryshkov, Konrad Dybcio,
Dmitry Torokhov, linux-input
Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar,
Luca Weiss
In-Reply-To: <20230922083801.3056724-1-quic_fenglinw@quicinc.com>
Add new SPMI vibrator module which is very similar to the SPMI vibrator
module inside PM8916 but just has a finer drive voltage step (1mV vs
100mV) hence its drive level control is expanded to across 2 registers.
The vibrator module can be found in Qualcomm PMIC PMI632, then following
PM7250B, PM7325B, PM7550BA PMICs.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
---
drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
1 file changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index d6b468324c77..990e8a9ac018 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -21,6 +21,13 @@
#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
#define SPMI_VIB_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV_REG 0x40
+#define SPMI_VIB_GEN2_DRV_MASK GENMASK(7, 0)
+#define SPMI_VIB_GEN2_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV2_REG 0x41
+#define SPMI_VIB_GEN2_DRV2_MASK GENMASK(3, 0)
+#define SPMI_VIB_GEN2_DRV2_SHIFT 8
+
#define SPMI_VIB_EN_REG 0x46
#define SPMI_VIB_EN_BIT BIT(7)
@@ -33,12 +40,14 @@
enum vib_hw_type {
SSBI_VIB,
SPMI_VIB,
+ SPMI_VIB_GEN2
};
struct pm8xxx_vib_data {
enum vib_hw_type hw_type;
unsigned int enable_addr;
unsigned int drv_addr;
+ unsigned int drv2_addr;
};
static const struct pm8xxx_vib_data ssbi_vib_data = {
@@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
.drv_addr = SPMI_VIB_DRV_REG,
};
+static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
+ .hw_type = SPMI_VIB_GEN2,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_GEN2_DRV_REG,
+ .drv2_addr = SPMI_VIB_GEN2_DRV2_REG,
+};
+
/**
* struct pm8xxx_vib - structure to hold vibrator data
* @vib_input_dev: input device supporting force feedback
@@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
- u32 shift = SPMI_VIB_DRV_SHIFT;
+ u32 mask, shift;
- if (vib->data->hw_type == SSBI_VIB) {
+
+ switch (vib->data->hw_type) {
+ case SSBI_VIB:
mask = SSBI_VIB_DRV_LEVEL_MASK;
shift = SSBI_VIB_DRV_SHIFT;
+ break;
+ case SPMI_VIB:
+ mask = SPMI_VIB_DRV_LEVEL_MASK;
+ shift = SPMI_VIB_DRV_SHIFT;
+ break;
+ case SPMI_VIB_GEN2:
+ mask = SPMI_VIB_GEN2_DRV_MASK;
+ shift = SPMI_VIB_GEN2_DRV_SHIFT;
+ break;
+ default:
+ return -EINVAL;
}
if (on)
@@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
vib->reg_vib_drv = val;
+ if (vib->data->hw_type == SPMI_VIB_GEN2) {
+ mask = SPMI_VIB_GEN2_DRV2_MASK;
+ shift = SPMI_VIB_GEN2_DRV2_SHIFT;
+ if (on)
+ val = (vib->level >> shift) & mask;
+ else
+ val = 0;
+ rc = regmap_update_bits(vib->regmap,
+ vib->reg_base + vib->data->drv2_addr, mask, val);
+ if (rc < 0)
+ return rc;
+ }
+
if (vib->data->hw_type == SSBI_VIB)
return 0;
@@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
vib->active = true;
vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
VIB_MIN_LEVEL_mV;
- vib->level /= 100;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
} else {
vib->active = false;
- vib->level = VIB_MIN_LEVEL_mV / 100;
+ vib->level = VIB_MIN_LEVEL_mV;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
}
pm8xxx_vib_set(vib, vib->active);
@@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
{ .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
+ { .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Johan Hovold @ 2023-09-22 9:08 UTC (permalink / raw)
To: Doug Anderson
Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, Maxime Ripard, Dmitry Torokhov, LinusW, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM
In-Reply-To: <CAD=FV=XK87TZuPy+d2r2g5QhowmghE-m9pGHe9-X7jnXAw9z1g@mail.gmail.com>
On Wed, Sep 20, 2023 at 08:41:12AM -0700, Doug Anderson wrote:
> On Wed, Sep 20, 2023 at 12:26 AM Johan Hovold <johan@kernel.org> wrote:
> > On Tue, Sep 19, 2023 at 11:15:46AM -0700, Doug Anderson wrote:
> > > On Tue, Sep 19, 2023 at 12:07 AM Johan Hovold <johan@kernel.org> wrote:
> > As I alluded to in the commit message, you probably want to be able to
> > support second-source touchscreen panel followers as well at some point
> > and then deferring checking whether device is populated until the panel
> > is powered on is not going to work.
>
> Yeah, I've been pondering this too. I _think_ it would work OK-ish if
> both devices probed and then only one of the two would actually make
> the sub-HID devices. So you'd actually see both devices succeed at
> probing but only one of them would actually be functional. It's a bit
> ugly, though. :( Maybe marginally better would be if we could figure
> out how to have the device which fails to get its interrupt later
> unbind itself, if that's possible...
>
> The only other thought I had would be to have the parent i2c bus
> understand that it had children that were panel followers, which it
> should be able to do by seeing the "panel" attribute in their device
> tree. Then the i2c bus could itself register as a panel follower and
> could wait to probe its children until they were powered on. This
> could happen in the i2c core so we didn't have to add code like this
> to all i2c bus drivers. ...and, if necessary, we could add this to
> other busses like SPI. It feels a little awkward but could work.
There may be other device on the bus that have nothing to do with
panels, but I guess you mean that this would only apply to a subset of
the children. In any case, this feels like a hack and layering
violation.
> > I skimmed the thread were you added this, but I'm not sure I saw any
> > reason for why powering on the panel follower temporarily during probe
> > would not work?
>
> My first instinct says we can't do this, but let's think about it...
>
> In general the "panel follower" API is designed to give all the
> decision making about when to power things on and off to the panel
> driver, which is controlled by DRM.
>
> The reason for this is from experience I had when dealing with the
> Samsung ATNA33XC20 panel that's on "sc7180-trogdor-homestar". The TCON
> on that panel tended to die if you didn't sequence it just right.
> Specifically, if you were sending pixels to the panel and then stopped
> then you absolutely needed to power the panel off and on again. Folks
> I talked to even claimed that the panel was working "to spec" since,
> in the "Power Sequencing" section of the eDP spec it clearly shows
> that you _must_ turn the panel off and on again after you stop giving
> it bits. ...this is despite the fact that no other panel I've worked
> with cares. ;-)
>
> On homestar, since we didn't have the "panel follower" API, we ended
> up adding cost to the hardware and putting the panel and touchscreens
> on different power rails. However, I wanted to make sure that if we
> ran into a similar situation in the future (or maybe if we were trying
> to make hardware work that we didn't have control over) that we could
> solve it.
>
> The other reason for giving full control to the panel driver is just
> how userspace usually works. Right now userspace tends to power off
> panels if they're not used (like if a lid is closed on a laptop) but
> doesn't necessarily power off the touchscreen. Thus if the touchscreen
> has the ability to keep things powered on then we'd never get to a low
> power state.
Don't you need to keep the touchscreen powered to support wakeup events
(e.g. when not closing the lid)?
And if you close the lid with wakeup disabled, you should still be able
to power down the touchscreen as part of suspend, right?
> The above all explains why panel followers like the touchscreen
> shouldn't be able to keep power on. However, you are specifically
> suggesting that we just turn the power on temporarily during probe. As
> I think about that, it might be possible? I guess you'd have to
> temporarily block DRM from changing the state of the panel while the
> touchscreen is probing. Then if the panel was off then you'd turn it
> on briefly, do your probe, and then turn it off again. If the panel
> was on then by blocking DRM you'd ensure that it stayed on. I'm not
> sure how palatable that would be or if there are any other tricky
> parts I'm not thinking about.
As this would allow actually probing the touchscreen during probe(), as
the driver model expects, this seems like a better approach then
deferring probe and registration if it's at all doable.
> > > Thinking that way, is there any reason you can't just move the
> > > i2c_hid_init_irq() into __do_i2c_hid_core_initial_power_up()? You
> > > could replace the call to enable_irq() with it and then remove the
> > > `IRQF_NO_AUTOEN` flag? I think that would also solve the issue if you
> > > wanted to use a 2nd source + the panel follower concept? Both devices
> > > would probe, but only one of them would actually grab the interrupt
> > > and only one of them would actually create real HID devices. We might
> > > need to do some work to keep from trying again at every poweron of the
> > > panel, but it would probably be workable? I think this would also be a
> > > smaller change...
> >
> > That was my first idea as well, but conceptually it is more correct to
> > request resources at probe time and not at some later point when you can
> > no longer fail probe.
> >
> > You'd also need to handle the fact that the interrupt may never have
> > been requested when remove() is called, which adds unnecessary
> > complexity.
>
> I don't think it's a lot of complexity, is it? Just an extra "if" statement...
Well you'd need keep track of whether the interrupt has been requested
or not (and manage serialisation) yourself for a start.
But the main reason is still that requesting resources belongs in
probe() and should not be deferred to some later random time where you
cannot inform driver core of failures (e.g. for probe deferral if the
interrupt controller is not yet available).
Johan
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-09-22 9:22 UTC (permalink / raw)
To: yang tylor
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, hbarnor, jingyliang@chromium.org
In-Reply-To: <CAGD2q_bkTpvXiomWb_yerNjQfMVKOctYgBqF_RBSo_jYqyyyxw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 764 bytes --]
On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > user to select.
> >
> > That seems like a bad idea, we want to be able to build one kernel that
> > works for all hardware at the same time.
> >
> I see, so I should take that back?
> I'll explain more about it.
Are there particular ICs where the firmware would always be in flash and
others where it would never be? Or is this a choice made by the board or
system designer?
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: yang tylor @ 2023-09-22 9:43 UTC (permalink / raw)
To: Conor Dooley
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, hbarnor, jingyliang@chromium.org
In-Reply-To: <20230922-unclothed-bottom-5531329f9724@spud>
On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
>
> > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > user to select.
> > >
> > > That seems like a bad idea, we want to be able to build one kernel that
> > > works for all hardware at the same time.
> > >
> > I see, so I should take that back?
> > I'll explain more about it.
>
> Are there particular ICs where the firmware would always be in flash and
> others where it would never be? Or is this a choice made by the board or
> system designer?
>
Most cases it's about the system designer's decision. But some ICs may be forced
to use flash because of its architecture(multiple IC inside, need to
load firmware to
multiple IC's sram by master IC). But if there is no limitation on
this part, most system
designers will prefer flashless.
> Thanks,
> Conor.
Thanks,
Tylor
^ permalink raw reply
* Re: [RFC PATCH] of: device: Support 2nd sources of probeable but undiscoverable devices
From: Rob Herring @ 2023-09-22 14:14 UTC (permalink / raw)
To: Douglas Anderson
Cc: Krzysztof Kozlowski, Conor Dooley, devicetree, Benjamin Tissoires,
Chen-Yu Tsai, linux-input, Jiri Kosina, Hsin-Yi Wang, linux-gpio,
linus.walleij, Dmitry Torokhov, Johan Hovold, andriy.shevchenko,
broonie, frowand.list, gregkh, hdegoede, james.clark, james,
keescook, linux-kernel, petr.tesarik.ext, rafael, tglx
In-Reply-To: <20230921102420.RFC.1.I9dddd99ccdca175e3ceb1b9fa1827df0928c5101@changeid>
On Thu, Sep 21, 2023 at 12:26 PM Douglas Anderson <dianders@chromium.org> wrote:
>
> Support for multiple "equivalent" sources for components (also known
> as second sourcing components) is a standard practice that helps keep
> cost down and also makes sure that if one component is unavailable due
> to a shortage that we don't need to stop production for the whole
> product.
>
> Some components are very easy to second source. eMMC, for instance, is
> fully discoverable and probable so you can stuff a wide variety of
> similar eMMC chips on your board and things will work without a hitch.
>
> Some components are more difficult to second source, specifically
> because it's difficult for software to probe what component is present
> on any given board. In cases like this software is provided
> supplementary information to help it, like a GPIO strap or a SKU ID
> programmed into an EEPROM. This helpful information can allow the
> bootloader to select a different device tree. The various different
> "SKUs" of different Chromebooks are examples of this.
>
> Some components are somewhere in between. These in-between components
> are the subject of this patch. Specifically, these components are
> easily "probeable" but not easily "discoverable".
>
> A good example of a probeable but undiscoverable device is an
> i2c-connected touchscreen or trackpad. Two separate components may be
> electrically compatible with each other and may have compatible power
> sequencing requirements but may require different software. If
> software is told about the different possible components (because it
> can't discover them), it can safely probe them to figure out which
> ones are present.
>
> On systems using device tree, if we want to tell the OS about all of
> the different components we need to list them all in the device
> tree. This leads to a problem. The multiple sources for components
> likely use the same resources (GPIOs, interrupts, regulators). If the
> OS tries to probe all of these components at the same time then it
> will detect a resource conflict and that's a fatal error.
>
> The fact that Linux can't handle these probeable but undiscoverable
> devices well has had a few consequences:
> 1. In some cases, we've abandoned the idea of second sourcing
> components for a given board, which increases cost / generates
> manufacturing headaches.
> 2. In some cases, we've been forced to add some sort of strapping /
> EEPROM to indicate which component is present. This adds difficulty
> to manufacturing / refurb processes.
> 3. In some cases, we've managed to make things work by the skin of our
> teeth through slightly hacky solutions. Specifically, if we remove
> the "pinctrl" entry from the various options then it won't
> conflict. Regulators inherently can have more than one consumer, so
> as long as there are no GPIOs involved in power sequencing and
> probing devices then things can work. This is how
> "sc8280xp-lenovo-thinkpad-x13s" works and also how
> "mt8173-elm-hana" works.
>
> Let's attempt to do something better. Specifically, we'll allow
> tagging nodes in the device tree as mutually exclusive from one
> another. This says that only one of the components in this group is
> present on any given board. To make it concrete, in my proposal this
> looks like:
>
> / {
> tp_ex_group: trackpad-exclusion-group {
> };
Interesting way to just get a unique identifier. But it could be any
phandle not used by another group. So just point all the devices in a
group to one of the devices in the group.
> };
>
> &i2c_bus {
> tp1: trackpad@10 {
> ...
> mutual-exclusion-group = <&tp_ex_group>;
> };
> tp2: trackpad@20 {
> ...
> mutual-exclusion-group = <&tp_ex_group>;
> };
> tp3: trackpad@30 {
> ...
> mutual-exclusion-group = <&tp_ex_group>;
> };
> };
>
> In Linux, we can make things work by simply only probing one of the
> devices in the group at a time. We can make a mutex per group and
> enforce locking that mutex around probe. If the first device that gets
> the mutex fails to probe then it won't try again. If it succeeds then
> it will acquire the shared resources and future devices (which we know
> can't be present) will fail to get the shared resources. Future
> patches could quiet down errors about failing to acquire shared
> resources or failing to probe if a device is in a
> mutual-exclusion-group.
This seems like overkill to me. Do we really need groups and a mutex
for each group? Worst case is what? 2-3 groups of 2-3 devices?
Instead, what about extending "status" with another value
("fail-needs-probe"? (fail-xxx is a documented value)). Currently, the
kernel would just ignore nodes with that status. Then we can process
those nodes separately 1-by-1. You may just have to change "status"
via a changeset as there's already some support in some buses (I2C,
SPI IIRC) for new devices showing up with overlays. I'm not really a
fan of adding the probe mutex and would prefer if we can serialize
this with just controlling "status". The challenge at that level is
knowing if/when you have probed especially if we have to wait on
modules to load. But if we must serialize with a mutex, with 1 group
it could be a global mutex and a 1 bit flag in struct device instead.
Rob
^ permalink raw reply
* Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device
From: Conor Dooley @ 2023-09-22 15:31 UTC (permalink / raw)
To: yang tylor
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, jikos,
benjamin.tissoires, linux-input, devicetree, linux-kernel,
poyuan_chang, hbarnor, jingyliang@chromium.org
In-Reply-To: <CAGD2q_YsFdDVhE4JCmQSGMWOdpe_yzG8-CdWYPXtjeZsManvgQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]
On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> >
> > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > user to select.
> > > >
> > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > works for all hardware at the same time.
> > > >
> > > I see, so I should take that back?
> > > I'll explain more about it.
> >
> > Are there particular ICs where the firmware would always be in flash and
> > others where it would never be? Or is this a choice made by the board or
> > system designer?
> >
> Most cases it's about the system designer's decision. But some ICs may be forced
> to use flash because of its architecture(multiple IC inside, need to
> load firmware to
> multiple IC's sram by master IC). But if there is no limitation on
> this part, most system
> designers will prefer flashless.
Forgive me if I am not understanding correctly, there are some ICs that
will need to load the firmware from flash and there are some where it
will be a decision made by the designer of the board. Is the flash part
of the IC or is it an external flash chip?
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: fix handling of unpopulated devices
From: Doug Anderson @ 2023-09-22 16:37 UTC (permalink / raw)
To: Johan Hovold
Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, Maxime Ripard, Dmitry Torokhov, LinusW, Rob Herring,
Krzysztof Kozlowski, Conor Dooley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:GPIO SUBSYSTEM
In-Reply-To: <ZQ1Zm6ec9NuBvqpl@hovoldconsulting.com>
Hi,
On Fri, Sep 22, 2023 at 2:08 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Sep 20, 2023 at 08:41:12AM -0700, Doug Anderson wrote:
> > On Wed, Sep 20, 2023 at 12:26 AM Johan Hovold <johan@kernel.org> wrote:
> > > On Tue, Sep 19, 2023 at 11:15:46AM -0700, Doug Anderson wrote:
> > > > On Tue, Sep 19, 2023 at 12:07 AM Johan Hovold <johan@kernel.org> wrote:
>
> > > As I alluded to in the commit message, you probably want to be able to
> > > support second-source touchscreen panel followers as well at some point
> > > and then deferring checking whether device is populated until the panel
> > > is powered on is not going to work.
> >
> > Yeah, I've been pondering this too. I _think_ it would work OK-ish if
> > both devices probed and then only one of the two would actually make
> > the sub-HID devices. So you'd actually see both devices succeed at
> > probing but only one of them would actually be functional. It's a bit
> > ugly, though. :( Maybe marginally better would be if we could figure
> > out how to have the device which fails to get its interrupt later
> > unbind itself, if that's possible...
> >
> > The only other thought I had would be to have the parent i2c bus
> > understand that it had children that were panel followers, which it
> > should be able to do by seeing the "panel" attribute in their device
> > tree. Then the i2c bus could itself register as a panel follower and
> > could wait to probe its children until they were powered on. This
> > could happen in the i2c core so we didn't have to add code like this
> > to all i2c bus drivers. ...and, if necessary, we could add this to
> > other busses like SPI. It feels a little awkward but could work.
>
> There may be other device on the bus that have nothing to do with
> panels, but I guess you mean that this would only apply to a subset of
> the children. In any case, this feels like a hack and layering
> violation.
Right, the idea would be to only do this for the subset of children
that are panel followers.
It definitely doesn't seem ideal, but it also didn't seem too terrible to me.
> > > I skimmed the thread were you added this, but I'm not sure I saw any
> > > reason for why powering on the panel follower temporarily during probe
> > > would not work?
> >
> > My first instinct says we can't do this, but let's think about it...
> >
> > In general the "panel follower" API is designed to give all the
> > decision making about when to power things on and off to the panel
> > driver, which is controlled by DRM.
> >
> > The reason for this is from experience I had when dealing with the
> > Samsung ATNA33XC20 panel that's on "sc7180-trogdor-homestar". The TCON
> > on that panel tended to die if you didn't sequence it just right.
> > Specifically, if you were sending pixels to the panel and then stopped
> > then you absolutely needed to power the panel off and on again. Folks
> > I talked to even claimed that the panel was working "to spec" since,
> > in the "Power Sequencing" section of the eDP spec it clearly shows
> > that you _must_ turn the panel off and on again after you stop giving
> > it bits. ...this is despite the fact that no other panel I've worked
> > with cares. ;-)
> >
> > On homestar, since we didn't have the "panel follower" API, we ended
> > up adding cost to the hardware and putting the panel and touchscreens
> > on different power rails. However, I wanted to make sure that if we
> > ran into a similar situation in the future (or maybe if we were trying
> > to make hardware work that we didn't have control over) that we could
> > solve it.
> >
> > The other reason for giving full control to the panel driver is just
> > how userspace usually works. Right now userspace tends to power off
> > panels if they're not used (like if a lid is closed on a laptop) but
> > doesn't necessarily power off the touchscreen. Thus if the touchscreen
> > has the ability to keep things powered on then we'd never get to a low
> > power state.
>
> Don't you need to keep the touchscreen powered to support wakeup events
> (e.g. when not closing the lid)?
No. The only reason you'd use panel follower is if the hardware was
designed such that the touchscreen needed to be power sequenced with
the panel. If the touchscreen can stay powered when the panel is off
then it is, by definition, not a panel follower.
For a laptop I don't think most people expect the touchscreen to stay
powered when the screen is off. I certainly wouldn't expect it. If the
screen was off and I wanted to interact with the device, I would hit a
key on the keyboard or touch the trackpad. When the people designing
sc7180-trogdor chose to have the display and touchscreen share a power
rail they made a conscious choice that they didn't need the
touchscreen active when the screen was off.
For the other hardware I'm aware of that needs panel-follower there is
a single external chip on the board that handles driving the panel and
the touchscreen. The power sequencing requirements for this chip
simply don't allow the touchscreen to be powered on while the display
is off.
One use case where I could intuitively think I might touch a
touchscreen of a screen that was "off" would be a kiosk of some sort.
It would make sense there to have two power rails. ...or, I suppose,
userspace could just choose to turn the backlight off but keep the
screen (and touchscreen) powered.
> And if you close the lid with wakeup disabled, you should still be able
> to power down the touchscreen as part of suspend, right?
>
> > The above all explains why panel followers like the touchscreen
> > shouldn't be able to keep power on. However, you are specifically
> > suggesting that we just turn the power on temporarily during probe. As
> > I think about that, it might be possible? I guess you'd have to
> > temporarily block DRM from changing the state of the panel while the
> > touchscreen is probing. Then if the panel was off then you'd turn it
> > on briefly, do your probe, and then turn it off again. If the panel
> > was on then by blocking DRM you'd ensure that it stayed on. I'm not
> > sure how palatable that would be or if there are any other tricky
> > parts I'm not thinking about.
>
> As this would allow actually probing the touchscreen during probe(), as
> the driver model expects, this seems like a better approach then
> deferring probe and registration if it's at all doable.
Yeah, I don't 100% know if it's doable but it seems possible.
Certainly it's something for future investigation.
Luckily, at the moment anything I'm aware of that truly needs panel
follower also doesn't have multiple sources for a touchscreen.
> > > > Thinking that way, is there any reason you can't just move the
> > > > i2c_hid_init_irq() into __do_i2c_hid_core_initial_power_up()? You
> > > > could replace the call to enable_irq() with it and then remove the
> > > > `IRQF_NO_AUTOEN` flag? I think that would also solve the issue if you
> > > > wanted to use a 2nd source + the panel follower concept? Both devices
> > > > would probe, but only one of them would actually grab the interrupt
> > > > and only one of them would actually create real HID devices. We might
> > > > need to do some work to keep from trying again at every poweron of the
> > > > panel, but it would probably be workable? I think this would also be a
> > > > smaller change...
> > >
> > > That was my first idea as well, but conceptually it is more correct to
> > > request resources at probe time and not at some later point when you can
> > > no longer fail probe.
> > >
> > > You'd also need to handle the fact that the interrupt may never have
> > > been requested when remove() is called, which adds unnecessary
> > > complexity.
> >
> > I don't think it's a lot of complexity, is it? Just an extra "if" statement...
>
> Well you'd need keep track of whether the interrupt has been requested
> or not (and manage serialisation) yourself for a start.
Sure. So I guess an "if" test plus a boolean state variable. I still
don't think it's a lot of complexity.
> But the main reason is still that requesting resources belongs in
> probe() and should not be deferred to some later random time where you
> cannot inform driver core of failures (e.g. for probe deferral if the
> interrupt controller is not yet available).
OK, I guess the -EPROBE_DEFER is technically possible though probably
not likely in practice. ...so that's a good reason to make sure we
request the IRQ in probe even in the "panel follower" case. I still
beleive Benjamin would prefer that this was abstracted out and not in
the actual probe() routine, but I guess we can wait to hear from him.
One last idea I had while digging would be to wonder if we could
somehow solve this case with "IRQF_PROBE_SHARED". I guess that doesn't
work well together with "IRQF_NO_AUTOEN", but conceivably we could
have the interrupt handler return "IRQ_NONE" if the initial power up
never happened? I haven't spent much time poking with shared
interrupts though, so I don't know if there are other side effects...
-Doug
^ permalink raw reply
* Re: [RFC PATCH] of: device: Support 2nd sources of probeable but undiscoverable devices
From: Doug Anderson @ 2023-09-22 17:40 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Conor Dooley, devicetree, Benjamin Tissoires,
Chen-Yu Tsai, linux-input, Jiri Kosina, Hsin-Yi Wang, linux-gpio,
linus.walleij, Dmitry Torokhov, Johan Hovold, andriy.shevchenko,
broonie, frowand.list, gregkh, hdegoede, james.clark, james,
keescook, linux-kernel, rafael, tglx
In-Reply-To: <CAL_Jsq+noP32-m5xdUCLFPFBXLxX9Ys1BNFM+9sga6KYTmDzqQ@mail.gmail.com>
Hi,
On Fri, Sep 22, 2023 at 7:14 AM Rob Herring <robh+dt@kernel.org> wrote:
>
> > Let's attempt to do something better. Specifically, we'll allow
> > tagging nodes in the device tree as mutually exclusive from one
> > another. This says that only one of the components in this group is
> > present on any given board. To make it concrete, in my proposal this
> > looks like:
> >
> > / {
> > tp_ex_group: trackpad-exclusion-group {
> > };
>
> Interesting way to just get a unique identifier. But it could be any
> phandle not used by another group. So just point all the devices in a
> group to one of the devices in the group.
Fair enough.
> > &i2c_bus {
> > tp1: trackpad@10 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > tp2: trackpad@20 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > tp3: trackpad@30 {
> > ...
> > mutual-exclusion-group = <&tp_ex_group>;
> > };
> > };
> >
> > In Linux, we can make things work by simply only probing one of the
> > devices in the group at a time. We can make a mutex per group and
> > enforce locking that mutex around probe. If the first device that gets
> > the mutex fails to probe then it won't try again. If it succeeds then
> > it will acquire the shared resources and future devices (which we know
> > can't be present) will fail to get the shared resources. Future
> > patches could quiet down errors about failing to acquire shared
> > resources or failing to probe if a device is in a
> > mutual-exclusion-group.
>
> This seems like overkill to me. Do we really need groups and a mutex
> for each group? Worst case is what? 2-3 groups of 2-3 devices?
> Instead, what about extending "status" with another value
> ("fail-needs-probe"? (fail-xxx is a documented value)). Currently, the
> kernel would just ignore nodes with that status. Then we can process
> those nodes separately 1-by-1.
My worry here is that this has the potential to impact boot speed in a
non-trivial way. While trackpads and touchscreens _are_ probable,
their probe routines are often quite slow. This is even mentioned in
Dmitry's initial patches adding async probe to the kernel. See commit
765230b5f084 ("driver-core: add asynchronous probing support for
drivers") where he specifically brings up input devices as examples.
It wouldn't be absurd to have a system that has multiple sources for
both the trackpad and the touchscreen. If we have to probe each of
these one at a time then it could be slow. It would be quicker to be
able to probe the trackpads (one at a time) at the same time we're
probing the touchscreens (one at a time). Using the "fail-needs-probe"
doesn't provide information needed to know which devices conflict with
each other. IMO this is still better than nothing, but it worries me
to pick the less-expressive solution for the dts which means that the
information simply isn't there and the OS can't be made better later.
Thinking about this more, I guess even my proposed solution isn't
ideal for probe speed. Let's imagine that we had:
&i2c_bus {
tp1: trackpad@10 {
compatible = "hid-over-i2c";
reg = <0x10>;
post-power-on-delay-ms = <200>;
...
mutual-exclusion-group = <&tp1>;
};
tp2: trackpad@20 {
compatible = "hid-over-i2c";
reg = <0x20>;
post-power-on-delay-ms = <200>;
...
mutual-exclusion-group = <&tp1>;
};
};
With my solution, we'd power the first device up, wait 200 ms, then
check to see if anything acks an i2c xfer at address 0x10. If it
didn't, we'd power down. Then we'd power up the second device
(presumably the same power rail), wait 200 ms, and check to see if
anything acks an i2c xfer at 0x20. It would have been better to just
power up once, wait 200 ms, then check for a device at either 0x10 or
0x20.
I guess with more complex touchscreens this could be more important. I
don't know if we need to try to solve it at this point, but I guess I
could imagine a case where we truly need to take into account all
possible devices (maybe taking the maximum of delays?) to ensure we
don't violate power sequencing requirements for any of them while
probing.
That would lead me to suggest this:
&i2c_bus {
trackpad-prober {
compatible = "mt8173-elm-hana-trackpad-prober";
tp1: trackpad@10 {
compatible = "hid-over-i2c";
reg = <0x10>;
...
post-power-on-delay-ms = <200>;
};
tp2: trackpad@20 {
compatible = "hid-over-i2c";
reg = <0x20>;
...
post-power-on-delay-ms = <200>;
};
};
};
...but I suspect that would be insta-NAKed because it's creating a
completely virtual device ("mt8173-elm-hana-trackpad-prober") in the
device tree. I don't know if there's something that's functionally
similar that would be OK?
-Doug
^ permalink raw reply
* [PATCH] input: Annotate struct evdev_client with __counted_by
From: Kees Cook @ 2023-09-22 17:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, linux-input, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct evdev_client.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/input/evdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 95f90699d2b1..51e0c4954600 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -50,7 +50,7 @@ struct evdev_client {
bool revoked;
unsigned long *evmasks[EV_CNT];
unsigned int bufsize;
- struct input_event buffer[];
+ struct input_event buffer[] __counted_by(bufsize);
};
static size_t evdev_get_mask_cnt(unsigned int type)
--
2.34.1
^ permalink raw reply related
* [PATCH] Input: Annotate struct input_leds with __counted_by
From: Kees Cook @ 2023-09-22 17:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, linux-input, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct input_leds.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/input/input-leds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
index 0b11990ade46..0e935914bc3a 100644
--- a/drivers/input/input-leds.c
+++ b/drivers/input/input-leds.c
@@ -44,7 +44,7 @@ struct input_led {
struct input_leds {
struct input_handle handle;
unsigned int num_leds;
- struct input_led leds[];
+ struct input_led leds[] __counted_by(num_leds);
};
static enum led_brightness input_leds_brightness_get(struct led_classdev *cdev)
--
2.34.1
^ permalink raw reply related
* [PATCH] input: mt: Annotate struct input_mt with __counted_by
From: Kees Cook @ 2023-09-22 17:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, linux-input, Nathan Chancellor, Nick Desaulniers,
Tom Rix, linux-kernel, llvm, linux-hardening
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
As found with Coccinelle[1], add __counted_by for struct input_mt.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/input/mt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index 3b8580bd33c1..2cf89a538b18 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -47,7 +47,7 @@ struct input_mt {
unsigned int flags;
unsigned int frame;
int *red;
- struct input_mt_slot slots[];
+ struct input_mt_slot slots[] __counted_by(num_slots);
};
static inline void input_mt_set_value(struct input_mt_slot *slot,
--
2.34.1
^ permalink raw reply related
* [PATCH 01/15] platform/x86/amd/pmf: Add PMF TEE interface
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
AMD PMF driver loads the PMF TA (Trusted Application) into the AMD
ASP's (AMD Security Processor) TEE (Trusted Execution Environment).
PMF Trusted Application is a secured firmware placed under
/lib/firmware/amdtee gets loaded only when the TEE environment is
initialized. Add the initial code path to build these pipes.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Makefile | 3 +-
drivers/platform/x86/amd/pmf/core.c | 11 ++-
drivers/platform/x86/amd/pmf/pmf.h | 16 ++++
drivers/platform/x86/amd/pmf/tee-if.c | 106 ++++++++++++++++++++++++++
4 files changed, 132 insertions(+), 4 deletions(-)
create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index fdededf54392..d2746ee7369f 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -6,4 +6,5 @@
obj-$(CONFIG_AMD_PMF) += amd-pmf.o
amd-pmf-objs := core.o acpi.o sps.o \
- auto-mode.o cnqf.o
+ auto-mode.o cnqf.o \
+ tee-if.o
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 78ed3ee22555..68f1389dda3e 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -309,8 +309,11 @@ static void amd_pmf_init_features(struct amd_pmf_dev *dev)
dev_dbg(dev->dev, "SPS enabled and Platform Profiles registered\n");
}
- /* Enable Auto Mode */
- if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+ if (amd_pmf_init_smart_pc(dev)) {
+ /* Enable Smart PC Solution builder */
+ dev_dbg(dev->dev, "Smart PC Solution Enabled\n");
+ } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+ /* Enable Auto Mode */
amd_pmf_init_auto_mode(dev);
dev_dbg(dev->dev, "Auto Mode Init done\n");
} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
@@ -330,7 +333,9 @@ static void amd_pmf_deinit_features(struct amd_pmf_dev *dev)
amd_pmf_deinit_sps(dev);
}
- if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+ if (dev->smart_pc_enabled) {
+ amd_pmf_deinit_smart_pc(dev);
+ } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
amd_pmf_deinit_auto_mode(dev);
} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index deba88e6e4c8..02460c2a31ea 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -179,6 +179,12 @@ struct amd_pmf_dev {
bool cnqf_enabled;
bool cnqf_supported;
struct notifier_block pwr_src_notifier;
+ /* Smart PC solution builder */
+ struct tee_context *tee_ctx;
+ struct tee_shm *fw_shm_pool;
+ u32 session_id;
+ void *shbuf;
+ bool smart_pc_enabled;
};
struct apmf_sps_prop_granular {
@@ -389,6 +395,13 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+struct ta_pmf_shared_memory {
+ int command_id;
+ int resp_id;
+ u32 pmf_result;
+ u32 if_version;
+};
+
/* Core Layer */
int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
@@ -433,4 +446,7 @@ void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
extern const struct attribute_group cnqf_feature_attribute_group;
+/* Smart PC builder Layer*/
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
new file mode 100644
index 000000000000..b48340edbf44
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - TEE Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include "pmf.h"
+
+#define MAX_TEE_PARAM 4
+static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
+ 0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+
+static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ return ver->impl_id == TEE_IMPL_ID_AMDTEE;
+}
+
+static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id)
+{
+ struct tee_ioctl_open_session_arg sess_arg = {};
+ int rc;
+
+ export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ sess_arg.num_params = 0;
+
+ rc = tee_client_open_session(ctx, &sess_arg, NULL);
+ if (rc < 0 || sess_arg.ret != 0) {
+ pr_err("%s: tee_client_open_session failed err:%#x, ret:%#x\n",
+ __func__, sess_arg.ret, rc);
+ rc = -EINVAL;
+ } else {
+ *id = sess_arg.session;
+ }
+
+ return rc;
+}
+
+static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
+{
+ int ret;
+ u32 size;
+
+ /* Open context with TEE driver */
+ dev->tee_ctx = tee_client_open_context(NULL, amd_pmf_amdtee_ta_match, NULL, NULL);
+ if (IS_ERR(dev->tee_ctx)) {
+ dev_err(dev->dev, "%s: tee_client_open_context failed\n", __func__);
+ return PTR_ERR(dev->tee_ctx);
+ }
+
+ /* Open session with PMF Trusted App */
+ ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id);
+ if (ret) {
+ dev_err(dev->dev, "%s: amd_pmf_ta_open_session failed(%d)\n", __func__, ret);
+ ret = -EINVAL;
+ goto out_ctx;
+ }
+
+ size = sizeof(struct ta_pmf_shared_memory);
+ dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
+ if (IS_ERR(dev->fw_shm_pool)) {
+ dev_err(dev->dev, "%s: tee_shm_alloc_kernel_buf failed\n", __func__);
+ ret = PTR_ERR(dev->fw_shm_pool);
+ goto out_sess;
+ }
+
+ dev->shbuf = tee_shm_get_va(dev->fw_shm_pool, 0);
+ dev_dbg(dev->dev, "TEE init done\n");
+
+ return 0;
+
+out_sess:
+ tee_client_close_session(dev->tee_ctx, dev->session_id);
+out_ctx:
+ tee_client_close_context(dev->tee_ctx);
+
+ return ret;
+}
+
+static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
+{
+ /* Free the shared memory pool */
+ tee_shm_free(dev->fw_shm_pool);
+
+ /* close the existing session with PMF TA*/
+ tee_client_close_session(dev->tee_ctx, dev->session_id);
+
+ /* close the context with TEE driver */
+ tee_client_close_context(dev->tee_ctx);
+}
+
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
+{
+ return amd_pmf_tee_init(dev);
+}
+
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
+{
+ amd_pmf_tee_deinit(dev);
+}
--
2.25.1
^ permalink raw reply related
* [PATCH 02/15] platform/x86/amd/pmf: Add support PMF-TA interaction
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF TA (Trusted Application) loads via the TEE environment into the
AMD ASP.
PMF-TA supports two commands:
1) Init: Initialize the TA with the PMF Smart PC policy binary and
start the policy engine. A policy is a combination of inputs and
outputs, where;
- the inputs are the changing dynamics of the system like the user
behaviour, system heuristics etc.
- the outputs, which are the actions to be set on the system which
lead to better power management and enhanced user experience.
PMF driver acts as a central manager in this case to supply the
inputs required to the TA (either by getting the information from
the other kernel subsystems or from userland)
2) Enact: Enact the output actions from the TA. The action could be
applying a new thermal limit to boost/throttle the power limits or
change system behavior.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 10 +++
drivers/platform/x86/amd/pmf/tee-if.c | 97 ++++++++++++++++++++++++++-
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 02460c2a31ea..a9333ff6c0a7 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -59,6 +59,9 @@
#define ARG_NONE 0
#define AVG_SAMPLE_SIZE 3
+/* TA macros */
+#define PMF_TA_IF_VERSION__MAJOR 1
+
/* AMD PMF BIOS interfaces */
struct apmf_verify_interface {
u16 size;
@@ -184,6 +187,7 @@ struct amd_pmf_dev {
struct tee_shm *fw_shm_pool;
u32 session_id;
void *shbuf;
+ struct delayed_work pb_work;
bool smart_pc_enabled;
};
@@ -395,6 +399,12 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+/* cmd ids for TA communication */
+enum ta_pmf_command {
+ TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE,
+ TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES
+};
+
struct ta_pmf_shared_memory {
int command_id;
int resp_id;
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index b48340edbf44..1fce04beacb3 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -13,9 +13,96 @@
#include "pmf.h"
#define MAX_TEE_PARAM 4
+
+/* Policy binary actions sampling frequency (in ms) */
+static int pb_actions_ms = 1000;
+#ifdef CONFIG_AMD_PMF_DEBUG
+module_param(pb_actions_ms, int, 0644);
+MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
+#endif
+
static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
+ struct tee_ioctl_invoke_arg *arg,
+ struct tee_param *param)
+{
+ memset(arg, 0, sizeof(*arg));
+ memset(param, 0, MAX_TEE_PARAM * sizeof(*param));
+
+ arg->func = cmd;
+ arg->session = dev->session_id;
+ arg->num_params = MAX_TEE_PARAM;
+
+ /* Fill invoke cmd params */
+ param[0].u.memref.size = sizeof(struct ta_pmf_shared_memory);
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ param[0].u.memref.shm = dev->fw_shm_pool;
+ param[0].u.memref.shm_offs = 0;
+}
+
+static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
+{
+ struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct tee_param param[MAX_TEE_PARAM];
+ struct tee_ioctl_invoke_arg arg;
+ int ret = 0;
+
+ if (!dev->tee_ctx)
+ return -ENODEV;
+
+ ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
+ memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
+ ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
+ ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
+
+ amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES, &arg, param);
+
+ ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(dev->dev, "%s failed TEE err: %x, ret:%x\n", __func__, arg.ret, ret);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
+{
+ struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct tee_param param[MAX_TEE_PARAM];
+ struct tee_ioctl_invoke_arg arg;
+ int ret = 0;
+
+ if (!dev->tee_ctx) {
+ dev_err(dev->dev, "%s tee_ctx no context\n", __func__);
+ return -ENODEV;
+ }
+
+ ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
+ ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE;
+ ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
+
+ amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE, &arg, param);
+
+ ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(dev->dev, "%s failed TEE err: %x, ret:%x\n", __func__, arg.ret, ret);
+ return -EINVAL;
+ }
+
+ return ta_sm->pmf_result;
+}
+
+static void amd_pmf_invoke_cmd(struct work_struct *work)
+{
+ struct amd_pmf_dev *dev = container_of(work, struct amd_pmf_dev, pb_work.work);
+
+ amd_pmf_invoke_cmd_enact(dev);
+ schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
+}
+
static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
{
return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -97,10 +184,18 @@ static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
{
- return amd_pmf_tee_init(dev);
+ int ret;
+
+ ret = amd_pmf_tee_init(dev);
+ if (ret)
+ return ret;
+
+ INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
+ return 0;
}
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+ cancel_delayed_work_sync(&dev->pb_work);
amd_pmf_tee_deinit(dev);
}
--
2.25.1
^ permalink raw reply related
* [PATCH 03/15] platform/x86/amd/pmf: Change signature of amd_pmf_set_dram_addr
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
Make amd_pmf_set_dram_addr() as non-static so that same function
can be used across files.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/core.c | 14 ++++++++------
drivers/platform/x86/amd/pmf/pmf.h | 1 +
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 68f1389dda3e..5fb03ed614ff 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -251,26 +251,28 @@ static const struct pci_device_id pmf_pci_ids[] = {
{ }
};
-static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
{
u64 phys_addr;
u32 hi, low;
+ /* Get Metrics Table Address */
+ dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
+ if (!dev->buf)
+ return -ENOMEM;
+
phys_addr = virt_to_phys(dev->buf);
hi = phys_addr >> 32;
low = phys_addr & GENMASK(31, 0);
amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
+
+ return 0;
}
int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
{
- /* Get Metrics Table Address */
- dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
- if (!dev->buf)
- return -ENOMEM;
-
INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
amd_pmf_set_dram_addr(dev);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index a9333ff6c0a7..ea15ce547d24 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
int amd_pmf_get_power_source(void);
int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev);
/* SPS Layer */
int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
--
2.25.1
^ permalink raw reply related
* [PATCH 05/15] platform/x86/amd/pmf: change debugfs init sequence
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
amd_pmf_dbgfs_register() needs to be called before amd_pmf_init_features().
Hence change the sequence.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 6f36c43e081e..dbfe7c1d6fc4 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -427,9 +427,9 @@ static int amd_pmf_probe(struct platform_device *pdev)
apmf_acpi_init(dev);
platform_set_drvdata(pdev, dev);
+ amd_pmf_dbgfs_register(dev);
amd_pmf_init_features(dev);
apmf_install_handler(dev);
- amd_pmf_dbgfs_register(dev);
dev_info(dev->dev, "registered PMF device successfully\n");
--
2.25.1
^ permalink raw reply related
* [PATCH 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF Policy binary is a encrypted and signed binary that will be part
of the BIOS. PMF driver via the ACPI interface checks the existence
of Smart PC bit. If the advertised bit is found, PMF driver walks
the acpi namespace to find out the policy binary size and the address
which has to be passed to the TA during the TA init sequence.
The policy binary is comprised of inputs (or the events) and outputs
(or the actions). With the PMF ecosystem, OEMs generate the policy
binary (or could be multiple binaries) that contains a supported set
of inputs and outputs which could be specifically carved out for each
usage segment (or for each user also) that could influence the system
behavior either by enriching the user experience or/and boost/throttle
power limits.
Once the TA init command succeeds, the PMF driver sends the changing
events in the current environment to the TA for a constant sampling
frequency time (the event here could be a lid close or open) and
if the policy binary has corresponding action built within it, the
TA sends the action for it in the subsequent enact command.
If the inputs sent to the TA has no output defined in the policy
binary generated by OEMs, there will be no action to be performed
by the PMF driver.
Example policies:
1) if slider is performance ; set the SPL to 40W
Here PMF driver registers with the platform profile interface and
when the slider position is changed, PMF driver lets the TA know
about this. TA sends back an action to update the Sustained
Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
2) if user_away ; then lock the system
Here PMF driver hooks to the AMD SFH driver to know the user presence
and send the inputs to TA and if the condition is met, the TA sends
the action of locking the system. PMF driver generates a uevent and
based on the udev rule in the userland the system gets locked with
systemctl.
The intent here is to provide the OEM's to make a policy to lock the
system when the user is away ; but the userland can make a choice to
ignore it.
and so on.
The OEMs will have an utility to create numerous such policies and
the policies shall be reviewed by AMD before signing and encrypting
them. Policies are shared between operating systems to have seemless user
experience.
Since all this action has to happen via the "amdtee" driver, currently
there is no caller for it in the kernel which can load the amdtee driver.
Without amdtee driver loading onto the system the "tee" calls shall fail
from the PMF driver. Hence an explicit "request_module" has been added
to address this.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/acpi.c | 37 +++++++
drivers/platform/x86/amd/pmf/core.c | 12 +++
drivers/platform/x86/amd/pmf/pmf.h | 132 ++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/tee-if.c | 141 +++++++++++++++++++++++++-
5 files changed, 321 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 3064bc8ea167..437b78c6d1c5 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -9,6 +9,7 @@ config AMD_PMF
depends on POWER_SUPPLY
depends on AMD_NB
select ACPI_PLATFORM_PROFILE
+ depends on AMDTEE
help
This driver provides support for the AMD Platform Management Framework.
The goal is to enhance end user experience by making AMD PCs smarter,
diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 3fc5e4547d9f..d0512af2cd42 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
return 0;
}
+static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
+{
+ struct amd_pmf_dev *dev = data;
+
+ switch (res->type) {
+ case ACPI_RESOURCE_TYPE_ADDRESS64:
+ dev->policy_addr = res->data.address64.address.minimum;
+ dev->policy_sz = res->data.address64.address.address_length;
+ break;
+ case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+ dev->policy_addr = res->data.fixed_memory32.address;
+ dev->policy_sz = res->data.fixed_memory32.address_length;
+ break;
+ }
+
+ if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) {
+ pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
+ return AE_ERROR;
+ }
+
+ return AE_OK;
+}
+
+int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
+{
+ acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
+ acpi_status status;
+
+ status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev);
+ if (ACPI_FAILURE(status)) {
+ dev_err(pmf_dev->dev, "acpi_walk_resources failed\n");
+ return status;
+ }
+
+ return 0;
+}
+
void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
{
acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 5fb03ed614ff..6f36c43e081e 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -376,6 +376,18 @@ static int amd_pmf_probe(struct platform_device *pdev)
return -ENOMEM;
dev->dev = &pdev->dev;
+ err = apmf_check_smart_pc(dev);
+ if (!err) {
+ /* in order for Smart PC solution to work it has a hard dependency
+ * on the amdtee driver to be loaded first even before the PMF driver
+ * loads. PMF ASL has a _CRS method that advertises the existence
+ * of Smart PC bit. If this information is present, use this to
+ * explicitly probe the amdtee driver, so that "tee" plumbing is done
+ * before the PMF Smart PC init happens.
+ */
+ if (request_module("amdtee"))
+ pr_err("Failed to load amdtee. PMF Smart PC not enabled!\n");
+ }
rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index ea15ce547d24..81acf2a37366 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -13,6 +13,8 @@
#include <linux/acpi.h>
#include <linux/platform_profile.h>
+#define POLICY_BUF_MAX_SZ 0x4b000
+#define POLICY_SIGN_COOKIE 0x31535024
/* APMF Functions */
#define APMF_FUNC_VERIFY_INTERFACE 0
@@ -59,8 +61,20 @@
#define ARG_NONE 0
#define AVG_SAMPLE_SIZE 3
+/* Policy Actions */
+#define PMF_POLICY_SPL 2
+#define PMF_POLICY_SPPT 3
+#define PMF_POLICY_FPPT 4
+#define PMF_POLICY_SPPT_APU_ONLY 5
+#define PMF_POLICY_STT_MIN 6
+#define PMF_POLICY_STT_SKINTEMP_APU 7
+#define PMF_POLICY_STT_SKINTEMP_HS2 8
+
/* TA macros */
#define PMF_TA_IF_VERSION__MAJOR 1
+#define TA_PMF_ACTION_MAX 32
+#define TA_PMF_UNDO_MAX 8
+#define MAX_OPERATION_PARAMS 4
/* AMD PMF BIOS interfaces */
struct apmf_verify_interface {
@@ -183,11 +197,16 @@ struct amd_pmf_dev {
bool cnqf_supported;
struct notifier_block pwr_src_notifier;
/* Smart PC solution builder */
+ unsigned char *policy_buf;
+ u32 policy_sz;
struct tee_context *tee_ctx;
struct tee_shm *fw_shm_pool;
u32 session_id;
void *shbuf;
struct delayed_work pb_work;
+ struct pmf_action_table *prev_data;
+ u64 policy_addr;
+ void *policy_base;
bool smart_pc_enabled;
};
@@ -399,17 +418,129 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+/* Smart PC - TA internals */
+enum ta_slider {
+ TA_BEST_BATTERY, /* Best Battery */
+ TA_BETTER_BATTERY, /* Better Battery */
+ TA_BETTER_PERFORMANCE, /* Better Performance */
+ TA_BEST_PERFORMANCE, /* Best Performance */
+ TA_MAX,
+};
+
/* cmd ids for TA communication */
enum ta_pmf_command {
TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE,
TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES
};
+enum ta_pmf_error_type {
+ TA_PMF_TYPE__SUCCESS,
+ TA_PMF_ERROR_TYPE__GENERIC,
+ TA_PMF_ERROR_TYPE__CRYPTO,
+ TA_PMF_ERROR_TYPE__CRYPTO_VALIDATE,
+ TA_PMF_ERROR_TYPE__CRYPTO_VERIFY_OEM,
+ TA_PMF_ERROR_TYPE__POLICY_BUILDER,
+ TA_PMF_ERROR_TYPE__PB_CONVERT,
+ TA_PMF_ERROR_TYPE__PB_SETUP,
+ TA_PMF_ERROR_TYPE__PB_ENACT,
+ TA_PMF_ERROR_TYPE__ASD_GET_DEVICE_INFO,
+ TA_PMF_ERROR_TYPE__ASD_GET_DEVICE_PCIE_INFO,
+ TA_PMF_ERROR_TYPE__SYS_DRV_FW_VALIDATION,
+ TA_PMF_ERROR_TYPE__MAX,
+};
+
+struct pmf_action_table {
+ unsigned long spl; /* in mW */
+ unsigned long sppt; /* in mW */
+ unsigned long sppt_apuonly; /* in mW */
+ unsigned long fppt; /* in mW */
+ unsigned long stt_minlimit; /* in mW */
+ unsigned long stt_skintemp_apu; /* in C */
+ unsigned long stt_skintemp_hs2; /* in C */
+};
+
+/* Input conditions */
+struct ta_pmf_condition_info {
+ u32 power_source;
+ u32 bat_percentage;
+ u32 power_slider;
+ u32 lid_state;
+ bool user_present;
+ u32 rsvd1[2];
+ u32 monitor_count;
+ u32 rsvd2[2];
+ u32 bat_design;
+ u32 full_charge_capacity;
+ int drain_rate;
+ bool user_engaged;
+ u32 device_state;
+ u32 socket_power;
+ u32 skin_temperature;
+ u32 rsvd3[5];
+ u32 ambient_light;
+ u32 length;
+ u32 avg_c0residency;
+ u32 max_c0residency;
+ u32 s0i3_entry;
+ u32 gfx_busy;
+ u32 rsvd4[7];
+ bool camera_state;
+ u32 workload_type;
+ u32 display_type;
+ u32 display_state;
+ u32 rsvd5[150];
+};
+
+struct ta_pmf_load_policy_table {
+ u32 table_size;
+ u8 table[POLICY_BUF_MAX_SZ];
+};
+
+/* TA initialization params */
+struct ta_pmf_init_table {
+ u32 frequency; /* SMU sampling frequency */
+ bool validate;
+ bool sku_check;
+ bool metadata_macrocheck;
+ struct ta_pmf_load_policy_table policies_table;
+};
+
+/* Everything the TA needs to Enact Policies */
+struct ta_pmf_enact_table {
+ struct ta_pmf_condition_info ev_info;
+ u32 name;
+};
+
+struct ta_pmf_action {
+ u32 action_index;
+ u32 value;
+};
+
+/* output actions from TA */
+struct ta_pmf_enact_result {
+ u32 actions_count;
+ struct ta_pmf_action actions_list[TA_PMF_ACTION_MAX];
+ u32 undo_count;
+ struct ta_pmf_action undo_list[TA_PMF_UNDO_MAX];
+};
+
+union ta_pmf_input {
+ struct ta_pmf_enact_table enact_table;
+ struct ta_pmf_init_table init_table;
+};
+
+union ta_pmf_output {
+ struct ta_pmf_enact_result policy_apply_table;
+ u32 rsvd[906];
+};
+
struct ta_pmf_shared_memory {
int command_id;
int resp_id;
u32 pmf_result;
u32 if_version;
+ union ta_pmf_output pmf_output;
+ union ta_pmf_input pmf_input;
};
/* Core Layer */
@@ -460,4 +591,5 @@ extern const struct attribute_group cnqf_feature_attribute_group;
/* Smart PC builder Layer*/
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
+int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 1fce04beacb3..a8b05e746efd 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -42,9 +42,77 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
param[0].u.memref.shm_offs = 0;
}
+static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
+{
+ u32 val;
+ int idx;
+
+ for (idx = 0; idx < out->actions_count; idx++) {
+ val = out->actions_list[idx].value;
+ switch (out->actions_list[idx].action_index) {
+ case PMF_POLICY_SPL:
+ if (dev->prev_data->spl != val) {
+ amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
+ dev_dbg(dev->dev, "update SPL : %d\n", val);
+ dev->prev_data->spl = val;
+ }
+ break;
+
+ case PMF_POLICY_SPPT:
+ if (dev->prev_data->sppt != val) {
+ amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
+ dev_dbg(dev->dev, "update SPPT : %d\n", val);
+ dev->prev_data->sppt = val;
+ }
+ break;
+
+ case PMF_POLICY_FPPT:
+ if (dev->prev_data->fppt != val) {
+ amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
+ dev_dbg(dev->dev, "update FPPT : %d\n", val);
+ dev->prev_data->fppt = val;
+ }
+ break;
+
+ case PMF_POLICY_SPPT_APU_ONLY:
+ if (dev->prev_data->sppt_apuonly != val) {
+ amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
+ dev_dbg(dev->dev, "update SPPT_APU_ONLY : %d\n", val);
+ dev->prev_data->sppt_apuonly = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_MIN:
+ if (dev->prev_data->stt_minlimit != val) {
+ amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_MIN : %d\n", val);
+ dev->prev_data->stt_minlimit = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_SKINTEMP_APU:
+ if (dev->prev_data->stt_skintemp_apu != val) {
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_SKINTEMP_APU : %d\n", val);
+ dev->prev_data->stt_skintemp_apu = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_SKINTEMP_HS2:
+ if (dev->prev_data->stt_skintemp_hs2 != val) {
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_SKINTEMP_HS2 : %d\n", val);
+ dev->prev_data->stt_skintemp_hs2 = val;
+ }
+ break;
+ }
+ }
+}
+
static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct ta_pmf_enact_result *out = NULL;
struct tee_param param[MAX_TEE_PARAM];
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -52,7 +120,10 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
if (!dev->tee_ctx)
return -ENODEV;
+ memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
+ out = &ta_sm->pmf_output.policy_apply_table;
+
memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
@@ -65,6 +136,12 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
return -EINVAL;
}
+ if (ta_sm->pmf_result == TA_PMF_TYPE__SUCCESS && out->actions_count) {
+ dev_dbg(dev->dev, "action count:%d result:%x\n", out->actions_count,
+ ta_sm->pmf_result);
+ amd_pmf_apply_policies(dev, out);
+ }
+
return 0;
}
@@ -72,6 +149,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
struct tee_param param[MAX_TEE_PARAM];
+ struct ta_pmf_init_table *in = NULL;
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -80,10 +158,20 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
return -ENODEV;
}
+ dev_dbg(dev->dev, "Policy Binary size: %d bytes\n", dev->policy_sz);
+ memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
+ in = &ta_sm->pmf_input.init_table;
+
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE;
ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
+ in->metadata_macrocheck = false;
+ in->sku_check = false;
+ in->validate = true;
+ in->frequency = pb_actions_ms;
+ in->policies_table.table_size = dev->policy_sz;
+ memcpy(in->policies_table.table, dev->policy_buf, dev->policy_sz);
amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__INITIALIZE, &arg, param);
ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
@@ -103,6 +191,47 @@ static void amd_pmf_invoke_cmd(struct work_struct *work)
schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
}
+static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
+{
+ u32 cookie, length;
+ int res;
+
+ cookie = readl(dev->policy_buf + 0x10);
+ length = readl(dev->policy_buf + 0x14);
+
+ if (cookie != POLICY_SIGN_COOKIE || !length)
+ return -EINVAL;
+
+ /* update the actual length */
+ dev->policy_sz = length + 512;
+ res = amd_pmf_invoke_cmd_init(dev);
+ if (res == TA_PMF_TYPE__SUCCESS) {
+ /* now its safe to announce that smart pc is enabled */
+ dev->smart_pc_enabled = 1;
+ schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms * 3));
+ } else {
+ dev_err(dev->dev, "%s ta invoke_cmd_init failed err: %x\n", __func__, res);
+ return res;
+ }
+
+ return 0;
+}
+
+static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
+{
+ dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
+ if (!dev->policy_buf)
+ return -ENOMEM;
+
+ dev->policy_base = ioremap(dev->policy_addr, dev->policy_sz);
+ if (!dev->policy_base)
+ return -ENOMEM;
+
+ memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+
+ return amd_pmf_start_policy_engine(dev);
+}
+
static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
{
return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -149,7 +278,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
goto out_ctx;
}
- size = sizeof(struct ta_pmf_shared_memory);
+ size = sizeof(struct ta_pmf_shared_memory) + dev->policy_sz;
dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
if (IS_ERR(dev->fw_shm_pool)) {
dev_err(dev->dev, "%s: tee_shm_alloc_kernel_buf failed\n", __func__);
@@ -191,11 +320,19 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
return ret;
INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
- return 0;
+ amd_pmf_set_dram_addr(dev);
+ amd_pmf_get_bios_buffer(dev);
+ dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
+ if (!dev->prev_data)
+ return -ENOMEM;
+
+ return dev->smart_pc_enabled;
}
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+ kfree(dev->prev_data);
+ kfree(dev->policy_buf);
cancel_delayed_work_sync(&dev->pb_work);
amd_pmf_tee_deinit(dev);
}
--
2.25.1
^ permalink raw reply related
* [PATCH 06/15] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF driver sends changing inputs from each subystem to TA for evaluating
the conditions in the policy binary.
Add initial support of plumbing in the PMF driver for Smart PC to get
information from other subsystems in the kernel.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Makefile | 2 +-
drivers/platform/x86/amd/pmf/pmf.h | 18 ++++
drivers/platform/x86/amd/pmf/spc.c | 118 ++++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/tee-if.c | 3 +
4 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/x86/amd/pmf/spc.c
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index d2746ee7369f..6b26e48ce8ad 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -7,4 +7,4 @@
obj-$(CONFIG_AMD_PMF) += amd-pmf.o
amd-pmf-objs := core.o acpi.o sps.o \
auto-mode.o cnqf.o \
- tee-if.o
+ tee-if.o spc.o
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 81acf2a37366..e64b4d285624 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -146,6 +146,21 @@ struct smu_pmf_metrics {
u16 infra_gfx_maxfreq; /* in MHz */
u16 skin_temp; /* in centi-Celsius */
u16 device_state;
+ u16 curtemp; /* in centi-Celsius */
+ u16 filter_alpha_value;
+ u16 avg_gfx_clkfrequency;
+ u16 avg_fclk_frequency;
+ u16 avg_gfx_activity;
+ u16 avg_socclk_frequency;
+ u16 avg_vclk_frequency;
+ u16 avg_vcn_activity;
+ u16 avg_dram_reads;
+ u16 avg_dram_writes;
+ u16 avg_socket_power;
+ u16 avg_core_power[2];
+ u16 avg_core_c0residency[16];
+ u16 spare1;
+ u32 metrics_counter;
} __packed;
enum amd_stt_skin_temp {
@@ -592,4 +607,7 @@ extern const struct attribute_group cnqf_feature_attribute_group;
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
+
+/* Smart PC - TA interfaces */
+void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
new file mode 100644
index 000000000000..08159cd5f853
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - Smart PC Capabilities
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Patil Rajesh Reddy <Patil.Reddy@amd.com>
+ */
+
+#include <acpi/button.h>
+#include <linux/power_supply.h>
+#include "pmf.h"
+
+static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ u16 max, avg = 0;
+ int i;
+
+ memset(dev->buf, 0, sizeof(dev->m_table));
+ amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
+ memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
+
+ in->ev_info.socket_power = dev->m_table.apu_power + dev->m_table.dgpu_power;
+ in->ev_info.skin_temperature = dev->m_table.skin_temp;
+
+ /* get the avg C0 residency of all the cores */
+ for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++)
+ avg += dev->m_table.avg_core_c0residency[i];
+
+ /* get the max C0 residency of all the cores */
+ max = dev->m_table.avg_core_c0residency[0];
+ for (i = 1; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
+ if (dev->m_table.avg_core_c0residency[i] > max)
+ max = dev->m_table.avg_core_c0residency[i];
+ }
+
+ in->ev_info.avg_c0residency = avg / ARRAY_SIZE(dev->m_table.avg_core_c0residency);
+ in->ev_info.max_c0residency = max;
+ in->ev_info.gfx_busy = dev->m_table.avg_gfx_activity;
+}
+
+static const char * const pmf_battery_supply_name[] = {
+ "BATT",
+ "BAT0",
+};
+
+static int get_battery_prop(enum power_supply_property prop)
+{
+ union power_supply_propval value;
+ struct power_supply *psy;
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(pmf_battery_supply_name); i++) {
+ psy = power_supply_get_by_name(pmf_battery_supply_name[i]);
+ if (!psy)
+ continue;
+
+ ret = power_supply_get_property(psy, prop, &value);
+ if (ret) {
+ power_supply_put(psy);
+ return ret;
+ }
+ }
+
+ return value.intval;
+}
+
+static int amd_pmf_get_battery_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ int val;
+
+ val = get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
+ if (val != 1)
+ return -EINVAL;
+
+ in->ev_info.bat_percentage = get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
+ /* all values in mWh metrics */
+ in->ev_info.bat_design = get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN) / 1000;
+ in->ev_info.full_charge_capacity = get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL) / 1000;
+ in->ev_info.drain_rate = get_battery_prop(POWER_SUPPLY_PROP_POWER_NOW) / 1000;
+
+ return 0;
+}
+
+static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ int val;
+
+ switch (dev->current_profile) {
+ case PLATFORM_PROFILE_PERFORMANCE:
+ val = TA_BEST_PERFORMANCE;
+ break;
+ case PLATFORM_PROFILE_BALANCED:
+ val = TA_BETTER_PERFORMANCE;
+ break;
+ case PLATFORM_PROFILE_LOW_POWER:
+ val = TA_BEST_BATTERY;
+ break;
+ default:
+ dev_err(dev->dev, "Unknown Platform Profile.\n");
+ return -EOPNOTSUPP;
+ }
+ in->ev_info.power_slider = val;
+
+ return 0;
+}
+
+void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ /* TA side lid open is 1 and close is 0, hence the ! here */
+ in->ev_info.lid_state = !acpi_lid_open();
+ in->ev_info.power_source = amd_pmf_get_power_source();
+ amd_pmf_get_smu_info(dev, in);
+ amd_pmf_get_battery_info(dev, in);
+ amd_pmf_get_slider_info(dev, in);
+}
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index a8b05e746efd..eb25d5ce3a9a 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -113,6 +113,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
struct ta_pmf_enact_result *out = NULL;
+ struct ta_pmf_enact_table *in = NULL;
struct tee_param param[MAX_TEE_PARAM];
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -123,11 +124,13 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = (struct ta_pmf_shared_memory *)dev->shbuf;
out = &ta_sm->pmf_output.policy_apply_table;
+ in = &ta_sm->pmf_input.enact_table;
memset(ta_sm, 0, sizeof(struct ta_pmf_shared_memory));
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES;
ta_sm->if_version = PMF_TA_IF_VERSION__MAJOR;
+ amd_pmf_populate_ta_inputs(dev, in);
amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER__ENACT_POLICIES, &arg, param);
ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
--
2.25.1
^ permalink raw reply related
* [PATCH 00/15] Introduce PMF Smart PC Solution Builder Feature
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
Smart PC Solutions Builder allows for OEM to define a large number of
custom system states to dynamically switch to. The system states are
referred to as policies, and multiple policies can be loaded onto the
system at any given time, however only one policy can be active at a
given time.
Policy is a combination of PMF input and output capabilities. The inputs
are the incoming information from the other kernel subsystems like LID
state, Sensor info, GPU info etc and the actions are the updating the
power limits of SMU etc.
The policy binary is signed and encrypted by a special key from AMD. This
policy binary shall have the inputs and outputs which the OEMs can build
for the platform customization that can enhance the user experience and
system behavior.
This series adds the initial support for Smart PC solution to PMF driver.
Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
shall have higher precedence and same applies for Auto Mode.
Basavaraj Natikar (2):
platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
Shyam Sundar S K (13):
platform/x86/amd/pmf: Add PMF TEE interface
platform/x86/amd/pmf: Add support PMF-TA interaction
platform/x86/amd/pmf: Change signature of amd_pmf_set_dram_addr
platform/x86/amd/pmf: Add support for PMF Policy Binary
platform/x86/amd/pmf: change debugfs init sequence
platform/x86/amd/pmf: Add support to get inputs from other subsystems
platform/x86/amd/pmf: Add support update p3t limit
platform/x86/amd/pmf: Add support to update system state
platform/x86/amd/pmf: Add facility to dump TA inputs
platform/x86/amd/pmf: Add capability to sideload of policy binary
platform/x86/amd/pmf: dump policy binary data
platform/x86/amd/pmf: Add PMF-AMDGPU get interface
platform/x86/amd/pmf: Add PMF-AMDGPU set interface
Documentation/admin-guide/pmf.rst | 24 +
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 91 ++++
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 6 +
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 17 +
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 48 ++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
drivers/platform/x86/amd/pmf/Kconfig | 2 +
drivers/platform/x86/amd/pmf/Makefile | 3 +-
drivers/platform/x86/amd/pmf/acpi.c | 37 ++
drivers/platform/x86/amd/pmf/core.c | 40 +-
drivers/platform/x86/amd/pmf/pmf.h | 199 +++++++
drivers/platform/x86/amd/pmf/spc.c | 194 +++++++
drivers/platform/x86/amd/pmf/sps.c | 2 +-
drivers/platform/x86/amd/pmf/tee-if.c | 492 ++++++++++++++++++
include/linux/amd-pmf-io.h | 51 ++
18 files changed, 1199 insertions(+), 13 deletions(-)
create mode 100644 Documentation/admin-guide/pmf.rst
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 drivers/platform/x86/amd/pmf/spc.c
create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
create mode 100644 include/linux/amd-pmf-io.h
--
2.25.1
^ permalink raw reply
* [PATCH 07/15] platform/x86/amd/pmf: Add support update p3t limit
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
P3T (Peak Package Power Limit) is a metric within the SMU controller
that can influence the power limits. Add support from the driver
to update P3T limits accordingly.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 3 +++
drivers/platform/x86/amd/pmf/tee-if.c | 8 ++++++++
2 files changed, 11 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index e64b4d285624..897f61b75e2f 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -46,6 +46,7 @@
#define GET_STT_MIN_LIMIT 0x1F
#define GET_STT_LIMIT_APU 0x20
#define GET_STT_LIMIT_HS2 0x21
+#define SET_P3T 0x23 /* P3T: Peak Package Power Limit */
/* OS slider update notification */
#define DC_BEST_PERF 0
@@ -69,6 +70,7 @@
#define PMF_POLICY_STT_MIN 6
#define PMF_POLICY_STT_SKINTEMP_APU 7
#define PMF_POLICY_STT_SKINTEMP_HS2 8
+#define PMF_POLICY_P3T 38
/* TA macros */
#define PMF_TA_IF_VERSION__MAJOR 1
@@ -472,6 +474,7 @@ struct pmf_action_table {
unsigned long stt_minlimit; /* in mW */
unsigned long stt_skintemp_apu; /* in C */
unsigned long stt_skintemp_hs2; /* in C */
+ unsigned long p3t_limit; /* in mW */
};
/* Input conditions */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index eb25d5ce3a9a..883dd143375a 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -105,6 +105,14 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
dev->prev_data->stt_skintemp_hs2 = val;
}
break;
+
+ case PMF_POLICY_P3T:
+ if (dev->prev_data->p3t_limit != val) {
+ amd_pmf_send_cmd(dev, SET_P3T, false, val, NULL);
+ dev_dbg(dev->dev, "update P3T : %d\n", val);
+ dev->prev_data->p3t_limit = val;
+ }
+ break;
}
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH 08/15] platform/x86/amd/pmf: Add support to update system state
From: Shyam Sundar S K @ 2023-09-22 17:50 UTC (permalink / raw)
To: hdegoede, markgross, basavaraj.natikar, jikos, benjamin.tissoires,
alexander.deucher, christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Shyam Sundar S K
In-Reply-To: <20230922175056.244940-1-Shyam-sundar.S-k@amd.com>
PMF driver based on the output actions from the TA can request to update
the system states like entering s0i3, lock screen etc. by generating
an uevent. Based on the udev rules set in the userspace the event id
matching the uevent shall get updated accordingly using the systemctl.
Sample udev rules under Documentation/admin-guide/pmf.rst.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
Documentation/admin-guide/pmf.rst | 24 ++++++++++++++++
drivers/platform/x86/amd/pmf/pmf.h | 9 ++++++
drivers/platform/x86/amd/pmf/tee-if.c | 40 ++++++++++++++++++++++++++-
3 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 Documentation/admin-guide/pmf.rst
diff --git a/Documentation/admin-guide/pmf.rst b/Documentation/admin-guide/pmf.rst
new file mode 100644
index 000000000000..b60f381410c3
--- /dev/null
+++ b/Documentation/admin-guide/pmf.rst
@@ -0,0 +1,24 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Set udev rules for PMF Smart PC Builder
+---------------------------------------
+
+AMD PMF(Platform Management Framework) Smart PC Solution builder has to set the system states
+like S0i3, Screen lock, hibernate etc, based on the output actions provided by the PMF
+TA (Trusted Application).
+
+In order for this to work the PMF driver generates a uevent for userspace to react to. Below are
+sample udev rules that can facilitate this experience when a machine has PMF Smart PC solution builder
+enabled.
+
+Please add the following line(s) to
+``/etc/udev/rules.d/99-local.rules``::
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1", RUN+="/usr/bin/systemctl suspend"
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2", RUN+="/usr/bin/systemctl hibernate"
+ DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="3", RUN+="/bin/loginctl lock-sessions"
+
+EVENT_ID values:
+1= Put the system to S0i3/S2Idle
+2= Put the system to hibernate
+3= Lock the screen
+
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 897f61b75e2f..c5334f1177a4 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -70,6 +70,7 @@
#define PMF_POLICY_STT_MIN 6
#define PMF_POLICY_STT_SKINTEMP_APU 7
#define PMF_POLICY_STT_SKINTEMP_HS2 8
+#define PMF_POLICY_SYSTEM_STATE 9
#define PMF_POLICY_P3T 38
/* TA macros */
@@ -436,6 +437,13 @@ struct apmf_dyn_slider_output {
} __packed;
/* Smart PC - TA internals */
+enum system_state {
+ SYSTEM_STATE__S0i3 = 1,
+ SYSTEM_STATE__S4,
+ SYSTEM_STATE__SCREEN_LOCK,
+ SYSTEM_STATE__MAX
+};
+
enum ta_slider {
TA_BEST_BATTERY, /* Best Battery */
TA_BETTER_BATTERY, /* Better Battery */
@@ -467,6 +475,7 @@ enum ta_pmf_error_type {
};
struct pmf_action_table {
+ enum system_state system_state;
unsigned long spl; /* in mW */
unsigned long sppt; /* in mW */
unsigned long sppt_apuonly; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 883dd143375a..1629856c20b4 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -24,6 +24,20 @@ MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (defau
static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+static const char *amd_pmf_uevent_as_str(unsigned int state)
+{
+ switch (state) {
+ case SYSTEM_STATE__S0i3:
+ return "S0i3";
+ case SYSTEM_STATE__S4:
+ return "S4";
+ case SYSTEM_STATE__SCREEN_LOCK:
+ return "SCREEN_LOCK";
+ default:
+ return "Unknown Smart PC event";
+ }
+}
+
static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
struct tee_ioctl_invoke_arg *arg,
struct tee_param *param)
@@ -42,9 +56,23 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
param[0].u.memref.shm_offs = 0;
}
+static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
+{
+ char *envp[2] = {};
+
+ envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event);
+ if (!envp[0])
+ return -EINVAL;
+
+ kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp);
+
+ kfree(envp[0]);
+ return 0;
+}
+
static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
{
- u32 val;
+ u32 val, event = 0;
int idx;
for (idx = 0; idx < out->actions_count; idx++) {
@@ -113,6 +141,16 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
dev->prev_data->p3t_limit = val;
}
break;
+
+ case PMF_POLICY_SYSTEM_STATE:
+ event = val + 1;
+ if (dev->prev_data->system_state != event) {
+ amd_pmf_update_uevents(dev, event);
+ dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
+ amd_pmf_uevent_as_str(event));
+ dev->prev_data->system_state = 0;
+ }
+ break;
}
}
}
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox