Hi all,
This patchset fixes several issues around unloading/unbinding the driver. There is still one WARNING when unloading the driver while vblank interrupts are enabled. I am not sure what/who should make sure that vblank interrupts get disabled before unloading the driver:
root@colibri-vf:~# echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/unbind [ 37.209656] Console: switching to colour dummy device 80x30 [ 37.224954] ------------[ cut here ]------------ [ 37.245375] WARNING: CPU: 0 PID: 170 at drivers/gpu/drm/drm_irq.c:380 drm_vblank_cleanup+0x5c/0x8c ...
Hints welcome...
Stefan Agner (6): drm/fsl-dcu: detach panel on destroy drm/fsl-dcu: handle missing panel gracefully drm/fsl-dcu: use variable name dev for struct drm_device drm/fsl-dcu: deallocate fbdev CMA on unload drm/fsl-dcu: disable output polling on driver unload drm/fsl-dcu: implement lastclose callback
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 50 +++++++++++++++++++++---------- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 23 +++++++++----- 2 files changed, 50 insertions(+), 23 deletions(-)
Disable the earlier attached panel on connector destroy.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index f586f1e..a07886f 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c @@ -79,7 +79,10 @@ int fsl_dcu_drm_encoder_create(struct fsl_dcu_drm_device *fsl_dev,
static void fsl_dcu_drm_connector_destroy(struct drm_connector *connector) { + struct fsl_dcu_drm_connector *fsl_con = to_fsl_dcu_connector(connector); + drm_connector_unregister(connector); + drm_panel_detach(fsl_con->panel); drm_connector_cleanup(connector); }
If the device tree property fsl,panel is missing, drm_panel_attach is called with a NULL pointer as first argument. Having a panel is basically mandatory since RGB is the only supported connector. Check if a panel node has been found, return -ENODEV and cleanup otherwise.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c index a07886f..819fe12 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c @@ -171,14 +171,18 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev, DRM_MODE_DPMS_OFF);
panel_node = of_parse_phandle(fsl_dev->np, "fsl,panel", 0); - if (panel_node) { - fsl_dev->connector.panel = of_drm_find_panel(panel_node); - if (!fsl_dev->connector.panel) { - ret = -EPROBE_DEFER; - goto err_sysfs; - } - of_node_put(panel_node); + if (!panel_node) { + dev_err(fsl_dev->dev, "fsl,panel property not found\n"); + ret = -ENODEV; + goto err_sysfs; + } + + fsl_dev->connector.panel = of_drm_find_panel(panel_node); + if (!fsl_dev->connector.panel) { + ret = -EPROBE_DEFER; + goto err_panel; } + of_node_put(panel_node);
ret = drm_panel_attach(fsl_dev->connector.panel, connector); if (ret) { @@ -188,6 +192,8 @@ int fsl_dcu_drm_connector_create(struct fsl_dcu_drm_device *fsl_dev,
return 0;
+err_panel: + of_node_put(panel_node); err_sysfs: drm_connector_unregister(connector); err_cleanup:
The driver uses different variable names for struct drm_device across functions which is confusing. Stick to the more common variable name dev. While at it, remove unnecessary if statement in error handling.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index f62bff2..9cbabb2 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -63,40 +63,37 @@ static int fsl_dcu_drm_irq_init(struct drm_device *dev) return ret; }
-static int fsl_dcu_load(struct drm_device *drm, unsigned long flags) +static int fsl_dcu_load(struct drm_device *dev, unsigned long flags) { - struct device *dev = drm->dev; - struct fsl_dcu_drm_device *fsl_dev = drm->dev_private; + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; int ret;
ret = fsl_dcu_drm_modeset_init(fsl_dev); if (ret < 0) { - dev_err(dev, "failed to initialize mode setting\n"); + dev_err(dev->dev, "failed to initialize mode setting\n"); return ret; }
- ret = drm_vblank_init(drm, drm->mode_config.num_crtc); + ret = drm_vblank_init(dev, dev->mode_config.num_crtc); if (ret < 0) { - dev_err(dev, "failed to initialize vblank\n"); + dev_err(dev->dev, "failed to initialize vblank\n"); goto done; } - drm->vblank_disable_allowed = true; + dev->vblank_disable_allowed = true;
- ret = fsl_dcu_drm_irq_init(drm); + ret = fsl_dcu_drm_irq_init(dev); if (ret < 0) goto done; - drm->irq_enabled = true; + dev->irq_enabled = true;
- fsl_dcu_fbdev_init(drm); + fsl_dcu_fbdev_init(dev);
return 0; done: - if (ret) { - drm_mode_config_cleanup(drm); - drm_vblank_cleanup(drm); - drm_irq_uninstall(drm); - drm->dev_private = NULL; - } + drm_mode_config_cleanup(dev); + drm_vblank_cleanup(dev); + drm_irq_uninstall(dev); + dev->dev_private = NULL;
return ret; }
Free fbdev CMA using drm_fbdev_cma_fini on unload. This fixes a warning when unloading the driver: WARNING: CPU: 0 PID: 164 at drivers/gpu/drm/drm_crtc.c:5930 drm_mode_config_cleanup+0x204/0x208
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 9cbabb2..182578d 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -23,6 +23,7 @@
#include <drm/drmP.h> #include <drm/drm_crtc_helper.h> +#include <drm/drm_fb_cma_helper.h> #include <drm/drm_gem_cma_helper.h>
#include "fsl_dcu_drm_crtc.h" @@ -90,6 +91,9 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
return 0; done: + if (fsl_dev->fbdev) + drm_fbdev_cma_fini(fsl_dev->fbdev); + drm_mode_config_cleanup(dev); drm_vblank_cleanup(dev); drm_irq_uninstall(dev); @@ -100,6 +104,11 @@ done:
static int fsl_dcu_unload(struct drm_device *dev) { + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + + if (fsl_dev->fbdev) + drm_fbdev_cma_fini(fsl_dev->fbdev); + drm_mode_config_cleanup(dev); drm_vblank_cleanup(dev); drm_irq_uninstall(dev);
Disabling output polling before unloading the driver.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 182578d..f9227b7 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -91,6 +91,8 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
return 0; done: + drm_kms_helper_poll_fini(dev); + if (fsl_dev->fbdev) drm_fbdev_cma_fini(fsl_dev->fbdev);
@@ -106,6 +108,8 @@ static int fsl_dcu_unload(struct drm_device *dev) { struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ drm_kms_helper_poll_fini(dev); + if (fsl_dev->fbdev) drm_fbdev_cma_fini(fsl_dev->fbdev);
Use CMA helper drm_fbdev_cma_restore_mode to restore fbdev mode in process which uses drm/kms dies.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index f9227b7..ef41bde 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -168,6 +168,13 @@ static void fsl_dcu_drm_disable_vblank(struct drm_device *dev, regmap_write(fsl_dev->regmap, DCU_INT_MASK, value); }
+static void fsl_dcu_drm_lastclose(struct drm_device *dev) +{ + struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + + drm_fbdev_cma_restore_mode(fsl_dev->fbdev); +} + static const struct file_operations fsl_dcu_drm_fops = { .owner = THIS_MODULE, .open = drm_open, @@ -185,6 +192,7 @@ static const struct file_operations fsl_dcu_drm_fops = { static struct drm_driver fsl_dcu_drm_driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC, + .lastclose = fsl_dcu_drm_lastclose, .load = fsl_dcu_load, .unload = fsl_dcu_unload, .irq_handler = fsl_dcu_drm_irq,
Applied patchset to my fsl-dcu tree.
-- Stefan
On 2016-04-16 22:25, Stefan Agner wrote:
Hi all,
This patchset fixes several issues around unloading/unbinding the driver. There is still one WARNING when unloading the driver while vblank interrupts are enabled. I am not sure what/who should make sure that vblank interrupts get disabled before unloading the driver:
root@colibri-vf:~# echo 40058000.dcu > /sys/bus/platform/drivers/fsl-dcu/unbind [ 37.209656] Console: switching to colour dummy device 80x30 [ 37.224954] ------------[ cut here ]------------ [ 37.245375] WARNING: CPU: 0 PID: 170 at drivers/gpu/drm/drm_irq.c:380 drm_vblank_cleanup+0x5c/0x8c ...
Hints welcome...
Stefan Agner (6): drm/fsl-dcu: detach panel on destroy drm/fsl-dcu: handle missing panel gracefully drm/fsl-dcu: use variable name dev for struct drm_device drm/fsl-dcu: deallocate fbdev CMA on unload drm/fsl-dcu: disable output polling on driver unload drm/fsl-dcu: implement lastclose callback
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 50 +++++++++++++++++++++---------- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 23 +++++++++----- 2 files changed, 50 insertions(+), 23 deletions(-)
dri-devel@lists.freedesktop.org
-
Stefan Agner