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

On Fri, Feb 18, 2022 at 11:50:32AM +0800, Andy Yan wrote:

Hi Sascha:

On 2/17/22 22:06, Heiko Stübner wrote:

quoted

Am Donnerstag, 17. Februar 2022, 14:58:23 CET schrieb Sascha Hauer:

quoted

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:

quoted

Hi Sascha:

quoted

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

Yes, but not for retrieving bit offsets into registers. Normally aliases
can be changed at board level without confusing drivers.

And your method also
add need additional dt binds to define RK3568_VOP2_EP_xxx

quoted

quoted

quoted

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.

The device tree parsing could be moved out of vop2_crtc_atomic_enable()
into some initialisation path. In the end it's static information,
there's no need to do it repeatedly in atomic_enable.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel