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

[Liu Ying]

On 07/30/2024, Krzysztof Kozlowski wrote:
> 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.

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

> 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.

> sub-devices of pipeline look wrong.

Why?  They are separate devices.  Though I agree with Rob that
aliases should be generic rather than vendor specific, it seems
that there are some vendor specific aliases in upstream device
trees.

Any better way to specify the instance ids?  OF graph ports?

> 
> Best regards,
> Krzysztof
> 
> 

-- 
Regards,
Liu Ying