From patchwork Wed Jul 15 08:46:56 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: sourab.gupta@intel.com X-Patchwork-Id: 6794561 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6E81B9F2E8 for ; Wed, 15 Jul 2015 08:44:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 458AC20603 for ; Wed, 15 Jul 2015 08:44:54 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 1B4A0205F4 for ; Wed, 15 Jul 2015 08:44:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AB6526E7D8; Wed, 15 Jul 2015 01:44:52 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 118EA6E7D8 for ; Wed, 15 Jul 2015 01:44:52 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 15 Jul 2015 01:44:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,479,1432623600"; d="scan'208";a="764782396" Received: from sourabgu-desktop.iind.intel.com ([10.223.82.35]) by orsmga002.jf.intel.com with ESMTP; 15 Jul 2015 01:44:49 -0700 From: sourab.gupta@intel.com To: intel-gfx@lists.freedesktop.org Date: Wed, 15 Jul 2015 14:16:56 +0530 Message-Id: <1436950023-13940-2-git-send-email-sourab.gupta@intel.com> X-Mailer: git-send-email 1.8.5.1 In-Reply-To: <1436950023-13940-1-git-send-email-sourab.gupta@intel.com> References: <1436950023-13940-1-git-send-email-sourab.gupta@intel.com> Cc: Insoo Woo , Peter Zijlstra , Jabin Wu , Sourab Gupta Subject: [Intel-gfx] [RFC 1/8] drm/i915: Have globally unique context ids, as opposed to drm file specific X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Sourab Gupta Currently the context ids are specific to a drm file instance, as opposed to being globally unique. There are some usecases, which may require globally unique context ids. For e.g. a system level GPU profiler tool may lean upon the context ids to associate the performance snapshots with individual contexts. If the context ids are unique, it may do so without relying on any additional information such as pid and drm fd. This patch proposes an implementation of globally unique context ids, by conceptually moving the idr table for holding the context ids, into device private structure instead of file private structure. The case of default context id for drm file (which is given by id=0) is handled by storing the same in file private during context creation, and retrieving as and when required. This patch is proposed an an enabler for the patches following in the series. In particular, I'm looking for feedback on the pros and cons of having a globally unique context id, and any specific inputs on this particular implementation. This implementation can be improved upon, if agreed upon conceptually. Signed-off-by: Sourab Gupta --- drivers/gpu/drm/i915/i915_debugfs.c | 4 +-- drivers/gpu/drm/i915/i915_drv.h | 4 ++- drivers/gpu/drm/i915/i915_gem_context.c | 53 +++++++++++++++++++++++---------- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 47636f3..38e0026 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2254,12 +2254,10 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev) } list_for_each_entry_reverse(file, &dev->filelist, lhead) { - struct drm_i915_file_private *file_priv = file->driver_priv; - seq_printf(m, "proc: %s\n", get_pid_task(file->pid, PIDTYPE_PID)->comm); - idr_for_each(&file_priv->context_idr, per_file_ctx, m); } + idr_for_each(&dev_priv->context_idr, per_file_ctx, m); seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK)); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 50977f0..baa0234 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -320,7 +320,7 @@ struct drm_i915_file_private { */ #define DRM_I915_THROTTLE_JIFFIES msecs_to_jiffies(20) } mm; - struct idr context_idr; + u32 first_ctx_id; struct intel_rps_client { struct list_head link; @@ -1754,6 +1754,8 @@ struct drm_i915_private { struct intel_opregion opregion; struct intel_vbt_data vbt; + struct idr context_idr; + bool preserve_bios_swizzle; /* overlay */ diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index d9ccad5..6b572c1 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -212,7 +212,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size) static struct intel_context * __create_hw_context(struct drm_device *dev, - struct drm_i915_file_private *file_priv) + struct drm_i915_file_private *file_priv, + bool is_first_ctx) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_context *ctx; @@ -237,10 +238,12 @@ __create_hw_context(struct drm_device *dev, /* Default context will never have a file_priv */ if (file_priv != NULL) { - ret = idr_alloc(&file_priv->context_idr, ctx, + ret = idr_alloc(&dev_priv->context_idr, ctx, DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL); if (ret < 0) goto err_out; + if (is_first_ctx) + file_priv->first_ctx_id = ret; } else ret = DEFAULT_CONTEXT_HANDLE; @@ -267,7 +270,8 @@ err_out: */ static struct intel_context * i915_gem_create_context(struct drm_device *dev, - struct drm_i915_file_private *file_priv) + struct drm_i915_file_private *file_priv, + bool is_first_ctx) { const bool is_global_default_ctx = file_priv == NULL; struct intel_context *ctx; @@ -275,7 +279,7 @@ i915_gem_create_context(struct drm_device *dev, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); - ctx = __create_hw_context(dev, file_priv); + ctx = __create_hw_context(dev, file_priv, is_first_ctx); if (IS_ERR(ctx)) return ctx; @@ -348,6 +352,14 @@ void i915_gem_context_reset(struct drm_device *dev) } } +static int context_idr_cleanup(int id, void *p, void *data) +{ + struct intel_context *ctx = p; + + i915_gem_context_unreference(ctx); + return 0; +} + int i915_gem_context_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -371,8 +383,9 @@ int i915_gem_context_init(struct drm_device *dev) dev_priv->hw_context_size = 0; } } + idr_init(&dev_priv->context_idr); - ctx = i915_gem_create_context(dev, NULL); + ctx = i915_gem_create_context(dev, NULL, false); if (IS_ERR(ctx)) { DRM_ERROR("Failed to create default global context (error %ld)\n", PTR_ERR(ctx)); @@ -398,6 +411,9 @@ void i915_gem_context_fini(struct drm_device *dev) struct intel_context *dctx = dev_priv->ring[RCS].default_context; int i; + idr_for_each(&dev_priv->context_idr, context_idr_cleanup, NULL); + idr_destroy(&dev_priv->context_idr); + if (dctx->legacy_hw_ctx.rcs_state) { /* The only known way to stop the gpu from accessing the hw context is * to reset it. Do this as the very last operation to avoid confusing @@ -465,11 +481,14 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv) return 0; } -static int context_idr_cleanup(int id, void *p, void *data) +static int cleanup_file_contexts(int id, void *p, void *data) { struct intel_context *ctx = p; + struct drm_i915_file_private *file_priv = data; + + if (ctx->file_priv == file_priv) + i915_gem_context_unreference(ctx); - i915_gem_context_unreference(ctx); return 0; } @@ -478,14 +497,11 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) struct drm_i915_file_private *file_priv = file->driver_priv; struct intel_context *ctx; - idr_init(&file_priv->context_idr); - mutex_lock(&dev->struct_mutex); - ctx = i915_gem_create_context(dev, file_priv); + ctx = i915_gem_create_context(dev, file_priv, true); mutex_unlock(&dev->struct_mutex); if (IS_ERR(ctx)) { - idr_destroy(&file_priv->context_idr); return PTR_ERR(ctx); } @@ -495,17 +511,21 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) { struct drm_i915_file_private *file_priv = file->driver_priv; + struct drm_i915_private *dev_priv = file_priv->dev_priv; - idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL); - idr_destroy(&file_priv->context_idr); + idr_for_each(&dev_priv->context_idr, cleanup_file_contexts, file_priv); } struct intel_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) { + struct drm_i915_private *dev_priv = file_priv->dev_priv; struct intel_context *ctx; - ctx = (struct intel_context *)idr_find(&file_priv->context_idr, id); + if (id == 0) + id = file_priv->first_ctx_id; + + ctx = (struct intel_context *)idr_find(&dev_priv->context_idr, id); if (!ctx) return ERR_PTR(-ENOENT); @@ -862,7 +882,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, if (ret) return ret; - ctx = i915_gem_create_context(dev, file_priv); + ctx = i915_gem_create_context(dev, file_priv, false); mutex_unlock(&dev->struct_mutex); if (IS_ERR(ctx)) return PTR_ERR(ctx); @@ -878,6 +898,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, { struct drm_i915_gem_context_destroy *args = data; struct drm_i915_file_private *file_priv = file->driver_priv; + struct drm_i915_private *dev_priv = dev->dev_private; struct intel_context *ctx; int ret; @@ -894,7 +915,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, return PTR_ERR(ctx); } - idr_remove(&ctx->file_priv->context_idr, ctx->user_handle); + idr_remove(&dev_priv->context_idr, ctx->user_handle); i915_gem_context_unreference(ctx); mutex_unlock(&dev->struct_mutex);