| Message ID | 20170418121120.31862-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Daniel, Thank you for the patch. On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote: > All the error codes we (ab)use are strictly not the right ones (since > they're all for the vfs, and the only thing we're allowed to do from > an ioctl is EINVAL). But ENOENT is the common error code for failed to > look up an object throughout drm, so let's use it in the cma helpers, > too. Regardless of which is best, it's true that ENOENT is used through the DRM code, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Someone should however mention that this changes the userspace API. I'll let you decide whether to ignore that comment :-) > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7 > 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -189,7 +189,7 @@ struct drm_framebuffer > *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj = > drm_gem_object_lookup(file_priv, mode_cmd->handles[i]); > if (!obj) { > dev_err(dev->dev, "Failed to lookup GEM object\n"); > - ret = -ENXIO; > + ret = -ENOENT; > goto err_gem_object_put; > }
On Tue, Apr 18, 2017 at 03:29:29PM +0300, Laurent Pinchart wrote: > Hi Daniel, > > Thank you for the patch. > > On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote: > > All the error codes we (ab)use are strictly not the right ones (since > > they're all for the vfs, and the only thing we're allowed to do from > > an ioctl is EINVAL). But ENOENT is the common error code for failed to > > look up an object throughout drm, so let's use it in the cma helpers, > > too. > > Regardless of which is best, it's true that ENOENT is used through the DRM > code, so > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Someone should however mention that this changes the userspace API. I'll let > you decide whether to ignore that comment :-) Yeah, let's make sure we don't break any existing userspaces (a la pulseaudio). Sean > > > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > > b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7 > > 100644 > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > > @@ -189,7 +189,7 @@ struct drm_framebuffer > > *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj = > > drm_gem_object_lookup(file_priv, mode_cmd->handles[i]); > > if (!obj) { > > dev_err(dev->dev, "Failed to lookup GEM object\n"); > > - ret = -ENXIO; > > + ret = -ENOENT; > > goto err_gem_object_put; > > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Apr 18, 2017 at 10:40:21AM -0400, Sean Paul wrote: > On Tue, Apr 18, 2017 at 03:29:29PM +0300, Laurent Pinchart wrote: > > Hi Daniel, > > > > Thank you for the patch. > > > > On Tuesday 18 Apr 2017 14:11:20 Daniel Vetter wrote: > > > All the error codes we (ab)use are strictly not the right ones (since > > > they're all for the vfs, and the only thing we're allowed to do from > > > an ioctl is EINVAL). But ENOENT is the common error code for failed to > > > look up an object throughout drm, so let's use it in the cma helpers, > > > too. > > > > Regardless of which is best, it's true that ENOENT is used through the DRM > > code, so > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Someone should however mention that this changes the userspace API. I'll let > > you decide whether to ignore that comment :-) > > Yeah, let's make sure we don't break any existing userspaces (a la pulseaudio). Good point, but I think this is safe. I augmented the commit message and merged the patch with Sean's irc r-b (plus Laurent's r-b from here). -Daniel > > Sean > > > > > > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > --- > > > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c > > > b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7 > > > 100644 > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > > > @@ -189,7 +189,7 @@ struct drm_framebuffer > > > *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj = > > > drm_gem_object_lookup(file_priv, mode_cmd->handles[i]); > > > if (!obj) { > > > dev_err(dev->dev, "Failed to lookup GEM object\n"); > > > - ret = -ENXIO; > > > + ret = -ENOENT; > > > goto err_gem_object_put; > > > } > > > > -- > > Regards, > > > > Laurent Pinchart > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Sean Paul, Software Engineer, Google / Chromium OS
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index d2b77b02830d..53f9bdf470d7 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -189,7 +189,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]); if (!obj) { dev_err(dev->dev, "Failed to lookup GEM object\n"); - ret = -ENXIO; + ret = -ENOENT; goto err_gem_object_put; }
All the error codes we (ab)use are strictly not the right ones (since they're all for the vfs, and the only thing we're allowed to do from an ioctl is EINVAL). But ENOENT is the common error code for failed to look up an object throughout drm, so let's use it in the cma helpers, too. Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)