[PATCH v4 0/7] Convert inno hdmi to drm bridge

Andy Yan posted 7 patches 7 months, 3 weeks ago
There is a newer version of this series
.../display/rockchip/rockchip,inno-hdmi.yaml  |  20 +-
arch/arm/boot/dts/rockchip/rk3036.dtsi        |   5 +-
drivers/gpu/drm/bridge/Kconfig                |   7 +
drivers/gpu/drm/bridge/Makefile               |   1 +
.../inno_hdmi.c => bridge/inno-hdmi.c}        | 907 ++++++++++--------
drivers/gpu/drm/rockchip/Kconfig              |   1 +
drivers/gpu/drm/rockchip/Makefile             |   2 +-
drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c | 187 ++++
drivers/gpu/drm/rockchip/inno_hdmi.h          | 349 -------
include/drm/bridge/inno_hdmi.h                |  33 +
10 files changed, 754 insertions(+), 758 deletions(-)
rename drivers/gpu/drm/{rockchip/inno_hdmi.c => bridge/inno-hdmi.c} (52%)
create mode 100644 drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c
delete mode 100644 drivers/gpu/drm/rockchip/inno_hdmi.h
create mode 100644 include/drm/bridge/inno_hdmi.h
[PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Andy Yan 7 months, 3 weeks ago
From: Andy Yan <andy.yan@rock-chips.com>

When preparing to convert the current inno hdmi driver into a
bridge driver, I found that there are several issues currently
existing with it:

1. When the system starts up, the first time it reads the EDID, it
   will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
   reference clock to be enabled first before normal DDC communication
   can be carried out.

2. The signal is unstable. When running the glmark2 test on the screen,
   there is a small probability of seeing some screen flickering.
   This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
   by GRF. This part is missing in the current driver.

PATCH 1~6 are try to Fix Document in the dt-binding, then add the
missing part in driver and dts.
PATCH 7 converts the curren driver to drm bridge mode.

Changes in v4:
- Do not store colorimetry within inno_hdmi struct
- Link to V3: https://lore.kernel.org/linux-rockchip/20250402123150.238234-1-andyshrk@163.com/

Changes in v3:
- Convert to drm bridge driver
- Link to V2: https://lore.kernel.org/dri-devel/20250325132944.171111-1-andyshrk@163.com/

Changes in v2:
- included the HSYNC/VSYNC polarity fix

Andy Yan (7):
  dt-bindings: display: rockchip,inno-hdmi: Fix Document of RK3036
    compatible
  dt-bindings: display: rockchip,inno-hdmi: Document GRF for RK3036 HDMI
  drm/rockchip: inno-hdmi: Simplify error handler with dev_err_probe
  drm/rockchip: inno-hdmi: Fix video timing HSYNC/VSYNC polarity setting
    for rk3036
  ARM: dts: rockchip: Add ref clk for hdmi
  Revert "ARM: dts: rockchip: drop grf reference from rk3036 hdmi"
  drm/rockchip: inno-hdmi: Convert to drm bridge

 .../display/rockchip/rockchip,inno-hdmi.yaml  |  20 +-
 arch/arm/boot/dts/rockchip/rk3036.dtsi        |   5 +-
 drivers/gpu/drm/bridge/Kconfig                |   7 +
 drivers/gpu/drm/bridge/Makefile               |   1 +
 .../inno_hdmi.c => bridge/inno-hdmi.c}        | 907 ++++++++++--------
 drivers/gpu/drm/rockchip/Kconfig              |   1 +
 drivers/gpu/drm/rockchip/Makefile             |   2 +-
 drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c | 187 ++++
 drivers/gpu/drm/rockchip/inno_hdmi.h          | 349 -------
 include/drm/bridge/inno_hdmi.h                |  33 +
 10 files changed, 754 insertions(+), 758 deletions(-)
 rename drivers/gpu/drm/{rockchip/inno_hdmi.c => bridge/inno-hdmi.c} (52%)
 create mode 100644 drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c
 delete mode 100644 drivers/gpu/drm/rockchip/inno_hdmi.h
 create mode 100644 include/drm/bridge/inno_hdmi.h

-- 
2.43.0
Re: [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Heiko Stübner 7 months, 2 weeks ago
Am Dienstag, 22. April 2025, 09:04:39 Mitteleuropäische Sommerzeit schrieb Andy Yan:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> When preparing to convert the current inno hdmi driver into a
> bridge driver, I found that there are several issues currently
> existing with it:
> 
> 1. When the system starts up, the first time it reads the EDID, it
>    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>    reference clock to be enabled first before normal DDC communication
>    can be carried out.
> 
> 2. The signal is unstable. When running the glmark2 test on the screen,
>    there is a small probability of seeing some screen flickering.
>    This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
>    by GRF. This part is missing in the current driver.
> 
> PATCH 1~6 are try to Fix Document in the dt-binding, then add the
> missing part in driver and dts.
> PATCH 7 converts the curren driver to drm bridge mode.

After resurrecting my rk3036-kylin which hasn't sucessfully booted in a
while, I could veryify this series, so on a rk3036-kylin

Tested-by: Heiko Stuebner <heiko@sntech.de>


I'll probably apply patches 1-4 to drm-misc later today, as that solely
touches the Rockchip (and only rk3036-)side and patches 5+6 to the
rockchip tree.

Patch 7 should probably get some attention by people more familiar with
drm-bridges, so I'll let that sit for a bit longer.


Thanks a lot for working on all this
Heiko
Re: [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Dmitry Baryshkov 7 months, 2 weeks ago
On Sat, May 03, 2025 at 04:42:04PM +0200, Heiko Stübner wrote:
> Am Dienstag, 22. April 2025, 09:04:39 Mitteleuropäische Sommerzeit schrieb Andy Yan:
> > From: Andy Yan <andy.yan@rock-chips.com>
> > 
> > When preparing to convert the current inno hdmi driver into a
> > bridge driver, I found that there are several issues currently
> > existing with it:
> > 
> > 1. When the system starts up, the first time it reads the EDID, it
> >    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
> >    reference clock to be enabled first before normal DDC communication
> >    can be carried out.
> > 
> > 2. The signal is unstable. When running the glmark2 test on the screen,
> >    there is a small probability of seeing some screen flickering.
> >    This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
> >    by GRF. This part is missing in the current driver.
> > 
> > PATCH 1~6 are try to Fix Document in the dt-binding, then add the
> > missing part in driver and dts.
> > PATCH 7 converts the curren driver to drm bridge mode.
> 
> After resurrecting my rk3036-kylin which hasn't sucessfully booted in a
> while, I could veryify this series, so on a rk3036-kylin
> 
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> 
> I'll probably apply patches 1-4 to drm-misc later today, as that solely
> touches the Rockchip (and only rk3036-)side and patches 5+6 to the
> rockchip tree.
> 
> Patch 7 should probably get some attention by people more familiar with
> drm-bridges, so I'll let that sit for a bit longer.

I will take a look later, but on the first glance it looks like there
are too many things going on in that patch, including some unnecessary
fnction movements and define movements, etc. I would kindly ask to split
the non-functional refactorings and the functional ones (splitting to a
library, etc).

> 
> 
> Thanks a lot for working on all this
> Heiko
> 
> 

-- 
With best wishes
Dmitry
Re:Re: [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Andy Yan 7 months, 1 week ago
Hi Dmitry,

 Thanks for you review.

在 2025-05-05 00:16:35,"Dmitry Baryshkov" <dmitry.baryshkov@oss.qualcomm.com> 写道:
>On Sat, May 03, 2025 at 04:42:04PM +0200, Heiko Stübner wrote:
>> Am Dienstag, 22. April 2025, 09:04:39 Mitteleuropäische Sommerzeit schrieb Andy Yan:
>> > From: Andy Yan <andy.yan@rock-chips.com>
>> > 
>> > When preparing to convert the current inno hdmi driver into a
>> > bridge driver, I found that there are several issues currently
>> > existing with it:
>> > 
>> > 1. When the system starts up, the first time it reads the EDID, it
>> >    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>> >    reference clock to be enabled first before normal DDC communication
>> >    can be carried out.
>> > 
>> > 2. The signal is unstable. When running the glmark2 test on the screen,
>> >    there is a small probability of seeing some screen flickering.
>> >    This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
>> >    by GRF. This part is missing in the current driver.
>> > 
>> > PATCH 1~6 are try to Fix Document in the dt-binding, then add the
>> > missing part in driver and dts.
>> > PATCH 7 converts the curren driver to drm bridge mode.
>> 
>> After resurrecting my rk3036-kylin which hasn't sucessfully booted in a
>> while, I could veryify this series, so on a rk3036-kylin
>> 
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> 
>> 
>> I'll probably apply patches 1-4 to drm-misc later today, as that solely
>> touches the Rockchip (and only rk3036-)side and patches 5+6 to the
>> rockchip tree.
>> 
>> Patch 7 should probably get some attention by people more familiar with
>> drm-bridges, so I'll let that sit for a bit longer.
>
>I will take a look later, but on the first glance it looks like there
>are too many things going on in that patch, including some unnecessary
>fnction movements and define movements, etc. I would kindly ask to split

These registers were initially defined in a separate header file(inno_hdmi.h), 
but they were only used by a single C file, so I think it's not necessary to put
them in a separate header file. I decided to simply merge them into the inno_hdmi.c file. 
If I first create a patch and separately carry out the merging of this register definition, would that be possible?

And I will try to avoid function movements in next version.


>the non-functional refactorings and the functional ones (splitting to a
>library, etc).

Will do in next version.

>
>> 
>> 
>> Thanks a lot for working on all this
>> Heiko
>> 
>> 
>
>-- 
>With best wishes
>Dmitry
Re: [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Dmitry Baryshkov 7 months, 1 week ago
On 09/05/2025 04:58, Andy Yan wrote:
> 
> Hi Dmitry,
> 
>   Thanks for you review.
> 
> 在 2025-05-05 00:16:35,"Dmitry Baryshkov" <dmitry.baryshkov@oss.qualcomm.com> 写道:
>> On Sat, May 03, 2025 at 04:42:04PM +0200, Heiko Stübner wrote:
>>> Am Dienstag, 22. April 2025, 09:04:39 Mitteleuropäische Sommerzeit schrieb Andy Yan:
>>>> From: Andy Yan <andy.yan@rock-chips.com>
>>>>
>>>> When preparing to convert the current inno hdmi driver into a
>>>> bridge driver, I found that there are several issues currently
>>>> existing with it:
>>>>
>>>> 1. When the system starts up, the first time it reads the EDID, it
>>>>     will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>>>>     reference clock to be enabled first before normal DDC communication
>>>>     can be carried out.
>>>>
>>>> 2. The signal is unstable. When running the glmark2 test on the screen,
>>>>     there is a small probability of seeing some screen flickering.
>>>>     This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
>>>>     by GRF. This part is missing in the current driver.
>>>>
>>>> PATCH 1~6 are try to Fix Document in the dt-binding, then add the
>>>> missing part in driver and dts.
>>>> PATCH 7 converts the curren driver to drm bridge mode.
>>>
>>> After resurrecting my rk3036-kylin which hasn't sucessfully booted in a
>>> while, I could veryify this series, so on a rk3036-kylin
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>
>>>
>>> I'll probably apply patches 1-4 to drm-misc later today, as that solely
>>> touches the Rockchip (and only rk3036-)side and patches 5+6 to the
>>> rockchip tree.
>>>
>>> Patch 7 should probably get some attention by people more familiar with
>>> drm-bridges, so I'll let that sit for a bit longer.
>>
>> I will take a look later, but on the first glance it looks like there
>> are too many things going on in that patch, including some unnecessary
>> fnction movements and define movements, etc. I would kindly ask to split
> 
> These registers were initially defined in a separate header file(inno_hdmi.h),
> but they were only used by a single C file, so I think it's not necessary to put
> them in a separate header file. I decided to simply merge them into the inno_hdmi.c file.
> If I first create a patch and separately carry out the merging of this register definition, would that be possible?

Yes, just create a separate commit, folding the header into the source file.

> 
> And I will try to avoid function movements in next version.
> 
> 
>> the non-functional refactorings and the functional ones (splitting to a
>> library, etc).
> 
> Will do in next version.
> 
>>
>>>
>>>
>>> Thanks a lot for working on all this
>>> Heiko
>>>
>>>
>>
>> -- 
>> With best wishes
>> Dmitry


-- 
With best wishes
Dmitry
Re: [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Heiko Stübner 7 months, 2 weeks ago
Hi Alex,

Am Dienstag, 22. April 2025, 09:04:39 Mitteleuropäische Sommerzeit schrieb Andy Yan:
> From: Andy Yan <andy.yan@rock-chips.com>
> 
> When preparing to convert the current inno hdmi driver into a
> bridge driver, I found that there are several issues currently
> existing with it:
> 
> 1. When the system starts up, the first time it reads the EDID, it
>    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>    reference clock to be enabled first before normal DDC communication
>    can be carried out.
> 
> 2. The signal is unstable. When running the glmark2 test on the screen,
>    there is a small probability of seeing some screen flickering.
>    This is because The HSYNC/VSYNC polarity of rk3036 HDMI are controlled
>    by GRF. This part is missing in the current driver.
> 
> PATCH 1~6 are try to Fix Document in the dt-binding, then add the
> missing part in driver and dts.
> PATCH 7 converts the curren driver to drm bridge mode.

this looks all pretty okay to me, but could you check that your rk3128
keeps working and maybe provide a Tested-by?

Thanks a lot
Heiko
Re: (subset) [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Heiko Stuebner 7 months, 2 weeks ago
On Tue, 22 Apr 2025 15:04:39 +0800, Andy Yan wrote:
> When preparing to convert the current inno hdmi driver into a
> bridge driver, I found that there are several issues currently
> existing with it:
> 
> 1. When the system starts up, the first time it reads the EDID, it
>    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>    reference clock to be enabled first before normal DDC communication
>    can be carried out.
> 
> [...]

Applied, thanks!

[1/7] dt-bindings: display: rockchip,inno-hdmi: Fix Document of RK3036 compatible
      commit: c0673bb356557136954b6725bf5fe327b94c6233
[2/7] dt-bindings: display: rockchip,inno-hdmi: Document GRF for RK3036 HDMI
      commit: e0c93980d293b6e6eb7483fd5d665f84e725b518
[3/7] drm/rockchip: inno-hdmi: Simplify error handler with dev_err_probe
      commit: 31b4403c6c523a710205eadb9ca75c6cfe1478ab
[4/7] drm/rockchip: inno-hdmi: Fix video timing HSYNC/VSYNC polarity setting for rk3036
      commit: ad10b82c2bcac7f87ac6eaecfca33378b43425ee

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>
Re: (subset) [PATCH v4 0/7] Convert inno hdmi to drm bridge
Posted by Heiko Stuebner 7 months, 1 week ago
On Tue, 22 Apr 2025 15:04:39 +0800, Andy Yan wrote:
> When preparing to convert the current inno hdmi driver into a
> bridge driver, I found that there are several issues currently
> existing with it:
> 
> 1. When the system starts up, the first time it reads the EDID, it
>    will fail. This is because RK3036 HDMI DDC bus requires it's PHY's
>    reference clock to be enabled first before normal DDC communication
>    can be carried out.
> 
> [...]

Applied, thanks!

[5/7] ARM: dts: rockchip: Add ref clk for hdmi
      commit: cdc602ad064009470b1c40af51d4a8cd804eaaf9
[6/7] Revert "ARM: dts: rockchip: drop grf reference from rk3036 hdmi"
      commit: dd6c77864aa69ba1079998c590b552e35649d51b

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>