[PATCH i-g-t v1] tests/kms_flip: Skip timestamp checks in Intel simulation environments

[Naladala, Ramanaidu]

Hi Jonathan,

Thanks for reviewing the patch.

On 7/2/2025 10:13 PM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Kamil Konieczny
> Sent: Wednesday, July 2, 2025 6:06 AM
> To: Naladala, Ramanaidu <ramanaidu.naladala at intel.com>
> Cc: igt-dev at lists.freedesktop.org; Sharma, Swati2 <swati2.sharma at intel.com>; Golani, Mitulkumar Ajitkumar <mitulkumar.ajitkumar.golani at intel.com>
> Subject: Re: [PATCH i-g-t v1] tests/kms_flip: Skip timestamp checks in Intel simulation environments
>> Hi Naladala,
>> On 2025-06-24 at 14:22:17 +0530, Naladala Ramanaidu wrote:
>>> Bypass timestamp and sequence validations when INTEL_SIMULATION is
>>> active to accommodate non-deterministic timing in simulated test
>>> environments.
>>>
>>> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala at intel.com>
>>> ---
>>>   tests/kms_flip.c | 14 +++++++++++++-
>>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
>>> index 65016c66d..db7ced8aa 100755
>>> --- a/tests/kms_flip.c
>>> +++ b/tests/kms_flip.c
>>> @@ -318,7 +318,19 @@ static bool should_skip_ts_checks(void) {
>>>   	 * timestamp to drift with a relatively larger standard deviation over a large sample.
>>>   	 * As it's a known issue, skip any Timestamp or Sequence checks for MTK drivers.
>>>   	 */
>>> -	return is_mtk_device(drm_fd);
>>> +	if (is_mtk_device(drm_fd))
>>> +		return true;
>>> +
>>> +	/*
>>> +	 * In simulation environments, hardware behavior may not accurately reflect real-world
>>> +	 * timing characteristics. To avoid false negatives in tests due to simulated timing
>>> +	 * artifacts, skip timestamp and sequence checks when the INTEL_SIMULATION environment
>>> +	 * variable is set to a truthy value.
>>> +	 */
>>> +	if (igt_check_boolean_env_var("INTEL_SIMULATION", false))
>> Please use function: igt_run_in_simulation() so it will be:
>>
>> 	if (igt_run_in_simulation())
> I think we're looking for the opposite, actually:
The current condition is correct. As Kamil mentioned, this direction 
aligns better with the intended logic.
>
> """
> 	if (!igt_run_in_simulation())
> 		return true;
>
> 	return false;
> """
>
> Or, alternatively:
>
> """
> 	if (igt_run_in_simulation())
> 		return false;
>
> 	return true;
> """
>
> Though, at that point, I think we'd may as well just return the value directly:
>
> """
> 	return !igt_run_in_simulation();
> """
>
> -Jonathan Cavitt
>
>> Regards,
>> Kamil
>>
>>> +		return true;
>>> +
>>> +	return false;
>>>   }
>>>   
>>>   static bool vblank_dependence(int flags)
>>> -- 
>>> 2.43.0
>>>