Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752877AbaJWHsV (ORCPT ); Thu, 23 Oct 2014 03:48:21 -0400 Received: from mail.fireflyinternet.com ([87.106.93.118]:64537 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752149AbaJWHsT (ORCPT ); Thu, 23 Oct 2014 03:48:19 -0400 X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Date: Thu, 23 Oct 2014 08:47:39 +0100 From: Chris Wilson To: Robert Bragg Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , Paul Mackerras , Ingo Molnar , Arnaldo Carvalho de Melo , Daniel Vetter , Rob Clark , Samuel Pitoiset , Ben Skeggs Subject: Re: [RFC PATCH 3/3] i915: Expose PMU for Observation Architecture Message-ID: <20141023074739.GH13512@nuc-i3427.alporthouse.com> References: <1413991731-20628-1-git-send-email-robert@sixbynine.org> <1413991731-20628-4-git-send-email-robert@sixbynine.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1413991731-20628-4-git-send-email-robert@sixbynine.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 22, 2014 at 04:28:51PM +0100, Robert Bragg wrote: > + /* XXX: Not sure that this is really acceptable... > + * > + * i915_gem_context.c currently owns pinning/unpinning legacy > + * context buffers and although that code has a > + * get_context_alignment() func to handle a different > + * constraint for gen6 we are assuming it's fixed for gen7 > + * here. Another option besides pinning here would be to > + * instead hook into context switching and update the > + * OACONTROL configuration on the fly. > + */ > + if (dev_priv->oa_pmu.specific_ctx) { > + struct intel_context *ctx = dev_priv->oa_pmu.specific_ctx; > + int ret; > + > + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state, > + 4096, 0); Right if you pin it here with a different alignment, when we try to pin it with the required hw ctx alignment it will fail. Easiest way is to record the ctx->legacy_hw_ctx.alignment and reuse that here. > + if (ret) { > + DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); > + ret = -EBUSY; As an exercise, think of all the possible error values from pin() and tell me why overriding that here is a bad, bad idea. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/