[igt-dev] [PATCH i-g-t] igt/tests: Clear pipes properly in kms_atomic_transition

[Maarten Lankhorst]

Op 23-06-2020 om 17:07 schreef Juha-Pekka Heikkila:
> On 23.6.2020 16.53, Stanislav Lisovskiy wrote:
>> There is an issue happening from time to time in kms_atomic_transition
>> (bug #1918). We periodically get assertion that some two outputs
>> attempt to use same pipe like this:
>>
>> "Failed assertion: output->pending_pipe != b->pending_pipe"
>>
>> After some investigation came to conclusion that this is happening
>> because we are calling igt_output_set_pipe(output, PIPE_NONE) only
>> for connected outputs, which is wrong.
>> Periodically igt_display_refresh/igt_output_refresh call calls can
>> update the output state to disconnected. However that doesn't clear
>> the pipe being assigned(i.e output->pending_pipe).
>> So this causes assertion to be triggered on next igt_display_refresh
>> called during commit.
>>
>> Bugzilla: https://gitlab.freedesktop.org/drm/intel/-/issues/1918
>>
>> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
>> ---
>>   tests/kms_atomic_transition.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_atomic_transition.c b/tests/kms_atomic_transition.c
>> index 754a4969..a55388cd 100644
>> --- a/tests/kms_atomic_transition.c
>> +++ b/tests/kms_atomic_transition.c
>> @@ -665,8 +665,9 @@ static unsigned set_combinations(igt_display_t *display, unsigned mask, struct i
>>       enum pipe pipe;
>>       unsigned event_mask = 0;
>>   -    for_each_connected_output(display, output)
>> -        igt_output_set_pipe(output, PIPE_NONE);
>> +    for_each_pipe(display, pipe)
>> +        for_each_valid_output_on_pipe(display, pipe, output)
>> +            igt_output_set_pipe(output, PIPE_NONE);
>>         for_each_pipe(display, pipe) {
>>           igt_plane_t *plane = igt_pipe_get_plane_type(&display->pipes[pipe],
>>
>
> Look ok to me.
>
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com> 

Nack, for_each_valid_output_on_pipe uses for_each_connected_output, so you keep the bug..

    for (i = 0; i < display->n_outputs; i++)
        igt_output_set_pipe(&display->outputs[i], PIPE_NONE); ?