Re: [PATCH v3 5/7] drm/imx: use drmm_simple_encoder_alloc()

On Fri, Sep 11, 2020 at 03:57:22PM +0200, Philipp Zabel wrote:

This allows to drop the custom drm_encoder_cleanup() actions.

Signed-off-by: Philipp Zabel p.zabel@pengutronix.de

New in v3, example conversion of drm_simple_encoder_init() users.

This and the following patches depend on the drm/imx conversion to use managed resources [1].

[1] https://lore.kernel.org/dri-devel/20200911133855.29801-3-p.zabel@pengutronix...

drivers/gpu/drm/imx/dw_hdmi-imx.c | 19 ++++--------------- drivers/gpu/drm/imx/imx-ldb.c | 20 ++++---------------- drivers/gpu/drm/imx/imx-tve.c | 22 ++++------------------ drivers/gpu/drm/imx/parallel-display.c | 22 ++++------------------ 4 files changed, 16 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c index 16be8bd92653..87428fb23d9f 100644 --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c @@ -188,13 +188,6 @@ static const struct of_device_id dw_hdmi_imx_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, dw_hdmi_imx_dt_ids);

-static void dw_hdmi_imx_encoder_cleanup(struct drm_device *drm, void *data) -{

• struct drm_encoder *encoder = data;
• drm_encoder_cleanup(encoder);

-}

static int dw_hdmi_imx_bind(struct device *dev, struct device *master, void *data) { @@ -203,9 +196,10 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master, struct drm_encoder *encoder; int ret;

• hdmi_encoder = drmm_kzalloc(drm, sizeof(*hdmi_encoder), GFP_KERNEL);

Where does this apply to? Neither upstream nor linux-next seems to have the drmm_ conversion for imx already applied, and that's kinda the juicy part I'd like to look at a bit. The patches here are just mechanical conversion. Can you pls include the drmm_ conversion too (maybe even squash these patches here in, I think that would be more readable)?

Or am I looking at the wrong tree?

Anyway I think it looks all neat. -Daniel

• if (!hdmi_encoder)

• return -ENOMEM;
  

• hdmi_encoder = drmm_simple_encoder_alloc(drm, struct imx_hdmi_encoder,

• 				 encoder, DRM_MODE_ENCODER_TMDS);
  

• if (IS_ERR(hdmi_encoder))

• return PTR_ERR(hdmi_encoder);
  
  

  hdmi_encoder->hdmi = dev_get_drvdata(dev); encoder = &hdmi_encoder->encoder;

@@ -215,11 +209,6 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master, return ret;

drm_encoder_helper_add(encoder, &dw_hdmi_imx_encoder_helper_funcs);

• drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);

• ret = drmm_add_action_or_reset(drm, dw_hdmi_imx_encoder_cleanup, encoder);

• if (ret)

• return ret;
  
  

  return drm_bridge_attach(encoder, hdmi_encoder->hdmi->bridge, NULL, 0);

} diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c index d4beb58f509d..dbfe39e2f7f6 100644 --- a/drivers/gpu/drm/imx/imx-ldb.c +++ b/drivers/gpu/drm/imx/imx-ldb.c @@ -414,13 +414,6 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno) return PTR_ERR_OR_ZERO(ldb->clk_pll[chno]); }

-static void imx_ldb_encoder_cleanup(struct drm_device *drm, void *data) -{

• struct drm_encoder *encoder = data;
• drm_encoder_cleanup(encoder);

-}

static int imx_ldb_register(struct drm_device *drm, struct imx_ldb_channel *imx_ldb_ch) { @@ -430,20 +423,15 @@ static int imx_ldb_register(struct drm_device *drm, struct drm_encoder *encoder; int ret;

• ldb_encoder = drmm_kzalloc(drm, sizeof(*ldb_encoder), GFP_KERNEL);
• if (!ldb_encoder)

• return -ENOMEM;
  

• ldb_encoder = drmm_simple_encoder_alloc(drm, struct imx_ldb_encoder,

• 				encoder, DRM_MODE_ENCODER_LVDS);
  

• if (IS_ERR(ldb_encoder))

• return PTR_ERR(ldb_encoder);
  
  

  ldb_encoder->channel = imx_ldb_ch; connector = &ldb_encoder->connector; encoder = &ldb_encoder->encoder;

• drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_LVDS);
• ret = drmm_add_action_or_reset(drm, imx_ldb_encoder_cleanup, encoder);
• if (ret)

• return ret;
  

• ret = imx_drm_encoder_parse_of(drm, encoder, imx_ldb_ch->child); if (ret) return ret;

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c index bac025eafa1f..0746f0b425df 100644 --- a/drivers/gpu/drm/imx/imx-tve.c +++ b/drivers/gpu/drm/imx/imx-tve.c @@ -433,13 +433,6 @@ static int tve_clk_init(struct imx_tve *tve, void __iomem *base) return 0; }

-static void imx_tve_encoder_cleanup(struct drm_device *drm, void *ptr) -{

• struct drm_encoder *encoder = ptr;
• drm_encoder_cleanup(encoder);

-}

static void imx_tve_disable_regulator(void *data) { struct imx_tve *tve = data; @@ -498,22 +491,15 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data) encoder_type = tve->mode == TVE_MODE_VGA ? DRM_MODE_ENCODER_DAC : DRM_MODE_ENCODER_TVDAC;

• tvee = drmm_kzalloc(drm, sizeof(*tvee), GFP_KERNEL);
• if (!tvee)

• return -ENOMEM;
  

• tvee = drmm_simple_encoder_alloc(drm, struct imx_tve_encoder, encoder,

• 			 encoder_type);
  

• if (IS_ERR(tvee))

• return PTR_ERR(tvee);
  
  

  tvee->tve = tve; encoder = &tvee->encoder; connector = &tvee->connector;

• ret = drm_simple_encoder_init(drm, encoder, encoder_type);
• if (ret)

• return ret;
  

• ret = drmm_add_action_or_reset(drm, imx_tve_encoder_cleanup, encoder);
• if (ret)

• return ret;
  

• ret = imx_drm_encoder_parse_of(drm, encoder, tve->dev->of_node); if (ret) return ret;

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index 50b5b89c2db2..9b1ec7e73c30 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -258,13 +258,6 @@ static const struct drm_bridge_funcs imx_pd_bridge_funcs = { .atomic_get_output_bus_fmts = imx_pd_bridge_atomic_get_output_bus_fmts, };

-static void imx_pd_encoder_cleanup(struct drm_device *drm, void *ptr) -{

• struct drm_encoder *encoder = ptr;
• drm_encoder_cleanup(encoder);

-}

static int imx_pd_bind(struct device *dev, struct device *master, void *data) { struct drm_device *drm = data; @@ -275,23 +268,16 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data) struct drm_bridge *bridge; int ret;

• imxpd_encoder = drmm_kzalloc(drm, sizeof(*imxpd_encoder), GFP_KERNEL);
• if (!imxpd_encoder)

• return -ENOMEM;
  

• imxpd_encoder = drmm_simple_encoder_alloc(drm, struct imx_parallel_display_encoder,

• 				  encoder, DRM_MODE_ENCODER_NONE);
  

• if (IS_ERR(imxpd_encoder))

• return PTR_ERR(imxpd_encoder);
  
  

  imxpd_encoder->pd = imxpd; connector = &imxpd_encoder->connector; encoder = &imxpd_encoder->encoder; bridge = &imxpd_encoder->bridge;

• ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_NONE);
• if (ret)

• return ret;
  

• ret = drmm_add_action_or_reset(drm, imx_pd_encoder_cleanup, encoder);
• if (ret)

• return ret;
  

• ret = imx_drm_encoder_parse_of(drm, encoder, imxpd->dev->of_node); if (ret) return ret;

-- 2.20.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel