[Intel-gfx] [I-G-T] igt/gem_mocs_settings: improve RC6 testings

[Dave Gordon]

On 22/07/16 10:40, Antoine, Peter wrote:
>
> -----Original Message-----
> From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> Sent: Friday, July 22, 2016 10:38 AM
> To: Antoine, Peter <peter.antoine at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [I-G-T] igt/gem_mocs_settings: improve RC6 testings
>
> On Thu, Jul 21, 2016 at 09:49:51PM +0000, Antoine, Peter wrote:
>>
>> -----Original Message-----
>> From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
>> Sent: Thursday, July 21, 2016 9:40 PM
>> To: Antoine, Peter <peter.antoine at intel.com>
>> Cc: intel-gfx at lists.freedesktop.org
>> Subject: Re: [I-G-T] igt/gem_mocs_settings: improve RC6 testings
>>
>> On Tue, Jul 19, 2016 at 11:25:29AM +0100, Peter Antoine wrote:
>>> On some platforms the MOCS values are not always saved and restored
>>> on
>>> RC6 enter/exit. The rational is that the context with restore these
>>> values. On these platforms the test will fail as it tests the values
>>> by directly reading the MOCS registers.
>>
>> But there's nothing wrong with the existing tests per-se? You just
>> want to add a new one that explicitly tests rc6 save/restore, and in
>> doing so just need to limit the forcewake. For that you just want to
>> limit intel_register_access to the critical sections. (You don't need
>> to find the pci_dev afresh everytime.)
>>
>> On some platforms (BXT) it does not restore L3CC registers. So that on these platforms the direct read will fail unless you hold forcewake for the whole test. It also implies that the registers are valid for the whole power lifecycle, so if you are using the tests as API documentation then it implies that the registers are always valid. This is not always true on all platforms.
>
> Right. If those registers are only valid when read from within an active context, please send the patch to remove the invalid tests first. That means all the direct register access is undefined, right?
>
> Ok, will do that.
>
> Peter.
>
>> So the direct memory reads are removed to give the correct API usage for the MOCS. That is that the l3CC registers are only valid while in the RCS context.
>
> Ok.
>
>>> +static void context_rc6_test(void)
>>> +{
>>> +	int fd = drm_open_driver(DRIVER_INTEL);
>>> +	int res_ms;
>>> +	uint32_t ctx_id = gem_context_create(fd);
>>> +
>>> +	igt_debug("RC6 Context Test\n");
>>> +	check_control_registers(fd, I915_EXEC_RENDER, ctx_id, false);
>>> +	check_l3cc_registers(fd, I915_EXEC_RENDER, ctx_id, false);
>>> +
>>> +	res_ms = read_rc6_residency();
>>> +	sleep(3);
>>
>> And you could spin here until rc6 residency increased.
>>
>> timeout = 3000 / 2;
>> while (read_rc6_residency() == initial_res && --timeout)
>> 	usleep(2000);
>>
>> Ok. Decided against that as 3 seconds (I know magic value) should have been enough. I can change to do the spin with a timeout.

Peter, your email client's quoting is broken in the thread above; it
makes it quite difficult to determine who said what. Different authors
should have different numbers of >> before their contributions, or some
other way of identifying who said what.

.Dave.