On Wed, Nov 30, 2016 at 4:07 AM, Daniel Vetter <daniel@xxxxxxxx> wrote: > 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? > I don't think we need this for UMS. It was added for evergreen and we only supported this feature on KMS. Alex >> + 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel