[PATCH 1/2] dumb_buffer: Remove invalid-bpp subtest

[PATCH 1/2] dumb_buffer: Remove invalid-bpp subtest

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

These allocations are not necessarily invalid.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 tests/dumb_buffer.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/tests/dumb_buffer.c b/tests/dumb_buffer.c
index 1d4bf296c53e..579d2f906b6d 100644
--- a/tests/dumb_buffer.c
+++ b/tests/dumb_buffer.c
@@ -136,25 +136,6 @@ static void dumb_destroy(int fd, uint32_t handle)
 	igt_assert_eq(__dumb_destroy(fd, handle), 0);
 }
 
-static void invalid_dimensions_test(int fd)
-{
-	struct drm_mode_create_dumb create;
-
-	memset(&create, 0, sizeof(create));
-	create.width = 4032;
-	create.height = 2016;
-	create.bpp = 24;
-	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
-
-	create.bpp = 32;
-	create.width = 0;
-	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
-
-	create.width = 4032;
-	create.height = 0;
-	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
-}
-
 static void valid_dumb_creation_test(int fd)
 {
 	struct drm_mode_create_dumb create = {
@@ -397,9 +378,6 @@ igt_main
 		fd = drm_open_driver(DRIVER_ANY);
 	}
 
-	igt_subtest("invalid-bpp")
-		invalid_dimensions_test(fd);
-
 	igt_subtest("create-valid-dumb")
 		valid_dumb_creation_test(fd);
 
-- 
2.46.0

[PATCH 2/2] device_reset: Require a PCI device

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Too much baked in assumptions that the drm device is a PCI device.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 tests/device_reset.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/device_reset.c b/tests/device_reset.c
index 8ed3ef5220fc..c49f94e2faed 100644
--- a/tests/device_reset.c
+++ b/tests/device_reset.c
@@ -200,6 +200,7 @@ static void init_device_fds(struct device_fds *dev)
 		igt_require_gem(dev->fds.dev);
 
 	igt_assert(device_sysfs_path(dev->fds.dev, dev_path));
+	igt_require(strstr(dev_path, "/sys/devices/pci") == dev_path);
 	addr_pos = strrchr(dev_path, '/');
 	igt_assert(addr_pos);
 	igt_assert_eq(sizeof(dev->dev_bus_addr) - 1,
-- 
2.46.0

✗ Fi.CI.BUILD: failure for series starting with [1/2] dumb_buffer: Remove invalid-bpp subtest

[Patchwork]

== Series Details ==

Series: series starting with [1/2] dumb_buffer: Remove invalid-bpp subtest
URL   : https://patchwork.freedesktop.org/series/137323/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
496e86347f8607d032ed59cdad5eb018e5be6933 tests/intel/xe_module_load: Rename enable_display module param

Tail of build.log:
[1710/1758] Linking target tools/intel_dp_compliance.
[1711/1758] Linking target tools/intel_reg_checker.
[1712/1758] Linking target tools/intel_tiling_detect.
[1713/1758] Linking target tools/xe-perf/xe-perf-reader.
[1714/1758] Linking target tools/intel_vbt_decode.
[1715/1758] Linking target tools/intel_watermark.
[1716/1758] Linking target tools/intel_residency.
[1717/1758] Linking target tools/lsgpu.
[1718/1758] Linking target runner/igt_runner.
[1719/1758] Linking target runner/testdata/skippers.
[1720/1758] Linking target runner/testdata/abort.
[1721/1758] Linking target runner/testdata/abort-dynamic.
[1722/1758] Linking target runner/testdata/abort-fixture.
[1723/1758] Linking target tools/intel_gvtg_test.
[1724/1758] Linking target runner/igt_resume.
[1725/1758] Linking target runner/runner_json_test.
[1726/1758] Linking target runner/testdata/no-subtests.
[1727/1758] Linking target runner/igt_comms_decoder.
[1728/1758] Linking target tools/msm_dp_compliance.
[1729/1758] Linking target runner/testdata/successtest.
[1730/1758] Linking target runner/igt_results.
[1731/1758] Linking target runner/testdata/dynamic.
[1732/1758] Linking target runner/testdata/abort-simple.
[1733/1758] Generating gem_stress.testlist with a meson_exe.py custom command.
[1734/1758] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_mtlgt3.c.o'.
[1735/1758] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt1.c.o'.
[1736/1758] Compiling C object 'runner/527aa9f@@runner_test@exe/runner_tests.c.o'.
[1737/1758] Linking target runner/runner_test.
[1738/1758] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt2.c.o'.
[1739/1758] Compiling C object 'lib/76b5a35@@i915_perf@sha/meson-generated_.._i915_perf_metrics_acmgt3.c.o'.
[1740/1758] Linking target lib/libi915_perf.so.1.5.
[1741/1758] Generating symbol file 'lib/76b5a35@@i915_perf@sha/libi915_perf.so.1.5.symbols'.
[1742/1758] Linking target tools/i915-perf/i915-perf-configs.
[1743/1758] Linking target tools/i915-perf/i915-perf-reader.
[1744/1758] Linking target tools/i915-perf/i915-perf-recorder.
[1745/1758] Linking target tests/core_hotunplug.
[1746/1758] Linking target tests/gem_barrier_race.
[1747/1758] Linking target tests/perf.
[1748/1758] Generating core_hotunplug.testlist with a meson_exe.py custom command.
[1749/1758] Generating gem_barrier_race.testlist with a meson_exe.py custom command.
[1750/1758] Generating perf.testlist with a meson_exe.py custom command.
[1751/1758] Generating xe_tests.rst with a custom command.
[1752/1758] Generating i915_tests.rst with a custom command.
FAILED: docs/testplan/i915_tests.rst 
/usr/src/igt-gpu-tools/scripts/igt_doc.py --config /usr/src/igt-gpu-tools/tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path /opt/igt/build
Warning: Documented igt@dumb_buffer@invalid-bpp doesn't exist on source files
[1753/1758] Generating kms_tests.rst with a custom command.
[1754/1758] Generating intel-ci-tests with a custom command.
[1755/1758] Generating xe_tests.html with a custom command.
ninja: build stopped: subcommand failed.

✗ GitLab.Pipeline: warning for series starting with [1/2] dumb_buffer: Remove invalid-bpp subtest

[Patchwork]

== Series Details ==

Series: series starting with [1/2] dumb_buffer: Remove invalid-bpp subtest
URL   : https://patchwork.freedesktop.org/series/137323/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1249227 for the overview.

build:tests-debian-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265901):
  [1873/1880] Generating kms_tests.rst with a custom command.
  [1874/1880] Generating intel-ci-tests with a custom command.
  [1875/1880] Generating xe_tests.html with a custom command.
  [1876/1880] Compiling C object 'runner/527aa9f@@runner_test@exe/runner_tests.c.o'.
  ninja: build stopped: subcommand failed.
  ninja: Entering directory `build'
  [1/835] Generating version.h with a custom command.
  [2/7] Linking target runner/runner_test.
  [3/7] Linking target assembler/intel-gen4asm.
  [4/7] Generating i915_tests.rst with a custom command.
  FAILED: docs/testplan/i915_tests.rst 
  /builds/gfx-ci/igt-ci-tags/scripts/igt_doc.py --config /builds/gfx-ci/igt-ci-tags/tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path /builds/gfx-ci/igt-ci-tags/build
  Warning: Documented igt@dumb_buffer@invalid-bpp doesn't exist on source files
  ninja: build stopped: subcommand failed.
  section_end:1723666955:step_script
  section_start:1723666955:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666958:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265896):
  [1880/1886] Generating kms_tests.rst with a custom command.
  [1881/1886] Generating intel-ci-tests with a custom command.
  [1882/1886] Generating xe_tests.html with a custom command.
  [1883/1886] Compiling C object 'runner/527aa9f@@runner_test@exe/runner_tests.c.o'.
  ninja: build stopped: subcommand failed.
  ninja: Entering directory `build'
  [1/837] Generating version.h with a custom command.
  [2/6] Linking target runner/runner_test.
  [3/6] Generating kms_tests.html with a custom command.
  [4/6] Generating i915_tests.rst with a custom command.
  FAILED: docs/testplan/i915_tests.rst 
  /builds/gfx-ci/igt-ci-tags/scripts/igt_doc.py --config /builds/gfx-ci/igt-ci-tags/tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path /builds/gfx-ci/igt-ci-tags/build
  Warning: Documented igt@dumb_buffer@invalid-bpp doesn't exist on source files
  ninja: build stopped: subcommand failed.
  section_end:1723666942:step_script
  section_start:1723666942:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666942:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265900):
  /builds/gfx-ci/igt-ci-tags/scripts/igt_doc.py --config /builds/gfx-ci/igt-ci-tags/tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path /builds/gfx-ci/igt-ci-tags/build
  Warning: Documented igt@dumb_buffer@invalid-bpp doesn't exist on source files
  [1882/1886] Generating kms_tests.rst with a custom command.
  [1883/1886] Generating intel-ci-tests with a custom command.
  [1884/1886] Generating xe_tests.html with a custom command.
  ninja: build stopped: subcommand failed.
  ninja: Entering directory `build'
  [1/837] Generating version.h with a custom command.
  [2/5] Generating kms_tests.html with a custom command.
  [3/5] Generating i915_tests.rst with a custom command.
  FAILED: docs/testplan/i915_tests.rst 
  /builds/gfx-ci/igt-ci-tags/scripts/igt_doc.py --config /builds/gfx-ci/igt-ci-tags/tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path /builds/gfx-ci/igt-ci-tags/build
  Warning: Documented igt@dumb_buffer@invalid-bpp doesn't exist on source files
  ninja: build stopped: subcommand failed.
  section_end:1723666985:step_script
  section_start:1723666985:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666986:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libdrm-nouveau has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265899):
  Checking if the job's project is part of a well-known group...
  Thank you for contributing to freedesktop.org
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  Checking out eb54a8b3 as detached HEAD (ref is intel/IGTPW_11581)...
  Removing build/
  Removing lib/i915/perf-configs/__pycache__/
  Removing lib/xe/oa-configs/__pycache__/
  Removing scripts/__pycache__/
  
  Skipping Git submodules setup
  section_end:1723666833:get_sources
  section_start:1723666833:step_script
  Executing "step_script" stage of the job script
  Using docker image sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f for registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora:commit-eb54a8b3b2169a2ed37fc095461963b8382f5d9b with digest registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora@sha256:17d64607d998df2bf29a56b88922d3a598e6f1daa3b51ece2a892c2f293daf83 ...
  section_end:1723666836:step_script
  section_start:1723666836:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666837:cleanup_file_variables
  ERROR: Job failed (system failure): Error response from daemon: no such image: docker.io/library/sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f: image not known (docker.go:652:0s)

build:tests-fedora-no-libunwind has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265897):
  section_start:1723666831:get_sources
  Getting source from Git repository
  $ /host/bin/curl -s -L --cacert /host/ca-certificates.crt --retry 4 -f --retry-delay 60 https://gitlab.freedesktop.org/freedesktop/helm-gitlab-infra/-/raw/main/runner-gating/runner-gating.sh | sh -s -- pre_get_sources_script
  Checking if the user of the pipeline is allowed...
  Checking if the job's project is part of a well-known group...
  Thank you for contributing to freedesktop.org
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  Checking out eb54a8b3 as detached HEAD (ref is intel/IGTPW_11581)...
  
  Skipping Git submodules setup
  section_end:1723666833:get_sources
  section_start:1723666833:step_script
  Executing "step_script" stage of the job script
  Using docker image sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f for registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora:commit-eb54a8b3b2169a2ed37fc095461963b8382f5d9b with digest registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora@sha256:17d64607d998df2bf29a56b88922d3a598e6f1daa3b51ece2a892c2f293daf83 ...
  section_end:1723666836:step_script
  section_start:1723666836:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666838:cleanup_file_variables
  ERROR: Job failed (system failure): Error response from daemon: no such image: docker.io/library/sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f: image not known (docker.go:652:0s)

build:tests-fedora-oldest-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/62265898):
  Checking if the user of the pipeline is allowed...
  Checking if the job's project is part of a well-known group...
  Thank you for contributing to freedesktop.org
  Fetching changes...
  Reinitialized existing Git repository in /builds/gfx-ci/igt-ci-tags/.git/
  Checking out eb54a8b3 as detached HEAD (ref is intel/IGTPW_11581)...
  Removing build/
  Removing installdir/
  Removing undocumented_tests.txt
  
  Skipping Git submodules setup
  section_end:1723666834:get_sources
  section_start:1723666834:step_script
  Executing "step_script" stage of the job script
  Using docker image sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f for registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora:commit-eb54a8b3b2169a2ed37fc095461963b8382f5d9b with digest registry.freedesktop.org/gfx-ci/igt-ci-tags/build-fedora@sha256:17d64607d998df2bf29a56b88922d3a598e6f1daa3b51ece2a892c2f293daf83 ...
  section_end:1723666836:step_script
  section_start:1723666836:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1723666837:cleanup_file_variables
  ERROR: Job failed (system failure): Error response from daemon: no such image: docker.io/library/sha256:4b3054d89ef79f9be95501786fbbbe22857d02c867fff99693808cd80909939f: image not known (docker.go:652:0s)

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/1249227

Re: [PATCH 2/2] device_reset: Require a PCI device

[Kamil Konieczny]

Hi Rob,
On 2024-08-14 at 11:54:26 -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Too much baked in assumptions that the drm device is a PCI device.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

> ---
>  tests/device_reset.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/device_reset.c b/tests/device_reset.c
> index 8ed3ef5220fc..c49f94e2faed 100644
> --- a/tests/device_reset.c
> +++ b/tests/device_reset.c
> @@ -200,6 +200,7 @@ static void init_device_fds(struct device_fds *dev)
>  		igt_require_gem(dev->fds.dev);
>  
>  	igt_assert(device_sysfs_path(dev->fds.dev, dev_path));
> +	igt_require(strstr(dev_path, "/sys/devices/pci") == dev_path);
>  	addr_pos = strrchr(dev_path, '/');
>  	igt_assert(addr_pos);
>  	igt_assert_eq(sizeof(dev->dev_bus_addr) - 1,
> -- 
> 2.46.0
> 

Re: [PATCH 1/2] dumb_buffer: Remove invalid-bpp subtest

[Kamil Konieczny]

Hi Rob,
On 2024-08-14 at 11:54:25 -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> These allocations are not necessarily invalid.

Please write a little more why. Also look at gitlab log,
you need to delete comments with this subtest name and description
as our CI is creating testlist at build time and that info
is there.

Add also tests/ prefix in subject:

[PATCH 1/2] tests/dumb_buffer: Remove invalid-bpp subtest

> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  tests/dumb_buffer.c | 22 ----------------------
>  1 file changed, 22 deletions(-)
> 
> diff --git a/tests/dumb_buffer.c b/tests/dumb_buffer.c
> index 1d4bf296c53e..579d2f906b6d 100644
> --- a/tests/dumb_buffer.c
> +++ b/tests/dumb_buffer.c
> @@ -136,25 +136,6 @@ static void dumb_destroy(int fd, uint32_t handle)
>  	igt_assert_eq(__dumb_destroy(fd, handle), 0);
>  }
>  
> -static void invalid_dimensions_test(int fd)
> -{
> -	struct drm_mode_create_dumb create;
> -
> -	memset(&create, 0, sizeof(create));
> -	create.width = 4032;
> -	create.height = 2016;
> -	create.bpp = 24;
> -	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);

These use 3-Bytes bpp with divisible by 4 screen W x H, so here
you could be right with removing this.

> -
> -	create.bpp = 32;
> -	create.width = 0;
> -	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);

Why 0-width is valid?

> -
> -	create.width = 4032;
> -	create.height = 0;
> -	igt_assert_eq(__dumb_create(fd, &create), -EINVAL);

Same here, imho looks invalid. Btw what about other settings?
Like bpp not divisible by 8 bits?

Regards,
Kamil

> -}
> -
>  static void valid_dumb_creation_test(int fd)
>  {
>  	struct drm_mode_create_dumb create = {
> @@ -397,9 +378,6 @@ igt_main
>  		fd = drm_open_driver(DRIVER_ANY);
>  	}
>  
> -	igt_subtest("invalid-bpp")
> -		invalid_dimensions_test(fd);
> -
>  	igt_subtest("create-valid-dumb")
>  		valid_dumb_creation_test(fd);
>  
> -- 
> 2.46.0
> 

Re: [PATCH 2/2] device_reset: Require a PCI device

[Kamil Konieczny]

Hi Rob,
On 2024-08-14 at 11:54:26 -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>

One nit about subject, please use 'tests/' prefix:

[PATCH 2/2] device_reset: Require a PCI device

should be:

[PATCH 2/2] tests/device_reset: Require a PCI device

> 
> Too much baked in assumptions that the drm device is a PCI device.

Do you have examples where it is not true (e.g. not a PCI device)
and reset is possible? Just curious.

Regards,
Kamil

> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  tests/device_reset.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/device_reset.c b/tests/device_reset.c
> index 8ed3ef5220fc..c49f94e2faed 100644
> --- a/tests/device_reset.c
> +++ b/tests/device_reset.c
> @@ -200,6 +200,7 @@ static void init_device_fds(struct device_fds *dev)
>  		igt_require_gem(dev->fds.dev);
>  
>  	igt_assert(device_sysfs_path(dev->fds.dev, dev_path));
> +	igt_require(strstr(dev_path, "/sys/devices/pci") == dev_path);
>  	addr_pos = strrchr(dev_path, '/');
>  	igt_assert(addr_pos);
>  	igt_assert_eq(sizeof(dev->dev_bus_addr) - 1,
> -- 
> 2.46.0
> 

Re: [PATCH 1/2] dumb_buffer: Remove invalid-bpp subtest

[Rob Clark]

On Mon, Aug 19, 2024 at 10:16 AM Kamil Konieczny
<kamil.konieczny@linux.intel.com> wrote:
>
> Hi Rob,
> On 2024-08-14 at 11:54:25 -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > These allocations are not necessarily invalid.
>
> Please write a little more why. Also look at gitlab log,
> you need to delete comments with this subtest name and description
> as our CI is creating testlist at build time and that info
> is there.
>
> Add also tests/ prefix in subject:
>
> [PATCH 1/2] tests/dumb_buffer: Remove invalid-bpp subtest
>
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  tests/dumb_buffer.c | 22 ----------------------
> >  1 file changed, 22 deletions(-)
> >
> > diff --git a/tests/dumb_buffer.c b/tests/dumb_buffer.c
> > index 1d4bf296c53e..579d2f906b6d 100644
> > --- a/tests/dumb_buffer.c
> > +++ b/tests/dumb_buffer.c
> > @@ -136,25 +136,6 @@ static void dumb_destroy(int fd, uint32_t handle)
> >       igt_assert_eq(__dumb_destroy(fd, handle), 0);
> >  }
> >
> > -static void invalid_dimensions_test(int fd)
> > -{
> > -     struct drm_mode_create_dumb create;
> > -
> > -     memset(&create, 0, sizeof(create));
> > -     create.width = 4032;
> > -     create.height = 2016;
> > -     create.bpp = 24;
> > -     igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
>
> These use 3-Bytes bpp with divisible by 4 screen W x H, so here
> you could be right with removing this.

I guess I could instead create.bpp = 0, since that is the only case
that the kernel actually rejects

BR,
-R

>
> > -
> > -     create.bpp = 32;
> > -     create.width = 0;
> > -     igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
>
> Why 0-width is valid?
>
> > -
> > -     create.width = 4032;
> > -     create.height = 0;
> > -     igt_assert_eq(__dumb_create(fd, &create), -EINVAL);
>
> Same here, imho looks invalid. Btw what about other settings?
> Like bpp not divisible by 8 bits?
>
> Regards,
> Kamil
>
> > -}
> > -
> >  static void valid_dumb_creation_test(int fd)
> >  {
> >       struct drm_mode_create_dumb create = {
> > @@ -397,9 +378,6 @@ igt_main
> >               fd = drm_open_driver(DRIVER_ANY);
> >       }
> >
> > -     igt_subtest("invalid-bpp")
> > -             invalid_dimensions_test(fd);
> > -
> >       igt_subtest("create-valid-dumb")
> >               valid_dumb_creation_test(fd);
> >
> > --
> > 2.46.0
> >

Re: [PATCH 2/2] device_reset: Require a PCI device

[Rob Clark]

On Mon, Aug 19, 2024 at 10:37 AM Kamil Konieczny
<kamil.konieczny@linux.intel.com> wrote:
>
> Hi Rob,
> On 2024-08-14 at 11:54:26 -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
>
> One nit about subject, please use 'tests/' prefix:
>
> [PATCH 2/2] device_reset: Require a PCI device
>
> should be:
>
> [PATCH 2/2] tests/device_reset: Require a PCI device
>
> >
> > Too much baked in assumptions that the drm device is a PCI device.
>
> Do you have examples where it is not true (e.g. not a PCI device)
> and reset is possible? Just curious.

maybe usb devices?

Platform devices cannot be reset at the bus level, since there is no bus.

BR,
-R

> Regards,
> Kamil
>
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  tests/device_reset.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tests/device_reset.c b/tests/device_reset.c
> > index 8ed3ef5220fc..c49f94e2faed 100644
> > --- a/tests/device_reset.c
> > +++ b/tests/device_reset.c
> > @@ -200,6 +200,7 @@ static void init_device_fds(struct device_fds *dev)
> >               igt_require_gem(dev->fds.dev);
> >
> >       igt_assert(device_sysfs_path(dev->fds.dev, dev_path));
> > +     igt_require(strstr(dev_path, "/sys/devices/pci") == dev_path);
> >       addr_pos = strrchr(dev_path, '/');
> >       igt_assert(addr_pos);
> >       igt_assert_eq(sizeof(dev->dev_bus_addr) - 1,
> > --
> > 2.46.0
> >

Re: [PATCH 2/2] device_reset: Require a PCI device

[Kamil Konieczny]

Hi Rob,
On 2024-08-19 at 11:51:51 -0700, Rob Clark wrote:
> On Mon, Aug 19, 2024 at 10:37 AM Kamil Konieczny
> <kamil.konieczny@linux.intel.com> wrote:
> >
> > Hi Rob,
> > On 2024-08-14 at 11:54:26 -0700, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> >
> > One nit about subject, please use 'tests/' prefix:
> >
> > [PATCH 2/2] device_reset: Require a PCI device
> >
> > should be:
> >
> > [PATCH 2/2] tests/device_reset: Require a PCI device
> >
> > >
> > > Too much baked in assumptions that the drm device is a PCI device.
> >
> > Do you have examples where it is not true (e.g. not a PCI device)
> > and reset is possible? Just curious.
> 
> maybe usb devices?
> 
> Platform devices cannot be reset at the bus level, since there is no bus.
> 
> BR,
> -R
> 

You are right, could you add a comment in code?
Imho it could help as a reminder that there are such devices.
Or at least document it in commit description.

Regards,
Kamil

> > Regards,
> > Kamil
> >
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > >  tests/device_reset.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tests/device_reset.c b/tests/device_reset.c
> > > index 8ed3ef5220fc..c49f94e2faed 100644
> > > --- a/tests/device_reset.c
> > > +++ b/tests/device_reset.c
> > > @@ -200,6 +200,7 @@ static void init_device_fds(struct device_fds *dev)
> > >               igt_require_gem(dev->fds.dev);
> > >
> > >       igt_assert(device_sysfs_path(dev->fds.dev, dev_path));
> > > +     igt_require(strstr(dev_path, "/sys/devices/pci") == dev_path);
> > >       addr_pos = strrchr(dev_path, '/');
> > >       igt_assert(addr_pos);
> > >       igt_assert_eq(sizeof(dev->dev_bus_addr) - 1,
> > > --
> > > 2.46.0
> > >