[Intel-gfx] [PATCH] drm: Differentiate the lack of an interface from invalid parameter

[Daniel Vetter]

On Wed, Sep 12, 2018 at 10:27 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> If the ioctl is not supported on a particular piece of HW/driver
> combination, report ENODEV so that it can be easily distinguished from
> both the lack of the ioctl and from a regular invalid parameter.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>

Hm, I thought the canonical errno for "ioctl doesn't apply to this
device" is ENOTTY? And ENODEV means that it applies, but atm nothing
plugged in, or device gone already.

I found a few more modeset ioctl:
- drm_mode_gamma_set_ioctl, drm_mode_gamma_get_ioctl
- drm_mode_getconnector
- drm_mode_getcrtc, drm_mode_setcrtc
- drm_mode_getencoder
- drm_mode_create_lease_ioctl, drm_mode_list_lessees_ioctl, ...

Ok now I stop looking through the grep thing, looks like we've been
using EINVAL consistently. I'm all for switching, it makes sense, but
I think we should at least try to be consistent across all kms ioctl.
I'm happy to r-b such a series.

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_framebuffer.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 6eaacd4eb8cc..eed6ad0fe84a 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -113,6 +113,9 @@ int drm_mode_addfb(struct drm_device *dev, struct drm_mode_fb_cmd *or,
>         struct drm_mode_fb_cmd2 r = {};
>         int ret;
>
> +       if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +               return -ENODEV;
> +
>         r.pixel_format = drm_mode_legacy_fb_format(or->bpp, or->depth);
>         if (r.pixel_format == DRM_FORMAT_INVALID) {
>                 DRM_DEBUG("bad {bpp:%d, depth:%d}\n", or->bpp, or->depth);
> @@ -352,7 +355,7 @@ int drm_mode_addfb2(struct drm_device *dev,
>         struct drm_framebuffer *fb;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
> -               return -EINVAL;
> +               return -ENODEV;
>
>         fb = drm_internal_framebuffer_create(dev, r, file_priv);
>         if (IS_ERR(fb))
> @@ -387,7 +390,7 @@ int drm_mode_addfb2_ioctl(struct drm_device *dev,
>                  * ADDFB.
>                  */
>                 DRM_DEBUG_KMS("addfb2 broken on bigendian");
> -               return -EINVAL;
> +               return -ENODEV;
>         }
>  #endif
>         return drm_mode_addfb2(dev, data, file_priv);
> @@ -432,7 +435,7 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id,
>         int found = 0;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
> -               return -EINVAL;
> +               return -ENODEV;
>
>         fb = drm_framebuffer_lookup(dev, file_priv, fb_id);
>         if (!fb)
> @@ -509,7 +512,7 @@ int drm_mode_getfb(struct drm_device *dev,
>         int ret;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
> -               return -EINVAL;
> +               return -ENODEV;
>
>         fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
>         if (!fb)
> @@ -582,7 +585,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
>         int ret;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
> -               return -EINVAL;
> +               return -ENODEV;
>
>         fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
>         if (!fb)
> --
> 2.19.0
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch