[Nouveau] [PATCH v3 08/27] drm/malidp: Don't set struct drm_device.irq_enabled
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
Reviewed-by: Laurent Pinchart
Acked-by: Liviu Dudau
Acked-by: Daniel Vetter
---
drivers/gpu/drm/arm/malidp_drv.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index de59f3302516..78d15b04b105 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
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().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _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 _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 _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.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used
drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function
fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
Or how about:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if defined(CONFIG_DRM_LEGACY)
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
#endif
return drm_dev_has_vblank(dev);
}
?
It's inline, but still readable.
Best regards
Thomas
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7
[Nouveau] [PATCH v3 17/27] drm/rockchip: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rockchip. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index b730b8d5d949..c8e60fd9ff24 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau/core: fix the uninitialized use in nvkm_ioctl_map()
In function nvkm_ioctl_map(), the variable "type" could be uninitialized if "nvkm_object_map()" returns error code, however, it does not check the return value and directly use the "type" in the if statement, which is potentially unsafe. Signed-off-by: Yizhuo --- drivers/gpu/drm/nouveau/nvkm/core/ioctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c index d777df5a64e6..7f2e8482f167 100644 --- a/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c +++ b/drivers/gpu/drm/nouveau/nvkm/core/ioctl.c @@ -266,6 +266,8 @@ nvkm_ioctl_map(struct nvkm_client *client, ret = nvkm_object_map(object, data, size, , >v0.handle, >v0.length); + if (ret) + return ret; if (type == NVKM_OBJECT_MAP_IO) args->v0.type = NVIF_IOCTL_MAP_V0_IO; else -- 2.17.1 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 20/27] drm/sun4i: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sun4i. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index af335f58bdfc..570f3af25e86 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -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) -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:51 schrieb Jani Nikula:
> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 3417e1ac7918..10fe16bafcb6 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
> > > > > + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> > > > > + if (!dev->irq_enabled)
> > > > > + return -EOPNOTSUPP;
> > > > > + } else /* if DRIVER_MODESET */
> > > > > +#endif
> > > > > + {
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > + return -EOPNOTSUPP;
> > > > > + }
> > > >
> > > > Sheesh I hate this kind of inline #ifdefs.
> > > >
> > > > Two alternate suggestions that I believe should be as just efficient:
> > >
> > > Or how about:
> > >
> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > >
> > > {
> > >
> > > if defined(CONFIG_DRM_LEGACY)
> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > >
> > > return dev->irq_enabled;
> > >
> > > #endif
> > > return drm_dev_has_vblank(dev);
> > >
> > > }
> > >
> > >
> > > ?
> > >
> > > It's inline, but still readable.
> >
> > It's definitely better than the original, but it's unclear to me why
> > you'd prefer this over option 2) below. I guess the only reason I can
> > think of is emphasizing the conditional compilation. However,
> > IS_ENABLED() is widely used in this manner specifically to avoid inline
> > #if, and the compiler optimizes it away.
>
> It's simply more readable to me as the condition is simpler. But option 2 is
> also ok.
Either option looks good to me.
Reviewed-by: Liviu Dudau
Thanks for doing that!
Liviu
>
> Best regards
> Thomas
>
> >
> > BR,
> > Jani.
> >
> >
> > >
> > > Best regards
> > > Thomas
> > >
> > > >
> > > > 1) The more verbose:
> > > >
> > > > #if defined(CONFIG_DRM_LEGACY)
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #else
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > > #endif
> > > >
> > > > 2) The more compact:
> > > >
> > > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > > > {
> > > > if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
> > > > unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > > > return dev->irq_enabled;
> > > > else
> > > > return drm_dev_has_vblank(dev);
> > > > }
> > > >
> > > > Then, in drm_wait_vblank_ioctl().
> > > >
> > > > if (!drm_wait_vblank_supported(dev))
> > > > return -EOPNOTSUPP;
> > > >
> > > > The compiler should do the right thing without any explicit inline
> > > > keywords etc.
> > > >
> > > > BR,
> > > > Jani.
> > > >
> > > > > if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> > > > > return -EINVAL;
> > > > > @@ -2023,7 +2031,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 +2090,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);
> > > >
> >
>
> --
> 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
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
[Nouveau] [PATCH v3 18/27] drm/sti: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sti. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sti/sti_compositor.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index 319962a2c17b..9caaf3ccfabe 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -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; } -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 11/27] drm/imx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in imx. v3: * move dcss changes into separate patch (Laurentiu) Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Philipp Zabel Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/imx-drm-core.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 76819a8ac37f..9558e9e1b431 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -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 -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 14/27] drm/nouveau: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in nouveau. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index a616cf4573b8..1cb14e99a60c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -553,8 +553,6 @@ nouveau_drm_device_init(struct drm_device *dev) if (ret) goto fail_master; - dev->irq_enabled = true; - nvxx_client(>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(>client.base); device = nvkm_device_find(client->device); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 07/27] drm/komeda: Don't set struct drm_device.irq_enabled
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 Reviewed-by: Laurent Pinchart Acked-by: Liviu Dudau Acked-by: Daniel Vetter --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index ff45f23f3d56..52a6db5707a3 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 23/27] drm/vc4: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vc4.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vc4/vc4_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 6a1a9e1d72ce..f0b3e4cf5bce 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -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");
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH -next] drm/nouveau/svm: Remove set but not used variable 'ret'
ping
在 2021/5/31 10:38, Baokun Li 写道:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/nouveau/nouveau_svm.c: In function 'nouveau_pfns_map':
drivers/gpu/drm/nouveau/nouveau_svm.c:814:6: warning:
variable ‘ret’ set but not used [-Wunused-but-set-variable]
It never used since introduction.
Signed-off-by: Baokun Li
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 84726a89e665..16fbf90f9f31 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -921,7 +921,6 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct
mm_struct *mm,
unsigned long addr, u64 *pfns, unsigned long npages)
{
struct nouveau_pfnmap_args *args = nouveau_pfns_to_args(pfns);
- int ret;
args->p.addr = addr;
args->p.size = npages << PAGE_SHIFT;
@@ -929,7 +928,7 @@ nouveau_pfns_map(struct nouveau_svmm *svmm, struct
mm_struct *mm,
mutex_lock(>mutex);
svmm->vmm->vmm.object.client->super = true;
- ret = nvif_object_ioctl(>vmm->vmm.object, args, sizeof(*args) +
+ nvif_object_ioctl(>vmm->vmm.object, args, sizeof(*args) +
npages * sizeof(args->p.phys[0]), NULL);
svmm->vmm->vmm.object.client->super = false;
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] remove unused varialble "struct device *dev"
fix the warning- variable 'dev' set but not used
Signed-off-by: Cai Huoqing
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 984721b..cb3ff4a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1242,7 +1242,6 @@ vm_fault_t nouveau_ttm_fault_reserve_notify(struct
ttm_buffer_object *bo)
{
struct ttm_tt *ttm_dma = (void *)ttm;
struct nouveau_drm *drm;
- struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
if (ttm_tt_is_populated(ttm))
@@ -1255,7 +1254,6 @@ vm_fault_t nouveau_ttm_fault_reserve_notify(struct
ttm_buffer_object *bo)
}
drm = nouveau_bdev(bdev);
- dev = drm->dev->dev;
return ttm_pool_alloc(>ttm.bdev.pool, ttm, ctx);
}
@@ -1265,14 +1263,12 @@ vm_fault_t nouveau_ttm_fault_reserve_notify(struct
ttm_buffer_object *bo)
struct ttm_tt *ttm)
{
struct nouveau_drm *drm;
- struct device *dev;
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
if (slave)
return;
drm = nouveau_bdev(bdev);
- dev = drm->dev->dev;
return ttm_pool_free(>ttm.bdev.pool, ttm);
}
--
1.8.3.1
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vkms.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
---
drivers/gpu/drm/vkms/vkms_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 027ffe759440..496de38ad983 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
goto out_devres;
}
- vkms_device->drm.irq_enabled = true;
-
ret = drm_vblank_init(_device->drm, 1);
if (ret) {
DRM_ERROR("Failed to vblank\n");
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] 答复: [PATCH v2 02/22] drm/hibmc: Call drm_irq_uninstall() unconditionally
-邮件原件-
发件人: Thomas Zimmermann [mailto:tzimmerm...@suse.de]
发送时间: 2021年6月22日 22:10
收件人: dan...@ffwll.ch; airl...@linux.ie; alexander.deuc...@amd.com;
christian.koe...@amd.com; xinhui@amd.com; james.qian.w...@arm.com;
liviu.du...@arm.com; mihail.atanas...@arm.com; brian.star...@arm.com;
maarten.lankho...@linux.intel.com; mrip...@kernel.org; inki@samsung.com;
jy0922.s...@samsung.com; sw0312@samsung.com; kyungmin.p...@samsung.com;
krzysztof.kozlow...@canonical.com; xinliang@linaro.org; tiantao (H)
; john.stu...@linaro.org; kongxinwei (A)
; Chenfeng (puck) ;
laurentiu.pa...@oss.nxp.com; l.st...@pengutronix.de; p.za...@pengutronix.de;
shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de;
feste...@gmail.com; linux-...@nxp.com; chunkuang...@kernel.org;
matthias@gmail.com; bske...@redhat.com; to...@kernel.org;
h...@rock-chips.com; he...@sntech.de; benjamin.gaign...@linaro.org;
yannick.fer...@foss.st.com; philippe.co...@foss.st.com;
mcoquelin.st...@gmail.com; alexandre.tor...@foss.st.com; w...@csie.org;
jernej.skra...@gmail.com; thierry.red...@gmail.com; jonath...@nvidia.com;
jyri.sa...@iki.fi; e...@anholt.net; linux-graphics-maintai...@vmware.com;
za...@vmware.com; hyun.k...@xilinx.com; laurent.pinch...@ideasonboard.com;
michal.si...@xilinx.com
抄送: amd-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
linux-arm-ker...@lists.infradead.org; linux-samsung-...@vger.kernel.org;
linux-media...@lists.infradead.org; nouveau@lists.freedesktop.org;
linux-rockc...@lists.infradead.org; linux-st...@st-md-mailman.stormreply.com;
linux-su...@lists.linux.dev; linux-te...@vger.kernel.org; Thomas Zimmermann
主题: [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.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Tian Tao
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 19/27] drm/stm: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in stm. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/stm/ltdc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 08b71248044d..e9c5a52f041a 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 22/22] drm/zte: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in zte. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/zte/zx_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 5506336594e2..064056503ebb 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 21/22] drm/xlnx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in xlnx. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index 0c1c50271a88..ac37053412a1 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -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); /* -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 09/27] drm/exynos: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in exynos. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e60257f1f24b..d8f1cf4d6b69 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 13/27] drm/mediatek: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in mediatek. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b46bdb8985da..9b60bec33d3b 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:51 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
Or how about:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if defined(CONFIG_DRM_LEGACY)
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
#endif
return drm_dev_has_vblank(dev);
}
?
It's inline, but still readable.
It's definitely better than the original, but it's unclear to me why
you'd prefer this over option 2) below. I guess the only reason I can
think of is emphasizing the conditional compilation. However,
IS_ENABLED() is widely used in this manner specifically to avoid inline
#if, and the compiler optimizes it away.
It's simply more readable to me as the condition is simpler. But option
2 is also ok.
Best regards
Thomas
BR,
Jani.
Best regards
Thomas
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,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 +2090,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);
--
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
OpenPGP_signature
Description: OpenPGP digital signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v4 25/27] drm/vmwgfx: Don't set struct drm_device.irq_enabled
> On Jun 25, 2021, at 04:22, Thomas Zimmermann wrote: > > The 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. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Laurent Pinchart > Acked-by: Daniel Vetter Looks good. Reviewed-by: Zack Rusin ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 08/22] drm/kirin: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in kirin.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index e590e19db657..98ae9a48f3fe 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -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);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 21/27] drm/tegra: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in tegra. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Thierry Reding Acked-by: Daniel Vetter --- drivers/gpu/drm/tegra/drm.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index f96c237b2242..8d27c21ddf48 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -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 = 0x; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 18/22] drm/tidss: Don't use struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't use it in tidss.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/tidss/tidss_irq.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_irq.c
b/drivers/gpu/drm/tidss/tidss_irq.c
index a5ec7931ef6b..2ed3e3296776 100644
--- a/drivers/gpu/drm/tidss/tidss_irq.c
+++ b/drivers/gpu/drm/tidss/tidss_irq.c
@@ -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++) {
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 21/27] drm/tegra: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in tegra. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/tegra/drm.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index f96c237b2242..8d27c21ddf48 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -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 = 0x; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 07/22] drm/exynos: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in exynos. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e60257f1f24b..d8f1cf4d6b69 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 09/22] drm/imx: Don't set struct drm_device.irq_enabled
Hi Thomas, On Tue, Jun 22, 2021 at 04:09:49PM +0200, Thomas Zimmermann wrote: > 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 > --- > 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: Acked-by: Laurentiu Palcu Thanks, laurentiu > drivers/gpu/drm/imx/imx-drm-core.c | 11 --- > 2 files changed, 14 deletions(-) > > diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c > b/drivers/gpu/drm/imx/dcss/dcss-kms.c > index 37ae68a7fba5..917834b1c80e 100644 > --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c > +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c > @@ -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(>crtc.base); > - drm->irq_enabled = false; > drm_mode_config_cleanup(drm); > dcss_crtc_deinit(>crtc, drm); > drm->dev_private = NULL; > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c > b/drivers/gpu/drm/imx/imx-drm-core.c > index 76819a8ac37f..9558e9e1b431 100644 > --- a/drivers/gpu/drm/imx/imx-drm-core.c > +++ b/drivers/gpu/drm/imx/imx-drm-core.c > @@ -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 > -- > 2.32.0 > ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 16/22] drm/sun4i: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sun4i. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index af335f58bdfc..570f3af25e86 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -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) -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
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().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _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 _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 _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.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,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 +2090,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);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> 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().
>
> v3:
> * optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
> * update docs for drm_irq_uninstall()
> v2:
> * keep the old test for legacy drivers in
> drm_wait_vblank_ioctl() (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Laurent Pinchart
> Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c| 13 -
> drivers/gpu/drm/drm_vblank.c | 16
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c3bd664ea733..945dd82e2ea3 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,10 +74,8 @@
> * only supports devices with a single interrupt on the main device stored in
> * _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 _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 _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.
> @@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
> *
> * Calls the driver's _driver.irq_uninstall function and unregisters the
> IRQ
> * handler. This should only be called by drivers which used
> drm_irq_install()
> - * to set up their interrupt handler. Other drivers must only reset
> - * _device.irq_enabled to false.
> + * to set up their interrupt handler.
> *
> * Note that for kernel modesetting drivers it is a bug if this function
> fails.
> * The sanity checks are only to catch buggy user modesetting drivers which
> call
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac7918..10fe16bafcb6 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
> + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> + if (!dev->irq_enabled)
> + return -EOPNOTSUPP;
> + } else /* if DRIVER_MODESET */
> +#endif
> + {
> + if (!drm_dev_has_vblank(dev))
> + return -EOPNOTSUPP;
> + }
Sheesh I hate this kind of inline #ifdefs.
Two alternate suggestions that I believe should be as just efficient:
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
>
> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
> return -EINVAL;
> @@ -2023,7 +2031,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))
>
[Nouveau] [PATCH v3 06/27] drm/i915: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct drm_i915_private.irq_enabled. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_irq.c | 8
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01e11fe38642..48c1835bd54b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,6 +1134,8 @@ struct drm_i915_private {
/* For i915gm/i945gm vblank irq workaround */
u8 vblank_enabled;
+ bool irq_enabled;
+
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a11bdb667241..987211f21761 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4488,14 +4488,14 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
*/
dev_priv->runtime_pm.irqs_enabled = true;
- dev_priv->drm.irq_enabled = true;
+ dev_priv->irq_enabled = true;
intel_irq_reset(dev_priv);
ret = request_irq(irq, intel_irq_handler(dev_priv),
IRQF_SHARED, DRIVER_NAME, dev_priv);
if (ret < 0) {
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
return ret;
}
@@ -4521,10 +4521,10 @@ void intel_irq_uninstall(struct drm_i915_private
*dev_priv)
* intel_modeset_driver_remove() calling us out of sequence.
* Would be nice if it didn't do that...
*/
- if (!dev_priv->drm.irq_enabled)
+ if (!dev_priv->irq_enabled)
return;
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
intel_irq_reset(dev_priv);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v10 07/10] mm: Device exclusive memory access
On Fri, Jun 11, 2021 at 01:43:20PM +1000, Alistair Popple wrote: > On Friday, 11 June 2021 11:00:34 AM AEST Peter Xu wrote: > > On Fri, Jun 11, 2021 at 09:17:14AM +1000, Alistair Popple wrote: > > > On Friday, 11 June 2021 9:04:19 AM AEST Peter Xu wrote: > > > > On Fri, Jun 11, 2021 at 12:21:26AM +1000, Alistair Popple wrote: > > > > > > Hmm, the thing is.. to me FOLL_SPLIT_PMD should have similar effect > > > > > > to explicit > > > > > > call split_huge_pmd_address(), afaict. Since both of them use > > > > > > __split_huge_pmd() > > > > > > internally which will generate that unwanted CLEAR notify. > > > > > > > > > > Agree that gup calls __split_huge_pmd() via split_huge_pmd_address() > > > > > which will always CLEAR. However gup only calls > > > > > split_huge_pmd_address() if it > > > > > finds a thp pmd. In follow_pmd_mask() we have: > > > > > > > > > > if (likely(!pmd_trans_huge(pmdval))) > > > > > return follow_page_pte(vma, address, pmd, flags, > > > > > >pgmap); > > > > > > > > > > So I don't think we have a problem here. > > > > > > > > Sorry I didn't follow here.. We do FOLL_SPLIT_PMD after this check, > > > > right? I > > > > mean, if it's a thp for the current mm, afaict pmd_trans_huge() should > > > > return > > > > true above, so we'll skip follow_page_pte(); then we'll check > > > > FOLL_SPLIT_PMD > > > > and do the split, then the CLEAR notify. Hmm.. Did I miss something? > > > > > > That seems correct - if the thp is not mapped with a pmd we won't split > > > and we > > > won't CLEAR. If there is a thp pmd we will split and CLEAR, but in that > > > case it > > > is fine - we will retry, but the retry will won't CLEAR because the pmd > > > has > > > already been split. > > > > Aha! > > > > > > > > The issue arises with doing it unconditionally in make device exclusive > > > is that > > > you *always* CLEAR even if there is no thp pmd to split. Or at least > > > that's my > > > understanding, please let me know if it doesn't make sense. > > > > Exactly. But if you see what I meant here, even if it can work like this, > > it > > sounds still fragile, isn't it? I just feel something is slightly off > > there.. > > > > IMHO split_huge_pmd() checked pmd before calling __split_huge_pmd() for > > performance, afaict, because if it's not a thp even without locking, then it > > won't be, so further __split_huge_pmd() is not necessary. > > > > IOW, it's very legal if someday we'd like to let split_huge_pmd() call > > __split_huge_pmd() directly, then AFAIU device exclusive API will be the 1st > > one to be broken with that seems-to-be-irrelevant change I'm afraid.. > > Well I would argue the performance of memory notifiers is becoming > increasingly > important, and a change that causes them to be called unnecessarily is > therefore not very legal. Likely the correct fix here is to optimise > __split_huge_pmd() to only call the notifier if it's actually going to split a > pmd. As you said though that's a completely different story which I think > would > be best done as a separate series. Right, maybe I can look a bit more into that later; but my whole point was to express that one functionality shouldn't depend on such a trivial detail of implementation of other modules (thp split in this case). > > > This lets me goes back a step to think about why do we need this notifier at > > all to cover this whole range of make_device_exclusive() procedure.. > > > > What I am thinking is, we're afraid some CPU accesses this page so the pte > > got > > quickly restored when device atomic operation is carrying on. Then with > > this > > notifier we'll be able to cancel it. Makes perfect sense. > > > > However do we really need to register this notifier so early? The thing is > > the > > GPU driver still has all the page locks, so even if there's a race to > > restore > > the ptes, they'll block at taking the page lock until the driver releases > > it. > > > > IOW, I'm wondering whether the "non-fragile" way to do this is not do > > mmu_interval_notifier_insert() that early: what if we register that notifier > > after make_device_exclusive_range() returns but before page_unlock() > > somehow? > > So before page_unlock(), race is protected fully by the lock itself; after > > that, it's done by mmu notifier. Then maybe we don't need to worry about > > all > > these notifications during marking exclusive (while we shouldn't)? > > The notifier is needed to protect against races with pte changes. Once a page > has been marked for exclusive access the driver will update it's page tables > to > allow atomic access to the page. However in the meantime the page could become > unmapped entirely or write protected. > > As I understand things the page lock won't protect against these kind of pte > changes, hence the need for mmu_interval_read_begin/retry which allows the > driver to hold a mutex protecting against invalidations via blocking the >
Re: [Nouveau] [PATCH v2 06/22] drm/malidp: Don't set struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 04:09:46PM +0200, Thomas Zimmermann wrote:
> 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
Acked-by: Liviu Dudau
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/malidp_drv.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c
> b/drivers/gpu/drm/arm/malidp_drv.c
> index de59f3302516..78d15b04b105 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -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;
> --
> 2.32.0
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 25/27] drm/vmwgfx: Don't set struct drm_device.irq_enabled
The 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
index b9a9b7ddadbd..4b82f5995452 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 27/27] drm/zte: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in zte. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/zte/zx_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 5506336594e2..064056503ebb 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 07/27] drm/komeda: Don't set struct drm_device.irq_enabled
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 Reviewed-by: Laurent Pinchart Acked-by: Liviu Dudau Acked-by: Daniel Vetter --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index ff45f23f3d56..52a6db5707a3 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 06/22] drm/malidp: Don't set struct drm_device.irq_enabled
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
---
drivers/gpu/drm/arm/malidp_drv.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index de59f3302516..78d15b04b105 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 03/27] drm/radeon: Track IRQ state in local device state
Replace 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/radeon/radeon_fence.c | 2 +-
drivers/gpu/drm/radeon/radeon_irq_kms.c | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index 0d8ef2368adf..7ec581363e23 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -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;
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 84d0b1a3355f..a36ce826d0c0 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -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(>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(>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(>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(>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(>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(>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(>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(>irq.lock, irqflags);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 25/27] drm/vmwgfx: Don't set struct drm_device.irq_enabled
The 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
index b9a9b7ddadbd..4b82f5995452 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 11/22] drm/nouveau: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in nouveau. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index a616cf4573b8..1cb14e99a60c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -553,8 +553,6 @@ nouveau_drm_device_init(struct drm_device *dev) if (ret) goto fail_master; - dev->irq_enabled = true; - nvxx_client(>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(>client.base); device = nvkm_device_find(client->device); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:51 schrieb Jani Nikula:
> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 3417e1ac7918..10fe16bafcb6 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
> > > > > + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
> > > > > + if (!dev->irq_enabled)
> > > > > + return -EOPNOTSUPP;
> > > > > + } else /* if DRIVER_MODESET */
> > > > > +#endif
> > > > > + {
> > > > > + if (!drm_dev_has_vblank(dev))
> > > > > + return -EOPNOTSUPP;
> > > > > + }
> > > >
> > > > Sheesh I hate this kind of inline #ifdefs.
> > > >
> > > > Two alternate suggestions that I believe should be as just efficient:
> > >
> > > Or how about:
> > >
> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
> > >
> > > {
> > >
> > > if defined(CONFIG_DRM_LEGACY)
> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> > >
> > > return dev->irq_enabled;
> > >
> > > #endif
> > > return drm_dev_has_vblank(dev);
> > >
> > > }
> > >
> > >
> > > ?
> > >
> > > It's inline, but still readable.
> >
> > It's definitely better than the original, but it's unclear to me why
> > you'd prefer this over option 2) below. I guess the only reason I can
> > think of is emphasizing the conditional compilation. However,
> > IS_ENABLED() is widely used in this manner specifically to avoid inline
> > #if, and the compiler optimizes it away.
>
> It's simply more readable to me as the condition is simpler. But option 2 is
> also ok.
Perhaps do something like this, then:
if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
}
return drm_dev_has_vblank(dev);
That's about just as readable as the variant involving the preprocessor
but has all the benefits of not using the preprocessor.
Thierry
signature.asc
Description: PGP signature
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 08/27] drm/malidp: Don't set struct drm_device.irq_enabled
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
Reviewed-by: Laurent Pinchart
Acked-by: Liviu Dudau
Acked-by: Daniel Vetter
---
drivers/gpu/drm/arm/malidp_drv.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index de59f3302516..78d15b04b105 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 04/27] drm: Don't test for IRQ support in VBLANK ioctls
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().
v4:
* avoid preprocessor ifdef in drm_wait_vblank_ioctl()
(Jani, Thierry)
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Reviewed-by: Liviu Dudau
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 15 ---
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _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 _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 _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.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..bba6781cc48f 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1737,6 +1737,15 @@ static void drm_wait_vblank_reply(struct drm_device
*dev, unsigned int pipe,
reply->tval_usec = ts.tv_nsec / 1000;
}
+static bool drm_wait_vblank_supported(struct drm_device *dev)
+{
+ if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
+ return dev->irq_enabled;
+ }
+ return drm_dev_has_vblank(dev);
+}
+
int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1748,7 +1757,7 @@ 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)
+ if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
@@ -2023,7 +2032,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 +2091,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);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vkms.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vkms/vkms_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 027ffe759440..496de38ad983 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
goto out_devres;
}
- vkms_device->drm.irq_enabled = true;
-
ret = drm_vblank_init(_device->drm, 1);
if (ret) {
DRM_ERROR("Failed to vblank\n");
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 23/27] drm/vc4: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vc4.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/vc4/vc4_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 6a1a9e1d72ce..f0b3e4cf5bce 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -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");
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 01/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.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 32ce0e679dc7..7dad44e73cf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -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)
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 13/27] drm/mediatek: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in mediatek. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b46bdb8985da..9b60bec33d3b 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 06/27] drm/i915: Track IRQ state in local device state
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> Replace usage of struct drm_device.irq_enabled with the driver's
> own state field struct drm_i915_private.irq_enabled. The field in
> the DRM device structure is considered legacy and should not be
> used by KMS drivers.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Jani Nikula
and ack for merging through drm-misc-next or whatever you think is best.
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 ++
> drivers/gpu/drm/i915/i915_irq.c | 8
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 01e11fe38642..48c1835bd54b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1134,6 +1134,8 @@ struct drm_i915_private {
> /* For i915gm/i945gm vblank irq workaround */
> u8 vblank_enabled;
>
> + bool irq_enabled;
> +
> /* perform PHY state sanity checks? */
> bool chv_phy_assert[2];
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a11bdb667241..987211f21761 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4488,14 +4488,14 @@ int intel_irq_install(struct drm_i915_private
> *dev_priv)
>*/
> dev_priv->runtime_pm.irqs_enabled = true;
>
> - dev_priv->drm.irq_enabled = true;
> + dev_priv->irq_enabled = true;
>
> intel_irq_reset(dev_priv);
>
> ret = request_irq(irq, intel_irq_handler(dev_priv),
> IRQF_SHARED, DRIVER_NAME, dev_priv);
> if (ret < 0) {
> - dev_priv->drm.irq_enabled = false;
> + dev_priv->irq_enabled = false;
> return ret;
> }
>
> @@ -4521,10 +4521,10 @@ void intel_irq_uninstall(struct drm_i915_private
> *dev_priv)
>* intel_modeset_driver_remove() calling us out of sequence.
>* Would be nice if it didn't do that...
>*/
> - if (!dev_priv->drm.irq_enabled)
> + if (!dev_priv->irq_enabled)
> return;
>
> - dev_priv->drm.irq_enabled = false;
> + dev_priv->irq_enabled = false;
>
> intel_irq_reset(dev_priv);
--
Jani Nikula, Intel Open Source Graphics Center
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 22/27] drm/tidss: Don't use struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't use it in tidss.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/tidss/tidss_irq.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_irq.c
b/drivers/gpu/drm/tidss/tidss_irq.c
index a5ec7931ef6b..2ed3e3296776 100644
--- a/drivers/gpu/drm/tidss/tidss_irq.c
+++ b/drivers/gpu/drm/tidss/tidss_irq.c
@@ -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++) {
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [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.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 20/22] drm/vmwgfx: Don't set struct drm_device.irq_enabled
The 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.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vmwgfx/vmwgfx_irq.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
index b9a9b7ddadbd..4b82f5995452 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_irq.c
@@ -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;
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 13/22] drm/rockchip: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rockchip. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index b730b8d5d949..c8e60fd9ff24 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 17/22] drm/tegra: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in tegra. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tegra/drm.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index f96c237b2242..8d27c21ddf48 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -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 = 0x; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
Hello,
On Tue, Jun 22, 2021 at 04:09:44PM +0200, Thomas Zimmermann wrote:
> 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
> ---
> drivers/gpu/drm/drm_irq.c| 10 +++---
> drivers/gpu/drm/drm_vblank.c | 13 +
> 2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index c3bd664ea733..1d7785721323 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -74,10 +74,8 @@
> * only supports devices with a single interrupt on the main device stored in
> * _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 _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 _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.
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 3417e1ac7918..a98a4aad5037 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -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;
> + }
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
>
> 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);
> --
> 2.32.0
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 00/22] Deprecate struct drm_device.irq_enabled
Am 22.06.21 um 18:11 schrieb Laurent Pinchart: Hi Thomas, Thank you for the patches. On Tue, Jun 22, 2021 at 04:09:40PM +0200, Thomas Zimmermann wrote: 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/20210608090301.4752-1-tzimmerm...@suse.de/ 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 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 -- 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 OpenPGP_signature Description: OpenPGP digital signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
Hi Thomas,
Thank you for the patch.
On Thu, Jun 24, 2021 at 09:29:13AM +0200, Thomas Zimmermann wrote:
> The field drm_device.irq_enabled is only used by legacy drivers
> with userspace modesetting. Don't set it in vkms.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
> ---
> drivers/gpu/drm/vkms/vkms_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 027ffe759440..496de38ad983 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
> goto out_devres;
> }
>
> - vkms_device->drm.irq_enabled = true;
> -
> ret = drm_vblank_init(_device->drm, 1);
> if (ret) {
> DRM_ERROR("Failed to vblank\n");
--
Regards,
Laurent Pinchart
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 02/27] 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Tian Tao
Acked-by: Daniel Vetter
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 00/22] Deprecate struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 07:11:33PM +0300, Laurent Pinchart wrote: > Hi Thomas, > > Thank you for the patches. > > On Tue, Jun 22, 2021 at 04:09:40PM +0200, Thomas Zimmermann wrote: > > 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/20210608090301.4752-1-tzimmerm...@suse.de/ > > > > 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 > > 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, Reviewed-by: Laurent Pinchart > > 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 -- Regards, Laurent Pinchart ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 03/22] drm/radeon: Track IRQ state in local device state
Replace 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.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/radeon/radeon_fence.c | 2 +-
drivers/gpu/drm/radeon/radeon_irq_kms.c | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index 0d8ef2368adf..7ec581363e23 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -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;
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 84d0b1a3355f..a36ce826d0c0 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -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(>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(>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(>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(>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(>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(>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(>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(>irq.lock, irqflags);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 05/27] drm/armada: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in armada. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/armada/armada_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index dab0a1f0983b..4a64f1b9ec4d 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -130,8 +130,6 @@ static int armada_drm_bind(struct device *dev) if (ret) goto err_comp; - priv->drm.irq_enabled = true; - drm_mode_config_reset(>drm); ret = armada_fbdev_init(>drm); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 19/22] drm/vc4: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in vc4.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/vc4/vc4_kms.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 6a1a9e1d72ce..f0b3e4cf5bce 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -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");
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 11/27] drm/imx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in imx. v3: * move dcss changes into separate patch (Laurentiu) Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/imx-drm-core.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 76819a8ac37f..9558e9e1b431 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -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 -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 26/27] drm/xlnx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in xlnx. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index 0c1c50271a88..ac37053412a1 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -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); /* -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 20/27] drm/sun4i: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sun4i. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c index af335f58bdfc..570f3af25e86 100644 --- a/drivers/gpu/drm/sun4i/sun4i_drv.c +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c @@ -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) -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 26/27] drm/xlnx: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in xlnx. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c index 0c1c50271a88..ac37053412a1 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c @@ -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); /* -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 16/27] drm/rcar-du: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rcar-du. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index bfbff90588cb..e289a66594a7 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -593,8 +593,6 @@ static int rcar_du_probe(struct platform_device *pdev) goto error; } - rcdu->ddev.irq_enabled = 1; - /* * Register the DRM device with the core and the connectors with * sysfs. -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 22/27] drm/tidss: Don't use struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't use it in tidss.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/tidss/tidss_irq.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/tidss/tidss_irq.c
b/drivers/gpu/drm/tidss/tidss_irq.c
index a5ec7931ef6b..2ed3e3296776 100644
--- a/drivers/gpu/drm/tidss/tidss_irq.c
+++ b/drivers/gpu/drm/tidss/tidss_irq.c
@@ -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++) {
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 04/22] drm: Don't test for IRQ support in VBLANK ioctls
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
---
drivers/gpu/drm/drm_irq.c| 10 +++---
drivers/gpu/drm/drm_vblank.c | 13 +
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..1d7785721323 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _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 _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 _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.
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..a98a4aad5037 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -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);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 16/27] drm/rcar-du: Don't set struct drm_device.irq_enabled
Hi Thomas, Thank you for the patch. On Thu, Jun 24, 2021 at 09:29:05AM +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in rcar-du. > > Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index bfbff90588cb..e289a66594a7 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -593,8 +593,6 @@ static int rcar_du_probe(struct platform_device *pdev) > goto error; > } > > - rcdu->ddev.irq_enabled = 1; > - > /* >* Register the DRM device with the core and the connectors with >* sysfs. -- Regards, Laurent Pinchart ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 12/27] drm/imx/dcss: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in dcss. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Laurentiu Palcu Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c index 37ae68a7fba5..917834b1c80e 100644 --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c @@ -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(>crtc.base); - drm->irq_enabled = false; drm_mode_config_cleanup(drm); dcss_crtc_deinit(>crtc, drm); drm->dev_private = NULL; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 00/27] 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. The first 3 patches replace uses of irq_enabled that are not required. Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs (for KMS drivers). The rest of the patchset removes irq_enabled from all non-legacy drivers. The only exceptions are i915 and omapdrm, which have an internal dpendency on the field's value. For these 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. v4: * avoid preprocessor ifdef in drm_wait_vblank_ioctl() (Jani, Thierry) v3: * update armada, i915, rcar-du and vkms as well (Laurent) * optimize drm_wait_vblank_ioctl() for KMS (Liviu) * move imx/dcss changes into their own patch (Laurentiu) * doc cleanups v2: * keep the original test for legacy drivers in drm_wait_vblank_ioctl() (Daniel) Thomas Zimmermann (27): 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/armada: Don't set struct drm_device.irq_enabled drm/i915: Track IRQ state in local device state 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/imx/dcss: 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/rcar-du: Don't set struct drm_device.irq_enabled 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/vkms: 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/armada/armada_drv.c | 2 -- drivers/gpu/drm/drm_irq.c | 13 - drivers/gpu/drm/drm_vblank.c| 15 --- 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/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 8 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/rcar-du/rcar_du_drv.c | 2 -- 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/vkms/vkms_drv.c | 2 -- 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 -- 31 files changed, 40 insertions(+), 122 deletions(-) base-commit: 8c1323b422f8473421682ba783b5949ddd89a3f4 prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 06/27] drm/i915: Track IRQ state in local device state
Replace usage of struct drm_device.irq_enabled with the driver's
own state field struct drm_i915_private.irq_enabled. The field in
the DRM device structure is considered legacy and should not be
used by KMS drivers.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_irq.c | 8
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01e11fe38642..48c1835bd54b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1134,6 +1134,8 @@ struct drm_i915_private {
/* For i915gm/i945gm vblank irq workaround */
u8 vblank_enabled;
+ bool irq_enabled;
+
/* perform PHY state sanity checks? */
bool chv_phy_assert[2];
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a11bdb667241..987211f21761 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4488,14 +4488,14 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
*/
dev_priv->runtime_pm.irqs_enabled = true;
- dev_priv->drm.irq_enabled = true;
+ dev_priv->irq_enabled = true;
intel_irq_reset(dev_priv);
ret = request_irq(irq, intel_irq_handler(dev_priv),
IRQF_SHARED, DRIVER_NAME, dev_priv);
if (ret < 0) {
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
return ret;
}
@@ -4521,10 +4521,10 @@ void intel_irq_uninstall(struct drm_i915_private
*dev_priv)
* intel_modeset_driver_remove() calling us out of sequence.
* Would be nice if it didn't do that...
*/
- if (!dev_priv->drm.irq_enabled)
+ if (!dev_priv->irq_enabled)
return;
- dev_priv->drm.irq_enabled = false;
+ dev_priv->irq_enabled = false;
intel_irq_reset(dev_priv);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v10 07/10] mm: Device exclusive memory access
On Tue, Jun 15, 2021 at 01:08:11PM +1000, Alistair Popple wrote: > On Saturday, 12 June 2021 1:01:42 AM AEST Peter Xu wrote: > > On Fri, Jun 11, 2021 at 01:43:20PM +1000, Alistair Popple wrote: > > > On Friday, 11 June 2021 11:00:34 AM AEST Peter Xu wrote: > > > > On Fri, Jun 11, 2021 at 09:17:14AM +1000, Alistair Popple wrote: > > > > > On Friday, 11 June 2021 9:04:19 AM AEST Peter Xu wrote: > > > > > > On Fri, Jun 11, 2021 at 12:21:26AM +1000, Alistair Popple wrote: > > > > > > > > Hmm, the thing is.. to me FOLL_SPLIT_PMD should have similar > > > > > > > > effect to explicit > > > > > > > > call split_huge_pmd_address(), afaict. Since both of them use > > > > > > > > __split_huge_pmd() > > > > > > > > internally which will generate that unwanted CLEAR notify. > > > > > > > > > > > > > > Agree that gup calls __split_huge_pmd() via > > > > > > > split_huge_pmd_address() > > > > > > > which will always CLEAR. However gup only calls > > > > > > > split_huge_pmd_address() if it > > > > > > > finds a thp pmd. In follow_pmd_mask() we have: > > > > > > > > > > > > > > if (likely(!pmd_trans_huge(pmdval))) > > > > > > > return follow_page_pte(vma, address, pmd, flags, > > > > > > > >pgmap); > > > > > > > > > > > > > > So I don't think we have a problem here. > > > > > > > > > > > > Sorry I didn't follow here.. We do FOLL_SPLIT_PMD after this > > > > > > check, right? I > > > > > > mean, if it's a thp for the current mm, afaict pmd_trans_huge() > > > > > > should return > > > > > > true above, so we'll skip follow_page_pte(); then we'll check > > > > > > FOLL_SPLIT_PMD > > > > > > and do the split, then the CLEAR notify. Hmm.. Did I miss > > > > > > something? > > > > > > > > > > That seems correct - if the thp is not mapped with a pmd we won't > > > > > split and we > > > > > won't CLEAR. If there is a thp pmd we will split and CLEAR, but in > > > > > that case it > > > > > is fine - we will retry, but the retry will won't CLEAR because the > > > > > pmd has > > > > > already been split. > > > > > > > > Aha! > > > > > > > > > > > > > > The issue arises with doing it unconditionally in make device > > > > > exclusive is that > > > > > you *always* CLEAR even if there is no thp pmd to split. Or at least > > > > > that's my > > > > > understanding, please let me know if it doesn't make sense. > > > > > > > > Exactly. But if you see what I meant here, even if it can work like > > > > this, it > > > > sounds still fragile, isn't it? I just feel something is slightly off > > > > there.. > > > > > > > > IMHO split_huge_pmd() checked pmd before calling __split_huge_pmd() for > > > > performance, afaict, because if it's not a thp even without locking, > > > > then it > > > > won't be, so further __split_huge_pmd() is not necessary. > > > > > > > > IOW, it's very legal if someday we'd like to let split_huge_pmd() call > > > > __split_huge_pmd() directly, then AFAIU device exclusive API will be > > > > the 1st > > > > one to be broken with that seems-to-be-irrelevant change I'm afraid.. > > > > > > Well I would argue the performance of memory notifiers is becoming > > > increasingly > > > important, and a change that causes them to be called unnecessarily is > > > therefore not very legal. Likely the correct fix here is to optimise > > > __split_huge_pmd() to only call the notifier if it's actually going to > > > split a > > > pmd. As you said though that's a completely different story which I think > > > would > > > be best done as a separate series. > > > > Right, maybe I can look a bit more into that later; but my whole point was > > to > > express that one functionality shouldn't depend on such a trivial detail of > > implementation of other modules (thp split in this case). > > > > > > > > > This lets me goes back a step to think about why do we need this > > > > notifier at > > > > all to cover this whole range of make_device_exclusive() procedure.. > > > > > > > > What I am thinking is, we're afraid some CPU accesses this page so the > > > > pte got > > > > quickly restored when device atomic operation is carrying on. Then > > > > with this > > > > notifier we'll be able to cancel it. Makes perfect sense. > > > > > > > > However do we really need to register this notifier so early? The > > > > thing is the > > > > GPU driver still has all the page locks, so even if there's a race to > > > > restore > > > > the ptes, they'll block at taking the page lock until the driver > > > > releases it. > > > > > > > > IOW, I'm wondering whether the "non-fragile" way to do this is not do > > > > mmu_interval_notifier_insert() that early: what if we register that > > > > notifier > > > > after make_device_exclusive_range() returns but before page_unlock() > > > > somehow? > > > > So before page_unlock(), race is protected fully by the lock itself; > > > > after > > > > that, it's done by mmu notifier. Then maybe we don't need to worry > > > > about all > > > > these
[Nouveau] [PATCH v2 10/22] drm/mediatek: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in mediatek. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index b46bdb8985da..9b60bec33d3b 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v4 24/27] drm/vkms: Don't set struct drm_device.irq_enabled
On 06/25, Thomas Zimmermann wrote:
> The field drm_device.irq_enabled is only used by legacy drivers
> with userspace modesetting. Don't set it in vkms.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Laurent Pinchart
I've also checked here, lgtm.
Reviewed-by: Melissa Wen
> ---
> drivers/gpu/drm/vkms/vkms_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 027ffe759440..496de38ad983 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -163,8 +163,6 @@ static int vkms_create(struct vkms_config *config)
> goto out_devres;
> }
>
> - vkms_device->drm.irq_enabled = true;
> -
> ret = drm_vblank_init(_device->drm, 1);
> if (ret) {
> DRM_ERROR("Failed to vblank\n");
> --
> 2.32.0
>
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 00/22] Deprecate struct drm_device.irq_enabled
Hi Thomas, Thank you for the patches. On Tue, Jun 22, 2021 at 04:09:40PM +0200, Thomas Zimmermann wrote: > 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/20210608090301.4752-1-tzimmerm...@suse.de/ > > 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 The list seems to be missing armada, rcar-du and vkms. It would also be nice to address i915 if possible. > 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 -- Regards, Laurent Pinchart ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 27/27] drm/zte: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in zte. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/zte/zx_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c index 5506336594e2..064056503ebb 100644 --- a/drivers/gpu/drm/zte/zx_drm_drv.c +++ b/drivers/gpu/drm/zte/zx_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 09/27] drm/exynos: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in exynos. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index e60257f1f24b..d8f1cf4d6b69 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thierry Reding wrote:
> On Thu, Jun 24, 2021 at 11:07:57AM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 24.06.21 um 10:51 schrieb Jani Nikula:
>> > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>> > > Hi
>> > >
>> > > Am 24.06.21 um 10:06 schrieb Jani Nikula:
>> > > > On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
>> > > > > b/drivers/gpu/drm/drm_vblank.c
>> > > > > index 3417e1ac7918..10fe16bafcb6 100644
>> > > > > --- a/drivers/gpu/drm/drm_vblank.c
>> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
>> > > > > @@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
>> > > > > +if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
>> > > > > +if (!dev->irq_enabled)
>> > > > > +return -EOPNOTSUPP;
>> > > > > +} else /* if DRIVER_MODESET */
>> > > > > +#endif
>> > > > > +{
>> > > > > +if (!drm_dev_has_vblank(dev))
>> > > > > +return -EOPNOTSUPP;
>> > > > > +}
>> > > >
>> > > > Sheesh I hate this kind of inline #ifdefs.
>> > > >
>> > > > Two alternate suggestions that I believe should be as just efficient:
>> > >
>> > > Or how about:
>> > >
>> > > static bool drm_wait_vblank_supported(struct drm_device *dev)
>> > >
>> > > {
>> > >
>> > > if defined(CONFIG_DRM_LEGACY)
>> > > if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> > >
>> > > return dev->irq_enabled;
>> > >
>> > > #endif
>> > > return drm_dev_has_vblank(dev);
>> > >
>> > > }
>> > >
>> > >
>> > > ?
>> > >
>> > > It's inline, but still readable.
>> >
>> > It's definitely better than the original, but it's unclear to me why
>> > you'd prefer this over option 2) below. I guess the only reason I can
>> > think of is emphasizing the conditional compilation. However,
>> > IS_ENABLED() is widely used in this manner specifically to avoid inline
>> > #if, and the compiler optimizes it away.
>>
>> It's simply more readable to me as the condition is simpler. But option 2 is
>> also ok.
>
> Perhaps do something like this, then:
>
> if (IS_ENABLED(CONFIG_DRM_LEGACY)) {
> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
> return dev->irq_enabled;
> }
>
> return drm_dev_has_vblank(dev);
>
> That's about just as readable as the variant involving the preprocessor
> but has all the benefits of not using the preprocessor.
Looks like a winner to me. :)
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v10 07/10] mm: Device exclusive memory access
On Fri, Jun 11, 2021 at 12:21:26AM +1000, Alistair Popple wrote: > > Hmm, the thing is.. to me FOLL_SPLIT_PMD should have similar effect to > > explicit > > call split_huge_pmd_address(), afaict. Since both of them use > > __split_huge_pmd() > > internally which will generate that unwanted CLEAR notify. > > Agree that gup calls __split_huge_pmd() via split_huge_pmd_address() > which will always CLEAR. However gup only calls split_huge_pmd_address() if it > finds a thp pmd. In follow_pmd_mask() we have: > > if (likely(!pmd_trans_huge(pmdval))) > return follow_page_pte(vma, address, pmd, flags, >pgmap); > > So I don't think we have a problem here. Sorry I didn't follow here.. We do FOLL_SPLIT_PMD after this check, right? I mean, if it's a thp for the current mm, afaict pmd_trans_huge() should return true above, so we'll skip follow_page_pte(); then we'll check FOLL_SPLIT_PMD and do the split, then the CLEAR notify. Hmm.. Did I miss something? -- Peter Xu ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 15/27] drm/omapdrm: Track IRQ state in local device state
Replace 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/omapdrm/omap_drv.h | 2 ++
drivers/gpu/drm/omapdrm/omap_irq.c | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
b/drivers/gpu/drm/omapdrm/omap_drv.h
index d6f136984da9..591d4c273f02 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -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];
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
b/drivers/gpu/drm/omapdrm/omap_irq.c
index 15148d4b35b5..bb6e3fc18204 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -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);
}
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 10/27] drm/kirin: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers
with userspace modesetting. Don't set it in kirin.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index e590e19db657..98ae9a48f3fe 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -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);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH -next] drm/nouveau: Remove set but not used variable 'width'
ping
在 2021/5/25 16:17, Baokun Li 写道:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/gpu/drm/nouveau/nouveau_display.c: In function
'nouveau_framebuffer_new':
drivers/gpu/drm/nouveau/nouveau_display.c:309:15: warning:
variable ‘width’ set but not used [-Wunused-but-set-variable]
It never used since introduction.
Signed-off-by: Baokun Li
---
drivers/gpu/drm/nouveau/nouveau_display.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 929de41c281f..2b460835a438 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -306,7 +306,7 @@ nouveau_framebuffer_new(struct drm_device *dev,
struct nouveau_bo *nvbo = nouveau_gem_object(gem);
struct drm_framebuffer *fb;
const struct drm_format_info *info;
- unsigned int width, height, i;
+ unsigned int height, i;
uint32_t tile_mode;
uint8_t kind;
int ret;
@@ -343,9 +343,6 @@ nouveau_framebuffer_new(struct drm_device *dev,
info = drm_get_format_info(dev, mode_cmd);
for (i = 0; i < info->num_planes; i++) {
- width = drm_format_info_plane_width(info,
- mode_cmd->width,
- i);
height = drm_format_info_plane_height(info,
mode_cmd->height,
i);
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 21/27] drm/tegra: Don't set struct drm_device.irq_enabled
On Thu, Jun 24, 2021 at 09:29:10AM +0200, Thomas Zimmermann wrote: > The field drm_device.irq_enabled is only used by legacy drivers > with userspace modesetting. Don't set it in tegra. > > Signed-off-by: Thomas Zimmermann > Reviewed-by: Laurent Pinchart > Acked-by: Daniel Vetter > --- > drivers/gpu/drm/tegra/drm.c | 7 --- > 1 file changed, 7 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 00/27] 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. The first 3 patches replace uses of irq_enabled that are not required. Patch 4 fixes vblank ioctls to actually test for vblank support instead of IRQs (for KMS drivers). The rest of the patchset removes irq_enabled from all non-legacy drivers. The only exceptions are i915 and omapdrm, which have an internal dpendency on the field's value. For these 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. v3: * update armada, i915, rcar-du and vkms as well (Laurent) * optimize drm_wait_vblank_ioctl() for KMS (Liviu) * move imx/dcss changes into their own patch (Laurentiu) * doc cleanups v2: * keep the original test for legacy drivers in drm_wait_vblank_ioctl() (Daniel) Thomas Zimmermann (27): 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/armada: Don't set struct drm_device.irq_enabled drm/i915: Track IRQ state in local device state 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/imx/dcss: 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/rcar-du: Don't set struct drm_device.irq_enabled 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/vkms: 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/armada/armada_drv.c | 2 -- drivers/gpu/drm/drm_irq.c | 13 - drivers/gpu/drm/drm_vblank.c| 16 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/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_irq.c | 8 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/rcar-du/rcar_du_drv.c | 2 -- 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/vkms/vkms_drv.c | 2 -- 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 -- 31 files changed, 40 insertions(+), 123 deletions(-) base-commit: 8c1323b422f8473421682ba783b5949ddd89a3f4 prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 18/27] drm/sti: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in sti. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/sti/sti_compositor.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_compositor.c b/drivers/gpu/drm/sti/sti_compositor.c index 319962a2c17b..9caaf3ccfabe 100644 --- a/drivers/gpu/drm/sti/sti_compositor.c +++ b/drivers/gpu/drm/sti/sti_compositor.c @@ -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; } -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 02/27] 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Tian Tao
Acked-by: Daniel Vetter
---
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index f4bc5386574a..f8ef711bbe5d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -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
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 17/27] drm/rockchip: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in rockchip. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c index b730b8d5d949..c8e60fd9ff24 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c @@ -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; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
> Hi
>
> Am 24.06.21 um 10:06 schrieb Jani Nikula:
>> On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
>>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>>> index 3417e1ac7918..10fe16bafcb6 100644
>>> --- a/drivers/gpu/drm/drm_vblank.c
>>> +++ b/drivers/gpu/drm/drm_vblank.c
>>> @@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
>>> + if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
>>> + if (!dev->irq_enabled)
>>> + return -EOPNOTSUPP;
>>> + } else /* if DRIVER_MODESET */
>>> +#endif
>>> + {
>>> + if (!drm_dev_has_vblank(dev))
>>> + return -EOPNOTSUPP;
>>> + }
>>
>> Sheesh I hate this kind of inline #ifdefs.
>>
>> Two alternate suggestions that I believe should be as just efficient:
>
> Or how about:
>
> static bool drm_wait_vblank_supported(struct drm_device *dev)
>
> {
>
> if defined(CONFIG_DRM_LEGACY)
> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>
> return dev->irq_enabled;
>
> #endif
> return drm_dev_has_vblank(dev);
>
> }
>
>
> ?
>
> It's inline, but still readable.
It's definitely better than the original, but it's unclear to me why
you'd prefer this over option 2) below. I guess the only reason I can
think of is emphasizing the conditional compilation. However,
IS_ENABLED() is widely used in this manner specifically to avoid inline
#if, and the compiler optimizes it away.
BR,
Jani.
>
> Best regards
> Thomas
>
>>
>> 1) The more verbose:
>>
>> #if defined(CONFIG_DRM_LEGACY)
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> return dev->irq_enabled;
>> else
>> return drm_dev_has_vblank(dev);
>> }
>> #else
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> return drm_dev_has_vblank(dev);
>> }
>> #endif
>>
>> 2) The more compact:
>>
>> static bool drm_wait_vblank_supported(struct drm_device *dev)
>> {
>> if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
>> unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
>> return dev->irq_enabled;
>> else
>> return drm_dev_has_vblank(dev);
>> }
>>
>> Then, in drm_wait_vblank_ioctl().
>>
>> if (!drm_wait_vblank_supported(dev))
>> return -EOPNOTSUPP;
>>
>> The compiler should do the right thing without any explicit inline
>> keywords etc.
>>
>> BR,
>> Jani.
>>
>>>
>>> if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
>>> return -EINVAL;
>>> @@ -2023,7 +2031,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 +2090,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);
>>
--
Jani Nikula, Intel Open Source Graphics Center
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 05/22] drm/komeda: Don't set struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 04:09:45PM +0200, Thomas Zimmermann wrote: > 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 Acked-by: Liviu Dudau Best regards, Liviu > --- > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > index ff45f23f3d56..52a6db5707a3 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > @@ -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); > -- > 2.32.0 > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 03/27] drm/radeon: Track IRQ state in local device state
Replace 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/radeon/radeon_fence.c | 2 +-
drivers/gpu/drm/radeon/radeon_irq_kms.c | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
b/drivers/gpu/drm/radeon/radeon_fence.c
index 0d8ef2368adf..7ec581363e23 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -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;
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 84d0b1a3355f..a36ce826d0c0 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -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(>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(>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(>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(>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(>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(>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(>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(>irq.lock, irqflags);
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 12/27] drm/imx/dcss: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in dcss. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Laurentiu Palcu Acked-by: Daniel Vetter --- drivers/gpu/drm/imx/dcss/dcss-kms.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/imx/dcss/dcss-kms.c b/drivers/gpu/drm/imx/dcss/dcss-kms.c index 37ae68a7fba5..917834b1c80e 100644 --- a/drivers/gpu/drm/imx/dcss/dcss-kms.c +++ b/drivers/gpu/drm/imx/dcss/dcss-kms.c @@ -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(>crtc.base); - drm->irq_enabled = false; drm_mode_config_cleanup(drm); dcss_crtc_deinit(>crtc, drm); drm->dev_private = NULL; -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 14/27] drm/nouveau: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in nouveau. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/nouveau/nouveau_drm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index a616cf4573b8..1cb14e99a60c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -553,8 +553,6 @@ nouveau_drm_device_init(struct drm_device *dev) if (ret) goto fail_master; - dev->irq_enabled = true; - nvxx_client(>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(>client.base); device = nvkm_device_find(client->device); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v10 07/10] mm: Device exclusive memory access
On Fri, Jun 11, 2021 at 09:17:14AM +1000, Alistair Popple wrote: > On Friday, 11 June 2021 9:04:19 AM AEST Peter Xu wrote: > > External email: Use caution opening links or attachments > > > > > > On Fri, Jun 11, 2021 at 12:21:26AM +1000, Alistair Popple wrote: > > > > Hmm, the thing is.. to me FOLL_SPLIT_PMD should have similar effect to > > > > explicit > > > > call split_huge_pmd_address(), afaict. Since both of them use > > > > __split_huge_pmd() > > > > internally which will generate that unwanted CLEAR notify. > > > > > > Agree that gup calls __split_huge_pmd() via split_huge_pmd_address() > > > which will always CLEAR. However gup only calls split_huge_pmd_address() > > > if it > > > finds a thp pmd. In follow_pmd_mask() we have: > > > > > > if (likely(!pmd_trans_huge(pmdval))) > > > return follow_page_pte(vma, address, pmd, flags, > > > >pgmap); > > > > > > So I don't think we have a problem here. > > > > Sorry I didn't follow here.. We do FOLL_SPLIT_PMD after this check, right? > > I > > mean, if it's a thp for the current mm, afaict pmd_trans_huge() should > > return > > true above, so we'll skip follow_page_pte(); then we'll check FOLL_SPLIT_PMD > > and do the split, then the CLEAR notify. Hmm.. Did I miss something? > > That seems correct - if the thp is not mapped with a pmd we won't split and we > won't CLEAR. If there is a thp pmd we will split and CLEAR, but in that case > it > is fine - we will retry, but the retry will won't CLEAR because the pmd has > already been split. Aha! > > The issue arises with doing it unconditionally in make device exclusive is > that > you *always* CLEAR even if there is no thp pmd to split. Or at least that's my > understanding, please let me know if it doesn't make sense. Exactly. But if you see what I meant here, even if it can work like this, it sounds still fragile, isn't it? I just feel something is slightly off there.. IMHO split_huge_pmd() checked pmd before calling __split_huge_pmd() for performance, afaict, because if it's not a thp even without locking, then it won't be, so further __split_huge_pmd() is not necessary. IOW, it's very legal if someday we'd like to let split_huge_pmd() call __split_huge_pmd() directly, then AFAIU device exclusive API will be the 1st one to be broken with that seems-to-be-irrelevant change I'm afraid.. This lets me goes back a step to think about why do we need this notifier at all to cover this whole range of make_device_exclusive() procedure.. What I am thinking is, we're afraid some CPU accesses this page so the pte got quickly restored when device atomic operation is carrying on. Then with this notifier we'll be able to cancel it. Makes perfect sense. However do we really need to register this notifier so early? The thing is the GPU driver still has all the page locks, so even if there's a race to restore the ptes, they'll block at taking the page lock until the driver releases it. IOW, I'm wondering whether the "non-fragile" way to do this is not do mmu_interval_notifier_insert() that early: what if we register that notifier after make_device_exclusive_range() returns but before page_unlock() somehow? So before page_unlock(), race is protected fully by the lock itself; after that, it's done by mmu notifier. Then maybe we don't need to worry about all these notifications during marking exclusive (while we shouldn't)? Sorry in advance if I overlooked anything as I know little on device side (even less than mm itself). Also sorry to know that this series got marked to-be-update in -mm; hopefully it'll still land soon even if it still needs some rebase to other more important bugfixes - I definitely jumped in too late even if to mess this all up. :-) -- Peter Xu ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 05/22] drm/komeda: Don't set struct drm_device.irq_enabled
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 --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index ff45f23f3d56..52a6db5707a3 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v3 04/27] drm: Don't test for IRQ support in VBLANK ioctls
Hi
Am 24.06.21 um 10:06 schrieb Jani Nikula:
On Thu, 24 Jun 2021, Thomas Zimmermann wrote:
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().
v3:
* optimize test in drm_wait_vblank_ioctl() for KMS case (Liviu)
* update docs for drm_irq_uninstall()
v2:
* keep the old test for legacy drivers in
drm_wait_vblank_ioctl() (Daniel)
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/drm_irq.c| 13 -
drivers/gpu/drm/drm_vblank.c | 16
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c3bd664ea733..945dd82e2ea3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -74,10 +74,8 @@
* only supports devices with a single interrupt on the main device stored in
* _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 _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 _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.
@@ -156,8 +152,7 @@ EXPORT_SYMBOL(drm_irq_install);
*
* Calls the driver's _driver.irq_uninstall function and unregisters the
IRQ
* handler. This should only be called by drivers which used
drm_irq_install()
- * to set up their interrupt handler. Other drivers must only reset
- * _device.irq_enabled to false.
+ * to set up their interrupt handler.
*
* Note that for kernel modesetting drivers it is a bug if this function
fails.
* The sanity checks are only to catch buggy user modesetting drivers which
call
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 3417e1ac7918..10fe16bafcb6 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1748,8 +1748,16 @@ 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 defined(CONFIG_DRM_LEGACY)
+ if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY))) {
+ if (!dev->irq_enabled)
+ return -EOPNOTSUPP;
+ } else /* if DRIVER_MODESET */
+#endif
+ {
+ if (!drm_dev_has_vblank(dev))
+ return -EOPNOTSUPP;
+ }
Sheesh I hate this kind of inline #ifdefs.
I don't like them either. I guess I'll go with suggestion 2. Thanks for
the feedback.
Best regards
Thomas
Two alternate suggestions that I believe should be as just efficient:
1) The more verbose:
#if defined(CONFIG_DRM_LEGACY)
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
#else
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
return drm_dev_has_vblank(dev);
}
#endif
2) The more compact:
static bool drm_wait_vblank_supported(struct drm_device *dev)
{
if (IS_ENABLED(CONFIG_DRM_LEGACY) &&
unlikely(drm_core_check_feature(dev, DRIVER_LEGACY)))
return dev->irq_enabled;
else
return drm_dev_has_vblank(dev);
}
Then, in drm_wait_vblank_ioctl().
if (!drm_wait_vblank_supported(dev))
return -EOPNOTSUPP;
The compiler should do the right thing without any explicit inline
keywords etc.
BR,
Jani.
if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
return -EINVAL;
@@ -2023,7 +2031,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
Re: [Nouveau] [PATCH v2 00/22] Deprecate struct drm_device.irq_enabled
On Tue, Jun 22, 2021 at 04:09:40PM +0200, Thomas Zimmermann wrote: > 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/20210608090301.4752-1-tzimmerm...@suse.de/ On the series: Acked-by: Daniel Vetter 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 > > 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 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v3 01/27] 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.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Laurent Pinchart
Acked-by: Daniel Vetter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 32ce0e679dc7..7dad44e73cf6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -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)
--
2.32.0
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v4 19/27] drm/stm: Don't set struct drm_device.irq_enabled
The field drm_device.irq_enabled is only used by legacy drivers with userspace modesetting. Don't set it in stm. Signed-off-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart Acked-by: Daniel Vetter --- drivers/gpu/drm/stm/ltdc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 08b71248044d..e9c5a52f041a 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -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); -- 2.32.0 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
