Re: [PATCH v4 14/23] drm/tests: hdmi: Add macro to support EDEADLK handling

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

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 <cristian.ciocaltea@collabora.com>
>>>> ---
>>>>  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.