Re: Must-Pass Test Suite for KMS drivers
On Mon, Nov 7, 2022 at 1:29 AM Maxime Ripard wrote: > > On Thu, Oct 27, 2022 at 08:08:28AM -0700, Rob Clark wrote: > > On Wed, Oct 26, 2022 at 1:17 AM wrote: > > > > > > Hi Rob, > > > > > > On Mon, Oct 24, 2022 at 08:48:15AM -0700, Rob Clark wrote: > > > > On Mon, Oct 24, 2022 at 5:43 AM wrote: > > > > > I've discussing the idea for the past year to add an IGT test suite > > > > > that > > > > > all well-behaved KMS drivers must pass. > > > > > > > > > > The main idea behind it comes from v4l2-compliance and cec-compliance, > > > > > that are being used to validate that the drivers are sane. > > > > > > > > > > We should probably start building up the test list, and eventually > > > > > mandate that all tests pass for all the new KMS drivers we would merge > > > > > in the kernel, and be run by KCi or similar. > > > > > > > > Let's get https://patchwork.freedesktop.org/patch/502641/ merged > > > > first, that already gives us a mechanism similar to what we use in > > > > mesa to track pass/fail/flake > > > > > > I'm not sure it's a dependency per-se, and I believe both can (and > > > should) happen separately. > > > > Basically my reasoning is that getting IGT green is a process that so > > far is consisting of equal parts IGT test fixes, to clear out > > lingering i915'isms, etc, and driver fixes. Yes, you could do this > > manually but the drm/ci approach seems like it would make it easier to > > track, so it is easier to see what tests are being run on which hw, > > and what the pass/fail/flake status is. And the expectation files can > > also be updated as we uprev the igt version being used in CI. > > > > I could be biased by how CI has been deployed (IMHO, successfully) in > > mesa.. my experience there doesn't make me see any value in a > > "mustpass" list. But does make me see value in automating and > > tracking status. Obviously we want all the tests to pass, but getting > > there is going to be a process. Tracking that progress is the thing > > that is useful now. > > Yeah, I understand where you're coming from, and for CI I agree that > your approach looks like the best one. > > It's not what I'm trying to address though. > > My issue is that, even though I have a bunch of KMS experience by now, > every time I need to use IGT, I have exactly zero idea what test I > need to run to check that a given driver behaves decently. > > I have no idea which tests I should run, which tests are supposed to be > working but can't really because of some intel-specific behavior, which > tests are skipped but shouldn't, which tests are broken but should be, > etc. yeah, I feel your pain.. I think the best suggestion I can make atm is to compare to the xfails from the other drivers, and if in doubt ask on #igt BR, -R > I don't want to have a nice table with everything green because there > was no regression, I want to see which bugs I haven't found out are > still lingering in my driver. I've been chasing bugs too many times > where it turned out that there was a test for that in IGT somewhere, > hidden in a 70k tests haystack with zero documentation. > > So, yeah, I get what you're saying, it makes sense, and please go > forward with drm/ci. I still think we need to find a beginning of a > solution for the issue I'm talking about. > > Maxime
Re: Must-Pass Test Suite for KMS drivers
Hi Am 07.11.22 um 10:30 schrieb Maxime Ripard: Hi Thomas, On Fri, Oct 28, 2022 at 09:31:38AM +0200, Thomas Zimmermann wrote: Am 24.10.22 um 14:43 schrieb max...@cerno.tech: I've discussing the idea for the past year to add an IGT test suite that all well-behaved KMS drivers must pass. The main idea behind it comes from v4l2-compliance and cec-compliance, that are being used to validate that the drivers are sane. We should probably start building up the test list, and eventually mandate that all tests pass for all the new KMS drivers we would merge in the kernel, and be run by KCi or similar. I did a first pass to create a draft of such a test-suite, which would contain: igt@core_auth@basic-auth igt@core_auth@getclient-master-drop igt@core_auth@getclient-simple igt@core_auth@many-magics igt@core_getclient igt@core_getstats igt@core_getversion igt@core_hotunplug@hotrebind-lateclose igt@core_hotunplug@hotunbind-rebind igt@core_hotunplug@unbind-rebind igt@core_setmaster igt@core_setmaster_vs_auth igt@device_reset@unbind-reset-rebind igt@drm_read igt@dumb_buffer igt@fbdev Maybe we make this test optional? At least sprd decided to not support fbdev at all. I'm not sure we need to make that test optional, or at least, we should run it all the time, but skip it if there's no fbdev emulation, and not report it as a failure? Sure. I just meant that fbdev support shouldn't be a blocker. If there, it should work of course. Best regards Thomas Maxime -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: Must-Pass Test Suite for KMS drivers
Hi Thomas, On Fri, Oct 28, 2022 at 09:31:38AM +0200, Thomas Zimmermann wrote: > Am 24.10.22 um 14:43 schrieb max...@cerno.tech: > > I've discussing the idea for the past year to add an IGT test suite that > > all well-behaved KMS drivers must pass. > > > > The main idea behind it comes from v4l2-compliance and cec-compliance, > > that are being used to validate that the drivers are sane. > > > > We should probably start building up the test list, and eventually > > mandate that all tests pass for all the new KMS drivers we would merge > > in the kernel, and be run by KCi or similar. > > > > I did a first pass to create a draft of such a test-suite, which would > > contain: > > > > igt@core_auth@basic-auth > > igt@core_auth@getclient-master-drop > > igt@core_auth@getclient-simple > > igt@core_auth@many-magics > > igt@core_getclient > > igt@core_getstats > > igt@core_getversion > > igt@core_hotunplug@hotrebind-lateclose > > igt@core_hotunplug@hotunbind-rebind > > igt@core_hotunplug@unbind-rebind > > igt@core_setmaster > > igt@core_setmaster_vs_auth > > igt@device_reset@unbind-reset-rebind > > igt@drm_read > > igt@dumb_buffer > > igt@fbdev > > Maybe we make this test optional? At least sprd decided to not support fbdev > at all. I'm not sure we need to make that test optional, or at least, we should run it all the time, but skip it if there's no fbdev emulation, and not report it as a failure? Maxime
Re: Must-Pass Test Suite for KMS drivers
On Thu, Oct 27, 2022 at 08:08:28AM -0700, Rob Clark wrote: > On Wed, Oct 26, 2022 at 1:17 AM wrote: > > > > Hi Rob, > > > > On Mon, Oct 24, 2022 at 08:48:15AM -0700, Rob Clark wrote: > > > On Mon, Oct 24, 2022 at 5:43 AM wrote: > > > > I've discussing the idea for the past year to add an IGT test suite that > > > > all well-behaved KMS drivers must pass. > > > > > > > > The main idea behind it comes from v4l2-compliance and cec-compliance, > > > > that are being used to validate that the drivers are sane. > > > > > > > > We should probably start building up the test list, and eventually > > > > mandate that all tests pass for all the new KMS drivers we would merge > > > > in the kernel, and be run by KCi or similar. > > > > > > Let's get https://patchwork.freedesktop.org/patch/502641/ merged > > > first, that already gives us a mechanism similar to what we use in > > > mesa to track pass/fail/flake > > > > I'm not sure it's a dependency per-se, and I believe both can (and > > should) happen separately. > > Basically my reasoning is that getting IGT green is a process that so > far is consisting of equal parts IGT test fixes, to clear out > lingering i915'isms, etc, and driver fixes. Yes, you could do this > manually but the drm/ci approach seems like it would make it easier to > track, so it is easier to see what tests are being run on which hw, > and what the pass/fail/flake status is. And the expectation files can > also be updated as we uprev the igt version being used in CI. > > I could be biased by how CI has been deployed (IMHO, successfully) in > mesa.. my experience there doesn't make me see any value in a > "mustpass" list. But does make me see value in automating and > tracking status. Obviously we want all the tests to pass, but getting > there is going to be a process. Tracking that progress is the thing > that is useful now. Yeah, I understand where you're coming from, and for CI I agree that your approach looks like the best one. It's not what I'm trying to address though. My issue is that, even though I have a bunch of KMS experience by now, every time I need to use IGT, I have exactly zero idea what test I need to run to check that a given driver behaves decently. I have no idea which tests I should run, which tests are supposed to be working but can't really because of some intel-specific behavior, which tests are skipped but shouldn't, which tests are broken but should be, etc. I don't want to have a nice table with everything green because there was no regression, I want to see which bugs I haven't found out are still lingering in my driver. I've been chasing bugs too many times where it turned out that there was a test for that in IGT somewhere, hidden in a 70k tests haystack with zero documentation. So, yeah, I get what you're saying, it makes sense, and please go forward with drm/ci. I still think we need to find a beginning of a solution for the issue I'm talking about. Maxime
Re: Must-Pass Test Suite for KMS drivers
Hi Maxime Am 24.10.22 um 14:43 schrieb max...@cerno.tech: Hi, I've discussing the idea for the past year to add an IGT test suite that all well-behaved KMS drivers must pass. The main idea behind it comes from v4l2-compliance and cec-compliance, that are being used to validate that the drivers are sane. We should probably start building up the test list, and eventually mandate that all tests pass for all the new KMS drivers we would merge in the kernel, and be run by KCi or similar. I did a first pass to create a draft of such a test-suite, which would contain: igt@core_auth@basic-auth igt@core_auth@getclient-master-drop igt@core_auth@getclient-simple igt@core_auth@many-magics igt@core_getclient igt@core_getstats igt@core_getversion igt@core_hotunplug@hotrebind-lateclose igt@core_hotunplug@hotunbind-rebind igt@core_hotunplug@unbind-rebind igt@core_setmaster igt@core_setmaster_vs_auth igt@device_reset@unbind-reset-rebind igt@drm_read igt@dumb_buffer igt@fbdev Maybe we make this test optional? At least sprd decided to not support fbdev at all. Best regards Thomas igt@feature_discovery@display igt@kms_3d igt@kms_addfb_basic igt@kms_async_flips igt@kms_color igt@kms_concurrent igt@kms_cursor_crc igt@kms_cursor_edge_walk igt@kms_cursor_legacy@basic-busy-flip-before-cursor igt@kms_cursor_legacy@basic-flip-after-cursor igt@kms_cursor_legacy@basic-flip-after-cursor igt@kms_display_modes igt@kms_dither igt@kms_dp_aux_dev igt@kms_flip@basic-flip-vs-dpms igt@kms_flip@basic-flip-vs-modeset igt@kms_flip@basic-flip-vs-wf_vblank igt@kms_flip@basic-plain-flip igt@kms_flip_event_leak@basic igt@kms_force_connector_basic@force-connector-state igt@kms_force_connector_basic@force-edid igt@kms_force_connector_basic@force-load-detect igt@kms_force_connector_basic@prune-stale-modes igt@kms_getfb igt@kms_hdmi_inject igt@kms_hdr igt@kms_invalid_mode igt@kms_lease igt@kms_panel_fitting igt@kms_pipe_crc_basic igt@kms_plane_alpha_blend igt@kms_plane igt@kms_plane_cursor igt@kms_plane_lowres igt@kms_plane_multiple igt@kms_plane_scaling igt@kms_prop_blob igt@kms_properties igt@kms_rmfb igt@kms_scaling_modes igt@kms_sequence igt@kms_setmode igt@kms_sysfs_edid_timing igt@kms_tv_load_detect igt@kms_universal_plane igt@kms_vblank igt@kms_vrr igt@kms_writeback Most of them are skipped on vc4 right now, but I could see that some of them fail already (kms_rmfb, core_hotunplug), so it proves to be useful already. What do you think? Is there some more tests needed, or did I include some tests that shouldn't have been there? Thanks! Maxime -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: Must-Pass Test Suite for KMS drivers
On Wed, Oct 26, 2022 at 1:17 AM wrote: > > Hi Rob, > > On Mon, Oct 24, 2022 at 08:48:15AM -0700, Rob Clark wrote: > > On Mon, Oct 24, 2022 at 5:43 AM wrote: > > > I've discussing the idea for the past year to add an IGT test suite that > > > all well-behaved KMS drivers must pass. > > > > > > The main idea behind it comes from v4l2-compliance and cec-compliance, > > > that are being used to validate that the drivers are sane. > > > > > > We should probably start building up the test list, and eventually > > > mandate that all tests pass for all the new KMS drivers we would merge > > > in the kernel, and be run by KCi or similar. > > > > Let's get https://patchwork.freedesktop.org/patch/502641/ merged > > first, that already gives us a mechanism similar to what we use in > > mesa to track pass/fail/flake > > I'm not sure it's a dependency per-se, and I believe both can (and > should) happen separately. Basically my reasoning is that getting IGT green is a process that so far is consisting of equal parts IGT test fixes, to clear out lingering i915'isms, etc, and driver fixes. Yes, you could do this manually but the drm/ci approach seems like it would make it easier to track, so it is easier to see what tests are being run on which hw, and what the pass/fail/flake status is. And the expectation files can also be updated as we uprev the igt version being used in CI. I could be biased by how CI has been deployed (IMHO, successfully) in mesa.. my experience there doesn't make me see any value in a "mustpass" list. But does make me see value in automating and tracking status. Obviously we want all the tests to pass, but getting there is going to be a process. Tracking that progress is the thing that is useful now. BR, -R > AFAIU, the CI patches are here to track which tests are supposed to be > working and which aren't so that we can track regressions. > > The list I was talking about is here to identify issues in the first > place. All tests must pass, and if one fails it should be considered a > hard failure. > > This would be eligible for CI only for drivers which have been known to > pass them all already, but we wouldn't need to track which ones can fail > or not, all of them must. > > > Beyond that, I think some of the igt tests need to get more stable > > before we could consider a "mustpass" list. > > I agree that IGT tests could get more stable on ARM platforms, but it's > also a chicken-and-egg issue. If no-one is using them regularly on ARM, > then they'll never get fixed. > > > The kms_lease tests seem to fail on msm due to bad assumptions in the > > test about which CRTCs primary planes can attach to. The legacy-cursor > > crc tests seem a bit racy (there was a patch posted for that, not sure > > if it landed yet), etc. > > And this is fine, we can merge that list without them, and if and when > they get stable, we'll add them later. > > > The best thing to do is actually start running CI and tracking xfails > > and flakes ;-) > > Again, I wouldn't oppose them. > > The issue I'm trying to solve is that there's just no way to know, at > the moment: > > - When you're running IGT, which tests are relevant for your platform > exactly. > > - If some of them fail, is it expected for them to fail or not. The > ci/ patch you mentioned help for that a bit, but only for platforms > where someone already did that work. When you want to do that work > in the first place, it's extremely tedious and obscure. > > - And if some of them fail, is it something that I should actually fix > or not. > > The mustpass list addresses all those issues by providing a baseline. > > Maxime
Re: Must-Pass Test Suite for KMS drivers
Hi Rob, On Mon, Oct 24, 2022 at 08:48:15AM -0700, Rob Clark wrote: > On Mon, Oct 24, 2022 at 5:43 AM wrote: > > I've discussing the idea for the past year to add an IGT test suite that > > all well-behaved KMS drivers must pass. > > > > The main idea behind it comes from v4l2-compliance and cec-compliance, > > that are being used to validate that the drivers are sane. > > > > We should probably start building up the test list, and eventually > > mandate that all tests pass for all the new KMS drivers we would merge > > in the kernel, and be run by KCi or similar. > > Let's get https://patchwork.freedesktop.org/patch/502641/ merged > first, that already gives us a mechanism similar to what we use in > mesa to track pass/fail/flake I'm not sure it's a dependency per-se, and I believe both can (and should) happen separately. AFAIU, the CI patches are here to track which tests are supposed to be working and which aren't so that we can track regressions. The list I was talking about is here to identify issues in the first place. All tests must pass, and if one fails it should be considered a hard failure. This would be eligible for CI only for drivers which have been known to pass them all already, but we wouldn't need to track which ones can fail or not, all of them must. > Beyond that, I think some of the igt tests need to get more stable > before we could consider a "mustpass" list. I agree that IGT tests could get more stable on ARM platforms, but it's also a chicken-and-egg issue. If no-one is using them regularly on ARM, then they'll never get fixed. > The kms_lease tests seem to fail on msm due to bad assumptions in the > test about which CRTCs primary planes can attach to. The legacy-cursor > crc tests seem a bit racy (there was a patch posted for that, not sure > if it landed yet), etc. And this is fine, we can merge that list without them, and if and when they get stable, we'll add them later. > The best thing to do is actually start running CI and tracking xfails > and flakes ;-) Again, I wouldn't oppose them. The issue I'm trying to solve is that there's just no way to know, at the moment: - When you're running IGT, which tests are relevant for your platform exactly. - If some of them fail, is it expected for them to fail or not. The ci/ patch you mentioned help for that a bit, but only for platforms where someone already did that work. When you want to do that work in the first place, it's extremely tedious and obscure. - And if some of them fail, is it something that I should actually fix or not. The mustpass list addresses all those issues by providing a baseline. Maxime
Re: Must-Pass Test Suite for KMS drivers
On Mon, Oct 24, 2022 at 5:43 AM wrote: > > Hi, > > I've discussing the idea for the past year to add an IGT test suite that > all well-behaved KMS drivers must pass. > > The main idea behind it comes from v4l2-compliance and cec-compliance, > that are being used to validate that the drivers are sane. > > We should probably start building up the test list, and eventually > mandate that all tests pass for all the new KMS drivers we would merge > in the kernel, and be run by KCi or similar. Let's get https://patchwork.freedesktop.org/patch/502641/ merged first, that already gives us a mechanism similar to what we use in mesa to track pass/fail/flake Beyond that, I think some of the igt tests need to get more stable before we could consider a "mustpass" list. The kms_lease tests seem to fail on msm due to bad assumptions in the test about which CRTCs primary planes can attach to. The legacy-cursor crc tests seem a bit racy (there was a patch posted for that, not sure if it landed yet), etc. The best thing to do is actually start running CI and tracking xfails and flakes ;-) BR, -R > I did a first pass to create a draft of such a test-suite, which would > contain: > > igt@core_auth@basic-auth > igt@core_auth@getclient-master-drop > igt@core_auth@getclient-simple > igt@core_auth@many-magics > igt@core_getclient > igt@core_getstats > igt@core_getversion > igt@core_hotunplug@hotrebind-lateclose > igt@core_hotunplug@hotunbind-rebind > igt@core_hotunplug@unbind-rebind > igt@core_setmaster > igt@core_setmaster_vs_auth > igt@device_reset@unbind-reset-rebind > igt@drm_read > igt@dumb_buffer > igt@fbdev > igt@feature_discovery@display > igt@kms_3d > igt@kms_addfb_basic > igt@kms_async_flips > igt@kms_color > igt@kms_concurrent > igt@kms_cursor_crc > igt@kms_cursor_edge_walk > igt@kms_cursor_legacy@basic-busy-flip-before-cursor > igt@kms_cursor_legacy@basic-flip-after-cursor > igt@kms_cursor_legacy@basic-flip-after-cursor > igt@kms_display_modes > igt@kms_dither > igt@kms_dp_aux_dev > igt@kms_flip@basic-flip-vs-dpms > igt@kms_flip@basic-flip-vs-modeset > igt@kms_flip@basic-flip-vs-wf_vblank > igt@kms_flip@basic-plain-flip > igt@kms_flip_event_leak@basic > igt@kms_force_connector_basic@force-connector-state > igt@kms_force_connector_basic@force-edid > igt@kms_force_connector_basic@force-load-detect > igt@kms_force_connector_basic@prune-stale-modes > igt@kms_getfb > igt@kms_hdmi_inject > igt@kms_hdr > igt@kms_invalid_mode > igt@kms_lease > igt@kms_panel_fitting > igt@kms_pipe_crc_basic > igt@kms_plane_alpha_blend > igt@kms_plane > igt@kms_plane_cursor > igt@kms_plane_lowres > igt@kms_plane_multiple > igt@kms_plane_scaling > igt@kms_prop_blob > igt@kms_properties > igt@kms_rmfb > igt@kms_scaling_modes > igt@kms_sequence > igt@kms_setmode > igt@kms_sysfs_edid_timing > igt@kms_tv_load_detect > igt@kms_universal_plane > igt@kms_vblank > igt@kms_vrr > igt@kms_writeback > > Most of them are skipped on vc4 right now, but I could see that some of > them fail already (kms_rmfb, core_hotunplug), so it proves to be useful > already. > > What do you think? Is there some more tests needed, or did I include > some tests that shouldn't have been there? > > Thanks! > Maxime
