[RFC PATCH] drm: split up drm_ioctl to allow drivers to hook into "core" functions

[Ilia Mirkin]

On Sun, Dec 31, 2017 at 1:15 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Currently there's no way to allow a driver to reimplement any ioctls
> from the drm core. This can be desirable to, e.g., override fixed format
> selection logic, without turning to a midlayer-like solution.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> I want drm_mode_addfb to pick a different format for depth=30 than the
> one it currently selects. Flipping it for all drivers would break a
> bunch of existing ones, so this enables a driver to take control.
>
> Alternatively I can stash something into drm_device which specifies the
> preferred depth=30 fb format. However from my cursory observations of
> dri-devel discussions, midlayering is seen as a problem and not a
> solution.

Ugh, of course this is turning into a disaster as well.
drm_mode_addfb2 isn't exported, so I have to copy all the
functionality into nouveau, or move it to drm_kms_helper or whatever.
The flag in drm_device approach is sounding a lot more palatable. Let
me know what the best way to proceed is.

This is all to fix a regression, btw, since previous to nouveau
becoming atomic this all worked fine -- it just looked at the
30bpp'ness of the fb and assumed. Now it actually cares about specific
formats, and we run into all these problems.

>
>  drivers/gpu/drm/drm_ioctl.c | 36 ++++++++++++++++++++++++------------
>  include/drm/drm_ioctl.h     |  2 ++
>  2 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 4aafe4802099..698d69c6db0a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -767,12 +767,7 @@ long drm_ioctl(struct file *filp,
>         struct drm_file *file_priv = filp->private_data;
>         struct drm_device *dev;
>         const struct drm_ioctl_desc *ioctl = NULL;
> -       drm_ioctl_t *func;
>         unsigned int nr = DRM_IOCTL_NR(cmd);
> -       int retcode = -EINVAL;
> -       char stack_kdata[128];
> -       char *kdata = NULL;
> -       unsigned int in_size, out_size, drv_size, ksize;
>         bool is_driver_ioctl;
>
>         dev = file_priv->minor->dev;
> @@ -784,16 +779,33 @@ long drm_ioctl(struct file *filp,
>
>         if (is_driver_ioctl) {
>                 /* driver ioctl */
> -               if (nr - DRM_COMMAND_BASE >= dev->driver->num_ioctls)
> -                       goto err_i1;
> -               ioctl = &dev->driver->ioctls[nr - DRM_COMMAND_BASE];
> +               if (nr - DRM_COMMAND_BASE < dev->driver->num_ioctls)
> +                       ioctl = &dev->driver->ioctls[nr - DRM_COMMAND_BASE];
>         } else {
>                 /* core ioctl */
> -               if (nr >= DRM_CORE_IOCTL_COUNT)
> -                       goto err_i1;
> -               ioctl = &drm_ioctls[nr];
> +               if (nr < DRM_CORE_IOCTL_COUNT)
> +                       ioctl = &drm_ioctls[nr];
>         }
>
> +       return __drm_ioctl(filp, cmd, arg, ioctl);
> +}
> +EXPORT_SYMBOL(drm_ioctl);
> +
> +long __drm_ioctl(struct file *filp,
> +                unsigned int cmd, unsigned long arg,
> +                const struct drm_ioctl_desc *ioctl)
> +{
> +       struct drm_file *file_priv = filp->private_data;
> +       drm_ioctl_t *func;
> +       unsigned int nr = DRM_IOCTL_NR(cmd);
> +       int retcode = -EINVAL;
> +       char stack_kdata[128];
> +       char *kdata = NULL;
> +       unsigned int in_size, out_size, drv_size, ksize;
> +
> +       if (!ioctl)
> +               goto err_i1;
> +
>         drv_size = _IOC_SIZE(ioctl->cmd);
>         out_size = in_size = _IOC_SIZE(cmd);
>         if ((cmd & ioctl->cmd & IOC_IN) == 0)
> @@ -851,7 +863,7 @@ long drm_ioctl(struct file *filp,
>                 DRM_DEBUG("ret = %d\n", retcode);
>         return retcode;
>  }
> -EXPORT_SYMBOL(drm_ioctl);
> +EXPORT_SYMBOL(__drm_ioctl);
>
>  /**
>   * drm_ioctl_flags - Check for core ioctl and return ioctl permission flags
> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> index add42809642a..e08f8ea66f2a 100644
> --- a/include/drm/drm_ioctl.h
> +++ b/include/drm/drm_ioctl.h
> @@ -172,6 +172,8 @@ struct drm_ioctl_desc {
>
>  int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
>  long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> +long __drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
> +                const struct drm_ioctl_desc *ioctl);
>  long drm_ioctl_kernel(struct file *, drm_ioctl_t, void *, u32);
>  #ifdef CONFIG_COMPAT
>  long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> --
> 2.13.6
>