From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 02/18] drm/i915: preliminary context support Date: Thu, 29 Mar 2012 00:43:00 +0200 Message-ID: <20120328224300.GH2046@phenom.ffwll.local> References: <1332103198-25852-1-git-send-email-ben@bwidawsk.net> <1332103198-25852-3-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 72D8D9E773 for ; Wed, 28 Mar 2012 15:42:17 -0700 (PDT) Received: by werp11 with SMTP id p11so1166620wer.36 for ; Wed, 28 Mar 2012 15:42:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1332103198-25852-3-git-send-email-ben@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sun, Mar 18, 2012 at 01:39:42PM -0700, Ben Widawsky wrote: > Very basic code for context setup/destruction in the driver. > = > There are 4 entry points into the contexts, load, unload, open, close. > The names are self-explanatory except that load can be called during > reset, and also during pm thaw/resume. As we expect our context to be > preserved across these events, we do not reinitialize in this case. > = > Also an important note, as I intend to use contexts for ILK RC6, the > context initialization must always come before RC6 initialization. > = > As Adam Jackson pointed out, I picked an arbitrary cutoff of 1MB where I > decide the HW context is too big. The reason for this is even though > context sizes are increasing with every generation, they are still > measured in pages. If we somehow read back way more than that, it > probably means BIOS has done something strange, or we're running on a > platform that wasn't designed for this. > = > The 1MB was just a nice round number. I'm open to changing it to > something sensible if someone has a better idea. > = > Signed-off-by: Ben Widawsky I see not that much precedence for _load and _unload for setup/teardown ... Also this patch is imo way too early in the series - you just add empty functions so I have no idea what they're doing. And hence can't check whether you add them at the right place. Whereas if this comes later I already know what they're doing and can check without applying whether they're all called at the right place. Cheers, Daniel > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/i915_dma.c | 4 ++ > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 9 +++ > drivers/gpu/drm/i915/i915_gem.c | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 114 +++++++++++++++++++++++++= ++++++ > 6 files changed, 130 insertions(+) > create mode 100644 drivers/gpu/drm/i915/i915_gem_context.c > = > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index ce7fc77..a625d30 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -7,6 +7,7 @@ i915-y :=3D i915_drv.o i915_dma.o i915_irq.o \ > i915_debugfs.o \ > i915_suspend.o \ > i915_gem.o \ > + i915_gem_context.o \ > i915_gem_debug.o \ > i915_gem_evict.o \ > i915_gem_execbuffer.o \ > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_= dma.c > index 9341eb8..4c7c1dc 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -2155,6 +2155,7 @@ int i915_driver_unload(struct drm_device *dev) > ret =3D i915_gpu_idle(dev, true); > if (ret) > DRM_ERROR("failed to idle hardware: %d\n", ret); > + i915_gem_context_unload(dev); > mutex_unlock(&dev->struct_mutex); > = > /* Cancel the retire work handler, which should be idle now. */ > @@ -2244,6 +2245,8 @@ int i915_driver_open(struct drm_device *dev, struct= drm_file *file) > spin_lock_init(&file_priv->mm.lock); > INIT_LIST_HEAD(&file_priv->mm.request_list); > = > + i915_gem_context_open(dev, file); > + > return 0; > } > = > @@ -2276,6 +2279,7 @@ void i915_driver_lastclose(struct drm_device * dev) > = > void i915_driver_preclose(struct drm_device * dev, struct drm_file *file= _priv) > { > + i915_gem_context_close(dev, file_priv); > i915_gem_release(dev, file_priv); > } > = > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index 0694e17..b2c56db 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -742,6 +742,7 @@ int i915_reset(struct drm_device *dev, u8 flags) > if (HAS_BLT(dev)) > dev_priv->ring[BCS].init(&dev_priv->ring[BCS]); > = > + i915_gem_context_load(dev); > i915_gem_init_ppgtt(dev); > = > mutex_unlock(&dev->struct_mutex); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index c0f19f5..33c232a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -779,6 +779,9 @@ typedef struct drm_i915_private { > = > struct drm_property *broadcast_rgb_property; > struct drm_property *force_audio_property; > + > + bool hw_contexts_disabled; > + uint32_t hw_context_size; > } drm_i915_private_t; > = > enum hdmi_force_audio { > @@ -1280,6 +1283,12 @@ i915_gem_get_unfenced_gtt_alignment(struct drm_dev= ice *dev, > int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, > enum i915_cache_level cache_level); > = > +/* i915_gem_context.c */ > +void i915_gem_context_load(struct drm_device *dev); > +void i915_gem_context_unload(struct drm_device *dev); > +void i915_gem_context_open(struct drm_device *dev, struct drm_file *file= ); > +void i915_gem_context_close(struct drm_device *dev, struct drm_file *fil= e); > + > /* i915_gem_gtt.c */ > int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev); > void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_= gem.c > index 1f441f5..6343a82 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3811,6 +3811,7 @@ i915_gem_init_hw(struct drm_device *dev) > = > dev_priv->next_seqno =3D 1; > = > + i915_gem_context_load(dev); > i915_gem_init_ppgtt(dev); > = > return 0; > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > new file mode 100644 > index 0000000..caa0e06 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -0,0 +1,114 @@ > +/* > + * Copyright =A9 2012 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the = next > + * paragraph) shall be included in all copies or substantial portions of= the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER D= EALINGS > + * IN THE SOFTWARE. > + * > + * Authors: > + * Ben Widawsky > + * > + */ > + > +#include "drmP.h" > +#include "i915_drm.h" > +#include "i915_drv.h" > + > +static int get_context_size(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + int ret; > + u32 reg; > + > + /* Context size (as of gen7) is determined in number of cache lines */ > + switch (INTEL_INFO(dev)->gen) { > + case 5: /* ILK & SNB have the same context reg layout */ > + case 6: > + reg =3D I915_READ(CXT_SIZE); > + ret =3D GEN6_CXT_TOTAL_SIZE(reg) * 64; > + break; > + case 7: > + reg =3D I915_READ(GEN7_CTX_SIZE); > + ret =3D GEN7_CTX_TOTAL_SIZE(reg) * 64; > + break; > + default: > + ret =3D -1; > + } > + > + return ret; > +} > + > +/** > + * The default context needs to exist per ring that uses contexts. It st= ores the > + * context state of the GPU for applications that don't utilize HW conte= xts, as > + * well as an idle case. > + */ > +static int create_default_context(struct drm_i915_private *dev_priv) > +{ > + return 0; > +} > + > +void i915_gem_context_load(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + uint32_t ctx_size; > + > + /* If called from reset, or thaw... we've been here already */ > + if (dev_priv->hw_contexts_disabled) > + return; > + > + ctx_size =3D get_context_size(dev); > + dev_priv->hw_context_size =3D get_context_size(dev); > + dev_priv->hw_context_size =3D round_up(dev_priv->hw_context_size, 4096); > + > + if (ctx_size <=3D 0 || ctx_size > (1<<20)) { > + dev_priv->hw_contexts_disabled =3D true; > + return; > + } > + > + if (create_default_context(dev_priv)) { > + dev_priv->hw_contexts_disabled =3D true; > + return; > + } > + > + DRM_DEBUG_DRIVER("HW context support initialized\n"); > +} > + > +void i915_gem_context_unload(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + if (dev_priv->hw_contexts_disabled) > + return; > +} > + > +void i915_gem_context_open(struct drm_device *dev, struct drm_file *file) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + if (dev_priv->hw_contexts_disabled) > + return; > +} > + > +void i915_gem_context_close(struct drm_device *dev, struct drm_file *fil= e) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + > + if (dev_priv->hw_contexts_disabled) > + return; > +} > -- = > 1.7.9.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48