| Message ID | 20220318021046.40348-1-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce multitile support | expand |
> • igt@i915_selftest@mock@requests: > > □ shard-kbl: PASS -> DMESG-FAIL > > □ shard-tglb: PASS -> DMESG-FAIL > > □ shard-apl: PASS -> DMESG-FAIL > > □ shard-glk: PASS -> DMESG-FAIL > > □ shard-skl: PASS -> DMESG-FAIL > > □ shard-snb: PASS -> DMESG-FAIL > > □ shard-iclb: PASS -> DMESG-FAIL I don't see how these failures can be related to the series I sent. Maybe a false positive? Andi
On Fri, 18 Mar 2022 at 08:18, Andi Shyti <andi.shyti@linux.intel.com> wrote: > > > • igt@i915_selftest@mock@requests: > > > > □ shard-kbl: PASS -> DMESG-FAIL > > > > □ shard-tglb: PASS -> DMESG-FAIL > > > > □ shard-apl: PASS -> DMESG-FAIL > > > > □ shard-glk: PASS -> DMESG-FAIL > > > > □ shard-skl: PASS -> DMESG-FAIL > > > > □ shard-snb: PASS -> DMESG-FAIL > > > > □ shard-iclb: PASS -> DMESG-FAIL > > I don't see how these failures can be related to the series I > sent. > > Maybe a false positive? AFAICT these look new. Did we forget to do something for the mock_device? Maybe something in patch 3? Nothing is jumping out at me... > > Andi
On 18/03/2022 13:25, Matthew Auld wrote: > On Fri, 18 Mar 2022 at 08:18, Andi Shyti <andi.shyti@linux.intel.com> wrote: >> >>> • igt@i915_selftest@mock@requests: >>> >>> □ shard-kbl: PASS -> DMESG-FAIL >>> >>> □ shard-tglb: PASS -> DMESG-FAIL >>> >>> □ shard-apl: PASS -> DMESG-FAIL >>> >>> □ shard-glk: PASS -> DMESG-FAIL >>> >>> □ shard-skl: PASS -> DMESG-FAIL >>> >>> □ shard-snb: PASS -> DMESG-FAIL >>> >>> □ shard-iclb: PASS -> DMESG-FAIL >> >> I don't see how these failures can be related to the series I >> sent. >> >> Maybe a false positive? > > AFAICT these look new. Did we forget to do something for the > mock_device? Maybe something in patch 3? Nothing is jumping out at > me... Yeah to "sus" :) [I like so don't recognise much of that patch I am supposed to be author of... I think it moved on a lot since my version. Anyway.. onto the bug.] Module init (executed in order): { .init = i915_mock_selftests }, -> this is the part which runs mock selftests ... { .init = i915_pci_register_driver, -> this is the part which sets up i915->gt[0] It happens via i915_pci_register_driver -> i915_pci_probe -> i915_driver_probe -> intel_gt_probe_all. Mock cleanup does: mock_device_release + intel_gt_driver_late_release(i915); -> + for_each_gt(gt, i915, id) { + intel_uc_driver_late_release(>->uc); + intel_gt_fini_requests(gt); + intel_gt_fini_reset(gt); + intel_gt_fini_timelines(gt); + intel_engines_free(gt); + } Hence I think for_each_gt does not see i915->gt[0] being set ergo does nothing. I also don't like the signature changes like: -void intel_gt_driver_late_release(struct intel_gt *gt) +void intel_gt_driver_late_release(struct drm_i915_private *i915) If it has to live in intel_gt.c, maybe at least use the "_all" suffix more consistently? Regards, Tvrtko
Hi Matt and Tvrtko, > On 18/03/2022 13:25, Matthew Auld wrote: > > On Fri, 18 Mar 2022 at 08:18, Andi Shyti <andi.shyti@linux.intel.com> wrote: > > > > > > > • igt@i915_selftest@mock@requests: > > > > > > > > □ shard-kbl: PASS -> DMESG-FAIL > > > > > > > > □ shard-tglb: PASS -> DMESG-FAIL > > > > > > > > □ shard-apl: PASS -> DMESG-FAIL > > > > > > > > □ shard-glk: PASS -> DMESG-FAIL > > > > > > > > □ shard-skl: PASS -> DMESG-FAIL > > > > > > > > □ shard-snb: PASS -> DMESG-FAIL > > > > > > > > □ shard-iclb: PASS -> DMESG-FAIL > > > > > > I don't see how these failures can be related to the series I > > > sent. > > > > > > Maybe a false positive? > > > > AFAICT these look new. Did we forget to do something for the > > mock_device? Maybe something in patch 3? Nothing is jumping out at > > me... it was of course suspicious, but I spent quite a lot of time at fixing the mock selftests, until I got all greens on trybot. But then, another refactoring happened... > Yeah to "sus" :) > > [I like so don't recognise much of that patch I am supposed to be author of... I think it moved on a lot since my version. Anyway.. onto the bug.] > > Module init (executed in order): > > { .init = i915_mock_selftests }, -> this is the part which runs mock selftests > ... > { .init = i915_pci_register_driver, -> this is the part which sets up i915->gt[0] > > It happens via i915_pci_register_driver -> i915_pci_probe -> i915_driver_probe -> intel_gt_probe_all. > > Mock cleanup does: > > mock_device_release > > + intel_gt_driver_late_release(i915); > > -> > > + for_each_gt(gt, i915, id) { > + intel_uc_driver_late_release(>->uc); > + intel_gt_fini_requests(gt); > + intel_gt_fini_reset(gt); > + intel_gt_fini_timelines(gt); > + intel_engines_free(gt); > + } > > Hence I think for_each_gt does not see i915->gt[0] being set ergo does nothing. goot point, I'm missing a i915->gt[0] = gt; somewhere, that is supposed to happen in the probe_all. Thanks! > I also don't like the signature changes like: > > -void intel_gt_driver_late_release(struct intel_gt *gt) > +void intel_gt_driver_late_release(struct drm_i915_private *i915) > > If it has to live in intel_gt.c, maybe at least use the "_all" suffix more consistently? yes... not nice indeed. Also Michal complained. I will add the "_all" suffix. Didn't want to make very long function names at first. Andi
Hi, This is the second series that prepares i915 to host multitile platforms. It introduces the for_each_gt() macro that loops over the tiles to perform per gt actions. This patch is a combination of two patches developed originally by Abdiel, who introduced some refactoring during probe, and then Tvrtko has added the necessary tools to start using the various tiles. The second patch re-organises the sysfs interface to expose the API for each of the GTs. I decided to prioritise this patch over others to unblock Sujaritha for further development. A third series will still follow this. Thanks Michal and Andrzej for the reviews and support! Thanks, Andi Patchwork: https://patchwork.freedesktop.org/series/98741/ Changelog ========= v5 -> v6 - address all Michal and Andrzej's reviews that consist mainly in code refactoring. v4 -> v5 - fixed Michal's reviews. - the sysfs patches have been split in 3 parts to make reviews easier. - Sujaritha's patch on pm throttle has been queued. - INTEL_REGION_LMEM has been renamed to INTEL_REGION_LMEM_0 - added the gt_is_root() helper - the sysfs files will be called intel_gt_sysfs_* instead of sysfs_gt_* v3 -> v4 - fixed Tvrtko's review: - remove the SYSFS_DEPRECATED_V2 mention from the commit log - reworded the error message when accessing deprecated files - errors in sysfs are printed as warnings as they are not fatal - the inline functions are moved to be out of line. and some other minor refactoring. v2 -> v3 - Added Matt and Sujaritha's r-b for patch 1 and 2. - Reworded the commit of patch 2 to underline the fact that the interface is useful also when used manually. v1 -> v2 - fixed a couple of coding style issues in patch 2. Andi Shyti (5): drm/i915: Rename INTEL_REGION_LMEM with INTEL_REGION_LMEM_0 drm/i915/gt: add gt_is_root() helper drm/i915/gt: create per-tile sysfs interface drm/i915/gt: Create per-tile RC6 sysfs interface drm/i915/gt: Create per-tile RPS sysfs interfaces Sujaritha Sundaresan (1): drm/i915/gt: Adding new sysfs frequency attributes Tvrtko Ursulin (1): drm/i915: Prepare for multiple GTs drivers/gpu/drm/i915/Makefile | 2 + drivers/gpu/drm/i915/display/intel_fb.c | 2 +- drivers/gpu/drm/i915/display/intel_fb_pin.c | 2 +- .../drm/i915/display/intel_plane_initial.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 4 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 6 +- .../drm/i915/gem/selftests/i915_gem_migrate.c | 8 +- drivers/gpu/drm/i915/gt/intel_gt.c | 135 +++- drivers/gpu/drm/i915/gt/intel_gt.h | 22 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +- drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 122 ++++ drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 34 + drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 601 ++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.h | 15 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 + drivers/gpu/drm/i915/gt/intel_rps.c | 18 + drivers/gpu/drm/i915/gt/intel_rps.h | 4 + drivers/gpu/drm/i915/i915_driver.c | 28 +- drivers/gpu/drm/i915/i915_drv.h | 8 + drivers/gpu/drm/i915/i915_reg.h | 11 + drivers/gpu/drm/i915/i915_sysfs.c | 310 +-------- drivers/gpu/drm/i915/i915_sysfs.h | 3 + drivers/gpu/drm/i915/intel_memory_region.c | 2 +- drivers/gpu/drm/i915/intel_memory_region.h | 7 +- drivers/gpu/drm/i915/intel_uncore.c | 11 +- drivers/gpu/drm/i915/intel_uncore.h | 3 +- .../gpu/drm/i915/selftests/mock_gem_device.c | 7 +- scripts/extract-cert | Bin 0 -> 17952 bytes 28 files changed, 1017 insertions(+), 366 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_sysfs.c create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_sysfs.h create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c create mode 100644 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.h create mode 100755 scripts/extract-cert