Deprecate struct drm_device.irq_enabled
Subsystems (29)
Thomas Zimmermann
maintainerv2 cover Deprecate struct drm_device.irq_enabled
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core.
KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs.
This used to be a single patch, [1] but it's now a full series.
The first 3 patches replace instances of irq_enabled that are not required.
Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs.
THe rest of the patchset removes irq_enabled from all non-legacy drivers. The only exception is omapdrm, which has an internal dpendency on the field's value. For this drivers, the state gets duplicated internally.
With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy.
v2:
* keep the original test for legacy drivers in drm_wait_vblank_ioctl() (Daniel)
[1] https://lore.kernel.org/dri-devel/[email protected]/
Thomas Zimmermann (22):
drm/amdgpu: Track IRQ state in local device state drm/hibmc: Call drm_irq_uninstall() unconditionally drm/radeon: Track IRQ state in local device state
drm: Don't test for IRQ support in VBLANK ioctls drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabled drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enabled
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 ---------- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ------ drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 ++++++++-------- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ------ drivers/gpu/drm/sti/sti_compositor.c | 2 -- drivers/gpu/drm/stm/ltdc.c | 3 --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- drivers/gpu/drm/tegra/drm.c | 7 ------- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 -------- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- drivers/gpu/drm/zte/zx_drm_drv.c | 6 ------ 26 files changed, 30 insertions(+), 111 deletions(-)
base-commit: 8c1323b422f8473421682ba783b5949ddd89a3f4
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 --
2.32.0
Thomas Zimmermann
maintainerPATCH v2 1/22 drm/amdgpu: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's own state field struct amdgpu_device.irq.installed. The field in the DRM device structure is considered legacy and should not be used by KMS drivers.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +3 -3
@@ -599,7 +599,7 @@ void amdgpu_irq_gpu_reset_resume_helper(struct amdgpu_device *adev)int amdgpu_irq_get (struct amdgpu_device *adev,struct amdgpu_irq_src *src,unsigned type) { -if (!adev_to_drm (adev)->irq_enabled) +if (!adev->irq.installed)return -ENOENT ;if (type >= src->num_types)
@@ -629,7 +629,7 @@ int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src,int amdgpu_irq_put (struct amdgpu_device *adev,struct amdgpu_irq_src *src,unsigned type) { -if (!adev_to_drm (adev)->irq_enabled) +if (!adev->irq.installed)return -ENOENT ;if (type >= src->num_types)
@@ -660,7 +660,7 @@ int amdgpu_irq_put(struct amdgpu_device *adev, struct amdgpu_irq_src *src,bool amdgpu_irq_enabled (struct amdgpu_device *adev,struct amdgpu_irq_src *src,unsigned type) { -if (!adev_to_drm (adev)->irq_enabled) +if (!adev->irq.installed)return false;if (type >= src->num_types)
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 2/22 drm/hibmc: Call drm_irq_uninstall() unconditionally
In reply to Thomas ZimmermannRemove the check around drm_irq_uninstall(). The same test is done by the function internally. The tested state in irq_enabled is considered obsolete and should not be used by KMS drivers.
--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +1 -2
@@ -253,8 +253,7 @@ static int hibmc_unload(struct drm_device *dev) {drm_atomic_helper_shutdown (dev); -if (dev->irq_enabled) -drm_irq_uninstall (dev); +drm_irq_uninstall (dev);pci_disable_msi (to_pci_dev (dev->dev));
Signature · 1 line
2.32.0
答复: [PATCH v2 02/22] drm/hibmc: Call drm_irq_uninstall() unconditionally
-----邮件原件-----
发件人: Thomas Zimmermann [mailto:[email protected]]
发送时间: 2021年6月22日 22:10
收件人: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; tiantao (H) <[email protected]>; [email protected]; kongxinwei (A) <[email protected]>; Chenfeng (puck) <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] 抄送: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Thomas Zimmermann <[email protected]>
主题: [PATCH v2 02/22] drm/hibmc: Call drm_irq_uninstall() unconditionally
Remove the check around drm_irq_uninstall(). The same test is done by the function internally. The tested state in irq_enabled is considered obsolete and should not be used by KMS drivers.
--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +1 -3
@@ -253,8 +253,7 @@ static int hibmc_unload(struct drm_device *dev) {drm_atomic_helper_shutdown (dev); -if (dev->irq_enabled) -drm_irq_uninstall (dev); +drm_irq_uninstall (dev);pci_disable_msi (to_pci_dev (dev->dev)); --
2.32.0
Thomas Zimmermann
maintainerPATCH v2 3/22 drm/radeon: Track IRQ state in local device state
In reply to Thomas ZimmermannReplace usage of struct drm_device.irq_enabled with the driver's own state field struct radeon_device.irq.installed. The field in the DRM device structure is considered legacy and should not be used by KMS drivers.
--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/gpu/drm/radeon/radeon_fence.c +1 -1
@@ -288,7 +288,7 @@ static void radeon_fence_check_lockup(struct work_struct *work)return ; } -if (fence_drv->delayed_irq && rdev->ddev->irq_enabled) { +if (fence_drv->delayed_irq && rdev->irq.installed) {unsigned long irqflags; fence_drv->delayed_irq = false;
drivers/gpu/drm/radeon/radeon_irq_kms.c +8 -8
@@ -357,7 +357,7 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev, int ring) {unsigned long irqflags; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;if (atomic_inc_return (&rdev->irq.ring_int[ring]) ==1 ) {
@@ -396,7 +396,7 @@ void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring) {unsigned long irqflags; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;if (atomic_dec_and_test (&rdev->irq.ring_int[ring])) {
@@ -422,7 +422,7 @@ void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc)if (crtc <0 || crtc >= rdev->num_crtc)return ; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;if (atomic_inc_return (&rdev->irq.pflip[crtc]) ==1 ) {
@@ -448,7 +448,7 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc)if (crtc <0 || crtc >= rdev->num_crtc)return ; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;if (atomic_dec_and_test (&rdev->irq.pflip[crtc])) {
@@ -470,7 +470,7 @@ void radeon_irq_kms_enable_afmt(struct radeon_device *rdev, int block) {unsigned long irqflags; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;spin_lock_irqsave (&rdev->irq.lock, irqflags);
@@ -492,7 +492,7 @@ void radeon_irq_kms_disable_afmt(struct radeon_device *rdev, int block) {unsigned long irqflags; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;spin_lock_irqsave (&rdev->irq.lock, irqflags);
@@ -514,7 +514,7 @@ void radeon_irq_kms_enable_hpd(struct radeon_device *rdev, unsigned hpd_mask)unsigned long irqflags;int i; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;spin_lock_irqsave (&rdev->irq.lock, irqflags);
@@ -537,7 +537,7 @@ void radeon_irq_kms_disable_hpd(struct radeon_device *rdev, unsigned hpd_mask)unsigned long irqflags;int i; -if (!rdev->ddev->irq_enabled) +if (!rdev->irq.installed)return ;spin_lock_irqsave (&rdev->irq.lock, irqflags);
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 4/22 drm: Don't test for IRQ support in VBLANK ioctls
In reply to Thomas ZimmermannFor KMS drivers, replace the IRQ check in VBLANK ioctls with a check for vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state for non-legacy drivers. For legacy drivers with userspace modesetting, the original test remains in drm_wait_vblank_ioctl().
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
--- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 11 deletions(-)
drivers/gpu/drm/drm_irq.c +3 -7
@@ -74,10 +74,8 @@* only supports devices with a single interrupt on the main device stored in * &drm_device.dev and set as the device paramter in drm_dev_alloc(). * -* These IRQ helpers are strictly optional. Drivers which roll their own only -* need to set &drm_device.irq_enabled to signal the DRM core that vblank -* interrupts are working. Since these helpers don't automatically clean up the -* requested interrupt like e.g. devm_request_irq() they're not really +* These IRQ helpers are strictly optional. Since these helpers don't automatically +* clean up the requested interrupt like e.g. devm_request_irq() they're not really * recommended. */
@@ -91,9 +89,7 @@* and after the installation. * * This is the simplified helper interface provided for drivers with no special -* needs. Drivers which need to install interrupt handlers for multiple -* interrupts must instead set &drm_device.irq_enabled to signal the DRM core -* that vblank interrupts are available. +* needs. * * @irq must match the interrupt number that would be passed to request_irq(), * if called directly instead of using this helper function.
drivers/gpu/drm/drm_vblank.c +9 -4
@@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,unsigned int pipe_index;unsigned int flags, pipe, high_pipe; -if (!dev->irq_enabled) -return -EOPNOTSUPP ; +if (drm_core_check_feature (dev,DRIVER_MODESET )) { +if (!drm_dev_has_vblank (dev)) +return -EOPNOTSUPP ; + }else { +if (!dev->irq_enabled) +return -EOPNOTSUPP ; + }if (vblwait->request.type & _DRM_VBLANK_SIGNAL)return -EINVAL ;
@@ -2023,7 +2028,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_MODESET ))return -EOPNOTSUPP ; -if (!dev->irq_enabled) +if (!drm_dev_has_vblank (dev))return -EOPNOTSUPP ; crtc =drm_crtc_find (dev, file_priv, get_seq->crtc_id);
@@ -2082,7 +2087,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_MODESET ))return -EOPNOTSUPP ; -if (!dev->irq_enabled) +if (!drm_dev_has_vblank (dev))return -EOPNOTSUPP ; crtc =drm_crtc_find (dev, file_priv, queue_seq->crtc_id);
Signature · 1 line
2.32.0
Liviu Dudau
maintainerRe: [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hello,
Quoted 52 lines · Thomas Zimmermann
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state for non-legacy drivers. For legacy drivers with userspace modesetting, the original test remains in
drm_wait_vblank_ioctl().v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl()(Daniel)Signed-off-by Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 11 deletions(-)drivers/gpu/drm/drm_irq.c +3
-7@@ -74,10 +74,8 @@* only supports devices with a single interrupt on the main device stored in * &drm_device.dev and set as the device paramter in drm_dev_alloc(). * -* These IRQ helpers are strictly optional. Drivers which roll their own only -* need to set &drm_device.irq_enabled to signal the DRM core that vblank -* interrupts are working. Since these helpers don't automatically clean up the -* requested interrupt like e.g. devm_request_irq() they're not really +* These IRQ helpers are strictly optional. Since these helpers don't automatically +* clean up the requested interrupt like e.g. devm_request_irq() they're not really * recommended. */ @@ -91,9 +89,7 @@* and after the installation. * * This is the simplified helper interface provided for drivers with no special -* needs. Drivers which need to install interrupt handlers for multiple -* interrupts must instead set &drm_device.irq_enabled to signal the DRM core -* that vblank interrupts are available. +* needs. * * @irq must match the interrupt number that would be passed to request_irq(), * if called directly instead of using this helper function. drivers/gpu/drm/drm_vblank.c +0
-0@@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,unsigned int pipe_index;unsigned int flags, pipe, high_pipe;
drivers/gpu/drm/drm_vblank.c int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, · line 1757
Quoted 10 lines
drivers/gpu/drm/drm_vblank.c +7
-2-if (!dev->irq_enabled) -return -EOPNOTSUPP ; +if (drm_core_check_feature (dev,DRIVER_MODESET )) { +if (!drm_dev_has_vblank (dev)) +return -EOPNOTSUPP ; + }else { +if (!dev->irq_enabled) +return -EOPNOTSUPP ; + }
For a system call that is used quite a lot by userspace we have increased the code size in a noticeable way. Can we not cache it privately?
Best regards,
Liviu
Quoted 24 lines
drivers/gpu/drm/drm_vblank.c +2
-2if (vblwait->request.type & _DRM_VBLANK_SIGNAL)return -EINVAL ;@@ -2023,7 +2028,7 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_MODESET ))return -EOPNOTSUPP ; -if (!dev->irq_enabled) +if (!drm_dev_has_vblank (dev))return -EOPNOTSUPP ; crtc =drm_crtc_find (dev, file_priv, get_seq->crtc_id);@@ -2082,7 +2087,7 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,if (!drm_core_check_feature (dev,DRIVER_MODESET ))return -EOPNOTSUPP ; -if (!dev->irq_enabled) +if (!drm_dev_has_vblank (dev))return -EOPNOTSUPP ; crtc =drm_crtc_find (dev, file_priv, queue_seq->crtc_id);Signature · 1 line
2.32.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Thomas Zimmermann
maintainerRe: [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hi Liviu
Am 22.06.21 um 17:25 schrieb Liviu Dudau:
Quoted 56 lines
Hello,
Quoted 53 lines · Thomas Zimmermann
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state for non-legacy drivers. For legacy drivers with userspace modesetting, the original test remains in
drm_wait_vblank_ioctl().v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl()(Daniel)Signed-off-by Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 11 deletions(-)drivers/gpu/drm/drm_irq.c +3
-7@@ -74,10 +74,8 @@* only supports devices with a single interrupt on the main device stored in * &drm_device.dev and set as the device paramter in drm_dev_alloc(). * -* These IRQ helpers are strictly optional. Drivers which roll their own only -* need to set &drm_device.irq_enabled to signal the DRM core that vblank -* interrupts are working. Since these helpers don't automatically clean up the -* requested interrupt like e.g. devm_request_irq() they're not really +* These IRQ helpers are strictly optional. Since these helpers don't automatically +* clean up the requested interrupt like e.g. devm_request_irq() they're not really * recommended. */ @@ -91,9 +89,7 @@* and after the installation. * * This is the simplified helper interface provided for drivers with no special -* needs. Drivers which need to install interrupt handlers for multiple -* interrupts must instead set &drm_device.irq_enabled to signal the DRM core -* that vblank interrupts are available. +* needs. * * @irq must match the interrupt number that would be passed to request_irq(), * if called directly instead of using this helper function. drivers/gpu/drm/drm_vblank.c +0
-0@@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,unsigned int pipe_index;unsigned int flags, pipe, high_pipe;
Quoted 12 lines
drivers/gpu/drm/drm_vblank.c int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, · line 1757
Quoted 9 lines
drivers/gpu/drm/drm_vblank.c +7
-2-if (!dev->irq_enabled) -return -EOPNOTSUPP ; +if (drm_core_check_feature (dev,DRIVER_MODESET )) { +if (!drm_dev_has_vblank (dev)) +return -EOPNOTSUPP ; + }else { +if (!dev->irq_enabled) +return -EOPNOTSUPP ; + }For a system call that is used quite a lot by userspace we have increased the code size in a noticeable way. Can we not cache it privately?
I'm not quite sure that I understand your concern. The additionally called functions are trivial one-liners; probably inlined anyway.
However, irq_enabled is only relevant for legacy drivers and will eventually disappear behind CONFIG_DRM_LEGACY. We can rewrite the test like this:
ifdef CONFIG_DRM_LEGACY
if (unlikely(check_feature(dev, DRIVER_LEGACY))) {
if (!irq_enabled)
return;
} else
#endif
{
if (!has_vblank_support(dev))
return;}
As CONFIG_DRM_LEGACY is most likely disabled on concurrent systems, we'd get a single test for the modern drivers. If DRM_LEGACYis on, the compiler at least knows that the else branch is preferred.
Best regards
Thomas
Signature · 6 lines
Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Liviu Dudau
maintainerRe: [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hi Thomas,
Quoted 70 lines · Thomas Zimmermann
Hi Liviu
Am 22.06.21 um 17:25 schrieb Liviu Dudau:
Quoted 58 lines
Hello,
Quoted 55 lines · Thomas Zimmermann
For KMS drivers, replace the IRQ check in VBLANK ioctls with a check for vblank support. IRQs might be enabled wthout vblanking being supported.
This change also removes the DRM framework's only dependency on IRQ state for non-legacy drivers. For legacy drivers with userspace modesetting, the original test remains in
drm_wait_vblank_ioctl().v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl()(Daniel)Signed-off-by Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 11 deletions(-)drivers/gpu/drm/drm_irq.c +3
-7@@ -74,10 +74,8 @@* only supports devices with a single interrupt on the main device stored in * &drm_device.dev and set as the device paramter in drm_dev_alloc(). * -* These IRQ helpers are strictly optional. Drivers which roll their own only -* need to set &drm_device.irq_enabled to signal the DRM core that vblank -* interrupts are working. Since these helpers don't automatically clean up the -* requested interrupt like e.g. devm_request_irq() they're not really +* These IRQ helpers are strictly optional. Since these helpers don't automatically +* clean up the requested interrupt like e.g. devm_request_irq() they're not really * recommended. */ @@ -91,9 +89,7 @@* and after the installation. * * This is the simplified helper interface provided for drivers with no special -* needs. Drivers which need to install interrupt handlers for multiple -* interrupts must instead set &drm_device.irq_enabled to signal the DRM core -* that vblank interrupts are available. +* needs. * * @irq must match the interrupt number that would be passed to request_irq(), * if called directly instead of using this helper function. drivers/gpu/drm/drm_vblank.c +2
-2@@ -1748,8 +1748,13 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,unsigned int pipe_index;unsigned int flags, pipe, high_pipe; -if (!dev->irq_enabled) -return -EOPNOTSUPP ; +if (drm_core_check_feature (dev,DRIVER_MODESET )) { +if (!drm_dev_has_vblank (dev))Quoted 8 lines
drivers/gpu/drm/drm_vblank.c int drm_wait_vblank_ioctl(struct drm_device *dev, void *data, · line 1757
Quoted 5 lines
drivers/gpu/drm/drm_vblank.c +5
-0+return -EOPNOTSUPP ; + }else { +if (!dev->irq_enabled) +return -EOPNOTSUPP ; + }For a system call that is used quite a lot by userspace we have increased the code size in a noticeable way. Can we not cache it privately?
Quoted 2 lines
I'm not quite sure that I understand your concern. The additionally called functions are trivial one-liners; probably inlined anyway.
They are inlined. However we replace the pointer dereference (which can be calculated at compile time as offset from a base pointer) with the code in drm_core_check_all_features() that does 3 pointer dereferences, masking and logical AND before checking for matching value.
Quoted 3 lines
However,
irq_enabledis only relevant for legacy drivers and will eventually disappear behindCONFIG_DRM_LEGACY. We can rewrite the test like this:
I get the point that irq_enabled is legacy. However the IOCTL call is not and usually is used in time critical code to wait for vblank before starting the old buffers for a new frame. At 60Hz that's probably less of a concern, but at 120Hz refresh rate and reduced vblank time your time slice allocation for new work matters.
Best regards,
Liviu
Quoted 27 lines
ifdef
CONFIG_DRM_LEGACYif (unlikely(check_feature(dev, DRIVER_LEGACY))) { if (!irq_enabled) return; } else #endif { if (!has_vblank_support(dev)) return;}
As
CONFIG_DRM_LEGACYis most likely disabled on concurrent systems, we'd get a single test for the modern drivers. IfDRM_LEGACYison, the compiler at least knows that the else branch is preferred.Best regards
ThomasSignature · 6 lines
Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Thomas Zimmermann
maintainerPATCH v2 5/22 drm/komeda: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in komeda.
--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/gpu/drm/arm/display/komeda/komeda_kms.c +0 -4
@@ -301,8 +301,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev)if (err) goto free_component_binding; - drm->irq_enabled = true; -drm_kms_helper_poll_init (drm); err =drm_dev_register (drm,0 );
@@ -313,7 +311,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) free_interrupts:drm_kms_helper_poll_fini (drm); - drm->irq_enabled = false; free_component_binding:component_unbind_all (mdev->dev, drm); cleanup_mode_config:
@@ -331,7 +328,6 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)drm_dev_unregister (drm);drm_kms_helper_poll_fini (drm);drm_atomic_helper_shutdown (drm); - drm->irq_enabled = false;component_unbind_all (mdev->dev, drm);drm_mode_config_cleanup (drm);komeda_kms_cleanup_private_objs (kms);
Signature · 1 line
2.32.0
Re: [PATCH v2 05/22] drm/komeda: Don't set struct drm_device.irq_enabled
Quoted 4 lines · Thomas Zimmermann
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in komeda.
Signed-off-by Thomas Zimmermann <[email protected]>
Best regards,
Liviu
Quoted 36 lines
--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- 1 file changed, 4 deletions(-)drivers/gpu/drm/arm/display/komeda/komeda_kms.c +0
-4@@ -301,8 +301,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev)if (err) goto free_component_binding; - drm->irq_enabled = true; -drm_kms_helper_poll_init (drm); err =drm_dev_register (drm,0 );@@ -313,7 +311,6 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) free_interrupts:drm_kms_helper_poll_fini (drm); - drm->irq_enabled = false; free_component_binding:component_unbind_all (mdev->dev, drm); cleanup_mode_config:@@ -331,7 +328,6 @@ void komeda_kms_detach(struct komeda_kms_dev *kms)drm_dev_unregister (drm);drm_kms_helper_poll_fini (drm);drm_atomic_helper_shutdown (drm); - drm->irq_enabled = false;component_unbind_all (mdev->dev, drm);drm_mode_config_cleanup (drm);komeda_kms_cleanup_private_objs (kms);Signature · 1 line
2.32.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Thomas Zimmermann
maintainerPATCH v2 6/22 drm/malidp: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in malidp.
--- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/gpu/drm/arm/malidp_drv.c +0 -4
@@ -847,8 +847,6 @@ static int malidp_bind(struct device *dev)if (ret <0 ) goto irq_init_fail; - drm->irq_enabled = true; - ret =drm_vblank_init (drm, drm->mode_config.num_crtc);if (ret <0 ) {DRM_ERROR ("failed to initialise vblank\n" );
@@ -874,7 +872,6 @@ static int malidp_bind(struct device *dev) vblank_fail:malidp_se_irq_fini (hwdev);malidp_de_irq_fini (hwdev); - drm->irq_enabled = false; irq_init_fail:drm_atomic_helper_shutdown (drm);component_unbind_all (dev, drm);
@@ -909,7 +906,6 @@ static void malidp_unbind(struct device *dev)drm_atomic_helper_shutdown (drm);malidp_se_irq_fini (hwdev);malidp_de_irq_fini (hwdev); - drm->irq_enabled = false;component_unbind_all (dev, drm);of_node_put (malidp->crtc.port); malidp->crtc.port =NULL ;
Signature · 1 line
2.32.0
Re: [PATCH v2 06/22] drm/malidp: Don't set struct drm_device.irq_enabled
Quoted 4 lines · Thomas Zimmermann
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in malidp.
Signed-off-by Thomas Zimmermann <[email protected]>
Best regards,
Liviu
Quoted 36 lines
--- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- 1 file changed, 4 deletions(-)drivers/gpu/drm/arm/malidp_drv.c +0
-4@@ -847,8 +847,6 @@ static int malidp_bind(struct device *dev)if (ret <0 ) goto irq_init_fail; - drm->irq_enabled = true; - ret =drm_vblank_init (drm, drm->mode_config.num_crtc);if (ret <0 ) {DRM_ERROR ("failed to initialise vblank\n" );@@ -874,7 +872,6 @@ static int malidp_bind(struct device *dev) vblank_fail:malidp_se_irq_fini (hwdev);malidp_de_irq_fini (hwdev); - drm->irq_enabled = false; irq_init_fail:drm_atomic_helper_shutdown (drm);component_unbind_all (dev, drm);@@ -909,7 +906,6 @@ static void malidp_unbind(struct device *dev)drm_atomic_helper_shutdown (drm);malidp_se_irq_fini (hwdev);malidp_de_irq_fini (hwdev); - drm->irq_enabled = false;component_unbind_all (dev, drm);of_node_put (malidp->crtc.port); malidp->crtc.port =NULL ;Signature · 1 line
2.32.0
Signature · 8 lines
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯Thomas Zimmermann
maintainerPATCH v2 7/22 drm/exynos: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in exynos.
--- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 ---------- 1 file changed, 10 deletions(-)
drivers/gpu/drm/exynos/exynos_drm_drv.c +0 -10
@@ -300,16 +300,6 @@ static int exynos_drm_bind(struct device *dev)drm_mode_config_reset (drm); -/* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -* -* P.S. note that we wouldn't use drm irq handler but -* just specific driver own one instead because -* drm framework supports only one irq handler. -*/ - drm->irq_enabled = true; -/* init kms poll for handling hpd */ drm_kms_helper_poll_init (drm);
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 8/22 drm/kirin: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in kirin.
--- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- 1 file changed, 2 deletions(-)
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c +0 -2
@@ -185,8 +185,6 @@ static int kirin_drm_kms_init(struct drm_device *dev,DRM_ERROR ("failed to initialize vblank.\n" ); goto err_unbind_all; } -/* with irq_enabled = true, we can use the vblank feature. */ - dev->irq_enabled = true;/* reset all the states of crtc/plane/encoder/connector */ drm_mode_config_reset (dev);
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 9/22 drm/imx: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in imx.
--- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- 2 files changed, 14 deletions(-)
drivers/gpu/drm/imx/dcss/dcss-kms.c +0 -3
@@ -133,8 +133,6 @@ struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss)if (ret) goto cleanup_mode_config; - drm->irq_enabled = true; - ret =dcss_kms_bridge_connector_init (kms);if (ret) goto cleanup_mode_config;
@@ -178,7 +176,6 @@ void dcss_kms_detach(struct dcss_kms_dev *kms)drm_kms_helper_poll_fini (drm);drm_atomic_helper_shutdown (drm);drm_crtc_vblank_off (&kms->crtc.base); - drm->irq_enabled = false;drm_mode_config_cleanup (drm);dcss_crtc_deinit (&kms->crtc, drm); drm->dev_private =NULL ;
drivers/gpu/drm/imx/imx-drm-core.c +0 -11
@@ -207,17 +207,6 @@ static int imx_drm_bind(struct device *dev)if (IS_ERR (drm))return PTR_ERR (drm); -/* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -* -* P.S. note that we wouldn't use drm irq handler but -* just specific driver own one instead because -* drm framework supports only one irq handler and -* drivers can well take care of their interrupts -*/ - drm->irq_enabled = true; -/* * set max width and height as default value(4096x4096). * this value would be used to check framebuffer size limitation
Signature · 1 line
2.32.0
Re: [PATCH v2 09/22] drm/imx: Don't set struct drm_device.irq_enabled
Hi Thomas,
Quoted 6 lines · Thomas Zimmermann
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in imx.
Signed-off-by Thomas Zimmermann <[email protected]>--- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 ---
Not sure if it's worth the effort but, since DCSS is a completely self-contained driver, maybe it would be good to split this patch in 2 as well.
Anyway, for DCSS bit:
Thanks,
laurentiu
Quoted 49 lines
drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- 2 files changed, 14 deletions(-)drivers/gpu/drm/imx/dcss/dcss-kms.c +0
-3@@ -133,8 +133,6 @@ struct dcss_kms_dev *dcss_kms_attach(struct dcss_dev *dcss)if (ret) goto cleanup_mode_config; - drm->irq_enabled = true; - ret =dcss_kms_bridge_connector_init (kms);if (ret) goto cleanup_mode_config;@@ -178,7 +176,6 @@ void dcss_kms_detach(struct dcss_kms_dev *kms)drm_kms_helper_poll_fini (drm);drm_atomic_helper_shutdown (drm);drm_crtc_vblank_off (&kms->crtc.base); - drm->irq_enabled = false;drm_mode_config_cleanup (drm);dcss_crtc_deinit (&kms->crtc, drm); drm->dev_private =NULL ;drivers/gpu/drm/imx/imx-drm-core.c +0
-11@@ -207,17 +207,6 @@ static int imx_drm_bind(struct device *dev)if (IS_ERR (drm))return PTR_ERR (drm); -/* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -* -* P.S. note that we wouldn't use drm irq handler but -* just specific driver own one instead because -* drm framework supports only one irq handler and -* drivers can well take care of their interrupts -*/ - drm->irq_enabled = true; -/* * set max width and height as default value(4096x4096). * this value would be used to check framebuffer size limitation Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 10/22 drm/mediatek: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in mediatek.
--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/gpu/drm/mediatek/mtk_drm_drv.c +0 -6
@@ -270,12 +270,6 @@ static int mtk_drm_kms_init(struct drm_device *drm) goto err_component_unbind; } -/* -* We don't use the drm_irq_install() helpers provided by the DRM -* core, so we need to set this manually in order to allow the -* DRM_IOCTL_WAIT_VBLANK to operate correctly. -*/ - drm->irq_enabled = true; ret =drm_vblank_init (drm,MAX_CRTC );if (ret <0 ) goto err_component_unbind;
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 11/22 drm/nouveau: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in nouveau.
--- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- 1 file changed, 3 deletions(-)
drivers/gpu/drm/nouveau/nouveau_drm.c +0 -3
@@ -553,8 +553,6 @@ nouveau_drm_device_init(struct drm_device *dev)if (ret) goto fail_master; - dev->irq_enabled = true; -nvxx_client (&drm->client.base)->debug =nvkm_dbgopt (nouveau_debug,"DRM" );
@@ -795,7 +793,6 @@ nouveau_drm_device_remove(struct drm_device *dev)drm_dev_unregister (dev); - dev->irq_enabled = false; client =nvxx_client (&drm->client.base); device =nvkm_device_find (client->device);
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 12/22 drm/omapdrm: Track IRQ state in local device state
In reply to Thomas ZimmermannReplace usage of struct drm_device.irq_enabled with the driver's own state field struct omap_drm_device.irq_enabled. The field in the DRM device structure is considered legacy and should not be used by KMS drivers.
--- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-)
drivers/gpu/drm/omapdrm/omap_drv.h +2 -0
@@ -48,6 +48,8 @@ struct omap_drm_private {struct dss_device *dss;struct dispc_device *dispc; +bool irq_enabled; +unsigned int num_pipes;struct omap_drm_pipeline pipes[8 ];struct omap_drm_pipeline *channels[8 ];
drivers/gpu/drm/omapdrm/omap_irq.c +3 -3
@@ -291,7 +291,7 @@ int omap_drm_irq_install(struct drm_device *dev)if (ret <0 )return ret; - dev->irq_enabled = true; + priv->irq_enabled = true;return 0 ; }
@@ -300,10 +300,10 @@ void omap_drm_irq_uninstall(struct drm_device *dev) {struct omap_drm_private *priv = dev->dev_private; -if (!dev->irq_enabled) +if (!priv->irq_enabled)return ; - dev->irq_enabled = false; + priv->irq_enabled = false;dispc_free_irq (priv->dispc, dev); }
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 13/22 drm/rockchip: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rockchip.
--- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/gpu/drm/rockchip/rockchip_drm_drv.c +0 -6
@@ -162,12 +162,6 @@ static int rockchip_drm_bind(struct device *dev)drm_mode_config_reset (drm_dev); -/* -* enable drm irq mode. -* - with irq_enabled = true, we can use the vblank feature. -*/ - drm_dev->irq_enabled = true; - ret =rockchip_drm_fbdev_init (drm_dev);if (ret) goto err_unbind_all;
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 14/22 drm/sti: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sti.
--- drivers/gpu/drm/sti/sti_compositor.c | 2 -- 1 file changed, 2 deletions(-)
drivers/gpu/drm/sti/sti_compositor.c +0 -2
@@ -145,8 +145,6 @@ static int sti_compositor_bind(struct device *dev, }drm_vblank_init (drm_dev, crtc_id); -/* Allow usage of vblank without having to call drm_irq_install */ - drm_dev->irq_enabled =1 ;return 0 ; }
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 15/22 drm/stm: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in stm.
--- drivers/gpu/drm/stm/ltdc.c | 3 --- 1 file changed, 3 deletions(-)
drivers/gpu/drm/stm/ltdc.c +0 -3
@@ -1339,9 +1339,6 @@ int ltdc_load(struct drm_device *ddev) goto err; } -/* Allow usage of vblank without having to call drm_irq_install */ - ddev->irq_enabled =1 ; -clk_disable_unprepare (ldev->pixel_clk);pinctrl_pm_select_sleep_state (ddev->dev);
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 16/22 drm/sun4i: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sun4i.
--- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- 1 file changed, 2 deletions(-)
drivers/gpu/drm/sun4i/sun4i_drv.c +0 -2
@@ -97,8 +97,6 @@ static int sun4i_drv_bind(struct device *dev)if (ret) goto cleanup_mode_config; - drm->irq_enabled = true; -/* Remove early framebuffers (ie. simplefb) */ ret =drm_aperture_remove_framebuffers (false,"sun4i-drm-fb" );if (ret)
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 17/22 drm/tegra: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in tegra.
--- drivers/gpu/drm/tegra/drm.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/gpu/drm/tegra/drm.c +0 -7
@@ -1188,13 +1188,6 @@ static int host1x_drm_probe(struct host1x_device *dev) goto device; } -/* -* We don't use the drm_irq_install() helpers provided by the DRM -* core, so we need to set this manually in order to allow the -* DRM_IOCTL_WAIT_VBLANK to operate correctly. -*/ - drm->irq_enabled = true; -/* syncpoints are used for full 32-bit hardware VBLANK counters */ drm->max_vblank_count =0xffffffff ;
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 18/22 drm/tidss: Don't use struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't use it in tidss.
--- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- 1 file changed, 3 deletions(-)
drivers/gpu/drm/tidss/tidss_irq.c +0 -3
@@ -57,9 +57,6 @@ irqreturn_t tidss_irq_handler(int irq, void *arg)unsigned int id;dispc_irq_t irqstatus; -if (WARN_ON (!ddev->irq_enabled)) -return IRQ_NONE ; - irqstatus =dispc_read_and_clear_irqstatus (tidss->dispc);for (id =0 ; id < tidss->num_crtcs; id++) {
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 19/22 drm/vc4: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in vc4.
--- drivers/gpu/drm/vc4/vc4_kms.c | 1 - 1 file changed, 1 deletion(-)
drivers/gpu/drm/vc4/vc4_kms.c +0 -1
@@ -880,7 +880,6 @@ int vc4_kms_load(struct drm_device *dev)/* Set support for vblank irq fast disable, before drm_vblank_init() */ dev->vblank_disable_immediate = true; - dev->irq_enabled = true; ret =drm_vblank_init (dev, dev->mode_config.num_crtc);if (ret <0 ) {dev_err (dev->dev,"failed to initialize vblank\n" );
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 20/22 drm/vmwgfx: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in vmxgfx. All usage of the field within vmwgfx can safely be removed.
--- drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c +0 -8
@@ -292,15 +292,11 @@ void vmw_irq_uninstall(struct drm_device *dev)if (!(dev_priv->capabilities &SVGA_CAP_IRQMASK ))return ; -if (!dev->irq_enabled) -return ; -vmw_write (dev_priv,SVGA_REG_IRQMASK ,0 ); status =vmw_irq_status_read (dev_priv);vmw_irq_status_write (dev_priv, status); - dev->irq_enabled = false;free_irq (dev->irq, dev); }
@@ -315,9 +311,6 @@ int vmw_irq_install(struct drm_device *dev, int irq) {int ret; -if (dev->irq_enabled) -return -EBUSY ; -vmw_irq_preinstall (dev); ret = request_threaded_irq(irq, vmw_irq_handler, vmw_thread_fn,
@@ -325,7 +318,6 @@ int vmw_irq_install(struct drm_device *dev, int irq)if (ret <0 )return ret; - dev->irq_enabled = true; dev->irq = irq;return ret;
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 21/22 drm/xlnx: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in xlnx.
--- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- 1 file changed, 2 deletions(-)
drivers/gpu/drm/xlnx/zynqmp_dpsub.c +0 -2
@@ -111,8 +111,6 @@ static int zynqmp_dpsub_drm_init(struct zynqmp_dpsub *dpsub)if (ret)return ret; - drm->irq_enabled =1 ; -drm_kms_helper_poll_init (drm);/*
Signature · 1 line
2.32.0
Thomas Zimmermann
maintainerPATCH v2 22/22 drm/zte: Don't set struct drm_device.irq_enabled
In reply to Thomas ZimmermannThe field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in zte.
--- drivers/gpu/drm/zte/zx_drm_drv.c | 6 ------ 1 file changed, 6 deletions(-)
drivers/gpu/drm/zte/zx_drm_drv.c +0 -6
@@ -75,12 +75,6 @@ static int zx_drm_bind(struct device *dev) goto out_unbind; } -/* -* We will manage irq handler on our own. In this case, irq_enabled -* need to be true for using vblank core support. -*/ - drm->irq_enabled = true; -drm_mode_config_reset (drm);drm_kms_helper_poll_init (drm);
Signature · 1 line
2.32.0
Quoted 21 lines · Thomas Zimmermann
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core.
KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via
irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs.This used to be a single patch, [1] but it's now a full series.
The first 3 patches replace instances of
irq_enabledthat are not required.Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs.
THe rest of the patchset removes
irq_enabledfrom all non-legacy drivers. The only exception is omapdrm, which has an internal dpendency on the field's value. For this drivers, the state gets duplicated internally.
Quoted 8 lines
With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy.
v2:
* keep the original test for legacy drivers indrm_wait_vblank_ioctl()(Daniel)
On the series:
But I've only done a very light reading of this, so please wait for driver folks to have some time to check their own before merging.
I think a devm_ version of drm_irq_install might be helpful in further untangling here, but that's definitely for another series. -Daniel
Quoted 60 lines
Thomas Zimmermann (22):
drm/amdgpu: Track IRQ state in local device state drm/hibmc: Calldrm_irq_uninstall()unconditionally drm/radeon: Track IRQ state in local device state
drm: Don't test for IRQ support in VBLANK ioctls drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabled drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enableddrivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 ---------- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ------ drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 ++++++++-------- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ------ drivers/gpu/drm/sti/sti_compositor.c | 2 -- drivers/gpu/drm/stm/ltdc.c | 3 --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- drivers/gpu/drm/tegra/drm.c | 7 ------- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 -------- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- drivers/gpu/drm/zte/zx_drm_drv.c | 6 ------ 26 files changed, 30 insertions(+), 111 deletions(-)base-commit:
8c1323b422f8473421682ba783b5949ddd89a3f4
prerequisite-patch-id:c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id:c67e5d886a47b7d0266d81100837557fda34cb24--
2.32.0
Signature · 3 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Laurent Pinchart
maintainerHi Thomas,
Thank you for the patches.
Quoted 45 lines · Thomas Zimmermann
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core.
KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via
irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs.This used to be a single patch, [1] but it's now a full series.
The first 3 patches replace instances of
irq_enabledthat are not required.Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs.
THe rest of the patchset removes
irq_enabledfrom all non-legacy drivers. The only exception is omapdrm, which has an internal dpendency on the field's value. For this drivers, the state gets duplicated internally.With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy.
v2:
* keep the original test for legacy drivers indrm_wait_vblank_ioctl()(Daniel)[1] https://lore.kernel.org/dri-devel/[email protected]/
Thomas Zimmermann (22):
drm/amdgpu: Track IRQ state in local device state drm/hibmc: Calldrm_irq_uninstall()unconditionally drm/radeon: Track IRQ state in local device state
drm: Don't test for IRQ support in VBLANK ioctls drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabled
Quoted 8 lines
drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enabled
The list seems to be missing armada, rcar-du and vkms. It would also be nice to address i915 if possible.
Quoted 32 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 ---------- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ------ drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 ++++++++-------- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ------ drivers/gpu/drm/sti/sti_compositor.c | 2 -- drivers/gpu/drm/stm/ltdc.c | 3 --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- drivers/gpu/drm/tegra/drm.c | 7 ------- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 -------- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- drivers/gpu/drm/zte/zx_drm_drv.c | 6 ------ 26 files changed, 30 insertions(+), 111 deletions(-)base-commit:
8c1323b422f8473421682ba783b5949ddd89a3f4
prerequisite-patch-id:c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id:c67e5d886a47b7d0266d81100837557fda34cb24
Signature · 3 lines
Regards, Laurent Pinchart
Quoted 59 lines · Laurent Pinchart
Hi Thomas,
Thank you for the patches.
Quoted 45 lines · Thomas Zimmermann
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core.
KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via
irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs.This used to be a single patch, [1] but it's now a full series.
The first 3 patches replace instances of
irq_enabledthat are not required.Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs.
THe rest of the patchset removes
irq_enabledfrom all non-legacy drivers. The only exception is omapdrm, which has an internal dpendency on the field's value. For this drivers, the state gets duplicated internally.With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy.
v2:
* keep the original test for legacy drivers indrm_wait_vblank_ioctl()(Daniel)[1] https://lore.kernel.org/dri-devel/[email protected]/
Thomas Zimmermann (22):
drm/amdgpu: Track IRQ state in local device state drm/hibmc: Calldrm_irq_uninstall()unconditionally drm/radeon: Track IRQ state in local device state
drm: Don't test for IRQ support in VBLANK ioctls drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabledQuoted 8 lines
drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enabled
Quoted 2 lines
The list seems to be missing armada, rcar-du and vkms. It would also be nice to address i915 if possible.
In addition to this, for all the existing patches,
Quoted 32 lines
Quoted 32 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++--- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 ---- drivers/gpu/drm/arm/malidp_drv.c | 4 ---- drivers/gpu/drm/drm_irq.c | 10 +++------- drivers/gpu/drm/drm_vblank.c | 13 +++++++++---- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 ---------- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 -- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- drivers/gpu/drm/imx/imx-drm-core.c | 11 ----------- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 ------ drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++ drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++--- drivers/gpu/drm/radeon/radeon_fence.c | 2 +- drivers/gpu/drm/radeon/radeon_irq_kms.c | 16 ++++++++-------- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 ------ drivers/gpu/drm/sti/sti_compositor.c | 2 -- drivers/gpu/drm/stm/ltdc.c | 3 --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- drivers/gpu/drm/tegra/drm.c | 7 ------- drivers/gpu/drm/tidss/tidss_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_kms.c | 1 - drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8 -------- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- drivers/gpu/drm/zte/zx_drm_drv.c | 6 ------ 26 files changed, 30 insertions(+), 111 deletions(-)base-commit:
8c1323b422f8473421682ba783b5949ddd89a3f4
prerequisite-patch-id:c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id:c67e5d886a47b7d0266d81100837557fda34cb24
Signature · 3 lines
Regards, Laurent Pinchart
Thomas Zimmermann
maintainerAm 22.06.21 um 18:11 schrieb Laurent Pinchart:
Quoted 59 lines
Hi Thomas,
Thank you for the patches.
Quoted 45 lines · Thomas Zimmermann
Remove references to struct drm_device.irq_enabled from modern DRM drivers and core.
KMS drivers enable IRQs for their devices internally. They don't have to keep track of the IRQ state via
irq_enabled. For vblanking, it's cleaner to test for vblanking support directly than to test for enabled IRQs.This used to be a single patch, [1] but it's now a full series.
The first 3 patches replace instances of
irq_enabledthat are not required.Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs.
THe rest of the patchset removes
irq_enabledfrom all non-legacy drivers. The only exception is omapdrm, which has an internal dpendency on the field's value. For this drivers, the state gets duplicated internally.With the patchset applied, drivers can later switch over to plain Linux IRQ interfaces and DRM's IRQ midlayer can be declared legacy.
v2:
* keep the original test for legacy drivers indrm_wait_vblank_ioctl()(Daniel)[1] https://lore.kernel.org/dri-devel/[email protected]/
Thomas Zimmermann (22):
drm/amdgpu: Track IRQ state in local device state drm/hibmc: Calldrm_irq_uninstall()unconditionally drm/radeon: Track IRQ state in local device state
drm: Don't test for IRQ support in VBLANK ioctls drm/komeda: Don't set struct drm_device.irq_enabled drm/malidp: Don't set struct drm_device.irq_enabled drm/exynos: Don't set struct drm_device.irq_enabled drm/kirin: Don't set struct drm_device.irq_enabled drm/imx: Don't set struct drm_device.irq_enabled drm/mediatek: Don't set struct drm_device.irq_enabled drm/nouveau: Don't set struct drm_device.irq_enabled drm/omapdrm: Track IRQ state in local device state drm/rockchip: Don't set struct drm_device.irq_enabled drm/sti: Don't set struct drm_device.irq_enabledQuoted 8 lines
drm/stm: Don't set struct drm_device.irq_enabled drm/sun4i: Don't set struct drm_device.irq_enabled drm/tegra: Don't set struct drm_device.irq_enabled drm/tidss: Don't use struct drm_device.irq_enabled drm/vc4: Don't set struct drm_device.irq_enabled drm/vmwgfx: Don't set struct drm_device.irq_enabled drm/xlnx: Don't set struct drm_device.irq_enabled drm/zte: Don't set struct drm_device.irq_enabled
Quoted 2 lines
The list seems to be missing armada, rcar-du and vkms. It would also be nice to address i915 if possible.
Indeed. I grepped for \>irq_enabled. But some few drivers use .irq_enabled. I'll fix this in the patchset's next iteration. Thanks for double checking.
Best regards
Thomas
Signature · 6 lines
Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer