Re: [PATCH i-g-t 01/12] tests/amdgpu/amd_mode_switch: Prepare for returning NULL from igt_crtc_for_pipe()

[Alex Hung <alex.hung@amd.com>]

On 3/18/26 14:01, Harry Wentland wrote:
> 
> 
> On 2026-03-18 14:01, Kamil Konieczny wrote:
>> Hi Jani,
>> On 2026-03-16 at 13:19:03 +0200, Jani Nikula wrote:
>>> After checking there are enough pipes, AMD should always have the
>>> CRTCs. Assert that the CRTC is valid.
>>>
>>> This prepares for returning NULL for invalid pipes from
>>> igt_crtc_for_pipe() by ensuring the test logic is sound.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> +cc AMD devs
>> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>
>>
>>> ---
>>>   tests/amdgpu/amd_mode_switch.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tests/amdgpu/amd_mode_switch.c b/tests/amdgpu/amd_mode_switch.c
>>> index 4da07c412996..ffa2952bb5c2 100644
>>> --- a/tests/amdgpu/amd_mode_switch.c
>>> +++ b/tests/amdgpu/amd_mode_switch.c
>>> @@ -98,6 +98,8 @@ static void run_mode_switch_first_last(data_t *data, int num_pipes)
>>>   	for (j = 0; j < num_pipes; j++) {
>>>   		igt_crtc_t *crtc = igt_crtc_for_pipe(display, j);
>>>   
>>> +		igt_assert_f(crtc->valid, "There is no pipe %s\n", kmstest_pipe_name(j));
>>> +
> 
> Not sure I follow how this prepares for returning NULL
> from igt_crtc_for_pipe(). Wouldn't this do the opposite
> and hit a NULL pointer with crtc->valid if the function
> returns NULL?
> 
> Harry
> 


This patch may be no longer applicable because the for-loop is replaced 
by for_each_crtc(...) which already checks crtc->valid.

However, Harry's comment is still applicable to patch 2

>>>   		output = data->output[j];
>>>   		if (!igt_output_is_connected(output))
>>>   			continue;
>>> -- 
>>> 2.47.3
>>>
>