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

Daniel Vetter
2015-07-01T15:38:58Z

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
--
Daniel Vetter
Software Engineer, Intel Corporation