[Intel-gfx] [RFC 1/6] drm/i915: cover ioctls with runtime_get/put

[Paulo Zanoni]

2014/1/22  <naresh.kumar.kachhi at intel.com>:
> From: Naresh Kumar Kachhi <naresh.kumar.kachhi at intel.com>
>
> With runtime PM enabled, we need to make sure that all HW access
> are valid (i.e.  Gfx is in D0). Invalid accesses might end up in
> HW hangs. Ex. A hang is seen if display register is accessed on
> BYT while display power island is power gated.
>
> This patch is covering all the IOCTLs with get/put.
> TODO: limit runtime_get/put to IOCTLs that accesses HW
>
> Signed-off-by: Naresh Kumar Kachhi <naresh.kumar.kachhi at intel.com>

We believe the current upstream code is enough to get runtime PM
references whenever they are needed. Can you please point the cases
where we fail to do this?

Also, if you find a spot we don't cover, please also patch
intel-gpu-tools, adding new test cases to pm_pc8.c.

Thanks,
Paulo

> ---
>  drivers/gpu/drm/i915/i915_drv.c   | 21 ++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_ioc32.c | 11 +++++++++--
>  2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 82c4605..80965be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -845,6 +845,25 @@ i915_pci_remove(struct pci_dev *pdev)
>         drm_put_dev(dev);
>  }
>
> +static long i915_unlocked_ioctl(struct file *filp,
> +             unsigned int cmd, unsigned long arg)
> +{
> +       struct drm_file *file_priv = filp->private_data;
> +       struct drm_device *dev = file_priv->minor->dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       int ret;
> +
> +       /* Don't do anything if device is not ready */
> +       if (drm_device_is_unplugged(dev))
> +               return -ENODEV;
> +
> +       intel_runtime_pm_get(dev_priv);
> +       ret = drm_ioctl(filp, cmd, arg);
> +       intel_runtime_pm_put(dev_priv);
> +
> +       return ret;
> +}
> +
>  static int i915_pm_suspend(struct device *dev)
>  {
>         struct pci_dev *pdev = to_pci_dev(dev);
> @@ -970,7 +989,7 @@ static const struct file_operations i915_driver_fops = {
>         .owner = THIS_MODULE,
>         .open = drm_open,
>         .release = drm_release,
> -       .unlocked_ioctl = drm_ioctl,
> +       .unlocked_ioctl = i915_unlocked_ioctl,
>         .mmap = drm_gem_mmap,
>         .poll = drm_poll,
>         .read = drm_read,
> diff --git a/drivers/gpu/drm/i915/i915_ioc32.c b/drivers/gpu/drm/i915/i915_ioc32.c
> index 3c59584..bfc773e 100644
> --- a/drivers/gpu/drm/i915/i915_ioc32.c
> +++ b/drivers/gpu/drm/i915/i915_ioc32.c
> @@ -201,12 +201,17 @@ static drm_ioctl_compat_t *i915_compat_ioctls[] = {
>   */
>  long i915_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
> +       struct drm_file *file_priv = filp->private_data;
> +       struct drm_device *dev = file_priv->minor->dev;
>         unsigned int nr = DRM_IOCTL_NR(cmd);
>         drm_ioctl_compat_t *fn = NULL;
>         int ret;
>
> -       if (nr < DRM_COMMAND_BASE)
> -               return drm_compat_ioctl(filp, cmd, arg);
> +       intel_runtime_pm_get(dev->dev_private);
> +       if (nr < DRM_COMMAND_BASE) {
> +               ret = drm_compat_ioctl(filp, cmd, arg);
> +               goto out;
> +       }
>
>         if (nr < DRM_COMMAND_BASE + DRM_ARRAY_SIZE(i915_compat_ioctls))
>                 fn = i915_compat_ioctls[nr - DRM_COMMAND_BASE];
> @@ -216,6 +221,8 @@ long i915_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         else
>                 ret = drm_ioctl(filp, cmd, arg);
>
> +out:
> +       intel_runtime_pm_put(dev->dev_private);
>         return ret;
>  }
>  #endif
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni