Re: [PATCH i-g-t] tests/amdgpu: Resolve DRM card index dynamically instead of hardcoding card0

[Alex Hung <alex.hung@amd.com>]

LGTM.

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 6/23/26 01:47, James Lin wrote:
> [Why]
> amd_assr and amd_abm build connector sysfs paths assuming the amdgpu
> device is card0. On systems where simpledrm registers the boot
> framebuffer first, it takes card0 and amdgpu comes up as card1.
> Since DRM minor numbers are never reclaimed, amdgpu stays card1 even
> after simpledrm goes away, so the hardcoded "card0-<connector>" paths
> do not exist and find_aux_dev() and set_abm_level() fail, preventing
> the tests from running.
> 
> [How]
> Open the connector's sysfs directory via igt_connector_sysfs_open(),
> which derives the real DRM card index, and access the AUX device and
> panel_power_savings relative to that directory instead of building a
> card0 path by hand.
> 
> Reviewed-by: Tom Chung <chiahsuan.chung@amd.com>
> Signed-off-by: James Lin <PingLei.Lin@amd.com>
> ---
>   tests/amdgpu/amd_abm.c  | 27 ++++++++++++++++++---------
>   tests/amdgpu/amd_assr.c | 26 ++++++++++++++------------
>   2 files changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/amdgpu/amd_abm.c b/tests/amdgpu/amd_abm.c
> index 207d075b0..ba90344d3 100644
> --- a/tests/amdgpu/amd_abm.c
> +++ b/tests/amdgpu/amd_abm.c
> @@ -35,7 +35,7 @@
>   #define DEBUGFS_CURRENT_BACKLIGHT_PWM "amdgpu_current_backlight_pwm"
>   #define DEBUGFS_TARGET_BACKLIGHT_PWM "amdgpu_target_backlight_pwm"
>   #define BACKLIGHT_PATH "/sys/class/backlight/amdgpu_bl0"
> -#define PANEL_POWER_SAVINGS_PATH "/sys/class/drm/card0-%s/amdgpu/panel_power_savings"
> +#define PANEL_POWER_SAVINGS_ATTR "amdgpu/panel_power_savings"
>   #define MK_COLOR(r, g, b)	((0 << 24) | (r << 16) | (g << 8) | b)
>   
>   typedef struct data {
> @@ -252,14 +252,23 @@ static int backlight_write_brightness(int value)
>   static void set_abm_level(data_t *data, igt_output_t *output, int level)
>   {
>   	char buf[PATH_MAX];
> -	int fd;
> -
> -	igt_assert(snprintf(buf, PATH_MAX, PANEL_POWER_SAVINGS_PATH,
> -			    output->name) < PATH_MAX);
> -
> -	fd = open(buf, O_WRONLY);
> -
> -	igt_skip_on_f(fd == -1, "Cannot find %s. Is it an OLED?\n", buf);
> +	int fd, conn_fd;
> +
> +	/* Open the connector's sysfs directory via the IGT helper, which
> +	 * derives the real DRM card index instead of assuming card0. On
> +	 * systems where the amdgpu device is not card0 (e.g. another DRM
> +	 * node grabbed card0), the previous hardcoded "card0-<conn>" path
> +	 * did not exist and panel_power_savings could never be found.
> +	 */
> +	conn_fd = igt_connector_sysfs_open(data->drm_fd, output->config.connector);
> +	igt_skip_on_f(conn_fd < 0, "Cannot open sysfs dir for connector %s\n",
> +		      output->name);
> +
> +	fd = openat(conn_fd, PANEL_POWER_SAVINGS_ATTR, O_WRONLY);
> +	close(conn_fd);
> +
> +	igt_skip_on_f(fd == -1, "Cannot find %s for %s. Is it an OLED?\n",
> +		      PANEL_POWER_SAVINGS_ATTR, output->name);
>   
>   	igt_assert_eq(snprintf(buf, sizeof(buf), "%d", level),
>   		      write(fd, buf, 1));
> diff --git a/tests/amdgpu/amd_assr.c b/tests/amdgpu/amd_assr.c
> index 71209e79f..ae8420183 100644
> --- a/tests/amdgpu/amd_assr.c
> +++ b/tests/amdgpu/amd_assr.c
> @@ -63,25 +63,27 @@ static void test_fini(data_t *data)
>   static char *find_aux_dev(data_t *data, igt_output_t *output,
>   				char *aux_dev, size_t max_aux_dev_len)
>   {
> -	char sysfs_name[PATH_MAX] = { 0 };
> -	/* +7 only to get rid of snprintf_chk warning.
> -	 * Path name cannot exceed the size of PATH_MAX anyway.
> -	 */
> -	char conn_dir_name[PATH_MAX+7] = { 0 };
>   	DIR *dir;
>   	struct dirent *dirent;
> +	int conn_fd;
>   
>   	aux_dev[0] = 0;
>   
> -	if(igt_sysfs_path(data->fd, sysfs_name, sizeof(sysfs_name))) {
> -			snprintf(conn_dir_name, sizeof(conn_dir_name),
> -					"%s%scard0%s%s",
> -					sysfs_name, "/", "-", output->name);
> -	}
> +	/* Open the connector's sysfs directory via the IGT helper, which
> +	 * derives the real DRM card index instead of assuming card0. On
> +	 * systems where the amdgpu device is not card0 (e.g. another DRM
> +	 * node grabbed card0), the previous hardcoded "card0-<conn>" path
> +	 * did not exist and the AUX device could never be found.
> +	 */
> +	conn_fd = igt_connector_sysfs_open(data->fd, output->config.connector);
> +	if (conn_fd < 0)
> +		return NULL;
>   
> -	dir = opendir(conn_dir_name);
> -	if (!dir)
> +	dir = fdopendir(conn_fd);
> +	if (!dir) {
> +		close(conn_fd);
>   		return NULL;
> +	}
>   
>   	while((dirent = readdir(dir))) {
>   		if (strncmp(dirent->d_name, "drm_dp_aux", sizeof("drm_dp_aux")-1))