[PATCH v2 0/5] tests/kms_writeback: Add clone mode subtest

[Jessica Zhang]

On 9/3/2024 4:33 PM, Alex Hung wrote:
> 
> 
> On 2024-08-30 12:32, Jessica Zhang wrote:
>>
>>
>> On 8/29/2024 1:28 PM, Alex Hung wrote:
>>> I ran a smoke test and Patch 4 (tests/kms_writeback: Disable all 
>>> outputs after igt_display_require()) is causing a regression on my 
>>> test platform:
>>>
>>>
>>> Starting subtest: writeback-check-output-XRGB2101010
>>> (kms_writeback:1973) CRITICAL: Test assertion failure function 
>>> get_and_wait_out_fence, file ../tests/kms_writeback.c:345:
>>> (kms_writeback:1973) CRITICAL: Failed assertion: ret == 0
>>> (kms_writeback:1973) CRITICAL: Last errno: 38, Function not implemented
>>> (kms_writeback:1973) CRITICAL: sync_fence_wait failed: Timer expired
>>> Stack trace:
>>>    #0 ../lib/igt_core.c:2051 __igt_fail_assert()
>>>    #1 [get_and_wait_out_fence+0x8e]
>>>    #2 ../tests/kms_writeback.c:401 writeback_sequence()
>>>    #3 ../tests/kms_writeback.c:424 writeback_check_output()
>>>    #4 ../tests/kms_writeback.c:782 __igt_unique____real_main557()
>>>    #5 ../tests/kms_writeback.c:557 main()
>>>    #6 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
>>>    #7 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
>>>    #8 [_start+0x25]
>>> Subtest writeback-check-output-XRGB2101010 failed.
>>
>> Hi Alex,
>>
>> Thanks for testing and reporting this.
>>
>> The overall goal of that patch was to force a modeset for cases where 
>> the primary display mode and the mode being used on writeback were the 
>> same. If the modes are the same between commits, the DRM framework 
>> won't perform a modeset on the encoder even if the connectors have 
>> been changed.
>>
>> To get around this, we thought it would be good to disable all the 
>> outputs and start from a clean state before doing the writeback 
>> connector setup.
>>
>> Is there anything in AMD's disable path that could cause the writeback 
>> connector/encoder to be in a bad state for the next commit?
>>
>> Thanks,
>>
>> Jessica Zhang
>>
> 
> I did a quick check but haven't found anything wrong in disable path yet.
> 
> If this is required specifically for your devices, maybe you can have 
> something similar to is_i915_device() or is_amdgpu_device() for time-being.

Abhinav has recently posted a patch [1] to directly address the issue 
instead of having the disable_all_outputs() as a workaround. Since his 
patch is also fixing the issue for me, I will drop the disable all 
outputs patch in this series.

[1] https://patchwork.freedesktop.org/series/138284/

> 
> 
>>>
>>>
>>>
>>> On 2024-08-28 21:05, Jessica Zhang wrote:
>>>> Add a subtest that will iterate through all pairs of
>>>> non-writeback and writeback cloned outputs and dump the writeback
>>>> framebuffer for each pair.
>>>>
>>>> This series includes:
>>>>
>>>> - Fixes for properly cleaning up the kms_writeback state and forcing a
>>>>    modeset after igt_display_require() to prevent commit failures when
>>>>    the writeback mode is the same as the primary display mode
>>>> - Adjust the duplicate pipe check in igt_display_refresh() to allow for
>>>>    combinations of valid clones
>>>> - Moving the writeback dump to its own subtest
>>>> - Adding a subtest to dump all writeback/non-writeback cloned pairs
>>>>
>>>> To dump the clone mode buffer, run the following command:
>>>>
>>>> IGT_FRAME_DUMP_PATH=<dump path> FRAME_PNG_FILE_NAME=<file name> \
>>>>     ./build/tests/kms_writeback [--run-subtest dump-valid-clones] \
>>>>     -dc <primary display mode>
>>>>
>>>> This will output a file named <file name>-encoder<writeback encoder
>>>> id>-encoder<primary display encoder id>
>>>>
>>>> ---
>>>> Changes in v2:
>>>> - Corrected author email
>>>> - Link to v1: 
>>>> https://lore.kernel.org/r/20240828-igt-cwb-v1-0-48aee421fc97@quicinc.com
>>>>
>>>> ---
>>>> Jessica Zhang (5):
>>>>        tests/kms_writeback: clear writeback properties in teardown path
>>>>        lib/igt_kms: Add helper to get encoder index
>>>>        lib/igt_kms: loosen duplicate check in igt_display_refresh
>>>>        tests/kms_writeback: Disable all outputs after 
>>>> igt_display_require()
>>>>        tests/kms_writeback: Add dump for valid clone mode
>>>>
>>>>   lib/igt_kms.c         | 57 +++++++++++++++++++++++++++++------
>>>>   lib/igt_kms.h         |  2 ++
>>>>   tests/kms_setmode.c   | 14 ++-------
>>>>   tests/kms_writeback.c | 83 
>>>> +++++++++++++++++++++++++++++++++++++++++++--------
>>>>   4 files changed, 122 insertions(+), 34 deletions(-)
>>>> ---
>>>> base-commit: 3b6b2d238e864ff1af9e33159d3bbf4b7f01d86d
>>>> change-id: 20240827-igt-cwb-23fd81d3a7a4
>>>>
>>>> Best regards,