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" Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_drv.c | 7 +- drivers/gpu/drm/i915/i915_guc_submission.c | 94 ++++---- drivers/gpu/drm/i915/intel_guc_loader.c | 32 +-- drivers/gpu/drm/i915/intel_guc_log.c | 362 ++++++++++++++--------------- drivers/gpu/drm/i915/intel_uc.h | 8 +- 5 files changed, 261 insertions(+), 242 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b1e9027..3329507 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_submission) + intel_guc_cleanup(dev_priv); i915_gem_cleanup_engines(dev_priv); i915_gem_context_fini(dev_priv); mutex_unlock(&dev_priv->drm.struct_mutex); @@ -612,7 +614,7 @@ static int i915_load_modeset_init(struct drm_device *dev) ret = i915_gem_init(dev_priv); if (ret) - goto cleanup_irq; + goto cleanup_uc; intel_modeset_gem_init(dev); @@ -634,9 +636,10 @@ static int i915_load_modeset_init(struct drm_device *dev) if (i915_gem_suspend(dev_priv)) DRM_ERROR("failed to idle hardware; continuing to unload!\n"); i915_gem_fini(dev_priv); -cleanup_irq: +cleanup_uc: intel_guc_fini(dev_priv); intel_huc_fini(dev_priv); +cleanup_irq: drm_irq_uninstall(dev); intel_teardown_gmbus(dev_priv); cleanup_csr: diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 843c73a..f33c659 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -809,7 +809,6 @@ static int guc_init_doorbell_hw(struct intel_guc *guc) ida_simple_remove(&guc->ctx_ids, client->ctx_index); err_client: kfree(client); - return ERR_PTR(ret); } @@ -835,7 +834,7 @@ static void guc_policies_init(struct guc_policies *policies) policies->is_valid = 1; } -static void guc_addon_create(struct intel_guc *guc) +static int guc_addon_create(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); struct i915_vma *vma; @@ -847,19 +846,18 @@ static void guc_addon_create(struct intel_guc *guc) struct page *page; u32 size; + GEM_BUG_ON(guc->ads_vma); + /* The ads obj includes the struct itself and buffers passed to GuC */ size = sizeof(struct guc_ads) + sizeof(struct guc_policies) + sizeof(struct guc_mmio_reg_state) + GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE; - vma = guc->ads_vma; - if (!vma) { - vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(size)); - if (IS_ERR(vma)) - return; + vma = intel_guc_allocate_vma(guc, PAGE_ALIGN(size)); + if (IS_ERR(vma)) + return PTR_ERR(vma); - guc->ads_vma = vma; - } + guc->ads_vma = vma; page = i915_vma_first_page(vma); ads = kmap(page); @@ -902,6 +900,13 @@ static void guc_addon_create(struct intel_guc *guc) sizeof(struct guc_mmio_reg_state); kunmap(page); + + return 0; +} + +static void guc_addon_destroy(struct intel_guc *guc) +{ + i915_vma_unpin_and_release(&guc->ads_vma); } /* @@ -916,6 +921,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv) struct intel_guc *guc = &dev_priv->guc; struct i915_vma *vma; void *vaddr; + int ret; if (!HAS_GUC_SCHED(dev_priv)) return 0; @@ -925,10 +931,10 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv) i915_guc_submission_disable(dev_priv); if (!i915.enable_guc_submission) - return 0; /* not enabled */ + return 0; if (guc->ctx_pool) - return 0; /* already allocated */ + return 0; vma = intel_guc_allocate_vma(guc, gemsize); if (IS_ERR(vma)) @@ -936,15 +942,23 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv) guc->ctx_pool = vma; - vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB); - if (IS_ERR(vaddr)) - goto err; + vaddr = i915_gem_object_pin_map(guc->ctx_pool->obj, I915_MAP_WB); + if (IS_ERR(vaddr)) { + ret = PTR_ERR(vaddr); + goto err_vma; + } guc->ctx_pool_vaddr = vaddr; + ret = intel_guc_log_create(guc); + if (ret < 0) + goto err_vaddr; + + ret = guc_addon_create(guc); + if (ret < 0) + goto err_log; + ida_init(&guc->ctx_ids); - intel_guc_log_create(guc); - guc_addon_create(guc); guc->execbuf_client = guc_client_alloc(dev_priv, INTEL_INFO(dev_priv)->ring_mask, @@ -952,14 +966,34 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv) dev_priv->kernel_context); if (IS_ERR(guc->execbuf_client)) { DRM_ERROR("Failed to create GuC client for execbuf!\n"); - goto err; + ret = PTR_ERR(guc->execbuf_client); + goto err_ads; } return 0; -err: - i915_guc_submission_fini(dev_priv); - return -ENOMEM; +err_ads: + guc_addon_destroy(guc); +err_log: + intel_guc_log_destroy(guc); +err_vaddr: + i915_gem_object_unpin_map(guc->ctx_pool->obj); +err_vma: + i915_vma_unpin_and_release(&guc->ctx_pool); + return ret; +} + +void i915_guc_submission_fini(struct drm_i915_private *dev_priv) +{ + struct intel_guc *guc = &dev_priv->guc; + + guc_client_free(guc->execbuf_client); + guc->execbuf_client = NULL; + ida_destroy(&guc->ctx_ids); + guc_addon_destroy(guc); + intel_guc_log_destroy(guc); + i915_gem_object_unpin_map(guc->ctx_pool->obj); + i915_vma_unpin_and_release(&guc->ctx_pool); } static void guc_reset_wq(struct i915_guc_client *client) @@ -1053,26 +1087,6 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv) intel_execlists_enable_submission(dev_priv); } -void i915_guc_submission_fini(struct drm_i915_private *dev_priv) -{ - struct intel_guc *guc = &dev_priv->guc; - struct i915_guc_client *client; - - client = fetch_and_zero(&guc->execbuf_client); - if (client && !IS_ERR(client)) - guc_client_free(client); - - i915_vma_unpin_and_release(&guc->ads_vma); - i915_vma_unpin_and_release(&guc->log.vma); - - if (guc->ctx_pool_vaddr) { - ida_destroy(&guc->ctx_ids); - i915_gem_object_unpin_map(guc->ctx_pool->obj); - } - - i915_vma_unpin_and_release(&guc->ctx_pool); -} - /** * intel_guc_suspend() - notify GuC entering suspend state * @dev_priv: i915 device private diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index b1ebfce..f9e183a 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -443,7 +443,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) err = i915_guc_submission_init(dev_priv); if (err) - goto fail; + goto err_guc; /* * WaEnableuKernelHeaderValidFix:skl,bxt @@ -458,7 +458,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) */ err = guc_hw_reset(dev_priv); if (err) - goto fail; + goto err_submission; intel_huc_load(dev_priv); err = guc_ucode_xfer(dev_priv); @@ -466,7 +466,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) break; if (--retries == 0) - goto fail; + goto err_submission; DRM_INFO("GuC fw load failed: %d; will reset and " "retry %d more time(s)\n", err, retries); @@ -482,7 +482,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) err = i915_guc_submission_enable(dev_priv); if (err) - goto fail; + goto err_interrupts; } DRM_INFO("GuC %s (firmware %s [version %u.%u])\n", @@ -492,15 +492,16 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) return 0; +err_interrupts: + gen9_disable_guc_interrupts(dev_priv); +err_submission: + i915_guc_submission_fini(dev_priv); +err_guc: + i915_ggtt_disable_guc(dev_priv); fail: if (guc_fw->load_status == INTEL_UC_FIRMWARE_PENDING) guc_fw->load_status = INTEL_UC_FIRMWARE_FAIL; - guc_interrupts_release(dev_priv); - i915_guc_submission_disable(dev_priv); - i915_guc_submission_fini(dev_priv); - i915_ggtt_disable_guc(dev_priv); - /* * We've failed to load the firmware :( * @@ -540,6 +541,13 @@ int intel_guc_setup(struct drm_i915_private *dev_priv) return ret; } +void intel_guc_cleanup(struct drm_i915_private *dev_priv) +{ + gen9_disable_guc_interrupts(dev_priv); + i915_guc_submission_fini(dev_priv); + i915_ggtt_disable_guc(dev_priv); +} + void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, struct intel_uc_fw *uc_fw) { @@ -745,12 +753,6 @@ void intel_guc_fini(struct drm_i915_private *dev_priv) struct intel_uc_fw *guc_fw = &dev_priv->guc.fw; struct drm_i915_gem_object *obj; - mutex_lock(&dev_priv->drm.struct_mutex); - guc_interrupts_release(dev_priv); - i915_guc_submission_disable(dev_priv); - i915_guc_submission_fini(dev_priv); - mutex_unlock(&dev_priv->drm.struct_mutex); - obj = fetch_and_zero(&guc_fw->obj); if (obj) i915_gem_object_put(obj); diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index 5c0f9a4..3b1644b 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -66,7 +66,6 @@ static int guc_log_control(struct intel_guc *guc, u32 control_val) return intel_guc_send(guc, action, ARRAY_SIZE(action)); } - /* * Sub buffer switch callback. Called whenever relay has to switch to a new * sub buffer, relay stays on the same sub buffer if 0 is returned. @@ -139,45 +138,15 @@ static int remove_buf_file_callback(struct dentry *dentry) .remove_buf_file = remove_buf_file_callback, }; -static void guc_log_remove_relay_file(struct intel_guc *guc) -{ - relay_close(guc->log.relay_chan); -} - -static int guc_log_create_relay_channel(struct intel_guc *guc) -{ - struct drm_i915_private *dev_priv = guc_to_i915(guc); - struct rchan *guc_log_relay_chan; - size_t n_subbufs, subbuf_size; - - /* Keep the size of sub buffers same as shared log buffer */ - subbuf_size = guc->log.vma->obj->base.size; - - /* Store up to 8 snapshots, which is large enough to buffer sufficient - * boot time logs and provides enough leeway to User, in terms of - * latency, for consuming the logs from relay. Also doesn't take - * up too much memory. - */ - n_subbufs = 8; - - guc_log_relay_chan = relay_open(NULL, NULL, subbuf_size, - n_subbufs, &relay_callbacks, dev_priv); - if (!guc_log_relay_chan) { - DRM_ERROR("Couldn't create relay chan for GuC logging\n"); - return -ENOMEM; - } - - GEM_BUG_ON(guc_log_relay_chan->subbuf_size < subbuf_size); - guc->log.relay_chan = guc_log_relay_chan; - return 0; -} - -static int guc_log_create_relay_file(struct intel_guc *guc) +static int guc_log_relay_file_create(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); struct dentry *log_dir; int ret; + if (i915.guc_log_level < 0) + return 0; + /* For now create the log file in /sys/kernel/debug/dri/0 dir */ log_dir = dev_priv->drm.primary->debugfs_root; @@ -198,7 +167,7 @@ static int guc_log_create_relay_file(struct intel_guc *guc) } ret = relay_late_setup_files(guc->log.relay_chan, "guc_log", log_dir); - if (ret) { + if (ret < 0 && ret != -EEXIST) { DRM_ERROR("Couldn't associate relay chan with file %d\n", ret); return ret; } @@ -371,31 +340,6 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) } } -static void guc_log_cleanup(struct intel_guc *guc) -{ - struct drm_i915_private *dev_priv = guc_to_i915(guc); - - lockdep_assert_held(&dev_priv->drm.struct_mutex); - - /* First disable the flush interrupt */ - gen9_disable_guc_interrupts(dev_priv); - - if (guc->log.flush_wq) - destroy_workqueue(guc->log.flush_wq); - - guc->log.flush_wq = NULL; - - if (guc->log.relay_chan) - guc_log_remove_relay_file(guc); - - guc->log.relay_chan = NULL; - - if (guc->log.buf_addr) - i915_gem_object_unpin_map(guc->log.vma->obj); - - guc->log.buf_addr = NULL; -} - static void capture_logs_work(struct work_struct *work) { struct intel_guc *guc = @@ -404,120 +348,105 @@ static void capture_logs_work(struct work_struct *work) guc_log_capture_logs(guc); } +static bool guc_log_has_extras(struct intel_guc *guc) +{ + return guc->log.buf_addr != NULL; +} + static int guc_log_create_extras(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); void *vaddr; - int ret; + struct rchan *guc_log_relay_chan; + size_t n_subbufs, subbuf_size; + int ret = 0; lockdep_assert_held(&dev_priv->drm.struct_mutex); - /* Nothing to do */ - if (i915.guc_log_level < 0) - return 0; - - if (!guc->log.buf_addr) { - /* Create a WC (Uncached for read) vmalloc mapping of log - * buffer pages, so that we can directly get the data - * (up-to-date) from memory. - */ - vaddr = i915_gem_object_pin_map(guc->log.vma->obj, I915_MAP_WC); - if (IS_ERR(vaddr)) { - ret = PTR_ERR(vaddr); - DRM_ERROR("Couldn't map log buffer pages %d\n", ret); - return ret; - } - - guc->log.buf_addr = vaddr; - } + GEM_BUG_ON(guc_log_has_extras(guc)); - if (!guc->log.relay_chan) { - /* Create a relay channel, so that we have buffers for storing - * the GuC firmware logs, the channel will be linked with a file - * later on when debugfs is registered. - */ - ret = guc_log_create_relay_channel(guc); - if (ret) - return ret; + /* Create a WC (Uncached for read) vmalloc mapping of log + * buffer pages, so that we can directly get the data + * (up-to-date) from memory. + */ + vaddr = i915_gem_object_pin_map(guc->log.vma->obj, I915_MAP_WC); + if (IS_ERR(vaddr)) { + DRM_ERROR("Couldn't map log buffer pages %d\n", ret); + return PTR_ERR(vaddr); } - if (!guc->log.flush_wq) { - INIT_WORK(&guc->log.flush_work, capture_logs_work); - - /* - * GuC log buffer flush work item has to do register access to - * send the ack to GuC and this work item, if not synced before - * suspend, can potentially get executed after the GFX device is - * suspended. - * By marking the WQ as freezable, we don't have to bother about - * flushing of this work item from the suspend hooks, the pending - * work item if any will be either executed before the suspend - * or scheduled later on resume. This way the handling of work - * item can be kept same between system suspend & rpm suspend. - */ - guc->log.flush_wq = alloc_ordered_workqueue("i915-guc_log", - WQ_HIGHPRI | WQ_FREEZABLE); - if (guc->log.flush_wq == NULL) { - DRM_ERROR("Couldn't allocate the wq for GuC logging\n"); - return -ENOMEM; - } - } + guc->log.buf_addr = vaddr; - return 0; -} + /* Keep the size of sub buffers same as shared log buffer */ + subbuf_size = guc->log.vma->obj->base.size; -void intel_guc_log_create(struct intel_guc *guc) -{ - struct i915_vma *vma; - unsigned long offset; - uint32_t size, flags; + /* Store up to 8 snapshots, which is large enough to buffer sufficient + * boot time logs and provides enough leeway to User, in terms of + * latency, for consuming the logs from relay. Also doesn't take + * up too much memory. + */ + n_subbufs = 8; - if (i915.guc_log_level > GUC_LOG_VERBOSITY_MAX) - i915.guc_log_level = GUC_LOG_VERBOSITY_MAX; + /* Create a relay channel, so that we have buffers for storing + * the GuC firmware logs, the channel will be linked with a file + * later on when debugfs is registered. + */ + guc_log_relay_chan = relay_open(NULL, NULL, subbuf_size, + n_subbufs, &relay_callbacks, dev_priv); + if (!guc_log_relay_chan) { + DRM_ERROR("Couldn't create relay chan for GuC logging\n"); - /* The first page is to save log buffer state. Allocate one - * extra page for others in case for overlap */ - size = (1 + GUC_LOG_DPC_PAGES + 1 + - GUC_LOG_ISR_PAGES + 1 + - GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT; + ret = -ENOMEM; + goto err_vaddr; + } - vma = guc->log.vma; - if (!vma) { - /* We require SSE 4.1 for fast reads from the GuC log buffer and - * it should be present on the chipsets supporting GuC based - * submisssions. - */ - if (WARN_ON(!i915_has_memcpy_from_wc())) { - /* logging will not be enabled */ - i915.guc_log_level = -1; - return; - } + GEM_BUG_ON(guc_log_relay_chan->subbuf_size < subbuf_size); + guc->log.relay_chan = guc_log_relay_chan; - vma = intel_guc_allocate_vma(guc, size); - if (IS_ERR(vma)) { - /* logging will be off */ - i915.guc_log_level = -1; - return; - } + INIT_WORK(&guc->log.flush_work, capture_logs_work); + + /* + * GuC log buffer flush work item has to do register access to + * send the ack to GuC and this work item, if not synced before + * suspend, can potentially get executed after the GFX device is + * suspended. + * By marking the WQ as freezable, we don't have to bother about + * flushing of this work item from the suspend hooks, the pending + * work item if any will be either executed before the suspend + * or scheduled later on resume. This way the handling of work + * item can be kept same between system suspend & rpm suspend. + */ + guc->log.flush_wq = alloc_ordered_workqueue("i915-guc_log", + WQ_HIGHPRI | WQ_FREEZABLE); + if (!guc->log.flush_wq) { + DRM_ERROR("Couldn't allocate the wq for GuC logging\n"); + ret = -ENOMEM; + goto err_relaychan; + } - guc->log.vma = vma; + return 0; - if (guc_log_create_extras(guc)) { - guc_log_cleanup(guc); - i915_vma_unpin_and_release(&guc->log.vma); - i915.guc_log_level = -1; - return; - } - } +err_relaychan: + relay_close(guc->log.relay_chan); +err_vaddr: + i915_gem_object_unpin_map(guc->log.vma->obj); + guc->log.buf_addr = NULL; + return ret; +} - /* each allocated unit is a page */ - flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL | - (GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) | - (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | - (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); +static void guc_log_destroy_extras(struct intel_guc *guc) +{ + /* + * It's possible that extras were never allocated because guc_log_level + * was < 0 at the time + **/ + if (!guc_log_has_extras(guc)) + return; - offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ - guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; + destroy_workqueue(guc->log.flush_wq); + relay_close(guc->log.relay_chan); + i915_gem_object_unpin_map(guc->log.vma->obj); + guc->log.buf_addr = NULL; } static int guc_log_late_setup(struct intel_guc *guc) @@ -527,24 +456,25 @@ static int guc_log_late_setup(struct intel_guc *guc) lockdep_assert_held(&dev_priv->drm.struct_mutex); - if (i915.guc_log_level < 0) - return -EINVAL; + if (!guc_log_has_extras(guc)) { + /* If log_level was set as -1 at boot time, then setup needed to + * handle log buffer flush interrupts would not have been done yet, + * so do that now. + */ + ret = guc_log_create_extras(guc); + if (ret) + goto err; + } - /* If log_level was set as -1 at boot time, then setup needed to - * handle log buffer flush interrupts would not have been done yet, - * so do that now. - */ - ret = guc_log_create_extras(guc); + ret = guc_log_relay_file_create(guc); if (ret) - goto err; - - ret = guc_log_create_relay_file(guc); - if (ret) - goto err; + goto err_extras; return 0; + +err_extras: + guc_log_destroy_extras(guc); err: - guc_log_cleanup(guc); /* logging will remain off */ i915.guc_log_level = -1; return ret; @@ -586,6 +516,72 @@ static void guc_flush_logs(struct intel_guc *guc) guc_log_capture_logs(guc); } +int intel_guc_log_create(struct intel_guc *guc) +{ + struct i915_vma *vma; + unsigned long offset; + uint32_t size, flags; + int ret; + + GEM_BUG_ON(guc->log.vma); + + if (i915.guc_log_level > GUC_LOG_VERBOSITY_MAX) + i915.guc_log_level = GUC_LOG_VERBOSITY_MAX; + + /* The first page is to save log buffer state. Allocate one + * extra page for others in case for overlap */ + size = (1 + GUC_LOG_DPC_PAGES + 1 + + GUC_LOG_ISR_PAGES + 1 + + GUC_LOG_CRASH_PAGES + 1) << PAGE_SHIFT; + + /* We require SSE 4.1 for fast reads from the GuC log buffer and + * it should be present on the chipsets supporting GuC based + * submisssions. + */ + if (WARN_ON(!i915_has_memcpy_from_wc())) { + ret = -EINVAL; + goto err; + } + + vma = intel_guc_allocate_vma(guc, size); + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + goto err; + } + + guc->log.vma = vma; + + if (i915.guc_log_level >= 0) { + ret = guc_log_create_extras(guc); + if (ret < 0) + goto err_vma; + } + + /* each allocated unit is a page */ + flags = GUC_LOG_VALID | GUC_LOG_NOTIFY_ON_HALF_FULL | + (GUC_LOG_DPC_PAGES << GUC_LOG_DPC_SHIFT) | + (GUC_LOG_ISR_PAGES << GUC_LOG_ISR_SHIFT) | + (GUC_LOG_CRASH_PAGES << GUC_LOG_CRASH_SHIFT); + + offset = guc_ggtt_offset(vma) >> PAGE_SHIFT; /* in pages */ + guc->log.flags = (offset << GUC_LOG_BUF_ADDR_SHIFT) | flags; + + return 0; + +err_vma: + i915_vma_unpin_and_release(&guc->log.vma); +err: + /* logging will be off */ + i915.guc_log_level = -1; + return ret; +} + +void intel_guc_log_destroy(struct intel_guc *guc) +{ + guc_log_destroy_extras(guc); + i915_vma_unpin_and_release(&guc->log.vma); +} + int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val) { struct intel_guc *guc = &dev_priv->guc; @@ -609,17 +605,21 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val) return ret; } - i915.guc_log_level = log_param.verbosity; + if (log_param.logging_enabled) { + i915.guc_log_level = log_param.verbosity; - /* If log_level was set as -1 at boot time, then the relay channel file - * wouldn't have been created by now and interrupts also would not have - * been enabled. - */ - if (!dev_priv->guc.log.relay_chan) { + /* If log_level was set as -1 at boot time, then the relay channel file + * wouldn't have been created by now and interrupts also would not have + * been enabled. Try again now, just in case. + */ ret = guc_log_late_setup(guc); - if (!ret) - gen9_enable_guc_interrupts(dev_priv); - } else if (!log_param.logging_enabled) { + if (ret < 0) { + DRM_DEBUG_DRIVER("GuC log late setup failed %d\n", ret); + return ret; + } + + gen9_enable_guc_interrupts(dev_priv); + } else { /* Once logging is disabled, GuC won't generate logs & send an * interrupt. But there could be some data in the log buffer * which is yet to be captured. So request GuC to update the log @@ -629,9 +629,6 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val) /* As logging is disabled, update log level to reflect that */ i915.guc_log_level = -1; - } else { - /* In case interrupts were disabled, enable them now */ - gen9_enable_guc_interrupts(dev_priv); } return ret; @@ -639,7 +636,7 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val) void i915_guc_log_register(struct drm_i915_private *dev_priv) { - if (!i915.enable_guc_submission) + if (!i915.enable_guc_submission || i915.guc_log_level < 0) return; mutex_lock(&dev_priv->drm.struct_mutex); @@ -653,6 +650,7 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv) return; mutex_lock(&dev_priv->drm.struct_mutex); - guc_log_cleanup(&dev_priv->guc); + gen9_disable_guc_interrupts(dev_priv); + guc_log_destroy_extras(&dev_priv->guc); mutex_unlock(&dev_priv->drm.struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 0d12c8e..1a75ef2 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -190,8 +190,9 @@ struct intel_huc { /* intel_guc_loader.c */ extern void intel_guc_init(struct drm_i915_private *dev_priv); -extern int intel_guc_setup(struct drm_i915_private *dev_priv); extern void intel_guc_fini(struct drm_i915_private *dev_priv); +extern int intel_guc_setup(struct drm_i915_private *dev_priv); +extern void intel_guc_cleanup(struct drm_i915_private *dev_priv); extern const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status); extern int intel_guc_suspend(struct drm_i915_private *dev_priv); extern int intel_guc_resume(struct drm_i915_private *dev_priv); @@ -209,10 +210,11 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size); /* intel_guc_log.c */ -void intel_guc_log_create(struct intel_guc *guc); +int intel_guc_log_create(struct intel_guc *guc); +void intel_guc_log_destroy(struct intel_guc *guc); +int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); void i915_guc_log_register(struct drm_i915_private *dev_priv); void i915_guc_log_unregister(struct drm_i915_private *dev_priv); -int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val); static inline u32 guc_ggtt_offset(struct i915_vma *vma) { -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx