On Tue, Jul 09, 2013 at 11:45:04AM +0200, Daniel Vetter wrote: > In kernel modeset driver mode we're in full control of the chip, > always. So there's no need at all to set mm.suspended in > i915_gem_idle. Hence move that out into the leavevt ioctl. Since > i915_gem_idle doesn't suspend gem any more we can also drop the > re-enabling for KMS in the thaw function. > > Also clean up the handling of mm.suspend at driver load by coalescing > all the assignments. > > Stumbled over while reading through our resume code for unrelated > reasons. > > v2: Shovel mm.suspended into the (newly created) ums dungeon as > suggested by Chris Wilson. The plan is that once we've completely > stopped relying on the register save/restore code we could shovel even > that in there. > > Cc: Chris Wilson <[email protected]> > Signed-off-by: Daniel Vetter <[email protected]>
Minor comments inline. Doing the rename in the same patch made it much easier to check all the users, Reviewed-by: Chris Wilson <[email protected]> > --- > drivers/gpu/drm/i915/i915_dma.c | 16 +++++++-------- > drivers/gpu/drm/i915/i915_drv.c | 5 ++--- > drivers/gpu/drm/i915/i915_drv.h | 24 +++++++++++++---------- > drivers/gpu/drm/i915/i915_gem.c | 31 > ++++++++++++++++++------------ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > 5 files changed, 43 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 0e22142..beca2d0 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1323,10 +1323,8 @@ static int i915_load_modeset_init(struct drm_device > *dev) > /* Always safe in the mode setting case. */ > /* FIXME: do pre/post-mode set stuff in core KMS code */ > dev->vblank_disable_allowed = 1; > - if (INTEL_INFO(dev)->num_pipes == 0) { > - dev_priv->mm.suspended = 0; > + if (INTEL_INFO(dev)->num_pipes == 0) > return 0; > - } > > ret = intel_fbdev_init(dev); > if (ret) > @@ -1352,9 +1350,6 @@ static int i915_load_modeset_init(struct drm_device > *dev) > > drm_kms_helper_poll_init(dev); > > - /* We're off and running w/KMS */ > - dev_priv->mm.suspended = 0; > - > return 0; > > cleanup_gem: > @@ -1629,9 +1624,6 @@ int i915_driver_load(struct drm_device *dev, unsigned > long flags) > goto out_gem_unload; > } > > - /* Start out suspended */ > - dev_priv->mm.suspended = 1; > - > if (HAS_POWER_WELL(dev)) > i915_init_power_well(dev); > > @@ -1641,6 +1633,12 @@ int i915_driver_load(struct drm_device *dev, unsigned > long flags) > DRM_ERROR("failed to init modeset\n"); > goto out_gem_unload; > } > + > + /* We're off and running w/KMS */ > + dev_priv->ums.mm_suspended = 0; Just don't bother touching it here (we kzalloc our driver private). > + } else { > + /* Start out suspended in ums mode. */ > + dev_priv->ums.mm_suspended = 1; > } > > i915_setup_sysfs(dev); > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index e6dc81c..50a76f59 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -661,7 +661,6 @@ static int __i915_drm_thaw(struct drm_device *dev) > intel_init_pch_refclk(dev); > > mutex_lock(&dev->struct_mutex); > - dev_priv->mm.suspended = 0; > > error = i915_gem_init_hw(dev); > mutex_unlock(&dev->struct_mutex); > @@ -960,11 +959,11 @@ int i915_reset(struct drm_device *dev) > * switched away). > */ > if (drm_core_check_feature(dev, DRIVER_MODESET) || > - !dev_priv->mm.suspended) { > + !dev_priv->ums.mm_suspended) { > struct intel_ring_buffer *ring; > int i; > > - dev_priv->mm.suspended = 0; > + dev_priv->ums.mm_suspended = 0; > > i915_gem_init_swizzling(dev); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index c8d6104..be2fcd3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -814,6 +814,18 @@ struct i915_dri1_state { > uint32_t counter; > }; > > +struct i915_ums_state { > + /** > + * Flag if the X Server, and thus DRM, is not currently in > + * control of the device. > + * > + * This is set between LeaveVT and EnterVT. It needs to be > + * replaced with a semaphore. It also needs to be > + * transitioned away from for kernel modesetting. > + */ > + int mm_suspended; > +}; > + > struct intel_l3_parity { > u32 *remap_info; > struct work_struct error_work; > @@ -884,16 +896,6 @@ struct i915_gem_mm { > */ > bool interruptible; > > - /** > - * Flag if the X Server, and thus DRM, is not currently in > - * control of the device. > - * > - * This is set between LeaveVT and EnterVT. It needs to be > - * replaced with a semaphore. It also needs to be > - * transitioned away from for kernel modesetting. > - */ > - int suspended; > - > /** Bit 6 swizzling required for X tiling */ > uint32_t bit_6_swizzle_x; > /** Bit 6 swizzling required for Y tiling */ > @@ -1187,6 +1189,8 @@ typedef struct drm_i915_private { > /* Old dri1 support infrastructure, beware the dragons ya fools entering > * here! */ > struct i915_dri1_state dri1; > + /* Old ums support infrastructure, same warning applies. */ > + struct i915_ums_state ums; > } drm_i915_private_t; > > /* Iterate over initialised rings */ > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index af61be8..5ad9f35 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2082,7 +2082,7 @@ int __i915_add_request(struct intel_ring_buffer *ring, > trace_i915_gem_request_add(ring, request->seqno); > ring->outstanding_lazy_request = 0; > > - if (!dev_priv->mm.suspended) { > + if (!dev_priv->ums.mm_suspended) { > if (i915_enable_hangcheck) { > mod_timer(&dev_priv->gpu_error.hangcheck_timer, > round_jiffies_up(jiffies + > DRM_I915_HANGCHECK_JIFFIES)); > @@ -2387,7 +2387,7 @@ i915_gem_retire_work_handler(struct work_struct *work) > idle &= list_empty(&ring->request_list); > } > > - if (!dev_priv->mm.suspended && !idle) > + if (!dev_priv->ums.mm_suspended && !idle) > queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, > round_jiffies_up_relative(HZ)); > if (idle) > @@ -3983,7 +3983,7 @@ i915_gem_idle(struct drm_device *dev) > > mutex_lock(&dev->struct_mutex); > > - if (dev_priv->mm.suspended) { > + if (dev_priv->ums.mm_suspended) { > mutex_unlock(&dev->struct_mutex); > return 0; > } > @@ -3999,11 +3999,6 @@ i915_gem_idle(struct drm_device *dev) > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > i915_gem_evict_everything(dev); > > - /* Hack! Don't let anybody do execbuf while we don't control the chip. > - * We need to replace this with a semaphore, or something. > - * And not confound mm.suspended! > - */ > - dev_priv->mm.suspended = 1; > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > > i915_kernel_lost_context(dev); > @@ -4220,7 +4215,7 @@ int > i915_gem_entervt_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > - drm_i915_private_t *dev_priv = dev->dev_private; > + struct drm_i915_private *dev_priv = dev->dev_private; > int ret; > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > @@ -4232,7 +4227,7 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void > *data, > } > > mutex_lock(&dev->struct_mutex); > - dev_priv->mm.suspended = 0; > + dev_priv->ums.mm_suspended = 0; > > ret = i915_gem_init_hw(dev); > if (ret != 0) { > @@ -4252,7 +4247,7 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void > *data, > cleanup_ringbuffer: > mutex_lock(&dev->struct_mutex); > i915_gem_cleanup_ringbuffer(dev); > - dev_priv->mm.suspended = 1; > + dev_priv->ums.mm_suspended = 1; > mutex_unlock(&dev->struct_mutex); > > return ret; > @@ -4262,11 +4257,23 @@ int > i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > + struct drm_i915_private *dev_priv = dev->dev_private; > + int ret; > + > if (drm_core_check_feature(dev, DRIVER_MODESET)) > return 0; > > drm_irq_uninstall(dev); > - return i915_gem_idle(dev); > + ret = i915_gem_idle(dev); > + > + /* Hack! Don't let anybody do execbuf while we don't control the chip. > + * We need to replace this with a semaphore, or something. > + * And not confound ums.mm_suspended! > + */ > + if (ret != 0) > + dev_priv->ums.mm_suspended = 1; ARGH. The locking, it hurts. Please never show this code in public again. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
