Re: [PATCH] drm/i915: Use true PPGTT in Gen8 if execlists are enabled

[Michel Thierry <michel.thierry@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2653 bytes --]

On 12/15/2014 2:28 PM, Daniel Vetter wrote:
> On Mon, Dec 15, 2014 at 03:22:08PM +0100, Daniel Vetter wrote:
>> On Mon, Dec 15, 2014 at 12:47:14PM +0000, Michel Thierry wrote:
>>> On 12/15/2014 10:11 AM, Daniel Vetter wrote:
>>>> On Thu, Dec 11, 2014 at 12:07:18PM +0000, Michel Thierry wrote:
>>>>> When execlists submission is enabled, try full ppgtt by default.
>>>>>
>>>>> Note, this patch considers that execlist support has been enabled by
>>>>> default on Gen8.
>>>>>
>>>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_gem_gtt.c | 9 ++++++---
>>>>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> index 171f6ea..4ed3904 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> @@ -40,8 +40,8 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
>>>>>   	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
>>>>>   	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
>>>>> -	if (IS_GEN8(dev))
>>>>> -		has_full_ppgtt = false; /* XXX why? */
>>>>> +	if (IS_GEN8(dev) && !i915.enable_execlists)
>>>>> +		has_full_ppgtt = false; /* Only enforce with execlists */
>>>> Imo this has outlived it's usefulness - enable_ppgtt is an unsafe
>>>> parameter so everyone setting it themselves gets what they need.
>>>> Afair this was just because of the execlist depency on gen8 for ppgtt.
>>>> -Daniel
>>> Not sure if I'm following you up on this...
>>> The aim was to change the default value to _full_ only when execlists are
>>> also enable (after Thomas' patch).  In gen8, we don't want to have full
>>> ppgtt with legacy ring submission.
>> Yeah I gotten confused. The comment is a bit misleading thought, maybe
>> "Full ppgtt needs execlist since otherwise the ctx switch can hang"?
> Actually I wasnt' confused: gen8+ has full ppgtt, like gen7. It's broken
> without execlist though, so what we should do is just change the default
> below. Otherwise if a user explicitly sets execlist=0 and ppgtt=2 we won't
> obey that, which doesn't make sense - for isolating bugs this might be a
> valid testcase (even when we know there's problems). So your patch should
> do two things:
> - Generally allow full ppgtt on gen8 - the current restriction doesn't
>    make sense.
> - Switch the default to full ppgtt when execlist is enabled on gen8+, leave
>    the default to aliasing/disabled everywhere else as is.
> -Daniel
Thanks for clarifying. I'll send a new patch.

-Michel


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5510 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx