Re: [PATCH v2 07/16] drm/imx: Add i.MX8qxp Display Controller pixel engine

On 30/07/2024 11:42, Liu Ying wrote:

On 07/30/2024, Krzysztof Kozlowski wrote:

quoted

On 30/07/2024 08:55, Liu Ying wrote:

quoted

On 07/28/2024, Dmitry Baryshkov wrote:

quoted

On Fri, Jul 12, 2024 at 05:32:34PM GMT, Liu Ying wrote:

quoted

i.MX8qxp Display Controller pixel engine consists of all processing
units that operate in the AXI bus clock domain.  Add drivers for
ConstFrame, ExtDst, FetchLayer, FetchWarp and LayerBlend units, as
well as a pixel engine driver, so that two displays with primary
planes can be supported.  The pixel engine driver as a master binds
those unit drivers as components.  While at it, the pixel engine
driver is a component to be bound with the upcoming DRM driver.

Same question / comment: create subnodes directly, without going
through the subdevices. A lot of small functions that would benefit
being inlined.

Like I replied in patch 06/16, I can't create sub devices directly.

Can you please point out typical ones for those small functions if
the comment still stands?

quoted

quoted

+static int dc_cf_bind(struct device *dev, struct device *master, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct dc_drm_device *dc_drm = data;
+	struct dc_pe *pe = dc_drm->pe;
+	struct dc_cf_priv *priv;
+	int id;
+
+	priv = drmm_kzalloc(&dc_drm->base, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->reg_cfg = devm_platform_ioremap_resource_byname(pdev, "cfg");
+	if (IS_ERR(priv->reg_cfg))
+		return PTR_ERR(priv->reg_cfg);
+
+	id = of_alias_get_id(dev->of_node, "dc0-constframe");

Is it documented? Acked?

Like I replied in patch 06/16, I can add aliases nodes to examples,
if needed.

No Nak from DT maintainers I'd say, but I hope there will be direct
Ack(s).

It was not Acked, because there was no documentation added for it.

I may add aliases nodes in examples in next version, if no objections.

Example is just example. It is not a documentation. You must explain it
in the binding, e.g. description.

quoted

Anyway, naming is quite cryptic, e.g. "0" in "dc0" is quite confusing.
Do you expect different aliases for dc1 or dc9? But anyway, aliases for

Yes, I do.  If the alias approach is used, DC instance ids need to be
specified in aliases.

Really? Uh, that does not look good. I tend to like this binding less
and less.


Best regards,
Krzysztof