[PATCH 7/7] drm/sun4i: switch DE33 to new bindings

Jernej Skrabec posted 7 patches 2 months, 3 weeks ago
[PATCH 7/7] drm/sun4i: switch DE33 to new bindings
Posted by Jernej Skrabec 2 months, 3 weeks ago
Now that everything is in place, switch DE33 to new bindings.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 130 +++++++++++++++-------------
 drivers/gpu/drm/sun4i/sun8i_mixer.h |  10 +--
 2 files changed, 71 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index fde3b677e925..da213e54e653 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -13,6 +13,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 
@@ -24,6 +25,7 @@
 #include <drm/drm_probe_helper.h>
 
 #include "sun4i_drv.h"
+#include "sun50i_planes.h"
 #include "sun8i_mixer.h"
 #include "sun8i_ui_layer.h"
 #include "sun8i_vi_layer.h"
@@ -256,7 +258,6 @@ static void sun8i_mixer_commit(struct sunxi_engine *engine,
 {
 	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
 	u32 bld_base = sun8i_blender_base(mixer);
-	struct regmap *bld_regs = sun8i_blender_regmap(mixer);
 	struct drm_plane_state *plane_state;
 	struct drm_plane *plane;
 	u32 route = 0, pipe_en = 0;
@@ -293,16 +294,16 @@ static void sun8i_mixer_commit(struct sunxi_engine *engine,
 		route |= layer->index << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos);
 		pipe_en |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
-		regmap_write(bld_regs,
+		regmap_write(engine->regs,
 			     SUN8I_MIXER_BLEND_ATTR_COORD(bld_base, zpos),
 			     SUN8I_MIXER_COORD(x, y));
-		regmap_write(bld_regs,
+		regmap_write(engine->regs,
 			     SUN8I_MIXER_BLEND_ATTR_INSIZE(bld_base, zpos),
 			     SUN8I_MIXER_SIZE(w, h));
 	}
 
-	regmap_write(bld_regs, SUN8I_MIXER_BLEND_ROUTE(bld_base), route);
-	regmap_write(bld_regs, SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
+	regmap_write(engine->regs, SUN8I_MIXER_BLEND_ROUTE(bld_base), route);
+	regmap_write(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
 		     pipe_en | SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0));
 
 	if (mixer->cfg->de_type != SUN8I_MIXER_DE33)
@@ -317,7 +318,6 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
 	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
 	int plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
 	enum drm_plane_type type;
-	unsigned int phy_index;
 	int i;
 
 	planes = devm_kcalloc(drm->dev, plane_cnt, sizeof(*planes), GFP_KERNEL);
@@ -332,12 +332,8 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
 		else
 			type = DRM_PLANE_TYPE_OVERLAY;
 
-		phy_index = i;
-		if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
-			phy_index = mixer->cfg->map[i];
-
 		layer = sun8i_vi_layer_init_one(drm, type, mixer->engine.regs,
-						i, phy_index, plane_cnt,
+						i, i, plane_cnt,
 						&mixer->cfg->lay_cfg);
 		if (IS_ERR(layer)) {
 			dev_err(drm->dev,
@@ -357,12 +353,8 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
 		else
 			type = DRM_PLANE_TYPE_OVERLAY;
 
-		phy_index = index;
-		if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
-			phy_index = mixer->cfg->map[index];
-
 		layer = sun8i_ui_layer_init_one(drm, type, mixer->engine.regs,
-						index, phy_index, plane_cnt,
+						index, index, plane_cnt,
 						&mixer->cfg->lay_cfg);
 		if (IS_ERR(layer)) {
 			dev_err(drm->dev, "Couldn't initialize %s plane\n",
@@ -376,16 +368,25 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
 	return planes;
 }
 
+static struct drm_plane **sun50i_layers_init(struct drm_device *drm,
+					     struct sunxi_engine *engine)
+{
+	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
+
+	if (IS_ENABLED(CONFIG_DRM_SUN50I_PLANES))
+		return sun50i_planes_setup(mixer->planes_dev, drm, engine->id);
+
+	return NULL;
+}
+
 static void sun8i_mixer_mode_set(struct sunxi_engine *engine,
 				 const struct drm_display_mode *mode)
 {
 	struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
-	struct regmap *bld_regs;
 	u32 bld_base, size, val;
 	bool interlaced;
 
 	bld_base = sun8i_blender_base(mixer);
-	bld_regs = sun8i_blender_regmap(mixer);
 	interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
 	size = SUN8I_MIXER_SIZE(mode->hdisplay, mode->vdisplay);
 
@@ -397,14 +398,14 @@ static void sun8i_mixer_mode_set(struct sunxi_engine *engine,
 	else
 		regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
 
-	regmap_write(bld_regs, SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
+	regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
 
 	if (interlaced)
 		val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
 	else
 		val = 0;
 
-	regmap_update_bits(bld_regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
+	regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
 			   SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val);
 
 	DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
@@ -417,8 +418,14 @@ static const struct sunxi_engine_ops sun8i_engine_ops = {
 	.mode_set	= sun8i_mixer_mode_set,
 };
 
+static const struct sunxi_engine_ops sun50i_engine_ops = {
+	.commit		= sun8i_mixer_commit,
+	.layers_init	= sun50i_layers_init,
+	.mode_set	= sun8i_mixer_mode_set,
+};
+
 static const struct regmap_config sun8i_mixer_regmap_config = {
-	.name		= "layers",
+	.name		= "display",
 	.reg_bits	= 32,
 	.val_bits	= 32,
 	.reg_stride	= 4,
@@ -433,14 +440,6 @@ static const struct regmap_config sun8i_top_regmap_config = {
 	.max_register	= 0x3c,
 };
 
-static const struct regmap_config sun8i_disp_regmap_config = {
-	.name		= "display",
-	.reg_bits	= 32,
-	.val_bits	= 32,
-	.reg_stride	= 4,
-	.max_register	= 0x20000,
-};
-
 static int sun8i_mixer_of_get_id(struct device_node *node)
 {
 	struct device_node *ep, *remote;
@@ -463,17 +462,14 @@ static int sun8i_mixer_of_get_id(struct device_node *node)
 
 static void sun8i_mixer_init(struct sun8i_mixer *mixer)
 {
-	struct regmap *top_regs, *disp_regs;
 	unsigned int base = sun8i_blender_base(mixer);
+	struct regmap *top_regs;
 	int plane_cnt, i;
 
-	if (mixer->cfg->de_type == SUN8I_MIXER_DE33) {
+	if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
 		top_regs = mixer->top_regs;
-		disp_regs = mixer->disp_regs;
-	} else {
+	else
 		top_regs = mixer->engine.regs;
-		disp_regs = mixer->engine.regs;
-	}
 
 	/* Enable the mixer */
 	regmap_write(top_regs, SUN8I_MIXER_GLOBAL_CTL,
@@ -483,25 +479,25 @@ static void sun8i_mixer_init(struct sun8i_mixer *mixer)
 		regmap_write(top_regs, SUN50I_MIXER_GLOBAL_CLK, 1);
 
 	/* Set background color to black */
-	regmap_write(disp_regs, SUN8I_MIXER_BLEND_BKCOLOR(base),
+	regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base),
 		     SUN8I_MIXER_BLEND_COLOR_BLACK);
 
 	/*
 	 * Set fill color of bottom plane to black. Generally not needed
 	 * except when VI plane is at bottom (zpos = 0) and enabled.
 	 */
-	regmap_write(disp_regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
+	regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
 		     SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0));
-	regmap_write(disp_regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0),
+	regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0),
 		     SUN8I_MIXER_BLEND_COLOR_BLACK);
 
 	plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
 	for (i = 0; i < plane_cnt; i++)
-		regmap_write(disp_regs,
+		regmap_write(mixer->engine.regs,
 			     SUN8I_MIXER_BLEND_MODE(base, i),
 			     SUN8I_MIXER_BLEND_MODE_DEF);
 
-	regmap_update_bits(disp_regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
+	regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
 			   SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
 }
 
@@ -532,7 +528,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 	if (!mixer)
 		return -ENOMEM;
 	dev_set_drvdata(dev, mixer);
-	mixer->engine.ops = &sun8i_engine_ops;
 	mixer->engine.node = dev->of_node;
 
 	if (of_property_present(dev->of_node, "iommus")) {
@@ -562,6 +557,11 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 	if (!mixer->cfg)
 		return -EINVAL;
 
+	if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
+		mixer->engine.ops = &sun50i_engine_ops;
+	else
+		mixer->engine.ops = &sun8i_engine_ops;
+
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
@@ -584,17 +584,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 			dev_err(dev, "Couldn't create the top regmap\n");
 			return PTR_ERR(mixer->top_regs);
 		}
-
-		regs = devm_platform_ioremap_resource_byname(pdev, "display");
-		if (IS_ERR(regs))
-			return PTR_ERR(regs);
-
-		mixer->disp_regs = devm_regmap_init_mmio(dev, regs,
-							 &sun8i_disp_regmap_config);
-		if (IS_ERR(mixer->disp_regs)) {
-			dev_err(dev, "Couldn't create the disp regmap\n");
-			return PTR_ERR(mixer->disp_regs);
-		}
 	}
 
 	mixer->reset = devm_reset_control_get(dev, NULL);
@@ -634,6 +623,33 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 
 	clk_prepare_enable(mixer->mod_clk);
 
+	if (mixer->cfg->de_type == SUN8I_MIXER_DE33) {
+		struct platform_device *pdev;
+		struct device_node *np;
+		void *data;
+
+		np = of_parse_phandle(dev->of_node, "allwinner,planes", 0);
+		if (!np) {
+			ret = -ENODEV;
+			goto err_disable_mod_clk;
+		}
+
+		pdev = of_find_device_by_node(np);
+		of_node_put(np);
+		if (!pdev) {
+			ret = -EPROBE_DEFER;
+			goto err_disable_mod_clk;
+		}
+
+		data = platform_get_drvdata(pdev);
+		if (!data) {
+			put_device(&pdev->dev);
+			return -EPROBE_DEFER;
+		}
+
+		mixer->planes_dev = &pdev->dev;
+	}
+
 	list_add_tail(&mixer->engine.list, &drv->engine_list);
 
 	/* Reset registers and disable unused sub-engines */
@@ -668,6 +684,8 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
 
 	return 0;
 
+err_disable_mod_clk:
+	clk_disable_unprepare(mixer->mod_clk);
 err_disable_bus_clk:
 	clk_disable_unprepare(mixer->bus_clk);
 err_assert_reset:
@@ -863,16 +881,8 @@ static const struct sun8i_mixer_cfg sun50i_h6_mixer0_cfg = {
 };
 
 static const struct sun8i_mixer_cfg sun50i_h616_mixer0_cfg = {
-	.lay_cfg = {
-		.de_type	= SUN8I_MIXER_DE33,
-		.scaler_mask	= 0xf,
-		.scanline_yuv	= 4096,
-	},
 	.de_type	= SUN8I_MIXER_DE33,
 	.mod_rate	= 600000000,
-	.ui_num		= 3,
-	.vi_num		= 1,
-	.map		= {0, 6, 7, 8},
 };
 
 static const struct of_device_id sun8i_mixer_of_table[] = {
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
index e2f83301aae8..7abc88c898d9 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
@@ -202,7 +202,6 @@ struct sun8i_mixer_cfg {
 	int			ui_num;
 	unsigned int		de_type;
 	unsigned long		mod_rate;
-	unsigned int		map[6];
 };
 
 struct sun8i_mixer {
@@ -216,7 +215,7 @@ struct sun8i_mixer {
 	struct clk			*mod_clk;
 
 	struct regmap			*top_regs;
-	struct regmap			*disp_regs;
+	struct device			*planes_dev;
 };
 
 enum {
@@ -252,13 +251,6 @@ sun8i_blender_base(struct sun8i_mixer *mixer)
 	return mixer->cfg->de_type == SUN8I_MIXER_DE3 ? DE3_BLD_BASE : DE2_BLD_BASE;
 }
 
-static inline struct regmap *
-sun8i_blender_regmap(struct sun8i_mixer *mixer)
-{
-	return mixer->cfg->de_type == SUN8I_MIXER_DE33 ?
-		mixer->disp_regs : mixer->engine.regs;
-}
-
 static inline u32
 sun8i_channel_base(struct sun8i_layer *layer)
 {
-- 
2.51.2
Re: [PATCH 7/7] drm/sun4i: switch DE33 to new bindings
Posted by Chen-Yu Tsai 1 month, 2 weeks ago
On Sat, Nov 15, 2025 at 10:14 PM Jernej Skrabec
<jernej.skrabec@gmail.com> wrote:
>
> Now that everything is in place, switch DE33 to new bindings.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 130 +++++++++++++++-------------
>  drivers/gpu/drm/sun4i/sun8i_mixer.h |  10 +--
>  2 files changed, 71 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index fde3b677e925..da213e54e653 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -13,6 +13,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>
> @@ -24,6 +25,7 @@
>  #include <drm/drm_probe_helper.h>
>
>  #include "sun4i_drv.h"
> +#include "sun50i_planes.h"
>  #include "sun8i_mixer.h"
>  #include "sun8i_ui_layer.h"
>  #include "sun8i_vi_layer.h"
> @@ -256,7 +258,6 @@ static void sun8i_mixer_commit(struct sunxi_engine *engine,
>  {
>         struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
>         u32 bld_base = sun8i_blender_base(mixer);
> -       struct regmap *bld_regs = sun8i_blender_regmap(mixer);
>         struct drm_plane_state *plane_state;
>         struct drm_plane *plane;
>         u32 route = 0, pipe_en = 0;
> @@ -293,16 +294,16 @@ static void sun8i_mixer_commit(struct sunxi_engine *engine,
>                 route |= layer->index << SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(zpos);
>                 pipe_en |= SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
>
> -               regmap_write(bld_regs,
> +               regmap_write(engine->regs,
>                              SUN8I_MIXER_BLEND_ATTR_COORD(bld_base, zpos),
>                              SUN8I_MIXER_COORD(x, y));
> -               regmap_write(bld_regs,
> +               regmap_write(engine->regs,
>                              SUN8I_MIXER_BLEND_ATTR_INSIZE(bld_base, zpos),
>                              SUN8I_MIXER_SIZE(w, h));
>         }
>
> -       regmap_write(bld_regs, SUN8I_MIXER_BLEND_ROUTE(bld_base), route);
> -       regmap_write(bld_regs, SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
> +       regmap_write(engine->regs, SUN8I_MIXER_BLEND_ROUTE(bld_base), route);
> +       regmap_write(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
>                      pipe_en | SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0));
>
>         if (mixer->cfg->de_type != SUN8I_MIXER_DE33)
> @@ -317,7 +318,6 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
>         struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
>         int plane_cnt = mixer->cfg->ui_num + mixer->cfg->vi_num;
>         enum drm_plane_type type;
> -       unsigned int phy_index;
>         int i;
>
>         planes = devm_kcalloc(drm->dev, plane_cnt, sizeof(*planes), GFP_KERNEL);
> @@ -332,12 +332,8 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
>                 else
>                         type = DRM_PLANE_TYPE_OVERLAY;
>
> -               phy_index = i;
> -               if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
> -                       phy_index = mixer->cfg->map[i];
> -
>                 layer = sun8i_vi_layer_init_one(drm, type, mixer->engine.regs,
> -                                               i, phy_index, plane_cnt,
> +                                               i, i, plane_cnt,
>                                                 &mixer->cfg->lay_cfg);
>                 if (IS_ERR(layer)) {
>                         dev_err(drm->dev,
> @@ -357,12 +353,8 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
>                 else
>                         type = DRM_PLANE_TYPE_OVERLAY;
>
> -               phy_index = index;
> -               if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
> -                       phy_index = mixer->cfg->map[index];
> -
>                 layer = sun8i_ui_layer_init_one(drm, type, mixer->engine.regs,
> -                                               index, phy_index, plane_cnt,
> +                                               index, index, plane_cnt,
>                                                 &mixer->cfg->lay_cfg);
>                 if (IS_ERR(layer)) {
>                         dev_err(drm->dev, "Couldn't initialize %s plane\n",
> @@ -376,16 +368,25 @@ static struct drm_plane **sun8i_layers_init(struct drm_device *drm,
>         return planes;
>  }
>
> +static struct drm_plane **sun50i_layers_init(struct drm_device *drm,
> +                                            struct sunxi_engine *engine)
> +{
> +       struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> +
> +       if (IS_ENABLED(CONFIG_DRM_SUN50I_PLANES))
> +               return sun50i_planes_setup(mixer->planes_dev, drm, engine->id);
> +
> +       return NULL;
> +}
> +
>  static void sun8i_mixer_mode_set(struct sunxi_engine *engine,
>                                  const struct drm_display_mode *mode)
>  {
>         struct sun8i_mixer *mixer = engine_to_sun8i_mixer(engine);
> -       struct regmap *bld_regs;
>         u32 bld_base, size, val;
>         bool interlaced;
>
>         bld_base = sun8i_blender_base(mixer);
> -       bld_regs = sun8i_blender_regmap(mixer);
>         interlaced = !!(mode->flags & DRM_MODE_FLAG_INTERLACE);
>         size = SUN8I_MIXER_SIZE(mode->hdisplay, mode->vdisplay);
>
> @@ -397,14 +398,14 @@ static void sun8i_mixer_mode_set(struct sunxi_engine *engine,
>         else
>                 regmap_write(mixer->engine.regs, SUN8I_MIXER_GLOBAL_SIZE, size);
>
> -       regmap_write(bld_regs, SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
> +       regmap_write(engine->regs, SUN8I_MIXER_BLEND_OUTSIZE(bld_base), size);
>
>         if (interlaced)
>                 val = SUN8I_MIXER_BLEND_OUTCTL_INTERLACED;
>         else
>                 val = 0;
>
> -       regmap_update_bits(bld_regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
> +       regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_OUTCTL(bld_base),
>                            SUN8I_MIXER_BLEND_OUTCTL_INTERLACED, val);
>
>         DRM_DEBUG_DRIVER("Switching display mixer interlaced mode %s\n",
> @@ -417,8 +418,14 @@ static const struct sunxi_engine_ops sun8i_engine_ops = {
>         .mode_set       = sun8i_mixer_mode_set,
>  };
>
> +static const struct sunxi_engine_ops sun50i_engine_ops = {
> +       .commit         = sun8i_mixer_commit,
> +       .layers_init    = sun50i_layers_init,
> +       .mode_set       = sun8i_mixer_mode_set,
> +};
> +
>  static const struct regmap_config sun8i_mixer_regmap_config = {
> -       .name           = "layers",
> +       .name           = "display",
>         .reg_bits       = 32,
>         .val_bits       = 32,
>         .reg_stride     = 4,
> @@ -433,14 +440,6 @@ static const struct regmap_config sun8i_top_regmap_config = {
>         .max_register   = 0x3c,
>  };
>
> -static const struct regmap_config sun8i_disp_regmap_config = {
> -       .name           = "display",
> -       .reg_bits       = 32,
> -       .val_bits       = 32,
> -       .reg_stride     = 4,
> -       .max_register   = 0x20000,
> -};
> -
>  static int sun8i_mixer_of_get_id(struct device_node *node)
>  {
>         struct device_node *ep, *remote;
> @@ -463,17 +462,14 @@ static int sun8i_mixer_of_get_id(struct device_node *node)
>
>  static void sun8i_mixer_init(struct sun8i_mixer *mixer)
>  {
> -       struct regmap *top_regs, *disp_regs;
>         unsigned int base = sun8i_blender_base(mixer);
> +       struct regmap *top_regs;
>         int plane_cnt, i;
>
> -       if (mixer->cfg->de_type == SUN8I_MIXER_DE33) {
> +       if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
>                 top_regs = mixer->top_regs;
> -               disp_regs = mixer->disp_regs;
> -       } else {
> +       else
>                 top_regs = mixer->engine.regs;
> -               disp_regs = mixer->engine.regs;
> -       }
>
>         /* Enable the mixer */
>         regmap_write(top_regs, SUN8I_MIXER_GLOBAL_CTL,
> @@ -483,25 +479,25 @@ static void sun8i_mixer_init(struct sun8i_mixer *mixer)
>                 regmap_write(top_regs, SUN50I_MIXER_GLOBAL_CLK, 1);
>
>         /* Set background color to black */
> -       regmap_write(disp_regs, SUN8I_MIXER_BLEND_BKCOLOR(base),
> +       regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_BKCOLOR(base),
>                      SUN8I_MIXER_BLEND_COLOR_BLACK);
>
>         /*
>          * Set fill color of bottom plane to black. Generally not needed
>          * except when VI plane is at bottom (zpos = 0) and enabled.
>          */
> -       regmap_write(disp_regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
> +       regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
>                      SUN8I_MIXER_BLEND_PIPE_CTL_FC_EN(0));
> -       regmap_write(disp_regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0),
> +       regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_ATTR_FCOLOR(base, 0),
>                      SUN8I_MIXER_BLEND_COLOR_BLACK);
>
>         plane_cnt = mixer->cfg->vi_num + mixer->cfg->ui_num;
>         for (i = 0; i < plane_cnt; i++)
> -               regmap_write(disp_regs,
> +               regmap_write(mixer->engine.regs,
>                              SUN8I_MIXER_BLEND_MODE(base, i),
>                              SUN8I_MIXER_BLEND_MODE_DEF);
>
> -       regmap_update_bits(disp_regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
> +       regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL(base),
>                            SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
>  }
>
> @@ -532,7 +528,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
>         if (!mixer)
>                 return -ENOMEM;
>         dev_set_drvdata(dev, mixer);
> -       mixer->engine.ops = &sun8i_engine_ops;
>         mixer->engine.node = dev->of_node;
>
>         if (of_property_present(dev->of_node, "iommus")) {
> @@ -562,6 +557,11 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
>         if (!mixer->cfg)
>                 return -EINVAL;
>
> +       if (mixer->cfg->de_type == SUN8I_MIXER_DE33)
> +               mixer->engine.ops = &sun50i_engine_ops;

You're missing an IS_ENABLED() clause here if you wanted to make the DE 3.3
planes driver optional. Though as I mentioned in the other patch, splittig
the two modules might not work.

> +       else
> +               mixer->engine.ops = &sun8i_engine_ops;
> +
>         regs = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(regs))
>                 return PTR_ERR(regs);
> @@ -584,17 +584,6 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
>                         dev_err(dev, "Couldn't create the top regmap\n");
>                         return PTR_ERR(mixer->top_regs);
>                 }
> -
> -               regs = devm_platform_ioremap_resource_byname(pdev, "display");
> -               if (IS_ERR(regs))
> -                       return PTR_ERR(regs);
> -
> -               mixer->disp_regs = devm_regmap_init_mmio(dev, regs,
> -                                                        &sun8i_disp_regmap_config);
> -               if (IS_ERR(mixer->disp_regs)) {
> -                       dev_err(dev, "Couldn't create the disp regmap\n");
> -                       return PTR_ERR(mixer->disp_regs);
> -               }
>         }
>
>         mixer->reset = devm_reset_control_get(dev, NULL);
> @@ -634,6 +623,33 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
>
>         clk_prepare_enable(mixer->mod_clk);
>
> +       if (mixer->cfg->de_type == SUN8I_MIXER_DE33) {
> +               struct platform_device *pdev;
> +               struct device_node *np;
> +               void *data;
> +
> +               np = of_parse_phandle(dev->of_node, "allwinner,planes", 0);
> +               if (!np) {
> +                       ret = -ENODEV;
> +                       goto err_disable_mod_clk;
> +               }
> +
> +               pdev = of_find_device_by_node(np);

You need to add a matching put_device() in the unbind function.

Side note:

This bind function is using a lot of devm_ functions. These have the wrong
lifetime. I think it would be better if we could move resource acquisition
into the probe function.


> +               of_node_put(np);
> +               if (!pdev) {
> +                       ret = -EPROBE_DEFER;
> +                       goto err_disable_mod_clk;
> +               }
> +
> +               data = platform_get_drvdata(pdev);
> +               if (!data) {
> +                       put_device(&pdev->dev);
> +                       return -EPROBE_DEFER;

Should be a goto here?


ChenYu

> +               }
> +
> +               mixer->planes_dev = &pdev->dev;
> +       }
> +
>         list_add_tail(&mixer->engine.list, &drv->engine_list);
>
>         /* Reset registers and disable unused sub-engines */
> @@ -668,6 +684,8 @@ static int sun8i_mixer_bind(struct device *dev, struct device *master,
>
>         return 0;
>
> +err_disable_mod_clk:
> +       clk_disable_unprepare(mixer->mod_clk);
>  err_disable_bus_clk:
>         clk_disable_unprepare(mixer->bus_clk);
>  err_assert_reset:
> @@ -863,16 +881,8 @@ static const struct sun8i_mixer_cfg sun50i_h6_mixer0_cfg = {
>  };
>
>  static const struct sun8i_mixer_cfg sun50i_h616_mixer0_cfg = {
> -       .lay_cfg = {
> -               .de_type        = SUN8I_MIXER_DE33,
> -               .scaler_mask    = 0xf,
> -               .scanline_yuv   = 4096,
> -       },
>         .de_type        = SUN8I_MIXER_DE33,
>         .mod_rate       = 600000000,
> -       .ui_num         = 3,
> -       .vi_num         = 1,
> -       .map            = {0, 6, 7, 8},
>  };
>
>  static const struct of_device_id sun8i_mixer_of_table[] = {
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> index e2f83301aae8..7abc88c898d9 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @@ -202,7 +202,6 @@ struct sun8i_mixer_cfg {
>         int                     ui_num;
>         unsigned int            de_type;
>         unsigned long           mod_rate;
> -       unsigned int            map[6];
>  };
>
>  struct sun8i_mixer {
> @@ -216,7 +215,7 @@ struct sun8i_mixer {
>         struct clk                      *mod_clk;
>
>         struct regmap                   *top_regs;
> -       struct regmap                   *disp_regs;
> +       struct device                   *planes_dev;
>  };
>
>  enum {
> @@ -252,13 +251,6 @@ sun8i_blender_base(struct sun8i_mixer *mixer)
>         return mixer->cfg->de_type == SUN8I_MIXER_DE3 ? DE3_BLD_BASE : DE2_BLD_BASE;
>  }
>
> -static inline struct regmap *
> -sun8i_blender_regmap(struct sun8i_mixer *mixer)
> -{
> -       return mixer->cfg->de_type == SUN8I_MIXER_DE33 ?
> -               mixer->disp_regs : mixer->engine.regs;
> -}
> -
>  static inline u32
>  sun8i_channel_base(struct sun8i_layer *layer)
>  {
> --
> 2.51.2
>
>