Re: [PATCH v4] drm/i915 : Added Programming of the MOCS

Peter Antoine
2015-07-02T09:15:42Z

Francisco,

I have had a quick chat here with people and we are good to not upstream
our version of the MOCS. We will handle the updates that we require for
Android separately from the upstream kernel.

Thanks,
Peter.

On Wed, 1 Jul 2015, Daniel Vetter wrote:

> On Wed, Jul 01, 2015 at 08:14:30AM -0700, Jesse Barnes wrote:
>> On 07/01/2015 06:53 AM, Peter Antoine wrote:
>>> On Wed, 1 Jul 2015, Francisco Jerez wrote:
>>>
>>>> Peter Antoine <peter.antoine@intel.com> writes:
>>>>
>>>>> On Tue, 30 Jun 2015, Francisco Jerez wrote:
>>>>>
>>>>>> Francisco Jerez <currojerez@riseup.net> writes:
>>>>>>
>>>>>>> Peter Antoine <peter.antoine@intel.com> writes:
>>>>>>>
>>>>>>>> On Mon, 29 Jun 2015, Peter Antoine wrote:
>>>>>>>>
>>>>>>>>> On Thu, 25 Jun 2015, Francisco Jerez wrote:
>>>>>>>>>
>>>>>>>>>> Peter Antoine <peter.antoine@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...
>
> runtime api needs an open-source user. And it sounds like mesa will be
> happy for a long time with just 3 fixed entries.
>
> I guess this mocs upstreaming went nowhere unfortunately :( Imo better to
> concentrate efforts in areas where we can get somewhere (guc, preempt,
> tdr, whatever).
> -Daniel
>

--
Peter Antoine (Android Graphics Driver Software Engineer)
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47