On Wed, Nov 30, 2016 at 05:30:02PM +0900, Michel Dänzer wrote: > From: Michel Dänzer <michel.daenzer@xxxxxxx> > > This is an attempt to make the previous fix a bit more robust going > forward. > > Signed-off-by: Michel Dänzer <michel.daenzer@xxxxxxx> > --- > drivers/gpu/drm/drm_ioctl.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 71c3473..32f484b 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -229,6 +229,19 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ > struct drm_crtc *crtc; > > req->value = 0; > + > + /* Only allow non-KMS caps with non-KMS drivers */ > + switch (req->capability) { > + case DRM_CAP_DUMB_BUFFER: Dumb buffers are only meant to be used for kms drivers, should be disallowed too. > + case DRM_CAP_VBLANK_HIGH_CRTC: Might be good to have a comment here that we need to allow this for old ums? > + case DRM_CAP_PRIME: > + case DRM_CAP_TIMESTAMP_MONOTONIC: This is pretty new, I don't think any of the old ums drivers was ever updated to use it. Should probably disallow it too. > + break; > + default: > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return -ENOTSUPP; > + } And one code org bikeshed: I don't like the duplicated switch, could we instead split it it into two disjoint sets like this? switch (req->capability) { case DRM_CAP_PRIME: req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT : 0; req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT : 0; break; ... all other non-modeset caps ... } if (!drm_core_check_feature(dev, DRIVER_MODESET)) return -ENOTSUPP; switch (req->capability) { ... handle remaining caps needed for DRIVER_MODSET ... default: return -EINVAL; } That way it would be a bit more obvious that people who add a new cap need to make a decision where to put it (and by default put it in the bottom pile). -Daniel > switch (req->capability) { > case DRM_CAP_DUMB_BUFFER: > if (dev->driver->dumb_create) > @@ -254,12 +267,10 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ > req->value = dev->mode_config.async_page_flip; > break; > case DRM_CAP_PAGE_FLIP_TARGET: > - if (drm_core_check_feature(dev, DRIVER_MODESET)) { > - req->value = 1; > - drm_for_each_crtc(crtc, dev) { > - if (!crtc->funcs->page_flip_target) > - req->value = 0; > - } > + req->value = 1; > + drm_for_each_crtc(crtc, dev) { > + if (!crtc->funcs->page_flip_target) > + req->value = 0; > } > break; > case DRM_CAP_CURSOR_WIDTH: > -- > 2.10.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel