From patchwork Mon Mar 19 09:53:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Winiarski?= X-Patchwork-Id: 10292035 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6A4CE602C2 for ; Mon, 19 Mar 2018 09:54:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 568C128673 for ; Mon, 19 Mar 2018 09:54:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4B349291F6; Mon, 19 Mar 2018 09:54:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 91B3B28673 for ; Mon, 19 Mar 2018 09:54:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F10E66E4F1; Mon, 19 Mar 2018 09:54:18 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by gabe.freedesktop.org (Postfix) with ESMTPS id A76B36E4EF for ; Mon, 19 Mar 2018 09:54:17 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Mar 2018 02:54:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,330,1517904000"; d="scan'208";a="43292992" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga002.jf.intel.com with ESMTP; 19 Mar 2018 02:54:16 -0700 Received: from localhost (172.28.172.64) by IRSMSX102.ger.corp.intel.com (163.33.3.155) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 19 Mar 2018 09:54:15 +0000 From: =?UTF-8?q?Micha=C5=82=20Winiarski?= To: Date: Mon, 19 Mar 2018 10:53:40 +0100 Message-ID: <20180319095348.9716-5-michal.winiarski@intel.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20180319095348.9716-1-michal.winiarski@intel.com> References: <20180319095348.9716-1-michal.winiarski@intel.com> MIME-Version: 1.0 X-Originating-IP: [172.28.172.64] Subject: [Intel-gfx] [PATCH v3 05/13] drm/i915/guc: Split relay control and GuC log level X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Those two concepts are really separate. Since GuC is writing data into its own buffer and we even provide a way for userspace to read directly from it using i915_guc_log_dump debugfs, there's no real reason to tie log level with relay creation. Let's create a separate debugfs, giving userspace a way to create a relay on demand, when it wants to read a continuous log rather than a snapshot. v2: Don't touch guc_log_level on relay creation error, adjust locking after rebase, s/dev_priv/i915, pass guc to file->private_data (Sagar) Use struct_mutex rather than runtime.lock for set_log_level v3: Tidy ordering of definitions (Sagar) Signed-off-by: Michał Winiarski Cc: Chris Wilson Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Michal Wajdeczko Reviewed-by: Sagar Arun Kamble --- drivers/gpu/drm/i915/i915_debugfs.c | 56 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_drv.c | 4 -- drivers/gpu/drm/i915/intel_guc_log.c | 75 +++++++++++++++--------------------- drivers/gpu/drm/i915/intel_guc_log.h | 9 +++-- drivers/gpu/drm/i915/intel_uc.c | 22 ----------- drivers/gpu/drm/i915/intel_uc.h | 2 - 6 files changed, 84 insertions(+), 84 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 5378863e3238..e857a9493b6f 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2495,32 +2495,73 @@ static int i915_guc_log_dump(struct seq_file *m, void *data) return 0; } -static int i915_guc_log_control_get(void *data, u64 *val) +static int i915_guc_log_level_get(void *data, u64 *val) { struct drm_i915_private *dev_priv = data; if (!USES_GUC(dev_priv)) return -ENODEV; - *val = intel_guc_log_control_get(&dev_priv->guc.log); + *val = intel_guc_log_level_get(&dev_priv->guc.log); return 0; } -static int i915_guc_log_control_set(void *data, u64 val) +static int i915_guc_log_level_set(void *data, u64 val) { struct drm_i915_private *dev_priv = data; if (!USES_GUC(dev_priv)) return -ENODEV; - return intel_guc_log_control_set(&dev_priv->guc.log, val); + return intel_guc_log_level_set(&dev_priv->guc.log, val); } -DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops, - i915_guc_log_control_get, i915_guc_log_control_set, +DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_level_fops, + i915_guc_log_level_get, i915_guc_log_level_set, "%lld\n"); +static int i915_guc_log_relay_open(struct inode *inode, struct file *file) +{ + struct drm_i915_private *dev_priv = inode->i_private; + + if (!USES_GUC(dev_priv)) + return -ENODEV; + + file->private_data = &dev_priv->guc.log; + + return intel_guc_log_relay_open(&dev_priv->guc.log); +} + +static ssize_t +i915_guc_log_relay_write(struct file *filp, + const char __user *ubuf, + size_t cnt, + loff_t *ppos) +{ + struct intel_guc_log *log = filp->private_data; + + intel_guc_log_relay_flush(log); + + return cnt; +} + +static int i915_guc_log_relay_release(struct inode *inode, struct file *file) +{ + struct drm_i915_private *dev_priv = inode->i_private; + + intel_guc_log_relay_close(&dev_priv->guc.log); + + return 0; +} + +static const struct file_operations i915_guc_log_relay_fops = { + .owner = THIS_MODULE, + .open = i915_guc_log_relay_open, + .write = i915_guc_log_relay_write, + .release = i915_guc_log_relay_release, +}; + static const char *psr2_live_status(u32 val) { static const char * const live_status[] = { @@ -4748,7 +4789,8 @@ static const struct i915_debugfs_files { {"i915_dp_test_data", &i915_displayport_test_data_fops}, {"i915_dp_test_type", &i915_displayport_test_type_fops}, {"i915_dp_test_active", &i915_displayport_test_active_fops}, - {"i915_guc_log_control", &i915_guc_log_control_fops}, + {"i915_guc_log_level", &i915_guc_log_level_fops}, + {"i915_guc_log_relay", &i915_guc_log_relay_fops}, {"i915_hpd_storm_ctl", &i915_hpd_storm_ctl_fops}, {"i915_ipc_status", &i915_ipc_status_fops}, {"i915_drrs_ctl", &i915_drrs_ctl_fops} diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3df5193487f3..1021bf40e236 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1239,9 +1239,6 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) i915_debugfs_register(dev_priv); i915_setup_sysfs(dev_priv); - /* Depends on debugfs having been initialized */ - intel_uc_register(dev_priv); - /* Depends on sysfs having been initialized */ i915_perf_register(dev_priv); } else @@ -1299,7 +1296,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) i915_pmu_unregister(dev_priv); i915_teardown_sysfs(dev_priv); - intel_uc_unregister(dev_priv); drm_dev_unregister(&dev_priv->drm); i915_gem_shrinker_unregister(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index 72a71bc94adf..20254dde172c 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -423,18 +423,13 @@ static int guc_log_relay_create(struct intel_guc_log *log) DRM_ERROR("Couldn't create relay chan for GuC logging\n"); ret = -ENOMEM; - goto err; + return ret; } GEM_BUG_ON(guc_log_relay_chan->subbuf_size < subbuf_size); log->runtime.relay_chan = guc_log_relay_chan; return 0; - -err: - /* logging will be off */ - i915_modparams.guc_log_level = 0; - return ret; } static void guc_log_relay_destroy(struct intel_guc_log *log) @@ -511,7 +506,7 @@ void intel_guc_log_destroy(struct intel_guc_log *log) i915_vma_unpin_and_release(&log->vma); } -int intel_guc_log_control_get(struct intel_guc_log *log) +int intel_guc_log_level_get(struct intel_guc_log *log) { GEM_BUG_ON(!log->vma); GEM_BUG_ON(i915_modparams.guc_log_level < 0); @@ -526,11 +521,10 @@ int intel_guc_log_control_get(struct intel_guc_log *log) LOG_LEVEL_TO_ENABLED(_x) ? _x - 1 : 0; \ }) #define VERBOSITY_TO_LOG_LEVEL(x) ((x) + 1) -int intel_guc_log_control_set(struct intel_guc_log *log, u64 val) +int intel_guc_log_level_set(struct intel_guc_log *log, u64 val) { struct intel_guc *guc = log_to_guc(log); struct drm_i915_private *dev_priv = guc_to_i915(guc); - bool enabled = LOG_LEVEL_TO_ENABLED(val); int ret; BUILD_BUG_ON(GUC_LOG_VERBOSITY_MIN != 0); @@ -553,7 +547,8 @@ int intel_guc_log_control_set(struct intel_guc_log *log, u64 val) } intel_runtime_pm_get(dev_priv); - ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); + ret = guc_log_control(guc, LOG_LEVEL_TO_ENABLED(val), + LOG_LEVEL_TO_VERBOSITY(val)); intel_runtime_pm_put(dev_priv); if (ret) { DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret); @@ -562,89 +557,79 @@ int intel_guc_log_control_set(struct intel_guc_log *log, u64 val) i915_modparams.guc_log_level = val; - mutex_unlock(&dev_priv->drm.struct_mutex); - - if (enabled && !guc_log_has_runtime(log)) { - ret = intel_guc_log_register(log); - if (ret) { - /* logging will remain off */ - i915_modparams.guc_log_level = 0; - goto out; - } - } else if (!enabled && guc_log_has_runtime(log)) { - intel_guc_log_unregister(log); - } - - return 0; - out_unlock: mutex_unlock(&dev_priv->drm.struct_mutex); -out: + return ret; } -int intel_guc_log_register(struct intel_guc_log *log) +int intel_guc_log_relay_open(struct intel_guc_log *log) { int ret; mutex_lock(&log->runtime.lock); - GEM_BUG_ON(guc_log_has_runtime(log)); + if (guc_log_has_runtime(log)) { + ret = -EEXIST; + goto out_unlock; + } ret = guc_log_relay_create(log); if (ret) - goto err; + goto out_unlock; ret = guc_log_map(log); if (ret) - goto err_relay; + goto out_relay; + + mutex_unlock(&log->runtime.lock); guc_flush_log_msg_enable(log_to_guc(log)); - mutex_unlock(&log->runtime.lock); + /* + * When GuC is logging without us relaying to userspace, we're ignoring + * the flush notification. This means that we need to unconditionally + * flush on relay enabling, since GuC only notifies us once. + */ + queue_work(log->runtime.flush_wq, &log->runtime.flush_work); return 0; -err_relay: +out_relay: guc_log_relay_destroy(log); -err: +out_unlock: mutex_unlock(&log->runtime.lock); return ret; } -void intel_guc_log_unregister(struct intel_guc_log *log) +void intel_guc_log_relay_flush(struct intel_guc_log *log) { struct intel_guc *guc = log_to_guc(log); struct drm_i915_private *i915 = guc_to_i915(guc); - guc_flush_log_msg_disable(guc); - /* * Before initiating the forceful flush, wait for any pending/ongoing * flush to complete otherwise forceful flush may not actually happen. */ flush_work(&log->runtime.flush_work); - /* - * 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 - * buffer state and then collect the left over logs. - */ intel_runtime_pm_get(i915); guc_log_flush(guc); intel_runtime_pm_put(i915); /* GuC would have updated log buffer by now, so capture it */ guc_log_capture_logs(log); +} - mutex_lock(&log->runtime.lock); +void intel_guc_log_relay_close(struct intel_guc_log *log) +{ + guc_flush_log_msg_disable(log_to_guc(log)); + flush_work(&log->runtime.flush_work); + mutex_lock(&log->runtime.lock); GEM_BUG_ON(!guc_log_has_runtime(log)); - guc_log_unmap(log); guc_log_relay_destroy(log); - mutex_unlock(&log->runtime.lock); } diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h index e0ea625032fb..3cf911eef3a8 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.h +++ b/drivers/gpu/drm/i915/intel_guc_log.h @@ -61,11 +61,12 @@ struct intel_guc_log { void intel_guc_log_init_early(struct intel_guc_log *log); int intel_guc_log_create(struct intel_guc_log *log); -int intel_guc_log_register(struct intel_guc_log *log); -void intel_guc_log_unregister(struct intel_guc_log *log); void intel_guc_log_destroy(struct intel_guc_log *log); -int intel_guc_log_control_get(struct intel_guc_log *log); -int intel_guc_log_control_set(struct intel_guc_log *log, u64 control); +int intel_guc_log_level_get(struct intel_guc_log *log); +int intel_guc_log_level_set(struct intel_guc_log *log, u64 control_val); +int intel_guc_log_relay_open(struct intel_guc_log *log); +void intel_guc_log_relay_flush(struct intel_guc_log *log); +void intel_guc_log_relay_close(struct intel_guc_log *log); #endif diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c index 765b86a53f19..9bb40cd047a0 100644 --- a/drivers/gpu/drm/i915/intel_uc.c +++ b/drivers/gpu/drm/i915/intel_uc.c @@ -221,28 +221,6 @@ static void guc_free_load_err_log(struct intel_guc *guc) i915_gem_object_put(guc->load_err_log); } -int intel_uc_register(struct drm_i915_private *i915) -{ - int ret = 0; - - if (!USES_GUC(i915)) - return 0; - - if (i915_modparams.guc_log_level) - ret = intel_guc_log_register(&i915->guc.log); - - return ret; -} - -void intel_uc_unregister(struct drm_i915_private *i915) -{ - if (!USES_GUC(i915)) - return; - - if (i915_modparams.guc_log_level) - intel_guc_log_unregister(&i915->guc.log); -} - static int guc_enable_communication(struct intel_guc *guc) { struct drm_i915_private *dev_priv = guc_to_i915(guc); diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h index 0a2b413e9cd0..937e61175258 100644 --- a/drivers/gpu/drm/i915/intel_uc.h +++ b/drivers/gpu/drm/i915/intel_uc.h @@ -30,8 +30,6 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv); void intel_uc_init_mmio(struct drm_i915_private *dev_priv); -int intel_uc_register(struct drm_i915_private *dev_priv); -void intel_uc_unregister(struct drm_i915_private *dev_priv); void intel_uc_init_fw(struct drm_i915_private *dev_priv); void intel_uc_fini_fw(struct drm_i915_private *dev_priv); int intel_uc_init_misc(struct drm_i915_private *dev_priv);