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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/07/2024 08:55, Liu Ying wrote:
> On 07/28/2024, Dmitry Baryshkov wrote:
>> On Fri, Jul 12, 2024 at 05:32:34PM GMT, Liu Ying wrote:
>>> 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?
> 
>>
>>> +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.
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
sub-devices of pipeline look wrong.

Best regards,
Krzysztof