On Thu, May 22, 2025 at 07:47:00PM +0300, Cristian Ciocaltea wrote: > On 5/22/25 7:06 PM, Maxime Ripard wrote: > > On Mon, May 19, 2025 at 01:35:46PM +0300, Cristian Ciocaltea wrote: > >> On 5/19/25 10:22 AM, Maxime Ripard wrote: > >>> Hi, > >>> > >>> On Fri, Apr 25, 2025 at 01:27:05PM +0300, Cristian Ciocaltea wrote: > >>>> In preparation to improve error handling throughout all test cases, > >>>> introduce a macro to check for EDEADLK and automate the restart of the > >>>> atomic sequence. > >>>> > >>>> Signed-off-by: Cristian Ciocaltea <[email protected]> > >>>> --- > >>>> drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 10 ++++++++++ > >>>> 1 file changed, 10 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c > >>>> b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c > >>>> index > >>>> c8969ee6518954ab4496d3a4398f428bf4104a36..c8bb131d63ea6d0c9e166c8d9ba5e403118cd9f1 > >>>> 100644 > >>>> --- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c > >>>> +++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c > >>>> @@ -224,6 +224,16 @@ drm_kunit_helper_connector_hdmi_init(struct kunit > >>>> *test, > >>>> test_edid_hdmi_1080p_rgb_max_200mhz); > >>>> } > >>>> > >>>> +#define drm_kunit_atomic_restart_on_deadlock(ret, state, ctx, start) do > >>>> { \ > >>>> + if (ret == -EDEADLK) { > >>>> \ > >>>> + if (state) > >>>> \ > >>>> + drm_atomic_state_clear(state); > >>>> \ > >>>> + ret = drm_modeset_backoff(ctx); > >>>> \ > >>>> + if (!ret) > >>>> \ > >>>> + goto start; > >>>> \ > >>>> + } > >>>> \ > >>>> +} while (0) > >>>> + > >>> > >>> I'm not sure here either, for pretty much the same reason. As far as > >>> locking goes, I really think we should prefer something explicit even if > >>> it means a bit more boilerplate. > >>> > >>> If you still want to push this forward though, this has nothing to do > >>> with kunit so it should be made a common helper. > >> > >> Ack. > >> > >>> I do think it should be > >>> done in a separate series though. Ever-expanding series are a nightmare, > >>> both to contribute and to review :) > >> > >> Indeed, let me take this separately. > >> > >> If you agree, I'd prefer to drop EDEADLK handling from the new tests as > >> well, to allow sorting this out for all in a consistent manner. > > > > We can't unfortunately. Most CI runners now run with WW_DEBUG that will > > test for EDEADBLK handling. > > Ok, in that case I'll proceed with the explicit error handling for the > new tests only. And as soon as the series gets merged, I'll come up > with a common helper and apply it for all tests.
Sounds like a good plan :) Maxime
signature.asc
Description: PGP signature
