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

[Imre Deak <imre.deak@intel.com>]

On Mon, Aug 12, 2019 at 06:08:16PM +0530, Gupta, Anshuman wrote:
> 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.

And even if the counters retain their value is it also guaranteed that
there will be a ->DC6 transition before ->DC9 would happen?

If you clarify these it would be good to add a comment here too.

> 
> 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
> > > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev