[igt-dev] [PATCH] tests/kms_dp_aux_dev: Check DP connection only before tests

[Alex Hung]

On 2022-10-26 05:14, Modem, Bhanuprakash wrote:
> On Wed-26-10-2022 01:29 pm, Petri Latvala wrote:
>> On Tue, Oct 25, 2022 at 08:16:15PM -0600, Alex Hung wrote:
>>> Without DP port connected, kernel reports "Too many retries"
>>> that indicates no sink to respond. This is the same across GPUs.
>>>
>>> amdgpu ... DM aux hw bus 2: Too many retries, giving up. First error: -5
>>> i915 ... AUX C/DDI C/PHY C: Too many retries, giving up. First error: 
>>> -110
>>>
>>> In addition, the test can result time out such as below:
>>>
>>> /dev/drm_dp_aux0: Connection timed out
>>> /dev/drm_dp_aux1: Connection timed out
> 
> These timeouts are already handled in this test: 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit.freedesktop.org%2Fdrm%2Figt-gpu-tools%2Ftree%2Ftests%2Fkms_dp_aux_dev.c%23n88&data=05%7C01%7Calex.hung%40amd.com%7Cf247127341e34be42d0608dab7434b53%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638023796906119251%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4kt0bcLG6jsPpagnkqwcVWxhvf00Omjr3kjCTs%2BEu2o%3D&reserved=0
> 
> IMO, It could be fine with existing test, since it'll work as a kind of 
> negative scenario.
1. If a case is known for not going to work, is it necessary to test it?

2. copied code here for discussion

igt_assert(ret == sizeof(buf) ||
	errno == ETIMEDOUT ||
	errno == EIO && is_mst_connector(drm_fd, connector_id)));

As in description, the i915 returns -110 (ETIMEOUT) but amdgpu returns 
-5 (EIO), and the above code handles i915 well but that's not always 
true for other scenario.

Do you prefer I made changes like the below instead?

igt_assert(ret == sizeof(buf) ||
	errno == ETIMEDOUT ||
-	errno == EIO && is_mst_connector(drm_fd, connector_id)));
+	errno == EIO);

> 
> - Bhanu
> 
>>>
>>> As a result, checking DP connection alone should be sufficient.
>>>
>>> Signed-off-by: Alex Hung <alex.hung at amd.com>
>>> ---
>>>   tests/kms_dp_aux_dev.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_dp_aux_dev.c b/tests/kms_dp_aux_dev.c
>>> index ed9dd510..7fce74a4 100644
>>> --- a/tests/kms_dp_aux_dev.c
>>> +++ b/tests/kms_dp_aux_dev.c
>>> @@ -53,8 +53,7 @@ static bool test(int drm_fd, uint32_t connector_id)
>>>       drmModeFreeConnector(connector);
>>>       igt_assert(dir_fd >= 0);
>>> -    if (connector->connection != DRM_MODE_CONNECTED &&
>>> -        is_mst_connector(drm_fd, connector_id)) {
>>> +    if (connector->connection != DRM_MODE_CONNECTED) {
>>>           close(dir_fd);
>>>           return false;
>>>       }
>>
>>
>> Bhanuprakash, Swati, you wrote/reviewed this piece of code, please review
>> this patch.
>>
>>
>