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

[Daniel Vetter <daniel@ffwll.ch>]

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@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 :-(

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev