Re: Make drm drivers use MTRR write-combine

2018-08-16 Thread Johannes Lundberg
On Tue, Aug 14, 2018 at 9:54 PM Johannes Lundberg 
wrote:

>
>
> On Tue, Aug 14, 2018 at 3:39 PM Konstantin Belousov 
> wrote:
>
>> On Tue, Aug 14, 2018 at 08:55:36AM -0500, Eric van Gyzen wrote:
>> > On 8/14/18 4:12 AM, Johannes Lundberg wrote:
>> > > Hi
>> > >
>> > > Something that we have seen for a long time on FreeBSD is the boot
>> message
>> > >
>> > > Failed to add WC MTRR for [0xd000-0xdfff]: -22; performance
>> may
>> > > suffer
>> > >
>> > > Taking a closer look at this with memcontrol I can see that the 256 MB
>> > > region that DRM wants to set as WC is already covered by this entry
>> > > 0xc000/0x4000 BIOS uncacheable set-by-firmware active
>> > >
>> > > Similar on both my Skylake and Broadwell systems.
>> > I see something similar on my Dell XPS 13 with a Kaby Lake R:
>> >
>> > Failed to add WC MTRR for [0x9000-0x9fff]: -22; performance may
>> > suffer
>> >
>> > 0x8000/0x8000 BIOS uncacheable set-by-firmware active
>> >
>> > The only mappings in this range are MMIO:
>> >
>> > machdep.efi_map:
>> >Type Physical  Virtual   #Pages Attr
>> > [snip]
>> > MemoryMappedIO e000   0xe000 0001 RUNTIME
>> > MemoryMappedIO fe00   0xfe00 0011 UC RUNTIME
>> > MemoryMappedIO fec0   0xfec0 0001 UC RUNTIME
>> > MemoryMappedIO fee0   0xfee0 0001 UC WT WB WP RUNTIME
>> > MemoryMappedIO ff00   0xff00 1000 UC WT WB WP RUNTIME
>>
>> Yes, the cause of the message is that current x86 mtrr code is not
>> sufficient to handle this situation. You have BIOS-configured variable
>> range MTRR which covers upper half of the low 4G, as uncacheable (UC).
>> It is reasonable for BIOS to set it up this way because this is where
>> PCIe BARs and other devices MMIO regions are located.
>>
>> One of the BARs there is the GPU aperture that really should be WC
>> (write-combining). There are two ways to achieve this: split the UC
>> variable-length MTRR range into three, UC/WC/UC, which would require
>> three MTRRs to cover. This is what current x86_mem.c code does not
>> support.
>>
>> Another way is to set WC mode in the page table entries (PTEs) using
>> Page Attribute Table (PAT), for all PTEs. According to the rules of
>> combination of the memory access types between MTRR and PAT, WC in PAT
>> and any access mode in MTRR gives effective WC.
>>
>> I saw the same warning when I initially ported GEM. My code used WC PAT
>> type, which makes the warning cosmetical, and which made me to not add
>> MTRR split. If new drm driver also consistently uses WC memattr when
>> creating aperture mappings, then the warning can be ignored as well.
>>
>
> Hi Kib
>
> Thanks for the detailed answer. This might already be the case for the out
> of tree drivers as well. From what I read about the VESA driver the
> performance difference should be quite big w/o WC and I haven't noticed and
> performance issues with the newer drivers at all.
>
> I will confirm this tomorrow.
>

Yeah, seems like it's already done.

https://github.com/FreeBSDDesktop/kms-drm/blob/drm-v4.15/linuxkpi/gplv2/src/linux_page.c#L260

https://github.com/FreeBSDDesktop/kms-drm/blob/drm-v4.15/drivers/gpu/drm/i915/i915_gem_gtt.c#L415
https://github.com/FreeBSDDesktop/kms-drm/blob/drm-v4.15/drivers/gpu/drm/ttm/ttm_page_alloc.c#L534


>
> ___
>> freebsd-current@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/freebsd-current
>> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org
>> "
>>
>
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: Make drm drivers use MTRR write-combine

2018-08-14 Thread Johannes Lundberg
On Tue, Aug 14, 2018 at 3:39 PM Konstantin Belousov 
wrote:

> On Tue, Aug 14, 2018 at 08:55:36AM -0500, Eric van Gyzen wrote:
> > On 8/14/18 4:12 AM, Johannes Lundberg wrote:
> > > Hi
> > >
> > > Something that we have seen for a long time on FreeBSD is the boot
> message
> > >
> > > Failed to add WC MTRR for [0xd000-0xdfff]: -22; performance may
> > > suffer
> > >
> > > Taking a closer look at this with memcontrol I can see that the 256 MB
> > > region that DRM wants to set as WC is already covered by this entry
> > > 0xc000/0x4000 BIOS uncacheable set-by-firmware active
> > >
> > > Similar on both my Skylake and Broadwell systems.
> > I see something similar on my Dell XPS 13 with a Kaby Lake R:
> >
> > Failed to add WC MTRR for [0x9000-0x9fff]: -22; performance may
> > suffer
> >
> > 0x8000/0x8000 BIOS uncacheable set-by-firmware active
> >
> > The only mappings in this range are MMIO:
> >
> > machdep.efi_map:
> >Type Physical  Virtual   #Pages Attr
> > [snip]
> > MemoryMappedIO e000   0xe000 0001 RUNTIME
> > MemoryMappedIO fe00   0xfe00 0011 UC RUNTIME
> > MemoryMappedIO fec0   0xfec0 0001 UC RUNTIME
> > MemoryMappedIO fee0   0xfee0 0001 UC WT WB WP RUNTIME
> > MemoryMappedIO ff00   0xff00 1000 UC WT WB WP RUNTIME
>
> Yes, the cause of the message is that current x86 mtrr code is not
> sufficient to handle this situation. You have BIOS-configured variable
> range MTRR which covers upper half of the low 4G, as uncacheable (UC).
> It is reasonable for BIOS to set it up this way because this is where
> PCIe BARs and other devices MMIO regions are located.
>
> One of the BARs there is the GPU aperture that really should be WC
> (write-combining). There are two ways to achieve this: split the UC
> variable-length MTRR range into three, UC/WC/UC, which would require
> three MTRRs to cover. This is what current x86_mem.c code does not
> support.
>
> Another way is to set WC mode in the page table entries (PTEs) using
> Page Attribute Table (PAT), for all PTEs. According to the rules of
> combination of the memory access types between MTRR and PAT, WC in PAT
> and any access mode in MTRR gives effective WC.
>
> I saw the same warning when I initially ported GEM. My code used WC PAT
> type, which makes the warning cosmetical, and which made me to not add
> MTRR split. If new drm driver also consistently uses WC memattr when
> creating aperture mappings, then the warning can be ignored as well.
>

Hi Kib

Thanks for the detailed answer. This might already be the case for the out
of tree drivers as well. From what I read about the VESA driver the
performance difference should be quite big w/o WC and I haven't noticed and
performance issues with the newer drivers at all.

I will confirm this tomorrow.

___
> freebsd-current@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
>
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: Make drm drivers use MTRR write-combine

2018-08-14 Thread Konstantin Belousov
On Tue, Aug 14, 2018 at 08:55:36AM -0500, Eric van Gyzen wrote:
> On 8/14/18 4:12 AM, Johannes Lundberg wrote:
> > Hi
> > 
> > Something that we have seen for a long time on FreeBSD is the boot message
> > 
> > Failed to add WC MTRR for [0xd000-0xdfff]: -22; performance may
> > suffer
> > 
> > Taking a closer look at this with memcontrol I can see that the 256 MB
> > region that DRM wants to set as WC is already covered by this entry
> > 0xc000/0x4000 BIOS uncacheable set-by-firmware active
> > 
> > Similar on both my Skylake and Broadwell systems.
> I see something similar on my Dell XPS 13 with a Kaby Lake R:
> 
> Failed to add WC MTRR for [0x9000-0x9fff]: -22; performance may 
> suffer
> 
> 0x8000/0x8000 BIOS uncacheable set-by-firmware active
> 
> The only mappings in this range are MMIO:
> 
> machdep.efi_map:
>Type Physical  Virtual   #Pages Attr
> [snip]
> MemoryMappedIO e000   0xe000 0001 RUNTIME
> MemoryMappedIO fe00   0xfe00 0011 UC RUNTIME
> MemoryMappedIO fec0   0xfec0 0001 UC RUNTIME
> MemoryMappedIO fee0   0xfee0 0001 UC WT WB WP RUNTIME
> MemoryMappedIO ff00   0xff00 1000 UC WT WB WP RUNTIME

Yes, the cause of the message is that current x86 mtrr code is not
sufficient to handle this situation. You have BIOS-configured variable
range MTRR which covers upper half of the low 4G, as uncacheable (UC).
It is reasonable for BIOS to set it up this way because this is where
PCIe BARs and other devices MMIO regions are located.

One of the BARs there is the GPU aperture that really should be WC
(write-combining). There are two ways to achieve this: split the UC
variable-length MTRR range into three, UC/WC/UC, which would require
three MTRRs to cover. This is what current x86_mem.c code does not
support.

Another way is to set WC mode in the page table entries (PTEs) using
Page Attribute Table (PAT), for all PTEs. According to the rules of
combination of the memory access types between MTRR and PAT, WC in PAT
and any access mode in MTRR gives effective WC.

I saw the same warning when I initially ported GEM. My code used WC PAT
type, which makes the warning cosmetical, and which made me to not add
MTRR split. If new drm driver also consistently uses WC memattr when
creating aperture mappings, then the warning can be ignored as well.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: Make drm drivers use MTRR write-combine

2018-08-14 Thread Eric van Gyzen

On 8/14/18 4:12 AM, Johannes Lundberg wrote:

Hi

Something that we have seen for a long time on FreeBSD is the boot message

Failed to add WC MTRR for [0xd000-0xdfff]: -22; performance may
suffer

Taking a closer look at this with memcontrol I can see that the 256 MB
region that DRM wants to set as WC is already covered by this entry
0xc000/0x4000 BIOS uncacheable set-by-firmware active

Similar on both my Skylake and Broadwell systems.

I see something similar on my Dell XPS 13 with a Kaby Lake R:

Failed to add WC MTRR for [0x9000-0x9fff]: -22; performance may 
suffer


0x8000/0x8000 BIOS uncacheable set-by-firmware active

The only mappings in this range are MMIO:

machdep.efi_map:
  Type Physical  Virtual   #Pages Attr
[snip]
MemoryMappedIO e000   0xe000 0001 RUNTIME
MemoryMappedIO fe00   0xfe00 0011 UC RUNTIME
MemoryMappedIO fec0   0xfec0 0001 UC RUNTIME
MemoryMappedIO fee0   0xfee0 0001 UC WT WB WP RUNTIME
MemoryMappedIO ff00   0xff00 1000 UC WT WB WP RUNTIME

Eric
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Make drm drivers use MTRR write-combine

2018-08-14 Thread Johannes Lundberg
Hi

Something that we have seen for a long time on FreeBSD is the boot message

Failed to add WC MTRR for [0xd000-0xdfff]: -22; performance may
suffer

Taking a closer look at this with memcontrol I can see that the 256 MB
region that DRM wants to set as WC is already covered by this entry
0xc000/0x4000 BIOS uncacheable set-by-firmware active

Similar on both my Skylake and Broadwell systems.

The linuxkpi wrapper can be found here:
https://github.com/FreeBSDDesktop/kms-drm/blob/drm-v4.15/linuxkpi/gplv2/src/linux_mtrr.c

There doesn't seem to exist a function for changing the properties of a sub
region:
https://github.com/FreeBSDDesktop/freebsd-base/blob/master/sys/dev/mem/memutil.c

Any ideas of a good solution to this? Can this region be blacklisted or is
there a safe way to split the big region into several regions with
different flags when the drm driver loads?

For reference, my AMD machine logs this
# dmesg | grep MTRR
Successfully added WC MTRR for [0xe000-0xefff]: 0;
# memcontrol list
--SNIP--
0xff000/0x1000 BIOS write-protect fixed-base fixed-length set-by-firmware
active
0x0/0x8000 BIOS write-back set-by-firmware active
0x8000/0x4000 BIOS write-back set-by-firmware active
0xc000/0x2000 BIOS write-back set-by-firmware active
0xe000/0x1000 drm write-combine active

Not sure if it's a BIOS or CPU vendor issue.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"