* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
The tilcdc driver could be compiled as a module, but was severely broken
and could not be used as such. This patchset attempts to fix the issues
preventing a proper load/unload of the module.
Issues included dangling sysfs nodes, dangling devices, memory leaks and
a double kfree.
It now seems to be working ok. We have tested this by loading and
unloading the driver repeteadly, with both panel and slave connectors
and found no flaws.
There is still one warning left on tilcdc_crtc_destroy, caused by
destroying the connector while still in an ON status. We don't know why
this happens or why it's an issue, so we did not fix it.
The first 7 patches are bug fixes which and should probably be applied
in the stable trees. The last two are clean-ups.
Resending this since I got no replies.
Guido Mart?nez (9):
drm/i2c: tda998x: move drm_i2c_encoder_destroy call
drm/tilcdc: panel: fix dangling sysfs connector node
drm/tilcdc: slave: fix dangling sysfs connector node
drm/tilcdc: tfp410: fix dangling sysfs connector node
drm/tilcdc: panel: fix leak when unloading the module
drm/tilcdc: fix release order on exit
drm/tilcdc: fix double kfree
drm/tilcdc: remove submodule destroy calls
drm/tilcdc: replace late_initcall with module_init
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
drivers/gpu/drm/tilcdc/Module.symvers | 0
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
7 files changed, 59 insertions(+), 60 deletions(-)
create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
--
2.0.0
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: robdclark, Ezequiel García, airlied, Daniel Mack,
Russell King, Daniel Vetter, Guido Martínez, stable
Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
as part of the release procedure. Such calls need to access the I2C bus
and therefore, we need to call them before drm_i2c_encoder_destroy()
which unregisters the I2C device.
This commit moves the latter so it's done afterwards.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Signed-off-by: Ezequiel García <ezequiel@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 240c331..db9515f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1183,7 +1183,6 @@ static void
tda998x_encoder_destroy(struct drm_encoder *encoder)
{
struct tda998x_priv *priv = to_tda998x_priv(encoder);
- drm_i2c_encoder_destroy(encoder);
/* disable all IRQs and free the IRQ handler */
cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
@@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
if (priv->cec)
i2c_unregister_device(priv->cec);
+ drm_i2c_encoder_destroy(encoder);
kfree(priv);
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
as part of the release procedure. Such calls need to access the I2C bus
and therefore, we need to call them before drm_i2c_encoder_destroy()
which unregisters the I2C device.
This commit moves the latter so it's done afterwards.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Signed-off-by: Ezequiel Garc?a <ezequiel@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 240c331..db9515f 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1183,7 +1183,6 @@ static void
tda998x_encoder_destroy(struct drm_encoder *encoder)
{
struct tda998x_priv *priv = to_tda998x_priv(encoder);
- drm_i2c_encoder_destroy(encoder);
/* disable all IRQs and free the IRQ handler */
cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
@@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
if (priv->cec)
i2c_unregister_device(priv->cec);
+ drm_i2c_encoder_destroy(encoder);
kfree(priv);
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-24 16:38 ` Russell King - ARM Linux
-1 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-24 16:38 UTC (permalink / raw)
To: Guido Martínez
Cc: dri-devel, linux-arm-kernel, linux-omap, robdclark,
Ezequiel García, airlied, Daniel Mack, Daniel Vetter,
stable
On Tue, Jun 17, 2014 at 11:17:03AM -0300, Guido Martínez wrote:
> Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
> as part of the release procedure. Such calls need to access the I2C bus
> and therefore, we need to call them before drm_i2c_encoder_destroy()
> which unregisters the I2C device.
>
> This commit moves the latter so it's done afterwards.
>
> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> Signed-off-by: Ezequiel García <ezequiel@vanguardiasur.com.ar>
> Cc: <stable@vger.kernel.org> #v3.9+
You really should have sent this with me in the To: header as I'm now the
maintainer of this driver. Yes, this is a valid fix, and I'll apply it
shortly. Thanks.
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 240c331..db9515f 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1183,7 +1183,6 @@ static void
> tda998x_encoder_destroy(struct drm_encoder *encoder)
> {
> struct tda998x_priv *priv = to_tda998x_priv(encoder);
> - drm_i2c_encoder_destroy(encoder);
>
> /* disable all IRQs and free the IRQ handler */
> cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
> @@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
>
> if (priv->cec)
> i2c_unregister_device(priv->cec);
> + drm_i2c_encoder_destroy(encoder);
> kfree(priv);
> }
>
> --
> 2.0.0
>
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
@ 2014-06-24 16:38 ` Russell King - ARM Linux
0 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-24 16:38 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 17, 2014 at 11:17:03AM -0300, Guido Mart?nez wrote:
> Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
> as part of the release procedure. Such calls need to access the I2C bus
> and therefore, we need to call them before drm_i2c_encoder_destroy()
> which unregisters the I2C device.
>
> This commit moves the latter so it's done afterwards.
>
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> Signed-off-by: Ezequiel Garc?a <ezequiel@vanguardiasur.com.ar>
> Cc: <stable@vger.kernel.org> #v3.9+
You really should have sent this with me in the To: header as I'm now the
maintainer of this driver. Yes, this is a valid fix, and I'll apply it
shortly. Thanks.
> ---
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 240c331..db9515f 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1183,7 +1183,6 @@ static void
> tda998x_encoder_destroy(struct drm_encoder *encoder)
> {
> struct tda998x_priv *priv = to_tda998x_priv(encoder);
> - drm_i2c_encoder_destroy(encoder);
>
> /* disable all IRQs and free the IRQ handler */
> cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
> @@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
>
> if (priv->cec)
> i2c_unregister_device(priv->cec);
> + drm_i2c_encoder_destroy(encoder);
> kfree(priv);
> }
>
> --
> 2.0.0
>
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
2014-06-24 16:38 ` Russell King - ARM Linux
@ 2014-06-25 3:55 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-25 3:55 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: dri-devel, linux-arm-kernel, linux-omap, robdclark,
Ezequiel García, airlied, Daniel Mack, Daniel Vetter,
stable
Hi Russell,
On Tue, Jun 24, 2014 at 05:38:13PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 17, 2014 at 11:17:03AM -0300, Guido Martínez wrote:
> > Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
> > as part of the release procedure. Such calls need to access the I2C bus
> > and therefore, we need to call them before drm_i2c_encoder_destroy()
> > which unregisters the I2C device.
> >
> > This commit moves the latter so it's done afterwards.
> >
> > Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> > Signed-off-by: Ezequiel García <ezequiel@vanguardiasur.com.ar>
> > Cc: <stable@vger.kernel.org> #v3.9+
>
> You really should have sent this with me in the To: header as I'm now the
> maintainer of this driver. Yes, this is a valid fix, and I'll apply it
> shortly. Thanks.
Sorry about that, I'm still kind of new to this whole deal. I'll keep it
mind for future patches.
Thanks,
Guido
> > ---
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > index 240c331..db9515f 100644
> > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > @@ -1183,7 +1183,6 @@ static void
> > tda998x_encoder_destroy(struct drm_encoder *encoder)
> > {
> > struct tda998x_priv *priv = to_tda998x_priv(encoder);
> > - drm_i2c_encoder_destroy(encoder);
> >
> > /* disable all IRQs and free the IRQ handler */
> > cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
> > @@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
> >
> > if (priv->cec)
> > i2c_unregister_device(priv->cec);
> > + drm_i2c_encoder_destroy(encoder);
> > kfree(priv);
> > }
> >
> > --
> > 2.0.0
> >
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 1/9] drm/i2c: tda998x: move drm_i2c_encoder_destroy call
@ 2014-06-25 3:55 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-25 3:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell,
On Tue, Jun 24, 2014 at 05:38:13PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 17, 2014 at 11:17:03AM -0300, Guido Mart?nez wrote:
> > Currently tda998x_encoder_destroy() calls cec_write() and reg_clear(),
> > as part of the release procedure. Such calls need to access the I2C bus
> > and therefore, we need to call them before drm_i2c_encoder_destroy()
> > which unregisters the I2C device.
> >
> > This commit moves the latter so it's done afterwards.
> >
> > Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> > Signed-off-by: Ezequiel Garc?a <ezequiel@vanguardiasur.com.ar>
> > Cc: <stable@vger.kernel.org> #v3.9+
>
> You really should have sent this with me in the To: header as I'm now the
> maintainer of this driver. Yes, this is a valid fix, and I'll apply it
> shortly. Thanks.
Sorry about that, I'm still kind of new to this whole deal. I'll keep it
mind for future patches.
Thanks,
Guido
> > ---
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> > index 240c331..db9515f 100644
> > --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> > +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> > @@ -1183,7 +1183,6 @@ static void
> > tda998x_encoder_destroy(struct drm_encoder *encoder)
> > {
> > struct tda998x_priv *priv = to_tda998x_priv(encoder);
> > - drm_i2c_encoder_destroy(encoder);
> >
> > /* disable all IRQs and free the IRQ handler */
> > cec_write(priv, REG_CEC_RXSHPDINTENA, 0);
> > @@ -1193,6 +1192,7 @@ tda998x_encoder_destroy(struct drm_encoder *encoder)
> >
> > if (priv->cec)
> > i2c_unregister_device(priv->cec);
> > + drm_i2c_encoder_destroy(encoder);
> > kfree(priv);
> > }
> >
> > --
> > 2.0.0
> >
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 2/9] drm/tilcdc: panel: fix dangling sysfs connector node
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, stable, Daniel Mack, Ezequiel García
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver as a
module. Without this, we would get a warning at re-load time like so:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 824 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x74()
sysfs: cannot create duplicate filename '/class/drm/card0-LVDS-1'
Modules linked in: [...]
CPU: 0 PID: 824 Comm: modprobe Not tainted 3.15.0-rc4-00027-g6484f96-dirty #81
[<c0013bb8>] (unwind_backtrace) from [<c0011824>] (show_stack+0x10/0x14)
[<c0011824>] (show_stack) from [<c0034e8c>] (warn_slowpath_common+0x68/0x88)
[<c0034e8c>] (warn_slowpath_common) from [<c0034edc>] (warn_slowpath_fmt+0x30/0x40)
[<c0034edc>] (warn_slowpath_fmt) from [<c01243f4>] (sysfs_warn_dup+0x54/0x74)
[<c01243f4>] (sysfs_warn_dup) from [<c0124708>] (sysfs_do_create_link_sd.isra.2+0xb0/0xb8)
[<c0124708>] (sysfs_do_create_link_sd.isra.2) from [<c02ae37c>] (device_add+0x338/0x520)
[<c02ae37c>] (device_add) from [<c02ae6e8>] (device_create_groups_vargs+0xa0/0xc4)
[<c02ae6e8>] (device_create_groups_vargs) from [<c02ae758>] (device_create+0x24/0x2c)
[<c02ae758>] (device_create) from [<c029b4ec>] (drm_sysfs_connector_add+0x64/0x204)
[<c029b4ec>] (drm_sysfs_connector_add) from [<bf0b1fec>] (panel_modeset_init+0xb8/0x134 [tilcdc])
[<bf0b1fec>] (panel_modeset_init [tilcdc]) from [<bf0b2bf0>] (tilcdc_load+0x214/0x4c0 [tilcdc])
[<bf0b2bf0>] (tilcdc_load [tilcdc]) from [<c029955c>] (drm_dev_register+0xa4/0x104)
[ .. snip .. ]
---[ end trace b2d09cd9578b0497 ]---
[drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -17
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 86c6732..1943b2f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -151,6 +151,7 @@ struct panel_connector {
static void panel_connector_destroy(struct drm_connector *connector)
{
struct panel_connector *panel_connector = to_panel_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(panel_connector);
}
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 2/9] drm/tilcdc: panel: fix dangling sysfs connector node
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver as a
module. Without this, we would get a warning at re-load time like so:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 824 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x74()
sysfs: cannot create duplicate filename '/class/drm/card0-LVDS-1'
Modules linked in: [...]
CPU: 0 PID: 824 Comm: modprobe Not tainted 3.15.0-rc4-00027-g6484f96-dirty #81
[<c0013bb8>] (unwind_backtrace) from [<c0011824>] (show_stack+0x10/0x14)
[<c0011824>] (show_stack) from [<c0034e8c>] (warn_slowpath_common+0x68/0x88)
[<c0034e8c>] (warn_slowpath_common) from [<c0034edc>] (warn_slowpath_fmt+0x30/0x40)
[<c0034edc>] (warn_slowpath_fmt) from [<c01243f4>] (sysfs_warn_dup+0x54/0x74)
[<c01243f4>] (sysfs_warn_dup) from [<c0124708>] (sysfs_do_create_link_sd.isra.2+0xb0/0xb8)
[<c0124708>] (sysfs_do_create_link_sd.isra.2) from [<c02ae37c>] (device_add+0x338/0x520)
[<c02ae37c>] (device_add) from [<c02ae6e8>] (device_create_groups_vargs+0xa0/0xc4)
[<c02ae6e8>] (device_create_groups_vargs) from [<c02ae758>] (device_create+0x24/0x2c)
[<c02ae758>] (device_create) from [<c029b4ec>] (drm_sysfs_connector_add+0x64/0x204)
[<c029b4ec>] (drm_sysfs_connector_add) from [<bf0b1fec>] (panel_modeset_init+0xb8/0x134 [tilcdc])
[<bf0b1fec>] (panel_modeset_init [tilcdc]) from [<bf0b2bf0>] (tilcdc_load+0x214/0x4c0 [tilcdc])
[<bf0b2bf0>] (tilcdc_load [tilcdc]) from [<c029955c>] (drm_dev_register+0xa4/0x104)
[ .. snip .. ]
---[ end trace b2d09cd9578b0497 ]---
[drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -17
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 86c6732..1943b2f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -151,6 +151,7 @@ struct panel_connector {
static void panel_connector_destroy(struct drm_connector *connector)
{
struct panel_connector *panel_connector = to_panel_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(panel_connector);
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 3/9] drm/tilcdc: slave: fix dangling sysfs connector node
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, stable, Daniel Mack, Ezequiel García
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver as a
module. Without this, we would get a warning at re-load time like so:
tda998x 0-0070: found TDA19988
------------[ cut here ]------------
WARNING: CPU: 0 PID: 825 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x74()
sysfs: cannot create duplicate filename '/class/drm/card0-HDMI-A-1'
Modules linked in: [..]
CPU: 0 PID: 825 Comm: modprobe Not tainted 3.15.0-rc4-00027-g9dcdef4 #82
[<c0013bb8>] (unwind_backtrace) from [<c0011824>] (show_stack+0x10/0x14)
[<c0011824>] (show_stack) from [<c0034e8c>] (warn_slowpath_common+0x68/0x88)
[<c0034e8c>] (warn_slowpath_common) from [<c0034edc>] (warn_slowpath_fmt+0x30/0x40)
[<c0034edc>] (warn_slowpath_fmt) from [<c01243f4>] (sysfs_warn_dup+0x54/0x74)
[<c01243f4>] (sysfs_warn_dup) from [<c0124708>] (sysfs_do_create_link_sd.isra.2+0xb0/0xb8)
[<c0124708>] (sysfs_do_create_link_sd.isra.2) from [<c02ae37c>] (device_add+0x338/0x520)
[<c02ae37c>] (device_add) from [<c02ae6e8>] (device_create_groups_vargs+0xa0/0xc4)
[<c02ae6e8>] (device_create_groups_vargs) from [<c02ae758>] (device_create+0x24/0x2c)
[<c02ae758>] (device_create) from [<c029b4ec>] (drm_sysfs_connector_add+0x64/0x204)
[<c029b4ec>] (drm_sysfs_connector_add) from [<bf0b1b40>] (slave_modeset_init+0x120/0x1bc [tilcdc])
[<bf0b1b40>] (slave_modeset_init [tilcdc]) from [<bf0b2be8>] (tilcdc_load+0x214/0x4c0 [tilcdc])
[<bf0b2be8>] (tilcdc_load [tilcdc]) from [<c029955c>] (drm_dev_register+0xa4/0x104)
[..snip..]
---[ end trace 4df8d614936ebdee ]---
[drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -17
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index 595068b..2f83ffb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -166,6 +166,7 @@ struct slave_connector {
static void slave_connector_destroy(struct drm_connector *connector)
{
struct slave_connector *slave_connector = to_slave_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(slave_connector);
}
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 3/9] drm/tilcdc: slave: fix dangling sysfs connector node
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver as a
module. Without this, we would get a warning at re-load time like so:
tda998x 0-0070: found TDA19988
------------[ cut here ]------------
WARNING: CPU: 0 PID: 825 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x74()
sysfs: cannot create duplicate filename '/class/drm/card0-HDMI-A-1'
Modules linked in: [..]
CPU: 0 PID: 825 Comm: modprobe Not tainted 3.15.0-rc4-00027-g9dcdef4 #82
[<c0013bb8>] (unwind_backtrace) from [<c0011824>] (show_stack+0x10/0x14)
[<c0011824>] (show_stack) from [<c0034e8c>] (warn_slowpath_common+0x68/0x88)
[<c0034e8c>] (warn_slowpath_common) from [<c0034edc>] (warn_slowpath_fmt+0x30/0x40)
[<c0034edc>] (warn_slowpath_fmt) from [<c01243f4>] (sysfs_warn_dup+0x54/0x74)
[<c01243f4>] (sysfs_warn_dup) from [<c0124708>] (sysfs_do_create_link_sd.isra.2+0xb0/0xb8)
[<c0124708>] (sysfs_do_create_link_sd.isra.2) from [<c02ae37c>] (device_add+0x338/0x520)
[<c02ae37c>] (device_add) from [<c02ae6e8>] (device_create_groups_vargs+0xa0/0xc4)
[<c02ae6e8>] (device_create_groups_vargs) from [<c02ae758>] (device_create+0x24/0x2c)
[<c02ae758>] (device_create) from [<c029b4ec>] (drm_sysfs_connector_add+0x64/0x204)
[<c029b4ec>] (drm_sysfs_connector_add) from [<bf0b1b40>] (slave_modeset_init+0x120/0x1bc [tilcdc])
[<bf0b1b40>] (slave_modeset_init [tilcdc]) from [<bf0b2be8>] (tilcdc_load+0x214/0x4c0 [tilcdc])
[<bf0b2be8>] (tilcdc_load [tilcdc]) from [<c029955c>] (drm_dev_register+0xa4/0x104)
[..snip..]
---[ end trace 4df8d614936ebdee ]---
[drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -17
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index 595068b..2f83ffb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -166,6 +166,7 @@ struct slave_connector {
static void slave_connector_destroy(struct drm_connector *connector)
{
struct slave_connector *slave_connector = to_slave_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(slave_connector);
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 4/9] drm/tilcdc: tfp410: fix dangling sysfs connector node
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, stable, Daniel Mack, Ezequiel García
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver, otherwise
we will get a warning about a duplicate filename in sysfs.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index c38b56b..ce75ac8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -167,6 +167,7 @@ struct tfp410_connector {
static void tfp410_connector_destroy(struct drm_connector *connector)
{
struct tfp410_connector *tfp410_connector = to_tfp410_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(tfp410_connector);
}
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 4/9] drm/tilcdc: tfp410: fix dangling sysfs connector node
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Add a drm_sysfs_connector_remove call when we destroy the panel to make
sure the connector node in sysfs gets deleted.
This is required for proper unload and re-load of this driver, otherwise
we will get a warning about a duplicate filename in sysfs.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index c38b56b..ce75ac8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -167,6 +167,7 @@ struct tfp410_connector {
static void tfp410_connector_destroy(struct drm_connector *connector)
{
struct tfp410_connector *tfp410_connector = to_tfp410_connector(connector);
+ drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
kfree(tfp410_connector);
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 5/9] drm/tilcdc: panel: fix leak when unloading the module
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: robdclark, Ezequiel García, airlied, Daniel Mack,
Russell King, Daniel Vetter, Guido Martínez, stable
The driver did not unregister the allocated framebuffer, which caused
memory leaks (and memory manager WARNs) when unloading. Also, the
framebuffer device under /dev still existed after unloading.
Add a call to drm_fbdev_cma_fini when unloading the module to prevent
both issues.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b20b694..490aee7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -122,6 +122,7 @@ static int tilcdc_unload(struct drm_device *dev)
struct tilcdc_drm_private *priv = dev->dev_private;
struct tilcdc_module *mod, *cur;
+ drm_fbdev_cma_fini(priv->fbdev);
drm_kms_helper_poll_fini(dev);
drm_mode_config_cleanup(dev);
drm_vblank_cleanup(dev);
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 5/9] drm/tilcdc: panel: fix leak when unloading the module
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
The driver did not unregister the allocated framebuffer, which caused
memory leaks (and memory manager WARNs) when unloading. Also, the
framebuffer device under /dev still existed after unloading.
Add a call to drm_fbdev_cma_fini when unloading the module to prevent
both issues.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b20b694..490aee7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -122,6 +122,7 @@ static int tilcdc_unload(struct drm_device *dev)
struct tilcdc_drm_private *priv = dev->dev_private;
struct tilcdc_module *mod, *cur;
+ drm_fbdev_cma_fini(priv->fbdev);
drm_kms_helper_poll_fini(dev);
drm_mode_config_cleanup(dev);
drm_vblank_cleanup(dev);
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 6/9] drm/tilcdc: fix release order on exit
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, stable, Daniel Mack, Ezequiel García
Unregister resources in the correct order on tilcdc_drm_fini, which is
the reverse order they were registered during tilcdc_drm_init.
This also means unregistering the driver before releasing its resources.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 490aee7..006a30e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -629,10 +629,10 @@ static int __init tilcdc_drm_init(void)
static void __exit tilcdc_drm_fini(void)
{
DBG("fini");
- tilcdc_tfp410_fini();
- tilcdc_slave_fini();
- tilcdc_panel_fini();
platform_driver_unregister(&tilcdc_platform_driver);
+ tilcdc_panel_fini();
+ tilcdc_slave_fini();
+ tilcdc_tfp410_fini();
}
late_initcall(tilcdc_drm_init);
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 6/9] drm/tilcdc: fix release order on exit
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Unregister resources in the correct order on tilcdc_drm_fini, which is
the reverse order they were registered during tilcdc_drm_init.
This also means unregistering the driver before releasing its resources.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 490aee7..006a30e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -629,10 +629,10 @@ static int __init tilcdc_drm_init(void)
static void __exit tilcdc_drm_fini(void)
{
DBG("fini");
- tilcdc_tfp410_fini();
- tilcdc_slave_fini();
- tilcdc_panel_fini();
platform_driver_unregister(&tilcdc_platform_driver);
+ tilcdc_panel_fini();
+ tilcdc_slave_fini();
+ tilcdc_tfp410_fini();
}
late_initcall(tilcdc_drm_init);
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 7/9] drm/tilcdc: fix double kfree
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, stable, Daniel Mack, Ezequiel García
display_timings_release calls kfree on the display_timings object passed
to it. Calling kfree after it is wrong. SLUB debug showed the following
warning:
=============================================================================
BUG kmalloc-64 (Tainted: G W ): Object already free
-----------------------------------------------------------------------------
Disabling lock debugging due to kernel taint
INFO: Allocated in of_get_display_timings+0x2c/0x214 age=601 cpu=0
pid=884
__slab_alloc.constprop.79+0x2e0/0x33c
kmem_cache_alloc+0xac/0xdc
of_get_display_timings+0x2c/0x214
panel_probe+0x7c/0x314 [tilcdc]
platform_drv_probe+0x18/0x48
[..snip..]
INFO: Freed in panel_destroy+0x18/0x3c [tilcdc] age=0 cpu=0 pid=907
__slab_free+0x34/0x330
panel_destroy+0x18/0x3c [tilcdc]
tilcdc_unload+0xd0/0x118 [tilcdc]
drm_dev_unregister+0x24/0x98
[..snip..]
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 1943b2f..b085dcc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -286,10 +286,8 @@ static void panel_destroy(struct tilcdc_module *mod)
{
struct panel_module *panel_mod = to_panel_module(mod);
- if (panel_mod->timings) {
+ if (panel_mod->timings)
display_timings_release(panel_mod->timings);
- kfree(panel_mod->timings);
- }
tilcdc_module_cleanup(mod);
kfree(panel_mod->info);
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 7/9] drm/tilcdc: fix double kfree
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
display_timings_release calls kfree on the display_timings object passed
to it. Calling kfree after it is wrong. SLUB debug showed the following
warning:
=============================================================================
BUG kmalloc-64 (Tainted: G W ): Object already free
-----------------------------------------------------------------------------
Disabling lock debugging due to kernel taint
INFO: Allocated in of_get_display_timings+0x2c/0x214 age=601 cpu=0
pid=884
__slab_alloc.constprop.79+0x2e0/0x33c
kmem_cache_alloc+0xac/0xdc
of_get_display_timings+0x2c/0x214
panel_probe+0x7c/0x314 [tilcdc]
platform_drv_probe+0x18/0x48
[..snip..]
INFO: Freed in panel_destroy+0x18/0x3c [tilcdc] age=0 cpu=0 pid=907
__slab_free+0x34/0x330
panel_destroy+0x18/0x3c [tilcdc]
tilcdc_unload+0xd0/0x118 [tilcdc]
drm_dev_unregister+0x24/0x98
[..snip..]
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
Cc: <stable@vger.kernel.org> #v3.9+
---
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index 1943b2f..b085dcc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -286,10 +286,8 @@ static void panel_destroy(struct tilcdc_module *mod)
{
struct panel_module *panel_mod = to_panel_module(mod);
- if (panel_mod->timings) {
+ if (panel_mod->timings)
display_timings_release(panel_mod->timings);
- kfree(panel_mod->timings);
- }
tilcdc_module_cleanup(mod);
kfree(panel_mod->info);
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread
* [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: Russell King, Daniel Vetter, Daniel Mack, Ezequiel García
The TI tilcdc driver is designed with a notion of submodules. Currently,
at unload time, these submodules are iterated and destroyed.
Now that the tilcdc remove order is fixed, this can be handled perfectly
by the kernel using the device infrastructure, since each submodule
is a kernel driver itself, and they are only destroy()'ed at unload
time. Therefore we move the destroy() functionality to each submodule's
remove().
Also, remove some checks in the unloading process since the new code
guarantees the resources are allocated and need a release.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
---
drivers/gpu/drm/tilcdc/Module.symvers | 0
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
6 files changed, 50 insertions(+), 53 deletions(-)
create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
new file mode 100644
index 0000000..e69de29
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 006a30e..2c860c4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
static int tilcdc_unload(struct drm_device *dev)
{
struct tilcdc_drm_private *priv = dev->dev_private;
- struct tilcdc_module *mod, *cur;
drm_fbdev_cma_fini(priv->fbdev);
drm_kms_helper_poll_fini(dev);
@@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
pm_runtime_disable(dev->dev);
- list_for_each_entry_safe(mod, cur, &module_list, list) {
- DBG("destroying module: %s", mod->name);
- mod->funcs->destroy(mod);
- }
-
kfree(priv);
return 0;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 0938036..7596c14 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -98,7 +98,6 @@ struct tilcdc_module;
struct tilcdc_module_ops {
/* create appropriate encoders/connectors: */
int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
- void (*destroy)(struct tilcdc_module *mod);
#ifdef CONFIG_DEBUG_FS
/* create debugfs nodes (can be NULL): */
int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index b085dcc..2f6efae 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
return 0;
}
-static void panel_destroy(struct tilcdc_module *mod)
-{
- struct panel_module *panel_mod = to_panel_module(mod);
-
- if (panel_mod->timings)
- display_timings_release(panel_mod->timings);
-
- tilcdc_module_cleanup(mod);
- kfree(panel_mod->info);
- kfree(panel_mod);
-}
-
static const struct tilcdc_module_ops panel_module_ops = {
.modeset_init = panel_modeset_init,
- .destroy = panel_destroy,
};
/*
@@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
return -ENOMEM;
mod = &panel_mod->base;
+ pdev->dev.platform_data = mod;
tilcdc_module_init(mod, "panel", &panel_module_ops);
@@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
if (IS_ERR(pinctrl))
dev_warn(&pdev->dev, "pins are not configured\n");
-
panel_mod->timings = of_get_display_timings(node);
if (!panel_mod->timings) {
dev_err(&pdev->dev, "could not get panel timings\n");
- goto fail;
+ goto fail_free;
}
panel_mod->info = of_get_panel_info(node);
if (!panel_mod->info) {
dev_err(&pdev->dev, "could not get panel info\n");
- goto fail;
+ goto fail_timings;
}
mod->preferred_bpp = panel_mod->info->bpp;
@@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
return 0;
-fail:
- panel_destroy(mod);
+fail_timings:
+ display_timings_release(panel_mod->timings);
+
+fail_free:
+ kfree(panel_mod);
+ tilcdc_module_cleanup(mod);
return ret;
}
static int panel_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct panel_module *panel_mod = to_panel_module(mod);
+
+ display_timings_release(panel_mod->timings);
+
+ tilcdc_module_cleanup(mod);
+ kfree(panel_mod->info);
+ kfree(panel_mod);
+
return 0;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index 2f83ffb..1e568ca 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
return 0;
}
-static void slave_destroy(struct tilcdc_module *mod)
-{
- struct slave_module *slave_mod = to_slave_module(mod);
-
- tilcdc_module_cleanup(mod);
- kfree(slave_mod);
-}
-
static const struct tilcdc_module_ops slave_module_ops = {
.modeset_init = slave_modeset_init,
- .destroy = slave_destroy,
};
/*
@@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
}
slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
- if (!slave_mod)
- return -ENOMEM;
+ if (!slave_mod) {
+ ret = -ENOMEM;
+ goto fail_adapter;
+ }
mod = &slave_mod->base;
+ pdev->dev.platform_data = mod;
mod->preferred_bpp = slave_info.bpp;
@@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
tilcdc_slave_probedefer(false);
return 0;
+
+fail_adapter:
+ i2c_put_adapter(slavei2c);
+ return ret;
}
static int slave_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct slave_module *slave_mod = to_slave_module(mod);
+
+ tilcdc_module_cleanup(mod);
+ kfree(slave_mod);
+
return 0;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index ce75ac8..32a0d2d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
return 0;
}
-static void tfp410_destroy(struct tilcdc_module *mod)
-{
- struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
-
- if (tfp410_mod->i2c)
- i2c_put_adapter(tfp410_mod->i2c);
-
- if (!IS_ERR_VALUE(tfp410_mod->gpio))
- gpio_free(tfp410_mod->gpio);
-
- tilcdc_module_cleanup(mod);
- kfree(tfp410_mod);
-}
-
static const struct tilcdc_module_ops tfp410_module_ops = {
.modeset_init = tfp410_modeset_init,
- .destroy = tfp410_destroy,
};
/*
@@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
return -ENOMEM;
mod = &tfp410_mod->base;
+ pdev->dev.platform_data = mod;
tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
@@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
if (!tfp410_mod->i2c) {
dev_err(&pdev->dev, "could not get i2c\n");
+ of_node_put(i2c_node);
goto fail;
}
@@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
if (ret) {
dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
- goto fail;
+ goto fail_adapter;
}
}
return 0;
+fail_adapter:
+ i2c_put_adapter(tfp410_mod->i2c);
+
fail:
- tfp410_destroy(mod);
+ kfree(tfp410_mod);
+ tilcdc_module_cleanup(mod);
return ret;
}
static int tfp410_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
+
+ i2c_put_adapter(tfp410_mod->i2c);
+ gpio_free(tfp410_mod->gpio);
+
+ tilcdc_module_cleanup(mod);
+ kfree(tfp410_mod);
+
return 0;
}
--
2.0.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
The TI tilcdc driver is designed with a notion of submodules. Currently,
at unload time, these submodules are iterated and destroyed.
Now that the tilcdc remove order is fixed, this can be handled perfectly
by the kernel using the device infrastructure, since each submodule
is a kernel driver itself, and they are only destroy()'ed at unload
time. Therefore we move the destroy() functionality to each submodule's
remove().
Also, remove some checks in the unloading process since the new code
guarantees the resources are allocated and need a release.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
drivers/gpu/drm/tilcdc/Module.symvers | 0
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
6 files changed, 50 insertions(+), 53 deletions(-)
create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
new file mode 100644
index 0000000..e69de29
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 006a30e..2c860c4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
static int tilcdc_unload(struct drm_device *dev)
{
struct tilcdc_drm_private *priv = dev->dev_private;
- struct tilcdc_module *mod, *cur;
drm_fbdev_cma_fini(priv->fbdev);
drm_kms_helper_poll_fini(dev);
@@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
pm_runtime_disable(dev->dev);
- list_for_each_entry_safe(mod, cur, &module_list, list) {
- DBG("destroying module: %s", mod->name);
- mod->funcs->destroy(mod);
- }
-
kfree(priv);
return 0;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 0938036..7596c14 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -98,7 +98,6 @@ struct tilcdc_module;
struct tilcdc_module_ops {
/* create appropriate encoders/connectors: */
int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
- void (*destroy)(struct tilcdc_module *mod);
#ifdef CONFIG_DEBUG_FS
/* create debugfs nodes (can be NULL): */
int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
index b085dcc..2f6efae 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
@@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
return 0;
}
-static void panel_destroy(struct tilcdc_module *mod)
-{
- struct panel_module *panel_mod = to_panel_module(mod);
-
- if (panel_mod->timings)
- display_timings_release(panel_mod->timings);
-
- tilcdc_module_cleanup(mod);
- kfree(panel_mod->info);
- kfree(panel_mod);
-}
-
static const struct tilcdc_module_ops panel_module_ops = {
.modeset_init = panel_modeset_init,
- .destroy = panel_destroy,
};
/*
@@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
return -ENOMEM;
mod = &panel_mod->base;
+ pdev->dev.platform_data = mod;
tilcdc_module_init(mod, "panel", &panel_module_ops);
@@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
if (IS_ERR(pinctrl))
dev_warn(&pdev->dev, "pins are not configured\n");
-
panel_mod->timings = of_get_display_timings(node);
if (!panel_mod->timings) {
dev_err(&pdev->dev, "could not get panel timings\n");
- goto fail;
+ goto fail_free;
}
panel_mod->info = of_get_panel_info(node);
if (!panel_mod->info) {
dev_err(&pdev->dev, "could not get panel info\n");
- goto fail;
+ goto fail_timings;
}
mod->preferred_bpp = panel_mod->info->bpp;
@@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
return 0;
-fail:
- panel_destroy(mod);
+fail_timings:
+ display_timings_release(panel_mod->timings);
+
+fail_free:
+ kfree(panel_mod);
+ tilcdc_module_cleanup(mod);
return ret;
}
static int panel_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct panel_module *panel_mod = to_panel_module(mod);
+
+ display_timings_release(panel_mod->timings);
+
+ tilcdc_module_cleanup(mod);
+ kfree(panel_mod->info);
+ kfree(panel_mod);
+
return 0;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index 2f83ffb..1e568ca 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
return 0;
}
-static void slave_destroy(struct tilcdc_module *mod)
-{
- struct slave_module *slave_mod = to_slave_module(mod);
-
- tilcdc_module_cleanup(mod);
- kfree(slave_mod);
-}
-
static const struct tilcdc_module_ops slave_module_ops = {
.modeset_init = slave_modeset_init,
- .destroy = slave_destroy,
};
/*
@@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
}
slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
- if (!slave_mod)
- return -ENOMEM;
+ if (!slave_mod) {
+ ret = -ENOMEM;
+ goto fail_adapter;
+ }
mod = &slave_mod->base;
+ pdev->dev.platform_data = mod;
mod->preferred_bpp = slave_info.bpp;
@@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
tilcdc_slave_probedefer(false);
return 0;
+
+fail_adapter:
+ i2c_put_adapter(slavei2c);
+ return ret;
}
static int slave_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct slave_module *slave_mod = to_slave_module(mod);
+
+ tilcdc_module_cleanup(mod);
+ kfree(slave_mod);
+
return 0;
}
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index ce75ac8..32a0d2d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
return 0;
}
-static void tfp410_destroy(struct tilcdc_module *mod)
-{
- struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
-
- if (tfp410_mod->i2c)
- i2c_put_adapter(tfp410_mod->i2c);
-
- if (!IS_ERR_VALUE(tfp410_mod->gpio))
- gpio_free(tfp410_mod->gpio);
-
- tilcdc_module_cleanup(mod);
- kfree(tfp410_mod);
-}
-
static const struct tilcdc_module_ops tfp410_module_ops = {
.modeset_init = tfp410_modeset_init,
- .destroy = tfp410_destroy,
};
/*
@@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
return -ENOMEM;
mod = &tfp410_mod->base;
+ pdev->dev.platform_data = mod;
tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
@@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
if (!tfp410_mod->i2c) {
dev_err(&pdev->dev, "could not get i2c\n");
+ of_node_put(i2c_node);
goto fail;
}
@@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
if (ret) {
dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
- goto fail;
+ goto fail_adapter;
}
}
return 0;
+fail_adapter:
+ i2c_put_adapter(tfp410_mod->i2c);
+
fail:
- tfp410_destroy(mod);
+ kfree(tfp410_mod);
+ tilcdc_module_cleanup(mod);
return ret;
}
static int tfp410_remove(struct platform_device *pdev)
{
+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
+ struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
+
+ i2c_put_adapter(tfp410_mod->i2c);
+ gpio_free(tfp410_mod->gpio);
+
+ tilcdc_module_cleanup(mod);
+ kfree(tfp410_mod);
+
return 0;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-24 22:06 ` Darren Etheridge
-1 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-24 22:06 UTC (permalink / raw)
To: Guido Martínez, dri-devel, linux-arm-kernel, linux-omap
Cc: Daniel Vetter, Russell King, Ezequiel García, Daniel Mack
Guido,
On 06/17/2014 09:17 AM, Guido Martínez wrote:
> The TI tilcdc driver is designed with a notion of submodules. Currently,
> at unload time, these submodules are iterated and destroyed.
>
> Now that the tilcdc remove order is fixed, this can be handled perfectly
I am not sure I understand the first part of the above sentence - did
something change with tilcdc ordering? I think you a referring to
previous patches in your series which really mean tilcdc can actually
unload now. So really the method this patch uses could always have been
used, it just wasn't for some reason?
I have tested all of the other patches in your series and all looks good
on BeagleBone Black and AM335xEVM, I tested as both built-ins and
modules and can load/unload on BeagleBone Black with HDMI enabled correctly.
I want to play around a bit more with this particular patch, to try and
understand how it differs from Rob's original intent with his module
registration/deregistration scheme. I prefer your method, but do we
loose anything that Rob's originally had in mind?
Darren
> by the kernel using the device infrastructure, since each submodule
> is a kernel driver itself, and they are only destroy()'ed at unload
> time. Therefore we move the destroy() functionality to each submodule's
> remove().
>
> Also, remove some checks in the unloading process since the new code
> guarantees the resources are allocated and need a release.
>
> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> ---
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
> 6 files changed, 50 insertions(+), 53 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
> new file mode 100644
> index 0000000..e69de29
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 006a30e..2c860c4 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
> static int tilcdc_unload(struct drm_device *dev)
> {
> struct tilcdc_drm_private *priv = dev->dev_private;
> - struct tilcdc_module *mod, *cur;
>
> drm_fbdev_cma_fini(priv->fbdev);
> drm_kms_helper_poll_fini(dev);
> @@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
>
> pm_runtime_disable(dev->dev);
>
> - list_for_each_entry_safe(mod, cur, &module_list, list) {
> - DBG("destroying module: %s", mod->name);
> - mod->funcs->destroy(mod);
> - }
> -
> kfree(priv);
>
> return 0;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index 0938036..7596c14 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -98,7 +98,6 @@ struct tilcdc_module;
> struct tilcdc_module_ops {
> /* create appropriate encoders/connectors: */
> int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
> - void (*destroy)(struct tilcdc_module *mod);
> #ifdef CONFIG_DEBUG_FS
> /* create debugfs nodes (can be NULL): */
> int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> index b085dcc..2f6efae 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> @@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> return 0;
> }
>
> -static void panel_destroy(struct tilcdc_module *mod)
> -{
> - struct panel_module *panel_mod = to_panel_module(mod);
> -
> - if (panel_mod->timings)
> - display_timings_release(panel_mod->timings);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(panel_mod->info);
> - kfree(panel_mod);
> -}
> -
> static const struct tilcdc_module_ops panel_module_ops = {
> .modeset_init = panel_modeset_init,
> - .destroy = panel_destroy,
> };
>
> /*
> @@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> mod = &panel_mod->base;
> + pdev->dev.platform_data = mod;
>
> tilcdc_module_init(mod, "panel", &panel_module_ops);
>
> @@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
> if (IS_ERR(pinctrl))
> dev_warn(&pdev->dev, "pins are not configured\n");
>
> -
> panel_mod->timings = of_get_display_timings(node);
> if (!panel_mod->timings) {
> dev_err(&pdev->dev, "could not get panel timings\n");
> - goto fail;
> + goto fail_free;
> }
>
> panel_mod->info = of_get_panel_info(node);
> if (!panel_mod->info) {
> dev_err(&pdev->dev, "could not get panel info\n");
> - goto fail;
> + goto fail_timings;
> }
>
> mod->preferred_bpp = panel_mod->info->bpp;
> @@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
>
> return 0;
>
> -fail:
> - panel_destroy(mod);
> +fail_timings:
> + display_timings_release(panel_mod->timings);
> +
> +fail_free:
> + kfree(panel_mod);
> + tilcdc_module_cleanup(mod);
> return ret;
> }
>
> static int panel_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct panel_module *panel_mod = to_panel_module(mod);
> +
> + display_timings_release(panel_mod->timings);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(panel_mod->info);
> + kfree(panel_mod);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> index 2f83ffb..1e568ca 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> @@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> return 0;
> }
>
> -static void slave_destroy(struct tilcdc_module *mod)
> -{
> - struct slave_module *slave_mod = to_slave_module(mod);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(slave_mod);
> -}
> -
> static const struct tilcdc_module_ops slave_module_ops = {
> .modeset_init = slave_modeset_init,
> - .destroy = slave_destroy,
> };
>
> /*
> @@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
> }
>
> slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
> - if (!slave_mod)
> - return -ENOMEM;
> + if (!slave_mod) {
> + ret = -ENOMEM;
> + goto fail_adapter;
> + }
>
> mod = &slave_mod->base;
> + pdev->dev.platform_data = mod;
>
> mod->preferred_bpp = slave_info.bpp;
>
> @@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
> tilcdc_slave_probedefer(false);
>
> return 0;
> +
> +fail_adapter:
> + i2c_put_adapter(slavei2c);
> + return ret;
> }
>
> static int slave_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct slave_module *slave_mod = to_slave_module(mod);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(slave_mod);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index ce75ac8..32a0d2d 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
> return 0;
> }
>
> -static void tfp410_destroy(struct tilcdc_module *mod)
> -{
> - struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> -
> - if (tfp410_mod->i2c)
> - i2c_put_adapter(tfp410_mod->i2c);
> -
> - if (!IS_ERR_VALUE(tfp410_mod->gpio))
> - gpio_free(tfp410_mod->gpio);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(tfp410_mod);
> -}
> -
> static const struct tilcdc_module_ops tfp410_module_ops = {
> .modeset_init = tfp410_modeset_init,
> - .destroy = tfp410_destroy,
> };
>
> /*
> @@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> mod = &tfp410_mod->base;
> + pdev->dev.platform_data = mod;
>
> tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
>
> @@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
> tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
> if (!tfp410_mod->i2c) {
> dev_err(&pdev->dev, "could not get i2c\n");
> + of_node_put(i2c_node);
> goto fail;
> }
>
> @@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
> ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
> if (ret) {
> dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
> - goto fail;
> + goto fail_adapter;
> }
> }
>
> return 0;
>
> +fail_adapter:
> + i2c_put_adapter(tfp410_mod->i2c);
> +
> fail:
> - tfp410_destroy(mod);
> + kfree(tfp410_mod);
> + tilcdc_module_cleanup(mod);
> return ret;
> }
>
> static int tfp410_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> +
> + i2c_put_adapter(tfp410_mod->i2c);
> + gpio_free(tfp410_mod->gpio);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(tfp410_mod);
> +
> return 0;
> }
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
@ 2014-06-24 22:06 ` Darren Etheridge
0 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-24 22:06 UTC (permalink / raw)
To: linux-arm-kernel
Guido,
On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> The TI tilcdc driver is designed with a notion of submodules. Currently,
> at unload time, these submodules are iterated and destroyed.
>
> Now that the tilcdc remove order is fixed, this can be handled perfectly
I am not sure I understand the first part of the above sentence - did
something change with tilcdc ordering? I think you a referring to
previous patches in your series which really mean tilcdc can actually
unload now. So really the method this patch uses could always have been
used, it just wasn't for some reason?
I have tested all of the other patches in your series and all looks good
on BeagleBone Black and AM335xEVM, I tested as both built-ins and
modules and can load/unload on BeagleBone Black with HDMI enabled correctly.
I want to play around a bit more with this particular patch, to try and
understand how it differs from Rob's original intent with his module
registration/deregistration scheme. I prefer your method, but do we
loose anything that Rob's originally had in mind?
Darren
> by the kernel using the device infrastructure, since each submodule
> is a kernel driver itself, and they are only destroy()'ed at unload
> time. Therefore we move the destroy() functionality to each submodule's
> remove().
>
> Also, remove some checks in the unloading process since the new code
> guarantees the resources are allocated and need a release.
>
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
> 6 files changed, 50 insertions(+), 53 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
> new file mode 100644
> index 0000000..e69de29
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 006a30e..2c860c4 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
> static int tilcdc_unload(struct drm_device *dev)
> {
> struct tilcdc_drm_private *priv = dev->dev_private;
> - struct tilcdc_module *mod, *cur;
>
> drm_fbdev_cma_fini(priv->fbdev);
> drm_kms_helper_poll_fini(dev);
> @@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
>
> pm_runtime_disable(dev->dev);
>
> - list_for_each_entry_safe(mod, cur, &module_list, list) {
> - DBG("destroying module: %s", mod->name);
> - mod->funcs->destroy(mod);
> - }
> -
> kfree(priv);
>
> return 0;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> index 0938036..7596c14 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> @@ -98,7 +98,6 @@ struct tilcdc_module;
> struct tilcdc_module_ops {
> /* create appropriate encoders/connectors: */
> int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
> - void (*destroy)(struct tilcdc_module *mod);
> #ifdef CONFIG_DEBUG_FS
> /* create debugfs nodes (can be NULL): */
> int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> index b085dcc..2f6efae 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> @@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> return 0;
> }
>
> -static void panel_destroy(struct tilcdc_module *mod)
> -{
> - struct panel_module *panel_mod = to_panel_module(mod);
> -
> - if (panel_mod->timings)
> - display_timings_release(panel_mod->timings);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(panel_mod->info);
> - kfree(panel_mod);
> -}
> -
> static const struct tilcdc_module_ops panel_module_ops = {
> .modeset_init = panel_modeset_init,
> - .destroy = panel_destroy,
> };
>
> /*
> @@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> mod = &panel_mod->base;
> + pdev->dev.platform_data = mod;
>
> tilcdc_module_init(mod, "panel", &panel_module_ops);
>
> @@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
> if (IS_ERR(pinctrl))
> dev_warn(&pdev->dev, "pins are not configured\n");
>
> -
> panel_mod->timings = of_get_display_timings(node);
> if (!panel_mod->timings) {
> dev_err(&pdev->dev, "could not get panel timings\n");
> - goto fail;
> + goto fail_free;
> }
>
> panel_mod->info = of_get_panel_info(node);
> if (!panel_mod->info) {
> dev_err(&pdev->dev, "could not get panel info\n");
> - goto fail;
> + goto fail_timings;
> }
>
> mod->preferred_bpp = panel_mod->info->bpp;
> @@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
>
> return 0;
>
> -fail:
> - panel_destroy(mod);
> +fail_timings:
> + display_timings_release(panel_mod->timings);
> +
> +fail_free:
> + kfree(panel_mod);
> + tilcdc_module_cleanup(mod);
> return ret;
> }
>
> static int panel_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct panel_module *panel_mod = to_panel_module(mod);
> +
> + display_timings_release(panel_mod->timings);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(panel_mod->info);
> + kfree(panel_mod);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> index 2f83ffb..1e568ca 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> @@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> return 0;
> }
>
> -static void slave_destroy(struct tilcdc_module *mod)
> -{
> - struct slave_module *slave_mod = to_slave_module(mod);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(slave_mod);
> -}
> -
> static const struct tilcdc_module_ops slave_module_ops = {
> .modeset_init = slave_modeset_init,
> - .destroy = slave_destroy,
> };
>
> /*
> @@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
> }
>
> slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
> - if (!slave_mod)
> - return -ENOMEM;
> + if (!slave_mod) {
> + ret = -ENOMEM;
> + goto fail_adapter;
> + }
>
> mod = &slave_mod->base;
> + pdev->dev.platform_data = mod;
>
> mod->preferred_bpp = slave_info.bpp;
>
> @@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
> tilcdc_slave_probedefer(false);
>
> return 0;
> +
> +fail_adapter:
> + i2c_put_adapter(slavei2c);
> + return ret;
> }
>
> static int slave_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct slave_module *slave_mod = to_slave_module(mod);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(slave_mod);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index ce75ac8..32a0d2d 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
> return 0;
> }
>
> -static void tfp410_destroy(struct tilcdc_module *mod)
> -{
> - struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> -
> - if (tfp410_mod->i2c)
> - i2c_put_adapter(tfp410_mod->i2c);
> -
> - if (!IS_ERR_VALUE(tfp410_mod->gpio))
> - gpio_free(tfp410_mod->gpio);
> -
> - tilcdc_module_cleanup(mod);
> - kfree(tfp410_mod);
> -}
> -
> static const struct tilcdc_module_ops tfp410_module_ops = {
> .modeset_init = tfp410_modeset_init,
> - .destroy = tfp410_destroy,
> };
>
> /*
> @@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> mod = &tfp410_mod->base;
> + pdev->dev.platform_data = mod;
>
> tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
>
> @@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
> tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
> if (!tfp410_mod->i2c) {
> dev_err(&pdev->dev, "could not get i2c\n");
> + of_node_put(i2c_node);
> goto fail;
> }
>
> @@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
> ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
> if (ret) {
> dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
> - goto fail;
> + goto fail_adapter;
> }
> }
>
> return 0;
>
> +fail_adapter:
> + i2c_put_adapter(tfp410_mod->i2c);
> +
> fail:
> - tfp410_destroy(mod);
> + kfree(tfp410_mod);
> + tilcdc_module_cleanup(mod);
> return ret;
> }
>
> static int tfp410_remove(struct platform_device *pdev)
> {
> + struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> + struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> +
> + i2c_put_adapter(tfp410_mod->i2c);
> + gpio_free(tfp410_mod->gpio);
> +
> + tilcdc_module_cleanup(mod);
> + kfree(tfp410_mod);
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
2014-06-24 22:06 ` Darren Etheridge
@ 2014-06-25 3:53 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-25 3:53 UTC (permalink / raw)
To: Darren Etheridge
Cc: dri-devel, linux-arm-kernel, linux-omap, Russell King,
Daniel Vetter, Daniel Mack, Ezequiel García
On Tue, Jun 24, 2014 at 05:06:24PM -0500, Darren Etheridge wrote:
> Guido,
>
> On 06/17/2014 09:17 AM, Guido Martínez wrote:
> >The TI tilcdc driver is designed with a notion of submodules. Currently,
> >at unload time, these submodules are iterated and destroyed.
> >
> >Now that the tilcdc remove order is fixed, this can be handled perfectly
>
> I am not sure I understand the first part of the above sentence - did
> something change with tilcdc ordering? I think you a referring to previous
> patches in your series which really mean tilcdc can actually unload now.
Right, I guess I got a bit dizzy while writing that commit log :). If we
find the patch reasonable I'll send a better explanation.
> So really the method this patch uses could always have been used, it
> just wasn't for some reason?
Yes, I think so.
> I have tested all of the other patches in your series and all looks good on
> BeagleBone Black and AM335xEVM, I tested as both built-ins and modules and
> can load/unload on BeagleBone Black with HDMI enabled correctly.
Nice to hear that :)
> I want to play around a bit more with this particular patch, to try and
> understand how it differs from Rob's original intent with his module
> registration/deregistration scheme. I prefer your method, but do we loose
> anything that Rob's originally had in mind?
Nothing really comes to mind, but I may be wrong on this...
Thanks a lot for your review!
> Darren
>
> >by the kernel using the device infrastructure, since each submodule
> >is a kernel driver itself, and they are only destroy()'ed at unload
> >time. Therefore we move the destroy() functionality to each submodule's
> >remove().
> >
> >Also, remove some checks in the unloading process since the new code
> >guarantees the resources are allocated and need a release.
> >
> >Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> >---
> > drivers/gpu/drm/tilcdc/Module.symvers | 0
> > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
> > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
> > drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
> > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
> > 6 files changed, 50 insertions(+), 53 deletions(-)
> > create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
> >
> >diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
> >new file mode 100644
> >index 0000000..e69de29
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >index 006a30e..2c860c4 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >@@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
> > static int tilcdc_unload(struct drm_device *dev)
> > {
> > struct tilcdc_drm_private *priv = dev->dev_private;
> >- struct tilcdc_module *mod, *cur;
> >
> > drm_fbdev_cma_fini(priv->fbdev);
> > drm_kms_helper_poll_fini(dev);
> >@@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
> >
> > pm_runtime_disable(dev->dev);
> >
> >- list_for_each_entry_safe(mod, cur, &module_list, list) {
> >- DBG("destroying module: %s", mod->name);
> >- mod->funcs->destroy(mod);
> >- }
> >-
> > kfree(priv);
> >
> > return 0;
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >index 0938036..7596c14 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >@@ -98,7 +98,6 @@ struct tilcdc_module;
> > struct tilcdc_module_ops {
> > /* create appropriate encoders/connectors: */
> > int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
> >- void (*destroy)(struct tilcdc_module *mod);
> > #ifdef CONFIG_DEBUG_FS
> > /* create debugfs nodes (can be NULL): */
> > int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >index b085dcc..2f6efae 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >@@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> > return 0;
> > }
> >
> >-static void panel_destroy(struct tilcdc_module *mod)
> >-{
> >- struct panel_module *panel_mod = to_panel_module(mod);
> >-
> >- if (panel_mod->timings)
> >- display_timings_release(panel_mod->timings);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(panel_mod->info);
> >- kfree(panel_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops panel_module_ops = {
> > .modeset_init = panel_modeset_init,
> >- .destroy = panel_destroy,
> > };
> >
> > /*
> >@@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > mod = &panel_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > tilcdc_module_init(mod, "panel", &panel_module_ops);
> >
> >@@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
> > if (IS_ERR(pinctrl))
> > dev_warn(&pdev->dev, "pins are not configured\n");
> >
> >-
> > panel_mod->timings = of_get_display_timings(node);
> > if (!panel_mod->timings) {
> > dev_err(&pdev->dev, "could not get panel timings\n");
> >- goto fail;
> >+ goto fail_free;
> > }
> >
> > panel_mod->info = of_get_panel_info(node);
> > if (!panel_mod->info) {
> > dev_err(&pdev->dev, "could not get panel info\n");
> >- goto fail;
> >+ goto fail_timings;
> > }
> >
> > mod->preferred_bpp = panel_mod->info->bpp;
> >@@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
> >
> > return 0;
> >
> >-fail:
> >- panel_destroy(mod);
> >+fail_timings:
> >+ display_timings_release(panel_mod->timings);
> >+
> >+fail_free:
> >+ kfree(panel_mod);
> >+ tilcdc_module_cleanup(mod);
> > return ret;
> > }
> >
> > static int panel_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct panel_module *panel_mod = to_panel_module(mod);
> >+
> >+ display_timings_release(panel_mod->timings);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(panel_mod->info);
> >+ kfree(panel_mod);
> >+
> > return 0;
> > }
> >
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >index 2f83ffb..1e568ca 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >@@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> > return 0;
> > }
> >
> >-static void slave_destroy(struct tilcdc_module *mod)
> >-{
> >- struct slave_module *slave_mod = to_slave_module(mod);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(slave_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops slave_module_ops = {
> > .modeset_init = slave_modeset_init,
> >- .destroy = slave_destroy,
> > };
> >
> > /*
> >@@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
> > }
> >
> > slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
> >- if (!slave_mod)
> >- return -ENOMEM;
> >+ if (!slave_mod) {
> >+ ret = -ENOMEM;
> >+ goto fail_adapter;
> >+ }
> >
> > mod = &slave_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > mod->preferred_bpp = slave_info.bpp;
> >
> >@@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
> > tilcdc_slave_probedefer(false);
> >
> > return 0;
> >+
> >+fail_adapter:
> >+ i2c_put_adapter(slavei2c);
> >+ return ret;
> > }
> >
> > static int slave_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct slave_module *slave_mod = to_slave_module(mod);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(slave_mod);
> >+
> > return 0;
> > }
> >
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >index ce75ac8..32a0d2d 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >@@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
> > return 0;
> > }
> >
> >-static void tfp410_destroy(struct tilcdc_module *mod)
> >-{
> >- struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> >-
> >- if (tfp410_mod->i2c)
> >- i2c_put_adapter(tfp410_mod->i2c);
> >-
> >- if (!IS_ERR_VALUE(tfp410_mod->gpio))
> >- gpio_free(tfp410_mod->gpio);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(tfp410_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops tfp410_module_ops = {
> > .modeset_init = tfp410_modeset_init,
> >- .destroy = tfp410_destroy,
> > };
> >
> > /*
> >@@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > mod = &tfp410_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
> >
> >@@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
> > tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
> > if (!tfp410_mod->i2c) {
> > dev_err(&pdev->dev, "could not get i2c\n");
> >+ of_node_put(i2c_node);
> > goto fail;
> > }
> >
> >@@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
> > ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
> > if (ret) {
> > dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
> >- goto fail;
> >+ goto fail_adapter;
> > }
> > }
> >
> > return 0;
> >
> >+fail_adapter:
> >+ i2c_put_adapter(tfp410_mod->i2c);
> >+
> > fail:
> >- tfp410_destroy(mod);
> >+ kfree(tfp410_mod);
> >+ tilcdc_module_cleanup(mod);
> > return ret;
> > }
> >
> > static int tfp410_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> >+
> >+ i2c_put_adapter(tfp410_mod->i2c);
> >+ gpio_free(tfp410_mod->gpio);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(tfp410_mod);
> >+
> > return 0;
> > }
> >
> >
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
@ 2014-06-25 3:53 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-25 3:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 24, 2014 at 05:06:24PM -0500, Darren Etheridge wrote:
> Guido,
>
> On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> >The TI tilcdc driver is designed with a notion of submodules. Currently,
> >at unload time, these submodules are iterated and destroyed.
> >
> >Now that the tilcdc remove order is fixed, this can be handled perfectly
>
> I am not sure I understand the first part of the above sentence - did
> something change with tilcdc ordering? I think you a referring to previous
> patches in your series which really mean tilcdc can actually unload now.
Right, I guess I got a bit dizzy while writing that commit log :). If we
find the patch reasonable I'll send a better explanation.
> So really the method this patch uses could always have been used, it
> just wasn't for some reason?
Yes, I think so.
> I have tested all of the other patches in your series and all looks good on
> BeagleBone Black and AM335xEVM, I tested as both built-ins and modules and
> can load/unload on BeagleBone Black with HDMI enabled correctly.
Nice to hear that :)
> I want to play around a bit more with this particular patch, to try and
> understand how it differs from Rob's original intent with his module
> registration/deregistration scheme. I prefer your method, but do we loose
> anything that Rob's originally had in mind?
Nothing really comes to mind, but I may be wrong on this...
Thanks a lot for your review!
> Darren
>
> >by the kernel using the device infrastructure, since each submodule
> >is a kernel driver itself, and they are only destroy()'ed at unload
> >time. Therefore we move the destroy() functionality to each submodule's
> >remove().
> >
> >Also, remove some checks in the unloading process since the new code
> >guarantees the resources are allocated and need a release.
> >
> >Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> >---
> > drivers/gpu/drm/tilcdc/Module.symvers | 0
> > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ------
> > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 36 +++++++++++++++++-----------------
> > drivers/gpu/drm/tilcdc/tilcdc_slave.c | 26 +++++++++++++-----------
> > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 34 ++++++++++++++++----------------
> > 6 files changed, 50 insertions(+), 53 deletions(-)
> > create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
> >
> >diff --git a/drivers/gpu/drm/tilcdc/Module.symvers b/drivers/gpu/drm/tilcdc/Module.symvers
> >new file mode 100644
> >index 0000000..e69de29
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >index 006a30e..2c860c4 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> >@@ -120,7 +120,6 @@ static int cpufreq_transition(struct notifier_block *nb,
> > static int tilcdc_unload(struct drm_device *dev)
> > {
> > struct tilcdc_drm_private *priv = dev->dev_private;
> >- struct tilcdc_module *mod, *cur;
> >
> > drm_fbdev_cma_fini(priv->fbdev);
> > drm_kms_helper_poll_fini(dev);
> >@@ -149,11 +148,6 @@ static int tilcdc_unload(struct drm_device *dev)
> >
> > pm_runtime_disable(dev->dev);
> >
> >- list_for_each_entry_safe(mod, cur, &module_list, list) {
> >- DBG("destroying module: %s", mod->name);
> >- mod->funcs->destroy(mod);
> >- }
> >-
> > kfree(priv);
> >
> > return 0;
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >index 0938036..7596c14 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
> >@@ -98,7 +98,6 @@ struct tilcdc_module;
> > struct tilcdc_module_ops {
> > /* create appropriate encoders/connectors: */
> > int (*modeset_init)(struct tilcdc_module *mod, struct drm_device *dev);
> >- void (*destroy)(struct tilcdc_module *mod);
> > #ifdef CONFIG_DEBUG_FS
> > /* create debugfs nodes (can be NULL): */
> > int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >index b085dcc..2f6efae 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> >@@ -282,21 +282,8 @@ static int panel_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> > return 0;
> > }
> >
> >-static void panel_destroy(struct tilcdc_module *mod)
> >-{
> >- struct panel_module *panel_mod = to_panel_module(mod);
> >-
> >- if (panel_mod->timings)
> >- display_timings_release(panel_mod->timings);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(panel_mod->info);
> >- kfree(panel_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops panel_module_ops = {
> > .modeset_init = panel_modeset_init,
> >- .destroy = panel_destroy,
> > };
> >
> > /*
> >@@ -372,6 +359,7 @@ static int panel_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > mod = &panel_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > tilcdc_module_init(mod, "panel", &panel_module_ops);
> >
> >@@ -379,17 +367,16 @@ static int panel_probe(struct platform_device *pdev)
> > if (IS_ERR(pinctrl))
> > dev_warn(&pdev->dev, "pins are not configured\n");
> >
> >-
> > panel_mod->timings = of_get_display_timings(node);
> > if (!panel_mod->timings) {
> > dev_err(&pdev->dev, "could not get panel timings\n");
> >- goto fail;
> >+ goto fail_free;
> > }
> >
> > panel_mod->info = of_get_panel_info(node);
> > if (!panel_mod->info) {
> > dev_err(&pdev->dev, "could not get panel info\n");
> >- goto fail;
> >+ goto fail_timings;
> > }
> >
> > mod->preferred_bpp = panel_mod->info->bpp;
> >@@ -400,13 +387,26 @@ static int panel_probe(struct platform_device *pdev)
> >
> > return 0;
> >
> >-fail:
> >- panel_destroy(mod);
> >+fail_timings:
> >+ display_timings_release(panel_mod->timings);
> >+
> >+fail_free:
> >+ kfree(panel_mod);
> >+ tilcdc_module_cleanup(mod);
> > return ret;
> > }
> >
> > static int panel_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct panel_module *panel_mod = to_panel_module(mod);
> >+
> >+ display_timings_release(panel_mod->timings);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(panel_mod->info);
> >+ kfree(panel_mod);
> >+
> > return 0;
> > }
> >
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >index 2f83ffb..1e568ca 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> >@@ -296,17 +296,8 @@ static int slave_modeset_init(struct tilcdc_module *mod, struct drm_device *dev)
> > return 0;
> > }
> >
> >-static void slave_destroy(struct tilcdc_module *mod)
> >-{
> >- struct slave_module *slave_mod = to_slave_module(mod);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(slave_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops slave_module_ops = {
> > .modeset_init = slave_modeset_init,
> >- .destroy = slave_destroy,
> > };
> >
> > /*
> >@@ -356,10 +347,13 @@ static int slave_probe(struct platform_device *pdev)
> > }
> >
> > slave_mod = kzalloc(sizeof(*slave_mod), GFP_KERNEL);
> >- if (!slave_mod)
> >- return -ENOMEM;
> >+ if (!slave_mod) {
> >+ ret = -ENOMEM;
> >+ goto fail_adapter;
> >+ }
> >
> > mod = &slave_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > mod->preferred_bpp = slave_info.bpp;
> >
> >@@ -374,10 +368,20 @@ static int slave_probe(struct platform_device *pdev)
> > tilcdc_slave_probedefer(false);
> >
> > return 0;
> >+
> >+fail_adapter:
> >+ i2c_put_adapter(slavei2c);
> >+ return ret;
> > }
> >
> > static int slave_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct slave_module *slave_mod = to_slave_module(mod);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(slave_mod);
> >+
> > return 0;
> > }
> >
> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >index ce75ac8..32a0d2d 100644
> >--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> >@@ -296,23 +296,8 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev
> > return 0;
> > }
> >
> >-static void tfp410_destroy(struct tilcdc_module *mod)
> >-{
> >- struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> >-
> >- if (tfp410_mod->i2c)
> >- i2c_put_adapter(tfp410_mod->i2c);
> >-
> >- if (!IS_ERR_VALUE(tfp410_mod->gpio))
> >- gpio_free(tfp410_mod->gpio);
> >-
> >- tilcdc_module_cleanup(mod);
> >- kfree(tfp410_mod);
> >-}
> >-
> > static const struct tilcdc_module_ops tfp410_module_ops = {
> > .modeset_init = tfp410_modeset_init,
> >- .destroy = tfp410_destroy,
> > };
> >
> > /*
> >@@ -342,6 +327,7 @@ static int tfp410_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > mod = &tfp410_mod->base;
> >+ pdev->dev.platform_data = mod;
> >
> > tilcdc_module_init(mod, "tfp410", &tfp410_module_ops);
> >
> >@@ -365,6 +351,7 @@ static int tfp410_probe(struct platform_device *pdev)
> > tfp410_mod->i2c = of_find_i2c_adapter_by_node(i2c_node);
> > if (!tfp410_mod->i2c) {
> > dev_err(&pdev->dev, "could not get i2c\n");
> >+ of_node_put(i2c_node);
> > goto fail;
> > }
> >
> >@@ -378,19 +365,32 @@ static int tfp410_probe(struct platform_device *pdev)
> > ret = gpio_request(tfp410_mod->gpio, "DVI_PDn");
> > if (ret) {
> > dev_err(&pdev->dev, "could not get DVI_PDn gpio\n");
> >- goto fail;
> >+ goto fail_adapter;
> > }
> > }
> >
> > return 0;
> >
> >+fail_adapter:
> >+ i2c_put_adapter(tfp410_mod->i2c);
> >+
> > fail:
> >- tfp410_destroy(mod);
> >+ kfree(tfp410_mod);
> >+ tilcdc_module_cleanup(mod);
> > return ret;
> > }
> >
> > static int tfp410_remove(struct platform_device *pdev)
> > {
> >+ struct tilcdc_module *mod = dev_get_platdata(&pdev->dev);
> >+ struct tfp410_module *tfp410_mod = to_tfp410_module(mod);
> >+
> >+ i2c_put_adapter(tfp410_mod->i2c);
> >+ gpio_free(tfp410_mod->gpio);
> >+
> >+ tilcdc_module_cleanup(mod);
> >+ kfree(tfp410_mod);
> >+
> > return 0;
> > }
> >
> >
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
2014-06-24 22:06 ` Darren Etheridge
@ 2014-06-25 14:53 ` Ezequiel García
-1 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 14:53 UTC (permalink / raw)
To: Darren Etheridge
Cc: Guido Martínez, dri-devel, linux-arm-kernel, linux-omap,
Daniel Vetter, Russell King, Daniel Mack
On 24 Jun 05:06 PM, Darren Etheridge wrote:
>
> On 06/17/2014 09:17 AM, Guido Martínez wrote:
> >The TI tilcdc driver is designed with a notion of submodules. Currently,
> >at unload time, these submodules are iterated and destroyed.
> >
> >Now that the tilcdc remove order is fixed, this can be handled perfectly
>
> I am not sure I understand the first part of the above sentence - did
> something change with tilcdc ordering?
Yes, patch [PATCH/RESEND 6/9] drm/tilcdc: fix release order on exit changes
the tilcdc remove ordering.
Currently, the tilcdc DRM is removed with this:
tilcdc_tfp410_fini();
tilcdc_slave_fini();
tilcdc_panel_fini();
platform_driver_unregister(&tilcdc_platform_driver);
Which is wrong as you shouldn't remove the tilcdc "modules" (panel, slave,
tfp410) before the DRM driver itself. So the above patch fixed it to be:
platform_driver_unregister(&tilcdc_platform_driver);
tilcdc_panel_fini();
tilcdc_slave_fini();
tilcdc_tfp410_fini();
> I think you a referring to previous
> patches in your series which really mean tilcdc can actually unload now. So
> really the method this patch uses could always have been used, it just
> wasn't for some reason?
>
No, I believe this patch which removes the tilcdc sub-module destroy
infrastructure can only be applied *after* the above remove order is fixed
(iow, the 6/9 patch mentioned above).
In other words, only once you have a proper remove() that releases things
in the right order you can rely in the kernel and avoid any custom
implementation.
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 8/9] drm/tilcdc: remove submodule destroy calls
@ 2014-06-25 14:53 ` Ezequiel García
0 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 14:53 UTC (permalink / raw)
To: linux-arm-kernel
On 24 Jun 05:06 PM, Darren Etheridge wrote:
>
> On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> >The TI tilcdc driver is designed with a notion of submodules. Currently,
> >at unload time, these submodules are iterated and destroyed.
> >
> >Now that the tilcdc remove order is fixed, this can be handled perfectly
>
> I am not sure I understand the first part of the above sentence - did
> something change with tilcdc ordering?
Yes, patch [PATCH/RESEND 6/9] drm/tilcdc: fix release order on exit changes
the tilcdc remove ordering.
Currently, the tilcdc DRM is removed with this:
tilcdc_tfp410_fini();
tilcdc_slave_fini();
tilcdc_panel_fini();
platform_driver_unregister(&tilcdc_platform_driver);
Which is wrong as you shouldn't remove the tilcdc "modules" (panel, slave,
tfp410) before the DRM driver itself. So the above patch fixed it to be:
platform_driver_unregister(&tilcdc_platform_driver);
tilcdc_panel_fini();
tilcdc_slave_fini();
tilcdc_tfp410_fini();
> I think you a referring to previous
> patches in your series which really mean tilcdc can actually unload now. So
> really the method this patch uses could always have been used, it just
> wasn't for some reason?
>
No, I believe this patch which removes the tilcdc sub-module destroy
infrastructure can only be applied *after* the above remove order is fixed
(iow, the 6/9 patch mentioned above).
In other words, only once you have a proper remove() that releases things
in the right order you can rely in the kernel and avoid any custom
implementation.
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-17 14:17 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap
Cc: robdclark, Ezequiel García, airlied, Daniel Mack,
Russell King, Daniel Vetter, Guido Martínez
Use module_init instead of late_initcall, as is the norm for modular
drivers.
module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
but it does not explain why. Tests show it's working properly with
module_init.
Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 2c860c4..6be623b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -629,7 +629,7 @@ static void __exit tilcdc_drm_fini(void)
tilcdc_tfp410_fini();
}
-late_initcall(tilcdc_drm_init);
+module_init(tilcdc_drm_init);
module_exit(tilcdc_drm_fini);
MODULE_AUTHOR("Rob Clark <robdclark@gmail.com");
--
2.0.0
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 76+ messages in thread* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-17 14:17 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-17 14:17 UTC (permalink / raw)
To: linux-arm-kernel
Use module_init instead of late_initcall, as is the norm for modular
drivers.
module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
but it does not explain why. Tests show it's working properly with
module_init.
Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
---
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 2c860c4..6be623b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -629,7 +629,7 @@ static void __exit tilcdc_drm_fini(void)
tilcdc_tfp410_fini();
}
-late_initcall(tilcdc_drm_init);
+module_init(tilcdc_drm_init);
module_exit(tilcdc_drm_fini);
MODULE_AUTHOR("Rob Clark <robdclark@gmail.com");
--
2.0.0
^ permalink raw reply related [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-24 22:04 ` Darren Etheridge
-1 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-24 22:04 UTC (permalink / raw)
To: Guido Martínez, dri-devel, linux-arm-kernel, linux-omap
Cc: Daniel Vetter, Russell King, Ezequiel García, Daniel Mack
On 06/17/2014 09:17 AM, Guido Martínez wrote:
> Use module_init instead of late_initcall, as is the norm for modular
> drivers.
>
> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> but it does not explain why. Tests show it's working properly with
> module_init.
>
If I recall, the late_initcall stuff was done to try and make sure the
tda998x/i2c subsystem came up before tilcdc. However it didn't always
work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
and ensure the ordering. This might be why changing back to module_init
is fine (and I agree with your assessment from my testing).
> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> ---
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 2c860c4..6be623b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -629,7 +629,7 @@ static void __exit tilcdc_drm_fini(void)
> tilcdc_tfp410_fini();
> }
>
> -late_initcall(tilcdc_drm_init);
> +module_init(tilcdc_drm_init);
> module_exit(tilcdc_drm_fini);
>
> MODULE_AUTHOR("Rob Clark <robdclark@gmail.com");
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-24 22:04 ` Darren Etheridge
0 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-24 22:04 UTC (permalink / raw)
To: linux-arm-kernel
On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> Use module_init instead of late_initcall, as is the norm for modular
> drivers.
>
> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> but it does not explain why. Tests show it's working properly with
> module_init.
>
If I recall, the late_initcall stuff was done to try and make sure the
tda998x/i2c subsystem came up before tilcdc. However it didn't always
work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
and ensure the ordering. This might be why changing back to module_init
is fine (and I agree with your assessment from my testing).
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
> ---
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> index 2c860c4..6be623b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
> @@ -629,7 +629,7 @@ static void __exit tilcdc_drm_fini(void)
> tilcdc_tfp410_fini();
> }
>
> -late_initcall(tilcdc_drm_init);
> +module_init(tilcdc_drm_init);
> module_exit(tilcdc_drm_fini);
>
> MODULE_AUTHOR("Rob Clark <robdclark@gmail.com");
>
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-24 22:04 ` Darren Etheridge
@ 2014-06-25 13:00 ` Russell King - ARM Linux
-1 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 13:00 UTC (permalink / raw)
To: Darren Etheridge
Cc: Daniel Vetter, dri-devel, Daniel Mack, linux-omap,
linux-arm-kernel, Ezequiel García
On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> On 06/17/2014 09:17 AM, Guido Martínez wrote:
>> Use module_init instead of late_initcall, as is the norm for modular
>> drivers.
>>
>> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
>> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
>> but it does not explain why. Tests show it's working properly with
>> module_init.
>>
>
> If I recall, the late_initcall stuff was done to try and make sure the
> tda998x/i2c subsystem came up before tilcdc. However it didn't always
> work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> and ensure the ordering. This might be why changing back to module_init
> is fine (and I agree with your assessment from my testing).
There's a solution to that... I have patches which convert the tda998x
driver to also register into the component helpers as well as remaining
as a drm slave device. This makes it possible to transition tilcdc to
use the component helper to solve the initialisation ordering.
I'll (re-)post them later today.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-25 13:00 ` Russell King - ARM Linux
0 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 13:00 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
>> Use module_init instead of late_initcall, as is the norm for modular
>> drivers.
>>
>> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
>> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
>> but it does not explain why. Tests show it's working properly with
>> module_init.
>>
>
> If I recall, the late_initcall stuff was done to try and make sure the
> tda998x/i2c subsystem came up before tilcdc. However it didn't always
> work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> and ensure the ordering. This might be why changing back to module_init
> is fine (and I agree with your assessment from my testing).
There's a solution to that... I have patches which convert the tda998x
driver to also register into the component helpers as well as remaining
as a drm slave device. This makes it possible to transition tilcdc to
use the component helper to solve the initialisation ordering.
I'll (re-)post them later today.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-25 13:00 ` Russell King - ARM Linux
@ 2014-06-25 13:13 ` Russell King - ARM Linux
-1 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 13:13 UTC (permalink / raw)
To: Darren Etheridge
Cc: Daniel Vetter, dri-devel, linux-omap, linux-arm-kernel,
Ezequiel García
On Wed, Jun 25, 2014 at 02:00:42PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > On 06/17/2014 09:17 AM, Guido Martínez wrote:
> >> Use module_init instead of late_initcall, as is the norm for modular
> >> drivers.
> >>
> >> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> >> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> >> but it does not explain why. Tests show it's working properly with
> >> module_init.
> >>
> >
> > If I recall, the late_initcall stuff was done to try and make sure the
> > tda998x/i2c subsystem came up before tilcdc. However it didn't always
> > work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> > and ensure the ordering. This might be why changing back to module_init
> > is fine (and I agree with your assessment from my testing).
>
> There's a solution to that... I have patches which convert the tda998x
> driver to also register into the component helpers as well as remaining
> as a drm slave device. This makes it possible to transition tilcdc to
> use the component helper to solve the initialisation ordering.
>
> I'll (re-)post them later today.
Except Daniel Mack will not be receiving any copies of them:
zonque@gmail.com
SMTP error from remote mail server after end of data:
host gmail-smtp-in.l.google.com [2a00:1450:400c:c03::1a]:
550-5.7.1 [2001:4d48:ad52:3201:214:fdff:fe10:1be6 12] Our system has
550-5.7.1 detected that this message is likely unsolicited mail. To reduce the
550-5.7.1 amount of spam sent to Gmail, this message has been blocked. Please
550-5.7.1 visit
550-5.7.1 http://support.google.com/mail/bin/answer.py?hl=en&answer=188131 for
550 5.7.1 more information. fs8si6627678wib.99 - gsmtp
Google's anti-ham filter seems to be working perfectly, allowing spam
through and blocking real messages... as usual. And as usual, google
provides no support for this crap. Gmail has a history of increasingly
blocking legitimate email in their alleged anti-spam fight. Don't use
gmail.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-25 13:13 ` Russell King - ARM Linux
0 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 13:13 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 25, 2014 at 02:00:42PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> >> Use module_init instead of late_initcall, as is the norm for modular
> >> drivers.
> >>
> >> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> >> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> >> but it does not explain why. Tests show it's working properly with
> >> module_init.
> >>
> >
> > If I recall, the late_initcall stuff was done to try and make sure the
> > tda998x/i2c subsystem came up before tilcdc. However it didn't always
> > work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> > and ensure the ordering. This might be why changing back to module_init
> > is fine (and I agree with your assessment from my testing).
>
> There's a solution to that... I have patches which convert the tda998x
> driver to also register into the component helpers as well as remaining
> as a drm slave device. This makes it possible to transition tilcdc to
> use the component helper to solve the initialisation ordering.
>
> I'll (re-)post them later today.
Except Daniel Mack will not be receiving any copies of them:
zonque at gmail.com
SMTP error from remote mail server after end of data:
host gmail-smtp-in.l.google.com [2a00:1450:400c:c03::1a]:
550-5.7.1 [2001:4d48:ad52:3201:214:fdff:fe10:1be6 12] Our system has
550-5.7.1 detected that this message is likely unsolicited mail. To reduce the
550-5.7.1 amount of spam sent to Gmail, this message has been blocked. Please
550-5.7.1 visit
550-5.7.1 http://support.google.com/mail/bin/answer.py?hl=en&answer=188131 for
550 5.7.1 more information. fs8si6627678wib.99 - gsmtp
Google's anti-ham filter seems to be working perfectly, allowing spam
through and blocking real messages... as usual. And as usual, google
provides no support for this crap. Gmail has a history of increasingly
blocking legitimate email in their alleged anti-spam fight. Don't use
gmail.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-25 13:00 ` Russell King - ARM Linux
@ 2014-06-25 14:32 ` Ezequiel García
-1 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 14:32 UTC (permalink / raw)
To: Russell King - ARM Linux, Darren Etheridge
Cc: Guido Martínez, Daniel Vetter, dri-devel, Daniel Mack,
linux-omap, linux-arm-kernel
(Ccing Guido back)
Hello Russell, Darren,
On 25 Jun 02:00 PM, Russell King - ARM Linux wrote:
> On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > On 06/17/2014 09:17 AM, Guido Martínez wrote:
> >> Use module_init instead of late_initcall, as is the norm for modular
> >> drivers.
> >>
> >> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> >> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> >> but it does not explain why. Tests show it's working properly with
> >> module_init.
> >>
> >
> > If I recall, the late_initcall stuff was done to try and make sure the
> > tda998x/i2c subsystem came up before tilcdc.
That doesn't make any sense. Using late_initcall for the tilcdc DRM
driver would make the tilcdc DRM get probed before any other regular
module_init driver, including the tda998x encoder.
> > However it didn't always
> > work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> > and ensure the ordering. This might be why changing back to module_init
> > is fine (and I agree with your assessment from my testing).
That commit is adds a proper probe deferal mechanism in case the
i2c is not available. OMAP is special because it has its own nasty initcall
to probe the i2c busses. However, that should be fixed and then i2c would be
always probed *after* the DRM, as per the ordering in drivers/Makefile.
>
> There's a solution to that...
A solution to *what* ?
> I have patches which convert the tda998x
> driver to also register into the component helpers as well as remaining
> as a drm slave device. This makes it possible to transition tilcdc to
> use the component helper to solve the initialisation ordering.
>
AFAIK, there's no issue with the initialisation ordering, except that the
DRM driver is currently probed before the TDA998x encoder and so the probe
is defered. Unless I'm missing something that's very easy to fix, you just
need to change the link order to guarantee that i2c encoders are probed
*before* the DRM drivers that require them.
I have just posted a very simple patch to fix it, but it seems Thierry wasn't
happy with it, not sure why yet [1].
[1] http://www.spinics.net/lists/dri-devel/msg61987.html
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-25 14:32 ` Ezequiel García
0 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 14:32 UTC (permalink / raw)
To: linux-arm-kernel
(Ccing Guido back)
Hello Russell, Darren,
On 25 Jun 02:00 PM, Russell King - ARM Linux wrote:
> On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> >> Use module_init instead of late_initcall, as is the norm for modular
> >> drivers.
> >>
> >> module_init was used until 6e8de0bd6a51fdeebd5d975c4fcc426f730b339b
> >> ("drm/tilcdc: add encoder slave (v2)") changed it to a late_initcall,
> >> but it does not explain why. Tests show it's working properly with
> >> module_init.
> >>
> >
> > If I recall, the late_initcall stuff was done to try and make sure the
> > tda998x/i2c subsystem came up before tilcdc.
That doesn't make any sense. Using late_initcall for the tilcdc DRM
driver would make the tilcdc DRM get probed before any other regular
module_init driver, including the tda998x encoder.
> > However it didn't always
> > work so we added commit: 39de6194131c155901f96686a063212656d80c2e to try
> > and ensure the ordering. This might be why changing back to module_init
> > is fine (and I agree with your assessment from my testing).
That commit is adds a proper probe deferal mechanism in case the
i2c is not available. OMAP is special because it has its own nasty initcall
to probe the i2c busses. However, that should be fixed and then i2c would be
always probed *after* the DRM, as per the ordering in drivers/Makefile.
>
> There's a solution to that...
A solution to *what* ?
> I have patches which convert the tda998x
> driver to also register into the component helpers as well as remaining
> as a drm slave device. This makes it possible to transition tilcdc to
> use the component helper to solve the initialisation ordering.
>
AFAIK, there's no issue with the initialisation ordering, except that the
DRM driver is currently probed before the TDA998x encoder and so the probe
is defered. Unless I'm missing something that's very easy to fix, you just
need to change the link order to guarantee that i2c encoders are probed
*before* the DRM drivers that require them.
I have just posted a very simple patch to fix it, but it seems Thierry wasn't
happy with it, not sure why yet [1].
[1] http://www.spinics.net/lists/dri-devel/msg61987.html
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-25 14:32 ` Ezequiel García
@ 2014-06-25 14:46 ` Russell King - ARM Linux
-1 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 14:46 UTC (permalink / raw)
To: Ezequiel García
Cc: Daniel Vetter, dri-devel, Daniel Mack, linux-arm-kernel, linux-omap
On Wed, Jun 25, 2014 at 11:32:46AM -0300, Ezequiel García wrote:
> (Ccing Guido back)
>
> Hello Russell, Darren,
>
> On 25 Jun 02:00 PM, Russell King - ARM Linux wrote:
> > On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > > If I recall, the late_initcall stuff was done to try and make sure the
> > > tda998x/i2c subsystem came up before tilcdc.
>
> That doesn't make any sense. Using late_initcall for the tilcdc DRM
> driver would make the tilcdc DRM get probed before any other regular
> module_init driver, including the tda998x encoder.
A module_init() is a device_initcall(), which is at level 6. A
late_initcall() is at level 7. Level 6 initcalls are run before level
7 initcalls. The tda998x is a module_init(), so the tda998x gets
initialised *before* tilcdc's late_initcall().
Now, if you build everything as a module, then you have no initialisation
ordering, and you can't rely on any kind of order. Initialisation
functions can even run in parallel on different CPUs due to modules being
loaded from userspace in a multi-threaded way. So anything which relies
on a certain initcall ordering is fundamentally broken if it can be
modular.
> > There's a solution to that...
>
> A solution to *what* ?
Maybe if you left the context above that line in place, you might
understand that my "that" refers to what was discussed in that
context. That being the initialisation ordering.
Convention and proper Internet etiquette is to trim the quoted text
and place relies below the paragraph to which they refer, the quoted
paragraph giving the context to the reply.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-25 14:46 ` Russell King - ARM Linux
0 siblings, 0 replies; 76+ messages in thread
From: Russell King - ARM Linux @ 2014-06-25 14:46 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 25, 2014 at 11:32:46AM -0300, Ezequiel Garc?a wrote:
> (Ccing Guido back)
>
> Hello Russell, Darren,
>
> On 25 Jun 02:00 PM, Russell King - ARM Linux wrote:
> > On Tue, Jun 24, 2014 at 05:04:36PM -0500, Darren Etheridge wrote:
> > > If I recall, the late_initcall stuff was done to try and make sure the
> > > tda998x/i2c subsystem came up before tilcdc.
>
> That doesn't make any sense. Using late_initcall for the tilcdc DRM
> driver would make the tilcdc DRM get probed before any other regular
> module_init driver, including the tda998x encoder.
A module_init() is a device_initcall(), which is at level 6. A
late_initcall() is at level 7. Level 6 initcalls are run before level
7 initcalls. The tda998x is a module_init(), so the tda998x gets
initialised *before* tilcdc's late_initcall().
Now, if you build everything as a module, then you have no initialisation
ordering, and you can't rely on any kind of order. Initialisation
functions can even run in parallel on different CPUs due to modules being
loaded from userspace in a multi-threaded way. So anything which relies
on a certain initcall ordering is fundamentally broken if it can be
modular.
> > There's a solution to that...
>
> A solution to *what* ?
Maybe if you left the context above that line in place, you might
understand that my "that" refers to what was discussed in that
context. That being the initialisation ordering.
Convention and proper Internet etiquette is to trim the quoted text
and place relies below the paragraph to which they refer, the quoted
paragraph giving the context to the reply.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
2014-06-25 14:46 ` Russell King - ARM Linux
@ 2014-06-25 15:48 ` Ezequiel García
-1 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 15:48 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Daniel Vetter, dri-devel, Daniel Mack, Guido Martinez,
linux-arm-kernel, linux-omap
Hi Russell,
On 25 Jun 03:46 PM, Russell King - ARM Linux wrote:
> >
> > That doesn't make any sense. Using late_initcall for the tilcdc DRM
> > driver would make the tilcdc DRM get probed before any other regular
> > module_init driver, including the tda998x encoder.
>
> A module_init() is a device_initcall(), which is at level 6. A
> late_initcall() is at level 7. Level 6 initcalls are run before level
> 7 initcalls. The tda998x is a module_init(), so the tda998x gets
> initialised *before* tilcdc's late_initcall().
>
Oh, thanks a lot for correcting me.
> Now, if you build everything as a module, then you have no initialisation
> ordering, and you can't rely on any kind of order. Initialisation
> functions can even run in parallel on different CPUs due to modules being
> loaded from userspace in a multi-threaded way. So anything which relies
> on a certain initcall ordering is fundamentally broken if it can be
> modular.
>
OK, but is there any outstanding issue with the tilcdc initialization, either
when compiled as built-in or as modules, *after* this patchset?
Of course, this driver only worked as built-in (see the horrible bugs we are
fixing here, that prevented unloading and re-loading it). This series fixes
all such bugs, and also adds the required changes to allow to build everything
as a module.
Again, I can't see any issues that remain to be fixed after this patchset.
When compiled as built-in, the tilcdc DRM will be defered and once the tda998x
is ready, the tilcdc DRM will load properly.
When compiled as module you can load any of them in any order. If the tda998x
module is not loaded by the time you load the tilcdc module, the former will
get loaded.
Hm... now that I think about this. We still get into trouble if the tda998x
is built as module, but the tilcdc is built-in. Shouldn't we just forbid that?
Either both built-ins or both as modules?
> > > There's a solution to that...
> >
> > A solution to *what* ?
>
> Maybe if you left the context above that line in place, you might
> understand that my "that" refers to what was discussed in that
> context. That being the initialisation ordering.
>
> Convention and proper Internet etiquette is to trim the quoted text
> and place relies below the paragraph to which they refer, the quoted
> paragraph giving the context to the reply.
>
Yeah, sorry about that.
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 9/9] drm/tilcdc: replace late_initcall with module_init
@ 2014-06-25 15:48 ` Ezequiel García
0 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-06-25 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell,
On 25 Jun 03:46 PM, Russell King - ARM Linux wrote:
> >
> > That doesn't make any sense. Using late_initcall for the tilcdc DRM
> > driver would make the tilcdc DRM get probed before any other regular
> > module_init driver, including the tda998x encoder.
>
> A module_init() is a device_initcall(), which is at level 6. A
> late_initcall() is at level 7. Level 6 initcalls are run before level
> 7 initcalls. The tda998x is a module_init(), so the tda998x gets
> initialised *before* tilcdc's late_initcall().
>
Oh, thanks a lot for correcting me.
> Now, if you build everything as a module, then you have no initialisation
> ordering, and you can't rely on any kind of order. Initialisation
> functions can even run in parallel on different CPUs due to modules being
> loaded from userspace in a multi-threaded way. So anything which relies
> on a certain initcall ordering is fundamentally broken if it can be
> modular.
>
OK, but is there any outstanding issue with the tilcdc initialization, either
when compiled as built-in or as modules, *after* this patchset?
Of course, this driver only worked as built-in (see the horrible bugs we are
fixing here, that prevented unloading and re-loading it). This series fixes
all such bugs, and also adds the required changes to allow to build everything
as a module.
Again, I can't see any issues that remain to be fixed after this patchset.
When compiled as built-in, the tilcdc DRM will be defered and once the tda998x
is ready, the tilcdc DRM will load properly.
When compiled as module you can load any of them in any order. If the tda998x
module is not loaded by the time you load the tilcdc module, the former will
get loaded.
Hm... now that I think about this. We still get into trouble if the tda998x
is built as module, but the tilcdc is built-in. Shouldn't we just forbid that?
Either both built-ins or both as modules?
> > > There's a solution to that...
> >
> > A solution to *what* ?
>
> Maybe if you left the context above that line in place, you might
> understand that my "that" refers to what was discussed in that
> context. That being the initialisation ordering.
>
> Convention and proper Internet etiquette is to trim the quoted text
> and place relies below the paragraph to which they refer, the quoted
> paragraph giving the context to the reply.
>
Yeah, sorry about that.
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-19 13:41 ` Darren Etheridge
-1 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-19 13:41 UTC (permalink / raw)
To: Guido Martínez, dri-devel, linux-arm-kernel, linux-omap
Cc: Daniel Vetter, Russell King, Ezequiel García, Daniel Mack
Guido,
Thanks and sorry I missed this the first time around. I'll give it a
try on a few of my AM335x based boards when I have access to them again
on Monday.
Darren
On 06/17/2014 09:17 AM, Guido Martínez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Martínez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-19 13:41 ` Darren Etheridge
0 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-19 13:41 UTC (permalink / raw)
To: linux-arm-kernel
Guido,
Thanks and sorry I missed this the first time around. I'll give it a
try on a few of my AM335x based boards when I have access to them again
on Monday.
Darren
On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Mart?nez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-19 13:41 ` Darren Etheridge
@ 2014-06-19 16:25 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-19 16:25 UTC (permalink / raw)
To: Darren Etheridge
Cc: dri-devel, linux-arm-kernel, linux-omap, Russell King,
Daniel Vetter, Daniel Mack, Ezequiel García
Hi Darren,
On Thu, Jun 19, 2014 at 08:41:43AM -0500, Darren Etheridge wrote:
> Guido,
>
> Thanks and sorry I missed this the first time around. I'll give it a try on
> a few of my AM335x based boards when I have access to them again on Monday.
Thanks! And no worries :)
>
> Darren
>
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-19 16:25 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-19 16:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Darren,
On Thu, Jun 19, 2014 at 08:41:43AM -0500, Darren Etheridge wrote:
> Guido,
>
> Thanks and sorry I missed this the first time around. I'll give it a try on
> a few of my AM335x based boards when I have access to them again on Monday.
Thanks! And no worries :)
>
> Darren
>
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-24 0:26 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-24 0:26 UTC (permalink / raw)
To: dri-devel, linux-arm-kernel, linux-omap, robdclark
Cc: Daniel Vetter, Russell King, Ezequiel García, Daniel Mack
Hi Rob,
On Tue, Jun 17, 2014 at 11:17:02AM -0300, Guido Martínez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
Any comment on this patchset?
Thanks!
>
> Resending this since I got no replies.
>
>
> Guido Martínez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> --
> 2.0.0
>
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-24 0:26 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-06-24 0:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On Tue, Jun 17, 2014 at 11:17:02AM -0300, Guido Mart?nez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
Any comment on this patchset?
Thanks!
>
> Resending this since I got no replies.
>
>
> Guido Mart?nez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> --
> 2.0.0
>
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-17 14:17 ` Guido Martínez
@ 2014-06-27 22:08 ` Darren Etheridge
-1 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-27 22:08 UTC (permalink / raw)
To: Guido Martínez, dri-devel, linux-arm-kernel, linux-omap
Cc: Daniel Vetter, Russell King, Ezequiel García, Daniel Mack
Guido,
On 06/17/2014 09:17 AM, Guido Martínez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some
sense because DPMS_OFF would have the effect of turning off clocks and
putting the monitor to sleep which seems logical considering we have
torn down the display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before
the WARN_ON confirms this, but it seems strange that this hasn't
happened automatically (+ Russell doesn't need to do it in his Armada
driver) - so I suspect there is a better way.
Otherwise I think this is a good and useful patch series.
Darren
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Martínez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-27 22:08 ` Darren Etheridge
0 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-06-27 22:08 UTC (permalink / raw)
To: linux-arm-kernel
Guido,
On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some
sense because DPMS_OFF would have the effect of turning off clocks and
putting the monitor to sleep which seems logical considering we have
torn down the display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before
the WARN_ON confirms this, but it seems strange that this hasn't
happened automatically (+ Russell doesn't need to do it in his Armada
driver) - so I suspect there is a better way.
Otherwise I think this is a good and useful patch series.
Darren
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Mart?nez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-27 22:08 ` Darren Etheridge
@ 2014-06-28 10:51 ` Rob Clark
-1 siblings, 0 replies; 76+ messages in thread
From: Rob Clark @ 2014-06-28 10:51 UTC (permalink / raw)
To: Darren Etheridge
Cc: Russell King, Daniel Vetter, dri-devel, Daniel Mack, linux-omap,
linux-arm-kernel, Ezequiel García
On Fri, Jun 27, 2014 at 6:08 PM, Darren Etheridge <detheridge@ti.com> wrote:
> Guido,
>
>
> On 06/17/2014 09:17 AM, Guido Martínez wrote:
>>
>> The tilcdc driver could be compiled as a module, but was severely broken
>> and could not be used as such. This patchset attempts to fix the issues
>> preventing a proper load/unload of the module.
>>
>> Issues included dangling sysfs nodes, dangling devices, memory leaks and
>> a double kfree.
>>
>> It now seems to be working ok. We have tested this by loading and
>> unloading the driver repeteadly, with both panel and slave connectors
>> and found no flaws.
>>
>> There is still one warning left on tilcdc_crtc_destroy, caused by
>> destroying the connector while still in an ON status. We don't know why
>> this happens or why it's an issue, so we did not fix it.
>>
>
> Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
> because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some sense
> because DPMS_OFF would have the effect of turning off clocks and putting the
> monitor to sleep which seems logical considering we have torn down the
> display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before the WARN_ON
> confirms this, but it seems strange that this hasn't happened automatically
> (+ Russell doesn't need to do it in his Armada driver) - so I suspect there
> is a better way.
tbh, I'm not entirely sure offhand why drm_mode_config_cleanup()
doesn't remove the fb's first (which should have the effect of
shutting down any lit crtc/encoder/connector).. that would seem like
the sensible way to shut down..
BR,
-R
> Otherwise I think this is a good and useful patch series.
>
> Darren
>
>
>> The first 7 patches are bug fixes which and should probably be applied
>> in the stable trees. The last two are clean-ups.
>>
>>
>> Resending this since I got no replies.
>>
>>
>> Guido Martínez (9):
>> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
>> drm/tilcdc: panel: fix dangling sysfs connector node
>> drm/tilcdc: slave: fix dangling sysfs connector node
>> drm/tilcdc: tfp410: fix dangling sysfs connector node
>> drm/tilcdc: panel: fix leak when unloading the module
>> drm/tilcdc: fix release order on exit
>> drm/tilcdc: fix double kfree
>> drm/tilcdc: remove submodule destroy calls
>> drm/tilcdc: replace late_initcall with module_init
>>
>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>> drivers/gpu/drm/tilcdc/Module.symvers | 0
>> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
>> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39
>> +++++++++++++++++-----------------
>> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
>> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35
>> +++++++++++++++---------------
>> 7 files changed, 59 insertions(+), 60 deletions(-)
>> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-06-28 10:51 ` Rob Clark
0 siblings, 0 replies; 76+ messages in thread
From: Rob Clark @ 2014-06-28 10:51 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 27, 2014 at 6:08 PM, Darren Etheridge <detheridge@ti.com> wrote:
> Guido,
>
>
> On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
>>
>> The tilcdc driver could be compiled as a module, but was severely broken
>> and could not be used as such. This patchset attempts to fix the issues
>> preventing a proper load/unload of the module.
>>
>> Issues included dangling sysfs nodes, dangling devices, memory leaks and
>> a double kfree.
>>
>> It now seems to be working ok. We have tested this by loading and
>> unloading the driver repeteadly, with both panel and slave connectors
>> and found no flaws.
>>
>> There is still one warning left on tilcdc_crtc_destroy, caused by
>> destroying the connector while still in an ON status. We don't know why
>> this happens or why it's an issue, so we did not fix it.
>>
>
> Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
> because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some sense
> because DPMS_OFF would have the effect of turning off clocks and putting the
> monitor to sleep which seems logical considering we have torn down the
> display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before the WARN_ON
> confirms this, but it seems strange that this hasn't happened automatically
> (+ Russell doesn't need to do it in his Armada driver) - so I suspect there
> is a better way.
tbh, I'm not entirely sure offhand why drm_mode_config_cleanup()
doesn't remove the fb's first (which should have the effect of
shutting down any lit crtc/encoder/connector).. that would seem like
the sensible way to shut down..
BR,
-R
> Otherwise I think this is a good and useful patch series.
>
> Darren
>
>
>> The first 7 patches are bug fixes which and should probably be applied
>> in the stable trees. The last two are clean-ups.
>>
>>
>> Resending this since I got no replies.
>>
>>
>> Guido Mart?nez (9):
>> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
>> drm/tilcdc: panel: fix dangling sysfs connector node
>> drm/tilcdc: slave: fix dangling sysfs connector node
>> drm/tilcdc: tfp410: fix dangling sysfs connector node
>> drm/tilcdc: panel: fix leak when unloading the module
>> drm/tilcdc: fix release order on exit
>> drm/tilcdc: fix double kfree
>> drm/tilcdc: remove submodule destroy calls
>> drm/tilcdc: replace late_initcall with module_init
>>
>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>> drivers/gpu/drm/tilcdc/Module.symvers | 0
>> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
>> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
>> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39
>> +++++++++++++++++-----------------
>> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
>> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35
>> +++++++++++++++---------------
>> 7 files changed, 59 insertions(+), 60 deletions(-)
>> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-28 10:51 ` Rob Clark
@ 2014-07-08 10:03 ` Daniel Vetter
-1 siblings, 0 replies; 76+ messages in thread
From: Daniel Vetter @ 2014-07-08 10:03 UTC (permalink / raw)
To: Rob Clark
Cc: Darren Etheridge, Guido Martínez, dri-devel,
linux-arm-kernel, linux-omap, Daniel Vetter, Russell King,
Ezequiel García, Daniel Mack
On Sat, Jun 28, 2014 at 06:51:15AM -0400, Rob Clark wrote:
> On Fri, Jun 27, 2014 at 6:08 PM, Darren Etheridge <detheridge@ti.com> wrote:
> > Guido,
> >
> >
> > On 06/17/2014 09:17 AM, Guido Martínez wrote:
> >>
> >> The tilcdc driver could be compiled as a module, but was severely broken
> >> and could not be used as such. This patchset attempts to fix the issues
> >> preventing a proper load/unload of the module.
> >>
> >> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> >> a double kfree.
> >>
> >> It now seems to be working ok. We have tested this by loading and
> >> unloading the driver repeteadly, with both panel and slave connectors
> >> and found no flaws.
> >>
> >> There is still one warning left on tilcdc_crtc_destroy, caused by
> >> destroying the connector while still in an ON status. We don't know why
> >> this happens or why it's an issue, so we did not fix it.
> >>
> >
> > Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
> > because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some sense
> > because DPMS_OFF would have the effect of turning off clocks and putting the
> > monitor to sleep which seems logical considering we have torn down the
> > display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before the WARN_ON
> > confirms this, but it seems strange that this hasn't happened automatically
> > (+ Russell doesn't need to do it in his Armada driver) - so I suspect there
> > is a better way.
>
> tbh, I'm not entirely sure offhand why drm_mode_config_cleanup()
> doesn't remove the fb's first (which should have the effect of
> shutting down any lit crtc/encoder/connector).. that would seem like
> the sensible way to shut down..
All userspace fbs should be cleared already before going into the driver
unload. Which only leaves you with driver internal fbs (usually just one
for fbdev emulation). It's the driver's job to clean that up explicitly.
Then you can call mode_config_cleanup and the WARN_ON in there is a really
nice space leak check.
If we'd unconditionally clean up all fbs we'd have trouble with
driver-private embedded fbs and their refcounting and would loose the
space leak check.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-08 10:03 ` Daniel Vetter
0 siblings, 0 replies; 76+ messages in thread
From: Daniel Vetter @ 2014-07-08 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Jun 28, 2014 at 06:51:15AM -0400, Rob Clark wrote:
> On Fri, Jun 27, 2014 at 6:08 PM, Darren Etheridge <detheridge@ti.com> wrote:
> > Guido,
> >
> >
> > On 06/17/2014 09:17 AM, Guido Mart?nez wrote:
> >>
> >> The tilcdc driver could be compiled as a module, but was severely broken
> >> and could not be used as such. This patchset attempts to fix the issues
> >> preventing a proper load/unload of the module.
> >>
> >> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> >> a double kfree.
> >>
> >> It now seems to be working ok. We have tested this by loading and
> >> unloading the driver repeteadly, with both panel and slave connectors
> >> and found no flaws.
> >>
> >> There is still one warning left on tilcdc_crtc_destroy, caused by
> >> destroying the connector while still in an ON status. We don't know why
> >> this happens or why it's an issue, so we did not fix it.
> >>
> >
> > Yes I see what you mean, it triggers the WARN_ON in tilcdc_crtc_destroy
> > because DRM_MODE_DPMS_ON is still set. This WARN_ON does make some sense
> > because DPMS_OFF would have the effect of turning off clocks and putting the
> > monitor to sleep which seems logical considering we have torn down the
> > display. Adding a tilcdc_crtc_dpms(DPMS_OFF) right before the WARN_ON
> > confirms this, but it seems strange that this hasn't happened automatically
> > (+ Russell doesn't need to do it in his Armada driver) - so I suspect there
> > is a better way.
>
> tbh, I'm not entirely sure offhand why drm_mode_config_cleanup()
> doesn't remove the fb's first (which should have the effect of
> shutting down any lit crtc/encoder/connector).. that would seem like
> the sensible way to shut down..
All userspace fbs should be cleared already before going into the driver
unload. Which only leaves you with driver internal fbs (usually just one
for fbdev emulation). It's the driver's job to clean that up explicitly.
Then you can call mode_config_cleanup and the WARN_ON in there is a really
nice space leak check.
If we'd unconditionally clean up all fbs we'd have trouble with
driver-private embedded fbs and their refcounting and would loose the
space leak check.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-27 22:08 ` Darren Etheridge
@ 2014-07-01 23:39 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-07-01 23:39 UTC (permalink / raw)
To: Darren Etheridge
Cc: Russell King, Daniel Vetter, dri-devel, Daniel Mack,
Ezequiel García, linux-omap, linux-arm-kernel
On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
> [snip]
>
> Otherwise I think this is a good and useful patch series.
It that a Tested-by tag? :)
Thanks!
Guido
>
> Darren
>
> >The first 7 patches are bug fixes which and should probably be applied
> >in the stable trees. The last two are clean-ups.
> >
> >
> >Resending this since I got no replies.
> >
> >
> >Guido Martínez (9):
> > drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> > drm/tilcdc: panel: fix dangling sysfs connector node
> > drm/tilcdc: slave: fix dangling sysfs connector node
> > drm/tilcdc: tfp410: fix dangling sysfs connector node
> > drm/tilcdc: panel: fix leak when unloading the module
> > drm/tilcdc: fix release order on exit
> > drm/tilcdc: fix double kfree
> > drm/tilcdc: remove submodule destroy calls
> > drm/tilcdc: replace late_initcall with module_init
> >
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > drivers/gpu/drm/tilcdc/Module.symvers | 0
> > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> > drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> > 7 files changed, 59 insertions(+), 60 deletions(-)
> > create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
> >
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-01 23:39 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-07-01 23:39 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
> [snip]
>
> Otherwise I think this is a good and useful patch series.
It that a Tested-by tag? :)
Thanks!
Guido
>
> Darren
>
> >The first 7 patches are bug fixes which and should probably be applied
> >in the stable trees. The last two are clean-ups.
> >
> >
> >Resending this since I got no replies.
> >
> >
> >Guido Mart?nez (9):
> > drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> > drm/tilcdc: panel: fix dangling sysfs connector node
> > drm/tilcdc: slave: fix dangling sysfs connector node
> > drm/tilcdc: tfp410: fix dangling sysfs connector node
> > drm/tilcdc: panel: fix leak when unloading the module
> > drm/tilcdc: fix release order on exit
> > drm/tilcdc: fix double kfree
> > drm/tilcdc: remove submodule destroy calls
> > drm/tilcdc: replace late_initcall with module_init
> >
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > drivers/gpu/drm/tilcdc/Module.symvers | 0
> > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> > drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> > drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> > drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> > drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> > 7 files changed, 59 insertions(+), 60 deletions(-)
> > create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
> >
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-07-01 23:39 ` Guido Martínez
@ 2014-07-02 2:31 ` Darren Etheridge
-1 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-07-02 2:31 UTC (permalink / raw)
To: Guido Martínez
Cc: Russell King, Daniel Vetter, dri-devel, Daniel Mack,
Ezequiel García, linux-omap, linux-arm-kernel
On 07/01/2014 06:39 PM, Guido Martínez wrote:
> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>> [snip]
>>
>> Otherwise I think this is a good and useful patch series.
> It that a Tested-by tag? :)
Sure - I would prefer that the WARN_ON was silenced when the tilcdc
module is removed, but the series adds improvements over what is there
now. I have tested it across a few variants of AM335x boards and
attached display hardware in both module form and built-in to the
kernel, therefore:
Tested-by: Darren Etheridge <detheridge@ti.com>
>
> Thanks!
> Guido
>
>>
>> Darren
>>
>>> The first 7 patches are bug fixes which and should probably be applied
>>> in the stable trees. The last two are clean-ups.
>>>
>>>
>>> Resending this since I got no replies.
>>>
>>>
>>> Guido Martínez (9):
>>> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
>>> drm/tilcdc: panel: fix dangling sysfs connector node
>>> drm/tilcdc: slave: fix dangling sysfs connector node
>>> drm/tilcdc: tfp410: fix dangling sysfs connector node
>>> drm/tilcdc: panel: fix leak when unloading the module
>>> drm/tilcdc: fix release order on exit
>>> drm/tilcdc: fix double kfree
>>> drm/tilcdc: remove submodule destroy calls
>>> drm/tilcdc: replace late_initcall with module_init
>>>
>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>> drivers/gpu/drm/tilcdc/Module.symvers | 0
>>> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
>>> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
>>> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
>>> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
>>> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
>>> 7 files changed, 59 insertions(+), 60 deletions(-)
>>> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>>>
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-02 2:31 ` Darren Etheridge
0 siblings, 0 replies; 76+ messages in thread
From: Darren Etheridge @ 2014-07-02 2:31 UTC (permalink / raw)
To: linux-arm-kernel
On 07/01/2014 06:39 PM, Guido Mart?nez wrote:
> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>> [snip]
>>
>> Otherwise I think this is a good and useful patch series.
> It that a Tested-by tag? :)
Sure - I would prefer that the WARN_ON was silenced when the tilcdc
module is removed, but the series adds improvements over what is there
now. I have tested it across a few variants of AM335x boards and
attached display hardware in both module form and built-in to the
kernel, therefore:
Tested-by: Darren Etheridge <detheridge@ti.com>
>
> Thanks!
> Guido
>
>>
>> Darren
>>
>>> The first 7 patches are bug fixes which and should probably be applied
>>> in the stable trees. The last two are clean-ups.
>>>
>>>
>>> Resending this since I got no replies.
>>>
>>>
>>> Guido Mart?nez (9):
>>> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
>>> drm/tilcdc: panel: fix dangling sysfs connector node
>>> drm/tilcdc: slave: fix dangling sysfs connector node
>>> drm/tilcdc: tfp410: fix dangling sysfs connector node
>>> drm/tilcdc: panel: fix leak when unloading the module
>>> drm/tilcdc: fix release order on exit
>>> drm/tilcdc: fix double kfree
>>> drm/tilcdc: remove submodule destroy calls
>>> drm/tilcdc: replace late_initcall with module_init
>>>
>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>> drivers/gpu/drm/tilcdc/Module.symvers | 0
>>> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
>>> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
>>> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
>>> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
>>> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
>>> 7 files changed, 59 insertions(+), 60 deletions(-)
>>> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>>>
>
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-07-02 2:31 ` Darren Etheridge
@ 2014-07-02 4:08 ` Dave Airlie
-1 siblings, 0 replies; 76+ messages in thread
From: Dave Airlie @ 2014-07-02 4:08 UTC (permalink / raw)
To: Darren Etheridge
Cc: Guido Martínez, Russell King, Daniel Vetter, dri-devel,
Daniel Mack, Ezequiel García, linux-omap, linux-arm-kernel
On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
> On 07/01/2014 06:39 PM, Guido Martínez wrote:
>>
>> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>>>
>>> [snip]
>>>
>>> Otherwise I think this is a good and useful patch series.
>>
>> It that a Tested-by tag? :)
>
>
> Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
> is removed, but the series adds improvements over what is there now. I have
> tested it across a few variants of AM335x boards and attached display
> hardware in both module form and built-in to the kernel, therefore:
>
> Tested-by: Darren Etheridge <detheridge@ti.com>
Has someone gathered that tags and put these in a git tree by any chance?
Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-02 4:08 ` Dave Airlie
0 siblings, 0 replies; 76+ messages in thread
From: Dave Airlie @ 2014-07-02 4:08 UTC (permalink / raw)
To: linux-arm-kernel
On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
> On 07/01/2014 06:39 PM, Guido Mart?nez wrote:
>>
>> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>>>
>>> [snip]
>>>
>>> Otherwise I think this is a good and useful patch series.
>>
>> It that a Tested-by tag? :)
>
>
> Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
> is removed, but the series adds improvements over what is there now. I have
> tested it across a few variants of AM335x boards and attached display
> hardware in both module form and built-in to the kernel, therefore:
>
> Tested-by: Darren Etheridge <detheridge@ti.com>
Has someone gathered that tags and put these in a git tree by any chance?
Dave.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-07-02 4:08 ` Dave Airlie
@ 2014-07-02 20:38 ` Ezequiel García
-1 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-07-02 20:38 UTC (permalink / raw)
To: Dave Airlie
Cc: Darren Etheridge, linux-arm-kernel, Russell King, Daniel Vetter,
dri-devel, Daniel Mack, linux-omap
On 02 Jul 02:08 PM, Dave Airlie wrote:
> On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
> > On 07/01/2014 06:39 PM, Guido Martínez wrote:
> >>
> >> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
> >>>
> >>> [snip]
> >>>
> >>> Otherwise I think this is a good and useful patch series.
> >>
> >> It that a Tested-by tag? :)
> >
> >
> > Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
> > is removed, but the series adds improvements over what is there now. I have
> > tested it across a few variants of AM335x boards and attached display
> > hardware in both module form and built-in to the kernel, therefore:
> >
> > Tested-by: Darren Etheridge <detheridge@ti.com>
>
> Has someone gathered that tags and put these in a git tree by any chance?
>
I don't think anyone has. Is that a problem?
If you need a pull request with these, I can prepare something in bitbucket.
Would that work you?
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-02 20:38 ` Ezequiel García
0 siblings, 0 replies; 76+ messages in thread
From: Ezequiel García @ 2014-07-02 20:38 UTC (permalink / raw)
To: linux-arm-kernel
On 02 Jul 02:08 PM, Dave Airlie wrote:
> On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
> > On 07/01/2014 06:39 PM, Guido Mart?nez wrote:
> >>
> >> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
> >>>
> >>> [snip]
> >>>
> >>> Otherwise I think this is a good and useful patch series.
> >>
> >> It that a Tested-by tag? :)
> >
> >
> > Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
> > is removed, but the series adds improvements over what is there now. I have
> > tested it across a few variants of AM335x boards and attached display
> > hardware in both module form and built-in to the kernel, therefore:
> >
> > Tested-by: Darren Etheridge <detheridge@ti.com>
>
> Has someone gathered that tags and put these in a git tree by any chance?
>
I don't think anyone has. Is that a problem?
If you need a pull request with these, I can prepare something in bitbucket.
Would that work you?
--
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-07-02 20:38 ` Ezequiel García
@ 2014-07-08 1:32 ` Dave Airlie
-1 siblings, 0 replies; 76+ messages in thread
From: Dave Airlie @ 2014-07-08 1:32 UTC (permalink / raw)
To: Ezequiel García
Cc: Darren Etheridge, linux-arm-kernel, Russell King, Daniel Vetter,
dri-devel, Daniel Mack, linux-omap
On 3 July 2014 06:38, Ezequiel García <ezequiel@vanguardiasur.com.ar> wrote:
> On 02 Jul 02:08 PM, Dave Airlie wrote:
>> On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
>> > On 07/01/2014 06:39 PM, Guido Martínez wrote:
>> >>
>> >> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>> >>>
>> >>> [snip]
>> >>>
>> >>> Otherwise I think this is a good and useful patch series.
>> >>
>> >> It that a Tested-by tag? :)
>> >
>> >
>> > Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
>> > is removed, but the series adds improvements over what is there now. I have
>> > tested it across a few variants of AM335x boards and attached display
>> > hardware in both module form and built-in to the kernel, therefore:
>> >
>> > Tested-by: Darren Etheridge <detheridge@ti.com>
>>
>> Has someone gathered that tags and put these in a git tree by any chance?
>>
>
> I don't think anyone has. Is that a problem?
>
> If you need a pull request with these, I can prepare something in bitbucket.
> Would that work you?
I've picked them up manually now, so should be all fine.
Thanks,
Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-08 1:32 ` Dave Airlie
0 siblings, 0 replies; 76+ messages in thread
From: Dave Airlie @ 2014-07-08 1:32 UTC (permalink / raw)
To: linux-arm-kernel
On 3 July 2014 06:38, Ezequiel Garc?a <ezequiel@vanguardiasur.com.ar> wrote:
> On 02 Jul 02:08 PM, Dave Airlie wrote:
>> On 2 July 2014 12:31, Darren Etheridge <detheridge@ti.com> wrote:
>> > On 07/01/2014 06:39 PM, Guido Mart?nez wrote:
>> >>
>> >> On Fri, Jun 27, 2014 at 05:08:51PM -0500, Darren Etheridge wrote:
>> >>>
>> >>> [snip]
>> >>>
>> >>> Otherwise I think this is a good and useful patch series.
>> >>
>> >> It that a Tested-by tag? :)
>> >
>> >
>> > Sure - I would prefer that the WARN_ON was silenced when the tilcdc module
>> > is removed, but the series adds improvements over what is there now. I have
>> > tested it across a few variants of AM335x boards and attached display
>> > hardware in both module form and built-in to the kernel, therefore:
>> >
>> > Tested-by: Darren Etheridge <detheridge@ti.com>
>>
>> Has someone gathered that tags and put these in a git tree by any chance?
>>
>
> I don't think anyone has. Is that a problem?
>
> If you need a pull request with these, I can prepare something in bitbucket.
> Would that work you?
I've picked them up manually now, so should be all fine.
Thanks,
Dave.
^ permalink raw reply [flat|nested] 76+ messages in thread
* Re: [PATCH/RESEND 0/9] drm: tilcdc driver fixes
2014-06-17 14:17 ` Guido Martínez
@ 2014-07-01 23:52 ` Guido Martínez
-1 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-07-01 23:52 UTC (permalink / raw)
To: airlied
Cc: robdclark, Ezequiel García, Daniel Mack, Russell King,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-omap
Hi Dave,
did you take a look at this patchset? I foolishly missed adding you on
the To: header, so apologies for that in advance.
Thanks,
On Tue, Jun 17, 2014 at 11:17:02AM -0300, Guido Martínez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Martínez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> --
> 2.0.0
>
--
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 76+ messages in thread
* [PATCH/RESEND 0/9] drm: tilcdc driver fixes
@ 2014-07-01 23:52 ` Guido Martínez
0 siblings, 0 replies; 76+ messages in thread
From: Guido Martínez @ 2014-07-01 23:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Dave,
did you take a look at this patchset? I foolishly missed adding you on
the To: header, so apologies for that in advance.
Thanks,
On Tue, Jun 17, 2014 at 11:17:02AM -0300, Guido Mart?nez wrote:
> The tilcdc driver could be compiled as a module, but was severely broken
> and could not be used as such. This patchset attempts to fix the issues
> preventing a proper load/unload of the module.
>
> Issues included dangling sysfs nodes, dangling devices, memory leaks and
> a double kfree.
>
> It now seems to be working ok. We have tested this by loading and
> unloading the driver repeteadly, with both panel and slave connectors
> and found no flaws.
>
> There is still one warning left on tilcdc_crtc_destroy, caused by
> destroying the connector while still in an ON status. We don't know why
> this happens or why it's an issue, so we did not fix it.
>
> The first 7 patches are bug fixes which and should probably be applied
> in the stable trees. The last two are clean-ups.
>
>
> Resending this since I got no replies.
>
>
> Guido Mart?nez (9):
> drm/i2c: tda998x: move drm_i2c_encoder_destroy call
> drm/tilcdc: panel: fix dangling sysfs connector node
> drm/tilcdc: slave: fix dangling sysfs connector node
> drm/tilcdc: tfp410: fix dangling sysfs connector node
> drm/tilcdc: panel: fix leak when unloading the module
> drm/tilcdc: fix release order on exit
> drm/tilcdc: fix double kfree
> drm/tilcdc: remove submodule destroy calls
> drm/tilcdc: replace late_initcall with module_init
>
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/Module.symvers | 0
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 15 +++++--------
> drivers/gpu/drm/tilcdc/tilcdc_drv.h | 1 -
> drivers/gpu/drm/tilcdc/tilcdc_panel.c | 39 +++++++++++++++++-----------------
> drivers/gpu/drm/tilcdc/tilcdc_slave.c | 27 +++++++++++++----------
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 35 +++++++++++++++---------------
> 7 files changed, 59 insertions(+), 60 deletions(-)
> create mode 100644 drivers/gpu/drm/tilcdc/Module.symvers
>
> --
> 2.0.0
>
--
Guido Mart?nez, VanguardiaSur
www.vanguardiasur.com.ar
^ permalink raw reply [flat|nested] 76+ messages in thread