[PATCH v6 21/23] drm: rockchip: Add VOP2 driver

[Andy Yan]

Hi Sascha:

On 2/17/22 22:06, Heiko Stübner wrote:
> Am Donnerstag, 17. Februar 2022, 14:58:23 CET schrieb Sascha Hauer:
>> Hi Andy,
>>
>> Please trim the context in your answers to the relevant parts, it makes
>> it easier to find the things you said.
>>
>> On Thu, Feb 17, 2022 at 08:00:11PM +0800, Andy Yan wrote:
>>> Hi Sascha:
>>>
>>>> +
>>>> +	drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
>>>> +		struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
>>>> +		struct device_node *node, *parent;
>>>> +
>>>> +		parent = of_get_parent(rkencoder->port);
>>>> +
>>>> +		for_each_endpoint_of_node(parent, node) {
>>> Is there any hurt directly use our downstream vendor kernel method here: use
>>> vcstate->output_if set by encoder driver to get which interface we should
>>> enable here?
>> There is no vcstate->output_if in mainline currently. Ok, we could add
>> that. The other thing is that there are multiple HDMI interfaces and
>> the id of the HDMI encoder is encoded into output_if. Downstream kernel
>> adds OF aliases to the HDMI ports. I didn't want to go that route
>> because it doesn't seem to be very elegant to me.
aliases is a very comm strategy in device tree world.  And your method 
also add need additional dt binds to define RK3568_VOP2_EP_xxx
>>> You method is ok with device tree,  but it tied up this driver to device
>>> tree, we are now tring to extend vop2 driver work with ACPI, so we hope this
>>> driver can be much more flexible.
>> The current rockchip drm driver seems to be pretty much tied to device
>> tree. There are probably many other places that need parallel paths for
>> ACPI support, I think we can delay this particular part until we see the
>> whole picture. In the end we can still retrieve the output_if
>> information differently with ACPI while still retrieving the information
>> from the device tree the way we are doing currently.

The current driver only reference device thee at driver initial, we not 
wrap

device tree related things in other parts, so if we extend it to support 
ACPI,

we just need modify the initial code, this make things easier.

> agreed :-) .
>
> I.e. adding ACPI support for Rockchip drivers separately later on
> makes things way easier.
>
> Having a separate discussion about ACPI changes at that point
> also makes the whole process easier, as adding the whole thing
> here will delay everything even more.


  Heiko: I am not  ask to add new code for future  ACPI support, I just

hope the original downstream method  can keep to make future work easier.

> Also if a later series really only is about adding ACPI support, this
> makes for easier discussion but also easier review of changes.
> The new VOP2 driver is big enough as it is.
>
>
> Heiko
>
>