[Intel-gfx] [PATCH v4] drm/i915 : Added Programming of the MOCS

[Jesse Barnes]

On 07/01/2015 06:53 AM, Peter Antoine wrote:
> On Wed, 1 Jul 2015, Francisco Jerez wrote:
> 
>> Peter Antoine <peter.antoine at intel.com> writes:
>>
>>> On Tue, 30 Jun 2015, Francisco Jerez wrote:
>>>
>>>> Francisco Jerez <currojerez at riseup.net> writes:
>>>>
>>>>> Peter Antoine <peter.antoine at intel.com> writes:
>>>>>
>>>>>> On Mon, 29 Jun 2015, Peter Antoine wrote:
>>>>>>
>>>>>>> On Thu, 25 Jun 2015, Francisco Jerez wrote:
>>>>>>>
>>>>>>>> Peter Antoine <peter.antoine at intel.com> writes:
>>>>>>>> Mesa will want an additional entry with TC=LLC/eLLC, LeCC=PTE,
>>>>>>>> L3CC=WB,
>>>>>>>> everything else unset, I'll reply with a userspace patch making
>>>>>>>> use of
>>>>>>>> your change if you add such an entry.
>>>>>> Ok. I think what you want is, same as entry two, but use the
>>>>>> underlying
>>>>>> pagetable settings and not specify the EDRAM settings. Please
>>>>>> confirm in
>>>>>> the new patchset.
>>>>>
>>>>> Yeah, that sounds good.
>>>>>
>>>>>>>>
>>>>>>>> Another thing worth mentioning is that entries 0, 2 and 5 seem
>>>>>>>> to do the
>>>>>>>> same thing suspiciously, the only difference is the LRUM field
>>>>>>>> which
>>>>>>>> AFAIK doesn't have any effect for LeCC=UC.  Is my understanding
>>>>>>>> correct?
>>>>>>>>
>>>>>>> These tables are generated via requests and then boiled down to
>>>>>>> the above.
>>>>>>> So some of the entries are by request. Swings and roundabouts,
>>>>>>> can remove
>>>>>>> the ones that look redundant but then the tuning that has been
>>>>>>> done wont
>>>>>>> match. I'll add the new entry at the end of the table.
>>>>
>>>> Are you planning to propagate the entry you just added back to the
>>>> original table this was generated from?  What about new entries we may
>>>> need to add in the future?  What should be the process to make sure
>>>> that
>>>> our table and the master table don't diverge and end up with
>>>> conflicting
>>>> entries we cannot remove because of ABI compatibility?  I guess there
>>>> should be a comment on the top warning that the table is part of the
>>>> kernel ABI and supposed to be kept in sync with your table, so other
>>>> people don't change it unknowingly?
>>>>
>>>> Thanks.
>>> I am talking to the team that handles this and see if they will add this
>>> (so future gens this is baked in) but it is unlikely that the other
>>> tables
>>> will stay in step as getting in changes will cause too much grief
>>> getting
>>> them upstreamed and as the table is auto-generated we will not be
>>> able to
>>> guarantee the ordering. It will have to be manual job for anyone doing
>>> this. It is required for other platforms for the tables to match the
>>> userspace for performance reasons, but on Linux it will be by request if
>>> there is a problem. We will see what happens.
>>>
>> I think it only makes sense for Linux to maintain compatibility with
>> Android's tables if we agree on some straightforward process for us to
>> allocate new entries without causing conflicts (otherwise people are
>> likely to ignore the issue completely and let the tables diverge, as you
>> mentioned yourself), and have some guarantee that any entries ever
>> contributed by your team to the Linux kernel (and therefore part of our
>> stable ABI) will never be changed or reordered in the future.
>>
> I think internally (and informally) that we cannot keep sync between
> Android
> and Linux. We need to keep compatibility with userspace and there is no
> guarantee of ordering as these tables are generated at runtime. The tables
> that are in Linux are a snapshot. These changes are supposed to
> stabilise at
> PV so they don't change in the future, but if a bug or good performance
> enhancement occurs I can't imagine that they wont make the changes.

Wow this discussion just keeps going.  Who'd have thought such a simple
table would cause so much trouble? :)

What you mention above is a key point: "these tables are generated at
runtime. The tables that are in Linux are a snapshot. These changes are
supposed to stabilise at PV so they don't change in the future, but if a
bug or good performance enhancement occurs I can't imagine that they
wont make the changes."

That really argues for a runtime API that allows the userland drivers to
load in MOCS values.  I'm not sure if it's practical to make the table
effectively part of the context (lazily applying new values if we detect
a change vs the defaults), but that would at least let the different
user level drivers do whatever they think is ideal...

>> I have the impression that because of your development model you have
>> far more freedom to make changes in your kernel ABI after the fact than
>> we do -- OTOH we would be locked in if we accept to import Android's
>> tables now, what brings me to the next question: How would you feel
>> about reversing the roles of our tables?  The workflow could be as
>> follows:
> The Android kernel is more flexible, in what it accepts, and secondly (and
> more importantly) you should be using the userspace drivers as this is
> the API and is tuned, so changing the tables are less of a problem.
>>
>>
>> - The MOCS tables part of the Linux kernel would be developed and
>>   discussed publicly in this mailing list, independently from your team
>>   (which doesn't mean that your contributions and feed-back regarding
>>   future changes in the MOCS tables wouldn't be very much welcome).
>>
> This is fine. But be aware the RFC for the MOCS was first floated in
> March and
> teams were directly contacted when this happened and not a lot of response
> was received. MOCS ain't sexy and people only get interested when they
> feel that they maybe a performance problem - then they look to caching.
> 
> But, I think this is the sensible model. For the new tables (new gens) a
> drop
> from the internal cache models as these will have had some form of
> tuning from
> different teams and requirements (OpenCL, OpenGL, Media, Security,
> etc...) then
> these can be discussed on the mailing list as required.
> 
> I am not sure if that is acceptable to everyone. As we are going to have to
> carry some patches in Android and drop any upstream MOCS changes.

Yeah given the potential for changes coming from the Android side, it
seems like this is the way to go for upstream.  I guess that means we
just need a couple of entries to start with in upstream, and we can add
to them in an ordered and compatible way as needed (unless we decide to
pursue a runtime interface anyway).

> Like you, it would be nice too see what others think.

I guess I'd prefer the simple approach of just adding a few entries at a
time after discussion, and a runtime interface if we need it.  It's too
bad the Android side can't commit to a stable set of entries with
specific ordering, with just additions as needed.  That would let us
just merge your patch as a baseline...

Jesse