On 03/23/2017 03:57 PM, Chris Wilson wrote:
On Wed, Mar 22, 2017 at 10:39:46AM -0700, Oscar Mateo wrote:
Starting with intel_guc_loader, down to intel_guc_submission
and finally to intel_guc_log.
v2:
- Null execbuf client outside guc_client_free (Daniele)
- Assert if things try to get allocated twice (Daniele/Joonas)
- Null guc->log.buf_addr when destroyed (Daniele)
- Newline between returning success and error labels (Joonas)
- Remove some unnecessary comments (Joonas)
- Keep guc_log_create_extras naming convention (Joonas)
- Helper function guc_log_has_extras (Joonas)
- No need for separate relay_channel create/destroy. It's just another extra.
- No need to nullify guc->log.flush_wq when destroyed (Joonas)
- Hoist the check for has_extras out of guc_log_create_extras (Joonas)
- Try to do i915_guc_log_register/unregister calls (kind of) symmetric (Daniele)
- Make sure initel_guc_fini is not called before init is ever called (Daniele)
v3:
- Remove unnecessary parenthesis (Joonas)
- Check for logs enabled on debugfs registration
- Rebase on top of Tvrtko's "Fix request re-submission after reset"
v4:
- Rebased
- Comment around enabling/disabling interrupts inside GuC logging (Joonas)
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 11 +-
drivers/gpu/drm/i915/i915_gem.c | 10 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 97 ++++----
drivers/gpu/drm/i915/intel_guc_loader.c | 21 --
drivers/gpu/drm/i915/intel_guc_log.c | 364 ++++++++++++++---------------
drivers/gpu/drm/i915/intel_uc.c | 55 +++--
drivers/gpu/drm/i915/intel_uc.h | 8 +-
7 files changed, 297 insertions(+), 269 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 03d9e45..6d9944a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -549,6 +549,8 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
static void i915_gem_fini(struct drm_i915_private *dev_priv)
{
mutex_lock(&dev_priv->drm.struct_mutex);
+ if (i915.enable_guc_loading)
+ intel_uc_fini_hw(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fd611b4..4eb46e4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4596,10 +4596,12 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
intel_mocs_init_l3cc_table(dev_priv);
- /* We can't enable contexts until all firmware is loaded */
- ret = intel_uc_init_hw(dev_priv);
- if (ret)
- goto out;
+ if (i915.enable_guc_loading) {
+ /* We can't enable contexts until all firmware is loaded */
+ ret = intel_uc_init_hw(dev_priv);
+ if (ret)
+ goto out;
+ }
out:
I'm not happy with moving subfeature detection logic into the core GEM
code. if (i915.enable_guc_loading) firstly should never be a module
parameter (it's derived state!) and secondly it should reside next to
the dependent logic and not be interrupting the central control flow.
-Chris
What do you mean it's derived state? from what?
As for interrupting the central control flow, you are probably right, I
can change it back (the code was refactored so many times that I cannot
remember my logic behind that change anymore)
- Oscar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
