[Intel-gfx] [PATCH v8 5/7] drm/i915: keep track of used entries in MOCS table

[Lucas De Marchi]

On Tue, Jan 22, 2019 at 02:40:48PM +0000, Chris Wilson wrote:
>Quoting Lucas De Marchi (2019-01-22 05:12:25)
>> Instead of considering we have defined entries for any index in the
>> table, let's keep track of the ones we explicitly defined. This will
>> allow Gen 11 to have it's new table defined in which we have holes of
>> undefined entries.
>>
>> Repeated comments about the meaning of undefined entries were removed
>> since they are overly verbose and copy-pasted in several functions: now
>> the definition is in the top only.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_mocs.c | 88 ++++++++++++++++++++-----------
>>  1 file changed, 57 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
>> index faae2eefc5cc..af2ae2f396ae 100644
>> --- a/drivers/gpu/drm/i915/intel_mocs.c
>> +++ b/drivers/gpu/drm/i915/intel_mocs.c
>> @@ -28,6 +28,7 @@
>>  struct drm_i915_mocs_entry {
>>         u32 control_value;
>>         u16 l3cc_value;
>> +       u16 used;
>>  };
>
>> +       /* Set unused values to PTE */
>> +       unused_index = I915_MOCS_PTE;
>> +
>> +       for (i = 0; i < table.size / 2; i++) {
>> +               u16 low = table.table[2 * i].used ?
>> +                       2 * i : unused_index;
>> +               u16 high = table.table[2 * i + 1].used ?
>> +                       2 * i + 1 : unused_index;
>
>I'm underwhelmed here.

Indeed, I should have put more effort in making this readable :)

>
>Could we not do something like
>
>static unsigned int
>get_entry_index(struct tbl *tbl, unsigned int idx, unsigned int unused_index)
>{
>	return tbl->used ? idx : unused_index;
>}
>
>		u16 lo = get_entry_index(table.table, 2 * i, unused_index);
>		u16 hi = get_entry_index(table.table, 2 * i + 1, unused_index);
>
>That just fits and repeated enough to be worth a little extra effort.

in order to make the l3cc and control parts more similar, I did it like
this:

    static u16 get_entry_l3cc(const struct drm_i915_mocs_table *table,
    			  unsigned int index)
    {
    	if (table->table[index].used)
    		return table->table[index].l3cc_value;
    
    	return table->table[I915_MOCS_PTE].l3cc_value;
    }
    
    static u32 get_entry_control(const struct drm_i915_mocs_table *table,
    			     unsigned int index)
    {
    	if (table->table[index].used)
    		return table->table[index].control_value;
    
    	return table->table[I915_MOCS_PTE].control_value;
    }

Then on all of them use as value rather than as value in one and index
in the other (adapting l3cc_combine appropriately).

This series is conflicting now with drm-intel, so I will need to re-send
it entirely.

thanks
Lucas De Marchi