[PATCH i-g-t v2] tests/intel/xe_pm: Introduce i2c subtests
Riana Tauro
riana.tauro at intel.com
Wed Jul 23 09:41:13 UTC 2025
Hi Raag
On 7/22/2025 9:27 PM, Raag Jadav wrote:
> On Tue, Jul 22, 2025 at 06:33:43PM +0530, Gupta, Anshuman wrote:
>>> From: Jadav, Raag <raag.jadav at intel.com>
>>> On Tue, Jul 22, 2025 at 05:27:35PM +0530, Gupta, Anshuman wrote:
>>>>> From: Jadav, Raag <raag.jadav at intel.com> On Mon, Jul 21, 2025 at
>>>>> 01:36:32PM +0300, Heikki Krogerus wrote:
>>>>>> On Sat, Jul 19, 2025 at 10:01:01PM +0530, Raag Jadav wrote:
>>>>>>> Introduce subtests for i2c adapter which is used to control
>>>>>>> on-board OEM sensors on selected devices. This will test
>>>>>>> D3hot/D3cold transition after i2c adapter access for the devices that
>>> support it.
>>>>>>>
>>>>>>> v2: Define macros for AMC constants (Lucas)
>>>>>>> s/index/adapter (Lucas)
>>>>>>>
>>>>>>> Signed-off-by: Raag Jadav <raag.jadav at intel.com>
>>>>>>
>>>>>> Looks good to me. FWIW:
>>>>>>
>>>>>> Reviewed-by: Heikki Krogerus <heikki.krogerus at linux.intel.com>
>>>>>
>>>>> Thanks Heikki.
>>>>>
>>>>> The BAT failures seems unrelated to the patch. So anything I can do
>>>>> to move this forward?
>>>>>
>>>>> Raag
>>>>>
>>>>>>> ---
>>>>>>> tests/intel/xe_pm.c | 98
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>>>> 1 file changed, 98 insertions(+)
>>>>>>>
>>>>>>> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c index
>>>>>>> 16b5fc686..ca935d518 100644
>>>>>>> --- a/tests/intel/xe_pm.c
>>>>>>> +++ b/tests/intel/xe_pm.c
>>>>>>> @@ -11,12 +11,18 @@
>>>>>>> * Test category: functionality test
>>>>>>> */
>>>>>>>
>>>>>>> +#include <dirent.h>
>>>>>>> #include <limits.h>
>>>>>>> #include <fcntl.h>
>>>>>>> #include <string.h>
>>>>>>> +#include <sys/ioctl.h>
>>>>>>> +
>>>>>>> +#include <linux/i2c-dev.h>
>>>>>>> +#include <linux/i2c.h>
>>>>>>>
>>>>>>> #include "igt.h"
>>>>>>> #include "lib/igt_device.h"
>>>>>>> +#include "lib/igt_kmod.h"
>>>>>>> #include "lib/igt_pm.h"
>>>>>>> #include "lib/igt_sysfs.h"
>>>>>>> #include "lib/igt_syncobj.h"
>>>>>>> @@ -38,6 +44,10 @@
>>>>>>> #define PREFETCH (0x1 << 1)
>>>>>>> #define UNBIND_ALL (0x1 << 2)
>>>>>>>
>>>>>>> +/* AMC slave details */
>>>>>>> +#define I2C_AMC_ADDR 0x40
>>>>>>> +#define I2C_AMC_REG 0x00
>>>>>>> +
>>>>>>> enum mem_op {
>>>>>>> READ,
>>>>>>> WRITE,
>>>>>>> @@ -779,6 +789,87 @@ static void
>>>>>>> test_mocs_suspend_resume(device_t
>>>>> device, enum igt_suspend_state s_s
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> +static int find_i2c_adapter(device_t device, int sysfs_fd) {
>>>>>>> + int adapter_fd, i2c_adapter = -1;
>>>>>>> + struct dirent *dirent;
>>>>>>> + char adapter[32];
>>>>>>> + DIR *dir;
>>>>>>> +
>>>>>>> + /* Make sure the /dev/i2c-* files exist */
>>>>>>> + igt_require(igt_kmod_load("i2c-dev", NULL) == 0);
>>>>>>> +
>>>>>>> + snprintf(adapter, sizeof(adapter), "%s.%hu",
>>> "device/i2c_designware",
>>>>>>> + (device.pci_xe->bus << 8) | (device.pci_xe->dev));
>>>>>>> + adapter_fd = openat(sysfs_fd, adapter, O_RDONLY);
>>>>>>> + igt_require_fd(adapter_fd);
>>>>>>> +
>>>>>>> + dir = fdopendir(adapter_fd);
>>>>>>> + igt_assert(dir);
>>>>>>> +
>>>>>>> + /* Find the i2c adapter */
>>>>>>> + while ((dirent = readdir(dir))) {
>>>>>>> + if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
>>>>>>> + sscanf(dirent->d_name, "i2c-%d",
>>> &i2c_adapter);
>>>>>>> + break;
>>>>>>> + }
>>>>>>> + }
>>>>>>> +
>>>>>>> + closedir(dir);
>>>>>>> + close(adapter_fd);
>>>>>>> + return i2c_adapter;
>>>>>>> +}
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * SUBTEST: %s-i2c
>>>>>>> + * Description:
>>>>>>> + * Validate whether the device is able to suspend after i2c
>>> adapter
runtime suspend
>>>>> access.
>>>>>>> + * Functionality: pm-d3
>>>>>>> + * GPU requirements: D3 feature should be supported
>>>>>>> + *
>>>>>>> + * arg[1]:
>>>>>>> + *
>>>>>>> + * @d3hot: d3hot
>>>>>>> + * @d3cold: d3cold
>>>>>>> + */
>>>>>>> +static bool i2c_test(device_t device, int sysfs_fd) {
bool return type is unnecessary. Not returning false anywhere
Thanks
Riana
>>>>>>> + uint8_t addr = I2C_AMC_ADDR, reg = I2C_AMC_REG, buf;
>>>>>>> + int i2c_adapter, i2c_fd;
>>>>>>> + char i2c_dev[16];
>>>>>>> + struct i2c_msg msgs[] = {
>>>>>>> + {
>>>>>>> + .addr = addr,
>>>>>>> + .flags = 0,
>>>>>>> + .len = sizeof(reg),
>>>>>>> + .buf = ®,
>>>>>>> + }, {
>>>>>>> + .addr = addr,
>>>>>>> + .flags = I2C_M_RD,
>>>>>>> + .len = sizeof(buf),
>>>>>>> + .buf = &buf,
>>>>>>> + }
>>>>>>> + };
>>>>>>> + struct i2c_rdwr_ioctl_data msgset = {
>>>>>>> + .msgs = msgs,
>>>>>>> + .nmsgs = ARRAY_SIZE(msgs),
>>>>>>> + };
>>>>>>> +
>>>>>>> + i2c_adapter = find_i2c_adapter(device, sysfs_fd);
>>>>>>> + igt_assert_lte(0, i2c_adapter);
>>>>>>> +
>>>>>>> + snprintf(i2c_dev, sizeof(i2c_dev), "/dev/i2c-%hd", i2c_adapter);
>>>>>>> + i2c_fd = open(i2c_dev, O_RDWR);>>>>>>> + igt_assert_fd(i2c_fd);
>>>> Can you wait for runtime suspend before triggering the transaction to test
>>> gfx transition from d3 to d0?
>>>
>>> Isn't that something setup_d3() already does?
>> What if i2c_open cause a device ref count in driver?
>
> If so, how does it decrement before ioctl call to allow suspend?
>
> Raag
>
>>>>>>> +
>>>>>>> + /* Perform an i2c transaction to trigger adapter wake */
>>>>>>> + igt_info("Accessing slave 0x%hhx on %s\n", addr, i2c_dev);
>>>>>>> + igt_assert_lte(0, igt_ioctl(i2c_fd, I2C_RDWR, &msgset));
>>>>>>> +
>>>>>>> + close(i2c_fd);
>>>>>>> + return true;
>>>>>>> +}
>>>>>>> +
>>>>>>> igt_main
>>>>>>> {
>>>>>>> device_t device;
>>>>>>> @@ -889,6 +980,13 @@ igt_main
>>>>>>> cleanup_d3(device);
>>>>>>> }
>>>>>>>
>>>>>>> + igt_subtest_f("%s-i2c", d->name) {
>>>>>>> + igt_assert(setup_d3(device, d->state));
>>>>>>> + igt_assert(i2c_test(device, sysfs_fd));
>>>>>>> + igt_assert(in_d3(device, d->state));
>>>>>>> + cleanup_d3(device);
>>>>>>> + }
>>>>>>> +
>>>>>>> igt_subtest_f("%s-multiple-execs", d->name) {
>>>>>>> igt_assert(setup_d3(device, d->state));
>>>>>>> test_exec(device, 16, 32, NO_SUSPEND, d->state, 0);
>>>>>>> --
>>>>>>> 2.34.1
>>>>>>
>>>>>> --
>>>>>> heikki
More information about the igt-dev
mailing list