Skip to contentSkip to conversation

drm/drm_vblank: Change EINVAL by the correct errno

PATCH Superseded Rodrigo Siqueira 2 participants 2 messages Jun 27, 2019

Subsystems (3)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v5 1/1 drm/drm_vblank: Change EINVAL by the correct errno

For historical reasons, the function drm_wait_vblank_ioctl always return -EINVAL if something gets wrong. This scenario limits the flexibility for the userspace to make detailed verification of any 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, drm_crtc_get_sequence_ioctl and drm_crtc_queue_sequence_ioctl, also returns EINVAL if vblank is not supported; this patch also changes the return value to EOPNOTSUPP in these functions. Lastly, these functions are 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 of "drmCrtcQueueSequence", "drmCrtcGetSequence", and "drmWaitVBlank" with the command git grep -n "STRING".
* Look in the git history of the project with the command git log -S<STRING>

None of the above projects validate the use of EINVAL when using drmWaitVBlank(), which make safe, at least for these projects, to change the return values. On the other hand, mesa and xserver project uses drmCrtcQueueSequence() and drmCrtcGetSequence(); this change is harmless for both projects.

Change since V4 (Daniel):
- Also return EOPNOTSUPP in drm_crtc_[get|queue]_sequence_ioctl

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

Cc Keith Packard <[email protected]>
Cc Maarten Lankhorst <[email protected]>
Cc Ville Syrjälä <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Daniel Vetter <[email protected]>
---
 drivers/gpu/drm/drm_vblank.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/drm_vblank.c +3 -3
index 603ab105125d..bd4ac834d3ef 100644
@@ -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;
@@ -1823,7 +1823,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
 		return -EOPNOTSUPP;
 
 	if (!dev->irq_enabled)
-		return -EINVAL;
+		return -EOPNOTSUPP;
 
 	crtc = drm_crtc_find(dev, file_priv, get_seq->crtc_id);
 	if (!crtc)
@@ -1881,7 +1881,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
 		return -EOPNOTSUPP;
 
 	if (!dev->irq_enabled)
-		return -EINVAL;
+		return -EOPNOTSUPP;
 
 	crtc = drm_crtc_find(dev, file_priv, queue_seq->crtc_id);
 	if (!crtc)
Signature · 1 line
2.21.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#1 lore raw

✗ Fi.CI.BAT: failure for drm/drm_vblank: Change EINVAL by the correct errno (rev5)

== Series Details ==

== Series Details ==

Series: drm/drm_vblank: Change EINVAL by the correct errno (rev5)
URL : https://patchwork.freedesktop.org/series/51147/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6370 -> Patchwork_13447
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with Patchwork_13447 absolutely need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in Patchwork_13447, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/

Possible new issues
-------------------

Here are the unknown changes that may have been introduced in Patchwork_13447:

### IGT changes ###

#### Possible regressions ####

* igt@i915_pm_rpm@module-reload:
- fi-cml-u: [PASS][1] -> [DMESG-WARN][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6370/fi-cml-u/igt@[email protected]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/fi-cml-u/igt@[email protected]

Known issues
------------

Here are the changes found in Patchwork_13447 that come from known issues:

### IGT changes ###

#### Possible fixes ####

* igt@gem_basic@create-close:
- fi-icl-u3: [DMESG-WARN][3] ([fdo#107724]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6370/fi-icl-u3/igt@[email protected]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/fi-icl-u3/igt@[email protected]

* igt@i915_selftest@live_blt:
- fi-skl-iommu: [INCOMPLETE][5] ([fdo#108602]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6370/fi-skl-iommu/igt@i915_selftest@live_blt.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/fi-skl-iommu/igt@i915_selftest@live_blt.html

* igt@kms_chamelium@dp-crc-fast:
- fi-cml-u2: [FAIL][7] ([fdo#110387]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6370/fi-cml-u2/igt@[email protected]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/fi-cml-u2/igt@[email protected]

* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [DMESG-WARN][9] ([fdo#102614]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6370/fi-hsw-peppy/igt@[email protected]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/fi-hsw-peppy/igt@[email protected]

[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
[fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387

Participating hosts (51 -> 45)
------------------------------

Additional (1): fi-skl-6700k2
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper

Build changes
-------------

* Linux: CI_DRM_6370 -> Patchwork_13447

CI_DRM_6370: 119db9d4a19578a69b923a9857fa0b0969ff52d9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5068: 15ad664534413628f06c0f172aac11598bfdb895 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13447: 5a1121d77377952bf7280ce9e88ae9c7f918d64f @ git://anongit.freedesktop.org/gfx-ci/linux

== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h

Kernel: arch/x86/boot/bzImage is ready (#1)
Building modules, stage 2.
MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined! scripts/Makefile.modpost:91: recipe for target '__modpost' failed make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2

== Linux commits ==

5a1121d77377 drm/drm_vblank: Change EINVAL by the correct errno

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13447/
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx