[Intel-gfx] [PATCH] drm/i915: Check engine->default_state mapping on module load

[Tvrtko Ursulin]

On 14/09/2018 10:51, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-09-14 10:43:12)
>>
>> On 14/09/2018 10:21, Chris Wilson wrote:
>>> Check we can indeed acquire a WB mapping of the context image on module
>>> load. Later this will give us the opportunity to validate that we can
>>> switch from WC to WB as required.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
>>>    1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index be9d012d851b..37353afec66e 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -5424,6 +5424,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>>>    
>>>        for_each_engine(engine, i915, id) {
>>>                struct i915_vma *state;
>>> +             void *vaddr;
>>>    
>>>                state = to_intel_context(ctx, engine)->state;
>>>                if (!state)
>>> @@ -5446,6 +5447,16 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>>>                        goto err_active;
>>>    
>>>                engine->default_state = i915_gem_object_get(state->obj);
>>> +
>>> +             /* Check we can acquire the image of the context state */
>>> +             vaddr = i915_gem_object_pin_map(engine->default_state,
>>> +                                             I915_MAP_WB);
>>> +             if (IS_ERR(vaddr)) {
>>> +                     err = PTR_ERR(vaddr);
>>> +                     goto err_active;
>>> +             }
>>> +
>>> +             i915_gem_object_unpin_map(engine->default_state);
>>
>> Ah this perhaps strengthens the argument to have
>> __intel_engines_pin_default_state? (And the unpin pair.)
> 
> I hope it doesn't come to making this into an interface :)
> Definitely not an idea I endorse, random manipulation of the
> default_state.

Double underscore alleviates those concerns, no? :)

Would makes it only one place the following patch needed to switch to 
MAP_FORCE_WB.

Regards,

Tvrtko

>> And I guess you made this patch come before the switch to WC since it is
>> not FORCE_WB here?
> 
> Yes, full sequence of patches in your inbox.
> -Chris
>