On Tue, Jun 18, 2019 at 11:07:50PM -0300, Rodrigo Siqueira wrote: > For historical reason, the function drm_wait_vblank_ioctl always return > -EINVAL if something gets wrong. This scenario limits the flexibility > for the userspace make detailed verification of the problem and take > some action. In particular, the validation of “if (!dev->irq_enabled)” > in the drm_wait_vblank_ioctl is responsible for checking if the driver > support vblank or not. If the driver does not support VBlank, the > function drm_wait_vblank_ioctl returns EINVAL which does not represent > the real issue; this patch changes this behavior by return EOPNOTSUPP. > Additionally, some operations are unsupported by this function, and > returns EINVAL; this patch also changes the return value to EOPNOTSUPP > in this case. Lastly, the function drm_wait_vblank_ioctl is invoked by > libdrm, which is used by many compositors; because of this, it is > important to check if this change breaks any compositor. In this sense, > the following projects were examined: > > * Drm-hwcomposer > * Kwin > * Sway > * Wlroots > * Wayland-core > * Weston > * Xorg (67 different drivers) > > For each repository the verification happened in three steps: > > * Update the main branch > * Look for any occurrence "drmWaitVBlank" with the command: > git grep -n "drmWaitVBlank" > * Look in the git history of the project with the command: > git log -SdrmWaitVBlank > > Finally, none of the above projects validate the use of EINVAL which > make safe, at least for these projects, to change the return values. > > Change since V3: > - Return EINVAL for _DRM_VBLANK_SIGNAL (Daniel) > > Change since V2: > Daniel Vetter and Chris Wilson > - Replace ENOTTY by EOPNOTSUPP > - Return EINVAL if the parameters are wrong > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Apologies for the confusion on the last time around. btw if someone tells you "r-b (or a-b) with these changes", then just apply the r-b/a-b tag next time around. Otherwise people will re-review the same thing over and over again. -Daniel > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx> > --- > drivers/gpu/drm/drm_vblank.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 603ab105125d..bed233361614 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -1582,7 +1582,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, > unsigned int flags, pipe, high_pipe; > > if (!dev->irq_enabled) > - return -EINVAL; > + return -EOPNOTSUPP; > > if (vblwait->request.type & _DRM_VBLANK_SIGNAL) > return -EINVAL; > -- > 2.21.0 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
