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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 14/09/2018 11:52, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-09-14 11:03:18)

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@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
    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? :)

Bah, did you see the EXPORT_SYMBOL_GPL(__i915_gem_object_set_pages),
nothing is safe!
Would makes it only one place the following patch needed to switch to
MAP_FORCE_WB.

But that was the point of having a separate step here. This is then the
only mapping of default_state that needs to do the force. I liked that.
The other one we need is the initialisation of the lrc state.

Hmm true.. makes some sense as much as I like avoiding code duplication. Okay..

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Regards,

Tvrtko

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux