Re:Re: [v3 5/6] drm/vs: Add hdmi driver
Hi Keith:
在 2023-12-13 09:40:31，"Keith Zhao" 写道：
>
>
>On 2023/12/11 20:13, Andy Yan wrote:
>> Hi Keith：
>>
>> 在 2023-12-11 18:24:35，"Keith Zhao" 写道：
>>>hi Maxime:
>>>hi Andy:
>>>
>>>On 2023/12/8 17:14, Maxime Ripard wrote:
Hi,
On Fri, Dec 08, 2023 at 11:23:37AM +0800, Andy Yan wrote:
> 在 2023-12-08 11:00:31，"Keith Zhao" 写道：
> >
> >
> >On 2023/12/8 8:37, Andy Yan wrote:
> >> Hi Keth：
> >>
> >>
> >>
> >>
> >>
> >>
> >> 在 2023-12-07 18:48:13，"Keith Zhao" 写道：
> >>>
> >>>
> >>>On 2023/12/7 17:02, Andy Yan wrote:
>
>
>
>
> Hi Keith：
>
>
>
>
>
>
>
>
>
>
>
> At 2023-12-06 22:11:33, "Keith Zhao"
> wrote:
> >
> >
> >On 2023/12/6 20:56, Maxime Ripard wrote:
> >> On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
> >>> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
> >>> >> + { .compatible = "starfive,jh7110-inno-hdmi",},
> >>> >
> >>> > So it's inno hdmi, just like Rockchip then?
> >>> >
> >>> > This should be a common driver.
> >>>
> >>> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
> >>> but the harewawre difference of them is big , it is not easy to
> >>> use the common driver
> >>> maybe i need the inno hdmi version here to make a distinction
> >>
> >> I just had a look at the rockchip header file: all the registers
> >> but the
> >> STARFIVE_* ones are identical.
> >>
> >> There's no need to have two identical drivers then, please use the
> >> rockchip driver instead.
> >>
> >> Maxime
> >
> >ok, have a simple test , edid can get . i will continue
>
> Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a
> reference， this
> is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
> We finally make it share one driver。
> >
> >>>hi Andy:
> >>>
> >>>dw_hdmi seems a good choice , it can handle inno hdmi hardware by
> >>>define its dw_hdmi_plat_data.
> >>>does it means i can write own driver files such as(dw_hdmi-starfive.c)
> >>>based on dw_hdmi instead of add plat_data in inno_hdmi.c
> >>>
> >>
> >> I think the process maybe like this：
> >>
> >> 1. split the inno_hdmi.c under rockchip to inno_hdmi.c(the common
> >> part), inno_hdmi-rockchip.c(the soc specific part)
> >> 2. move the common part inno_hdmi.c to
> >> drivers/gpu/drm/bridge/innosilicon/
> >> 3. add startfive specific part, inno_hdmi-startfive.c
> >>
> >> bellow git log from kernel three show how we convert dw_hdmi to a
> >> common driver:
> >>
> >>
> >>
> >> 12b9f204e804 drm: bridge/dw_hdmi: add rockchip rk3288 support
> >> 74af9e4d03b8 dt-bindings: Add documentation for rockchip dw hdmi
> >> d346c14eeea9 drm: bridge/dw_hdmi: add function dw_hdmi_phy_enable_spare
> >> a4d3b8b050d5 drm: bridge/dw_hdmi: clear i2cmphy_stat0 reg in
> >> hdmi_phy_wait_i2c_done
> >> 632d035bace2 drm: bridge/dw_hdmi: add mode_valid support
> >> 0cd9d1428322 drm: bridge/dw_hdmi: add support for multi-byte register
> >> width access
> >> cd152393967e dt-bindings: add document for dw_hdmi
> >> b21f4b658df8 drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
> >> aaa757a092c2 drm: imx: imx-hdmi: split phy configuration to platform
> >> driver
> >> 3d1b35a3d9f3 drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
> >> c2c3848851a7 drm: imx: imx-hdmi: return defer if can't get ddc i2c
> >> adapter
> >> b587833933de drm: imx: imx-hdmi: make checkpatch happy
> >>
> >hi Andy:
> >I got you means,
> >as I don't have a rockchip board on hand , to split the inno_hdmi.c can
> >not be tested.
> >
> >how adout this idea:
> >1、split the starfive_hdmi.c under verisilicion to inno_hdmi.c(the
> >common part), inno_hdmi-starfive.c(the soc specific part)
> >2. move the common part inno_hdmi.c to
> >drivers/gpu/drm/bridge/innosilicon/
> >3. In the future, inno hdmi.c under rockchip will reuse the public
> >driver.
>
> I am not sure if drm maintainers are happy with this。
Not really, no.
Because we would still have two drivers for the same controller, and a
common one that haven't really been tested on anything but a single
platform. So arguably a worse situation than what you were suggesting in
the first place.
The best solution would be to find someone with a Rockchip board to test
your changes, or to get one if it's doable so you
Re:Re: [v3 5/6] drm/vs: Add hdmi driver
Hi Keith：
在 2023-12-11 18:24:35，"Keith Zhao" 写道：
>hi Maxime:
>hi Andy:
>
>On 2023/12/8 17:14, Maxime Ripard wrote:
>> Hi,
>>
>> On Fri, Dec 08, 2023 at 11:23:37AM +0800, Andy Yan wrote:
>>> 在 2023-12-08 11:00:31，"Keith Zhao" 写道：
>>> >
>>> >
>>> >On 2023/12/8 8:37, Andy Yan wrote:
>>> >> Hi Keth：
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> 在 2023-12-07 18:48:13，"Keith Zhao" 写道：
>>> >>>
>>> >>>
>>> >>>On 2023/12/7 17:02, Andy Yan wrote:
>>>
>>>
>>>
>>>
>>> Hi Keith：
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> At 2023-12-06 22:11:33, "Keith Zhao"
>>> wrote:
>>> >
>>> >
>>> >On 2023/12/6 20:56, Maxime Ripard wrote:
>>> >> On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
>>> >>> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
>>> >>> >> +{ .compatible = "starfive,jh7110-inno-hdmi",},
>>> >>> >
>>> >>> > So it's inno hdmi, just like Rockchip then?
>>> >>> >
>>> >>> > This should be a common driver.
>>> >>>
>>> >>> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
>>> >>> but the harewawre difference of them is big , it is not easy to use
>>> >>> the common driver
>>> >>> maybe i need the inno hdmi version here to make a distinction
>>> >>
>>> >> I just had a look at the rockchip header file: all the registers but
>>> >> the
>>> >> STARFIVE_* ones are identical.
>>> >>
>>> >> There's no need to have two identical drivers then, please use the
>>> >> rockchip driver instead.
>>> >>
>>> >> Maxime
>>> >
>>> >ok, have a simple test , edid can get . i will continue
>>>
>>> Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a
>>> reference， this
>>> is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
>>> We finally make it share one driver。
>>> >
>>> >>>hi Andy:
>>> >>>
>>> >>>dw_hdmi seems a good choice , it can handle inno hdmi hardware by define
>>> >>>its dw_hdmi_plat_data.
>>> >>>does it means i can write own driver files such as(dw_hdmi-starfive.c)
>>> >>>based on dw_hdmi instead of add plat_data in inno_hdmi.c
>>> >>>
>>> >>
>>> >> I think the process maybe like this：
>>> >>
>>> >> 1. split the inno_hdmi.c under rockchip to inno_hdmi.c(the common
>>> >> part), inno_hdmi-rockchip.c(the soc specific part)
>>> >> 2. move the common part inno_hdmi.c to
>>> >> drivers/gpu/drm/bridge/innosilicon/
>>> >> 3. add startfive specific part, inno_hdmi-startfive.c
>>> >>
>>> >> bellow git log from kernel three show how we convert dw_hdmi to a
>>> >> common driver:
>>> >>
>>> >>
>>> >>
>>> >> 12b9f204e804 drm: bridge/dw_hdmi: add rockchip rk3288 support
>>> >> 74af9e4d03b8 dt-bindings: Add documentation for rockchip dw hdmi
>>> >> d346c14eeea9 drm: bridge/dw_hdmi: add function dw_hdmi_phy_enable_spare
>>> >> a4d3b8b050d5 drm: bridge/dw_hdmi: clear i2cmphy_stat0 reg in
>>> >> hdmi_phy_wait_i2c_done
>>> >> 632d035bace2 drm: bridge/dw_hdmi: add mode_valid support
>>> >> 0cd9d1428322 drm: bridge/dw_hdmi: add support for multi-byte register
>>> >> width access
>>> >> cd152393967e dt-bindings: add document for dw_hdmi
>>> >> b21f4b658df8 drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
>>> >> aaa757a092c2 drm: imx: imx-hdmi: split phy configuration to platform
>>> >> driver
>>> >> 3d1b35a3d9f3 drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
>>> >> c2c3848851a7 drm: imx: imx-hdmi: return defer if can't get ddc i2c
>>> >> adapter
>>> >> b587833933de drm: imx: imx-hdmi: make checkpatch happy
>>> >>
>>> >hi Andy:
>>> >I got you means,
>>> >as I don't have a rockchip board on hand , to split the inno_hdmi.c can
>>> >not be tested.
>>> >
>>> >how adout this idea:
>>> >1、split the starfive_hdmi.c under verisilicion to inno_hdmi.c(the common
>>> >part), inno_hdmi-starfive.c(the soc specific part)
>>> >2. move the common part inno_hdmi.c to drivers/gpu/drm/bridge/innosilicon/
>>> >3. In the future, inno hdmi.c under rockchip will reuse the public driver.
>>>
>>> I am not sure if drm maintainers are happy with this。
>>
>> Not really, no.
>>
>> Because we would still have two drivers for the same controller, and a
>> common one that haven't really been tested on anything but a single
>> platform. So arguably a worse situation than what you were suggesting in
>> the first place.
>>
>> The best solution would be to find someone with a Rockchip board to test
>> your changes, or to get one if it's doable so you can test yourself.
>
>ok I will also try to buy a Rockchip 3036 board for self-test.
>According to the commit log idea provided by Andy before, make the inno_hdmi
>driver common module.
I finally make my rk3036 based kylin board bootup （use a linux 4.4 downstream
bsp，I will find time to try boot
it with mainline）。 So I can help do the test for rockchip side。
It seems not
Re:Re: [v3 5/6] drm/vs: Add hdmi driver
Hi Keith：
在 2023-12-08 11:00:31，"Keith Zhao" 写道：
>
>
>On 2023/12/8 8:37, Andy Yan wrote:
>> Hi Keth：
>>
>>
>>
>>
>>
>>
>> 在 2023-12-07 18:48:13，"Keith Zhao" 写道：
>>>
>>>
>>>On 2023/12/7 17:02, Andy Yan wrote:
Hi Keith：
At 2023-12-06 22:11:33, "Keith Zhao" wrote:
>
>
>On 2023/12/6 20:56, Maxime Ripard wrote:
>> On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
>>> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
>>> >> +{ .compatible = "starfive,jh7110-inno-hdmi",},
>>> >
>>> > So it's inno hdmi, just like Rockchip then?
>>> >
>>> > This should be a common driver.
>>>
>>> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
>>> but the harewawre difference of them is big , it is not easy to use the
>>> common driver
>>> maybe i need the inno hdmi version here to make a distinction
>>
>> I just had a look at the rockchip header file: all the registers but the
>> STARFIVE_* ones are identical.
>>
>> There's no need to have two identical drivers then, please use the
>> rockchip driver instead.
>>
>> Maxime
>
>ok, have a simple test , edid can get . i will continue
Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a reference，
this
is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
We finally make it share one driver。
>
>>>hi Andy:
>>>
>>>dw_hdmi seems a good choice , it can handle inno hdmi hardware by define its
>>>dw_hdmi_plat_data.
>>>does it means i can write own driver files such as(dw_hdmi-starfive.c) based
>>>on dw_hdmi instead of add plat_data in inno_hdmi.c
>>>
>>
>> I think the process maybe like this：
>>
>> 1. split the inno_hdmi.c under rockchip to inno_hdmi.c(the common part),
>> inno_hdmi-rockchip.c(the soc specific part)
>> 2. move the common part inno_hdmi.c to drivers/gpu/drm/bridge/innosilicon/
>> 3. add startfive specific part, inno_hdmi-startfive.c
>>
>> bellow git log from kernel three show how we convert dw_hdmi to a common
>> driver:
>>
>>
>>
>> 12b9f204e804 drm: bridge/dw_hdmi: add rockchip rk3288 support
>> 74af9e4d03b8 dt-bindings: Add documentation for rockchip dw hdmi
>> d346c14eeea9 drm: bridge/dw_hdmi: add function dw_hdmi_phy_enable_spare
>> a4d3b8b050d5 drm: bridge/dw_hdmi: clear i2cmphy_stat0 reg in
>> hdmi_phy_wait_i2c_done
>> 632d035bace2 drm: bridge/dw_hdmi: add mode_valid support
>> 0cd9d1428322 drm: bridge/dw_hdmi: add support for multi-byte register width
>> access
>> cd152393967e dt-bindings: add document for dw_hdmi
>> b21f4b658df8 drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
>> aaa757a092c2 drm: imx: imx-hdmi: split phy configuration to platform driver
>> 3d1b35a3d9f3 drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
>> c2c3848851a7 drm: imx: imx-hdmi: return defer if can't get ddc i2c adapter
>> b587833933de drm: imx: imx-hdmi: make checkpatch happy
>>
>hi Andy:
>I got you means,
>as I don't have a rockchip board on hand , to split the inno_hdmi.c can not be
>tested.
>
>how adout this idea:
>1、split the starfive_hdmi.c under verisilicion to inno_hdmi.c(the common
>part), inno_hdmi-starfive.c(the soc specific part)
>2. move the common part inno_hdmi.c to drivers/gpu/drm/bridge/innosilicon/
>3. In the future, inno hdmi.c under rockchip will reuse the public driver.
I am not sure if drm maintainers are happy with this。
To be honest， I also don't have a i.mx board when I start convert dw_hdmi to a
common driver,
some respectable people from the community help test and give me many valuable
advice， this
is the power of open source。
I found a rk3036 based kylin board this week，but it can't boot yet，I will go
on try if
I can boot it this weekend。 I can do the test on rockchip side， if i can make
this board work。
>
>>
>>>Thanks for pointing this out!!!
>>>
>
>___
>linux-riscv mailing list
>linux-ri...@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>
>>>___
>>>linux-riscv mailing list
>>>linux-ri...@lists.infradead.org
>>>http://lists.infradead.org/mailman/listinfo/linux-riscv
Re:Re: [v3 5/6] drm/vs: Add hdmi driver
Hi Keth：
在 2023-12-07 18:48:13，"Keith Zhao" 写道：
>
>
>On 2023/12/7 17:02, Andy Yan wrote:
>>
>>
>>
>>
>> Hi Keith：
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> At 2023-12-06 22:11:33, "Keith Zhao" wrote:
>>>
>>>
>>>On 2023/12/6 20:56, Maxime Ripard wrote:
On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
> >> + { .compatible = "starfive,jh7110-inno-hdmi",},
> >
> > So it's inno hdmi, just like Rockchip then?
> >
> > This should be a common driver.
>
> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
> but the harewawre difference of them is big , it is not easy to use the
> common driver
> maybe i need the inno hdmi version here to make a distinction
I just had a look at the rockchip header file: all the registers but the
STARFIVE_* ones are identical.
There's no need to have two identical drivers then, please use the
rockchip driver instead.
Maxime
>>>
>>>ok, have a simple test , edid can get . i will continue
>>
>> Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a reference，
>> this
>> is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
>> We finally make it share one driver。
>>>
>hi Andy:
>
>dw_hdmi seems a good choice , it can handle inno hdmi hardware by define its
>dw_hdmi_plat_data.
>does it means i can write own driver files such as(dw_hdmi-starfive.c) based
>on dw_hdmi instead of add plat_data in inno_hdmi.c
>
I think the process maybe like this：
1. split the inno_hdmi.c under rockchip to inno_hdmi.c(the common part),
inno_hdmi-rockchip.c(the soc specific part)
2. move the common part inno_hdmi.c to drivers/gpu/drm/bridge/innosilicon/
3. add startfive specific part, inno_hdmi-startfive.c
bellow git log from kernel three show how we convert dw_hdmi to a common
driver:
12b9f204e804 drm: bridge/dw_hdmi: add rockchip rk3288 support
74af9e4d03b8 dt-bindings: Add documentation for rockchip dw hdmi
d346c14eeea9 drm: bridge/dw_hdmi: add function dw_hdmi_phy_enable_spare
a4d3b8b050d5 drm: bridge/dw_hdmi: clear i2cmphy_stat0 reg in
hdmi_phy_wait_i2c_done
632d035bace2 drm: bridge/dw_hdmi: add mode_valid support
0cd9d1428322 drm: bridge/dw_hdmi: add support for multi-byte register width
access
cd152393967e dt-bindings: add document for dw_hdmi
b21f4b658df8 drm: imx: imx-hdmi: move imx-hdmi to bridge/dw_hdmi
aaa757a092c2 drm: imx: imx-hdmi: split phy configuration to platform driver
3d1b35a3d9f3 drm: imx: imx-hdmi: convert imx-hdmi to drm_bridge mode
c2c3848851a7 drm: imx: imx-hdmi: return defer if can't get ddc i2c adapter
b587833933de drm: imx: imx-hdmi: make checkpatch happy
>Thanks for pointing this out!!!
>
>>>
>>>___
>>>linux-riscv mailing list
>>>linux-ri...@lists.infradead.org
>>>http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>___
>linux-riscv mailing list
>linux-ri...@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-riscv
Re:Re: [v3 5/6] drm/vs: Add hdmi driver
Hi Keith：
At 2023-12-06 22:11:33, "Keith Zhao" wrote:
>
>
>On 2023/12/6 20:56, Maxime Ripard wrote:
>> On Wed, Dec 06, 2023 at 08:02:55PM +0800, Keith Zhao wrote:
>>> >> +static const struct of_device_id starfive_hdmi_dt_ids[] = {
>>> >> +{ .compatible = "starfive,jh7110-inno-hdmi",},
>>> >
>>> > So it's inno hdmi, just like Rockchip then?
>>> >
>>> > This should be a common driver.
>>>
>>> Rockchip has a inno hdmi IP. and Starfive has a inno hdmi IP.
>>> but the harewawre difference of them is big , it is not easy to use the
>>> common driver
>>> maybe i need the inno hdmi version here to make a distinction
>>
>> I just had a look at the rockchip header file: all the registers but the
>> STARFIVE_* ones are identical.
>>
>> There's no need to have two identical drivers then, please use the
>> rockchip driver instead.
>>
>> Maxime
>
>ok, have a simple test , edid can get . i will continue
Maybe you can take drivers/gpu/drm/bridge/synopsys/dw-hdmi as a reference， this
is also a hdmi ip used by rockchip/meson/sunxi/jz/imx。
We finally make it share one driver。
>
>
>___
>linux-riscv mailing list
>linux-ri...@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-riscv
