[Mesa-dev] [PATCH v4 1/2] intel: 48b ppgtt support (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag)
On Fri, Dec 4, 2015 at 6:24 AM, Michel Thierry wrote: > On 11/18/2015 10:53 PM, Kristian Høgsberg wrote: >> >> On Wed, Oct 14, 2015 at 5:11 AM, Michel Thierry >> wrote: >>> >>> On 10/14/2015 8:19 AM, Daniel Vetter wrote: On Tue, Oct 13, 2015 at 02:51:36PM -0700, Kristian Høgsberg wrote: > > > On Tue, Oct 13, 2015 at 7:55 AM, Michel Thierry > wrote: >> >> >> On 10/13/2015 3:13 PM, Emil Velikov wrote: >>> >>> >>> >>> On 13 October 2015 at 13:16, Michel Thierry >>> >>> wrote: On 10/6/2015 2:12 PM, Michel Thierry wrote: > > > > > On 10/5/2015 7:06 PM, Kristian Høgsberg wrote: >> >> >> >> >> On Mon, Oct 5, 2015 at 7:03 AM, Michel Thierry >> wrote: >>> >>> >>> >>> >>> On 9/14/2015 2:54 PM, MichaÅ Winiarski wrote: On Thu, Sep 03, 2015 at 03:23:58PM +0100, Michel Thierry wrote: > > > > > > Gen8+ supports 48-bit virtual addresses, but some objects must > always be > allocated inside the 32-bit address range. > > In specific, any resource used with flat/heapless > (0x-0xf000) > General State Heap (GSH) or Instruction State Heap (ISH) must > be > in > a > 32-bit range, because the General State Offset and Instruction > State > Offset > are limited to 32-bits. > > The i915 driver has been modified to provide a flag to set when > the > 4GB > limit is not necessary in a given bo > (EXEC_OBJECT_SUPPORTS_48B_ADDRESS). > 48-bit range will only be used when explicitly requested. > > Callers to the existing drm_intel_bo_emit_reloc function should > set > the > use_48b_address_range flag beforehand, in order to use full > ppgtt > range. > > v2: Make set/clear functions nops on pre-gen8 platforms, and > use > them > internally in emit_reloc functions (Ben) > s/48BADDRESS/48B_ADDRESS/ (Dave) > v3: Keep set/clear functions internal, no-one needs to use them > directly. > v4: Don't set 48bit-support flag in emit reloc, check for ppgtt > type > before enabling set/clear function, print full offsets > in > debug > statements, using port of lower_32_bits and > upper_32_bits > from linux > kernel (MichaÅ) > > References: > > > > http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html > Cc: Ben Widawsky > Cc: MichaÅ Winiarski +Kristian LGTM. Acked-by: MichaÅ Winiarski > Signed-off-by: Michel Thierry >>> >>> >>> >>> >>> >>> >>> >>> Hi Kristian, >>> >>> More comments on this? >>> I've resent the mesa patch with the last changes >>> >>> >>> >>> >>> (http://lists.freedesktop.org/archives/dri-devel/2015-October/091752.html). >>> >>> >>> MichaÅ has agreed with the interface and will use it for his >>> work. >>> If mesa doesn't plan to use this for now, it's ok. The kernel >>> changes >>> have >>> been done to prevent any regressions when the support 48-bit flag >>> is >>> not >>> set. >> >> >> >> >> >> I didn't get any replies to my last comments on this: >> >> >> >> http://lists.freedesktop.org/archives/mesa-dev/2015-August/091398.html >> >> Basically, I tried explicitly placing buffers above 8G and didn't >> see >> the HW problem described (ie it all worked fine). I still think >> there's some confusion as to what the W/A is about. >> >> Here's my take: the W/A is a SW-only workaround to handle the >> cases >> where a driver uses heapless and 48-bit ppgtt. The problem is that >> the >> heaps are limited to 4G but with 48bit ppgtt a buffer can be >> placed >> anywhere it the 48 bit address space. If that happens it's
[Mesa-dev] [PATCH v4 1/2] intel: 48b ppgtt support (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag)
On 11/18/2015 10:53 PM, Kristian Høgsberg wrote: > On Wed, Oct 14, 2015 at 5:11 AM, Michel Thierry > wrote: >> On 10/14/2015 8:19 AM, Daniel Vetter wrote: >>> >>> On Tue, Oct 13, 2015 at 02:51:36PM -0700, Kristian Høgsberg wrote: On Tue, Oct 13, 2015 at 7:55 AM, Michel Thierry wrote: > > On 10/13/2015 3:13 PM, Emil Velikov wrote: >> >> >> On 13 October 2015 at 13:16, Michel Thierry >> wrote: >>> >>> >>> On 10/6/2015 2:12 PM, Michel Thierry wrote: On 10/5/2015 7:06 PM, Kristian Høgsberg wrote: > > > > On Mon, Oct 5, 2015 at 7:03 AM, Michel Thierry > wrote: >> >> >> >> On 9/14/2015 2:54 PM, MichaÅ Winiarski wrote: >>> >>> >>> >>> >>> On Thu, Sep 03, 2015 at 03:23:58PM +0100, Michel Thierry wrote: Gen8+ supports 48-bit virtual addresses, but some objects must always be allocated inside the 32-bit address range. In specific, any resource used with flat/heapless (0x-0xf000) General State Heap (GSH) or Instruction State Heap (ISH) must be in a 32-bit range, because the General State Offset and Instruction State Offset are limited to 32-bits. The i915 driver has been modified to provide a flag to set when the 4GB limit is not necessary in a given bo (EXEC_OBJECT_SUPPORTS_48B_ADDRESS). 48-bit range will only be used when explicitly requested. Callers to the existing drm_intel_bo_emit_reloc function should set the use_48b_address_range flag beforehand, in order to use full ppgtt range. v2: Make set/clear functions nops on pre-gen8 platforms, and use them internally in emit_reloc functions (Ben) s/48BADDRESS/48B_ADDRESS/ (Dave) v3: Keep set/clear functions internal, no-one needs to use them directly. v4: Don't set 48bit-support flag in emit reloc, check for ppgtt type before enabling set/clear function, print full offsets in debug statements, using port of lower_32_bits and upper_32_bits from linux kernel (MichaÅ) References: http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html Cc: Ben Widawsky Cc: MichaÅ Winiarski >>> >>> >>> >>> >>> >>> +Kristian >>> >>> LGTM. >>> Acked-by: MichaÅ Winiarski >>> Signed-off-by: Michel Thierry >> >> >> >> >> >> >> Hi Kristian, >> >> More comments on this? >> I've resent the mesa patch with the last changes >> >> >> >> (http://lists.freedesktop.org/archives/dri-devel/2015-October/091752.html). >> >> >> MichaÅ has agreed with the interface and will use it for his work. >> If mesa doesn't plan to use this for now, it's ok. The kernel >> changes >> have >> been done to prevent any regressions when the support 48-bit flag >> is >> not >> set. > > > > > I didn't get any replies to my last comments on this: > > > http://lists.freedesktop.org/archives/mesa-dev/2015-August/091398.html > > Basically, I tried explicitly placing buffers above 8G and didn't > see > the HW problem described (ie it all worked fine). I still think > there's some confusion as to what the W/A is about. > > Here's my take: the W/A is a SW-only workaround to handle the cases > where a driver uses heapless and 48-bit ppgtt. The problem is that > the > heaps are limited to 4G but with 48bit ppgtt a buffer can be placed > anywhere it the 48 bit address space. If that happens it's > consideredd > out-of-bounds for the heap and access fails. To prevent this we need > to make sure the bos we address in a heapless fashion are placed > below > 4g. > > For mesa, we only configure general state base as heap-less, which > affects scratch bos. What this boils down to is that we need the 4G > restriction only for the scratch bos set up on 3DSTATE_VS, > 3DSTATE_GS > and 3DSTATE_PS
[Mesa-dev] [PATCH v4 1/2] intel: 48b ppgtt support (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag)
On Wed, Oct 14, 2015 at 5:11 AM, Michel Thierry wrote: > On 10/14/2015 8:19 AM, Daniel Vetter wrote: >> >> On Tue, Oct 13, 2015 at 02:51:36PM -0700, Kristian Høgsberg wrote: >>> >>> On Tue, Oct 13, 2015 at 7:55 AM, Michel Thierry >>> wrote: On 10/13/2015 3:13 PM, Emil Velikov wrote: > > > On 13 October 2015 at 13:16, Michel Thierry > wrote: >> >> >> On 10/6/2015 2:12 PM, Michel Thierry wrote: >>> >>> >>> >>> On 10/5/2015 7:06 PM, Kristian Høgsberg wrote: On Mon, Oct 5, 2015 at 7:03 AM, Michel Thierry wrote: > > > > On 9/14/2015 2:54 PM, MichaÅ Winiarski wrote: >> >> >> >> >> On Thu, Sep 03, 2015 at 03:23:58PM +0100, Michel Thierry wrote: >>> >>> >>> >>> >>> Gen8+ supports 48-bit virtual addresses, but some objects must >>> always be >>> allocated inside the 32-bit address range. >>> >>> In specific, any resource used with flat/heapless >>> (0x-0xf000) >>> General State Heap (GSH) or Instruction State Heap (ISH) must be >>> in >>> a >>> 32-bit range, because the General State Offset and Instruction >>> State >>> Offset >>> are limited to 32-bits. >>> >>> The i915 driver has been modified to provide a flag to set when >>> the >>> 4GB >>> limit is not necessary in a given bo >>> (EXEC_OBJECT_SUPPORTS_48B_ADDRESS). >>> 48-bit range will only be used when explicitly requested. >>> >>> Callers to the existing drm_intel_bo_emit_reloc function should >>> set >>> the >>> use_48b_address_range flag beforehand, in order to use full ppgtt >>> range. >>> >>> v2: Make set/clear functions nops on pre-gen8 platforms, and use >>> them >>> internally in emit_reloc functions (Ben) >>> s/48BADDRESS/48B_ADDRESS/ (Dave) >>> v3: Keep set/clear functions internal, no-one needs to use them >>> directly. >>> v4: Don't set 48bit-support flag in emit reloc, check for ppgtt >>> type >>> before enabling set/clear function, print full offsets in >>> debug >>> statements, using port of lower_32_bits and upper_32_bits >>> from linux >>> kernel (MichaÅ) >>> >>> References: >>> >>> >>> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html >>> Cc: Ben Widawsky >>> Cc: MichaÅ Winiarski >> >> >> >> >> >> +Kristian >> >> LGTM. >> Acked-by: MichaÅ Winiarski >> >>> Signed-off-by: Michel Thierry > > > > > > > Hi Kristian, > > More comments on this? > I've resent the mesa patch with the last changes > > > > (http://lists.freedesktop.org/archives/dri-devel/2015-October/091752.html). > > > MichaÅ has agreed with the interface and will use it for his work. > If mesa doesn't plan to use this for now, it's ok. The kernel > changes > have > been done to prevent any regressions when the support 48-bit flag > is > not > set. I didn't get any replies to my last comments on this: http://lists.freedesktop.org/archives/mesa-dev/2015-August/091398.html Basically, I tried explicitly placing buffers above 8G and didn't see the HW problem described (ie it all worked fine). I still think there's some confusion as to what the W/A is about. Here's my take: the W/A is a SW-only workaround to handle the cases where a driver uses heapless and 48-bit ppgtt. The problem is that the heaps are limited to 4G but with 48bit ppgtt a buffer can be placed anywhere it the 48 bit address space. If that happens it's consideredd out-of-bounds for the heap and access fails. To prevent this we need to make sure the bos we address in a heapless fashion are placed below 4g. For mesa, we only configure general state base as heap-less, which affects scratch bos. What this boils down to is that we need the 4G restriction only for the scratch bos set up on 3DSTATE_VS, 3DSTATE_GS and 3DSTATE_PS (and tesselation stage eventually). Look for the OUT_RELOC64 for stage->scratch_bo in gen8_vs_state.c, gen8_gs_state.c and gen8_ps_state.c >>> >>> >>> >>> >>> I
[Mesa-dev] [PATCH v4 1/2] intel: 48b ppgtt support (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag)
On 10/14/2015 8:19 AM, Daniel Vetter wrote: > On Tue, Oct 13, 2015 at 02:51:36PM -0700, Kristian Høgsberg wrote: >> On Tue, Oct 13, 2015 at 7:55 AM, Michel Thierry >> wrote: >>> On 10/13/2015 3:13 PM, Emil Velikov wrote: On 13 October 2015 at 13:16, Michel Thierry wrote: > > On 10/6/2015 2:12 PM, Michel Thierry wrote: >> >> >> On 10/5/2015 7:06 PM, Kristian Høgsberg wrote: >>> >>> >>> On Mon, Oct 5, 2015 at 7:03 AM, Michel Thierry >>> wrote: On 9/14/2015 2:54 PM, MichaÅ Winiarski wrote: > > > > On Thu, Sep 03, 2015 at 03:23:58PM +0100, Michel Thierry wrote: >> >> >> >> Gen8+ supports 48-bit virtual addresses, but some objects must >> always be >> allocated inside the 32-bit address range. >> >> In specific, any resource used with flat/heapless >> (0x-0xf000) >> General State Heap (GSH) or Instruction State Heap (ISH) must be in >> a >> 32-bit range, because the General State Offset and Instruction State >> Offset >> are limited to 32-bits. >> >> The i915 driver has been modified to provide a flag to set when the >> 4GB >> limit is not necessary in a given bo >> (EXEC_OBJECT_SUPPORTS_48B_ADDRESS). >> 48-bit range will only be used when explicitly requested. >> >> Callers to the existing drm_intel_bo_emit_reloc function should set >> the >> use_48b_address_range flag beforehand, in order to use full ppgtt >> range. >> >> v2: Make set/clear functions nops on pre-gen8 platforms, and use >> them >> internally in emit_reloc functions (Ben) >> s/48BADDRESS/48B_ADDRESS/ (Dave) >> v3: Keep set/clear functions internal, no-one needs to use them >> directly. >> v4: Don't set 48bit-support flag in emit reloc, check for ppgtt type >> before enabling set/clear function, print full offsets in >> debug >> statements, using port of lower_32_bits and upper_32_bits >> from linux >> kernel (MichaÅ) >> >> References: >> >> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html >> Cc: Ben Widawsky >> Cc: MichaÅ Winiarski > > > > > +Kristian > > LGTM. > Acked-by: MichaÅ Winiarski > >> Signed-off-by: Michel Thierry Hi Kristian, More comments on this? I've resent the mesa patch with the last changes (http://lists.freedesktop.org/archives/dri-devel/2015-October/091752.html). MichaÅ has agreed with the interface and will use it for his work. If mesa doesn't plan to use this for now, it's ok. The kernel changes have been done to prevent any regressions when the support 48-bit flag is not set. >>> >>> >>> >>> I didn't get any replies to my last comments on this: >>> >>> http://lists.freedesktop.org/archives/mesa-dev/2015-August/091398.html >>> >>> Basically, I tried explicitly placing buffers above 8G and didn't see >>> the HW problem described (ie it all worked fine). I still think >>> there's some confusion as to what the W/A is about. >>> >>> Here's my take: the W/A is a SW-only workaround to handle the cases >>> where a driver uses heapless and 48-bit ppgtt. The problem is that the >>> heaps are limited to 4G but with 48bit ppgtt a buffer can be placed >>> anywhere it the 48 bit address space. If that happens it's consideredd >>> out-of-bounds for the heap and access fails. To prevent this we need >>> to make sure the bos we address in a heapless fashion are placed below >>> 4g. >>> >>> For mesa, we only configure general state base as heap-less, which >>> affects scratch bos. What this boils down to is that we need the 4G >>> restriction only for the scratch bos set up on 3DSTATE_VS, 3DSTATE_GS >>> and 3DSTATE_PS (and tesselation stage eventually). Look for the >>> OUT_RELOC64 for stage->scratch_bo in gen8_vs_state.c, gen8_gs_state.c >>> and gen8_ps_state.c >> >> >> >> I think it also affects _3DSTATE_VIEWPORT_STATE_POINTERS_CC, maybe it >> isn't exclusive to the scratch bos (the error state points to that >> instruction). >> >> > > Hi, > > Anymore inputs about this patch? > AFAIK, if changes are required based on further comments from Kristian, > these will be in the mesa patch[1], not libdrm. Also, MichaÅ will use > this > flag in another project. > The comment seems
[Mesa-dev] [PATCH v4 1/2] intel: 48b ppgtt support (EXEC_OBJECT_SUPPORTS_48B_ADDRESS flag)
On Tue, Oct 13, 2015 at 02:51:36PM -0700, Kristian Høgsberg wrote: > On Tue, Oct 13, 2015 at 7:55 AM, Michel Thierry > wrote: > > On 10/13/2015 3:13 PM, Emil Velikov wrote: > >> > >> On 13 October 2015 at 13:16, Michel Thierry > >> wrote: > >>> > >>> On 10/6/2015 2:12 PM, Michel Thierry wrote: > > > On 10/5/2015 7:06 PM, Kristian Høgsberg wrote: > > > > > > On Mon, Oct 5, 2015 at 7:03 AM, Michel Thierry > > wrote: > >> > >> > >> On 9/14/2015 2:54 PM, MichaÅ Winiarski wrote: > >>> > >>> > >>> > >>> On Thu, Sep 03, 2015 at 03:23:58PM +0100, Michel Thierry wrote: > > > > Gen8+ supports 48-bit virtual addresses, but some objects must > always be > allocated inside the 32-bit address range. > > In specific, any resource used with flat/heapless > (0x-0xf000) > General State Heap (GSH) or Instruction State Heap (ISH) must be in > a > 32-bit range, because the General State Offset and Instruction State > Offset > are limited to 32-bits. > > The i915 driver has been modified to provide a flag to set when the > 4GB > limit is not necessary in a given bo > (EXEC_OBJECT_SUPPORTS_48B_ADDRESS). > 48-bit range will only be used when explicitly requested. > > Callers to the existing drm_intel_bo_emit_reloc function should set > the > use_48b_address_range flag beforehand, in order to use full ppgtt > range. > > v2: Make set/clear functions nops on pre-gen8 platforms, and use > them > internally in emit_reloc functions (Ben) > s/48BADDRESS/48B_ADDRESS/ (Dave) > v3: Keep set/clear functions internal, no-one needs to use them > directly. > v4: Don't set 48bit-support flag in emit reloc, check for ppgtt type > before enabling set/clear function, print full offsets in > debug > statements, using port of lower_32_bits and upper_32_bits > from linux > kernel (MichaÅ) > > References: > > http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html > Cc: Ben Widawsky > Cc: MichaÅ Winiarski > >>> > >>> > >>> > >>> > >>> +Kristian > >>> > >>> LGTM. > >>> Acked-by: MichaÅ Winiarski > >>> > Signed-off-by: Michel Thierry > >> > >> > >> > >> > >> > >> Hi Kristian, > >> > >> More comments on this? > >> I've resent the mesa patch with the last changes > >> > >> > >> (http://lists.freedesktop.org/archives/dri-devel/2015-October/091752.html). > >> > >> > >> MichaÅ has agreed with the interface and will use it for his work. > >> If mesa doesn't plan to use this for now, it's ok. The kernel changes > >> have > >> been done to prevent any regressions when the support 48-bit flag is > >> not > >> set. > > > > > > > > I didn't get any replies to my last comments on this: > > > > http://lists.freedesktop.org/archives/mesa-dev/2015-August/091398.html > > > > Basically, I tried explicitly placing buffers above 8G and didn't see > > the HW problem described (ie it all worked fine). I still think > > there's some confusion as to what the W/A is about. > > > > Here's my take: the W/A is a SW-only workaround to handle the cases > > where a driver uses heapless and 48-bit ppgtt. The problem is that the > > heaps are limited to 4G but with 48bit ppgtt a buffer can be placed > > anywhere it the 48 bit address space. If that happens it's consideredd > > out-of-bounds for the heap and access fails. To prevent this we need > > to make sure the bos we address in a heapless fashion are placed below > > 4g. > > > > For mesa, we only configure general state base as heap-less, which > > affects scratch bos. What this boils down to is that we need the 4G > > restriction only for the scratch bos set up on 3DSTATE_VS, 3DSTATE_GS > > and 3DSTATE_PS (and tesselation stage eventually). Look for the > > OUT_RELOC64 for stage->scratch_bo in gen8_vs_state.c, gen8_gs_state.c > > and gen8_ps_state.c > > > > I think it also affects _3DSTATE_VIEWPORT_STATE_POINTERS_CC, maybe it > isn't exclusive to the scratch bos (the error state points to that > instruction). > > > >>> > >>> Hi, > >>> > >>> Anymore inputs about this patch? > >>> AFAIK, if changes are required based on further comments from Kristian, > >>> these will be in the mesa patch[1], not libdrm. Also, MichaÅ will use > >>> this > >>> flag in another project. > >>> > >> The comment seems quite brief and I'm not sure it fully addresses > >>
