From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] [v3] drm/i915: Require HW contexts (when possible) Date: Fri, 18 Oct 2013 10:39:47 +0300 Message-ID: <20131018073947.GL13047@intel.com> References: <1382046594-892-1-git-send-email-benjamin.widawsky@intel.com> <1382049546-19615-1-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id CDE75E7379 for ; Fri, 18 Oct 2013 00:39:50 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1382049546-19615-1-git-send-email-benjamin.widawsky@intel.com> 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 , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Thu, Oct 17, 2013 at 03:39:06PM -0700, Ben Widawsky wrote: > v2: Fixed the botched locking on init_hw failure in i915_reset (Ville) > Call cleanup_ringbuffer on failed context create in init_hw (Ville) > = > v3: Add dev argument ti clean_ringbuffer > = > Reviewed-by: Kenneth Graunke > Signed-off-by: Ben Widawsky Reviewed-by: Ville Syrj=E4l=E4 > --- > drivers/gpu/drm/i915/i915_drv.c | 3 --- > drivers/gpu/drm/i915/i915_drv.h | 3 +-- > drivers/gpu/drm/i915/i915_gem.c | 8 +++++- > drivers/gpu/drm/i915/i915_gem_context.c | 43 +++++++++++++++------------= ------ > drivers/gpu/drm/i915/i915_sysfs.c | 6 ++--- > 5 files changed, 31 insertions(+), 32 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index 1060a96..839925b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -750,12 +750,9 @@ int i915_reset(struct drm_device *dev) > */ > if (drm_core_check_feature(dev, DRIVER_MODESET) || > !dev_priv->ums.mm_suspended) { > - bool hw_contexts_disabled =3D dev_priv->hw_contexts_disabled; > dev_priv->ums.mm_suspended =3D 0; > = > ret =3D i915_gem_init_hw(dev); > - if (!hw_contexts_disabled && dev_priv->hw_contexts_disabled) > - DRM_ERROR("HW contexts didn't survive reset\n"); > mutex_unlock(&dev->struct_mutex); > if (ret) { > DRM_ERROR("Failed hw init on reset %d\n", ret); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index e4ff8e9..21b2c37 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1411,7 +1411,6 @@ 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; > struct list_head context_list; > = > @@ -2140,7 +2139,7 @@ i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *o= bj, > } > = > /* i915_gem_context.c */ > -void i915_gem_context_init(struct drm_device *dev); > +int __must_check i915_gem_context_init(struct drm_device *dev); > void i915_gem_context_fini(struct drm_device *dev); > void i915_gem_context_close(struct drm_device *dev, struct drm_file *fil= e); > int i915_switch_context(struct intel_ring_buffer *ring, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_= gem.c > index 34df59b..b43cd30 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4463,7 +4463,13 @@ i915_gem_init_hw(struct drm_device *dev) > * XXX: There was some w/a described somewhere suggesting loading > * contexts before PPGTT. > */ > - i915_gem_context_init(dev); > + ret =3D i915_gem_context_init(dev); > + if (ret) { > + i915_gem_cleanup_ringbuffer(dev); > + DRM_ERROR("Context initialization failed %d\n", ret); > + return ret; > + } > + > if (dev_priv->mm.aliasing_ppgtt) { > ret =3D dev_priv->mm.aliasing_ppgtt->enable(dev); > if (ret) { > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i9= 15/i915_gem_context.c > index cc619c1..4625670 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -244,36 +244,34 @@ err_destroy: > return ret; > } > = > -void i915_gem_context_init(struct drm_device *dev) > +int i915_gem_context_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > + int ret; > = > - if (!HAS_HW_CONTEXTS(dev)) { > - dev_priv->hw_contexts_disabled =3D true; > - DRM_DEBUG_DRIVER("Disabling HW Contexts; old hardware\n"); > - return; > - } > + if (!HAS_HW_CONTEXTS(dev)) > + return 0; > = > /* If called from reset, or thaw... we've been here already */ > - if (dev_priv->hw_contexts_disabled || > - dev_priv->ring[RCS].default_context) > - return; > + if (dev_priv->ring[RCS].default_context) > + return 0; > = > dev_priv->hw_context_size =3D round_up(get_context_size(dev), 4096); > = > if (dev_priv->hw_context_size > (1<<20)) { > - dev_priv->hw_contexts_disabled =3D true; > DRM_DEBUG_DRIVER("Disabling HW Contexts; invalid size\n"); > - return; > + return -E2BIG; > } > = > - if (create_default_context(dev_priv)) { > - dev_priv->hw_contexts_disabled =3D true; > - DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed\n"); > - return; > + ret =3D create_default_context(dev_priv); > + if (ret) { > + DRM_DEBUG_DRIVER("Disabling HW Contexts; create failed %d\n", > + ret); > + return ret; > } > = > DRM_DEBUG_DRIVER("HW context support initialized\n"); > + return 0; > } > = > void i915_gem_context_fini(struct drm_device *dev) > @@ -281,7 +279,7 @@ void i915_gem_context_fini(struct drm_device *dev) > struct drm_i915_private *dev_priv =3D dev->dev_private; > struct i915_hw_context *dctx =3D dev_priv->ring[RCS].default_context; > = > - if (dev_priv->hw_contexts_disabled) > + if (!HAS_HW_CONTEXTS(dev)) > return; > = > /* The only known way to stop the gpu from accessing the hw context is > @@ -324,16 +322,16 @@ i915_gem_context_get_hang_stats(struct drm_device *= dev, > struct drm_file *file, > u32 id) > { > - struct drm_i915_private *dev_priv =3D dev->dev_private; > struct drm_i915_file_private *file_priv =3D file->driver_priv; > struct i915_hw_context *ctx; > = > if (id =3D=3D DEFAULT_CONTEXT_ID) > return &file_priv->hang_stats; > = > - ctx =3D NULL; > - if (!dev_priv->hw_contexts_disabled) > - ctx =3D i915_gem_context_get(file->driver_priv, id); > + if (!HAS_HW_CONTEXTS(dev)) > + return ERR_PTR(-ENOENT); > + > + ctx =3D i915_gem_context_get(file->driver_priv, id); > if (ctx =3D=3D NULL) > return ERR_PTR(-ENOENT); > = > @@ -506,7 +504,7 @@ int i915_switch_context(struct intel_ring_buffer *rin= g, > struct drm_i915_private *dev_priv =3D ring->dev->dev_private; > struct i915_hw_context *to; > = > - if (dev_priv->hw_contexts_disabled) > + if (!HAS_HW_CONTEXTS(ring->dev)) > return 0; > = > WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); > @@ -531,7 +529,6 @@ int i915_switch_context(struct intel_ring_buffer *rin= g, > int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > struct drm_file *file) > { > - struct drm_i915_private *dev_priv =3D dev->dev_private; > struct drm_i915_gem_context_create *args =3D data; > struct drm_i915_file_private *file_priv =3D file->driver_priv; > struct i915_hw_context *ctx; > @@ -540,7 +537,7 @@ int i915_gem_context_create_ioctl(struct drm_device *= dev, void *data, > if (!(dev->driver->driver_features & DRIVER_GEM)) > return -ENODEV; > = > - if (dev_priv->hw_contexts_disabled) > + if (!HAS_HW_CONTEXTS(dev)) > return -ENODEV; > = > ret =3D i915_mutex_lock_interruptible(dev); > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i91= 5_sysfs.c > index bb31ff3..47bc51b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -183,13 +183,13 @@ i915_l3_write(struct file *filp, struct kobject *ko= bj, > int slice =3D (int)(uintptr_t)attr->private; > int ret; > = > + if (!HAS_HW_CONTEXTS(drm_dev)) > + return -ENXIO; > + > ret =3D l3_access_valid(drm_dev, offset); > if (ret) > return ret; > = > - if (dev_priv->hw_contexts_disabled) > - return -ENXIO; > - > ret =3D i915_mutex_lock_interruptible(drm_dev); > if (ret) > return ret; > -- = > 1.8.4.1 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC