[igt-dev] [PATCH i-g-t v8 3/6] tests/i915_pm_lpsp: lpsp platform agnostic support

[Anshuman Gupta]

On 2020-05-19 at 12:32:09 +0300, Petri Latvala wrote:
> On Tue, May 12, 2020 at 04:04:12PM +0530, Anshuman Gupta wrote:
> > Current implementation of lpsp igt test, assumed that every non-edp
> > panel isn't a lpsp panel but it is not true on TGL anymore,
> > any HDMI/DP/DSI panel connected on pipe A and connected to PORT_{A,B,C}
> > can drive LPSP.
> > Even on older Gen9 platform a DP panel can drive lpsp on Port A.
> > This requires complete design change in current lpsp igt for a platform
> > agnostic support.
> > 
> > The new igt approach is relies on connector specific
> > i915_lpsp_capability and i915_lpsp_status debugfs attributes,
> > these debugfs exposes whether an output is capable of driving lpsp
> > and lpsp is enabled.
> > 
> > Nuking edp-native and non-edp test, introducing a new dynamic
> > igt subtest kms-lpsp, which validates lpsp on each connected output
> > and skip the subtest if output is not lpsp capable.
> > 
> > Skip screens-disabled test for platform which support DC states.
> > screens-disabled test is only valid for the platforms like HSW/BDW
> > where PG1 is Always-ON power domain, so these platform test lpsp
> > with all screen disabled i.e validate PG2 when all screen are
> > disabled.
> > DC state i.e DMC f/w supported platforms don't have any ROI to validate
> > screens-disabled subtest as existing dc*-dpms i915_pm_dc
> > subtest already validate it implicitly.
> > 
> > v2:
> > - CI failures fixup.
> > v3:
> > - removed unloading of snd_modules. [Martin]
> > v4:
> > - Don't test non-lpsp(if lpsp disabled), no ROI to test that.
> > - nuke panel-fitter test.
> > v5:
> > - Added dynamic subtest and igt changes according to kernel
> >   debugfs i915_lpsp_status changes.
> > v6:
> > - Avoided pipe big joiner by using 4k or lesser mode,
> >   so igt will not fail when pipe big joiner patches will
> >   land to kernel.
> > v7:
> > - Combined [v6 4/6] patch to this patch, skip the screens-disabled
> >   subtest for DC state supported platforms. [Animesh]
> > v8:
> > - 4k mode correction 3840*2160 and for loop for available modes
> >   should iterate from c->modes[0]. [Ankit]
> > 
> > Reviewed-by: Animesh Manna <animesh.manna at intel.com>
> > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> > ---
> >  tests/i915/i915_pm_lpsp.c | 313 ++++++++++++++++++--------------------
> >  1 file changed, 147 insertions(+), 166 deletions(-)
> > 
> > diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> > index 08f82e7c..0e9452fd 100644
> > --- a/tests/i915/i915_pm_lpsp.c
> > +++ b/tests/i915/i915_pm_lpsp.c
> > @@ -25,210 +25,191 @@
> >   */
> >  
> >  #include "igt.h"
> > +#include "igt_kmod.h"
> > +#include "igt_pm.h"
> > +#include "igt_sysfs.h"
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> >  #include <fcntl.h>
> >  #include <unistd.h>
> >  
> > +#define MAX_SINK_LPSP_INFO_BUF_LEN	4096
> >  
> > -static bool supports_lpsp(uint32_t devid)
> > -{
> > -	return IS_HASWELL(devid) || IS_BROADWELL(devid);
> > -}
> > +#define PWR_DOMAIN_INFO "i915_power_domain_info"
> >  
> > -static bool lpsp_is_enabled(int drm_fd)
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	uint32_t devid;
> > +	char *pwr_dmn_info;
> > +	igt_display_t display;
> > +	struct igt_fb fb;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +} data_t;
> > +
> > +static bool lpsp_is_enabled(data_t *data)
> >  {
> > -	uint32_t val;
> > +	char buf[MAX_SINK_LPSP_INFO_BUF_LEN];
> > +	int len;
> >  
> > -	val = INREG(HSW_PWR_WELL_CTL2);
> > -	return !(val & HSW_PWR_WELL_STATE_ENABLED);
> > -}
> > +	len = igt_debugfs_simple_read(data->debugfs_fd, "i915_lpsp_status",
> > +				      buf, sizeof(buf));
> > +	if (len < 0)
> > +		igt_assert_eq(len, -ENODEV);
> >  
> > -/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
> > - * low power mode when no pipes are enabled, so do this check anyway. */
> > -static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
> > -{
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -	igt_assert(lpsp_is_enabled(drm_fd));
> > +	igt_skip_on(strstr(buf, "LPSP: not supported"));
> > +
> > +	return strstr(buf, "LPSP: enabled");
> >  }
> >  
> > -static uint32_t create_fb(int drm_fd, int width, int height)
> > +static bool dmc_supported(int debugfs)
> >  {
> > -	struct igt_fb fb;
> > +	char buf[15];
> > +	int len;
> >  
> > -	return igt_create_pattern_fb(drm_fd, width, height, DRM_FORMAT_XRGB8888,
> > -				     LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> > +	len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
> > +
> > +	if (len < 0)
> > +		return false;
> > +	else
> > +		return true;
> >  }
> >  
> > -static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> > -			drmModeConnectorPtr *drm_connectors, uint32_t devid,
> > -			bool use_panel_fitter)
> > +/*
> > + * The LPSP mode is all about an enabled pipe, but we expect to also be in the
> > + * low power mode when no pipes are enabled, so do this check anyway.
> > + */
> > +static void screens_disabled_subtest(data_t *data)
> >  {
> > -	int i, rc;
> > -	uint32_t crtc_id = 0, buffer_id = 0;
> > -	drmModeConnectorPtr connector = NULL;
> > -	drmModeModeInfoPtr mode = NULL;
> > -	drmModeModeInfo std_1024_mode = {
> > -		.clock = 65000,
> > -		.hdisplay = 1024,
> > -		.hsync_start = 1048,
> > -		.hsync_end = 1184,
> > -		.htotal = 1344,
> > -		.hskew = 0,
> > -		.vdisplay = 768,
> > -		.vsync_start = 771,
> > -		.vsync_end = 777,
> > -		.vtotal = 806,
> > -		.vscan = 0,
> > -		.vrefresh = 60,
> > -		.flags = 0xA,
> > -		.type = 0x40,
> > -		.name = "Custom 1024x768",
> > -	};
> > -
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> > -	for (i = 0; i < drm_res->count_connectors; i++) {
> > -		drmModeConnectorPtr c = drm_connectors[i];
> > -
> > -		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > -			continue;
> > -		if (c->connection != DRM_MODE_CONNECTED)
> > -			continue;
> > -
> > -		if (!use_panel_fitter && c->count_modes) {
> > -			connector = c;
> > -			mode = &c->modes[0];
> > -			break;
> > -		}
> > -		if (use_panel_fitter) {
> > -			connector = c;
> > -
> > -			/* This is one of the modes Xorg creates for panels, so
> > -			 * it should work just fine. Notice that Gens that
> > -			 * support LPSP are too new for panels with native
> > -			 * 1024x768 resolution, so this should force the panel
> > -			 * fitter. */
> > -			igt_assert(c->count_modes &&
> > -				   c->modes[0].hdisplay > 1024);
> > -			igt_assert(c->count_modes &&
> > -				   c->modes[0].vdisplay > 768);
> > -			mode = &std_1024_mode;
> > -			break;
> > -		}
> > -	}
> > -	igt_require(connector);
> > -
> > -	crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> > -						  0);
> > -	buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
> > -
> > -	igt_assert(buffer_id);
> > -	igt_assert(connector);
> > -	igt_assert(mode);
> > -
> > -	rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
> > -			    &connector->connector_id, 1, mode);
> > -	igt_assert_eq(rc, 0);
> > -
> > -	if (use_panel_fitter) {
> > -		if (IS_HASWELL(devid))
> > -			igt_assert(!lpsp_is_enabled(drm_fd));
> > -		else
> > -			igt_assert(lpsp_is_enabled(drm_fd));
> > -	} else {
> > -		igt_assert(lpsp_is_enabled(drm_fd));
> > +	igt_output_t *output;
> > +	int valid_output = 0;
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe_with_single_output(&data->display, pipe, output) {
> > +		data->output = output;
> > +		igt_output_set_pipe(data->output, PIPE_NONE);
> > +		igt_display_commit(&data->display);
> > +		valid_output++;
> >  	}
> 
> This code is already merged, but nonetheless:
Thanks for reporting this,
I will fix this, is there a way to replicate what legacy kmstest_unset_all_crtcs() does,
Thanks,
Anshuman Gupta.
> 
> This for loop doesn't do what you probably think it does. This loops
> through each pipe, possibly giving you the same output for every pipe.
> 
> An immediate effect is that this introduces a new compile warning:
> 
> ../tests/i915/i915_pm_lpsp.c: In function ‘screens_disabled_subtest’:
> ../tests/i915/i915_pm_lpsp.c:87:12: warning: variable ‘pipe’ set but not used [-Wunused-but-set-variable]
>   enum pipe pipe;
>               ^~~~
> 
> 
> 
> -- 
> Petri Latvala