[igt-dev] [PATCH i-g-t v2] i915/gem_mocs_settings: Add mocs table for icelake
Lis, Tomasz
tomasz.lis at intel.com
Mon Feb 25 13:17:00 UTC 2019
On 2019-02-22 22:48, Chris Wilson wrote:
> Quoting Kumar Valsan, Prathap (2019-02-22 21:32:55)
>> On Fri, Feb 22, 2019 at 03:21:18PM +0000, Chris Wilson wrote:
>>> Quoting Lis, Tomasz (2019-02-22 15:17:34)
>>>>
>>>> On 2019-02-22 15:17, Prathap Kumar Valsan wrote:
>>>>> From: "Kumar Valsan, Prathap" <prathap.kumar.valsan at intel.com>
>>>>>
>>>>> This patch adds mocs table for icelake with expected L3 and eDRAM
>>>>> control values.
>>>>>
>>>>> Signed-off-by: Kumar Valsan, Prathap <prathap.kumar.valsan at intel.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Cleaned up the code based on review comments from Lucas and Chris
>>>>> tests/i915/gem_mocs_settings.c | 96 ++++++++++++++++++++++++++--------
>>>>> 1 file changed, 73 insertions(+), 23 deletions(-)
>>>>>
>>>>> diff --git a/tests/i915/gem_mocs_settings.c b/tests/i915/gem_mocs_settings.c
>>>>> index 5b3b6bc1..6d111076 100644
>>>>> --- a/tests/i915/gem_mocs_settings.c
>>>>> +++ b/tests/i915/gem_mocs_settings.c
>>>>> @@ -33,6 +33,9 @@
>>>>> #include "igt_sysfs.h"
>>>>>
>>>>> #define MAX_NUMBER_MOCS_REGISTERS (64)
>>>>> +#define GEN9_NUM_MOCS_ENTRIES 62 /* 62 out of 64 - 63 & 64 are reserved. */
>>>>> +#define GEN11_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */
>>>>> +
>>>>> enum {
>>>>> NONE,
>>>>> RESET,
>>>>> @@ -72,36 +75,66 @@ struct mocs_table {
>>>>> const struct mocs_entry *table;
>>>>> };
>>>>>
>>>>> +static const struct mocs_entry icelake_mocs_pte = {0x00000004, 0x0030};
>>>>> +static const struct mocs_entry mocs_pte = {0x00000038, 0x0030};
>>>>> +
>>>>> /* The first entries in the MOCS tables are defined by uABI */
>>>>> -static const struct mocs_entry skylake_mocs_table[] = {
>>>>> - { 0x00000009, 0x0010 },
>>>>> - { 0x00000038, 0x0030 },
>>>>> - { 0x0000003b, 0x0030 },
>>>>> +static const struct mocs_entry icelake_mocs_table[GEN11_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN11_NUM_MOCS_ENTRIES - 1] = icelake_mocs_pte,
>>>>> + [0] = { 0x00000005, 0x0010},
>>>>> + [1] = { 0x00000004, 0x0030},
>>>>> + [2] = { 0x00000037, 0x0030},
>>>>> + [3] = { 0x00000005, 0x0010},
>>>>> + [4] = { 0x00000005, 0x0030},
>>>>> + [5] = { 0x00000037, 0x0010},
>>>>> + [6] = { 0x00000017, 0x0010},
>>>>> + [7] = { 0x00000017, 0x0030},
>>>>> + [8] = { 0x00000027, 0x0010},
>>>>> + [9] = { 0x00000027, 0x0030},
>>>>> + [10] = { 0x00000077, 0x0010},
>>>>> + [11] = { 0x00000077, 0x0030},
>>>>> + [12] = { 0x00000057, 0x0010},
>>>>> + [13] = { 0x00000057, 0x0030},
>>>>> + [14] = { 0x00000067, 0x0010},
>>>>> + [15] = { 0x00000067, 0x0030},
>>>>> + [18] = { 0x00060037, 0x0030},
>>>>> + [19] = { 0x00000737, 0x0030},
>>>>> + [20] = { 0x00000337, 0x0030},
>>>>> + [21] = { 0x00000137, 0x0030},
>>>>> + [22] = { 0x000003b7, 0x0030},
>>>>> + [23] = { 0x000007b7, 0x0030},
>>>>> + [62] = { 0x00000037, 0x0010},
>>>>> + [63] = { 0x00000037, 0x0010},
>>>>> +};
>>>>> +
>>>>> +static const struct mocs_entry dirty_icelake_mocs_table[GEN11_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN11_NUM_MOCS_ENTRIES - 1] = { 0x0007FFFF, 0x003F },
>>>>> };
>>>>>
>>>>> -static const struct mocs_entry dirty_skylake_mocs_table[] = {
>>>>> - { 0x00003FFF, 0x003F }, /* no snoop bit */
>>>>> - { 0x00003FFF, 0x003F },
>>>>> - { 0x00003FFF, 0x003F },
>>>>> +static const struct mocs_entry skylake_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = mocs_pte,
>>>>> + [0] = { 0x00000009, 0x0010},
>>>>> + [1] = { 0x00000038, 0x0030},
>>>>> + [2] = { 0x0000003b, 0x0030},
>>>>> };
>>>>>
>>>>> -static const struct mocs_entry broxton_mocs_table[] = {
>>>>> - { 0x00000009, 0x0010 },
>>>>> - { 0x00000038, 0x0030 },
>>>>> - { 0x00000039, 0x0030 },
>>>>> +static const struct mocs_entry dirty_skylake_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = { 0x00003FFF, 0x003F },
>>>>> };
>>>>>
>>>>> -static const struct mocs_entry dirty_broxton_mocs_table[] = {
>>>>> - { 0x00007FFF, 0x003F },
>>>>> - { 0x00007FFF, 0x003F },
>>>>> - { 0x00007FFF, 0x003F },
>>>>> +static const struct mocs_entry broxton_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = mocs_pte,
>>>>> + [0] = { 0x00000009, 0x0010},
>>>>> + [1] = { 0x00000038, 0x0030},
>>>>> + [2] = { 0x00000039, 0x0030},
>>>>> };
>>>>>
>>>>> -static const uint32_t write_values[] = {
>>>>> - 0xFFFFFFFF,
>>>>> - 0xFFFFFFFF,
>>>>> - 0xFFFFFFFF,
>>>>> - 0xFFFFFFFF
>>>>> +static const struct mocs_entry dirty_broxton_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
>>>>> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = { 0x00007FFF, 0x003F },
>>>>> +};
>>>>> +
>>>>> +static const uint32_t write_values[MAX_NUMBER_MOCS_REGISTERS] = {
>>>>> + [0 ... MAX_NUMBER_MOCS_REGISTERS - 1] = 0xFFFFFFFF,
>>>>> };
>>>>>
>>>>> static bool get_mocs_settings(int fd, struct mocs_table *table, bool dirty)
>>>>> @@ -127,6 +160,15 @@ static bool get_mocs_settings(int fd, struct mocs_table *table, bool dirty)
>>>>> table->table = broxton_mocs_table;
>>>>> }
>>>>> result = true;
>>>>> + } else if (IS_ICELAKE(devid)) {
>>>>> + if (dirty) {
>>>>> + table->size = ARRAY_SIZE(dirty_icelake_mocs_table);
>>>>> + table->table = dirty_icelake_mocs_table;
>>>>> + } else {
>>>>> + table->size = ARRAY_SIZE(icelake_mocs_table);
>>>>> + table->table = icelake_mocs_table;
>>>>> + }
>>>>> + result = true;
>>>>> }
>>>>>
>>>>> return result;
>>>>> @@ -374,13 +416,21 @@ static void check_mocs_values(int fd, unsigned engine, uint32_t ctx_id, bool dir
>>>>>
>>>>> static void write_dirty_mocs(int fd, unsigned engine, uint32_t ctx_id)
>>>>> {
>>>>> + uint32_t devid = intel_get_drm_devid(fd);
>>>>> + int num_of_mocs_entries;
>>>>> +
>>>>> + if (IS_ICELAKE(devid))
>>>>> + num_of_mocs_entries = GEN11_NUM_MOCS_ENTRIES;
>>>>> + else
>>>>> + num_of_mocs_entries = GEN9_NUM_MOCS_ENTRIES;
>>>>> +
>>>>> write_registers(fd, ctx_id, get_engine_base(engine),
>>>>> - write_values, ARRAY_SIZE(write_values),
>>>>> + write_values, num_of_mocs_entries,
>>>>> engine);
>>>>>
>>>>> if (engine == I915_EXEC_RENDER)
>>>>> write_registers(fd, ctx_id, GEN9_LNCFCMOCS0,
>>>>> - write_values, ARRAY_SIZE(write_values),
>>>>> + write_values, num_of_mocs_entries/2,
>>>>> engine);
>>>>> }
>>>>>
>>>> The "-dirty" subcase of the test doesn't seem suitable to current hardware..
>>>>
>>>> The table is now global. So writing it from one context and checking if
>>>> another context is unaffected is bound to fail.
>> As you pointed out On Gen11, "-dirty" subcases are failing.
>> Test is not able to write to registers.
>>> We shouldn't be able to do nonpriv writes to it then. Hopefully those
>>> writes are already being dropped?
>> So probably on latest hardware, test should still try to write and call
>> it pass if the dirty values are not being written?
> Yes, that would be useful confirmation that one context can't shoot
> another in the back, and that our global MOCS table remain invariant.
> -Chris
Don't we have a separate test for privileged registers? I'm not very
familiar with all the tests, but I remember someone told me that.
The scope of this test, as written at its beginning, is:
/** @file gem_mocs_settings.c
*
* Check that the MOCs cache settings are valid.
*/
If we already write to MOCS instead of only checking, the scope should
be expanded.
If we're going to check privileged operations, this should also be
included in the scope.
-Tomasz
More information about the igt-dev
mailing list