[igt-dev] [PATCH i-g-t 2/2] tests/kms_psr_sink_crc: Test PSR source HW status.

[Daniel Vetter]

On Tue, Jul 03, 2018 at 10:43:55AM +0200, Daniel Vetter wrote:
> On Mon, Jul 02, 2018 at 04:35:59PM -0700, Dhinakaran Pandiyan wrote:
> > We make use of the status MMIO to tell whether the HW entered and
> > exited.
> > 
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> 
> The trouble with this is that this isn't independent verification. We
> essentially have to believe the hw folks that their hw works, and the
> Bspec folks that they documented stuff correctly.
> 
> Which is very little validation :-/
> 
> The entire point of the sink crc stuff was that we'd independently check
> that the panel is actually showing the right pixels. Is there no way to
> salvage that, using some hacks to make sure the dp aux stuff doesn't wake
> up the panel or accidentally cause a psr exit? We'd need to make sure that
> the hw isn't using the aux channel while we poke it ofc ...
> 
> Before we toss in the towel here I think would be good to check once more
> with display hw architect whether our tests can't be salvaged. Since
> without sink crc there's not much point in having them :-(

E.g. if the entire problem is the vblank wait in the debugfs interface
then it's easy to convert that into a polling wait.
-Daniel

> 
> Cheers, Daniel
> 
> > ---
> >  tests/kms_psr_sink_crc.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > index d36be7dd..08d0ce9a 100644
> > --- a/tests/kms_psr_sink_crc.c
> > +++ b/tests/kms_psr_sink_crc.c
> > @@ -195,7 +195,7 @@ static bool sink_support(data_t *data)
> >  		strstr(buf, "Sink_Support: yes\n");
> >  }
> >  
> > -static bool psr_enabled(data_t *data)
> > +static bool psr_hw_enabled(data_t *data)
> >  {
> >  	char buf[512];
> >  
> > @@ -205,15 +205,23 @@ static bool psr_enabled(data_t *data)
> >  		strstr(buf, "HW Enabled & Active bit: yes\n");
> >  }
> >  
> > +static bool psr_hw_status(data_t *data, bool active)
> > +{
> > +	char buf[512];
> > +
> > +	igt_debugfs_read(data->drm_fd, "i915_edp_psr_status", buf);
> > +
> > +	/* TODO: Update the checks for PSR2 */
> > +	return strstr(buf, "Source PSR status:") &&
> > +	       (active ? !!strstr(buf, "SRDENT") : !strstr(buf, "SRDENT"));
> > +}
> > +
> >  static bool wait_psr_entry(data_t *data)
> >  {
> > -	int timeout = 5;
> > -	while (timeout--) {
> > -		if (psr_enabled(data))
> > -			return true;
> > -		sleep(1);
> > -	}
> > -	return false;
> > +	if (!psr_hw_enabled(data))
> > +		return false;
> > +
> > +	return igt_wait(psr_hw_status(data, true), 500, 1);
> >  }
> >  
> >  static inline void manual(const char *expected)
> > @@ -303,6 +311,7 @@ static void run_test(data_t *data)
> >  		expected = "screen GREEN";
> >  		break;
> >  	}
> > +	igt_assert(psr_hw_status(data, false));
> >  	manual(expected);
> >  }
> >  
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch