Re: [PATCH i-g-t 2/3] tests/intel/kms_pm_dc: Update deep-pkgc function

["Naladala, Ramanaidu" <ramanaidu.naladala@intel.com>]

On 7/17/2024 9:01 PM, Kamil Konieczny wrote:
> Hi Naladala,
> On 2024-07-17 at 19:35:55 +0530, Naladala Ramanaidu wrote:
>> Increased the C-state polling time from its previous value
> 
> Why?
Certain features can prevent timely entry into the C10 state, causing 
inconsistent results. So increase test time to get accurate results.

> 
>> to 4 sec.Implement polling for PKG-C10 counters for every
> 
> Add space after dot '.', s/sec.Implement/sec. Implement/
> 
>> 5ms up to 16ms.
> 
> Same here, write why you need this change.
> 
> Also, imho you should split this into two patches and update subjects:
Sure Kamil. In next rev i will split into two patches.
> 
> [PATCH i-g-t 2/4] tests/intel/kms_pm_dc: Increase C-state polling
> 
> [PATCH i-g-t 3/4] tests/intel/kms_pm_dc: Add polling for deep-pkgc
> 
> Regards,
> Kamil
> 
>>
>> v2: Address and correct review comments (Suraj, Jeevan)
>>
>> Signed-off-by: Naladala Ramanaidu <ramanaidu.naladala@intel.com>
>> ---
>>   tests/intel/kms_pm_dc.c | 18 +++++++++++-------
>>   1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/intel/kms_pm_dc.c b/tests/intel/kms_pm_dc.c
>> index 8fb35a2df..c1ca3acc0 100644
>> --- a/tests/intel/kms_pm_dc.c
>> +++ b/tests/intel/kms_pm_dc.c
>> @@ -592,7 +592,7 @@ static unsigned int read_pkgc_counter(int debugfs_root_fd)
>>   static void test_deep_pkgc_state(data_t *data)
>>   {
>>   	unsigned int pre_val = 0, cur_val = 0;
>> -	time_t start = time(NULL), duration = 2, vb_delay, delay;
>> +	time_t start = time(NULL), duration = 4, vb_delay, delay;
>>   	enum pipe pipe;
>>   	bool pkgc_flag = false;
>>   	bool flip = true;
>> @@ -623,10 +623,10 @@ static void test_deep_pkgc_state(data_t *data)
>>   		data->mode = &output->config.connector->modes[j__];
>>   		vb_delay = 1 * (MSECS / (data->mode->vrefresh));
>>   		/*
>> -		 * At least 5ms vblank time required to program higher
>> +		 * At least 5ms vblank time for programming higher
>>   		 * watermark levels
>>   		 */
>> -		if (vb_delay > (5 * MSEC))
>> +		if (vb_delay > (5 * MSECS))
>>   			break;
>>   	}
>>   
>> @@ -635,23 +635,27 @@ static void test_deep_pkgc_state(data_t *data)
>>   	setup_videoplayback(data);
>>   
>>   	primary = igt_output_get_plane_type(data->output, DRM_PLANE_TYPE_PRIMARY);
>> -	pre_val = read_pkgc_counter(data->debugfs_root_fd);
>> -	delay = 1 * (MSECS / (data->mode->vrefresh - 10));
>> +	delay = vb_delay * 0.5;
>>   
>>   	igt_plane_set_fb(primary, &data->fb_rgb);
>>   	igt_display_commit(&data->display);
>>   
>> +	/* Waiting for the vblank to sync the frame time */
>> +	igt_wait_for_vblank_count(data->drm_fd, data->display.pipes[pipe].crtc_offset, 1);
>> +	usleep(delay);
>> +	pre_val = read_pkgc_counter(data->debugfs_root_fd);
>> +
>>   	while (time(NULL) - start < duration) {
>>   		flip = !flip;
>>   		igt_plane_set_fb(primary, flip ? &data->fb_rgb : &data->fb_rgr);
>>   		igt_display_commit(&data->display);
>>   
>> -		cur_val = read_pkgc_counter(data->debugfs_root_fd);
>> +		igt_wait((cur_val = read_pkgc_counter(data->debugfs_root_fd)) > pre_val,
>> +			  vb_delay * 2, 5);
>>   		if (cur_val > pre_val) {
>>   			pkgc_flag = true;
>>   			break;
>>   		}
>> -		usleep(delay);
>>   	}
>>   
>>   	cleanup_dc3co_fbs(data);
>> -- 
>> 2.43.0
>>