Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Scott D Phillips]

Kenneth Graunke  writes:

> This isn't strictly necessary, but anyone running Cannonlake will
> already have Kernel 4.5 or later, so there's no reason to support
> the relocation model on Gen10+.
>
> This will let us avoid dealing with them for new features.

I think the discussion about aliasing ppgtt won't impact this bit of
code where we've already checked for gtt_size > 4 GiB. Maybe we should
warn about aliasing ppgtt on newer gens? Either way,

Reviewed-by: Scott D Phillips 

> ---
>  src/mesa/drivers/dri/i965/brw_bufmgr.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c 
> b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> index 4fd95e1d78c..9a059f38aaa 100644
> --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> @@ -1738,6 +1738,10 @@ brw_bufmgr_init(struct gen_device_info *devinfo, int 
> fd)
>  4096, _4GB);
>   util_vma_heap_init(>vma_allocator[BRW_MEMZONE_OTHER],
>  1 * _4GB, gtt_size - 1 * _4GB);
> +  } else if (devinfo->gen >= 10) {
> + fprintf(stderr, "i965 requires softpin (Kernel 4.5) on Gen10+.");
> + free(bufmgr);
> + return NULL;
>}
> }
>  
> -- 
> 2.17.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Kenneth Graunke]

On Friday, May 4, 2018 2:10:19 AM PDT Daniel Vetter wrote:
> On Fri, May 04, 2018 at 11:07:45AM +0200, Daniel Vetter wrote:
> > On Fri, May 04, 2018 at 01:28:02AM -0700, Kenneth Graunke wrote:
> > > On Friday, May 4, 2018 1:16:29 AM PDT Kenneth Graunke wrote:
> > > > On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > > > > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > > > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > > > > This isn't strictly necessary, but anyone running Cannonlake 
> > > > > > > > will
> > > > > > > > already have Kernel 4.5 or later, so there's no reason to 
> > > > > > > > support
> > > > > > > > the relocation model on Gen10+.
> > > > > > > 
> > > > > > > /o\ gvt. Need I say more?
> > > > > > > -Chris
> > > > > > 
> > > > > > Yes.  What's the deal with GVT?
> > > > > 
> > > > > Their current restrictions involve forcing the use of a 32b
> > > > > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > > > > to lift that restriction in time.
> > > > > -Chris
> > > > 
> > > > Wow, that's really miserable.  So, we can't actually depend on real
> > > > PPGTT existing?  Do you know if/when they might fix this?
> > > > 
> > > > This seriously wrecks a lot of my plans if we can't assume PPGTT
> > > > and have to deal with relocations for all of eternity.  Jason and I
> > > > have been planning on doing PPGTT-only drivers for months.  We figured
> > > > that full PPGTT had been working since Gen8 and surely would be viable
> > > > on anything modern.  If it weren't for enterprise kernels, I would have
> > > > required this all the way back to Gen8 in a heartbeat.
> > > > 
> > > > --Ken
> > > 
> > > Okay, whew, it looks like we were wrong.  There's a bit of confusing
> > > code in the kernel:
> > > 
> > > if (intel_vgpu_active(dev_priv)) {
> > > /* GVT-g has no support for 32bit ppgtt */
> > > has_full_ppgtt = false;
> > > has_full_48bit_ppgtt = 
> > > intel_vgpu_has_full_48bit_ppgtt(dev_priv);
> > > }
> > > 
> > > But Joonas explained that this means GVT-g does support full PPGTT
> > > with 48-bit addresses, it just never did the 32-bit only thing.
> > > 
> > > So, I think we're fine here after all.
> > 
> > Oh indeed. But only fixed in 4.14 by:
> > 
> > commit 6b3816d69628becb7ff35978aa0751798b4a940a
> > Author: Tina Zhang 
> > Date:   Mon Aug 14 15:24:14 2017 +0800
> > 
> > drm/i915/gvt: Fix guest i915 full ppgtt blocking issue
> > 
> > Maybe note that somewhere in your huge table of "stuff the kernel
> > supports" ...
> 
> Also, for you patch here this means you're fine - since cnl support isn't
> merged yet, but will have full 48bit ppgtt support (since it's there
> already) I think it's safe to require it.
> -Daniel

Yeah, I agree.  I've added a note above this condition:

+ /* Softpin landed in 4.5, but GVT used an aliasing PPGTT until
+  * kernel commit 6b3816d69628becb7ff35978aa0751798b4a940a in
+  * 4.14.  Gen10+ GVT hasn't landed yet, so it's not actually a
+  * problem - but extending this requirement back to earlier gens
+  * might actually mean requiring 4.14.
+  */

That way, if we ever are thinking about bumping kernel requirements to
4.5 and requiring softpin support on Gen8+, we'll remember that GVT
actually needs 4.14, and think twice :)


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Daniel Vetter]

On Fri, May 04, 2018 at 11:07:45AM +0200, Daniel Vetter wrote:
> On Fri, May 04, 2018 at 01:28:02AM -0700, Kenneth Graunke wrote:
> > On Friday, May 4, 2018 1:16:29 AM PDT Kenneth Graunke wrote:
> > > On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > > > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > > > > the relocation model on Gen10+.
> > > > > > 
> > > > > > /o\ gvt. Need I say more?
> > > > > > -Chris
> > > > > 
> > > > > Yes.  What's the deal with GVT?
> > > > 
> > > > Their current restrictions involve forcing the use of a 32b
> > > > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > > > to lift that restriction in time.
> > > > -Chris
> > > 
> > > Wow, that's really miserable.  So, we can't actually depend on real
> > > PPGTT existing?  Do you know if/when they might fix this?
> > > 
> > > This seriously wrecks a lot of my plans if we can't assume PPGTT
> > > and have to deal with relocations for all of eternity.  Jason and I
> > > have been planning on doing PPGTT-only drivers for months.  We figured
> > > that full PPGTT had been working since Gen8 and surely would be viable
> > > on anything modern.  If it weren't for enterprise kernels, I would have
> > > required this all the way back to Gen8 in a heartbeat.
> > > 
> > > --Ken
> > 
> > Okay, whew, it looks like we were wrong.  There's a bit of confusing
> > code in the kernel:
> > 
> > if (intel_vgpu_active(dev_priv)) {
> > /* GVT-g has no support for 32bit ppgtt */
> > has_full_ppgtt = false;
> > has_full_48bit_ppgtt = 
> > intel_vgpu_has_full_48bit_ppgtt(dev_priv);
> > }
> > 
> > But Joonas explained that this means GVT-g does support full PPGTT
> > with 48-bit addresses, it just never did the 32-bit only thing.
> > 
> > So, I think we're fine here after all.
> 
> Oh indeed. But only fixed in 4.14 by:
> 
> commit 6b3816d69628becb7ff35978aa0751798b4a940a
> Author: Tina Zhang 
> Date:   Mon Aug 14 15:24:14 2017 +0800
> 
> drm/i915/gvt: Fix guest i915 full ppgtt blocking issue
> 
> Maybe note that somewhere in your huge table of "stuff the kernel
> supports" ...

Also, for you patch here this means you're fine - since cnl support isn't
merged yet, but will have full 48bit ppgtt support (since it's there
already) I think it's safe to require it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Daniel Vetter]

On Fri, May 04, 2018 at 01:28:02AM -0700, Kenneth Graunke wrote:
> On Friday, May 4, 2018 1:16:29 AM PDT Kenneth Graunke wrote:
> > On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > > > the relocation model on Gen10+.
> > > > > 
> > > > > /o\ gvt. Need I say more?
> > > > > -Chris
> > > > 
> > > > Yes.  What's the deal with GVT?
> > > 
> > > Their current restrictions involve forcing the use of a 32b
> > > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > > to lift that restriction in time.
> > > -Chris
> > 
> > Wow, that's really miserable.  So, we can't actually depend on real
> > PPGTT existing?  Do you know if/when they might fix this?
> > 
> > This seriously wrecks a lot of my plans if we can't assume PPGTT
> > and have to deal with relocations for all of eternity.  Jason and I
> > have been planning on doing PPGTT-only drivers for months.  We figured
> > that full PPGTT had been working since Gen8 and surely would be viable
> > on anything modern.  If it weren't for enterprise kernels, I would have
> > required this all the way back to Gen8 in a heartbeat.
> > 
> > --Ken
> 
> Okay, whew, it looks like we were wrong.  There's a bit of confusing
> code in the kernel:
> 
> if (intel_vgpu_active(dev_priv)) {
> /* GVT-g has no support for 32bit ppgtt */
> has_full_ppgtt = false;
> has_full_48bit_ppgtt = 
> intel_vgpu_has_full_48bit_ppgtt(dev_priv);
> }
> 
> But Joonas explained that this means GVT-g does support full PPGTT
> with 48-bit addresses, it just never did the 32-bit only thing.
> 
> So, I think we're fine here after all.

Oh indeed. But only fixed in 4.14 by:

commit 6b3816d69628becb7ff35978aa0751798b4a940a
Author: Tina Zhang 
Date:   Mon Aug 14 15:24:14 2017 +0800

drm/i915/gvt: Fix guest i915 full ppgtt blocking issue

Maybe note that somewhere in your huge table of "stuff the kernel
supports" ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Chris Wilson]

Quoting Daniel Vetter (2018-05-04 09:45:15)
> On Fri, May 04, 2018 at 01:16:29AM -0700, Kenneth Graunke wrote:
> > On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > > > the relocation model on Gen10+.
> > > > > 
> > > > > /o\ gvt. Need I say more?
> > > > > -Chris
> > > > 
> > > > Yes.  What's the deal with GVT?
> > > 
> > > Their current restrictions involve forcing the use of a 32b
> > > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > > to lift that restriction in time.
> > > -Chris
> > 
> > Wow, that's really miserable.  So, we can't actually depend on real
> > PPGTT existing?  Do you know if/when they might fix this?
> > 
> > This seriously wrecks a lot of my plans if we can't assume PPGTT
> > and have to deal with relocations for all of eternity.  Jason and I
> > have been planning on doing PPGTT-only drivers for months.  We figured
> > that full PPGTT had been working since Gen8 and surely would be viable
> > on anything modern.  If it weren't for enterprise kernels, I would have
> > required this all the way back to Gen8 in a heartbeat.
> 
> softpin with aliasing ppgtt should still work, just thrash really badly.
> It only stops working when there's something pinned at a given offset.

Stops working = -ENOSPC returned to userspace (-EINVAL if it was
soft-pinned by the same batch). Having the error there is the batch
conflicts with a scanout, makes aliasing-ppgtt unusable for userspace if
you haven't been tracking the relocations for this batch, and so can't
redo those post-hoc as you would then likely have to drop the batch on
the floor.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Daniel Vetter]

On Fri, May 04, 2018 at 01:16:29AM -0700, Kenneth Graunke wrote:
> On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > > the relocation model on Gen10+.
> > > > 
> > > > /o\ gvt. Need I say more?
> > > > -Chris
> > > 
> > > Yes.  What's the deal with GVT?
> > 
> > Their current restrictions involve forcing the use of a 32b
> > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > to lift that restriction in time.
> > -Chris
> 
> Wow, that's really miserable.  So, we can't actually depend on real
> PPGTT existing?  Do you know if/when they might fix this?
> 
> This seriously wrecks a lot of my plans if we can't assume PPGTT
> and have to deal with relocations for all of eternity.  Jason and I
> have been planning on doing PPGTT-only drivers for months.  We figured
> that full PPGTT had been working since Gen8 and surely would be viable
> on anything modern.  If it weren't for enterprise kernels, I would have
> required this all the way back to Gen8 in a heartbeat.

softpin with aliasing ppgtt should still work, just thrash really badly.
It only stops working when there's something pinned at a given offset. It
only fails if there's something pinned there. A slightly evil plan would
be to try that, given the gvt some good motivation to implement ppgtt :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Kenneth Graunke]

On Friday, May 4, 2018 1:16:29 AM PDT Kenneth Graunke wrote:
> On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> > Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > > the relocation model on Gen10+.
> > > > 
> > > > /o\ gvt. Need I say more?
> > > > -Chris
> > > 
> > > Yes.  What's the deal with GVT?
> > 
> > Their current restrictions involve forcing the use of a 32b
> > aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> > to lift that restriction in time.
> > -Chris
> 
> Wow, that's really miserable.  So, we can't actually depend on real
> PPGTT existing?  Do you know if/when they might fix this?
> 
> This seriously wrecks a lot of my plans if we can't assume PPGTT
> and have to deal with relocations for all of eternity.  Jason and I
> have been planning on doing PPGTT-only drivers for months.  We figured
> that full PPGTT had been working since Gen8 and surely would be viable
> on anything modern.  If it weren't for enterprise kernels, I would have
> required this all the way back to Gen8 in a heartbeat.
> 
> --Ken

Okay, whew, it looks like we were wrong.  There's a bit of confusing
code in the kernel:

if (intel_vgpu_active(dev_priv)) {
/* GVT-g has no support for 32bit ppgtt */
has_full_ppgtt = false;
has_full_48bit_ppgtt = 
intel_vgpu_has_full_48bit_ppgtt(dev_priv);
}

But Joonas explained that this means GVT-g does support full PPGTT
with 48-bit addresses, it just never did the 32-bit only thing.

So, I think we're fine here after all.


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Kenneth Graunke]

On Friday, May 4, 2018 12:39:12 AM PDT Chris Wilson wrote:
> Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > the relocation model on Gen10+.
> > > 
> > > /o\ gvt. Need I say more?
> > > -Chris
> > 
> > Yes.  What's the deal with GVT?
> 
> Their current restrictions involve forcing the use of a 32b
> aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> to lift that restriction in time.
> -Chris

Wow, that's really miserable.  So, we can't actually depend on real
PPGTT existing?  Do you know if/when they might fix this?

This seriously wrecks a lot of my plans if we can't assume PPGTT
and have to deal with relocations for all of eternity.  Jason and I
have been planning on doing PPGTT-only drivers for months.  We figured
that full PPGTT had been working since Gen8 and surely would be viable
on anything modern.  If it weren't for enterprise kernels, I would have
required this all the way back to Gen8 in a heartbeat.

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Jason Ekstrand]

On Fri, May 4, 2018 at 12:39 AM, Chris Wilson 
wrote:

> Quoting Kenneth Graunke (2018-05-04 08:34:07)
> > On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > > This isn't strictly necessary, but anyone running Cannonlake will
> > > > already have Kernel 4.5 or later, so there's no reason to support
> > > > the relocation model on Gen10+.
> > >
> > > /o\ gvt. Need I say more?
> > > -Chris
> >
> > Yes.  What's the deal with GVT?
>
> Their current restrictions involve forcing the use of a 32b
> aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
> to lift that restriction in time.
>

Oh my... Why can't we have nice things? :-(
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Chris Wilson]

Quoting Kenneth Graunke (2018-05-04 08:34:07)
> On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> > Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > > This isn't strictly necessary, but anyone running Cannonlake will
> > > already have Kernel 4.5 or later, so there's no reason to support
> > > the relocation model on Gen10+.
> > 
> > /o\ gvt. Need I say more?
> > -Chris
> 
> Yes.  What's the deal with GVT?

Their current restrictions involve forcing the use of a 32b
aliasing-ppgtt. Not that they support cnl+ yet, so they might remember
to lift that restriction in time.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Kenneth Graunke]

On Thursday, May 3, 2018 11:53:24 PM PDT Chris Wilson wrote:
> Quoting Kenneth Graunke (2018-05-04 02:12:40)
> > This isn't strictly necessary, but anyone running Cannonlake will
> > already have Kernel 4.5 or later, so there's no reason to support
> > the relocation model on Gen10+.
> 
> /o\ gvt. Need I say more?
> -Chris

Yes.  What's the deal with GVT?


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/10] i965: Require softpin support for Cannonlake and later.

[Chris Wilson]

Quoting Kenneth Graunke (2018-05-04 02:12:40)
> This isn't strictly necessary, but anyone running Cannonlake will
> already have Kernel 4.5 or later, so there's no reason to support
> the relocation model on Gen10+.

/o\ gvt. Need I say more?
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev