Re: [i-g-t,v3] tests/intel/xe_pm: Introduce i2c subtests

["Poosa, Karthik" <karthik.poosa@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4657 bytes --]


On 23-07-2025 17:46, 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
> before and after i2c adapter access for the devices that support it.
>
> v2: Define macros for AMC constants (Lucas)
>      s/index/adapter (Lucas)
> v3: Wait for runtime suspend before ioctl (Anshuman)
>      Make i2c_test() void (Riana)
>      Fix test description (Riana)
>
> Signed-off-by: Raag Jadav<raag.jadav@intel.com>
> Reviewed-by: Heikki Krogerus<heikki.krogerus@linux.intel.com>
> ---
>   tests/intel/xe_pm.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 101 insertions(+)
>
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index 16b5fc686..838b8cb4a 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,90 @@ 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 runtime suspend before and after
> + * 	i2c adapter access.
> + * Functionality: pm-d3
> + * GPU requirements: D3 feature should be supported
> + *
> + * arg[1]:
> + *
> + * @d3hot:	d3hot
> + * @d3cold:	d3cold
> + */
> +static void i2c_test(device_t device, int sysfs_fd)
> +{
> +	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 = &reg,
> +		}, {
> +			.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);
> +
> +	/* Make sure open() doesn't cause runtime resume */
> +	igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED);
> +
> +	/* 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);
> +}
> +
>   igt_main
>   {
>   	device_t device;
> @@ -889,6 +983,13 @@ igt_main
>   			cleanup_d3(device);
>   		}
>   
> +		igt_subtest_f("%s-i2c", d->name) {
> +			igt_assert(setup_d3(device, d->state));
> +			i2c_test(device, sysfs_fd);
> +			igt_assert(in_d3(device, d->state));
> +			cleanup_d3(device);
> +		}
> +

Hi Raag,

1. This LGTM.

Acked-by: Karthik Poosa <karthik.poosa@intel.com>

2. Could you add a new sub-test which runs i2c_test, after the system 
suspend and resume cycle, similar to exec-after test ?

If there is any issue with I2C after system suspend/resume, we would 
know with that.

You can add it as a separate patch if you want.

>   		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);

[-- Attachment #2: Type: text/html, Size: 5825 bytes --]