Re: [igt-dev] [PATCH i-g-t 4/6] tests/i915/i915_pm_dc: Added test for DC5 during DPMS

Gupta, Anshuman
2019-08-12T12:38:20Z

Hi Imre,

Thanks for review comments.
I have one comment below,
On 8/12/2019 5:41 PM, Imre Deak wrote:
> On Tue, Jun 18, 2019 at 11:26:15PM +0530, Anshuman Gupta wrote:
>> From: Jyoti Yadav <jyoti.r.yadav@intel.com>
>>
>> Added new subtest for DC5 entry during DPMS on/off cycle.
>> During DPMS on/off cycle DC5 counter is incremented.
>>
>> v2: Rename the subtest with meaningful name.
>> v3: Rebased.
>> v4: Addressed review comments by removing leftover code
>> cleanup().
>> v5: Addressed the review comment by removing redundant
>> read_dc_counter() suggested by Imre.
>> Listing actual change in patch set changelog to make review easier.
>> v6: Three way patch applied, no functional change.
>> v7: Disabling runtime suspend for the platform which support, DC9.
>> rebased due to test name pm_dc changed to i915_pm_dc, aligning to
>> other PM tests.
>> v8: Introduced setup_dc_dpms() in order to disable runtime pm, restoring
>> POWER_DIR values to its original and enabling runtime pm for other
>> followed sub-tests.
>> v9: Check DC5 counter value after DPMS off, broke the dpms_on_off
>> function to dpms_on and dpms_off. [Imre]
>>
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>> ---
>> tests/i915/i915_pm_dc.c | 62 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 62 insertions(+)
>>
>> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
>> index ae2247f4..5300d273 100644
>> --- a/tests/i915/i915_pm_dc.c
>> +++ b/tests/i915/i915_pm_dc.c
>> @@ -57,6 +57,7 @@ typedef struct {
>> enum psr_mode op_psr_mode;
>> drmModeModeInfo *mode;
>> igt_output_t *output;
>> + bool runtime_suspend_disabled;
>> } data_t;
>>
>> int msr_fd;
>> @@ -213,6 +214,62 @@ static void test_dc_state_psr(data_t *data, int dc_flag)
>> cleanup(data);
>> }
>>
>> +static void setup_dc_dpms(data_t *data)
>> +{
>> + if (IS_BROXTON(data->devid) || IS_GEMINILAKE(data->devid) ||
>> + IS_ICELAKE(data->devid)) {
>
> IS_BXT || IS_GLK || intel_gen() >= 11, so that it works correctly on
> later GENs too?
TGL onward we have DMC_DEBUG registers, DMC_DEBUG1 for DC5 and
DMC_DEBUG2 for DC6 BSpecs:49786, i believe they will retain there values
upon DC9, I will confirm once again with DMC folks.

Thanks ,
Anshuman.
>
>> + data->runtime_suspend_disabled = igt_disable_runtime_pm();
>> + igt_require_f(data->runtime_suspend_disabled,
>> + "unable to disable runtime pm for i915\n");
>> + } else {
>> + data->runtime_suspend_disabled = false;
>> + }
>> +}
>> +
>> +static void dpms_off(data_t *data)
>> +{
>> + for (int i = 0; i < data->display.n_outputs; i++) {
>> + kmstest_set_connector_dpms(data->drm_fd,
>> + data->display.outputs[i].config.connector,
>> + DRM_MODE_DPMS_OFF);
>> + }
>> +
>> + if (!data->runtime_suspend_disabled)
>> + igt_assert(igt_wait_for_pm_status
>> + (IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> +}
>> +
>> +static void dpms_on(data_t *data)
>> +{
>> + for (int i = 0; i < data->display.n_outputs; i++) {
>> + kmstest_set_connector_dpms(data->drm_fd,
>> + data->display.outputs[i].config.connector,
>> + DRM_MODE_DPMS_ON);
>> + }
>> +
>> + if (!data->runtime_suspend_disabled)
>> + igt_assert(igt_wait_for_pm_status
>> + (IGT_RUNTIME_PM_STATUS_ACTIVE));
>> +}
>> +
>> +static void test_dc_state_dpms(data_t *data, int dc_flag)
>> +{
>> + uint32_t dc_counter;
>> +
>> + dc_counter = read_dc_counter(data->drm_fd, dc_flag);
>> + dpms_off(data);
>> + check_dc_counter(data->drm_fd, dc_flag, dc_counter);
>> + dpms_on(data);
>> +
>> + /* if runtime PM is disabled for i915 restore it,
>> + * so any other sub-test can use runtime-PM.
>> + */
>> + if (data->runtime_suspend_disabled) {
>> + igt_restore_runtime_pm();
>> + igt_setup_runtime_pm();
>> + }
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> bool has_runtime_pm;
>> @@ -258,6 +315,11 @@ int main(int argc, char *argv[])
>> test_dc_state_psr(&data, CHECK_DC6);
>> }
>>
>> + igt_subtest("dc5-dpms") {
>> + setup_dc_dpms(&data);
>> + test_dc_state_dpms(&data, CHECK_DC5);
>> + }
>> +
>> igt_fixture {
>> close(data.debugfs_fd);
>> display_fini(&data);
>> --
>> 2.21.0
>>