? 2017?5?15? GMT+08:00 ??5:20:01, Maxime Ripard [off-list ref] ??:

On Mon, May 15, 2017 at 12:30:37AM +0800, Icenowy Zheng wrote:

quoted

As we are going to add support for the Allwinner DE2 engine in

sun4i-drm

quoted

driver, we will finally have two types of display engines -- the DE1
backend and the DE2 mixer. They both do some display blending and

feed

quoted

graphics data to TCON, and is part of the "Display Engine" called by
Allwinner, so I choose to call them both "engine" here.

Abstract the engine type to a new struct with an ops struct, which

contains

quoted

functions that should be called outside the engine-specified code (in
TCON, CRTC or TV Encoder code).

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Reviewed-by: Chen-Yu Tsai <redacted>
---
Changes in v7:
- Mention "Display Engine" for the name "engine".
- Fixed some small issues found by Chen-Yu and added his ACK.
Changes in v6:
- Rebased on wens's multi-pipeline patchset.
- Split out Makefile changes.

You also added a get_id callback here...

quoted

+static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
+	.commit				= sun4i_backend_commit,
+	.layers_init			= sun4i_layers_init,
+	.apply_color_correction		= sun4i_backend_apply_color_correction,
+	.disable_color_correction	= sun4i_backend_disable_color_correction,
+};
+

... that you don't populate ...

quoted

@@ -477,7 +481,7 @@ static int sun4i_tcon_bind(struct device *dev,

struct device *master,

quoted

 	dev_set_drvdata(dev, tcon);
 	tcon->drm = drm;
 	tcon->dev = dev;
-	tcon->id = backend->id;
+	tcon->id = sunxi_engine_get_id(engine);

... that you call to fill the TCON ID ...

quoted

+/**
+ * sunxi_engine_get_id - Get the ID of the engine.
+ * @engine:	pointer to the engine
+ *
+ * If the ID is not necessary, just do not implement it in

sunxi_engine_ops,

quoted

+ * and a default -1 will be returned.
+ */
+static inline int
+sunxi_engine_get_id(struct sunxi_engine *engine)
+{
+	if (engine->ops && engine->ops->get_id)
+		return engine->ops->get_id(engine);
+
+	return -1;

... and will return -1 if not populated, which essentially means that
instead of having 0 or 1, we're now having -1 as our id.

This is a regression, and I'm even wondering if we can't just store
the ID in the sunxi_engine structure. Is anything preventing us to do
that, instead of using a callback?

Some engines (de2 mixer) doesn't use the ID. But 0 is a valid ID here.

If we just store it in sunxi_engine, it should be assigned -1 for de2 mixer.
Is this applicable?

If it's okay I will do so.

Maxime