[2/8] drm/loongson: Introduce a drm bridge driver for it66121 HDMI transmitter

Message ID 20231029194607.379459-3-suijingfeng@loongson.cn
State New
Headers
Series drm/loongson: Submit a mini VBIOS support and a display bridge driver |

Commit Message

Sui Jingfeng Oct. 29, 2023, 7:46 p.m. UTC
  The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
chip to support HDMI output. Thus add a drm bridge based driver for it.
This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.

Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
---
 drivers/gpu/drm/loongson/Kconfig            |   1 +
 drivers/gpu/drm/loongson/Makefile           |   2 +
 drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
 drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
 drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
 5 files changed, 1039 insertions(+)
 create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
 create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
 create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
  

Comments

Dmitry Baryshkov Oct. 29, 2023, 10:53 p.m. UTC | #1
On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>
> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
> chip to support HDMI output. Thus add a drm bridge based driver for it.
> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.

Please use the original bridge driver instead of adding a new one. If
it needs to be changed in any way, please help everyone else by
improving it instead of introducing new driver.

>
> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
> ---
>  drivers/gpu/drm/loongson/Kconfig            |   1 +
>  drivers/gpu/drm/loongson/Makefile           |   2 +
>  drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
>  drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
>  drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
>  5 files changed, 1039 insertions(+)
>  create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
>  create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
>  create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
  
Sui Jingfeng Oct. 30, 2023, 9:42 a.m. UTC | #2
Hi,


On 2023/10/30 06:53, Dmitry Baryshkov wrote:
> On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
>> chip to support HDMI output. Thus add a drm bridge based driver for it.
>> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
> Please use the original bridge driver instead of adding a new one.

I'm agree with the spirit of code sharing, but this is nearly impossible for non-DT system.

Because the original bridge driver(say it66121.ko) is fully dependent on the DT.
UEFI+ACPI based system can not use with it.

Our I2C adapter is created by the drm/loongson.ko on the runtime.
The potential problem is that *cyclic dependency* !

I2C adapter driver is depend on drm/loongson
drm/loongson depend on drm bridge driver (say it66121.ko)
drm bridge driver (say it66121.ko) depend on I2C adapter to setup.

This plus the defer probe mechanism is totally a trap,
incurring troubles and don't work.


>   If
> it needs to be changed in any way, please help everyone else by
> improving it instead of introducing new driver.
>
>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
>> ---
>>   drivers/gpu/drm/loongson/Kconfig            |   1 +
>>   drivers/gpu/drm/loongson/Makefile           |   2 +
>>   drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
>>   drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
>>   drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
>>   5 files changed, 1039 insertions(+)
>>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
>>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
>>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
>
  
Dmitry Baryshkov Oct. 30, 2023, 10:01 a.m. UTC | #3
On Mon, 30 Oct 2023 at 11:42, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>
> Hi,
>
>
> On 2023/10/30 06:53, Dmitry Baryshkov wrote:
> > On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
> >> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
> >> chip to support HDMI output. Thus add a drm bridge based driver for it.
> >> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
> > Please use the original bridge driver instead of adding a new one.
>
> I'm agree with the spirit of code sharing, but this is nearly impossible for non-DT system.
>
> Because the original bridge driver(say it66121.ko) is fully dependent on the DT.

I can not agree here. It doesn't depend on DT. It has fully populated
i2c_device_id structures, so it will work with bare I2C buses.
Most likely you will have to change of calls into fwnode calls, that's it.

> UEFI+ACPI based system can not use with it.
>
> Our I2C adapter is created by the drm/loongson.ko on the runtime.
> The potential problem is that *cyclic dependency* !
>
> I2C adapter driver is depend on drm/loongson
> drm/loongson depend on drm bridge driver (say it66121.ko)
> drm bridge driver (say it66121.ko) depend on I2C adapter to setup.
>
> This plus the defer probe mechanism is totally a trap,
> incurring troubles and don't work.

Welcome. We had this kind of issue for DP AUX buses.

I can suggest the following approach:
- In the root probe function you can create an i2c bus and populate it
with the i2c devices.
- I have not checked whether you use components or not. If not, please
use an auxiliary or a platform device for the main DRM functionality.
- In the subdevice probe / bind function you check for the next
bridge. Then you get one of the following:drm_bridge pointer,
-EPROBE_DEFER, or any other error case. Your driver can react
accordingly.

Basically duplicating the existing driver code is not really a way to
go. Consider somebody adding a new feature or fixing a bug in your
driver copy. Then they have to check if the fix applies to the driver
at drivers/gpu/drm/bridge/ite-it66121.c. And vice versa. After fixing
an issue in the standard driver one has to keep in mind to check your
private copy.

So, please, use the OF code as an inspiration and register all your
devices in the device tree. Yes, this requires some effort from your
side. Yes, this pays off in the longer distance.

> >   If
> > it needs to be changed in any way, please help everyone else by
> > improving it instead of introducing new driver.
> >
> >> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
> >> ---
> >>   drivers/gpu/drm/loongson/Kconfig            |   1 +
> >>   drivers/gpu/drm/loongson/Makefile           |   2 +
> >>   drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
> >>   drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
> >>   drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
> >>   5 files changed, 1039 insertions(+)
> >>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
> >>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
> >>   create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
  
Maxime Ripard Oct. 30, 2023, 10:13 a.m. UTC | #4
Hi,

On Mon, Oct 30, 2023 at 05:42:28PM +0800, Sui Jingfeng wrote:
> On 2023/10/30 06:53, Dmitry Baryshkov wrote:
> > On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
> > > The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
> > > chip to support HDMI output. Thus add a drm bridge based driver for it.
> > > This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
> > Please use the original bridge driver instead of adding a new one.

FTR I agree with Dmitry here, duplicating drivers because it's easier is
a show-stopper

> I'm agree with the spirit of code sharing, but this is nearly
> impossible for non-DT system.
> 
> Because the original bridge driver(say it66121.ko) is fully dependent
> on the DT. UEFI+ACPI based system can not use with it.
>
> Our I2C adapter is created by the drm/loongson.ko on the runtime.
> The potential problem is that *cyclic dependency* !
> 
> I2C adapter driver is depend on drm/loongson
> drm/loongson depend on drm bridge driver (say it66121.ko)
> drm bridge driver (say it66121.ko) depend on I2C adapter to setup.
>
> This plus the defer probe mechanism is totally a trap,
> incurring troubles and don't work.

I'm sure all those issues can be fixed :)

Maxime
  
Sui Jingfeng Oct. 30, 2023, 1:25 p.m. UTC | #5
Hi,


On 2023/10/30 18:01, Dmitry Baryshkov wrote:
> On Mon, 30 Oct 2023 at 11:42, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>> Hi,
>>
>>
>> On 2023/10/30 06:53, Dmitry Baryshkov wrote:
>>> On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>>>> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
>>>> chip to support HDMI output. Thus add a drm bridge based driver for it.
>>>> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
>>> Please use the original bridge driver instead of adding a new one.
>> I'm agree with the spirit of code sharing, but this is nearly impossible for non-DT system.
>>
>> Because the original bridge driver(say it66121.ko) is fully dependent on the DT.
> I can not agree here. It doesn't depend on DT. It has fully populated
> i2c_device_id structures, so it will work with bare I2C buses.
> Most likely you will have to change of calls into fwnode calls, that's it.
>
>> UEFI+ACPI based system can not use with it.
>>
>> Our I2C adapter is created by the drm/loongson.ko on the runtime.
>> The potential problem is that *cyclic dependency* !
>>
>> I2C adapter driver is depend on drm/loongson
>> drm/loongson depend on drm bridge driver (say it66121.ko)
>> drm bridge driver (say it66121.ko) depend on I2C adapter to setup.
>>
>> This plus the defer probe mechanism is totally a trap,
>> incurring troubles and don't work.
> Welcome. We had this kind of issue for DP AUX buses.
>
> I can suggest the following approach:
> - In the root probe function you can create an i2c bus and populate it
> with the i2c devices.
> - I have not checked whether you use components or not. If not, please
> use an auxiliary or a platform device for the main DRM functionality.
> - In the subdevice probe / bind function you check for the next
> bridge. Then you get one of the following:drm_bridge pointer,
> -EPROBE_DEFER, or any other error case. Your driver can react
> accordingly.

I have similar way to solve this problem, and I have solved it one and a half years ago.
See [1] for a reference.

[1] https://patchwork.freedesktop.org/patch/478998/?series=99512&rev=11

When the PCI device get probed, we create the I2C bus first.
This ensure that when drm/lsdc.ko get loaded, the I2C bus is presence
and ready to be get by the drm_bridge driver.
This is basically a PCI-to-GPIO-emulated-I2C adapter,
then wait the display bridges driver get loaded and set up.

I also need to create a virtual platform device for the display controller.
which allow the drm drivers instance for this virtual platform device
be able to probed due to defer probe mechanism.

This solution made the framework of my driver distortion severely,
and in the end we still solve a easy problem by workaround.

I know how to use the component framework also, but the component framework just
a wrapper. Similar with above approach, it brings no gains in the end.
It does not make this driver better. I got trapped one years ago,
and I don't want to got trapped another time.
And I know how solve such a problem by workaround, but that's not worthy for the effort.

I think my approach provide a solution, while still keep the bridges drivers
to a modular at the same time. Despite simple, it indeed solve the problem.
It simple because of explicit control of the loading order by myself, not by
rely on the framework or something else (say component)

It is not totally duplicating, I have rewrite part of them. You can compare
to see what I'm changed. It is just that it66162 was upstream-ed earlier than
our solution. But I also have write display drivers for lt8618 and lt8619
completely by myself.


Even though our local drm bridges driver will not be able to enjoy the updates.
We will accept such a results(or pain). I can maintain our local drm bridges
drivers by myself. Sorry, on this technique point, we will not follow your idea.
I'm sure that my approach is toward to right direction for our device at now.
If someone invent a better solution to handle this problem, which make the
various drm bridges drivers usable out of box, then I will follow and cooperate
to test.
  

> Basically duplicating the existing driver code is not really a way to
> go. Consider somebody adding a new feature or fixing a bug in your
> driver copy. Then they have to check if the fix applies to the driver
> at drivers/gpu/drm/bridge/ite-it66121.c. And vice versa. After fixing
> an issue in the standard driver one has to keep in mind to check your
> private copy.
>
> So, please, use the OF code as an inspiration and register all your
> devices in the device tree. Yes, this requires some effort from your
> side. Yes, this pays off in the longer distance.
>
>>>    If
>>> it needs to be changed in any way, please help everyone else by
>>> improving it instead of introducing new driver.
>>>
>>>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>>> ---
>>>>    drivers/gpu/drm/loongson/Kconfig            |   1 +
>>>>    drivers/gpu/drm/loongson/Makefile           |   2 +
>>>>    drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
>>>>    drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
>>>>    drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
>>>>    5 files changed, 1039 insertions(+)
>>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
>>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
>>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
>
  
Maxime Ripard Oct. 30, 2023, 1:39 p.m. UTC | #6
On Mon, Oct 30, 2023 at 09:25:50PM +0800, Sui Jingfeng wrote:
> I think my approach provide a solution, while still keep the bridges drivers
> to a modular at the same time. Despite simple, it indeed solve the problem.
> It simple because of explicit control of the loading order by myself, not by
> rely on the framework or something else (say component)
> 
> It is not totally duplicating, I have rewrite part of them. You can compare
> to see what I'm changed. It is just that it66162 was upstream-ed earlier than
> our solution. But I also have write display drivers for lt8618 and lt8619
> completely by myself.
>
> Even though our local drm bridges driver will not be able to enjoy the updates.
> We will accept such a results(or pain). I can maintain our local drm bridges
> drivers by myself. Sorry, on this technique point, we will not follow your idea.
> I'm sure that my approach is toward to right direction for our device at now.
> If someone invent a better solution to handle this problem, which make the
> various drm bridges drivers usable out of box, then I will follow and cooperate
> to test.

As far as I'm concerned, the two options are either you reuse the
already existing driver or this series isn't merged.

Ignoring what issue we raised and still merging your patch isn't on the
table, sorry.

Maxime
  
Dmitry Baryshkov Oct. 30, 2023, 1:48 p.m. UTC | #7
On Mon, 30 Oct 2023 at 15:26, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>
> Hi,
>
>
> On 2023/10/30 18:01, Dmitry Baryshkov wrote:
> > On Mon, 30 Oct 2023 at 11:42, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
> >> Hi,
> >>
> >>
> >> On 2023/10/30 06:53, Dmitry Baryshkov wrote:
> >>> On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
> >>>> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
> >>>> chip to support HDMI output. Thus add a drm bridge based driver for it.
> >>>> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
> >>> Please use the original bridge driver instead of adding a new one.
> >> I'm agree with the spirit of code sharing, but this is nearly impossible for non-DT system.
> >>
> >> Because the original bridge driver(say it66121.ko) is fully dependent on the DT.
> > I can not agree here. It doesn't depend on DT. It has fully populated
> > i2c_device_id structures, so it will work with bare I2C buses.
> > Most likely you will have to change of calls into fwnode calls, that's it.
> >
> >> UEFI+ACPI based system can not use with it.
> >>
> >> Our I2C adapter is created by the drm/loongson.ko on the runtime.
> >> The potential problem is that *cyclic dependency* !
> >>
> >> I2C adapter driver is depend on drm/loongson
> >> drm/loongson depend on drm bridge driver (say it66121.ko)
> >> drm bridge driver (say it66121.ko) depend on I2C adapter to setup.
> >>
> >> This plus the defer probe mechanism is totally a trap,
> >> incurring troubles and don't work.
> > Welcome. We had this kind of issue for DP AUX buses.
> >
> > I can suggest the following approach:
> > - In the root probe function you can create an i2c bus and populate it
> > with the i2c devices.
> > - I have not checked whether you use components or not. If not, please
> > use an auxiliary or a platform device for the main DRM functionality.
> > - In the subdevice probe / bind function you check for the next
> > bridge. Then you get one of the following:drm_bridge pointer,
> > -EPROBE_DEFER, or any other error case. Your driver can react
> > accordingly.
>
> I have similar way to solve this problem, and I have solved it one and a half years ago.
> See [1] for a reference.
>
> [1] https://patchwork.freedesktop.org/patch/478998/?series=99512&rev=11
>
> When the PCI device get probed, we create the I2C bus first.
> This ensure that when drm/lsdc.ko get loaded, the I2C bus is presence
> and ready to be get by the drm_bridge driver.
> This is basically a PCI-to-GPIO-emulated-I2C adapter,
> then wait the display bridges driver get loaded and set up.
>
> I also need to create a virtual platform device for the display controller.
> which allow the drm drivers instance for this virtual platform device
> be able to probed due to defer probe mechanism.
>
> This solution made the framework of my driver distortion severely,

I don't think I could catch this phrase. Did you see distortions on the screen?

> and in the end we still solve a easy problem by workaround.

No workarounds for the kernel subsystems are allowed.

>
> I know how to use the component framework also, but the component framework just
> a wrapper. Similar with above approach, it brings no gains in the end.
> It does not make this driver better. I got trapped one years ago,
> and I don't want to got trapped another time.
> And I know how solve such a problem by workaround, but that's not worthy for the effort.
>
> I think my approach provide a solution, while still keep the bridges drivers
> to a modular at the same time. Despite simple, it indeed solve the problem.
> It simple because of explicit control of the loading order by myself, not by
> rely on the framework or something else (say component)

PCI media drivers have had this issue for ages. And all of them found
a way to work.

> It is not totally duplicating, I have rewrite part of them.

This is even worse. Now one can not apply fixes to the second one.

> You can compare
> to see what I'm changed. It is just that it66162 was upstream-ed earlier than
> our solution. But I also have write display drivers for lt8618 and lt8619
> completely by myself.
>
>
> Even though our local drm bridges driver will not be able to enjoy the updates.
> We will accept such a results(or pain). I can maintain our local drm bridges
> drivers by myself.

What happens if anybody wants to reuse your bridge driver for their
own platform?
Linux kernel uses driver model and frameworks to improve code sharing,
not to reduce it.

> Sorry, on this technique point, we will not follow your idea.
> I'm sure that my approach is toward to right direction for our device at now.
> If someone invent a better solution to handle this problem, which make the
> various drm bridges drivers usable out of box, then I will follow and cooperate
> to test.
>
>
> > Basically duplicating the existing driver code is not really a way to
> > go. Consider somebody adding a new feature or fixing a bug in your
> > driver copy. Then they have to check if the fix applies to the driver
> > at drivers/gpu/drm/bridge/ite-it66121.c. And vice versa. After fixing
> > an issue in the standard driver one has to keep in mind to check your
> > private copy.
> >
> > So, please, use the OF code as an inspiration and register all your
> > devices in the device tree. Yes, this requires some effort from your
> > side. Yes, this pays off in the longer distance.
> >
> >>>    If
> >>> it needs to be changed in any way, please help everyone else by
> >>> improving it instead of introducing new driver.
> >>>
> >>>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
> >>>> ---
> >>>>    drivers/gpu/drm/loongson/Kconfig            |   1 +
> >>>>    drivers/gpu/drm/loongson/Makefile           |   2 +
> >>>>    drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
> >>>>    drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
> >>>>    drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
> >>>>    5 files changed, 1039 insertions(+)
> >>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
> >>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
> >>>>    create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
> >
>
  
Sui Jingfeng Oct. 30, 2023, 2:25 p.m. UTC | #8
Hi,


On 2023/10/30 21:48, Dmitry Baryshkov wrote:
> On Mon, 30 Oct 2023 at 15:26, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>> Hi,
>>
>>
>> On 2023/10/30 18:01, Dmitry Baryshkov wrote:
>>> On Mon, 30 Oct 2023 at 11:42, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>>>> Hi,
>>>>
>>>>
>>>> On 2023/10/30 06:53, Dmitry Baryshkov wrote:
>>>>> On Sun, 29 Oct 2023 at 21:46, Sui Jingfeng <suijingfeng@loongson.cn> wrote:
>>>>>> The IT66121 is a DVO to HDMI converter, LS3A5000+LS7A1000 ML5A_MB use this
>>>>>> chip to support HDMI output. Thus add a drm bridge based driver for it.
>>>>>> This patch is developed with drivers/gpu/drm/bridge/ite-it66121.c as base.
>>>>> Please use the original bridge driver instead of adding a new one.
>>>> I'm agree with the spirit of code sharing, but this is nearly impossible for non-DT system.
>>>>
>>>> Because the original bridge driver(say it66121.ko) is fully dependent on the DT.
>>> I can not agree here. It doesn't depend on DT. It has fully populated
>>> i2c_device_id structures, so it will work with bare I2C buses.
>>> Most likely you will have to change of calls into fwnode calls, that's it.
>>>
>>>> UEFI+ACPI based system can not use with it.
>>>>
>>>> Our I2C adapter is created by the drm/loongson.ko on the runtime.
>>>> The potential problem is that *cyclic dependency* !
>>>>
>>>> I2C adapter driver is depend on drm/loongson
>>>> drm/loongson depend on drm bridge driver (say it66121.ko)
>>>> drm bridge driver (say it66121.ko) depend on I2C adapter to setup.
>>>>
>>>> This plus the defer probe mechanism is totally a trap,
>>>> incurring troubles and don't work.
>>> Welcome. We had this kind of issue for DP AUX buses.
>>>
>>> I can suggest the following approach:
>>> - In the root probe function you can create an i2c bus and populate it
>>> with the i2c devices.
>>> - I have not checked whether you use components or not. If not, please
>>> use an auxiliary or a platform device for the main DRM functionality.
>>> - In the subdevice probe / bind function you check for the next
>>> bridge. Then you get one of the following:drm_bridge pointer,
>>> -EPROBE_DEFER, or any other error case. Your driver can react
>>> accordingly.
>> I have similar way to solve this problem, and I have solved it one and a half years ago.
>> See [1] for a reference.
>>
>> [1] https://patchwork.freedesktop.org/patch/478998/?series=99512&rev=11
>>
>> When the PCI device get probed, we create the I2C bus first.
>> This ensure that when drm/lsdc.ko get loaded, the I2C bus is presence
>> and ready to be get by the drm_bridge driver.
>> This is basically a PCI-to-GPIO-emulated-I2C adapter,
>> then wait the display bridges driver get loaded and set up.
>>
>> I also need to create a virtual platform device for the display controller.
>> which allow the drm drivers instance for this virtual platform device
>> be able to probed due to defer probe mechanism.
>>
>> This solution made the framework of my driver distortion severely,
> I don't think I could catch this phrase. Did you see distortions on the screen?


I means that it destroy the my drm driver's framework.
I means that we are all-in-one driver. The solution you
mentioned have side effect also. That is because user-space
will open the PCI device first, not your created virtual platform device.



>> and in the end we still solve a easy problem by workaround.
> No workarounds for the kernel subsystems are allowed.


I means that the idea(solution) you told me is still a workaround.
bring no benifits to the drm driver itself.


>> I know how to use the component framework also, but the component framework just
>> a wrapper. Similar with above approach, it brings no gains in the end.
>> It does not make this driver better. I got trapped one years ago,
>> and I don't want to got trapped another time.
>> And I know how solve such a problem by workaround, but that's not worthy for the effort.
>>
>> I think my approach provide a solution, while still keep the bridges drivers
>> to a modular at the same time. Despite simple, it indeed solve the problem.
>> It simple because of explicit control of the loading order by myself, not by
>> rely on the framework or something else (say component)
> PCI media drivers have had this issue for ages. And all of them found
> a way to work.

I have said that PCI KMS display drivers is different,  because of user 
space open the PCI device.


>> It is not totally duplicating, I have rewrite part of them.
> This is even worse. Now one can not apply fixes to the second one.


I don't need to either, I want to maintain this by myself.


>> You can compare
>> to see what I'm changed. It is just that it66162 was upstream-ed earlier than
>> our solution. But I also have write display drivers for lt8618 and lt8619
>> completely by myself.
>>
>>
>> Even though our local drm bridges driver will not be able to enjoy the updates.
>> We will accept such a results(or pain). I can maintain our local drm bridges
>> drivers by myself.
> What happens if anybody wants to reuse your bridge driver for their
> own platform?

Copy and modify.

> Linux kernel uses driver model and frameworks to improve code sharing,
> not to reduce it.


Well I don't think my patch actually reduce something.
Please see i915, amdgpu, radeon and nouveau.
Non of them use the DRM bridge drivers.
It is just that the various DRM bridge drivers are not suitable to use for my driver.


>> Sorry, on this technique point, we will not follow your idea.
>> I'm sure that my approach is toward to right direction for our device at now.
>> If someone invent a better solution to handle this problem, which make the
>> various drm bridges drivers usable out of box, then I will follow and cooperate
>> to test.
>>
>>
>>> Basically duplicating the existing driver code is not really a way to
>>> go. Consider somebody adding a new feature or fixing a bug in your
>>> driver copy. Then they have to check if the fix applies to the driver
>>> at drivers/gpu/drm/bridge/ite-it66121.c. And vice versa. After fixing
>>> an issue in the standard driver one has to keep in mind to check your
>>> private copy.
>>>
>>> So, please, use the OF code as an inspiration and register all your
>>> devices in the device tree. Yes, this requires some effort from your
>>> side. Yes, this pays off in the longer distance.
>>>
>>>>>     If
>>>>> it needs to be changed in any way, please help everyone else by
>>>>> improving it instead of introducing new driver.
>>>>>
>>>>>> Signed-off-by: Sui Jingfeng <suijingfeng@loongson.cn>
>>>>>> ---
>>>>>>     drivers/gpu/drm/loongson/Kconfig            |   1 +
>>>>>>     drivers/gpu/drm/loongson/Makefile           |   2 +
>>>>>>     drivers/gpu/drm/loongson/ite_it66121.c      | 749 ++++++++++++++++++++
>>>>>>     drivers/gpu/drm/loongson/ite_it66121.h      |  19 +
>>>>>>     drivers/gpu/drm/loongson/ite_it66121_regs.h | 268 +++++++
>>>>>>     5 files changed, 1039 insertions(+)
>>>>>>     create mode 100644 drivers/gpu/drm/loongson/ite_it66121.c
>>>>>>     create mode 100644 drivers/gpu/drm/loongson/ite_it66121.h
>>>>>>     create mode 100644 drivers/gpu/drm/loongson/ite_it66121_regs.h
>
  
Sui Jingfeng Oct. 30, 2023, 2:39 p.m. UTC | #9
Hi,


On 2023/10/30 21:39, Maxime Ripard wrote:
> On Mon, Oct 30, 2023 at 09:25:50PM +0800, Sui Jingfeng wrote:
>> I think my approach provide a solution, while still keep the bridges drivers
>> to a modular at the same time. Despite simple, it indeed solve the problem.
>> It simple because of explicit control of the loading order by myself, not by
>> rely on the framework or something else (say component)
>>
>> It is not totally duplicating, I have rewrite part of them. You can compare
>> to see what I'm changed. It is just that it66162 was upstream-ed earlier than
>> our solution. But I also have write display drivers for lt8618 and lt8619
>> completely by myself.
>>
>> Even though our local drm bridges driver will not be able to enjoy the updates.
>> We will accept such a results(or pain). I can maintain our local drm bridges
>> drivers by myself. Sorry, on this technique point, we will not follow your idea.
>> I'm sure that my approach is toward to right direction for our device at now.
>> If someone invent a better solution to handle this problem, which make the
>> various drm bridges drivers usable out of box, then I will follow and cooperate
>> to test.
> As far as I'm concerned, the two options are either you reuse the
> already existing driver or this series isn't merged.

It's not that I don't want to use thealready existing display bridge driver, It is just that it is not 
suitable for non DT-based system to use. Our system using UEFI+ACPI, 
beside the I2C, there also have GPIO HPD interrupt hardware. ACPI-based system and DT-based system have different way to use(request) the hardware.
Can you feel my words?

If the variousdisplay bridge drivers are really ready to use, why I have to refuse?


> Ignoring what issue we raised and still merging your patch isn't on the
> table, sorry.
>
> Maxime
  
Maxime Ripard Oct. 30, 2023, 2:56 p.m. UTC | #10
On Mon, Oct 30, 2023 at 10:39:32PM +0800, Sui Jingfeng wrote:
> Hi,
> 
> 
> On 2023/10/30 21:39, Maxime Ripard wrote:
> > On Mon, Oct 30, 2023 at 09:25:50PM +0800, Sui Jingfeng wrote:
> > > I think my approach provide a solution, while still keep the bridges drivers
> > > to a modular at the same time. Despite simple, it indeed solve the problem.
> > > It simple because of explicit control of the loading order by myself, not by
> > > rely on the framework or something else (say component)
> > > 
> > > It is not totally duplicating, I have rewrite part of them. You can compare
> > > to see what I'm changed. It is just that it66162 was upstream-ed earlier than
> > > our solution. But I also have write display drivers for lt8618 and lt8619
> > > completely by myself.
> > > 
> > > Even though our local drm bridges driver will not be able to enjoy the updates.
> > > We will accept such a results(or pain). I can maintain our local drm bridges
> > > drivers by myself. Sorry, on this technique point, we will not follow your idea.
> > > I'm sure that my approach is toward to right direction for our device at now.
> > > If someone invent a better solution to handle this problem, which make the
> > > various drm bridges drivers usable out of box, then I will follow and cooperate
> > > to test.
> > As far as I'm concerned, the two options are either you reuse the
> > already existing driver or this series isn't merged.
> 
> It's not that I don't want to use thealready existing display bridge driver,
> It is just that it is not suitable for non DT-based system to use.

The code is there, you can modify it to make it suitable for non
DT-based systems.

> Our system using UEFI+ACPI, beside the I2C, there also have GPIO HPD
> interrupt hardware. ACPI-based system and DT-based system have
> different way to use(request) the hardware. Can you feel my words?

Not really, no. There's plenty of drivers supporting both ACPI and DT
based systems.

> If the variousdisplay bridge drivers are really ready to use

Nobody said they would be ready to use. You are expected to make them
work for you though.

> why I have to refuse?

I mean, you can totally refuse to do whatever we ask. Just like we can
also totally refuse to merge these patches.

Maxime
  

Patch

diff --git a/drivers/gpu/drm/loongson/Kconfig b/drivers/gpu/drm/loongson/Kconfig
index df6946d505fa..a96f5699099e 100644
--- a/drivers/gpu/drm/loongson/Kconfig
+++ b/drivers/gpu/drm/loongson/Kconfig
@@ -7,6 +7,7 @@  config DRM_LOONGSON
 	select DRM_TTM
 	select I2C
 	select I2C_ALGOBIT
+	select REGMAP_I2C
 	help
 	  This is a DRM driver for Loongson Graphics, it may including
 	  LS7A2000, LS7A1000, LS2K2000 and LS2K1000 etc. Loongson LS7A
diff --git a/drivers/gpu/drm/loongson/Makefile b/drivers/gpu/drm/loongson/Makefile
index bef00b2c5569..1459d19b2c90 100644
--- a/drivers/gpu/drm/loongson/Makefile
+++ b/drivers/gpu/drm/loongson/Makefile
@@ -20,4 +20,6 @@  loongson-y += loongson_device.o \
 	      loongson_module.o \
 	      loongson_vbios.o
 
+loongson-y += ite_it66121.o
+
 obj-$(CONFIG_DRM_LOONGSON) += loongson.o
diff --git a/drivers/gpu/drm/loongson/ite_it66121.c b/drivers/gpu/drm/loongson/ite_it66121.c
new file mode 100644
index 000000000000..7b085575f864
--- /dev/null
+++ b/drivers/gpu/drm/loongson/ite_it66121.c
@@ -0,0 +1,749 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 BayLibre, SAS
+ * Author: Phong LE <ple@baylibre.com>
+ * Copyright (C) 2018-2019, Artem Mygaiev
+ * Copyright (C) 2017, Fresco Logic, Incorporated.
+ *
+ * IT66121 HDMI transmitter driver
+ */
+
+#include <linux/media-bus-format.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/bitfield.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+
+#include "ite_it66121.h"
+#include "ite_it66121_regs.h"
+
+#define IT66121_CHIP_NAME                       "IT66121"
+
+struct it66121_bridge {
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+	struct regmap *regmap;
+	struct i2c_client *client;
+	/* Protects fields below and device registers */
+	struct mutex lock;
+	u16 vendor_id;
+	u16 device_id;
+	u32 revision;
+};
+
+static inline struct it66121_bridge *
+bridge_to_it66121(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct it66121_bridge, bridge);
+}
+
+static inline struct it66121_bridge *
+connector_to_it66121(struct drm_connector *connector)
+{
+	return container_of(connector, struct it66121_bridge, connector);
+}
+
+static const struct regmap_range_cfg it66121_regmap_banks[] = {
+	{
+		.name = IT66121_CHIP_NAME,
+		.range_min = 0x00,
+		.range_max = 0x1FF,
+		.selector_reg = IT66121_CLK_BANK_REG,
+		.selector_mask = 0x1,
+		.selector_shift = 0,
+		.window_start = 0x00,
+		.window_len = 0x100,
+	},
+};
+
+static const struct regmap_config it66121_regmap_config = {
+	.val_bits = 8,
+	.reg_bits = 8,
+	.max_register = 0x1FF,
+	.ranges = it66121_regmap_banks,
+	.num_ranges = ARRAY_SIZE(it66121_regmap_banks),
+};
+
+static inline int it66121_preamble_ddc(struct it66121_bridge *itb)
+{
+	return regmap_write(itb->regmap, IT66121_MASTER_SEL_REG,
+			    IT66121_MASTER_SEL_HOST);
+}
+
+static inline int it66121_fire_afe(struct it66121_bridge *itb)
+{
+	return regmap_write(itb->regmap, IT66121_AFE_DRV_REG, 0);
+}
+
+static int it66121_configure_input(struct it66121_bridge *itb)
+{
+	int ret;
+
+	ret = regmap_write(itb->regmap, IT66121_INPUT_MODE_REG,
+			   IT66121_INPUT_MODE_RGB888);
+	if (ret)
+		return ret;
+
+	return regmap_write(itb->regmap, IT66121_INPUT_CSC_REG,
+			    IT66121_INPUT_CSC_NO_CONV);
+}
+
+/*
+ * it66121_configure_afe() - Configure the analog front end
+ * @ctx: it66121_ctx object
+ * @mode: mode to configure
+ *
+ * RETURNS:
+ * zero if success, a negative error code otherwise.
+ */
+static int it66121_configure_afe(struct it66121_bridge *itb,
+				 const struct drm_display_mode *mode)
+{
+	int ret;
+
+	ret = regmap_write(itb->regmap, IT66121_AFE_DRV_REG,
+			   IT66121_AFE_DRV_RST);
+	if (ret)
+		return ret;
+
+	if (mode->clock > IT66121_AFE_CLK_HIGH) {
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_REG,
+					IT66121_AFE_XP_GAINBIT |
+					IT66121_AFE_XP_ENO,
+					IT66121_AFE_XP_GAINBIT);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+					IT66121_AFE_IP_GAINBIT |
+					IT66121_AFE_IP_ER0,
+					IT66121_AFE_IP_GAINBIT);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+					IT66121_AFE_IP_EC1, 0);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_EC1_REG,
+					IT66121_AFE_XP_EC1_LOWCLK, 0x80);
+		if (ret)
+			return ret;
+	} else {
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_REG,
+					IT66121_AFE_XP_GAINBIT |
+					IT66121_AFE_XP_ENO,
+					IT66121_AFE_XP_ENO);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+					IT66121_AFE_IP_GAINBIT |
+					IT66121_AFE_IP_ER0,
+					IT66121_AFE_IP_ER0);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+					IT66121_AFE_IP_EC1,
+					IT66121_AFE_IP_EC1);
+		if (ret)
+			return ret;
+
+		ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_EC1_REG,
+					IT66121_AFE_XP_EC1_LOWCLK,
+					IT66121_AFE_XP_EC1_LOWCLK);
+		if (ret)
+			return ret;
+	}
+
+	/* Clear reset flags */
+	ret = regmap_write_bits(itb->regmap, IT66121_SW_RST_REG,
+				IT66121_SW_RST_REF | IT66121_SW_RST_VID, 0);
+	if (ret)
+		return ret;
+
+	return it66121_fire_afe(itb);
+}
+
+static inline int it66121_wait_ddc_ready(struct it66121_bridge *itb)
+{
+	u32 error = IT66121_DDC_STATUS_NOACK |
+		    IT66121_DDC_STATUS_WAIT_BUS |
+		    IT66121_DDC_STATUS_ARBI_LOSE;
+	u32 done = IT66121_DDC_STATUS_TX_DONE;
+	int ret, val;
+
+	ret = regmap_read_poll_timeout(itb->regmap, IT66121_DDC_STATUS_REG,
+				       val, val & (error | done),
+				       IT66121_EDID_SLEEP_US,
+				       IT66121_EDID_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	if (val & error)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static int it66121_abort_ddc_ops(struct it66121_bridge *itb)
+{
+	unsigned int swreset, cpdesire;
+	int ret;
+
+	ret = regmap_read(itb->regmap, IT66121_SW_RST_REG, &swreset);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(itb->regmap, IT66121_HDCP_REG, &cpdesire);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(itb->regmap, IT66121_HDCP_REG,
+			   cpdesire & (~IT66121_HDCP_CPDESIRED & 0xFF));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(itb->regmap, IT66121_SW_RST_REG,
+			   (swreset | IT66121_SW_RST_HDCP));
+	if (ret)
+		return ret;
+
+	ret = it66121_preamble_ddc(itb);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(itb->regmap, IT66121_DDC_COMMAND_REG,
+			   IT66121_DDC_COMMAND_ABORT);
+	if (ret)
+		return ret;
+
+	return it66121_wait_ddc_ready(itb);
+}
+
+static int it66121_get_edid_block(void *context,
+				  u8 *buf,
+				  unsigned int block,
+				  size_t len)
+{
+	struct it66121_bridge *itb = (struct it66121_bridge *)context;
+	int remain = len;
+	int offset = 0;
+	int ret, cnt;
+
+	offset = (block % 2) * len;
+	block = block / 2;
+
+	while (remain > 0) {
+		cnt = (remain > IT66121_EDID_FIFO_SIZE) ?
+				IT66121_EDID_FIFO_SIZE : remain;
+
+		ret = regmap_write(itb->regmap, IT66121_DDC_COMMAND_REG,
+				   IT66121_DDC_COMMAND_FIFO_CLR);
+		if (ret)
+			return ret;
+
+		ret = it66121_wait_ddc_ready(itb);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(itb->regmap, IT66121_DDC_OFFSET_REG, offset);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(itb->regmap, IT66121_DDC_BYTE_REG, cnt);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(itb->regmap, IT66121_DDC_SEGMENT_REG, block);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(itb->regmap, IT66121_DDC_COMMAND_REG,
+				   IT66121_DDC_COMMAND_EDID_READ);
+		if (ret)
+			return ret;
+
+		offset += cnt;
+		remain -= cnt;
+
+		ret = it66121_wait_ddc_ready(itb);
+		if (ret) {
+			it66121_abort_ddc_ops(itb);
+			return ret;
+		}
+
+		ret = regmap_noinc_read(itb->regmap, IT66121_DDC_RD_FIFO_REG,
+					buf, cnt);
+		if (ret)
+			return ret;
+
+		buf += cnt;
+	}
+
+	return 0;
+}
+
+static bool it66121_is_hpd_detect(struct it66121_bridge *itb)
+{
+	int val;
+
+	if (regmap_read(itb->regmap, IT66121_SYS_STATUS_REG, &val))
+		return false;
+
+	return val & IT66121_SYS_STATUS_HPDETECT;
+}
+
+static int it66121_connector_get_modes(struct drm_connector *connector)
+{
+	struct it66121_bridge *itb = connector_to_it66121(connector);
+	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	int num_modes = 0;
+	struct edid *edid;
+	int ret;
+
+	edid = drm_bridge_get_edid(&itb->bridge, connector);
+	if (!edid) {
+		drm_err(connector->dev, "Failed to read EDID\n");
+		goto failed;
+	}
+
+	if (drm_connector_update_edid_property(connector, edid)) {
+		drm_err(connector->dev, "Failed to update EDID\n");
+		goto failed;
+	}
+
+	ret = drm_display_info_set_bus_formats(&connector->display_info,
+					       &bus_format, 1);
+	if (ret)
+		goto failed;
+
+	num_modes = drm_add_edid_modes(connector, edid);
+
+failed:
+	return num_modes;
+}
+
+static int it66121_connector_detect_ctx(struct drm_connector *connector,
+					struct drm_modeset_acquire_ctx *ctx,
+					bool force)
+{
+	struct it66121_bridge *itb = connector_to_it66121(connector);
+
+	return it66121_is_hpd_detect(itb) ? connector_status_connected
+					  : connector_status_disconnected;
+}
+
+static struct drm_connector_helper_funcs it66121_connector_helper_funcs = {
+	.get_modes = it66121_connector_get_modes,
+	.detect_ctx = it66121_connector_detect_ctx,
+};
+
+static const struct drm_connector_funcs it66121_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int it66121_bridge_connector_init(struct drm_bridge *bridge)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	struct drm_connector *connector = &itb->connector;
+	int ret;
+
+	if (bridge->ops & DRM_BRIDGE_OP_HPD) {
+		connector->polled = DRM_CONNECTOR_POLL_HPD;
+	} else {
+		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+				    DRM_CONNECTOR_POLL_DISCONNECT;
+	}
+
+	ret = drm_connector_init(bridge->dev,
+				 connector,
+				 &it66121_connector_funcs,
+				 bridge->type);
+	if (ret)
+		return ret;
+
+	drm_connector_helper_add(connector, &it66121_connector_helper_funcs);
+
+	drm_connector_attach_encoder(connector, bridge->encoder);
+
+	return 0;
+}
+
+static int it66121_bridge_attach(struct drm_bridge *bridge,
+				 enum drm_bridge_attach_flags flags)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	int ret;
+
+	ret = it66121_bridge_connector_init(bridge);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_CLK_BANK_REG,
+				IT66121_CLK_BANK_PWROFF_RCLK, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_INT_REG,
+				IT66121_INT_TX_CLK_OFF, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_DRV_REG,
+				IT66121_AFE_DRV_PWD, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_REG,
+				IT66121_AFE_XP_PWDI | IT66121_AFE_XP_PWDPLL, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+				IT66121_AFE_IP_PWDPLL, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_DRV_REG,
+				IT66121_AFE_DRV_RST, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_XP_REG,
+				IT66121_AFE_XP_RESETB, IT66121_AFE_XP_RESETB);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_AFE_IP_REG,
+				IT66121_AFE_IP_RESETB, IT66121_AFE_IP_RESETB);
+	if (ret)
+		return ret;
+
+	ret = regmap_write_bits(itb->regmap, IT66121_SW_RST_REG,
+				IT66121_SW_RST_REF,
+				IT66121_SW_RST_REF);
+	if (ret)
+		return ret;
+
+	drm_info(bridge->dev,
+		 "IT66121 attached, Vendor ID: 0x%x, Device ID: 0x%x\n",
+		 itb->vendor_id, itb->device_id);
+
+	/* Per programming manual, sleep here for bridge to settle */
+	msleep(50);
+
+	return 0;
+}
+
+static void it66121_bridge_enable(struct drm_bridge *bridge,
+				  struct drm_bridge_state *state)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	struct regmap *regmap = itb->regmap;
+	int ret;
+
+	ret = regmap_clear_bits(regmap, IT66121_AVMUTE_REG, IT66121_AVMUTE_BIT);
+	if (ret)
+		drm_err(bridge->dev, "Enable it66121 bridge failed");
+
+	regmap_write(regmap, IT66121_PKT_GEN_CTRL_REG,
+		     IT66121_PKT_GEN_CTRL_ON | IT66121_PKT_GEN_CTRL_RPT);
+}
+
+static void it66121_bridge_disable(struct drm_bridge *bridge,
+				   struct drm_bridge_state *bridge_state)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	struct regmap *regmap = itb->regmap;
+	int ret;
+
+	ret = regmap_set_bits(regmap, IT66121_AVMUTE_REG, IT66121_AVMUTE_BIT);
+	if (ret)
+		drm_err(bridge->dev, "Disable it66121 bridge failed");
+
+	regmap_write(regmap, IT66121_PKT_GEN_CTRL_REG,
+		     IT66121_PKT_GEN_CTRL_ON | IT66121_PKT_GEN_CTRL_RPT);
+}
+
+static void it66121_bridge_mode_set(struct drm_bridge *bridge,
+				    const struct drm_display_mode *mode,
+				    const struct drm_display_mode *adj_mode)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	struct hdmi_avi_infoframe avi_infoframe;
+	u8 av_buf[HDMI_INFOFRAME_SIZE(AVI)];
+	int ret;
+
+	mutex_lock(&itb->lock);
+
+	hdmi_avi_infoframe_init(&avi_infoframe);
+
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_infoframe,
+						       &itb->connector,
+						       adj_mode);
+	if (ret) {
+		drm_err(bridge->dev, "Failed to setup AVI infoframe\n");
+		goto unlock;
+	}
+
+	ret = hdmi_avi_infoframe_pack(&avi_infoframe, av_buf, sizeof(av_buf));
+	if (ret < 0) {
+		drm_err(bridge->dev, "Failed to pack infoframe\n");
+		goto unlock;
+	}
+
+	/* Write new AVI infoframe packet */
+	ret = regmap_bulk_write(itb->regmap, IT66121_AVIINFO_DB1_REG,
+				&av_buf[HDMI_INFOFRAME_HEADER_SIZE],
+				HDMI_AVI_INFOFRAME_SIZE);
+	if (ret)
+		goto unlock;
+
+	if (regmap_write(itb->regmap, IT66121_AVIINFO_CSUM_REG, av_buf[3]))
+		goto unlock;
+
+	/* Enable AVI infoframe */
+	if (regmap_write(itb->regmap, IT66121_AVI_INFO_PKT_REG,
+			 IT66121_AVI_INFO_PKT_ON | IT66121_AVI_INFO_PKT_RPT))
+		goto unlock;
+
+	/* Set TX mode to HDMI */
+	if (regmap_write(itb->regmap, IT66121_HDMI_MODE_REG, IT66121_HDMI_MODE_HDMI))
+		goto unlock;
+
+	if (regmap_write_bits(itb->regmap, IT66121_CLK_BANK_REG,
+			      IT66121_CLK_BANK_PWROFF_TXCLK,
+			      IT66121_CLK_BANK_PWROFF_TXCLK))
+		goto unlock;
+
+	if (it66121_configure_input(itb))
+		goto unlock;
+
+	if (it66121_configure_afe(itb, adj_mode))
+		goto unlock;
+
+	if (regmap_write_bits(itb->regmap, IT66121_CLK_BANK_REG,
+			      IT66121_CLK_BANK_PWROFF_TXCLK, 0))
+		goto unlock;
+
+unlock:
+	mutex_unlock(&itb->lock);
+}
+
+static enum drm_mode_status
+it66121_bridge_mode_valid(struct drm_bridge *bridge,
+			  const struct drm_display_info *info,
+			  const struct drm_display_mode *mode)
+{
+	if (mode->clock > 148500)
+		return MODE_CLOCK_HIGH;
+
+	if (mode->clock < 25000)
+		return MODE_CLOCK_LOW;
+
+	return MODE_OK;
+}
+
+static struct edid *it66121_bridge_get_edid(struct drm_bridge *bridge,
+					    struct drm_connector *connector)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+	struct edid *edid;
+	int ret;
+
+	mutex_lock(&itb->lock);
+	ret = it66121_preamble_ddc(itb);
+	if (ret) {
+		edid = NULL;
+		goto unlock;
+	}
+
+	ret = regmap_write(itb->regmap, IT66121_DDC_HEADER_REG,
+			   IT66121_DDC_HEADER_EDID);
+	if (ret) {
+		edid = NULL;
+		goto unlock;
+	}
+
+	edid = drm_do_get_edid(connector, it66121_get_edid_block, itb);
+
+unlock:
+	mutex_unlock(&itb->lock);
+
+	return edid;
+}
+
+static void it66121_bridge_detach(struct drm_bridge *bridge)
+{
+	struct it66121_bridge *itb = bridge_to_it66121(bridge);
+
+	mutex_destroy(&itb->lock);
+
+	i2c_unregister_device(itb->client);
+
+	drm_bridge_remove(bridge);
+}
+
+static const struct drm_bridge_funcs it66121_bridge_funcs = {
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+	.attach = it66121_bridge_attach,
+	.detach = it66121_bridge_detach,
+	.atomic_enable = it66121_bridge_enable,
+	.atomic_disable = it66121_bridge_disable,
+	.mode_set = it66121_bridge_mode_set,
+	.mode_valid = it66121_bridge_mode_valid,
+	.get_edid = it66121_bridge_get_edid,
+};
+
+static void it66121_bridge_get_version(struct it66121_bridge *itb)
+{
+	u32 vendor_ids[2] = { 0 };
+	u32 device_ids[2] = { 0 };
+
+	regmap_read(itb->regmap, IT66121_VENDOR_ID0_REG, &vendor_ids[0]);
+	regmap_read(itb->regmap, IT66121_VENDOR_ID1_REG, &vendor_ids[1]);
+	regmap_read(itb->regmap, IT66121_DEVICE_ID0_REG, &device_ids[0]);
+	regmap_read(itb->regmap, IT66121_DEVICE_ID1_REG, &device_ids[1]);
+
+	/* Revision is shared with DEVICE_ID1 */
+	itb->revision = FIELD_GET(IT66121_REVISION_MASK, device_ids[1]);
+	device_ids[1] &= IT66121_DEVICE_ID1_MASK;
+
+	itb->vendor_id = vendor_ids[1] << 8 | vendor_ids[0];
+	itb->device_id = device_ids[1] << 8 | device_ids[0];
+}
+
+static void it66121_bridge_init_base(struct it66121_bridge *itb, bool hpd)
+{
+	struct drm_bridge *bridge = &itb->bridge;
+
+	bridge->funcs = &it66121_bridge_funcs;
+	bridge->type = DRM_MODE_CONNECTOR_HDMIA;
+	bridge->ops = DRM_BRIDGE_OP_EDID;
+
+	if (hpd)
+		bridge->ops |= DRM_BRIDGE_OP_HPD;
+
+	drm_bridge_add(bridge);
+}
+
+/*
+ * The device address is 0x98 if PCADR pin is pulled low, 0x98 >> 1 = 0x4c
+ * The device address is 0x9A if PCADR pin is pulled high, 0x9A >> 1 = 0x4d
+ */
+static bool it66121_probe_slave(struct drm_device *ddev,
+				struct i2c_adapter *adapter,
+				u8 *addr)
+{
+	struct i2c_msg msg = {
+		.len = 0,
+	};
+	int num = 3;
+	int count;
+	int i;
+
+	/* Try slave address 0x4c */
+	msg.addr = 0x4c;
+	count = 0;
+	for (i = 0; i < num; i++) {
+		count += i2c_transfer(adapter, &msg, 1);
+		udelay(9);
+	}
+
+	if (count == num) {
+		*addr = 0x4c;
+		return true;
+	}
+
+	/* Try slave address 0x4d */
+	msg.addr = 0x4d;
+	count = 0;
+	for (i = 0; i < num; i++) {
+		count += i2c_transfer(adapter, &msg, 1);
+		udelay(9);
+	}
+
+	if (count == num) {
+		*addr = 0x4d;
+		return true;
+	}
+
+	drm_err(ddev, "No reliable slave i2c device found\n");
+
+	/*
+	 * If no reliable slave i2c device found, we would like drop the
+	 * support.
+	 */
+	return false;
+}
+
+struct drm_bridge *it66121_bridge_create(struct drm_device *ddev,
+					 struct i2c_adapter *i2c,
+					 u8 addr,
+					 bool enable_hpd,
+					 u32 int_gpio,
+					 unsigned int pipe)
+{
+	struct i2c_board_info it66121_board_info = {
+		.type = IT66121_CHIP_NAME,
+	};
+	struct it66121_bridge *itb;
+	struct i2c_client *client;
+	u8 addr_probed;
+
+	if (!it66121_probe_slave(ddev, i2c, &addr_probed))
+		return NULL;
+
+	if (addr != addr_probed) {
+		drm_warn(ddev, "device address(0x%x) is not correct\n", addr);
+		addr = addr_probed;
+	}
+
+	it66121_board_info.addr = addr;
+
+	itb = devm_kzalloc(ddev->dev, sizeof(*itb), GFP_KERNEL);
+	if (!itb)
+		return NULL;
+
+	client = i2c_new_client_device(i2c, &it66121_board_info);
+	if (IS_ERR(client))
+		return NULL;
+
+	drm_info(ddev, "i2c client %s@0x%02x created\n",
+		 it66121_board_info.type, it66121_board_info.addr);
+
+	itb->client = client;
+
+	i2c_set_clientdata(client, itb);
+
+	mutex_init(&itb->lock);
+
+	itb->regmap = devm_regmap_init_i2c(client, &it66121_regmap_config);
+	if (IS_ERR(itb->regmap)) {
+		drm_err(ddev, "Failed to map registers\n");
+		return NULL;
+	}
+
+	it66121_bridge_get_version(itb);
+
+	it66121_bridge_init_base(itb, enable_hpd);
+
+	return &itb->bridge;
+}
diff --git a/drivers/gpu/drm/loongson/ite_it66121.h b/drivers/gpu/drm/loongson/ite_it66121.h
new file mode 100644
index 000000000000..c3e26cce1b02
--- /dev/null
+++ b/drivers/gpu/drm/loongson/ite_it66121.h
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2023 Loongson Technology Corporation Limited
+ */
+
+#ifndef __ITE_IT66121_H__
+#define __ITE_IT66121_H__
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_device.h>
+
+struct drm_bridge *it66121_bridge_create(struct drm_device *ddev,
+					 struct i2c_adapter *i2c,
+					 u8 addr,
+					 bool enable_hpd,
+					 u32 int_gpio,
+					 unsigned int pipe);
+
+#endif
diff --git a/drivers/gpu/drm/loongson/ite_it66121_regs.h b/drivers/gpu/drm/loongson/ite_it66121_regs.h
new file mode 100644
index 000000000000..57118a4501c1
--- /dev/null
+++ b/drivers/gpu/drm/loongson/ite_it66121_regs.h
@@ -0,0 +1,268 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+/*
+ * Copyright (C) 2020 BayLibre, SAS
+ * Author: Phong LE <ple@baylibre.com>
+ * Copyright (C) 2018-2019, Artem Mygaiev
+ * Copyright (C) 2017, Fresco Logic, Incorporated.
+ */
+
+#ifndef __ITE_IT66121_h__
+#define __ITE_IT66121_h__
+
+#define IT66121_VENDOR_ID0_REG			0x00
+#define IT66121_VENDOR_ID1_REG			0x01
+#define IT66121_DEVICE_ID0_REG			0x02
+#define IT66121_DEVICE_ID1_REG			0x03
+
+#define IT66121_REVISION_MASK			GENMASK(7, 4)
+#define IT66121_DEVICE_ID1_MASK			GENMASK(3, 0)
+
+#define IT66121_MASTER_SEL_REG			0x10
+#define IT66121_MASTER_SEL_HOST			BIT(0)
+
+#define IT66121_AFE_DRV_REG			0x61
+#define IT66121_AFE_DRV_RST			BIT(4)
+#define IT66121_AFE_DRV_PWD			BIT(5)
+
+#define IT66121_INPUT_MODE_REG			0x70
+#define IT66121_INPUT_MODE_RGB888		(0 << 6)
+#define IT66121_INPUT_MODE_YUV422		BIT(6)
+#define IT66121_INPUT_MODE_YUV444		(2 << 6)
+#define IT66121_INPUT_MODE_CCIR656		BIT(4)
+#define IT66121_INPUT_MODE_SYNCEMB		BIT(3)
+#define IT66121_INPUT_MODE_DDR			BIT(2)
+
+#define IT66121_INPUT_CSC_REG			0x72
+#define IT66121_INPUT_CSC_ENDITHER		BIT(7)
+#define IT66121_INPUT_CSC_ENUDFILTER		BIT(6)
+#define IT66121_INPUT_CSC_DNFREE_GO		BIT(5)
+#define IT66121_INPUT_CSC_RGB_TO_YUV		0x02
+#define IT66121_INPUT_CSC_YUV_TO_RGB		0x03
+#define IT66121_INPUT_CSC_NO_CONV		0x00
+
+#define IT66121_AFE_XP_REG			0x62
+#define IT66121_AFE_XP_GAINBIT			BIT(7)
+#define IT66121_AFE_XP_PWDPLL			BIT(6)
+#define IT66121_AFE_XP_ENI			BIT(5)
+#define IT66121_AFE_XP_ENO			BIT(4)
+#define IT66121_AFE_XP_RESETB			BIT(3)
+#define IT66121_AFE_XP_PWDI			BIT(2)
+#define IT6610_AFE_XP_BYPASS			BIT(0)
+
+#define IT66121_AFE_IP_REG			0x64
+#define IT66121_AFE_IP_GAINBIT			BIT(7)
+#define IT66121_AFE_IP_PWDPLL			BIT(6)
+#define IT66121_AFE_IP_CKSEL_05			(0 << 4)
+#define IT66121_AFE_IP_CKSEL_1			BIT(4)
+#define IT66121_AFE_IP_CKSEL_2			(2 << 4)
+#define IT66121_AFE_IP_CKSEL_2OR4		(3 << 4)
+#define IT66121_AFE_IP_ER0			BIT(3)
+#define IT66121_AFE_IP_RESETB			BIT(2)
+#define IT66121_AFE_IP_ENC			BIT(1)
+#define IT66121_AFE_IP_EC1			BIT(0)
+
+#define IT66121_AFE_XP_EC1_REG			0x68
+#define IT66121_AFE_XP_EC1_LOWCLK		BIT(4)
+
+#define IT66121_SW_RST_REG			0x04
+#define IT66121_SW_RST_REF			BIT(5)
+#define IT66121_SW_RST_AREF			BIT(4)
+#define IT66121_SW_RST_VID			BIT(3)
+#define IT66121_SW_RST_AUD			BIT(2)
+#define IT66121_SW_RST_HDCP			BIT(0)
+
+#define IT66121_DDC_COMMAND_REG			0x15
+#define IT66121_DDC_COMMAND_BURST_READ		0x0
+#define IT66121_DDC_COMMAND_EDID_READ		0x3
+#define IT66121_DDC_COMMAND_FIFO_CLR		0x9
+#define IT66121_DDC_COMMAND_SCL_PULSE		0xA
+#define IT66121_DDC_COMMAND_ABORT		0xF
+
+#define IT66121_HDCP_REG			0x20
+#define IT66121_HDCP_CPDESIRED			BIT(0)
+#define IT66121_HDCP_EN1P1FEAT			BIT(1)
+
+#define IT66121_INT_STATUS1_REG			0x06
+#define IT66121_INT_STATUS1_AUD_OVF		BIT(7)
+#define IT66121_INT_STATUS1_DDC_NOACK		BIT(5)
+#define IT66121_INT_STATUS1_DDC_FIFOERR		BIT(4)
+#define IT66121_INT_STATUS1_DDC_BUSHANG		BIT(2)
+#define IT66121_INT_STATUS1_RX_SENS_STATUS	BIT(1)
+#define IT66121_INT_STATUS1_HPD_STATUS		BIT(0)
+
+#define IT66121_DDC_HEADER_REG			0x11
+#define IT66121_DDC_HEADER_HDCP			0x74
+#define IT66121_DDC_HEADER_EDID			0xA0
+
+#define IT66121_DDC_OFFSET_REG			0x12
+#define IT66121_DDC_BYTE_REG			0x13
+#define IT66121_DDC_SEGMENT_REG			0x14
+#define IT66121_DDC_RD_FIFO_REG			0x17
+
+#define IT66121_CLK_BANK_REG			0x0F
+#define IT66121_CLK_BANK_PWROFF_RCLK		BIT(6)
+#define IT66121_CLK_BANK_PWROFF_ACLK		BIT(5)
+#define IT66121_CLK_BANK_PWROFF_TXCLK		BIT(4)
+#define IT66121_CLK_BANK_PWROFF_CRCLK		BIT(3)
+#define IT66121_CLK_BANK_0			0
+#define IT66121_CLK_BANK_1			1
+
+#define IT66121_INT_REG				0x05
+#define IT66121_INT_ACTIVE_HIGH			BIT(7)
+#define IT66121_INT_OPEN_DRAIN			BIT(6)
+#define IT66121_INT_TX_CLK_OFF			BIT(0)
+
+#define IT66121_INT_MASK1_REG			0x09
+#define IT66121_INT_MASK1_AUD_OVF		BIT(7)
+#define IT66121_INT_MASK1_DDC_NOACK		BIT(5)
+#define IT66121_INT_MASK1_DDC_FIFOERR		BIT(4)
+#define IT66121_INT_MASK1_DDC_BUSHANG		BIT(2)
+#define IT66121_INT_MASK1_RX_SENS		BIT(1)
+#define IT66121_INT_MASK1_HPD			BIT(0)
+
+#define IT66121_INT_CLR1_REG			0x0C
+#define IT66121_INT_CLR1_PKTACP			BIT(7)
+#define IT66121_INT_CLR1_PKTNULL		BIT(6)
+#define IT66121_INT_CLR1_PKTGEN			BIT(5)
+#define IT66121_INT_CLR1_KSVLISTCHK		BIT(4)
+#define IT66121_INT_CLR1_AUTHDONE		BIT(3)
+#define IT66121_INT_CLR1_AUTHFAIL		BIT(2)
+#define IT66121_INT_CLR1_RX_SENS		BIT(1)
+#define IT66121_INT_CLR1_HPD			BIT(0)
+
+#define IT66121_AVMUTE_REG			0xC1
+#define IT66121_AVMUTE_BIT			BIT(0)
+#define IT66121_AVMUTE_BLUESCR			BIT(1)
+
+#define IT66121_PKT_CTS_CTRL_REG		0xC5
+#define IT66121_PKT_CTS_CTRL_SEL		BIT(1)
+
+#define IT66121_PKT_GEN_CTRL_REG		0xC6
+#define IT66121_PKT_GEN_CTRL_ON			BIT(0)
+#define IT66121_PKT_GEN_CTRL_RPT		BIT(1)
+
+#define IT66121_AVIINFO_DB1_REG			0x158
+#define IT66121_AVIINFO_DB2_REG			0x159
+#define IT66121_AVIINFO_DB3_REG			0x15A
+#define IT66121_AVIINFO_DB4_REG			0x15B
+#define IT66121_AVIINFO_DB5_REG			0x15C
+#define IT66121_AVIINFO_CSUM_REG		0x15D
+#define IT66121_AVIINFO_DB6_REG			0x15E
+#define IT66121_AVIINFO_DB7_REG			0x15F
+#define IT66121_AVIINFO_DB8_REG			0x160
+#define IT66121_AVIINFO_DB9_REG			0x161
+#define IT66121_AVIINFO_DB10_REG		0x162
+#define IT66121_AVIINFO_DB11_REG		0x163
+#define IT66121_AVIINFO_DB12_REG		0x164
+#define IT66121_AVIINFO_DB13_REG		0x165
+
+#define IT66121_AVI_INFO_PKT_REG		0xCD
+#define IT66121_AVI_INFO_PKT_ON			BIT(0)
+#define IT66121_AVI_INFO_PKT_RPT		BIT(1)
+
+#define IT66121_HDMI_MODE_REG			0xC0
+#define IT66121_HDMI_MODE_HDMI			BIT(0)
+
+#define IT66121_SYS_STATUS_REG			0x0E
+#define IT66121_SYS_STATUS_ACTIVE_IRQ		BIT(7)
+#define IT66121_SYS_STATUS_HPDETECT		BIT(6)
+#define IT66121_SYS_STATUS_SENDECTECT		BIT(5)
+#define IT66121_SYS_STATUS_VID_STABLE		BIT(4)
+#define IT66121_SYS_STATUS_AUD_CTS_CLR		BIT(1)
+#define IT66121_SYS_STATUS_CLEAR_IRQ		BIT(0)
+
+#define IT66121_DDC_STATUS_REG			0x16
+#define IT66121_DDC_STATUS_TX_DONE		BIT(7)
+#define IT66121_DDC_STATUS_ACTIVE		BIT(6)
+#define IT66121_DDC_STATUS_NOACK		BIT(5)
+#define IT66121_DDC_STATUS_WAIT_BUS		BIT(4)
+#define IT66121_DDC_STATUS_ARBI_LOSE		BIT(3)
+#define IT66121_DDC_STATUS_FIFO_FULL		BIT(2)
+#define IT66121_DDC_STATUS_FIFO_EMPTY		BIT(1)
+#define IT66121_DDC_STATUS_FIFO_VALID		BIT(0)
+
+#define IT66121_EDID_SLEEP_US			20000
+#define IT66121_EDID_TIMEOUT_US			200000
+#define IT66121_EDID_FIFO_SIZE			32
+
+#define IT66121_CLK_CTRL0_REG			0x58
+#define IT66121_CLK_CTRL0_AUTO_OVER_SAMPLING	BIT(4)
+#define IT66121_CLK_CTRL0_EXT_MCLK_MASK		GENMASK(3, 2)
+#define IT66121_CLK_CTRL0_EXT_MCLK_128FS	(0 << 2)
+#define IT66121_CLK_CTRL0_EXT_MCLK_256FS	BIT(2)
+#define IT66121_CLK_CTRL0_EXT_MCLK_512FS	(2 << 2)
+#define IT66121_CLK_CTRL0_EXT_MCLK_1024FS	(3 << 2)
+#define IT66121_CLK_CTRL0_AUTO_IPCLK		BIT(0)
+#define IT66121_CLK_STATUS1_REG			0x5E
+#define IT66121_CLK_STATUS2_REG			0x5F
+
+#define IT66121_AUD_CTRL0_REG			0xE0
+#define IT66121_AUD_SWL				(3 << 6)
+#define IT66121_AUD_16BIT			(0 << 6)
+#define IT66121_AUD_18BIT			BIT(6)
+#define IT66121_AUD_20BIT			(2 << 6)
+#define IT66121_AUD_24BIT			(3 << 6)
+#define IT66121_AUD_SPDIFTC			BIT(5)
+#define IT66121_AUD_SPDIF			BIT(4)
+#define IT66121_AUD_I2S				(0 << 4)
+#define IT66121_AUD_EN_I2S3			BIT(3)
+#define IT66121_AUD_EN_I2S2			BIT(2)
+#define IT66121_AUD_EN_I2S1			BIT(1)
+#define IT66121_AUD_EN_I2S0			BIT(0)
+#define IT66121_AUD_CTRL0_AUD_SEL		BIT(4)
+
+#define IT66121_AUD_CTRL1_REG			0xE1
+#define IT66121_AUD_FIFOMAP_REG			0xE2
+#define IT66121_AUD_CTRL3_REG			0xE3
+#define IT66121_AUD_SRCVALID_FLAT_REG		0xE4
+#define IT66121_AUD_FLAT_SRC0			BIT(4)
+#define IT66121_AUD_FLAT_SRC1			BIT(5)
+#define IT66121_AUD_FLAT_SRC2			BIT(6)
+#define IT66121_AUD_FLAT_SRC3			BIT(7)
+#define IT66121_AUD_HDAUDIO_REG			0xE5
+
+#define IT66121_AUD_PKT_CTS0_REG		0x130
+#define IT66121_AUD_PKT_CTS1_REG		0x131
+#define IT66121_AUD_PKT_CTS2_REG		0x132
+#define IT66121_AUD_PKT_N0_REG			0x133
+#define IT66121_AUD_PKT_N1_REG			0x134
+#define IT66121_AUD_PKT_N2_REG			0x135
+
+#define IT66121_AUD_CHST_MODE_REG		0x191
+#define IT66121_AUD_CHST_CAT_REG		0x192
+#define IT66121_AUD_CHST_SRCNUM_REG		0x193
+#define IT66121_AUD_CHST_CHTNUM_REG		0x194
+#define IT66121_AUD_CHST_CA_FS_REG		0x198
+#define IT66121_AUD_CHST_OFS_WL_REG		0x199
+
+#define IT66121_AUD_PKT_CTS_CNT0_REG		0x1A0
+#define IT66121_AUD_PKT_CTS_CNT1_REG		0x1A1
+#define IT66121_AUD_PKT_CTS_CNT2_REG		0x1A2
+
+#define IT66121_AUD_FS_22P05K			0x4
+#define IT66121_AUD_FS_44P1K			0x0
+#define IT66121_AUD_FS_88P2K			0x8
+#define IT66121_AUD_FS_176P4K			0xC
+#define IT66121_AUD_FS_24K			0x6
+#define IT66121_AUD_FS_48K			0x2
+#define IT66121_AUD_FS_96K			0xA
+#define IT66121_AUD_FS_192K			0xE
+#define IT66121_AUD_FS_768K			0x9
+#define IT66121_AUD_FS_32K			0x3
+#define IT66121_AUD_FS_OTHER			0x1
+
+#define IT66121_AUD_SWL_21BIT			0xD
+#define IT66121_AUD_SWL_24BIT			0xB
+#define IT66121_AUD_SWL_23BIT			0x9
+#define IT66121_AUD_SWL_22BIT			0x5
+#define IT66121_AUD_SWL_20BIT			0x3
+#define IT66121_AUD_SWL_17BIT			0xC
+#define IT66121_AUD_SWL_19BIT			0x8
+#define IT66121_AUD_SWL_18BIT			0x4
+#define IT66121_AUD_SWL_16BIT			0x2
+#define IT66121_AUD_SWL_NOT_INDICATED		0x0
+
+#define IT66121_AFE_CLK_HIGH			80000 /* Khz */
+
+#endif