于 2017年5月15日 GMT+08:00 下午5:20:01, Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> 写到: >On Mon, May 15, 2017 at 12:30:37AM +0800, Icenowy Zheng wrote: >> As we are going to add support for the Allwinner DE2 engine in >sun4i-drm >> 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 >> 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 >> functions that should be called outside the engine-specified code (in >> TCON, CRTC or TV Encoder code). >> >> Signed-off-by: Icenowy Zheng <icenowy@xxxxxxx> >> Reviewed-by: Chen-Yu Tsai <wens@xxxxxxxx> >> --- >> 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... > >> +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 ... > >> @@ -477,7 +481,7 @@ static int sun4i_tcon_bind(struct device *dev, >struct device *master, >> 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 ... > >> +/** >> + * 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, >> + * 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html