The best_encoder() callback is used by the drm-core to find an encoder if the connector is connected to multiple encoders but the parallel, tve and ldb uses always the 1-encoder : 1-connector setup. Such a simple setup can be handled by the drm-core.

Signed-off-by: Marco Felsch m.felsch@pengutronix.de Reviewed-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/imx/imx-ldb.c | 9 --------- drivers/gpu/drm/imx/imx-tve.c | 9 --------- drivers/gpu/drm/imx/parallel-display.c | 9 --------- 3 files changed, 27 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 8cb2665b2c74..cd399058b00e 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -155,14 +155,6 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector) return num_modes; }

-static struct drm_encoder *imx_ldb_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector); - - return &imx_ldb_ch->encoder; -} - static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, unsigned long serial_clk, unsigned long di_clk) { @@ -390,7 +382,6 @@ static const struct drm_connector_funcs imx_ldb_connector_funcs = {

static const struct drm_connector_helper_funcs imx_ldb_connector_helper_funcs = { .get_modes = imx_ldb_connector_get_modes, - .best_encoder = imx_ldb_connector_best_encoder, };

static const struct drm_encoder_funcs imx_ldb_encoder_funcs = { diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index 5bbfaa2cd0f4..e113014a1a79 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -259,14 +259,6 @@ static int imx_tve_connector_mode_valid(struct drm_connector *connector, return MODE_BAD; }

-static struct drm_encoder *imx_tve_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_tve *tve = con_to_tve(connector); - - return &tve->encoder; -} - static void imx_tve_encoder_mode_set(struct drm_encoder *encoder, struct drm_display_mode *orig_mode, struct drm_display_mode *mode) @@ -344,7 +336,6 @@ static const struct drm_connector_funcs imx_tve_connector_funcs = {

static const struct drm_connector_helper_funcs imx_tve_connector_helper_funcs = { .get_modes = imx_tve_connector_get_modes, - .best_encoder = imx_tve_connector_best_encoder, .mode_valid = imx_tve_connector_mode_valid, };

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 3dca424059f7..b2dbf0bc4cdc 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -81,14 +81,6 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector) return num_modes; }

-static struct drm_encoder *imx_pd_connector_best_encoder( - struct drm_connector *connector) -{ - struct imx_parallel_display *imxpd = con_to_imxpd(connector); - - return &imxpd->encoder; -} - static void imx_pd_encoder_enable(struct drm_encoder *encoder) { struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); @@ -136,7 +128,6 @@ static const struct drm_connector_funcs imx_pd_connector_funcs = {

static const struct drm_connector_helper_funcs imx_pd_connector_helper_funcs = { .get_modes = imx_pd_connector_get_modes, - .best_encoder = imx_pd_connector_best_encoder, };

static const struct drm_encoder_funcs imx_pd_encoder_funcs = {

We do some string parsing and string comparison in front of drm_of_find_panel_or_bridge(). All this work is useless if the call fails. Move drm_of_find_panel_or_bridge() infront of the parsing work to fail early.

Signed-off-by: Marco Felsch m.felsch@pengutronix.de Reviewed-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/imx/parallel-display.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 1f08fe8337d7..6ee028faa9a8 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -200,6 +200,11 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) if (!imxpd) return -ENOMEM;

+ /* port@1 is the output port */ + ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel, &imxpd->bridge); + if (ret && ret != -ENODEV) + return ret; + edidp = of_get_property(np, "edid", &imxpd->edid_len); if (edidp) imxpd->edid = devm_kmemdup(dev, edidp, imxpd->edid_len, @@ -218,10 +223,6 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) } imxpd->bus_format = bus_format;

- /* port@1 is the output port */ - ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel, &imxpd->bridge); - if (ret && ret != -ENODEV) - return ret;

imxpd->dev = dev;

This commit prepares the driver to fix the race condition reported by [1]. The goal is to switch from the devres-kmalloc and the component framework to a 'normal' kmalloc and the drm framework to release the memory resources. So all acquired memory resoruces are freed by a drm_mode_config_cleanup() call and the ->destroy() callbacks. For this purpose we need to split off the drm_connector structure.

[1] https://www.spinics.net/lists/dri-devel/msg189388.html

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/parallel-display.c | 84 ++++++++++++++++---------- 1 file changed, 53 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index e731b19fd6b2..a05808982f2f 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -22,21 +22,25 @@ #include "imx-drm.h"

struct imx_parallel_display { - struct drm_connector connector; struct drm_encoder encoder; struct device *dev; - void *edid; - int edid_len; u32 bus_format; u32 bus_flags; - struct drm_display_mode mode; struct drm_panel *panel; struct drm_bridge *bridge; };

-static inline struct imx_parallel_display *con_to_imxpd(struct drm_connector *c) +struct imx_parallel_connector { + struct imx_parallel_display *imxpd; + struct drm_connector connector; + void *edid; + int edid_len; + struct drm_display_mode mode; +}; + +static inline struct imx_parallel_connector *con_to_imxpc(struct drm_connector *c) { - return container_of(c, struct imx_parallel_display, connector); + return container_of(c, struct imx_parallel_connector, connector); }

static inline struct imx_parallel_display *enc_to_imxpd(struct drm_encoder *e) @@ -44,9 +48,19 @@ static inline struct imx_parallel_display *enc_to_imxpd(struct drm_encoder *e) return container_of(e, struct imx_parallel_display, encoder); }

+static void imx_pd_connector_destroy(struct drm_connector *connector) +{ + struct imx_parallel_connector *imxpc = con_to_imxpc(connector); + + imx_drm_connector_destroy(connector); + /* avoid dangling pointer */ + imxpc->imxpd = NULL; +} + static int imx_pd_connector_get_modes(struct drm_connector *connector) { - struct imx_parallel_display *imxpd = con_to_imxpd(connector); + struct imx_parallel_connector *imxpc = con_to_imxpc(connector); + struct imx_parallel_display *imxpd = imxpc->imxpd; struct device_node *np = imxpd->dev->of_node; int num_modes;

@@ -54,9 +68,9 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector) if (num_modes > 0) return num_modes;

- if (imxpd->edid) { - drm_connector_update_edid_property(connector, imxpd->edid); - num_modes = drm_add_edid_modes(connector, imxpd->edid); + if (imxpc->edid) { + drm_connector_update_edid_property(connector, imxpc->edid); + num_modes = drm_add_edid_modes(connector, imxpc->edid); }

if (np) { @@ -66,13 +80,13 @@ static int imx_pd_connector_get_modes(struct drm_connector *connector) if (!mode) return -EINVAL;

- ret = of_get_drm_display_mode(np, &imxpd->mode, + ret = of_get_drm_display_mode(np, &imxpc->mode, &imxpd->bus_flags, OF_USE_NATIVE_MODE); if (ret) return ret;

- drm_mode_copy(mode, &imxpd->mode); + drm_mode_copy(mode, &imxpc->mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED, drm_mode_probed_add(connector, mode); num_modes++; @@ -129,7 +143,7 @@ static int imx_pd_encoder_atomic_check(struct drm_encoder *encoder,

static const struct drm_connector_funcs imx_pd_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = imx_drm_connector_destroy, + .destroy = imx_pd_connector_destroy, .reset = drm_atomic_helper_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, @@ -150,7 +164,8 @@ static const struct drm_encoder_helper_funcs imx_pd_encoder_helper_funcs = { };

static int imx_pd_register(struct drm_device *drm, - struct imx_parallel_display *imxpd) + struct imx_parallel_display *imxpd, + struct imx_parallel_connector *imxpc) { struct drm_encoder *encoder = &imxpd->encoder; int ret; @@ -159,27 +174,26 @@ static int imx_pd_register(struct drm_device *drm, if (ret) return ret;

- /* set the connector's dpms to OFF so that - * drm_helper_connector_dpms() won't return - * immediately since the current state is ON - * at this point. - */ - imxpd->connector.dpms = DRM_MODE_DPMS_OFF; - drm_encoder_helper_add(encoder, &imx_pd_encoder_helper_funcs); drm_encoder_init(drm, encoder, &imx_pd_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL);

if (!imxpd->bridge) { - drm_connector_helper_add(&imxpd->connector, + /* set the connector's dpms to OFF so that + * drm_helper_connector_dpms() won't return + * immediately since the current state is ON + * at this point. + */ + imxpc->connector.dpms = DRM_MODE_DPMS_OFF; + drm_connector_helper_add(&imxpc->connector, &imx_pd_connector_helper_funcs); - drm_connector_init(drm, &imxpd->connector, + drm_connector_init(drm, &imxpc->connector, &imx_pd_connector_funcs, DRM_MODE_CONNECTOR_DPI); }

if (imxpd->panel) - drm_panel_attach(imxpd->panel, &imxpd->connector); + drm_panel_attach(imxpd->panel, &imxpc->connector);

if (imxpd->bridge) { ret = drm_bridge_attach(encoder, imxpd->bridge, NULL); @@ -189,7 +203,7 @@ static int imx_pd_register(struct drm_device *drm, return ret; } } else { - drm_connector_attach_encoder(&imxpd->connector, encoder); + drm_connector_attach_encoder(&imxpc->connector, encoder); }

return 0; @@ -201,6 +215,7 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) struct device_node *np = dev->of_node; const u8 *edidp; struct imx_parallel_display *imxpd; + struct imx_parallel_connector *imxpc; int ret; u32 bus_format = 0; const char *fmt; @@ -214,10 +229,18 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) if (ret && ret != -ENODEV) return ret;

- edidp = of_get_property(np, "edid", &imxpd->edid_len); - if (edidp) - imxpd->edid = devm_kmemdup(dev, edidp, imxpd->edid_len, - GFP_KERNEL); + if (!imxpd->bridge) { + imxpc = devm_kzalloc(dev, sizeof(*imxpc), GFP_KERNEL); + if (!imxpc) + return -ENOMEM; + + imxpc->imxpd = imxpd; + + edidp = of_get_property(np, "edid", &imxpc->edid_len); + if (edidp) + imxpc->edid = devm_kmemdup(dev, edidp, imxpc->edid_len, + GFP_KERNEL); + }

ret = of_property_read_string(np, "interface-pix-fmt", &fmt); if (!ret) { @@ -232,10 +255,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) } imxpd->bus_format = bus_format;

- imxpd->dev = dev;

- ret = imx_pd_register(drm, imxpd); + ret = imx_pd_register(drm, imxpd, imxpc); if (ret) return ret;

To avoid a dedicated unbind() callback we can use the new devm_action mechanism. This allows us to drop the error check because the callback will only be executed if the regulator was registered.

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/imx-tve.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index e113014a1a79..3340ba6e0222 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -485,6 +485,13 @@ static int imx_tve_register(struct drm_device *drm, struct imx_tve *tve) return 0; }

+static void imx_tve_disable_regulator(void *data) +{ + struct imx_tve *tve = data; + + regulator_disable(tve->dac_reg); +} + static bool imx_tve_readable_reg(struct device *dev, unsigned int reg) { return (reg % 4 == 0) && (reg <= 0xdc); @@ -609,6 +616,9 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) ret = regulator_enable(tve->dac_reg); if (ret) return ret; + ret = devm_add_action_or_reset(dev, imx_tve_disable_regulator, tve); + if (ret) + return ret; }

tve->clk = devm_clk_get(dev, "tve"); @@ -655,18 +665,8 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) return 0; }

-static void imx_tve_unbind(struct device *dev, struct device *master, - void *data) -{ - struct imx_tve *tve = dev_get_drvdata(dev); - - if (!IS_ERR(tve->dac_reg)) - regulator_disable(tve->dac_reg); -} - static const struct component_ops imx_tve_ops = { .bind = imx_tve_bind, - .unbind = imx_tve_unbind, };

static int imx_tve_probe(struct platform_device *pdev)

Since commit 5e501ed7253b ("drm/imx: imx-ldb: allow to determine bus format from the connected panel") the enum isn't used anymore. Drop it to cleanup the code a bit.

Signed-off-by: Marco Felsch m.felsch@pengutronix.de Reviewed-by: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/imx/imx-ldb.c | 5 ----- 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index cd399058b00e..92fb85703c73 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -468,11 +468,6 @@ static int imx_ldb_register(struct drm_device *drm, return 0; }

-enum { - LVDS_BIT_MAP_SPWG, - LVDS_BIT_MAP_JEIDA -}; - struct imx_ldb_bit_mapping { u32 bus_format; u32 datawidth;

There is no reason why the resoruces should be freed manually within the component.unbind() call instead it is easier to use the drm-core destroy helper. The destroy helper gets called for each registered encoder during a drm_mode_config_cleanup() call.

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/imx-ldb.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 06b435f9b6c9..87bf659990da 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -155,6 +155,16 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector) return num_modes; }

+static void imx_ldb_encoder_destroy(struct drm_encoder *encoder) +{ + struct imx_ldb_channel *channel = enc_to_imx_ldb_ch(encoder); + + if (channel->panel) + drm_panel_detach(channel->panel); + drm_encoder_cleanup(encoder); + i2c_put_adapter(channel->ddc); +} + static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, unsigned long serial_clk, unsigned long di_clk) { @@ -385,7 +395,7 @@ static const struct drm_connector_helper_funcs imx_ldb_connector_helper_funcs = };

static const struct drm_encoder_funcs imx_ldb_encoder_funcs = { - .destroy = imx_drm_encoder_destroy, + .destroy = imx_ldb_encoder_destroy, };

static const struct drm_encoder_helper_funcs imx_ldb_encoder_helper_funcs = { @@ -699,25 +709,8 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) return ret; }

-static void imx_ldb_unbind(struct device *dev, struct device *master, - void *data) -{ - struct imx_ldb *imx_ldb = dev_get_drvdata(dev); - int i; - - for (i = 0; i < 2; i++) { - struct imx_ldb_channel *channel = &imx_ldb->channel[i]; - - if (channel->panel) - drm_panel_detach(channel->panel); - - i2c_put_adapter(channel->ddc); - } -} - static const struct component_ops imx_ldb_ops = { .bind = imx_ldb_bind, - .unbind = imx_ldb_unbind, };

static int imx_ldb_probe(struct platform_device *pdev)

This patch prepares the driver for a overall 'make drm/imx drm_mode_config_cleanup() aware' patch to avoid race conditions like [1]. The patch also improves the driver memory usage by allocating only the necessary number of 'struct imx_ldb_channel'.

To make the driver drm_mode_config_cleanup() aware we need to split the driver state 'struct imx_ldb'. All the hardware settings like clocks and the regmap can be allocated during the driver probe() and so the devres context is moved to the probe/remove.

Now upon a component.bind() call only the necessary number of channels are allocated. The difference is that now the 'struct imx_ldb_channel' memory is allocated within the bind/unbind devres context.

So now the 'struct imx_ldb' driver state is still available after a component.unbind() call.

[1] https://www.spinics.net/lists/dri-devel/msg189388.html

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/imx-ldb.c | 187 ++++++++++++++++++---------------- 1 file changed, 98 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 87bf659990da..e3fa46e1639d 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -87,7 +87,6 @@ struct bus_mux { struct imx_ldb { struct regmap *regmap; struct device *dev; - struct imx_ldb_channel channel[2]; struct clk *clk[2]; /* our own clock */ struct clk *clk_sel[4]; /* parent of display clock */ struct clk *clk_parent[4]; /* original parent of clk_sel */ @@ -212,14 +211,14 @@ static void imx_ldb_encoder_enable(struct drm_encoder *encoder) clk_set_parent(ldb->clk_sel[mux], ldb->clk[imx_ldb_ch->chno]); }

- if (imx_ldb_ch == &ldb->channel[0] || dual) { + if (imx_ldb_ch->chno == 0 || dual) { ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK; if (mux == 0 || ldb->lvds_mux) ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI0; else if (mux == 1) ldb->ldb_ctrl |= LDB_CH0_MODE_EN_TO_DI1; } - if (imx_ldb_ch == &ldb->channel[1] || dual) { + if (imx_ldb_ch->chno == 1 || dual) { ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK; if (mux == 1 || ldb->lvds_mux) ldb->ldb_ctrl |= LDB_CH1_MODE_EN_TO_DI1; @@ -230,9 +229,9 @@ static void imx_ldb_encoder_enable(struct drm_encoder *encoder) if (ldb->lvds_mux) { const struct bus_mux *lvds_mux = NULL;

- if (imx_ldb_ch == &ldb->channel[0]) + if (imx_ldb_ch->chno == 0) lvds_mux = &ldb->lvds_mux[0]; - else if (imx_ldb_ch == &ldb->channel[1]) + else if (imx_ldb_ch->chno == 1) lvds_mux = &ldb->lvds_mux[1];

regmap_update_bits(ldb->regmap, lvds_mux->reg, lvds_mux->mask, @@ -278,13 +277,13 @@ imx_ldb_encoder_atomic_mode_set(struct drm_encoder *encoder, }

/* FIXME - assumes straight connections DI0 --> CH0, DI1 --> CH1 */ - if (imx_ldb_ch == &ldb->channel[0] || dual) { + if (imx_ldb_ch->chno == 0 || dual) { if (mode->flags & DRM_MODE_FLAG_NVSYNC) ldb->ldb_ctrl |= LDB_DI0_VS_POL_ACT_LOW; else if (mode->flags & DRM_MODE_FLAG_PVSYNC) ldb->ldb_ctrl &= ~LDB_DI0_VS_POL_ACT_LOW; } - if (imx_ldb_ch == &ldb->channel[1] || dual) { + if (imx_ldb_ch->chno == 1 || dual) { if (mode->flags & DRM_MODE_FLAG_NVSYNC) ldb->ldb_ctrl |= LDB_DI1_VS_POL_ACT_LOW; else if (mode->flags & DRM_MODE_FLAG_PVSYNC) @@ -309,9 +308,9 @@ static void imx_ldb_encoder_disable(struct drm_encoder *encoder)

drm_panel_disable(imx_ldb_ch->panel);

- if (imx_ldb_ch == &ldb->channel[0]) + if (imx_ldb_ch->chno == 0) ldb->ldb_ctrl &= ~LDB_CH0_MODE_EN_MASK; - else if (imx_ldb_ch == &ldb->channel[1]) + else if (imx_ldb_ch->chno == 1) ldb->ldb_ctrl &= ~LDB_CH1_MODE_EN_MASK;

regmap_write(ldb->regmap, IOMUXC_GPR2, ldb->ldb_ctrl); @@ -324,16 +323,16 @@ static void imx_ldb_encoder_disable(struct drm_encoder *encoder) if (ldb->lvds_mux) { const struct bus_mux *lvds_mux = NULL;

- if (imx_ldb_ch == &ldb->channel[0]) + if (imx_ldb_ch->chno == 0) lvds_mux = &ldb->lvds_mux[0]; - else if (imx_ldb_ch == &ldb->channel[1]) + else if (imx_ldb_ch->chno == 1) lvds_mux = &ldb->lvds_mux[1];

regmap_read(ldb->regmap, lvds_mux->reg, &mux); mux &= lvds_mux->mask; mux >>= lvds_mux->shift; } else { - mux = (imx_ldb_ch == &ldb->channel[0]) ? 0 : 1; + mux = imx_ldb_ch->chno; }

/* set display clock mux back to original input clock */ @@ -513,31 +512,6 @@ static u32 of_get_bus_format(struct device *dev, struct device_node *np) return -ENOENT; }

-static struct bus_mux imx6q_lvds_mux[2] = { - { - .reg = IOMUXC_GPR3, - .shift = 6, - .mask = IMX6Q_GPR3_LVDS0_MUX_CTL_MASK, - }, { - .reg = IOMUXC_GPR3, - .shift = 8, - .mask = IMX6Q_GPR3_LVDS1_MUX_CTL_MASK, - } -}; - -/* - * For a device declaring compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb", - * of_match_device will walk through this list and take the first entry - * matching any of its compatible values. Therefore, the more generic - * entries (in this case fsl,imx53-ldb) need to be ordered last. - */ -static const struct of_device_id imx_ldb_dt_ids[] = { - { .compatible = "fsl,imx6q-ldb", .data = imx6q_lvds_mux, }, - { .compatible = "fsl,imx53-ldb", .data = NULL, }, - { } -}; -MODULE_DEVICE_TABLE(of, imx_ldb_dt_ids); - static int imx_ldb_panel_ddc(struct device *dev, struct imx_ldb_channel *channel, struct device_node *child) { @@ -582,59 +556,12 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) { struct drm_device *drm = data; struct device_node *np = dev->of_node; - const struct of_device_id *of_id = - of_match_device(imx_ldb_dt_ids, dev); + struct imx_ldb *imx_ldb = dev_get_drvdata(dev); + int dual = imx_ldb->ldb_ctrl & LDB_SPLIT_MODE_EN; struct device_node *child; - struct imx_ldb *imx_ldb; - int dual; int ret; int i;

- imx_ldb = devm_kzalloc(dev, sizeof(*imx_ldb), GFP_KERNEL); - if (!imx_ldb) - return -ENOMEM; - - imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr"); - if (IS_ERR(imx_ldb->regmap)) { - dev_err(dev, "failed to get parent regmap\n"); - return PTR_ERR(imx_ldb->regmap); - } - - /* disable LDB by resetting the control register to POR default */ - regmap_write(imx_ldb->regmap, IOMUXC_GPR2, 0); - - imx_ldb->dev = dev; - - if (of_id) - imx_ldb->lvds_mux = of_id->data; - - dual = of_property_read_bool(np, "fsl,dual-channel"); - if (dual) - imx_ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN; - - /* - * There are three different possible clock mux configurations: - * i.MX53: ipu1_di0_sel, ipu1_di1_sel - * i.MX6q: ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, ipu2_di1_sel - * i.MX6dl: ipu1_di0_sel, ipu1_di1_sel, lcdif_sel - * Map them all to di0_sel...di3_sel. - */ - for (i = 0; i < 4; i++) { - char clkname[16]; - - sprintf(clkname, "di%d_sel", i); - imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname); - if (IS_ERR(imx_ldb->clk_sel[i])) { - ret = PTR_ERR(imx_ldb->clk_sel[i]); - imx_ldb->clk_sel[i] = NULL; - break; - } - - imx_ldb->clk_parent[i] = clk_get_parent(imx_ldb->clk_sel[i]); - } - if (i == 0) - return ret; - for_each_child_of_node(np, child) { struct imx_ldb_channel *channel; int bus_format; @@ -653,7 +580,10 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) continue; }

- channel = &imx_ldb->channel[i]; + channel = devm_kzalloc(dev, sizeof(*channel), GFP_KERNEL); + if (!channel) + return -ENOMEM; + channel->ldb = imx_ldb; channel->chno = i;

@@ -700,8 +630,6 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) } }

- dev_set_drvdata(dev, imx_ldb); - return 0;

free_child: @@ -713,8 +641,89 @@ static const struct component_ops imx_ldb_ops = { .bind = imx_ldb_bind, };

+static struct bus_mux imx6q_lvds_mux[2] = { + { + .reg = IOMUXC_GPR3, + .shift = 6, + .mask = IMX6Q_GPR3_LVDS0_MUX_CTL_MASK, + }, { + .reg = IOMUXC_GPR3, + .shift = 8, + .mask = IMX6Q_GPR3_LVDS1_MUX_CTL_MASK, + } +}; + +/* + * For a device declaring compatible = "fsl,imx6q-ldb", "fsl,imx53-ldb", + * of_match_device will walk through this list and take the first entry + * matching any of its compatible values. Therefore, the more generic + * entries (in this case fsl,imx53-ldb) need to be ordered last. + */ +static const struct of_device_id imx_ldb_dt_ids[] = { + { .compatible = "fsl,imx6q-ldb", .data = imx6q_lvds_mux, }, + { .compatible = "fsl,imx53-ldb", .data = NULL, }, + { } +}; +MODULE_DEVICE_TABLE(of, imx_ldb_dt_ids); + static int imx_ldb_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; + const struct of_device_id *of_id = + of_match_device(imx_ldb_dt_ids, dev); + struct device_node *np = dev->of_node; + struct imx_ldb *imx_ldb; + int dual; + int ret; + int i; + + imx_ldb = devm_kzalloc(dev, sizeof(*imx_ldb), GFP_KERNEL); + if (!imx_ldb) + return -ENOMEM; + + imx_ldb->regmap = syscon_regmap_lookup_by_phandle(np, "gpr"); + if (IS_ERR(imx_ldb->regmap)) { + dev_err(dev, "failed to get parent regmap\n"); + return PTR_ERR(imx_ldb->regmap); + } + + /* disable LDB by resetting the control register to POR default */ + regmap_write(imx_ldb->regmap, IOMUXC_GPR2, 0); + + imx_ldb->dev = dev; + + if (of_id) + imx_ldb->lvds_mux = of_id->data; + + dual = of_property_read_bool(np, "fsl,dual-channel"); + if (dual) + imx_ldb->ldb_ctrl |= LDB_SPLIT_MODE_EN; + + /* + * There are three different possible clock mux configurations: + * i.MX53: ipu1_di0_sel, ipu1_di1_sel + * i.MX6q: ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel, ipu2_di1_sel + * i.MX6dl: ipu1_di0_sel, ipu1_di1_sel, lcdif_sel + * Map them all to di0_sel...di3_sel. + */ + for (i = 0; i < 4; i++) { + char clkname[16]; + + sprintf(clkname, "di%d_sel", i); + imx_ldb->clk_sel[i] = devm_clk_get(imx_ldb->dev, clkname); + if (IS_ERR(imx_ldb->clk_sel[i])) { + ret = PTR_ERR(imx_ldb->clk_sel[i]); + imx_ldb->clk_sel[i] = NULL; + break; + } + + imx_ldb->clk_parent[i] = clk_get_parent(imx_ldb->clk_sel[i]); + } + if (i == 0) + return ret; + + dev_set_drvdata(dev, imx_ldb); + return component_add(&pdev->dev, &imx_ldb_ops); }

The race condition reported by [1] applies to the all drm-imx subcomponent drivers which implements an encoder:connector combination. The goal is to switch from the devres-kmalloc and the component framework to a 'normal' kmalloc and the drm framework to release the memory resources. So all acquired memory resoruces are freed by a drm_mode_config_cleanup() call and the ->destroy() callbacks. For this purpose we need to slit the drm_encoder and the drm_connector state.

[1] https://www.spinics.net/lists/dri-devel/msg189388.html

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/imx-ldb.c | 106 +++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 5ef9fcb9ae94..0e5a3c84df10 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -50,7 +50,6 @@ struct imx_ldb;

struct imx_ldb_channel { struct imx_ldb *ldb; - struct drm_connector connector; struct drm_encoder encoder;

/* Defines what is connected to the ldb, only one at a time */ @@ -58,19 +57,26 @@ struct imx_ldb_channel { struct drm_bridge *bridge;

struct device_node *child; - struct i2c_adapter *ddc; int chno; + u32 bus_format; + u32 bus_flags; +}; + +struct imx_ldb_connector { + struct imx_ldb_channel *ldb_channel; + struct drm_connector connector; + + struct i2c_adapter *ddc; void *edid; int edid_len; + struct drm_display_mode mode; int mode_valid; - u32 bus_format; - u32 bus_flags; };

-static inline struct imx_ldb_channel *con_to_imx_ldb_ch(struct drm_connector *c) +static inline struct imx_ldb_connector *con_to_imx_ldb_con(struct drm_connector *c) { - return container_of(c, struct imx_ldb_channel, connector); + return container_of(c, struct imx_ldb_connector, connector); }

static inline struct imx_ldb_channel *enc_to_imx_ldb_ch(struct drm_encoder *e) @@ -126,31 +132,41 @@ static void imx_ldb_ch_set_bus_format(struct imx_ldb_channel *imx_ldb_ch, } }

+static void imx_ldb_connector_destroy(struct drm_connector *connector) +{ + struct imx_ldb_connector *imx_ldb_con = con_to_imx_ldb_con(connector); + + imx_drm_connector_destroy(connector); + i2c_put_adapter(imx_ldb_con->ddc); + /* avoid dangling pointers */ + imx_ldb_con->ldb_channel = NULL; +} + static int imx_ldb_connector_get_modes(struct drm_connector *connector) { - struct imx_ldb_channel *imx_ldb_ch = con_to_imx_ldb_ch(connector); + struct imx_ldb_connector *imx_ldb_con = con_to_imx_ldb_con(connector); + struct imx_ldb_channel *imx_ldb_ch = imx_ldb_con->ldb_channel; int num_modes;

num_modes = drm_panel_get_modes(imx_ldb_ch->panel, connector); if (num_modes > 0) return num_modes;

- if (!imx_ldb_ch->edid && imx_ldb_ch->ddc) - imx_ldb_ch->edid = drm_get_edid(connector, imx_ldb_ch->ddc); + if (!imx_ldb_con->edid && imx_ldb_con->ddc) + imx_ldb_con->edid = drm_get_edid(connector, imx_ldb_con->ddc);

- if (imx_ldb_ch->edid) { - drm_connector_update_edid_property(connector, - imx_ldb_ch->edid); - num_modes = drm_add_edid_modes(connector, imx_ldb_ch->edid); + if (imx_ldb_con->edid) { + drm_connector_update_edid_property(connector, imx_ldb_con->edid); + num_modes = drm_add_edid_modes(connector, imx_ldb_con->edid); }

- if (imx_ldb_ch->mode_valid) { + if (imx_ldb_con->mode_valid) { struct drm_display_mode *mode;

mode = drm_mode_create(connector->dev); if (!mode) return -EINVAL; - drm_mode_copy(mode, &imx_ldb_ch->mode); + drm_mode_copy(mode, &imx_ldb_con->mode); mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; drm_mode_probed_add(connector, mode); num_modes++; @@ -166,7 +182,8 @@ static void imx_ldb_encoder_destroy(struct drm_encoder *encoder) if (channel->panel) drm_panel_detach(channel->panel); drm_encoder_cleanup(encoder); - i2c_put_adapter(channel->ddc); + /* avoid dangling pointers */ + channel->ldb = NULL; }

static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, @@ -388,7 +405,7 @@ static int imx_ldb_encoder_atomic_check(struct drm_encoder *encoder,

static const struct drm_connector_funcs imx_ldb_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = imx_drm_connector_destroy, + .destroy = imx_ldb_connector_destroy, .reset = drm_atomic_helper_connector_reset, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, @@ -424,11 +441,13 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) return PTR_ERR_OR_ZERO(ldb->clk_pll[chno]); }

-static int imx_ldb_register(struct drm_device *drm, - struct imx_ldb_channel *imx_ldb_ch) +static int imx_ldb_register_channel(struct drm_device *drm, + struct imx_ldb_channel *imx_ldb_ch, + struct imx_ldb_connector *imx_ldb_con) { struct imx_ldb *ldb = imx_ldb_ch->ldb; struct drm_encoder *encoder = &imx_ldb_ch->encoder; + struct drm_connector *connector = &imx_ldb_con->connector; int ret;

ret = imx_drm_encoder_parse_of(drm, encoder, imx_ldb_ch->child); @@ -450,8 +469,7 @@ static int imx_ldb_register(struct drm_device *drm, DRM_MODE_ENCODER_LVDS, NULL);

if (imx_ldb_ch->bridge) { - ret = drm_bridge_attach(&imx_ldb_ch->encoder, - imx_ldb_ch->bridge, NULL); + ret = drm_bridge_attach(encoder, imx_ldb_ch->bridge, NULL); if (ret) { DRM_ERROR("Failed to initialize bridge with drm\n"); return ret; @@ -463,18 +481,17 @@ static int imx_ldb_register(struct drm_device *drm, * historical reasons, the ldb driver can also work without * a panel. */ - drm_connector_helper_add(&imx_ldb_ch->connector, - &imx_ldb_connector_helper_funcs); - drm_connector_init_with_ddc(drm, &imx_ldb_ch->connector, + drm_connector_helper_add(connector, + &imx_ldb_connector_helper_funcs); + drm_connector_init_with_ddc(drm, connector, &imx_ldb_connector_funcs, DRM_MODE_CONNECTOR_LVDS, - imx_ldb_ch->ddc); - drm_connector_attach_encoder(&imx_ldb_ch->connector, encoder); + imx_ldb_con->ddc); + drm_connector_attach_encoder(connector, encoder); }

if (imx_ldb_ch->panel) { - ret = drm_panel_attach(imx_ldb_ch->panel, - &imx_ldb_ch->connector); + ret = drm_panel_attach(imx_ldb_ch->panel, connector); if (ret) return ret; } @@ -518,7 +535,9 @@ static u32 of_get_bus_format(struct device *dev, struct device_node *np) }

static int imx_ldb_panel_ddc(struct device *dev, - struct imx_ldb_channel *channel, struct device_node *child) + struct imx_ldb_channel *channel, + struct imx_ldb_connector *connector, + struct device_node *child) { struct device_node *ddc_node; const u8 *edidp; @@ -526,32 +545,32 @@ static int imx_ldb_panel_ddc(struct device *dev,

ddc_node = of_parse_phandle(child, "ddc-i2c-bus", 0); if (ddc_node) { - channel->ddc = of_find_i2c_adapter_by_node(ddc_node); + connector->ddc = of_find_i2c_adapter_by_node(ddc_node); of_node_put(ddc_node); - if (!channel->ddc) { + if (!connector->ddc) { dev_warn(dev, "failed to get ddc i2c adapter\n"); return -EPROBE_DEFER; } }

- if (!channel->ddc) { + if (!connector->ddc) { /* if no DDC available, fallback to hardcoded EDID */ dev_dbg(dev, "no ddc available\n");

edidp = of_get_property(child, "edid", - &channel->edid_len); + &connector->edid_len); if (edidp) { - channel->edid = devm_kmemdup(dev, edidp, - channel->edid_len, - GFP_KERNEL); + connector->edid = devm_kmemdup(dev, edidp, + connector->edid_len, + GFP_KERNEL); } else if (!channel->panel) { /* fallback to display-timings node */ ret = of_get_drm_display_mode(child, - &channel->mode, + &connector->mode, &channel->bus_flags, OF_USE_NATIVE_MODE); if (!ret) - channel->mode_valid = 1; + connector->mode_valid = 1; } } return 0; @@ -569,6 +588,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)

for_each_child_of_node(np, child) { struct imx_ldb_channel *channel; + struct imx_ldb_connector *connector; int bus_format;

ret = of_property_read_u32(child, "reg", &i); @@ -604,9 +624,15 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data)

/* panel ddc only if there is no bridge */ if (!channel->bridge) { - ret = imx_ldb_panel_ddc(dev, channel, child); + connector = devm_kzalloc(dev, sizeof(*connector), GFP_KERNEL); + if (!connector) + return -ENOMEM; + + ret = imx_ldb_panel_ddc(dev, channel, connector, child); if (ret) goto free_child; + + connector->ldb_channel = channel; }

bus_format = of_get_bus_format(dev, child); @@ -628,7 +654,7 @@ static int imx_ldb_bind(struct device *dev, struct device *master, void *data) channel->bus_format = bus_format; channel->child = child;

- ret = imx_ldb_register(drm, channel); + ret = imx_ldb_register_channel(drm, channel, connector); if (ret) { channel->child = NULL; goto free_child;

Currently there is a race conditions if the panel can't be probed e.g. it is not connected [1]. There where several attempts to fix this [2,3] but non of them made it into mainline.

The problem is the combination of the component framework and the drm framework, as Philipp already explained [1]. To fix this we need to drop the devres-kmalloc and move the plain kmalloc to let the drm framework free the resources upon a drm_mode_config_cleanup(). So we need to implement a .destroy() callback for each component. We also need to reorder the master.unbind() callback to ensure that the driver states are accessible during a component.unbind() call. This reordering also aligns the master.unbind() with the error-cleanup path during master.bind().

[1] https://www.spinics.net/lists/dri-devel/msg189388.html [2] https://lkml.org/lkml/2018/10/16/1148 [3] https://lkml.org/lkml/2019/4/2/612

Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- drivers/gpu/drm/imx/dw_hdmi-imx.c | 28 ++++++++++++--------- drivers/gpu/drm/imx/imx-drm-core.c | 3 ++- drivers/gpu/drm/imx/imx-ldb.c | 34 +++++++++++++++++--------- drivers/gpu/drm/imx/imx-tve.c | 15 +++++++++--- drivers/gpu/drm/imx/ipuv3-crtc.c | 28 ++++++++++++++++++--- drivers/gpu/drm/imx/parallel-display.c | 30 ++++++++++++++++------- 6 files changed, 96 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c index f22cfbf9353e..86a62796c151 100644 --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c @@ -137,6 +137,12 @@ static int dw_hdmi_imx_atomic_check(struct drm_encoder *encoder, return 0; }

+static void dw_hdmi_imx_encoder_destroy(struct drm_encoder *encoder) +{ + drm_encoder_cleanup(encoder); + kfree(enc_to_imx_hdmi(encoder)); +} + static const struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = { .enable = dw_hdmi_imx_encoder_enable, .disable = dw_hdmi_imx_encoder_disable, @@ -144,7 +150,7 @@ static const struct drm_encoder_helper_funcs dw_hdmi_imx_encoder_helper_funcs = };

static const struct drm_encoder_funcs dw_hdmi_imx_encoder_funcs = { - .destroy = drm_encoder_cleanup, + .destroy = dw_hdmi_imx_encoder_destroy, };

static enum drm_mode_status @@ -212,7 +218,7 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master, if (!pdev->dev.of_node) return -ENODEV;

- hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); + hdmi = kzalloc(sizeof(*hdmi), GFP_KERNEL); if (!hdmi) return -ENOMEM;

@@ -228,12 +234,16 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master, * not been registered yet. Defer probing, and hope that * the required CRTC is added later. */ - if (encoder->possible_crtcs == 0) + if (encoder->possible_crtcs == 0) { + kfree(hdmi); return -EPROBE_DEFER; + }

ret = dw_hdmi_imx_parse_dt(hdmi); - if (ret < 0) + if (ret < 0) { + kfree(hdmi); return ret; + }

drm_encoder_helper_add(encoder, &dw_hdmi_imx_encoder_helper_funcs); drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs, @@ -242,15 +252,9 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master, platform_set_drvdata(pdev, hdmi);

hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data); - - /* - * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(), - * which would have called the encoder cleanup. Do it manually. - */ - if (IS_ERR(hdmi->hdmi)) { + /* Don't call kfree() here, this is done by the .destroy() handler. */ + if (IS_ERR(hdmi->hdmi)) ret = PTR_ERR(hdmi->hdmi); - drm_encoder_cleanup(encoder); - }

return ret; } diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 9979547ca883..feab6eb9e7e5 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -275,9 +275,10 @@ static void imx_drm_unbind(struct device *dev)

drm_kms_helper_poll_fini(drm);

+ component_unbind_all(drm->dev, drm); + drm_mode_config_cleanup(drm);

- component_unbind_all(drm->dev, drm); dev_set_drvdata(dev, NULL);

drm_dev_put(drm); diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index 5e6c1b09dbfa..4a5d31da592a 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -140,6 +140,8 @@ static void imx_ldb_connector_destroy(struct drm_connector *connector) i2c_put_adapter(imx_ldb_con->ddc); /* avoid dangling pointers */ imx_ldb_con->ldb_channel = NULL; + kfree(imx_ldb_con->edid); + kfree(imx_ldb_con); }

static int imx_ldb_connector_get_modes(struct drm_connector *connector) @@ -184,6 +186,7 @@ static void imx_ldb_encoder_destroy(struct drm_encoder *encoder) drm_encoder_cleanup(encoder); /* avoid dangling pointers */ channel->ldb = NULL; + kfree(channel); }

static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno, @@ -502,9 +505,8 @@ static int imx_ldb_panel_ddc(struct device *dev, edidp = of_get_property(child, "edid", &connector->edid_len); if (edidp) { - connector->edid = devm_kmemdup(dev, edidp, - connector->edid_len, - GFP_KERNEL); + connector->edid = kmemdup(edidp, connector->edid_len, + GFP_KERNEL); } else if (!channel->panel) { /* fallback to display-timings node */ ret = of_get_drm_display_mode(child, @@ -525,7 +527,7 @@ static int imx_ldb_setup_channel(struct device *dev, int channel_number) { struct imx_ldb_channel *channel; - struct imx_ldb_connector *imx_ldb_con; + struct imx_ldb_connector *imx_ldb_con = NULL; struct drm_encoder *encoder; struct drm_connector *connector = NULL; int bus_format; @@ -537,7 +539,7 @@ static int imx_ldb_setup_channel(struct device *dev, * 3) Register it with the DRM framework * 4) Attach bridge or connector to encoder */ - channel = devm_kzalloc(dev, sizeof(*channel), GFP_KERNEL); + channel = kzalloc(sizeof(*channel), GFP_KERNEL); if (!channel) return -ENOMEM;

@@ -554,17 +556,19 @@ static int imx_ldb_setup_channel(struct device *dev, ldb->lvds_mux ? 4 : 2, 0, &channel->panel, &channel->bridge); if (ret && ret != -ENODEV) - return ret; + goto err_free;

/* panel ddc only if there is no bridge */ if (!channel->bridge) { - imx_ldb_con = devm_kzalloc(dev, sizeof(*connector), GFP_KERNEL); - if (!imx_ldb_con) - return -ENOMEM; + imx_ldb_con = kzalloc(sizeof(*connector), GFP_KERNEL); + if (!imx_ldb_con) { + ret = -ENOMEM; + goto err_free; + }

ret = imx_ldb_panel_ddc(dev, channel, imx_ldb_con, child); if (ret) - return ret; + goto err_free;

imx_ldb_con->ldb_channel = channel; connector = &imx_ldb_con->connector; @@ -628,7 +632,7 @@ static int imx_ldb_setup_channel(struct device *dev, ret = drm_bridge_attach(encoder, channel->bridge, NULL); if (ret) { DRM_ERROR("Failed to initialize bridge with drm\n"); - goto err_put_ddc; + goto err_out; } } else { drm_connector_attach_encoder(connector, encoder); @@ -637,7 +641,7 @@ static int imx_ldb_setup_channel(struct device *dev, if (channel->panel) { ret = drm_panel_attach(channel->panel, connector); if (ret) - goto err_put_ddc; + goto err_out; }

return 0; @@ -645,6 +649,12 @@ static int imx_ldb_setup_channel(struct device *dev, err_put_ddc: if (imx_ldb_con) i2c_put_adapter(imx_ldb_con->ddc); +err_free: + if (imx_ldb_con) + kfree(imx_ldb_con->edid); + kfree(imx_ldb_con); + kfree(channel); +err_out: return ret; }

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index a7a05c47f68b..15ff5f35ff0e 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -241,6 +241,7 @@ static void imx_tve_connector_destroy(struct drm_connector *connector) i2c_put_adapter(tvec->ddc); /* avoid dangling pointers */ tvec->tve = NULL; + kfree(tvec); }

static int imx_tve_connector_get_modes(struct drm_connector *connector) @@ -292,6 +293,7 @@ static void imx_tve_encoder_destroy(struct drm_encoder *encoder) drm_encoder_cleanup(encoder); /* avoid dangling pointers */ tvee->tve = NULL; + kfree(tvee); }

static void imx_tve_encoder_mode_set(struct drm_encoder *encoder, @@ -577,13 +579,15 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) struct imx_tve_connector *tvec; int ret;

- tvee = devm_kzalloc(dev, sizeof(*tvee), GFP_KERNEL); + tvee = kzalloc(sizeof(*tvee), GFP_KERNEL); if (!tvee) return -ENOMEM;

- tvec = devm_kzalloc(dev, sizeof(*tvec), GFP_KERNEL); - if (!tvec) - return -ENOMEM; + tvec = kzalloc(sizeof(*tvec), GFP_KERNEL); + if (!tvec) { + ret = -ENOMEM; + goto err_free; + }

tvee->tve = imx_tve; tvec->tve = imx_tve; @@ -602,6 +606,9 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)

err_put_ddc: i2c_put_adapter(tvec->ddc); +err_free: + kfree(tvec); + kfree(tvee); return ret; }

diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c b/drivers/gpu/drm/imx/ipuv3-crtc.c index 63c0284f8b3c..2d24677f7fef 100644 --- a/drivers/gpu/drm/imx/ipuv3-crtc.c +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c @@ -105,6 +105,12 @@ static void ipu_crtc_atomic_disable(struct drm_crtc *crtc, spin_unlock_irq(&crtc->dev->event_lock); }

+static void imx_drm_crtc_destroy(struct drm_crtc *crtc) +{ + drm_crtc_cleanup(crtc); + kfree(to_ipu_crtc(crtc)); +} + static void imx_drm_crtc_reset(struct drm_crtc *crtc) { struct imx_crtc_state *state; @@ -166,7 +172,7 @@ static void ipu_disable_vblank(struct drm_crtc *crtc)

static const struct drm_crtc_funcs ipu_crtc_funcs = { .set_config = drm_atomic_helper_set_config, - .destroy = drm_crtc_cleanup, + .destroy = imx_drm_crtc_destroy, .page_flip = drm_atomic_helper_page_flip, .reset = imx_drm_crtc_reset, .atomic_duplicate_state = imx_drm_crtc_duplicate_state, @@ -357,7 +363,8 @@ static int ipu_get_resources(struct ipu_crtc *ipu_crtc, }

static int ipu_crtc_init(struct ipu_crtc *ipu_crtc, - struct ipu_client_platformdata *pdata, struct drm_device *drm) + struct ipu_client_platformdata *pdata, + struct drm_device *drm) { struct ipu_soc *ipu = dev_get_drvdata(ipu_crtc->dev->parent); struct drm_crtc *crtc = &ipu_crtc->base; @@ -437,10 +444,11 @@ static int ipu_drm_bind(struct device *dev, struct device *master, void *data) { struct ipu_client_platformdata *pdata = dev->platform_data; struct drm_device *drm = data; + struct drm_crtc *registered_crtc = NULL; struct ipu_crtc *ipu_crtc; int ret;

- ipu_crtc = devm_kzalloc(dev, sizeof(*ipu_crtc), GFP_KERNEL); + ipu_crtc = kzalloc(sizeof(*ipu_crtc), GFP_KERNEL); if (!ipu_crtc) return -ENOMEM;

@@ -448,11 +456,23 @@ static int ipu_drm_bind(struct device *dev, struct device *master, void *data)

ret = ipu_crtc_init(ipu_crtc, pdata, drm); if (ret) - return ret; + goto err;

dev_set_drvdata(dev, ipu_crtc);

return 0; + +err: + drm_for_each_crtc(registered_crtc, drm) { + /* + * The crtc was registered with the drm core framework if we + * enter here. So let the core .destroy() helper cleanup the + * code. + */ + return ret; + } + kfree(ipu_crtc); + return ret; }

static void ipu_drm_unbind(struct device *dev, struct device *master, diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 78703b15c7cf..3e247383a498 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -55,6 +55,8 @@ static void imx_pd_connector_destroy(struct drm_connector *connector) imx_drm_connector_destroy(connector); /* avoid dangling pointer */ imxpc->imxpd = NULL; + kfree(imxpc->edid); + kfree(imxpc); }

static int imx_pd_connector_get_modes(struct drm_connector *connector) @@ -102,6 +104,7 @@ static void imx_pd_encoder_destroy(struct drm_encoder *encoder) if (imxpd->panel) drm_panel_detach(imxpd->panel); drm_encoder_cleanup(encoder); + kfree(imxpd); }

static void imx_pd_encoder_enable(struct drm_encoder *encoder) @@ -225,31 +228,32 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) struct device_node *np = dev->of_node; const u8 *edidp; struct imx_parallel_display *imxpd; - struct imx_parallel_connector *imxpc; + struct imx_parallel_connector *imxpc = NULL; int ret; u32 bus_format = 0; const char *fmt;

- imxpd = devm_kzalloc(dev, sizeof(*imxpd), GFP_KERNEL); + imxpd = kzalloc(sizeof(*imxpd), GFP_KERNEL); if (!imxpd) return -ENOMEM;

/* port@1 is the output port */ ret = drm_of_find_panel_or_bridge(np, 1, 0, &imxpd->panel, &imxpd->bridge); if (ret && ret != -ENODEV) - return ret; + goto err_free;

if (!imxpd->bridge) { - imxpc = devm_kzalloc(dev, sizeof(*imxpc), GFP_KERNEL); - if (!imxpc) - return -ENOMEM; + imxpc = kzalloc(sizeof(*imxpc), GFP_KERNEL); + if (!imxpc) { + ret = -ENOMEM; + goto err_free; + }

imxpc->imxpd = imxpd;

edidp = of_get_property(np, "edid", &imxpc->edid_len); if (edidp) - imxpc->edid = devm_kmemdup(dev, edidp, imxpc->edid_len, - GFP_KERNEL); + imxpc->edid = kmemdup(edidp, imxpc->edid_len, GFP_KERNEL); }

ret = of_property_read_string(np, "interface-pix-fmt", &fmt); @@ -269,9 +273,17 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)

ret = imx_pd_register(drm, imxpd, imxpc); if (ret) - return ret; + goto err_free;

return imx_pd_attach(drm, imxpd, imxpc); + +err_free: + if (imxpc) + kfree(imxpc->edid); + kfree(imxpc); + kfree(imxpd); + + return ret; }

static const struct component_ops imx_pd_ops = {