[Mesa-dev] [PATCH] i965/gen9: Use custom MOCS entries set up by the kernel.
Francisco Jerez
currojerez at riseup.net
Tue Jun 30 16:04:49 PDT 2015
Ben Widawsky <ben at bwidawsk.net> writes:
> On Wed, Jul 01, 2015 at 12:33:54AM +0300, Francisco Jerez wrote:
>> Ben Widawsky <ben at bwidawsk.net> writes:
>>
>> > On Tue, Jun 30, 2015 at 11:25:42PM +0300, Francisco Jerez wrote:
>> >> Instead of relying on hardware defaults the i915 kernel driver is
>> >> going program custom MOCS tables system-wide on Gen9 hardware. The
>> >> "WT" entry previously used for renderbuffers had a number of problems:
>> >> It disabled caching on eLLC, it used a reserved L3 cacheability
>> >> setting, and it used to override the PTE controls making renderbuffers
>> >> always WT on LLC regardless of the kernel's setting. Instead use an
>> >> entry from the new MOCS tables with parameters: TC=LLC/eLLC, LeCC=PTE,
>> >> L3CC=WB.
>> >>
>> >> Even though the corresponding kernel change is in a way an ABI break
>> >> it doesn't seem necessary to check that the kernel is recent enough
>> >> because the change should only affect Gen9 which is still unreleased
>> >> hardware.
>> >
>> > I think the commit message is a bit confusing. You correctly mention the WT->PTE
>> > fix, but then the reasoning for the WB change isn't clear [to me].
>> >
>> Right, I probably didn't mention it because the meaning of the WB define
>> didn't change at all, the index into the new MOCS table is different but
>> it should have the same semantics.
>>
>
> I figured, just add it to the commit message :-)
OK, fixed.
>
>> > In any case, I think it makes a lot more sense to fix the PTE setting as one
>> > patch for the old table, then a patch to update both WB and WT to the new table
>> > settings.
>>
>> I tried to split up the patch that way originally, but unfortunately
>> there's no entry in the default MOCS table equivalent to the new PTE
>> setting, and there is also no equivalent to the old WT setting in the
>> custom MOCS table (and it probably doesn't make sense to add one just
>> for the sake of having a nice git history), so it doesn't seem easily
>> possible to do it backwards either (first update to the new table, then
>> switch to the PTE MOCS setting).
>
> Hmm. I must not be following something because it sure looks like the HW
> defaults have indices for the PTE setting. The index you're using from the new
> table, 9 is just the hardware index 2, isn't it?
>
> 000010 00 10 11 0 0 00 000
>
> Can you explain what I'm missing?
>
I don't have the spec in front of me right now, but I remember that all
entries that had matching (e)LLC settings had different L3 settings (and
the other way around), so the change switching to the new table entries
would have necessarily been a functional change either way.
>>
>> > Also, we do have customers (Canonical) that want to make this work on
>> > mesa 10.5, and with an older kernel. Therefore I think the two separate patches,
>> > and doing it without the dependency on Ville's patch (which I like FWIW) make
>> > the lives of everyone easiest. Then Ville can rebase his patch on top of this
>> > for mesa 10.7 time.
>> >
>> The problem is that an equivalent patch not based on Ville's refactor
>> would involve a considerable amount of churn because the BXT and SKL WB
>> entries (which are used in many different places) don't match (sigh).
>> It may not be suitable for stable either way, unless we drop BXT support
>> or are OK with adding a bunch of ternary operators, basically anywhere
>> SKL_MOCS_WB is used.
>
> Yeah, we don't need BXT support in stable since BXT won't have PCI IDs until
> 10.7. So I'd be in favor of doing the easy SKL specific thing first if it's
> possible
>
That sounds OK to me, I'll send another version of the patch for stable
with the minimal set of changes for it to work on SKL.
> Please tell me there is a good reason that they didn't make BXT and SKL the
> same...
Heh, ask Peter. :)
>
>>
>> > I did think of it, but never broached the subject if we want to send both my
>> > MOCS patch, and the PTE version of this patch to stable.
>> >
>> > Anyway, the concept here is definitely
>> > Acked-by: Ben Widawsky <ben at bwidawsk.net>
>> >
>> >> ---
>> >> Note that this change is based on Ville's "[PATCH 1/2] i965: House
>> >> MOCS settings in brw_context/brw_device_info":
>> >>
>> >> http://lists.freedesktop.org/archives/mesa-dev/2015-June/086665.html
>> >
>> > Could you include a reference to the kernel patch too if you end up resending?
>>
>> Ah, sure, here it is FTR, I didn't notice I hadn't included the link
>> until it was too late:
>>
>> http://lists.freedesktop.org/archives/intel-gfx/2015-June/070244.html
>>
>
> Thanks.
>
>> >
>> >>
>> >> src/mesa/drivers/dri/i965/brw_defines.h | 16 +++++++++++-----
>> >> src/mesa/drivers/dri/i965/brw_device_info.c | 5 +++--
>> >> 2 files changed, 14 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> index 497da9c..2889118 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> >> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> >> @@ -2499,12 +2499,18 @@ enum brw_wm_barycentric_interp_mode {
>> >> */
>> >> #define CHV_MOCS_L3 0x78
>> >>
>> >> -/* Skylake: MOCS is now an index into an array of 64 different configurable
>> >> - * cache settings. We still use only either write-back or write-through; and
>> >> - * rely on the documented default values.
>> >> +/* Skylake: MOCS is now an index into an array of 64 different caching
>> >> + * configurations programmed by the kernel.
>> >> */
>> >> -#define SKL_MOCS_WB (0b001001 << 1)
>> >> -#define SKL_MOCS_WT (0b000101 << 1)
>> >> +/* TC=LLC/eLLC, LeCC=WB, LRUM=3, L3CC=WB */
>> >> +#define SKL_MOCS_WB (1 << 1)
>> >> +/* TC=LLC/eLLC, LeCC=PTE, LRUM=3, L3CC=WB */
>> >> +#define SKL_MOCS_PTE (9 << 1)
>> >> +
>> >> +/* Broxton: As for Skylake this should match the tables set up by the kernel.
>> >> + */
>> >> +/* L3CC=WB */
>> >> +#define BXT_MOCS_L3 (9 << 1)
>> >>
>> >> #define MEDIA_VFE_STATE 0x7000
>> >> /* GEN7 DW2, GEN8+ DW3 */
>> >> diff --git a/src/mesa/drivers/dri/i965/brw_device_info.c b/src/mesa/drivers/dri/i965/brw_device_info.c
>> >> index 167ecb5..d5133e0 100644
>> >> --- a/src/mesa/drivers/dri/i965/brw_device_info.c
>> >> +++ b/src/mesa/drivers/dri/i965/brw_device_info.c
>> >> @@ -305,7 +305,6 @@ static const struct brw_device_info brw_device_info_chv = {
>> >> };
>> >>
>> >> /* Thread counts and URB limits are placeholders, and may not be accurate. */
>> >> -/* FINISHME: Use PTE MOCS on Skylake. */
>> >> #define GEN9_FEATURES \
>> >> .gen = 9, \
>> >> .has_hiz_and_separate_stencil = true, \
>> >> @@ -315,7 +314,7 @@ static const struct brw_device_info brw_device_info_chv = {
>> >> .max_vs_threads = 280, \
>> >> .max_gs_threads = 256, \
>> >> .max_wm_threads = 408, \
>> >> - .mocs_pte = SKL_MOCS_WT, \
>> >> + .mocs_pte = SKL_MOCS_PTE, \
>> >> .mocs_wb = SKL_MOCS_WB, \
>> >> .urb = { \
>> >> .size = 128, \
>> >> @@ -352,6 +351,8 @@ static const struct brw_device_info brw_device_info_bxt = {
>> >> .max_vs_threads = 112,
>> >> .max_gs_threads = 112,
>> >> .max_wm_threads = 32,
>> >> + .mocs_pte = BXT_MOCS_L3,
>> >> + .mocs_wb = BXT_MOCS_L3,
>> >> .urb = {
>> >> .size = 64,
>> >> .min_vs_entries = 34,
>> >> --
>> >> 2.4.3
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> > --
>> > Ben Widawsky, Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150701/633e5d10/attachment-0001.sig>
More information about the mesa-dev
mailing list