Re: [v3 5/6] drm/vs: Add hdmi driver
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 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
Re: [v3 5/6] drm/vs: Add hdmi driver
On Mon, Dec 4, 2023 at 6:33 AM Keith Zhao wrote: > > add hdmi driver as encoder and connect > > Signed-off-by: Keith Zhao > --- > drivers/gpu/drm/verisilicon/Kconfig | 8 + > drivers/gpu/drm/verisilicon/Makefile| 1 + > drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849 > drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 +++ > drivers/gpu/drm/verisilicon/vs_drv.c| 3 + > drivers/gpu/drm/verisilicon/vs_drv.h| 4 + > 6 files changed, 1169 insertions(+) > create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c > create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h > > diff --git a/drivers/gpu/drm/verisilicon/Kconfig > b/drivers/gpu/drm/verisilicon/Kconfig > index e10fa97635aa..122c786e3948 100644 > --- a/drivers/gpu/drm/verisilicon/Kconfig > +++ b/drivers/gpu/drm/verisilicon/Kconfig > @@ -11,3 +11,11 @@ config DRM_VERISILICON > This driver provides VeriSilicon kernel mode > setting and buffer management. It does not > provide 2D or 3D acceleration. > + > +config DRM_VERISILICON_STARFIVE_HDMI > + bool "Starfive HDMI extensions" > + depends on DRM_VERISILICON > + help > + This selects support for StarFive soc specific extensions > + for the Innosilicon HDMI driver. If you want to enable > + HDMI on JH7110 based soc, you should select this option. > diff --git a/drivers/gpu/drm/verisilicon/Makefile > b/drivers/gpu/drm/verisilicon/Makefile > index bf6f2b7ee480..71fadafcee13 100644 > --- a/drivers/gpu/drm/verisilicon/Makefile > +++ b/drivers/gpu/drm/verisilicon/Makefile > @@ -6,4 +6,5 @@ vs_drm-objs := vs_dc_hw.o \ > vs_drv.o \ > vs_modeset.o \ > vs_plane.o > +vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o > obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o > diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c > b/drivers/gpu/drm/verisilicon/starfive_hdmi.c > new file mode 100644 > index ..aa621db0dee0 > --- /dev/null > +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c > @@ -0,0 +1,849 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 StarFive Technology Co., Ltd. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include You probably don't need this header and the implicit includes it makes are dropped now in linux-next. Please check what you actually need and make them explicit. Rob
Re: [v3 5/6] drm/vs: Add hdmi driver
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.
would the steps be ok? (if I tested rockchip and starifve pass)
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
Thanks
>
> Maxime
Re: [v3 5/6] drm/vs: Add hdmi driver
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.
Maxime
signature.asc
Description: PGP signature
Re: [v3 5/6] drm/vs: Add hdmi driver
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.
>
>>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: [v3 5/6] drm/vs: Add hdmi driver
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
Thanks for pointing this out!!!
>>
>>___
>>linux-riscv mailing list
>>linux-ri...@lists.infradead.org
>>http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [v3 5/6] drm/vs: Add hdmi driver
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
Re: [v3 5/6] drm/vs: Add hdmi driver
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
signature.asc
Description: PGP signature
Re: [v3 5/6] drm/vs: Add hdmi driver
On 2023/12/6 17:04, Maxime Ripard wrote:
> On Mon, Dec 04, 2023 at 08:33:14PM +0800, Keith Zhao wrote:
>> add hdmi driver as encoder and connect
>>
>> Signed-off-by: Keith Zhao
>> ---
>> drivers/gpu/drm/verisilicon/Kconfig | 8 +
>> drivers/gpu/drm/verisilicon/Makefile| 1 +
>> drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849
>> drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 +++
>> drivers/gpu/drm/verisilicon/vs_drv.c| 3 +
>> drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
>> 6 files changed, 1169 insertions(+)
>> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
>> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
>>
>> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
>> b/drivers/gpu/drm/verisilicon/Kconfig
>> index e10fa97635aa..122c786e3948 100644
>> --- a/drivers/gpu/drm/verisilicon/Kconfig
>> +++ b/drivers/gpu/drm/verisilicon/Kconfig
>> @@ -11,3 +11,11 @@ config DRM_VERISILICON
>>This driver provides VeriSilicon kernel mode
>>setting and buffer management. It does not
>>provide 2D or 3D acceleration.
>> +
>> +config DRM_VERISILICON_STARFIVE_HDMI
>> +bool "Starfive HDMI extensions"
>> +depends on DRM_VERISILICON
>> +help
>> + This selects support for StarFive soc specific extensions
>> + for the Innosilicon HDMI driver. If you want to enable
>> + HDMI on JH7110 based soc, you should select this option.
>
> I'm confused, is it a starfive or verisilicon IP?
it is Innosilicon ip ，I need describe it clearer in my next.
>
>> diff --git a/drivers/gpu/drm/verisilicon/Makefile
>> b/drivers/gpu/drm/verisilicon/Makefile
>> index bf6f2b7ee480..71fadafcee13 100644
>> --- a/drivers/gpu/drm/verisilicon/Makefile
>> +++ b/drivers/gpu/drm/verisilicon/Makefile
>> @@ -6,4 +6,5 @@ vs_drm-objs := vs_dc_hw.o \
>> vs_drv.o \
>> vs_modeset.o \
>> vs_plane.o
>> +vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
>> obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
>> diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
>> b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
>> new file mode 100644
>> index ..aa621db0dee0
>> --- /dev/null
>> +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
>> @@ -0,0 +1,849 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "starfive_hdmi.h"
>> +#include "vs_drv.h"
>> +#include "vs_crtc.h"
>> +
>> +static const char * const hdmi_clocks[] = {
>> +"sysclk",
>> +"mclk",
>> +"bclk"
>> +};
>> +
>> +static struct starfive_hdmi_encoder *encoder_to_hdmi(struct drm_encoder
>> *encoder)
>> +{
>> +return container_of(encoder, struct starfive_hdmi_encoder, encoder);
>> +}
>> +
>> +static struct starfive_hdmi *connector_to_hdmi(struct drm_connector
>> *connector)
>> +{
>> +return container_of(connector, struct starfive_hdmi, connector);
>> +}
>> +
>> +static const struct post_pll_config post_pll_cfg_table[] = {
>> +{2520, 1, 80, 13, 3, 1},
>> +{2700, 1, 40, 11, 3, 1},
>> +{3375, 1, 40, 11, 3, 1},
>> +{4900, 1, 20, 1, 3, 3},
>> +{24170, 1, 20, 1, 3, 3},
>> +{29700, 4, 20, 0, 0, 3},
>> +{59400, 4, 20, 0, 0, 0},
>
> If you don't support modes > 340MHz, then there's no point in listing 594MHz
> here
sure . max should be 297M here.
>
>> +{ /* sentinel */ }
>> +};
>> +
>> +inline u8 hdmi_readb(struct starfive_hdmi *hdmi, u16 offset)
>> +{
>> +return readl_relaxed(hdmi->regs + (offset) * 0x04);
>> +}
>> +
>> +inline void hdmi_writeb(struct starfive_hdmi *hdmi, u16 offset, u32 val)
>> +{
>> +writel_relaxed(val, hdmi->regs + (offset) * 0x04);
>> +}
>> +
>> +inline void hdmi_writew(struct starfive_hdmi *hdmi, u16 offset, u32 val)
>> +{
>> +writew_relaxed(val & 0xFF, hdmi->regs + (offset) * 0x04);
>> +writew_relaxed((val >> 8) & 0xFF, hdmi->regs + (offset + 1) * 0x04);
>> +}
>> +
>> +inline void hdmi_modb(struct starfive_hdmi *hdmi, u16 offset,
>> + u32 msk, u32 val)
>> +{
>> +u8 temp = hdmi_readb(hdmi, offset) & ~msk;
>> +
>> +temp |= val & msk;
>> +hdmi_writeb(hdmi, offset, temp);
>> +}
>> +
>> +static int starfive_hdmi_enable_clk_deassert_rst(struct device *dev, struct
>> starfive_hdmi *hdmi)
>> +{
>> +int ret;
>> +
>> +ret = clk_bulk_prepare_enable(hdmi->nclks, hdmi->clk_hdmi);
>> +if (ret) {
>> +dev_err(dev, "failed to enable clocks\n");
>> +return ret;
>> +}
>> +
>> +ret = reset_control_deassert(hdmi->tx_rst);
>> +
Re: [v3 5/6] drm/vs: Add hdmi driver
On Mon, Dec 04, 2023 at 08:33:14PM +0800, Keith Zhao wrote:
> add hdmi driver as encoder and connect
>
> Signed-off-by: Keith Zhao
> ---
> drivers/gpu/drm/verisilicon/Kconfig | 8 +
> drivers/gpu/drm/verisilicon/Makefile| 1 +
> drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849
> drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 +++
> drivers/gpu/drm/verisilicon/vs_drv.c| 3 +
> drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
> 6 files changed, 1169 insertions(+)
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
>
> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> b/drivers/gpu/drm/verisilicon/Kconfig
> index e10fa97635aa..122c786e3948 100644
> --- a/drivers/gpu/drm/verisilicon/Kconfig
> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> @@ -11,3 +11,11 @@ config DRM_VERISILICON
> This driver provides VeriSilicon kernel mode
> setting and buffer management. It does not
> provide 2D or 3D acceleration.
> +
> +config DRM_VERISILICON_STARFIVE_HDMI
> + bool "Starfive HDMI extensions"
> + depends on DRM_VERISILICON
> + help
> +This selects support for StarFive soc specific extensions
> +for the Innosilicon HDMI driver. If you want to enable
> +HDMI on JH7110 based soc, you should select this option.
I'm confused, is it a starfive or verisilicon IP?
> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> b/drivers/gpu/drm/verisilicon/Makefile
> index bf6f2b7ee480..71fadafcee13 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -6,4 +6,5 @@ vs_drm-objs := vs_dc_hw.o \
> vs_drv.o \
> vs_modeset.o \
> vs_plane.o
> +vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
> obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> new file mode 100644
> index ..aa621db0dee0
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> @@ -0,0 +1,849 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "starfive_hdmi.h"
> +#include "vs_drv.h"
> +#include "vs_crtc.h"
> +
> +static const char * const hdmi_clocks[] = {
> + "sysclk",
> + "mclk",
> + "bclk"
> +};
> +
> +static struct starfive_hdmi_encoder *encoder_to_hdmi(struct drm_encoder
> *encoder)
> +{
> + return container_of(encoder, struct starfive_hdmi_encoder, encoder);
> +}
> +
> +static struct starfive_hdmi *connector_to_hdmi(struct drm_connector
> *connector)
> +{
> + return container_of(connector, struct starfive_hdmi, connector);
> +}
> +
> +static const struct post_pll_config post_pll_cfg_table[] = {
> + {2520, 1, 80, 13, 3, 1},
> + {2700, 1, 40, 11, 3, 1},
> + {3375, 1, 40, 11, 3, 1},
> + {4900, 1, 20, 1, 3, 3},
> + {24170, 1, 20, 1, 3, 3},
> + {29700, 4, 20, 0, 0, 3},
> + {59400, 4, 20, 0, 0, 0},
If you don't support modes > 340MHz, then there's no point in listing 594MHz
here
> + { /* sentinel */ }
> +};
> +
> +inline u8 hdmi_readb(struct starfive_hdmi *hdmi, u16 offset)
> +{
> + return readl_relaxed(hdmi->regs + (offset) * 0x04);
> +}
> +
> +inline void hdmi_writeb(struct starfive_hdmi *hdmi, u16 offset, u32 val)
> +{
> + writel_relaxed(val, hdmi->regs + (offset) * 0x04);
> +}
> +
> +inline void hdmi_writew(struct starfive_hdmi *hdmi, u16 offset, u32 val)
> +{
> + writew_relaxed(val & 0xFF, hdmi->regs + (offset) * 0x04);
> + writew_relaxed((val >> 8) & 0xFF, hdmi->regs + (offset + 1) * 0x04);
> +}
> +
> +inline void hdmi_modb(struct starfive_hdmi *hdmi, u16 offset,
> + u32 msk, u32 val)
> +{
> + u8 temp = hdmi_readb(hdmi, offset) & ~msk;
> +
> + temp |= val & msk;
> + hdmi_writeb(hdmi, offset, temp);
> +}
> +
> +static int starfive_hdmi_enable_clk_deassert_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(hdmi->nclks, hdmi->clk_hdmi);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks\n");
> + return ret;
> + }
> +
> + ret = reset_control_deassert(hdmi->tx_rst);
> + if (ret < 0) {
> + dev_err(dev, "failed to deassert tx_rst\n");
> + return ret;
> + }
> + return 0;
> +}
> +
> +static void starfive_hdmi_disable_clk_assert_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> +
Re: [v3 5/6] drm/vs: Add hdmi driver
On Mon, 4 Dec 2023 at 14:33, Keith Zhao wrote:
>
> add hdmi driver as encoder and connect
>
> Signed-off-by: Keith Zhao
> ---
> drivers/gpu/drm/verisilicon/Kconfig | 8 +
> drivers/gpu/drm/verisilicon/Makefile| 1 +
> drivers/gpu/drm/verisilicon/starfive_hdmi.c | 849
> drivers/gpu/drm/verisilicon/starfive_hdmi.h | 304 +++
> drivers/gpu/drm/verisilicon/vs_drv.c| 3 +
> drivers/gpu/drm/verisilicon/vs_drv.h| 4 +
> 6 files changed, 1169 insertions(+)
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.c
> create mode 100644 drivers/gpu/drm/verisilicon/starfive_hdmi.h
>
> diff --git a/drivers/gpu/drm/verisilicon/Kconfig
> b/drivers/gpu/drm/verisilicon/Kconfig
> index e10fa97635aa..122c786e3948 100644
> --- a/drivers/gpu/drm/verisilicon/Kconfig
> +++ b/drivers/gpu/drm/verisilicon/Kconfig
> @@ -11,3 +11,11 @@ config DRM_VERISILICON
> This driver provides VeriSilicon kernel mode
> setting and buffer management. It does not
> provide 2D or 3D acceleration.
> +
> +config DRM_VERISILICON_STARFIVE_HDMI
> + bool "Starfive HDMI extensions"
> + depends on DRM_VERISILICON
> + help
> + This selects support for StarFive soc specific extensions
> + for the Innosilicon HDMI driver. If you want to enable
> + HDMI on JH7110 based soc, you should select this option.
> diff --git a/drivers/gpu/drm/verisilicon/Makefile
> b/drivers/gpu/drm/verisilicon/Makefile
> index bf6f2b7ee480..71fadafcee13 100644
> --- a/drivers/gpu/drm/verisilicon/Makefile
> +++ b/drivers/gpu/drm/verisilicon/Makefile
> @@ -6,4 +6,5 @@ vs_drm-objs := vs_dc_hw.o \
> vs_drv.o \
> vs_modeset.o \
> vs_plane.o
> +vs_drm-$(CONFIG_DRM_VERISILICON_STARFIVE_HDMI) += starfive_hdmi.o
> obj-$(CONFIG_DRM_VERISILICON) += vs_drm.o
> diff --git a/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> new file mode 100644
> index ..aa621db0dee0
> --- /dev/null
> +++ b/drivers/gpu/drm/verisilicon/starfive_hdmi.c
> @@ -0,0 +1,849 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "starfive_hdmi.h"
> +#include "vs_drv.h"
> +#include "vs_crtc.h"
> +
> +static const char * const hdmi_clocks[] = {
> + "sysclk",
> + "mclk",
> + "bclk"
> +};
> +
> +static struct starfive_hdmi_encoder *encoder_to_hdmi(struct drm_encoder
> *encoder)
> +{
> + return container_of(encoder, struct starfive_hdmi_encoder, encoder);
> +}
> +
> +static struct starfive_hdmi *connector_to_hdmi(struct drm_connector
> *connector)
> +{
> + return container_of(connector, struct starfive_hdmi, connector);
> +}
> +
> +static const struct post_pll_config post_pll_cfg_table[] = {
> + {2520, 1, 80, 13, 3, 1},
> + {2700, 1, 40, 11, 3, 1},
> + {3375, 1, 40, 11, 3, 1},
> + {4900, 1, 20, 1, 3, 3},
> + {24170, 1, 20, 1, 3, 3},
> + {29700, 4, 20, 0, 0, 3},
> + {59400, 4, 20, 0, 0, 0},
> + { /* sentinel */ }
> +};
> +
> +inline u8 hdmi_readb(struct starfive_hdmi *hdmi, u16 offset)
> +{
> + return readl_relaxed(hdmi->regs + (offset) * 0x04);
> +}
> +
> +inline void hdmi_writeb(struct starfive_hdmi *hdmi, u16 offset, u32 val)
> +{
> + writel_relaxed(val, hdmi->regs + (offset) * 0x04);
> +}
> +
> +inline void hdmi_writew(struct starfive_hdmi *hdmi, u16 offset, u32 val)
> +{
> + writew_relaxed(val & 0xFF, hdmi->regs + (offset) * 0x04);
> + writew_relaxed((val >> 8) & 0xFF, hdmi->regs + (offset + 1) * 0x04);
> +}
> +
> +inline void hdmi_modb(struct starfive_hdmi *hdmi, u16 offset,
> +u32 msk, u32 val)
> +{
> + u8 temp = hdmi_readb(hdmi, offset) & ~msk;
> +
> + temp |= val & msk;
> + hdmi_writeb(hdmi, offset, temp);
> +}
> +
> +static int starfive_hdmi_enable_clk_deassert_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(hdmi->nclks, hdmi->clk_hdmi);
> + if (ret) {
> + dev_err(dev, "failed to enable clocks\n");
> + return ret;
> + }
> +
> + ret = reset_control_deassert(hdmi->tx_rst);
> + if (ret < 0) {
> + dev_err(dev, "failed to deassert tx_rst\n");
> + return ret;
> + }
> + return 0;
> +}
> +
> +static void starfive_hdmi_disable_clk_assert_rst(struct device *dev, struct
> starfive_hdmi *hdmi)
> +{
> + int ret;
> +
> + ret =
