Hi,
On 05/06/2024 11:25, Andy Yan wrote:
Hi,
At 2024-06-05 04:33:57, "Cristian Ciocaltea" <cristian.ciocaltea@xxxxxxxxxxxxx> wrote:
On 6/3/24 4:08 PM, neil.armstrong@xxxxxxxxxx wrote:
Hi,
On 03/06/2024 15:03, Heiko Stuebner wrote:
Am Montag, 3. Juni 2024, 14:14:17 CEST schrieb Andy Yan:
Hi Neil:
On 6/3/24 16:55, Neil Armstrong wrote:
Hi Christian,
On 01/06/2024 15:12, Cristian Ciocaltea wrote:
The RK3588 SoC family integrates a Quad-Pixel (QP) variant of the
Synopsys DesignWare HDMI TX controller used in the previous SoCs.
It is HDMI 2.1 compliant and supports the following features, among
others:
.
..
* SCDC I2C DDC access
* TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4
* YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds
* Multi-stream audio
* Enhanced Audio Return Channel (EARC)
-> Those features were already supported by the HDMI 2.0a compliant
HW, just
list the _new_ features for HDMI 2.1
I did a quick review of your patchset and I don't understand why you
need
to add a separate dw-hdmi-qp.c since you only need simple variants
of the I2C
bus, infoframe and bridge setup.
Can you elaborate further ? isn't this Quad-Pixel (QP) TX controller
version
detectable at runtime ?
I would prefer to keep a single dw-hdmi driver if possible.
The QP HDMI controller is a completely different variant with totally
different
registers layout, see PATCH 13/14.
I think make it a separate driver will be easier for development and
maintenance.
I'm with Andy here. Trying to navigate a driver for two IP blocks really
sounds taxing especially when both are so different.
Thank you all for the valuable feedback!
I agree, I just wanted more details than "variant of the
Synopsys DesignWare HDMI TX controller", if the register mapping is 100%
different, and does not match at all with the old IP, then it's indeed time
to make a brand new driver, but instead of doing a mix up, it's time to
extract
the dw-hdmi code that could be common helpers into a dw-hdmi-common module
and use them.
Sounds good, will handle this in v2.
As I see, no "driver" code can be shared, only DRM plumbings, so perhaps
those
plumbing code should go into the DRM core ?
In any case, please add more details on the cover letter, including the
detailed
HW differrence and the design you chose so support this new IP.
Andy, could you please help with a summary of the HW changes?
The information I could provide is rather limited, since I don't have
access to any DW IP datasheets and I'm also not familiar enough with the
old variant.
Accurately, we should refer to it as an entirely new IP，it has nothing in common with
the current mainline dw-hdmi。 The only commonality is that they both come from
Synopsys DesignWare：
（1）It has a 100% different register mapping
（2）It supports FRL and DSC
（3）different configuration flow in many places。
So I have the same feeling with Heiko and Maxime：
The DW_HDMI_QP should have a separate driver and with it's own CONFIG such as DRM_DW_HDMI_QP in Kconfig.
and the rockchip part should also be split from dw_hdmi-rockchip.c.
I am sorry we mixed them in dw_hdmi-rockchip.c when we develop the bsp driver，but we really regretted this decision
when we repeatedly broke compatibility with dw-hdmi on other socs。
Yes please, and as I say, if there's code common with the old dw-hdmi, please add a common
module if this code can't be moved in core bridge helpers.
Neil
Neil
Synopsis also created a new dsi controller for the DSI2 standard, with
a vastly different registers layout.
I guess at some point there is time to say this really is a new IP ;-) .
Though while on that thought, I don't fully understand why both a
compiled
under the dw_hdmi kconfig symbol. People going for a minimal kernel might
want one or the other, but not both for their specific board.
Indeed, it makes sense to have a dedicated Kconfig option. This is
mostly a leftover from downstream implementation, will fix in v2.
Thanks again,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
