Re: [PATCH 1/3] drm/i915: Make i915_gem_load_power_context take struct intel_gt

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 16/10/2019 13:42, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-10-16 13:25:55)
> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >
> > The code underneath works on intel_gt and also rename to
> > i915_gem_load_gt_power_context to signify it is operating on GT.
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_pm.c | 24 +++++++++++++-----------
> >   drivers/gpu/drm/i915/gem/i915_gem_pm.h |  3 ++-
> >   drivers/gpu/drm/i915/i915_gem.c        |  2 +-
> >   3 files changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > index 7987b54fb1f5..b2c18674c455 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > @@ -56,9 +56,9 @@ static bool switch_to_kernel_context_sync(struct intel_gt *gt)
> >          return result;
> >   }
> >   
> > -bool i915_gem_load_power_context(struct drm_i915_private *i915)
> > +bool i915_gem_load_gt_power_context(struct intel_gt *gt)
> >   {
> > -       return switch_to_kernel_context_sync(&i915->gt);
> > +       return switch_to_kernel_context_sync(gt);
> >   }
> >   
> >   static void user_forcewake(struct intel_gt *gt, bool suspend)
> > @@ -172,11 +172,13 @@ void i915_gem_suspend_late(struct drm_i915_private *i915)
> >   
> >   void i915_gem_resume(struct drm_i915_private *i915)
> >   {
> > +       struct intel_gt *gt = &i915->gt;
> > +
> >          GEM_TRACE("\n");
> >   
> > -       intel_uncore_forcewake_get(&i915->uncore, FORCEWAKE_ALL);
> > +       intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
> >   
> > -       if (intel_gt_init_hw(&i915->gt))
> > +       if (intel_gt_init_hw(gt))
> >                  goto err_wedged;
> >   
> >          /*
> > @@ -184,26 +186,26 @@ void i915_gem_resume(struct drm_i915_private *i915)
> >           * guarantee that the context image is complete. So let's just reset
> >           * it and start again.
> >           */
> > -       if (intel_gt_resume(&i915->gt))
> > +       if (intel_gt_resume(gt))
> >                  goto err_wedged;
> >   
> > -       intel_uc_resume(&i915->gt.uc);
> > +       intel_uc_resume(&gt->uc);
> >   
> >          /* Always reload a context for powersaving. */
> > -       if (!i915_gem_load_power_context(i915))
> > +       if (!i915_gem_load_gt_power_context(gt))
> >                  goto err_wedged;
> >   
> > -       user_forcewake(&i915->gt, false);
> > +       user_forcewake(gt, false);
> >   
> >   out_unlock:
> > -       intel_uncore_forcewake_put(&i915->uncore, FORCEWAKE_ALL);
> > +       intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
> >          return;
> >   
> >   err_wedged:
> > -       if (!intel_gt_is_wedged(&i915->gt)) {
> > +       if (!intel_gt_is_wedged(gt)) {
> >                  dev_err(i915->drm.dev,
> >                          "Failed to re-initialize GPU, declaring it wedged!\n");
> > -               intel_gt_set_wedged(&i915->gt);
> > +               intel_gt_set_wedged(gt);
> >          }
> >          goto out_unlock;
> >   }
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.h b/drivers/gpu/drm/i915/gem/i915_gem_pm.h
> > index 6f7d5d11ac3b..38eace4cb6ba 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.h
> > @@ -10,11 +10,12 @@
> >   #include <linux/types.h>
> >   
> >   struct drm_i915_private;
> > +struct intel_gt;
> >   struct work_struct;
> >   
> >   void i915_gem_init__pm(struct drm_i915_private *i915);
> >   
> > -bool i915_gem_load_power_context(struct drm_i915_private *i915);
> > +bool i915_gem_load_gt_power_context(struct intel_gt *gt);
> >   void i915_gem_resume(struct drm_i915_private *i915);
> >   
> >   void i915_gem_idle_work_handler(struct work_struct *work);
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 0ddbd3a5fb8d..c4ebc21c6820 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1132,7 +1132,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
> >          }
> >   
> >          /* Flush the default context image to memory, and enable powersaving. */
> > -       if (!i915_gem_load_power_context(i915)) {
> > +       if (!i915_gem_load_gt_power_context(&i915->gt)) {
> >                  err = -EIO;
> >                  goto out;
> >          }
>
> Not quite, the plan is to move this all to the gt. Paging Andi, come in
> Andi!

Have you copied him? :) I now have.

I don't have any complaints there - but can it be done quickly?

Regards,

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